Skip to content

Commit d7d7854

Browse files
RafaelGSSRafaelGSS
committed
test: fix policy-deny tests
1 parent 1945c2c commit d7d7854

12 files changed

+157
-250
lines changed

‎doc/api/cli.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,16 @@ unless either the `--pending-deprecation` command-line flag, or the
783783
are used to provide a kind of selective "early warning" mechanism that
784784
developers may leverage to detect deprecated API usage.
785785

786+
### `--policy-deny-fs`
787+
788+
<!-- YAML
789+
added: REPLACEME
790+
-->
791+
792+
> Stability: 1 - Experimental
793+
794+
TODO
795+
786796
### `--policy-integrity=sri`
787797

788798
<!-- YAML
@@ -1650,6 +1660,7 @@ Node.js options that are allowed are:
16501660
* `--openssl-config`
16511661
* `--openssl-legacy-provider`
16521662
* `--pending-deprecation`
1663+
* `--policy-deny-fs`
16531664
* `--policy-integrity`
16541665
* `--preserve-symlinks-main`
16551666
* `--preserve-symlinks`

‎doc/node.1

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,9 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
309309
.It Fl -pending-deprecation
310310
Emit pending deprecation warnings.
311311
.
312+
.It Fl -policy-deny-fs
313+
Instructs Node.js to restrict access to the given resource type
314+
.
312315
.It Fl -policy-integrity Ns = Ns Ar sri
313316
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.
314317
.

‎lib/internal/repl/history.js

Lines changed: 6 additions & 7 deletions
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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,8 +1235,13 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {
12351235

12361236
BufferValue old_path(isolate, args[0]);
12371237
CHECK_NOT_NULL(*old_path);
1238+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1239+
env, policy::Permission::kFileSystemOut, *old_path);
1240+
12381241
BufferValue new_path(isolate, args[1]);
12391242
CHECK_NOT_NULL(*new_path);
1243+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1244+
env, policy::Permission::kFileSystemOut, *new_path);
12401245

12411246
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
12421247
if (req_wrap_async != nullptr) {
@@ -1353,6 +1358,8 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
13531358

13541359
BufferValue path(env->isolate(), args[0]);
13551360
CHECK_NOT_NULL(*path);
1361+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1362+
env, policy::Permission::kFileSystemOut, *path);
13561363

13571364
FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req)
13581365
if (req_wrap_async != nullptr) {
@@ -1557,6 +1564,8 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
15571564

15581565
BufferValue path(env->isolate(), args[0]);
15591566
CHECK_NOT_NULL(*path);
1567+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1568+
env, policy::Permission::kFileSystemOut, *path);
15601569

15611570
CHECK(args[1]->IsInt32());
15621571
const int mode = args[1].As<Int32>()->Value();
@@ -1755,7 +1764,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
17551764
env, policy::Permission::kFileSystemIn, *path);
17561765
THROW_IF_INSUFFICIENT_PERMISSIONS(
17571766
env, policy::Permission::kFileSystemOut, *path);
1758-
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
1767+
} else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
17591768
THROW_IF_INSUFFICIENT_PERMISSIONS(
17601769
env, policy::Permission::kFileSystemIn, *path);
17611770
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
@@ -1806,7 +1815,7 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
18061815
env, policy::Permission::kFileSystemIn, *path);
18071816
THROW_IF_INSUFFICIENT_PERMISSIONS(
18081817
env, policy::Permission::kFileSystemOut, *path);
1809-
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
1818+
} else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
18101819
THROW_IF_INSUFFICIENT_PERMISSIONS(
18111820
env, policy::Permission::kFileSystemIn, *path);
18121821
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
@@ -1843,9 +1852,13 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {
18431852

18441853
BufferValue src(isolate, args[0]);
18451854
CHECK_NOT_NULL(*src);
1855+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1856+
env, policy::Permission::kFileSystemIn, *src);
18461857

18471858
BufferValue dest(isolate, args[1]);
18481859
CHECK_NOT_NULL(*dest);
1860+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1861+
env, policy::Permission::kFileSystemOut, *dest);
18491862

18501863
CHECK(args[2]->IsInt32());
18511864
const int flags = args[2].As<Int32>()->Value();
@@ -2334,6 +2347,8 @@ static void UTimes(const FunctionCallbackInfo<Value>& args) {
23342347

23352348
BufferValue path(env->isolate(), args[0]);
23362349
CHECK_NOT_NULL(*path);
2350+
THROW_IF_INSUFFICIENT_PERMISSIONS(
2351+
env, policy::Permission::kFileSystemOut, *path);
23372352

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

‎src/node_options.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
709709
&PerIsolateOptions::experimental_top_level_await,
710710
kAllowedInEnvironment);
711711
AddOption("--harmony-top-level-await", "", V8Option{});
712-
713712
Implies("--experimental-top-level-await", "--harmony-top-level-await");
714713
Implies("--harmony-top-level-await", "--experimental-top-level-await");
715714
ImpliesNot("--no-harmony-top-level-await", "--experimental-top-level-await");

‎src/policy/policy.h

Lines changed: 1 addition & 1 deletion
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 {

‎test/fixtures/policy/deny/regular-file.md

Lines changed: 0 additions & 3 deletions
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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const expectedModules = new Set([
2727
'Internal Binding options',
2828
'Internal Binding performance',
2929
'Internal Binding pipe_wrap',
30+
'Internal Binding policy',
3031
'Internal Binding process_methods',
3132
'Internal Binding report',
3233
'Internal Binding serdes',
@@ -100,10 +101,13 @@ const expectedModules = new Set([
100101
'NativeModule internal/perf/timerify',
101102
'NativeModule internal/perf/usertiming',
102103
'NativeModule internal/perf/utils',
104+
'NativeModule internal/policy/manifest',
105+
'NativeModule internal/policy/sri',
103106
'NativeModule internal/priority_queue',
104107
'NativeModule internal/process/esm_loader',
105108
'NativeModule internal/process/execution',
106109
'NativeModule internal/process/per_thread',
110+
'NativeModule internal/process/policy',
107111
'NativeModule internal/process/promises',
108112
'NativeModule internal/process/report',
109113
'NativeModule internal/process/signal',

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

Lines changed: 6 additions & 9 deletions
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)