Skip to content

Commit fb2cf03

Browse files
committed
permission: intersect when Worker execArgv asks for grants
Detect permission-related tokens from the original execArgv so an explicit `--permission` without fs allows stays on the intersect path instead of the parent ceiling (which re-widened the allowlist). Signed-off-by: NG YUN SHING <yunshingng25@gmail.com>
1 parent 409f2e1 commit fb2cf03

1 file changed

Lines changed: 61 additions & 19 deletions

File tree

src/node_worker.cc

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -645,24 +645,6 @@ void IntersectPermissionGrants(Environment* env,
645645
FilterPathListToParentSubset(env, &w->allow_fs_write, parent->allow_fs_write);
646646
}
647647

648-
void ClampWorkerPermissionToParent(Environment* env,
649-
PerIsolateOptions* worker_opts) {
650-
if (worker_opts == nullptr || env == nullptr ||
651-
!env->permission()->enabled()) {
652-
return;
653-
}
654-
EnvironmentOptions* parent =
655-
env->isolate_data()->options()->get_per_env_options();
656-
EnvironmentOptions* w = worker_opts->get_per_env_options();
657-
if (parent == nullptr || w == nullptr) return;
658-
659-
if (!WorkerConfiguredPermission(w)) {
660-
ApplyParentPermissionCeiling(w, parent);
661-
} else {
662-
IntersectPermissionGrants(env, w, parent);
663-
}
664-
}
665-
666648
bool IsPermissionCliToken(const std::string& a) {
667649
if (a == "--permission" || a == "--permission-audit") return true;
668650
if (a == "--allow-fs-read" || a == "--allow-fs-write") return true;
@@ -680,6 +662,43 @@ bool IsPermissionCliToken(const std::string& a) {
680662
return false;
681663
}
682664

665+
bool ExecArgvHasPermissionToken(const std::vector<std::string>& argv) {
666+
for (const std::string& tok : argv) {
667+
if (IsPermissionCliToken(tok)) return true;
668+
}
669+
return false;
670+
}
671+
672+
void ClampWorkerPermissionToParent(
673+
Environment* env,
674+
PerIsolateOptions* worker_opts,
675+
const std::vector<std::string>& exec_argv_out) {
676+
if (worker_opts == nullptr || env == nullptr ||
677+
!env->permission()->enabled()) {
678+
return;
679+
}
680+
EnvironmentOptions* parent =
681+
env->isolate_data()->options()->get_per_env_options();
682+
EnvironmentOptions* w = worker_opts->get_per_env_options();
683+
if (parent == nullptr || w == nullptr) return;
684+
685+
// Ceiling (inherit parent grants) only when the worker did not ask for any
686+
// permission-related configuration. If execArgv contains permission tokens
687+
// (or options already reflect them), intersect so an explicit
688+
// `--permission` without fs grants stays restrictive instead of being
689+
// widened back to the parent allowlist.
690+
const bool permission_requested =
691+
WorkerConfiguredPermission(w) || ExecArgvHasPermissionToken(exec_argv_out);
692+
if (!permission_requested) {
693+
ApplyParentPermissionCeiling(w, parent);
694+
} else {
695+
if (!w->permission && !w->permission_audit) {
696+
w->permission = true;
697+
}
698+
IntersectPermissionGrants(env, w, parent);
699+
}
700+
}
701+
683702
bool PermissionFlagTakesNextArg(const std::string& a) {
684703
return a == "--allow-fs-read" || a == "--allow-fs-write";
685704
}
@@ -930,7 +949,30 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
930949
// parent. [] stays on the fresh-parse path above so NODE_OPTIONS still
931950
// applies; the ceiling then re-attaches parent permission grants.
932951
if (env->permission()->enabled() && per_isolate_opts && explicit_exec_argv) {
933-
ClampWorkerPermissionToParent(env, per_isolate_opts.get());
952+
// Prefer the original JS execArgv strings for "did the caller ask for
953+
// permission flags?" — Parse may consume known tokens out of
954+
// exec_argv_out, which would otherwise make a restrictive
955+
// `--permission` (no fs grants) look unconfigured and hit the ceiling.
956+
std::vector<std::string> permission_argv_probe = exec_argv_out;
957+
if (args[2]->IsArray()) {
958+
Local<Array> array = args[2].As<Array>();
959+
uint32_t length = array->Length();
960+
for (uint32_t i = 0; i < length; i++) {
961+
Local<Value> arg;
962+
if (!array->Get(env->context(), i).ToLocal(&arg)) {
963+
return;
964+
}
965+
Local<String> arg_v8;
966+
if (!arg->ToString(env->context()).ToLocal(&arg_v8)) {
967+
return;
968+
}
969+
Utf8Value arg_utf8_value(args.GetIsolate(), arg_v8);
970+
permission_argv_probe.emplace_back(arg_utf8_value.out(),
971+
arg_utf8_value.length());
972+
}
973+
}
974+
ClampWorkerPermissionToParent(
975+
env, per_isolate_opts.get(), permission_argv_probe);
934976
RebuildExecArgvOutFromPermissionOptions(per_isolate_opts.get(),
935977
&exec_argv_out);
936978
}

0 commit comments

Comments
 (0)