Skip to content

Commit bca44e6

Browse files
committed
permission: clamp worker grants to parent for explicit execArgv
C++-side intersection after Worker option parse when the parent has the Permission Model enabled: - No JS process.execArgv copying (avoids NODE_OPTIONS / repeated-flag gaps) - If the worker did not configure permission flags (e.g. execArgv: []), effective grants become the parent grant set - If the worker configured permission flags, boolean and fs grants are intersected with the parent so the worker cannot exceed the parent - Non-permission execArgv differences remain possible May be semver-major relative to documented non-inheritance; for reviewer call. Signed-off-by: yunshingng <yunshingng25@gmail.com>
1 parent b2b2b41 commit bca44e6

2 files changed

Lines changed: 209 additions & 0 deletions

File tree

‎src/node_worker.cc‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,113 @@ Worker::~Worker() {
504504
Debug(this, "Worker %llu destroyed", thread_id_.id);
505505
}
506506

507+
508+
// When the parent has --permission enabled, explicit Worker execArgv (including
509+
// []) must not yield a wider grant set than the parent. Enforcement is on the
510+
// C++ side after options are parsed so NODE_OPTIONS and repeated --allow-* are
511+
// already reflected in EnvironmentOptions (no JS process.execArgv copying).
512+
//
513+
// If the worker did not configure Permission Model flags at all, treat requested
514+
// grants as unrestricted under the model so the intersection equals the parent
515+
// grant set. If the worker did configure permission flags, intersect with parent.
516+
517+
static bool WorkerConfiguredPermission(const EnvironmentOptions* w) {
518+
if (w->permission || w->permission_audit) return true;
519+
if (!w->allow_fs_read.empty() || !w->allow_fs_write.empty()) return true;
520+
if (w->allow_addons || w->allow_inspector || w->allow_child_process ||
521+
w->allow_net || w->allow_wasi || w->allow_ffi || w->allow_openssl_store ||
522+
w->allow_worker_threads) {
523+
return true;
524+
}
525+
return false;
526+
}
527+
528+
static bool PathGrantedByParentList(const std::vector<std::string>& parent_paths,
529+
const std::string& requested) {
530+
if (parent_paths.empty()) return false;
531+
for (const std::string& p : parent_paths) {
532+
if (p == "*" || p == requested) return true;
533+
if (p.empty() || requested.size() < p.size()) continue;
534+
if (requested.compare(0, p.size(), p) != 0) continue;
535+
if (requested.size() == p.size()) return true;
536+
if (p.back() == '/' || requested[p.size()] == '/') return true;
537+
}
538+
return false;
539+
}
540+
541+
static void IntersectPathList(std::vector<std::string>* worker,
542+
const std::vector<std::string>& parent) {
543+
if (worker == nullptr || worker->empty()) return;
544+
std::vector<std::string> out;
545+
out.reserve(worker->size());
546+
for (const std::string& wpath : *worker) {
547+
if (wpath == "*") {
548+
for (const std::string& p : parent) {
549+
if (p == "*") {
550+
out.push_back(wpath);
551+
break;
552+
}
553+
}
554+
continue;
555+
}
556+
if (PathGrantedByParentList(parent, wpath)) out.push_back(wpath);
557+
}
558+
*worker = std::move(out);
559+
}
560+
561+
static void CopyParentPermissionGrants(EnvironmentOptions* w,
562+
const EnvironmentOptions* parent) {
563+
w->permission = parent->permission;
564+
w->permission_audit = parent->permission_audit;
565+
w->allow_addons = parent->allow_addons;
566+
w->allow_inspector = parent->allow_inspector;
567+
w->allow_child_process = parent->allow_child_process;
568+
w->allow_net = parent->allow_net;
569+
w->allow_wasi = parent->allow_wasi;
570+
w->allow_ffi = parent->allow_ffi;
571+
w->allow_openssl_store = parent->allow_openssl_store;
572+
w->allow_worker_threads = parent->allow_worker_threads;
573+
w->allow_fs_read = parent->allow_fs_read;
574+
w->allow_fs_write = parent->allow_fs_write;
575+
}
576+
577+
static void IntersectPermissionGrants(EnvironmentOptions* w,
578+
const EnvironmentOptions* parent) {
579+
w->permission = true;
580+
w->permission_audit = w->permission_audit || parent->permission_audit;
581+
582+
w->allow_addons = w->allow_addons && parent->allow_addons;
583+
w->allow_inspector = w->allow_inspector && parent->allow_inspector;
584+
w->allow_child_process = w->allow_child_process && parent->allow_child_process;
585+
w->allow_net = w->allow_net && parent->allow_net;
586+
w->allow_wasi = w->allow_wasi && parent->allow_wasi;
587+
w->allow_ffi = w->allow_ffi && parent->allow_ffi;
588+
w->allow_openssl_store = w->allow_openssl_store && parent->allow_openssl_store;
589+
w->allow_worker_threads =
590+
w->allow_worker_threads && parent->allow_worker_threads;
591+
592+
IntersectPathList(&w->allow_fs_read, parent->allow_fs_read);
593+
IntersectPathList(&w->allow_fs_write, parent->allow_fs_write);
594+
}
595+
596+
static void ClampWorkerPermissionToParent(Environment* env,
597+
PerIsolateOptions* worker_opts) {
598+
if (worker_opts == nullptr || !env->permission()->enabled()) return;
599+
600+
EnvironmentOptions* parent =
601+
env->isolate_data()->options()->get_per_env_options();
602+
EnvironmentOptions* w = worker_opts->get_per_env_options();
603+
if (parent == nullptr || w == nullptr) return;
604+
605+
if (!WorkerConfiguredPermission(w)) {
606+
CopyParentPermissionGrants(w, parent);
607+
w->permission = true;
608+
return;
609+
}
610+
611+
IntersectPermissionGrants(w, parent);
612+
}
613+
507614
void Worker::New(const FunctionCallbackInfo<Value>& args) {
508615
Environment* env = Environment::GetCurrent(args);
509616
THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -688,6 +795,10 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
688795
// essential to load user codes and must not be blocked by the inspector
689796
// for internal scripts.
690797
// Still, `--inspect-node` can break on the first line of internal scripts.
798+
if (env->permission()->enabled() && per_isolate_opts) {
799+
ClampWorkerPermissionToParent(env, per_isolate_opts.get());
800+
}
801+
691802
if (is_internal) {
692803
per_isolate_opts->per_env->get_debug_options()
693804
->DisableWaitOrBreakFirstLine();
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const { isMainThread } = require('worker_threads');
5+
if (!isMainThread) common.skip('main thread only');
6+
7+
const assert = require('assert');
8+
const fs = require('fs');
9+
const path = require('path');
10+
const { spawnSync } = require('child_process');
11+
const tmpdir = require('../common/tmpdir');
12+
tmpdir.refresh();
13+
14+
const allowed = tmpdir.path;
15+
const deniedFile = path.join(tmpdir.path, '..', 'permission-worker-denied-file');
16+
fs.writeFileSync(deniedFile, 'secret\n');
17+
fs.writeFileSync(path.join(allowed, 'ok.txt'), 'allowed\n');
18+
19+
function runWorker(workerSource, execArgvFragment) {
20+
const code = `
21+
const { Worker } = require('worker_threads');
22+
const w = new Worker(${JSON.stringify(workerSource)}, {
23+
eval: true,
24+
${execArgvFragment}
25+
});
26+
w.on('message', (msg) => {
27+
process.stdout.write(JSON.stringify(msg) + '\\n');
28+
process.exit(0);
29+
});
30+
w.on('error', (err) => { console.error(err); process.exit(1); });
31+
`;
32+
return spawnSync(process.execPath, [
33+
'--permission',
34+
`--allow-fs-read=${allowed}`,
35+
'--allow-worker',
36+
'-e',
37+
code,
38+
], { encoding: 'utf8', timeout: 20000 });
39+
}
40+
41+
const readDenied = `
42+
const { parentPort } = require('worker_threads');
43+
const fs = require('fs');
44+
try {
45+
parentPort.postMessage({
46+
ok: true,
47+
data: fs.readFileSync(${JSON.stringify(deniedFile)}, 'utf8'),
48+
});
49+
} catch (err) {
50+
parentPort.postMessage({ ok: false, code: err.code });
51+
}
52+
`;
53+
54+
const readAllowed = `
55+
const { parentPort } = require('worker_threads');
56+
const fs = require('fs');
57+
try {
58+
parentPort.postMessage({
59+
ok: true,
60+
data: fs.readFileSync(${JSON.stringify(path.join(allowed, 'ok.txt'))}, 'utf8'),
61+
});
62+
} catch (err) {
63+
parentPort.postMessage({ ok: false, code: err.code });
64+
}
65+
`;
66+
67+
function lastMsg(r) {
68+
assert.strictEqual(r.status, 0, r.stderr);
69+
return JSON.parse(r.stdout.trim().split('\n').pop());
70+
}
71+
72+
{
73+
const msg = lastMsg(runWorker(readDenied, ''));
74+
assert.strictEqual(msg.ok, false);
75+
assert.strictEqual(msg.code, 'ERR_ACCESS_DENIED');
76+
}
77+
78+
{
79+
const msg = lastMsg(runWorker(readDenied, 'execArgv: [],'));
80+
assert.strictEqual(msg.ok, false, JSON.stringify(msg));
81+
assert.strictEqual(msg.code, 'ERR_ACCESS_DENIED');
82+
}
83+
84+
{
85+
const msg = lastMsg(runWorker(readAllowed, 'execArgv: [],'));
86+
assert.strictEqual(msg.ok, true, JSON.stringify(msg));
87+
}
88+
89+
{
90+
const frag = `execArgv: ${JSON.stringify([
91+
'--permission',
92+
'--allow-fs-read=*',
93+
'--allow-worker',
94+
])},`;
95+
const msg = lastMsg(runWorker(readDenied, frag));
96+
assert.strictEqual(msg.ok, false, JSON.stringify(msg));
97+
assert.strictEqual(msg.code, 'ERR_ACCESS_DENIED');
98+
}

0 commit comments

Comments
 (0)