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

src/runtime/jsc_hooks.rs

@@ -1457,24 +1457,71 @@ unsafe fn apply_standalone_runtime_flags(
 /// 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;
+
+    fn find_short(c: u8) -> Option<&'static bun_clap::Param<bun_clap::Help>> {
+        RUN_PARAMS.iter().find(|p| p.names.short == Some(c))
+    }
+
+    // Does `bytes` name a `RUN_PARAMS` flag whose value is supplied by the
+    // *next* token? `--long=val` and `OneOptional` params never pull from the
+    // iterator. A bare `--long` / `-s` pulls when its `takes_value` is
+    // `One`/`Many`. A chained-short cluster (`-br`) pulls when every prefix
+    // char is a `None`/`OneOptional` short and the last char is a
+    // `One`/`Many` short with nothing after it — `StreamingClap::chainging`
+    // calls `iter.next()` in exactly that case.
+    fn flag_consumes_next_token(bytes: &[u8]) -> bool {
+        if let Some(long) = bytes.strip_prefix(b"--") {
+            let param = RUN_PARAMS.iter().find(|p| p.names.matches_long(long));
+            return matches!(
+                param.map(|p| p.takes_value),
+                Some(Values::One | Values::Many)
+            );
+        }
+        if let Some(shorts) = bytes.strip_prefix(b"-") {
+            for (i, &c) in shorts.iter().enumerate() {
+                let Some(param) = find_short(c) else {
+                    return false;
+                };
+                match param.takes_value {
+                    Values::None | Values::OneOptional => continue,
+                    // `One`/`Many`: pulls from the iterator only when this is
+                    // the last char; otherwise the rest of the cluster (or
+                    // the text after `=`) is the inline value.
+                    Values::One | Values::Many => return i + 1 == shorts.len(),
+                }
+            }
+        }
+        false
+    }
+
     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.
+        // The previous `One`/`Many` flag consumes this token as its value via
+        // raw `iter.next()` — regardless of a leading `-` or a literal `--`.
+        if core::mem::take(&mut prev_wants_value) {
+            continue;
+        }
+        // `stop_after_positional_at = 1` — first positional ends parsing.
         if bytes.first() != Some(&b'-') {
             break;
         }
@@ -1484,6 +1531,7 @@ unsafe fn parse_worker_exec_argv_allow_addons(
         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/no-addons.test.ts

@@ -1,6 +1,7 @@
 import { spawnSync } from "bun";
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
 import { bunExe, bunEnv as env } from "harness";
+import { Worker } from "node:worker_threads";
 
 test("--no-addons throws an error on process.dlopen", () => {
   const { stdout, stderr, exitCode } = spawnSync({
@@ -15,3 +16,40 @@ test("--no-addons throws an error on process.dlopen", () => {
   expect(out).toBeEmpty();
   expect(err).toContain("\nerror: Cannot load native addon because loading addons is disabled.");
 });
+
+describe("worker execArgv --no-addons parsing matches RunCommand clap", () => {
+  // A value token following a value-taking flag (`-r x`, `--title x`, ...)
+  // must not be treated as the first positional when scanning execArgv, and
+  // the value is consumed regardless of its own content (even `--` or a
+  // `-`-prefixed string).
+  const body = `try { process.dlopen({ exports: {} }, "/nonexistent.node"); } catch (e) { require("node:worker_threads").parentPort.postMessage(e.code); }`;
+  test.each([
+    [["--no-addons"], "ERR_DLOPEN_DISABLED"],
+    [["-r", "./preload.js", "--no-addons"], "ERR_DLOPEN_DISABLED"],
+    [["--require", "./preload.js", "--no-addons"], "ERR_DLOPEN_DISABLED"],
+    [["--title", "foo", "--no-addons"], "ERR_DLOPEN_DISABLED"],
+    [["--port", "3000", "--no-addons"], "ERR_DLOPEN_DISABLED"],
+    [["-e", "void 0", "--no-addons"], "ERR_DLOPEN_DISABLED"],
+    [["--no-addons", "-r", "./preload.js"], "ERR_DLOPEN_DISABLED"],
+    // chained shorts ending in a value-taking short pull the next token
+    [["-br", "./preload.js", "--no-addons"], "ERR_DLOPEN_DISABLED"],
+    // the value is consumed via raw iter.next(), even if it is `--`
+    [["-r", "--", "--no-addons"], "ERR_DLOPEN_DISABLED"],
+    // here `--no-addons` is `-r`'s value, not a flag; addons stay enabled
+    [["-r", "--no-addons"], "ERR_DLOPEN_FAILED"],
+  ])("%j", async (execArgv, expected) => {
+    const worker = new Worker(body, { eval: true, execArgv });
+    try {
+      const code = await new Promise((resolve, reject) => {
+        worker.once("message", resolve);
+        worker.once("error", reject);
+        worker.once("exit", exitCode =>
+          reject(new Error(`worker exited (code=${exitCode}) before posting a message`)),
+        );
+      });
+      expect(code).toBe(expected);
+    } finally {
+      await worker.terminate();
+    }
+  });
+});