Skip to content

Commit

Permalink
test: fix policy-deny tests
Browse files Browse the repository at this point in the history
  • Loading branch information
RafaelGSS committed Jul 27, 2022
1 parent 49c43b6 commit ca0ebe3
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 249 deletions.
11 changes: 11 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,16 @@ unless either the `--pending-deprecation` command-line flag, or the
are used to provide a kind of selective "early warning" mechanism that
developers may leverage to detect deprecated API usage.

### `--policy-deny-fs`

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental
TODO

### `--policy-integrity=sri`

<!-- YAML
Expand Down Expand Up @@ -1703,6 +1713,7 @@ Node.js options that are allowed are:
* `--openssl-legacy-provider`
* `--openssl-shared-config`
* `--pending-deprecation`
* `--policy-deny-fs`
* `--policy-integrity`
* `--preserve-symlinks-main`
* `--preserve-symlinks`
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
.It Fl -pending-deprecation
Emit pending deprecation warnings.
.
.It Fl -policy-deny-fs
Instructs Node.js to restrict access to the given resource type
.
.It Fl -policy-integrity Ns = Ns Ar sri
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.
.
Expand Down
13 changes: 6 additions & 7 deletions lib/internal/repl/history.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,6 @@ function setupHistory(repl, historyPath, ready) {
return ready(null, repl);
}

// TODO: make it more granular (fs.in os.homedir())
if (!process.policy.check('fs.in') || !process.policy.check('fs.out')) {
_writeToOutput(repl, '\nAccess to FileSystemIn/Out is restricted.\n' +
'REPL session history will not be persisted.\n');
return ready(null, repl);
}

if (!historyPath) {
try {
historyPath = path.join(os.homedir(), '.node_repl_history');
Expand All @@ -60,6 +53,12 @@ function setupHistory(repl, historyPath, ready) {
}
}

if (!process.policy.check('fs.out', historyPath)) {
_writeToOutput(repl, '\nAccess to FileSystemOut is restricted.\n' +
'REPL session history will not be persisted.\n');
return ready(null, repl);
}

let timer = null;
let writing = false;
let pending = false;
Expand Down
19 changes: 17 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1240,8 +1240,13 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {

BufferValue old_path(isolate, args[0]);
CHECK_NOT_NULL(*old_path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *old_path);

BufferValue new_path(isolate, args[1]);
CHECK_NOT_NULL(*new_path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *new_path);

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

BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *path);

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

BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *path);

CHECK(args[1]->IsInt32());
const int mode = args[1].As<Int32>()->Value();
Expand Down Expand Up @@ -1761,7 +1770,7 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
env, policy::Permission::kFileSystemIn, *path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *path);
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
} else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemIn, *path);
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
Expand Down Expand Up @@ -1812,7 +1821,7 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
env, policy::Permission::kFileSystemIn, *path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *path);
} else if ((flags &~ (O_RDONLY | O_SYNC)) == 0) {
} else if ((flags & ~(O_RDONLY | O_SYNC)) == 0) {
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemIn, *path);
} else if ((flags & (O_APPEND | O_TRUNC | O_CREAT | O_WRONLY)) != 0) {
Expand Down Expand Up @@ -1849,9 +1858,13 @@ static void CopyFile(const FunctionCallbackInfo<Value>& args) {

BufferValue src(isolate, args[0]);
CHECK_NOT_NULL(*src);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemIn, *src);

BufferValue dest(isolate, args[1]);
CHECK_NOT_NULL(*dest);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *dest);

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

BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, policy::Permission::kFileSystemOut, *path);

CHECK(args[1]->IsNumber());
const double atime = args[1].As<Number>()->Value();
Expand Down
2 changes: 1 addition & 1 deletion src/policy/policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace policy {

#define THROW_IF_INSUFFICIENT_PERMISSIONS(env, perm_, resource_, ...) \
if (!node::policy::root_policy.is_granted(perm_, resource_)) { \
return node::policy::Policy::ThrowAccessDenied((env), perm_); \
return node::policy::Policy::ThrowAccessDenied((env), perm_); \
}

class Policy {
Expand Down
3 changes: 0 additions & 3 deletions test/fixtures/policy/deny/regular-file.md
Original file line number Diff line number Diff line change
@@ -1,3 +0,0 @@
# Regular File

Example of a regular file to be used in the PolicyDenyFs module
4 changes: 4 additions & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const expectedModules = new Set([
'Internal Binding options',
'Internal Binding performance',
'Internal Binding pipe_wrap',
'Internal Binding policy',
'Internal Binding process_methods',
'Internal Binding report',
'Internal Binding serdes',
Expand Down Expand Up @@ -101,10 +102,13 @@ const expectedModules = new Set([
'NativeModule internal/perf/usertiming',
'NativeModule internal/perf/resource_timing',
'NativeModule internal/perf/utils',
'NativeModule internal/policy/manifest',
'NativeModule internal/policy/sri',
'NativeModule internal/priority_queue',
'NativeModule internal/process/esm_loader',
'NativeModule internal/process/execution',
'NativeModule internal/process/per_thread',
'NativeModule internal/process/policy',
'NativeModule internal/process/promises',
'NativeModule internal/process/report',
'NativeModule internal/process/signal',
Expand Down
15 changes: 6 additions & 9 deletions test/parallel/test-cli-policy-deny-fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const fixtures = require('../common/fixtures');

const { spawnSync } = require('child_process');
const assert = require('assert');

const dep = fixtures.path('policy', 'deny', 'check.js');
const fs = require('fs');

{
const { status, stdout } = spawnSync(
Expand All @@ -18,7 +15,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
'--policy-deny-fs', 'fs', '-e',
`console.log(process.policy.check("fs"));
console.log(process.policy.check("fs.in"));
console.log(process.policy.check("fs.out"));`
console.log(process.policy.check("fs.out"));`,
]
);

Expand All @@ -37,7 +34,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
'--policy-deny-fs', 'in', '-e',
`console.log(process.policy.check("fs"));
console.log(process.policy.check("fs.in"));
console.log(process.policy.check("fs.out"));`
console.log(process.policy.check("fs.out"));`,
]
);

Expand All @@ -55,7 +52,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
'--policy-deny-fs', 'out', '-e',
`console.log(process.policy.check("fs"));
console.log(process.policy.check("fs.in"));
console.log(process.policy.check("fs.out"));`
console.log(process.policy.check("fs.out"));`,
]
);

Expand All @@ -73,7 +70,7 @@ const dep = fixtures.path('policy', 'deny', 'check.js');
'--policy-deny-fs', 'out,in', '-e',
`console.log(process.policy.check("fs"));
console.log(process.policy.check("fs.in"));
console.log(process.policy.check("fs.out"));`
console.log(process.policy.check("fs.out"));`,
]
);

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

0 comments on commit ca0ebe3

Please sign in to comment.