Skip to content

Commit fee1996

Browse files
committed
permission: clamp Worker grants to parent for explicit execArgv
SEMVER-MAJOR: when the parent has the Permission Model enabled, a Worker with explicit execArgv (including []) cannot obtain wider permission-related grants than the parent. Clamp EnvironmentOptions, rebuild exec_argv_out, and re-parse so CreateEnvironment and Isolate options stay aligned. Signed-off-by: yunshingng <yunshingng25@gmail.com>
1 parent 76bb3f7 commit fee1996

4 files changed

Lines changed: 334 additions & 0 deletions

File tree

‎doc/api/permissions.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ changes:
3838
description: This feature is no longer experimental.
3939
-->
4040

41+
<!-- worker-execargv-permission-ceiling -->
42+
When the Permission Model is enabled in the parent process, creating a
43+
`worker_threads.Worker` with an explicit `execArgv` option (including an empty
44+
array) no longer allows the worker to obtain a wider permission-related grant
45+
set than the parent. Non-permission `execArgv` flags are unaffected. This is a
46+
breaking change relative to earlier releases where `execArgv: []` could drop
47+
the parent's Permission Model grants.
48+
49+
4150
> Stability: 2 - Stable
4251
4352
The Node.js Permission Model is a mechanism for restricting access to specific

