Skip to content

Commit ca0ebe3

Browse files
committed
test: fix policy-deny tests
1 parent 49c43b6 commit ca0ebe3

11 files changed

+160
-249
lines changed

doc/api/cli.md

+11
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,16 @@ unless either the `--pending-deprecation` command-line flag, or the
832832
are used to provide a kind of selective "early warning" mechanism that
833833
developers may leverage to detect deprecated API usage.
834834

835+
### `--policy-deny-fs`
836+
837+
<!-- YAML
838+
added: REPLACEME
839+
-->
840+
841+
> Stability: 1 - Experimental
842+
843+
TODO
844+
835845
### `--policy-integrity=sri`
836846

837847
<!-- YAML
@@ -1703,6 +1713,7 @@ Node.js options that are allowed are:
17031713
* `--openssl-legacy-provider`
17041714
* `--openssl-shared-config`
17051715
* `--pending-deprecation`
1716+
* `--policy-deny-fs`
17061717
* `--policy-integrity`
17071718
* `--preserve-symlinks-main`
17081719
* `--preserve-symlinks`

doc/node.1

+3
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,9 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
315315
.It Fl -pending-deprecation
316316
Emit pending deprecation warnings.
317317
.
318+
.It Fl -policy-deny-fs
319+
Instructs Node.js to restrict access to the given resource type
320+
.
318321
.It Fl -policy-integrity Ns = Ns Ar sri
319322
Instructs Node.js to error prior to running any code if the policy does not have the specified integrity. It expects a Subresource Integrity string as a parameter.
320323
.

lib/internal/repl/history.js

+6-7
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,6 @@ function setupHistory(repl, historyPath, ready) {
4040
return ready(null, repl);
4141
}
4242

43-
// TODO: make it more granular (fs.in os.homedir())
44-
if (!process.policy.check('fs.in') || !process.policy.check('fs.out')) {
45-
_writeToOutput(repl, '\nAccess to FileSystemIn/Out is restricted.\n' +
46-
'REPL session history will not be persisted.\n');
47-
return ready(null, repl);
48-
}
49-
5043
if (!historyPath) {
5144
try {
5245
historyPath = path.join(os.homedir(), '.node_repl_history');
@@ -60,6 +53,12 @@ function setupHistory(repl, historyPath, ready) {
6053
}
6154
}
6255

56+
if (!process.policy.check('fs.out', historyPath)) {
57+
_writeToOutput(repl, '\nAccess to FileSystemOut is restricted.\n' +
58+
'REPL session history will not be persisted.\n');
59+
return ready(null, repl);
60+
}
61+
6362
let timer = null;
6463
let writing = false;
6564
let pending = false;

src/node_file.cc

+17-2
Original file line numberDiff line numberDiff line change
@@ -1240,8 +1240,13 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {
12401240

12411241
BufferValue old_path(isolate, args[0]);
12421242
CHECK_NOT_NULL(*old_path);
1243+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1244+
env, policy::Permission::kFileSystemOut, *old_path);
1245+
12431246
BufferValue new_path(isolate, args[1]);
12441247
CHECK_NOT_NULL(*new_path);
1248+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1249+
env, policy::Permission::kFileSystemOut, *new_path);
12451250

12461251
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
12471252
if (req_wrap_async != nullptr) {
@@ -1358,6 +1363,8 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
13581363

13591364
BufferValue path(env->isolate(), args[0]);
13601365
CHECK_NOT_NULL(*path);
1366+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1367+
env, policy::Permission::kFileSystemOut, *path);
13611368

13621369
FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req)
13631370
if (req_wrap_async != nullptr) {
@@ -1563,6 +1570,8 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
15631570

15641571
BufferValue path(env->isolate(), args[0]);
15651572
CHECK_NOT_NULL(*path);
1573+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1574+
env, policy::Permission::kFileSystemOut, *path);
15661575

15671576
CHECK(args[1]->IsInt32());
15681577
const int mode = args[1].As<Int32>()->Value();
@@ -1761,7 +1770,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
17611770
env, policy::Permission::kFileSystemIn, *path);
17621771
THROW_IF_INSUFFICIENT_PERMISSIONS(
17631772
env, policy::Permission::kFileSystemOut, *path);
1764-
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
1773+
} else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
17651774
THROW_IF_INSUFFICIENT_PERMISSIONS(
17661775
env, policy::Permission::kFileSystemIn, *path);
17671776
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
@@ -1812,7 +1821,7 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
18121821
env, policy::Permission::kFileSystemIn, *path);
18131822
THROW_IF_INSUFFICIENT_PERMISSIONS(
18141823
env, policy::Permission::kFileSystemOut, *path);
1815-
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
1824+
} else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
18161825
THROW_IF_INSUFFICIENT_PERMISSIONS(
18171826
env, policy::Permission::kFileSystemIn, *path);
18181827
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
@@ -1849,9 +1858,13 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {
18491858

18501859
BufferValue src(isolate, args[0]);
18511860
CHECK_NOT_NULL(*src);
1861+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1862+
env, policy::Permission::kFileSystemIn, *src);
18521863

