permission: clamp Worker grants for explicit execArgv (SEMVER-MAJOR)

When the parent runs with the Permission Model, an explicit Worker execArgv
(including []) cannot drop or exceed the parent's permission-related grants.

- No worker permission flags → apply parent grant ceiling
- Worker permission flags set → intersect (restrict OK, escalate no)
- Rebuild permission argv from clamped options; drop space-form path tokens
- Default Worker (omit execArgv) unchanged

Signed-off-by: yunshingng <yunshingng25@gmail.com>

Author: yunshingng

src/node_worker.cc

@@ -507,20 +507,29 @@ Worker::~Worker() {
 
 
 // SEMVER-MAJOR: Permission ceiling for Worker explicit execArgv.
-static bool WorkerConfiguredPermission(const EnvironmentOptions* w) {
-  if (w->permission || w->permission_audit) {
-    return true;
-  }
-  if (!w->allow_fs_read.empty() || !w->allow_fs_write.empty()) {
-    return true;
-  }
+// SEMVER-MAJOR: Permission ceiling for Worker when execArgv is explicit
+// (including []). Default Worker (no execArgv) is unchanged.
+//
+// After options parse, NODE_OPTIONS and repeated --allow-* are already in
+// EnvironmentOptions. Runtime FSPermission remains authoritative for FS checks;
+// path filtering here is create-time only (prefix / exact / *).
+//
+// Parent allow-list containing "*" already means unrestricted FS for that
+// dimension; FilterPathList early-return keeps worker paths (cannot exceed *).
+
+namespace {
+
+bool WorkerConfiguredPermission(const EnvironmentOptions* w) {
+  if (w == nullptr) return false;
+  if (w->permission || w->permission_audit) return true;
+  if (!w->allow_fs_read.empty() || !w->allow_fs_write.empty()) return true;
   return w->allow_addons || w->allow_inspector || w->allow_child_process ||
          w->allow_net || w->allow_wasi || w->allow_ffi ||
          w->allow_openssl_store || w->allow_worker_threads;
 }
 
-static void ApplyParentPermissionCeiling(EnvironmentOptions* w,
-                                         const EnvironmentOptions* parent) {
+void ApplyParentPermissionCeiling(EnvironmentOptions* w,
+                                  const EnvironmentOptions* parent) {
   w->permission = true;
   w->permission_audit = parent->permission_audit;
   w->allow_addons = parent->allow_addons;
@@ -535,87 +544,67 @@ static void ApplyParentPermissionCeiling(EnvironmentOptions* w,
   w->allow_fs_write = parent->allow_fs_write;
 }
 
-static void NormalizePathForCompare(std::string* s) {
-  while (s->size() > 1 && (s->back() == '/' || s->back() == '\\')) {
+void NormalizePathForCompare(std::string* s) {
+  while (s->size() > 1 && (s->back() == '/' || s->back() == '\')) {
     s->pop_back();
   }
 #ifdef _WIN32
   for (char& c : *s) {
     if (c >= 'A' && c <= 'Z') {
       c = static_cast<char>(c - 'A' + 'a');
     }
-    if (c == '/') {
-      c = '\\';
-    }
+    if (c == '/') c = '\';
   }
 #endif
 }
 
-static std::string ResolveForCompare(Environment* env, const std::string& in) {
-  if (in.empty() || in == "*") {
-    return in;
-  }
+std::string ResolveForCompare(Environment* env, const std::string& in) {
+  if (in.empty() || in == "*") return in;
   std::string resolved =
       PathResolve(env, std::vector<std::string_view>{std::string_view(in)});
-  if (resolved.empty()) {
-    resolved = in;
-  }
+  if (resolved.empty()) resolved = in;
   NormalizePathForCompare(&resolved);
   return resolved;
 }
 
-// Create-time prefix filter; runtime FSPermission is authoritative.
-static bool PathCoveredByParentEntry(Environment* env,
-                                     const std::string& parent_raw,
-                                     const std::string& requested_raw) {
-  if (parent_raw == "*") {
-    return true;
-  }
+// Create-time filter only.
+bool PathCoveredByParentEntry(Environment* env,
+                              const std::string& parent_raw,
+                              const std::string& requested_raw) {
+  if (parent_raw == "*") return true;
   const std::string parent = ResolveForCompare(env, parent_raw);
   const std::string requested = ResolveForCompare(env, requested_raw);
-  if (parent.empty()) {
-    return false;
-  }
-  if (requested == parent) {
-    return true;
-  }
-  if (requested.size() <= parent.size()) {
-    return false;
-  }
-  if (requested.compare(0, parent.size(), parent) != 0) {
-    return false;
-  }
+  if (parent.empty()) return false;
+  if (requested == parent) return true;
+  if (requested.size() <= parent.size()) return false;
+  if (requested.compare(0, parent.size(), parent) != 0) return false;
   const char next = requested[parent.size()];
-  return next == '/' || next == '\\';
+  return next == '/' || next == '\';
 }
 
-static bool ParentListHasWildcard(const std::vector<std::string>& parent) {
+bool ParentListHasWildcard(const std::vector<std::string>& parent) {
   for (const std::string& entry : parent) {
-    if (entry == "*") {
-      return true;
-    }
+    if (entry == "*") return true;
   }
   return false;
 }
 
-static void FilterPathListToParentSubset(
-    Environment* env,
-    EnvironmentOptions* w,
-    std::vector<std::string>* worker,
-    const std::vector<std::string>& parent) {
-  if (worker == nullptr) {
-    return;
-  }
+void FilterPathListToParentSubset(Environment* env,
+                                  EnvironmentOptions* w,
+                                  std::vector<std::string>* worker,
+                                  const std::vector<std::string>& parent) {
+  if (worker == nullptr) return;
+
+  // Worker enabled permission but listed no paths → keep empty (restrict).
   if (worker->empty()) {
-    if (w->permission || w->permission_audit) {
-      return;
-    }
+    if (w->permission || w->permission_audit) return;
     *worker = parent;
     return;
   }
-  if (ParentListHasWildcard(parent)) {
-    return;
-  }
+
+  // Parent "*" → FS already unrestricted; worker paths cannot exceed parent.
+  if (ParentListHasWildcard(parent)) return;
+
   std::vector<std::string> out;
   out.reserve(worker->size());
   bool saw_star = false;
@@ -638,11 +627,12 @@ static void FilterPathListToParentSubset(
   *worker = std::move(out);
 }
 
-static void IntersectPermissionGrants(Environment* env,
-                                      EnvironmentOptions* w,
-                                      const EnvironmentOptions* parent) {
+void IntersectPermissionGrants(Environment* env,
+                               EnvironmentOptions* w,
+                               const EnvironmentOptions* parent) {
   w->permission = true;
   w->permission_audit = w->permission_audit || parent->permission_audit;
+
   w->allow_addons = w->allow_addons && parent->allow_addons;
   w->allow_inspector = w->allow_inspector && parent->allow_inspector;
   w->allow_child_process =
@@ -654,32 +644,33 @@ static void IntersectPermissionGrants(Environment* env,
       w->allow_openssl_store && parent->allow_openssl_store;
   w->allow_worker_threads =
       w->allow_worker_threads && parent->allow_worker_threads;
+
   FilterPathListToParentSubset(env, w, &w->allow_fs_read, parent->allow_fs_read);
   FilterPathListToParentSubset(
       env, w, &w->allow_fs_write, parent->allow_fs_write);
 }
 
-static void ClampWorkerPermissionToParent(Environment* env,
-                                          PerIsolateOptions* worker_opts) {
-  if (worker_opts == nullptr || !env->permission()->enabled()) {
+void ClampWorkerPermissionToParent(Environment* env,
+                                   PerIsolateOptions* worker_opts) {
+  if (worker_opts == nullptr || env == nullptr ||
+      !env->permission()->enabled()) {
     return;
   }
-  EnvironmentOptions* parent = env->isolate_data()->options()->get_per_env_options();
+  EnvironmentOptions* parent =
+      env->isolate_data()->options()->get_per_env_options();
   EnvironmentOptions* w = worker_opts->get_per_env_options();
-  if (parent == nullptr || w == nullptr) {
-    return;
-  }
+  if (parent == nullptr || w == nullptr) return;
+
   if (!WorkerConfiguredPermission(w)) {
     ApplyParentPermissionCeiling(w, parent);
   } else {
     IntersectPermissionGrants(env, w, parent);
   }
 }
 
-static bool IsPermissionCliToken(const std::string& a) {
-  if (a == "--permission" || a == "--permission-audit") {
-    return true;
-  }
+// Exact flag name or flag=value (not a longer unrelated prefix).
+bool IsPermissionCliToken(const std::string& a) {
+  if (a == "--permission" || a == "--permission-audit") return true;
   static const char* kFlags[] = {
       "--allow-fs-read",
       "--allow-fs-write",
@@ -694,74 +685,75 @@ static bool IsPermissionCliToken(const std::string& a) {
   };
   for (const char* flag : kFlags) {
     const size_t n = std::char_traits<char>::length(flag);
-    if (a == flag) {
-      return true;
-    }
-    if (a.size() > n && a.compare(0, n, flag) == 0 && a[n] == '=') {
-      return true;
-    }
+    if (a == flag) return true;
+    if (a.size() > n && a.compare(0, n, flag) == 0 && a[n] == '=') return true;
   }
   return false;
 }
 
-// Rebuild argv from clamped options. Do not assume argv[0] layout from Parse.
-static void RebuildExecArgvOutFromPermissionOptions(
-    PerIsolateOptions* worker_opts, std::vector<std::string>* exec_argv_out) {
-  if (worker_opts == nullptr || exec_argv_out == nullptr) {
-    return;
-  }
-  EnvironmentOptions* w = worker_opts->get_per_env_options();
-  if (w == nullptr || !w->permission) {
-    return;
+bool PermissionFlagTakesNextArg(const std::string& a) {
+  return a == "--allow-fs-read" || a == "--allow-fs-write";
+}
+
+bool PathSafeForAllowFlag(const std::string& path) {
+  if (path.empty()) return false;
+  for (char c : path) {
+    if (c == '
+' || c == '') return false;
   }
+  return true;
+}
 
-  std::vector<std::string> out;
-  out.emplace_back("");  // program-name placeholder for parsers that expect it
-  for (const std::string& tok : *exec_argv_out) {
-    if (tok.empty()) {
+void RebuildExecArgvOutFromPermissionOptions(
+    PerIsolateOptions* worker_opts, std::vector<std::string>* exec_argv_out) {
+  if (worker_opts == nullptr || exec_argv_out == nullptr) return;
+  EnvironmentOptions* w = worker_opts->get_per_env_options();
+  if (w == nullptr || !w->permission) return;
+
+  std::vector<std::string> kept;
+  kept.reserve(exec_argv_out->size());
+  for (size_t i = 0; i < exec_argv_out->size(); ++i) {
+    const std::string& tok = (*exec_argv_out)[i];
+    if (tok.empty()) continue;
+    if (IsPermissionCliToken(tok)) {
+      if (PermissionFlagTakesNextArg(tok) && i + 1 < exec_argv_out->size()) {
+        const std::string& next = (*exec_argv_out)[i + 1];
+        if (!next.empty() && next[0] != '-') ++i;
+      }
       continue;
     }
-    if (!IsPermissionCliToken(tok)) {
-      out.push_back(tok);
-    }
+    kept.push_back(tok);
   }
+
+  std::vector<std::string> out;
+  out.reserve(kept.size() + 16 + w->allow_fs_read.size() +
+              w->allow_fs_write.size());
+  out.emplace_back("");
+  for (const std::string& tok : kept) out.push_back(tok);
+
   out.push_back("--permission");
-  if (w->permission_audit) {
-    out.push_back("--permission-audit");
-  }
-  if (w->allow_addons) {
-    out.push_back("--allow-addons");
-  }
-  if (w->allow_inspector) {
-    out.push_back("--allow-inspector");
-  }
-  if (w->allow_child_process) {
-    out.push_back("--allow-child-process");
-  }
-  if (w->allow_net) {
-    out.push_back("--allow-net");
-  }
-  if (w->allow_wasi) {
-    out.push_back("--allow-wasi");
-  }
-  if (w->allow_ffi) {
-    out.push_back("--allow-ffi");
-  }
-  if (w->allow_openssl_store) {
-    out.push_back("--allow-openssl-store");
-  }
-  if (w->allow_worker_threads) {
-    out.push_back("--allow-worker");
-  }
-  for (const std::string& path : w->allow_fs_read) {
-    out.push_back("--allow-fs-read=" + path);
-  }
-  for (const std::string& path : w->allow_fs_write) {
-    out.push_back("--allow-fs-write=" + path);
+  if (w->permission_audit) out.push_back("--permission-audit");
+  if (w->allow_addons) out.push_back("--allow-addons");
+  if (w->allow_inspector) out.push_back("--allow-inspector");
+  if (w->allow_child_process) out.push_back("--allow-child-process");
+  if (w->allow_net) out.push_back("--allow-net");
+  if (w->allow_wasi) out.push_back("--allow-wasi");
+  if (w->allow_ffi) out.push_back("--allow-ffi");
+  if (w->allow_openssl_store) out.push_back("--allow-openssl-store");
+  if (w->allow_worker_threads) out.push_back("--allow-worker");
+  for (const std::string& p : w->allow_fs_read) {
+    if (!PathSafeForAllowFlag(p)) continue;
+    out.push_back("--allow-fs-read=" + p);
+  }
+  for (const std::string& p : w->allow_fs_write) {
+    if (!PathSafeForAllowFlag(p)) continue;
+    out.push_back("--allow-fs-write=" + p);
   }
   *exec_argv_out = std::move(out);
 }
 
+}  // namespace
+
 
 void Worker::New(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
@@ -944,13 +936,16 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
 
   // Only explicit execArgv (including []): clamp options + rebuild argv.
   // Default Worker path (clone parent) is left unchanged.
-  if (env->permission()->enabled() && per_isolate_opts && args[2]->IsArray()) {
+
+  // // Explicit execArgv only (including []). Default Worker path unchanged.
+  if (env->permission()->enabled() && per_isolate_opts &&
+      args[2]->IsArray()) {
     ClampWorkerPermissionToParent(env, per_isolate_opts.get());
     RebuildExecArgvOutFromPermissionOptions(per_isolate_opts.get(),
                                             &exec_argv_out);
   }
 
-  // Internal workers should not wait for inspector frontend to connect or
+  Internal workers should not wait for inspector frontend to connect or
   // break on the first line of internal scripts. Module loader threads are
   // essential to load user codes and must not be blocked by the inspector
   // for internal scripts.