Skip to content

Commit d015e51

Browse files
committed
permission: clamp Worker grants for explicit execArgv (SEMVER-MAJOR)
When the parent has the Permission Model enabled, an explicit Worker execArgv (including []) cannot obtain a wider permission-related grant set than the parent. Default Worker (no execArgv) is unchanged. Keep the main-branch options gate so custom env / NODE_OPTIONS and execArgv: [] still fresh-parse; apply the permission ceiling afterward for any explicit execArgv. Refs: #65359 Signed-off-by: NG YUN SHING <yunshingng25@gmail.com>
1 parent 1309975 commit d015e51

4 files changed

Lines changed: 675 additions & 0 deletions

File tree

doc/api/permissions.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@ 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+
4149
> Stability: 2 - Stable
4250
4351
The Node.js Permission Model is a mechanism for restricting access to specific

doc/api/worker_threads.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,6 +1605,12 @@ 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+
16081614
* `filename` {string|URL} The path to the Worker's main script or module. Must
16091615
be either an absolute path or a relative path (i.e. relative to the
16101616
current working directory) starting with `./` or `../`, or a WHATWG `URL`

src/node_worker.cc

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "node_perf.h"
1111
#include "node_profiling.h"
1212
#include "node_snapshot_builder.h"
13+
#include "path.h"
1314
#include "permission/permission.h"
1415
#include "util-inl.h"
1516
#include "v8-cppgc.h"
@@ -503,6 +504,243 @@ Worker::~Worker() {
503504
Debug(this, "Worker %llu destroyed", thread_id_.id);
504505
}
505506