18531864
BufferValue dest(isolate, args[1]);
18541865
CHECK_NOT_NULL(*dest);
1866+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1867+
env, policy::Permission::kFileSystemOut, *dest);
18551868

18561869
CHECK(args[2]->IsInt32());
18571870
const int flags = args[2].As<Int32>()->Value();
@@ -2340,6 +2353,8 @@ static void UTimes(const FunctionCallbackInfo<Value>& args) {
23402353

23412354
BufferValue path(env->isolate(), args[0]);
23422355
CHECK_NOT_NULL(*path);
2356+
THROW_IF_INSUFFICIENT_PERMISSIONS(
2357+
env, policy::Permission::kFileSystemOut, *path);
23432358

23442359
CHECK(args[1]->IsNumber());
23452360
const double atime = args[1].As<Number>()->Value();

src/policy/policy.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace policy {
1919

2020
#define THROW_IF_INSUFFICIENT_PERMISSIONS(env, perm_, resource_, ...) \
2121
if (!node::policy::root_policy.is_granted(perm_, resource_)) { \
22-
return node::policy::Policy::ThrowAccessDenied((env), perm_); \
22+
return node::policy::Policy::ThrowAccessDenied((env), perm_); \
2323
}
2424

2525
class Policy {
Original file line numberDiff line numberDiff line change
@@ -1,3 +0,0 @@
1-
# Regular File
2-
3-
Example of a regular file to be used in the PolicyDenyFs module

test/parallel/test-bootstrap-modules.js

+4
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const expectedModules = new Set([
2828
'Internal Binding options',
2929
'Internal Binding performance',
3030
'Internal Binding pipe_wrap',
31+
'Internal Binding policy',
3132
'Internal Binding process_methods',
3233
'Internal Binding report',
3334
'Internal Binding serdes',
@@ -101,10 +102,13 @@ const expectedModules = new Set([
101102
'NativeModule internal/perf/usertiming',
102103
'NativeModule internal/perf/resource_timing',
103104
'NativeModule internal/perf/utils',
105+
'NativeModule internal/policy/manifest',
106+
'NativeModule internal/policy/sri',
104107
'NativeModule internal/priority_queue',
105108
'NativeModule internal/process/esm_loader',
106109
'NativeModule internal/process/execution',
107110
'NativeModule internal/process/per_thread',
111+
'NativeModule internal/process/policy',
108112
'NativeModule internal/process/promises',
109113
'NativeModule internal/process/report',
110114
'NativeModule internal/process/signal',

test/parallel/test-cli-policy-deny-fs.js

+6-9
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@ const common = require('../common');
44
if (!common.hasCrypto)
55
common.skip('missing crypto');
66

7-
const fixtures = require('../common/fixtures');
8-
97
const { spawnSync } = require('child_process');
108
const assert = require('assert');
11-
12-
const dep = fixtures.path('policy', 'deny', 'check.js');
9+
const fs = require('fs');
1310

1411
{
1512
const { status, stdout } = spawnSync(
@@ -18,7 +15,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
1815
'--policy-deny-fs', 'fs', '-e',
1916
`console.log(process.policy.check("fs"));
2017
console.log(process.policy.check("fs.in"));
21-
console.log(process.policy.check("fs.out"));`
18+
console.log(process.policy.check("fs.out"));`,
2219
]
2320
);
2421

@@ -37,7 +34,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
3734
'--policy-deny-fs', 'in', '-e',
3835
`console.log(process.policy.check("fs"));
3936
console.log(process.policy.check("fs.in"));
40-
console.log(process.policy.check("fs.out"));`
37+
console.log(process.policy.check("fs.out"));`,
4138
]
4239
);
4340

@@ -55,7 +52,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
5552
'--policy-deny-fs', 'out', '-e',
5653
`console.log(process.policy.check("fs"));
5754
console.log(process.policy.check("fs.in"));
58-
console.log(process.policy.check("fs.out"));`
55+
console.log(process.policy.check("fs.out"));`,
5956
]
6057
);
6158

@@ -73,7 +70,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
7370
'--policy-deny-fs', 'out,in', '-e',
7471
`console.log(process.policy.check("fs"));
7572
console.log(process.policy.check("fs.in"));
76-
console.log(process.policy.check("fs.out"));`
73+
console.log(process.policy.check("fs.out"));`,
7774
]
7875
);
7976

@@ -112,5 +109,5 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
112109
stderr.toString().includes('Access to this API has been restricted'),
113110
stderr);
114111
assert.strictEqual(status, 1);
115-
assert.ok(fs.existsSync('policy-deny-example.md'));
112+
assert.ok(!fs.existsSync('policy-deny-example.md'));
116113
}

0 commit comments

Comments
 (0)