worker_threads: honor --no-addons in execArgv after value-taking flags

src/runtime/jsc_hooks.rs

@@ -1457,26 +1457,56 @@
 /// reads the single `--no-addons` flag from the result (per the in-tree
 /// `// TODO: currently this only checks for --no-addons`), so this body scans
 /// the converted argv directly with the same `stop_after_positional_at = 1`
-/// short-circuit. Full clap routing can return when `ComptimeClap` grows a
-/// borrowed-lifetime variant.
+/// short-circuit — consulting `RUN_PARAMS` so a value token following a
+/// value-taking flag (`-r x`, `--title x`, …) is consumed rather than treated
+/// as the first positional. Full clap routing can return when `ComptimeClap`
+/// grows a borrowed-lifetime variant.
 ///
 /// # Safety
 /// Each `WTFStringImpl` in `exec_argv` is a live WTF string (the C++
 /// `Worker::create` array, kept alive for the worker's lifetime).
 unsafe fn parse_worker_exec_argv_allow_addons(
     exec_argv: &[bun_core::WTFStringImpl],
 ) -> Option<bool> {
+    use crate::cli::arguments::RUN_PARAMS;
+    use bun_clap::Values;
+
+    // Does `bytes` name a `RUN_PARAMS` flag whose value is supplied by the
+    // *next* token? True only for the bare `--long` / `-s` spellings of a
+    // `One`/`Many` param; `--long=val`, `-s=val`, `-sval`, chained shorts and
+    // `OneOptional` params all carry their value (if any) inline and do not
+    // pull from the iterator in `StreamingClap`.
+    fn flag_consumes_next_token(bytes: &[u8]) -> bool {
+        let param = if let Some(long) = bytes.strip_prefix(b"--") {
+            RUN_PARAMS.iter().find(|p| p.names.matches_long(long))
+        } else if bytes.len() == 2 && bytes[0] == b'-' {
+            RUN_PARAMS.iter().find(|p| p.names.short == Some(bytes[1]))
+        } else {
+            None
+        };
+        matches!(
+            param.map(|p| p.takes_value),
+            Some(Values::One | Values::Many)
+        )
+    }
+
     let mut no_addons = false;
+    let mut prev_wants_value = false;
     for &arg in exec_argv {
         if arg.is_null() {
             continue;
         }
         // SAFETY: per fn contract — `arg` is a live `WTFStringImpl*`.
         let owned = unsafe { &*arg }.to_owned_slice_z();
         let bytes = owned.as_bytes();
-        // `stop_after_positional_at = 1` — first non-flag token ends parsing.
+        // `stop_after_positional_at = 1` — first positional ends parsing. A
+        // non-`-` token that the previous flag consumes as its value is not a
+        // positional.
         if bytes.first() != Some(&b'-') {
+            if core::mem::take(&mut prev_wants_value) {
+                continue;
+            }
             break;
         }
         if bytes == b"--" {
             break;
@@ -1484,6 +1514,7 @@
         if bytes == b"--no-addons" {
             no_addons = true;
         }
+        prev_wants_value = flag_consumes_next_token(bytes);
     }
     // Override `allow_addons` unconditionally on successful parse.
     Some(!no_addons)

test/js/node/worker_threads/worker_threads.test.ts

@@ -278,6 +278,32 @@ describe("execArgv option", async () => {
     await run('["--no-warnings"]', '["--no-warnings"]\n');
   });
   // TODO(@190n) get our handling of non-string array elements in line with Node's
+
+  describe("--no-addons is honored after a value-taking flag", () => {
+    // The value token for `-r` / `--title` / `--port` / `-e` must not be
+    // treated as the first positional when parsing the worker's execArgv.
+    const body = `try { process.dlopen({ exports: {} }, "/nonexistent.node"); } catch (e) { require("node:worker_threads").parentPort.postMessage(e.code); }`;
+    it.each([
+      [["--no-addons"]],
+      [["-r", "./preload.js", "--no-addons"]],
+      [["--require", "./preload.js", "--no-addons"]],
+      [["--title", "foo", "--no-addons"]],
+      [["--port", "3000", "--no-addons"]],
+      [["-e", "void 0", "--no-addons"]],
+      [["--no-addons", "-r", "./preload.js"]],
+    ])("%j", async execArgv => {
+      const worker = new Worker(body, { eval: true, execArgv });
+      try {
+        const code = await new Promise((resolve, reject) => {
+          worker.on("message", resolve);
+          worker.on("error", reject);
+        });
+        expect(code).toBe("ERR_DLOPEN_DISABLED");
+      } finally {
+        await worker.terminate();
+      }
+    });
+  });
 });
 
 test("eval does not leak source code", async () => {