‎doc/api/worker_threads.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,6 +1605,13 @@ changes:
16051605
description: The `resourceLimits` option was introduced.
16061606
-->
16071607
1608+
<!-- worker-execargv-permission-ceiling -->
1609+
**Permission Model (breaking):** If the parent process runs with the
1610+
Permission Model enabled, an explicit `execArgv` (including `[]`) does not
1611+
disable or exceed the parent's permission-related grants. See the
1612+
[Permission Model](permissions.md#permission-model) documentation.
1613+
1614+
16081615
* `filename` {string|URL} The path to the Worker's main script or module. Must
16091616
be either an absolute path or a relative path (i.e. relative to the
16101617
current working directory) starting with `./` or `../`, or a WHATWG `URL`

‎src/node_worker.cc‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "node_profiling.h"
1212
#include "node_snapshot_builder.h"
1313
#include "permission/permission.h"
14+
#include "path.h"
1415
#include "util-inl.h"
1516
#include "v8-cppgc.h"
1617
#include "v8-profiler.h"
@@ -504,6 +505,173 @@ Worker::~Worker() {
504505
Debug(this, "Worker %llu destroyed", thread_id_.id);
505506
}
506507

508+
509+
// SEMVER-MAJOR: Permission ceiling for Worker explicit execArgv.
510+
static bool WorkerConfiguredPermission(const EnvironmentOptions* w) {
511+
if (w->permission || w->permission_audit) return true;
512+
if (!w->allow_fs_read.empty() || !w->allow_fs_write.empty()) return true;
513+
return w->allow_addons || w->allow_inspector || w->allow_child_process ||
514+
w->allow_net || w->allow_wasi || w->allow_ffi ||
515+
w->allow_openssl_store || w->allow_worker_threads;
516+
}
517+
518+
static void ApplyParentPermissionCeiling(EnvironmentOptions* w,
519+
const EnvironmentOptions* parent) {
520+
w->permission = true;
521+
w->permission_audit = parent->permission_audit;
522+
w->allow_addons = parent->allow_addons;
523+
w->allow_inspector = parent->allow_inspector;
524+
w->allow_child_process = parent->allow_child_process;
525+
w->allow_net = parent->allow_net;
526+
w->allow_wasi = parent->allow_wasi;
527+
w->allow_ffi = parent->allow_ffi;
528+
w->allow_openssl_store = parent->allow_openssl_store;
529+
w->allow_worker_threads = parent->allow_worker_threads;
530+
w->allow_fs_read = parent->allow_fs_read;
531+
w->allow_fs_write = parent->allow_fs_write;
532+
}
533+
534+
static void NormalizePathForCompare(std::string* s) {
535+
while (s->size() > 1 && (s->back() == '/' || s->back() == '\\')) {
536+
s->pop_back();
537+
}
538+
#ifdef _WIN32
539+
for (char& c : *s) {
540+
if (c >= 'A' && c <= 'Z') c = static_cast<char>(c - 'A' + 'a');
541+
if (c == '/') c = '\\';
542+
}
543+
#endif
544+
}
545+
546+
static std::string ResolveForCompare(Environment* env, const std::string& in) {
547+
if (in.empty() || in == "*") return in;
548+
std::string resolved =
549+
PathResolve(env, std::vector<std::string_view>{std::string_view(in)});
550+
if (resolved.empty()) resolved = in;
551+
NormalizePathForCompare(&resolved);
552+
return resolved;
553+
}
554+
555+
static bool PathCoveredByParentEntry(Environment* env,
556+
const std::string& parent_raw,
557+
const std::string& requested_raw) {
558+
if (parent_raw == "*") return true;
559+
const std::string parent = ResolveForCompare(env, parent_raw);
560+
const std::string requested = ResolveForCompare(env, requested_raw);
561+
if (parent.empty()) return false;
562+
if (requested == parent) return true;
563+
if (requested.size() <= parent.size()) return false;
564+
if (requested.compare(0, parent.size(), parent) != 0) return false;
565+
const char next = requested[parent.size()];
566+
return next == '/' || next == '\\';
567+
}
568+
569+
static bool ParentListHasWildcard(const std::vector<std::string>& parent) {
570+
for (const std::string& p : parent) {
571+
if (p == "*") return true;
572+
}
573+
return false;
574+
}
575+
576+
static void FilterPathListToParentSubset(
577+
Environment* env,
578+
EnvironmentOptions* w,
579+
std::vector<std::string>* worker,
580+
const std::vector<std::string>& parent) {
581+
if (worker == nullptr) return;
582+
if (worker->empty()) {
583+
if (w->permission || w->permission_audit) return;
584+
*worker = parent;
585+
return;
586+
}
587+
if (ParentListHasWildcard(parent)) return;
588+
std::vector<std::string> out;
589+
out.reserve(worker->size());
590+
for (const std::string& wpath : *worker) {
591+
if (wpath == "*") continue;
592+
for (const std::string& p : parent) {
593+
if (PathCoveredByParentEntry(env, p, wpath)) {
594+
out.push_back(wpath);
595+
break;
596+
}
597+
}
598+
}
599+
*worker = std::move(out);
600+
}
601+
602+
static void IntersectPermissionGrants(Environment* env,
603+
EnvironmentOptions* w,
604+
const EnvironmentOptions* parent) {
605+
w->permission = true;
606+
w->permission_audit = w->permission_audit || parent->permission_audit;
607+
w->allow_addons = w->allow_addons && parent->allow_addons;
608+
w->allow_inspector = w->allow_inspector && parent->allow_inspector;
609+
w->allow_child_process =
610+
w->allow_child_process && parent->allow_child_process;
611+
w->allow_net = w->allow_net && parent->allow_net;
612+
w->allow_wasi = w->allow_wasi && parent->allow_wasi;
613+
w->allow_ffi = w->allow_ffi && parent->allow_ffi;
614+
w->allow_openssl_store =
615+
w->allow_openssl_store && parent->allow_openssl_store;
616+
w->allow_worker_threads =
617+
w->allow_worker_threads && parent->allow_worker_threads;
618+
FilterPathListToParentSubset(env, w, &w->allow_fs_read, parent->allow_fs_read);
619+
FilterPathListToParentSubset(
620+
env, w, &w->allow_fs_write, parent->allow_fs_write);
621+
}
622+
623+
static void ClampWorkerPermissionToParent(Environment* env,
624+
PerIsolateOptions* worker_opts) {
625+
if (worker_opts == nullptr || !env->permission()->enabled()) return;
626+
EnvironmentOptions* parent = env->isolate_data()->options()->get_per_env_options();
627+
EnvironmentOptions* w = worker_opts->get_per_env_options();
628+
if (parent == nullptr || w == nullptr) return;
629+
if (!WorkerConfiguredPermission(w)) {
630+
ApplyParentPermissionCeiling(w, parent);
631+
} else {
632+
IntersectPermissionGrants(env, w, parent);
633+
}
634+
}
635+
636+
static void RebuildExecArgvOutFromPermissionOptions(
637+
PerIsolateOptions* worker_opts, std::vector<std::string>* exec_argv_out) {
638+
if (worker_opts == nullptr || exec_argv_out == nullptr) return;
639+
EnvironmentOptions* w = worker_opts->get_per_env_options();
640+
if (w == nullptr || !w->permission) return;
641+
642+
std::vector<std::string> out;
643+
out.emplace_back("");
644+
auto is_perm = [](const std::string& a) {
645+
return a == "--permission" || a == "--permission-audit" ||
646+
a.rfind("--allow-fs-read", 0) == 0 ||
647+
a.rfind("--allow-fs-write", 0) == 0 || a == "--allow-addons" ||
648+
a == "--allow-inspector" || a == "--allow-child-process" ||
649+
a == "--allow-net" || a == "--allow-wasi" || a == "--allow-ffi" ||
650+
a == "--allow-openssl-store" || a == "--allow-worker";
651+
};
652+
for (size_t i = 1; i < exec_argv_out->size(); i++) {
653+
if (!is_perm((*exec_argv_out)[i])) out.push_back((*exec_argv_out)[i]);
654+
}
655+
out.push_back("--permission");
656+
if (w->permission_audit) out.push_back("--permission-audit");
657+
if (w->allow_addons) out.push_back("--allow-addons");
658+
if (w->allow_inspector) out.push_back("--allow-inspector");
659+
if (w->allow_child_process) out.push_back("--allow-child-process");
660+
if (w->allow_net) out.push_back("--allow-net");
661+
if (w->allow_wasi) out.push_back("--allow-wasi");
662+
if (w->allow_ffi) out.push_back("--allow-ffi");
663+
if (w->allow_openssl_store) out.push_back("--allow-openssl-store");
664+
if (w->allow_worker_threads) out.push_back("--allow-worker");
665+
for (const std::string& path : w->allow_fs_read) {
666+
out.push_back("--allow-fs-read=" + path);
667+
}
668+
for (const std::string& path : w->allow_fs_write) {
669+
out.push_back("--allow-fs-write=" + path);
670+
}
671+
*exec_argv_out = std::move(out);
672+
}
673+
674+
507675
void Worker::New(const FunctionCallbackInfo<Value>& args) {
508676
Environment* env = Environment::GetCurrent(args);
509677
THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -683,6 +851,27 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
683851
per_isolate_opts = env->isolate_data()->options()->Clone();
684852
}
685853

854+
if (env->permission()->enabled() && per_isolate_opts) {
855+
ClampWorkerPermissionToParent(env, per_isolate_opts.get());
856+
if (args[2]->IsArray()) {
857+
RebuildExecArgvOutFromPermissionOptions(per_isolate_opts.get(),
858+
&exec_argv_out);
859+
// Re-parse so per_isolate_opts stay aligned with exec_argv_out.
860+
std::vector<std::string> exec_argv_copy = exec_argv_out;
861+
std::vector<std::string> ignored_out;
862+
std::vector<std::string> invalid_args2;
863+
std::vector<std::string> errors2;
864+
options_parser::Parse(&exec_argv_copy,
865+
&ignored_out,
866+
&invalid_args2,
867+
per_isolate_opts.get(),
868+
kDisallowedInEnvvar,
869+
&errors2);
870+
// Keep the rebuilt argv as the one passed to CreateEnvironment.
871+
// Parse consumes/moves copy; exec_argv_out already holds final tokens.
872+
}
873+
}
874+
686875
// Internal workers should not wait for inspector frontend to connect or
687876
// break on the first line of internal scripts. Module loader threads are
688877
// essential to load user codes and must not be blocked by the inspector
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
'use strict';
2+
const common = require('../common');
3+
const { isMainThread } = require('worker_threads');
4+
if (!isMainThread) common.skip('This test only works on a main thread');
5+
if (!common.hasCrypto) common.skip('no crypto');
6+
const assert = require('assert');
7+
const fs = require('fs');
8+
const path = require('path');
9+
const { spawnSync } = require('child_process');
10+
const tmpdir = require('../common/tmpdir');
11+
tmpdir.refresh();
12+
const allowedDir = tmpdir.path;
13+
const allowedFile = path.join(allowedDir, 'ok.txt');
14+
const subFile = path.join(allowedDir, 'sub', 'nested.txt');
15+
fs.mkdirSync(path.dirname(subFile), { recursive: true });
16+
const deniedDir = fs.mkdtempSync(path.join(path.dirname(tmpdir.path), 'perm-deny-'));
17+
const deniedFile = path.join(deniedDir, 'secret.txt');
18+
fs.writeFileSync(allowedFile, 'allowed\n');
19+
fs.writeFileSync(subFile, 'nested\n');
20+
fs.writeFileSync(deniedFile, 'secret\n');
21+
function runWorkerCase(workerBody, execArgvFragment) {
22+
const code = `
23+
const { Worker } = require('worker_threads');
24+
const w = new Worker(${JSON.stringify(workerBody)}, {
25+
eval: true,
26+
${execArgvFragment}
27+
});
28+
w.on('message', (msg) => {
29+
process.stdout.write(JSON.stringify(msg) + '\\n');
30+
process.exit(0);
31+
});
32+
w.on('error', (err) => { console.error(err); process.exit(1); });
33+
`;
34+
return spawnSync(
35+
process.execPath,
36+
['--permission', `--allow-fs-read=${allowedDir}`, '--allow-worker', '-e', code],
37+
{ encoding: 'utf8', timeout: 30000, env: { ...process.env } },
38+
);
39+
}
40+
function parseLastJsonLine(stdout) {
41+
const lines = stdout.trim().split('\n').filter(Boolean);
42+
assert.ok(lines.length > 0, 'expected worker JSON output');
43+
return JSON.parse(lines[lines.length - 1]);
44+
}
45+
function workerRead(filePath) {
46+
return `
47+
const { parentPort } = require('worker_threads');
48+
const fs = require('fs');
49+
try {
50+
parentPort.postMessage({
51+
ok: true,
52+
data: fs.readFileSync(${JSON.stringify(filePath)}, 'utf8'),
53+
});
54+
} catch (err) {
55+
parentPort.postMessage({ ok: false, code: err.code });
56+
}
57+
`;
58+
}
59+
function assertAccessDenied(msg) {
60+
assert.strictEqual(msg.ok, false, JSON.stringify(msg));
61+
assert.strictEqual(msg.code, 'ERR_ACCESS_DENIED');
62+
}
63+
function assertOk(msg) {
64+
assert.strictEqual(msg.ok, true, JSON.stringify(msg));
65+
}
66+
try {
67+
{
68+
const r = runWorkerCase(workerRead(deniedFile), '');
69+
assert.strictEqual(r.status, 0, r.stderr);
70+
assertAccessDenied(parseLastJsonLine(r.stdout));
71+
}
72+
{
73+
const denied = runWorkerCase(workerRead(deniedFile), 'execArgv: [],');
74+
assert.strictEqual(denied.status, 0, denied.stderr);
75+
assertAccessDenied(parseLastJsonLine(denied.stdout));
76+
const allowed = runWorkerCase(workerRead(allowedFile), 'execArgv: [],');
77+
assert.strictEqual(allowed.status, 0, allowed.stderr);
78+
assertOk(parseLastJsonLine(allowed.stdout));
79+
const nested = runWorkerCase(workerRead(subFile), 'execArgv: [],');
80+
assert.strictEqual(nested.status, 0, nested.stderr);
81+
assertOk(parseLastJsonLine(nested.stdout));
82+
}
83+
{
84+
const denied = runWorkerCase(workerRead(deniedFile), 'execArgv: ["--no-warnings"],');
85+
assert.strictEqual(denied.status, 0, denied.stderr);
86+
assertAccessDenied(parseLastJsonLine(denied.stdout));
87+
const allowed = runWorkerCase(workerRead(allowedFile), 'execArgv: ["--no-warnings"],');
88+
assert.strictEqual(allowed.status, 0, allowed.stderr);
89+
assertOk(parseLastJsonLine(allowed.stdout));
90+
}
91+
{
92+
const frag = `execArgv: ${JSON.stringify(['--permission', '--allow-worker'])},`;
93+
const d = runWorkerCase(workerRead(deniedFile), frag);
94+
assert.strictEqual(d.status, 0, d.stderr);
95+
assertAccessDenied(parseLastJsonLine(d.stdout));
96+
const a = runWorkerCase(workerRead(allowedFile), frag);
97+
assert.strictEqual(a.status, 0, a.stderr);
98+
assertAccessDenied(parseLastJsonLine(a.stdout));
99+
}
100+
{
101+
const frag = `execArgv: ${JSON.stringify(['--permission', '--allow-fs-read=*', '--allow-worker'])},`;
102+
const r = runWorkerCase(workerRead(deniedFile), frag);
103+
assert.strictEqual(r.status, 0, r.stderr);
104+
assertAccessDenied(parseLastJsonLine(r.stdout));
105+
}
106+
{
107+
const frag = `execArgv: ${JSON.stringify([
108+
'--permission',
109+
`--allow-fs-read=${allowedDir}`,
110+
`--allow-fs-read=${deniedFile}`,
111+
'--allow-worker',
112+
])},`;
113+
const r = runWorkerCase(workerRead(deniedFile), frag);
114+
assert.strictEqual(r.status, 0, r.stderr);
115+
assertAccessDenied(parseLastJsonLine(r.stdout));
116+
}
117+
{
118+
const frag = `execArgv: ${JSON.stringify([
119+
'--permission',
120+
`--allow-fs-read=${subFile}`,
121+
'--allow-worker',
122+
])},`;
123+
const r = runWorkerCase(workerRead(subFile), frag);
124+
assert.strictEqual(r.status, 0, r.stderr);
125+
assertOk(parseLastJsonLine(r.stdout));
126+
}
127+
} finally {
128+
fs.rmSync(deniedDir, { recursive: true, force: true });
129+
}

0 commit comments

Comments
 (0)