507+
// SEMVER-MAJOR: Permission ceiling for Worker when execArgv is explicit
508+
// (including []). Default Worker (no execArgv) is unchanged.
509+
//
510+
// After options parse, NODE_OPTIONS and repeated --allow-* are already in
511+
// EnvironmentOptions. Runtime FSPermission remains authoritative for FS
512+
// checks; path filtering here is create-time only (prefix / exact / *).
513+
//
514+
// Boolean --allow-* dimensions are listed once in PERMISSION_BOOL_FLAGS so
515+
// ceiling / intersect / CLI token / rebuild cannot drift.
516+
517+
namespace {
518+
519+
// Single source of truth for boolean permission dimensions (not fs path
520+
// lists, which are handled separately since they're not simple booleans).
521+
#define PERMISSION_BOOL_FLAGS(V) \
522+
V(allow_addons, "--allow-addons") \
523+
V(allow_inspector, "--allow-inspector") \
524+
V(allow_child_process, "--allow-child-process") \
525+
V(allow_net, "--allow-net") \
526+
V(allow_wasi, "--allow-wasi") \
527+
V(allow_ffi, "--allow-ffi") \
528+
V(allow_openssl_store, "--allow-openssl-store") \
529+
V(allow_worker_threads, "--allow-worker")
530+
531+
bool WorkerConfiguredPermission(const EnvironmentOptions* w) {
532+
if (w == nullptr) return false;
533+
if (w->permission || w->permission_audit) return true;
534+
if (!w->allow_fs_read.empty() || !w->allow_fs_write.empty()) return true;
535+
#define V(field, flag) || w->field
536+
return false PERMISSION_BOOL_FLAGS(V);
537+
#undef V
538+
}
539+
540+
void ApplyParentPermissionCeiling(EnvironmentOptions* w,
541+
const EnvironmentOptions* parent) {
542+
w->permission = true;
543+
w->permission_audit = parent->permission_audit;
544+
#define V(field, flag) w->field = parent->field;
545+
PERMISSION_BOOL_FLAGS(V)
546+
#undef V
547+
w->allow_fs_read = parent->allow_fs_read;
548+
w->allow_fs_write = parent->allow_fs_write;
549+
}
550+
551+
void NormalizePathForCompare(std::string* s) {
552+
while (s->size() > 1 &&
553+
(s->back() == '/' || s->back() == static_cast<char>(92))) {
554+
s->pop_back();
555+
}
556+
#ifdef _WIN32
557+
for (char& c : *s) {
558+
if (c >= 'A' && c <= 'Z') {
559+
c = static_cast<char>(c - 'A' + 'a');
560+
}
561+
if (c == '/') c = static_cast<char>(92);
562+
}
563+
#endif
564+
}
565+
566+
std::string ResolveForCompare(Environment* env, const std::string& in) {
567+
if (in.empty() || in == "*") return in;
568+
std::string resolved =
569+
PathResolve(env, std::vector<std::string_view>{std::string_view(in)});
570+
if (resolved.empty()) resolved = in;
571+
NormalizePathForCompare(&resolved);
572+
return resolved;
573+
}
574+
575+
bool ParentEntryCoversResolvedPath(Environment* env,
576+
const std::string& parent_raw,
577+
const std::string& resolved_requested) {
578+
if (parent_raw == "*") return true;
579+
const std::string parent = ResolveForCompare(env, parent_raw);
580+
if (parent.empty()) return false;
581+
if (resolved_requested == parent) return true;
582+
if (resolved_requested.size() <= parent.size()) return false;
583+
if (resolved_requested.compare(0, parent.size(), parent) != 0) return false;
584+
const char next = resolved_requested[parent.size()];
585+
return next == '/' || next == static_cast<char>(92);
586+
}
587+
588+
bool ParentListHasWildcard(const std::vector<std::string>& parent) {
589+
for (const std::string& entry : parent) {
590+
if (entry == "*") return true;
591+
}
592+
return false;
593+
}
594+
595+
void FilterPathListToParentSubset(Environment* env,
596+
EnvironmentOptions* w,
597+
std::vector<std::string>* worker,
598+
const std::vector<std::string>& parent) {
599+
if (worker == nullptr) return;
600+
// Worker listed no fs paths → keep empty (restrict).
601+
if (worker->empty()) return;
602+
// Parent "*" → FS already unrestricted; worker paths cannot exceed parent.
603+
if (ParentListHasWildcard(parent)) return;
604+
605+
std::vector<std::string> out;
606+
out.reserve(worker->size());
607+
bool saw_star = false;
608+
for (const std::string& wpath : *worker) {
609+
if (wpath == "*") {
610+
saw_star = true;
611+
continue;
612+
}
613+
const std::string resolved_wpath = ResolveForCompare(env, wpath);
614+
for (const std::string& entry : parent) {
615+
if (ParentEntryCoversResolvedPath(env, entry, resolved_wpath)) {
616+
out.push_back(resolved_wpath);
617+
break;
618+
}
619+
}
620+
}
621+
// "*" alone or combined with concrete paths still means "everything the
622+
// parent allows" here, not "just the concrete paths that also matched" —
623+
// treating it as a subset would silently grant *less* than requesting "*"
624+
// by itself, which is backwards. See PR discussion for why this needs to
625+
// be unconditional on saw_star, not just "saw_star && out.empty()".
626+
if (saw_star) {
627+
*worker = parent;
628+
return;
629+
}
630+
*worker = std::move(out);
631+
}
632+
633+
void IntersectPermissionGrants(Environment* env,
634+
EnvironmentOptions* w,
635+
const EnvironmentOptions* parent) {
636+
w->permission = true;
637+
w->permission_audit = w->permission_audit || parent->permission_audit;
638+
#define V(field, flag) w->field = w->field && parent->field;
639+
PERMISSION_BOOL_FLAGS(V)
640+
#undef V
641+
FilterPathListToParentSubset(
642+
env, w, &w->allow_fs_read, parent->allow_fs_read);
643+
FilterPathListToParentSubset(
644+
env, w, &w->allow_fs_write, parent->allow_fs_write);
645+
}
646+
647+
void ClampWorkerPermissionToParent(Environment* env,
648+
PerIsolateOptions* worker_opts) {
649+
if (worker_opts == nullptr || env == nullptr ||
650+
!env->permission()->enabled()) {
651+
return;
652+
}
653+
EnvironmentOptions* parent =
654+
env->isolate_data()->options()->get_per_env_options();
655+
EnvironmentOptions* w = worker_opts->get_per_env_options();
656+
if (parent == nullptr || w == nullptr) return;
657+
658+
if (!WorkerConfiguredPermission(w)) {
659+
ApplyParentPermissionCeiling(w, parent);
660+
} else {
661+
IntersectPermissionGrants(env, w, parent);
662+
}
663+
}
664+
665+
bool IsPermissionCliToken(const std::string& a) {
666+
if (a == "--permission" || a == "--permission-audit") return true;
667+
if (a == "--allow-fs-read" || a == "--allow-fs-write") return true;
668+
if (a.rfind("--allow-fs-read=", 0) == 0) return true;
669+
if (a.rfind("--allow-fs-write=", 0) == 0) return true;
670+
#define V(field, flag) \
671+
if (a == flag) return true; \
672+
{ \
673+
const size_t n = sizeof(flag) - 1; \
674+
if (a.size() > n && a.compare(0, n, flag) == 0 && a[n] == '=') \
675+
return true; \
676+
}
677+
PERMISSION_BOOL_FLAGS(V)
678+
#undef V
679+
return false;
680+
}
681+
682+
bool PermissionFlagTakesNextArg(const std::string& a) {
683+
return a == "--allow-fs-read" || a == "--allow-fs-write";
684+
}
685+
686+
bool PathSafeForAllowFlag(const std::string& path) {
687+
if (path.empty()) return false;
688+
for (unsigned char c : path) {
689+
if (c == 0 || c == 10 || c == 13) return false;
690+
}
691+
return true;
692+
}
693+
694+
void RebuildExecArgvOutFromPermissionOptions(
695+
PerIsolateOptions* worker_opts, std::vector<std::string>* exec_argv_out) {
696+
if (worker_opts == nullptr || exec_argv_out == nullptr) return;
697+
EnvironmentOptions* w = worker_opts->get_per_env_options();
698+
if (w == nullptr || !w->permission) return;
699+
700+
std::vector<std::string> kept;
701+
kept.reserve(exec_argv_out->size());
702+
for (size_t i = 0; i < exec_argv_out->size(); ++i) {
703+
const std::string& tok = (*exec_argv_out)[i];
704+
if (tok.empty()) continue;
705+
if (IsPermissionCliToken(tok)) {
706+
// Space-form --allow-fs-read/--allow-fs-write always consume the next
707+
// token as their argument, regardless of its first character — paths
708+
// are legal starting with '-' on Unix, so gating on that would leave
709+
// a stray token behind instead of consuming it as the flag's value.
710+
if (PermissionFlagTakesNextArg(tok) && i + 1 < exec_argv_out->size()) {
711+
++i;
712+
}
713+
continue;
714+
}
715+
kept.push_back(tok);
716+
}
717+
718+
std::vector<std::string> out;
719+
out.reserve(kept.size() + 16 + w->allow_fs_read.size() +
720+
w->allow_fs_write.size());
721+
for (const std::string& tok : kept) out.push_back(tok);
722+
723+
out.push_back("--permission");
724+
if (w->permission_audit) out.push_back("--permission-audit");
725+
#define V(field, flag) \
726+
if (w->field) out.push_back(flag);
727+
PERMISSION_BOOL_FLAGS(V)
728+
#undef V
729+
for (const std::string& p : w->allow_fs_read) {
730+
if (!PathSafeForAllowFlag(p)) continue;
731+
out.push_back("--allow-fs-read=" + p);
732+
}
733+
for (const std::string& p : w->allow_fs_write) {
734+
if (!PathSafeForAllowFlag(p)) continue;
735+
out.push_back("--allow-fs-write=" + p);
736+
}
737+
*exec_argv_out = std::move(out);
738+
}
739+
740+
#undef PERMISSION_BOOL_FLAGS
741+
742+
} // namespace
743+
506744
void Worker::New(const FunctionCallbackInfo<Value>& args) {
507745
Environment* env = Environment::GetCurrent(args);
508746
THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -559,6 +797,11 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
559797
THROW_ERR_OPERATION_FAILED(env, "Failed to copy environment variables");
560798
}
561799

