Skip to content

Commit 1ee70f6

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. Signed-off-by: yunshingng <yunshingng25@gmail.com>
1 parent 1309975 commit 1ee70f6

4 files changed

Lines changed: 662 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: 238 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"
@@ -503,6 +504,235 @@ Worker::~Worker() {
503504
Debug(this, "Worker %llu destroyed", thread_id_.id);
504505
}
505506

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

915+
// Explicit execArgv only (including []). Default Worker path unchanged.
916+
if (env->permission()->enabled() && per_isolate_opts &&
917+
args[2]->IsArray()) {
918+
ClampWorkerPermissionToParent(env, per_isolate_opts.get());
919+
RebuildExecArgvOutFromPermissionOptions(per_isolate_opts.get(),
920+
&exec_argv_out);
921+
}
922+
685923
// Internal workers should not wait for inspector frontend to connect or
686924
// break on the first line of internal scripts. Module loader threads are
687925
// essential to load user codes and must not be blocked by the inspector

0 commit comments

Comments
 (0)