800+
// Keep the main-branch gate so custom env / NODE_OPTIONS and execArgv
801+
// (including []) still go through fresh option parsing. Empty execArgv
802+
// must not skip that path — only the permission ceiling below differs.
803+
const bool explicit_exec_argv = args[2]->IsArray();
804+
562805
if (args[1]->IsObject() || args[2]->IsArray()) {
563806
per_isolate_opts.reset(new PerIsolateOptions());
564807

@@ -682,6 +925,15 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
682925
per_isolate_opts = env->isolate_data()->options()->Clone();
683926
}
684927

928+
// Any explicit execArgv (including []): clamp permission grants to the
929+
// parent. [] stays on the fresh-parse path above so NODE_OPTIONS still
930+
// applies; the ceiling then re-attaches parent permission grants.
931+
if (env->permission()->enabled() && per_isolate_opts && explicit_exec_argv) {
932+
ClampWorkerPermissionToParent(env, per_isolate_opts.get());
933+
RebuildExecArgvOutFromPermissionOptions(per_isolate_opts.get(),
934+
&exec_argv_out);
935+
}
936+
685937
// Internal workers should not wait for inspector frontend to connect or
686938
// break on the first line of internal scripts. Module loader threads are
687939
// essential to load user codes and must not be blocked by the inspector

0 commit comments

Comments
 (0)