From d17c9f2056fb0d093ebf02ab4d37563aa7e43eda Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 3 Apr 2026 19:22:13 +0000 Subject: [PATCH 1/5] Support NODE_OPTIONS environment variable Node.js honors flags passed via the NODE_OPTIONS env var. Bun ignored it entirely, so things like NODE_OPTIONS=--dns-result-order=ipv4first were dropped and 'bun install' would hang on hosts with broken IPv6. Parse NODE_OPTIONS (quote- and escape-aware, like Node) and inject the tokens into argv before clap parses them. Only flags on an allowlist of Bun-supported Node-compatible options are honored; positionals and unknown flags are dropped so the env var can't be used to inject scripts or change the entrypoint. Also fixes dns.getDefaultResultOrder() to return the order string ("verbatim"/"ipv4first"/"ipv6first") instead of the internal function. Fixes #28817 --- src/bun.zig | 161 ++++++++++++++++++++++++++++ src/bun_core/env_var.zig | 1 + test/regression/issue/28817.test.ts | 119 ++++++++++++++++++++ 3 files changed, 281 insertions(+) create mode 100644 test/regression/issue/28817.test.ts diff --git a/src/bun.zig b/src/bun.zig index 5a3132ed8c75..f69ce3dcc812 100644 --- a/src/bun.zig +++ b/src/bun.zig @@ -2148,6 +2148,157 @@ pub fn appendOptionsEnv(env: []const u8, comptime ArgType: type, args: *std.arra } } +/// Kind of flag in `NODE_OPTIONS`: either a boolean flag (no value) or an +/// option that takes a value. Used by `appendNodeOptionsEnv` to decide +/// whether the next token should be consumed as a value. +const NodeOptionKind = enum { bool_flag, option }; + +/// Allowlist of flags supported by Bun that may appear in `NODE_OPTIONS`. +/// +/// Matches Node.js's behavior of restricting `NODE_OPTIONS` to a subset of +/// flags (for security and predictability). We include only flags that Bun +/// actually implements — unknown flags are silently ignored. +pub const node_options_allowlist = ComptimeStringMap(NodeOptionKind, .{ + // Boolean flags (no value) + .{ "--expose-gc", .bool_flag }, + .{ "--no-addons", .bool_flag }, + .{ "--no-deprecation", .bool_flag }, + .{ "--preserve-symlinks", .bool_flag }, + .{ "--preserve-symlinks-main", .bool_flag }, + .{ "--throw-deprecation", .bool_flag }, + .{ "--use-bundled-ca", .bool_flag }, + .{ "--use-openssl-ca", .bool_flag }, + .{ "--use-system-ca", .bool_flag }, + .{ "--zero-fill-buffers", .bool_flag }, + .{ "--cpu-prof", .bool_flag }, + .{ "--cpu-prof-md", .bool_flag }, + .{ "--heap-prof", .bool_flag }, + .{ "--heap-prof-md", .bool_flag }, + + // Options that take a value + .{ "--conditions", .option }, + .{ "-C", .option }, + .{ "--cpu-prof-dir", .option }, + .{ "--cpu-prof-interval", .option }, + .{ "--cpu-prof-name", .option }, + .{ "--dns-result-order", .option }, + .{ "--heap-prof-dir", .option }, + .{ "--heap-prof-name", .option }, + .{ "--import", .option }, + .{ "--inspect", .option }, + .{ "--inspect-brk", .option }, + .{ "--inspect-wait", .option }, + .{ "--max-http-header-size", .option }, + .{ "--require", .option }, + .{ "-r", .option }, + .{ "--title", .option }, + .{ "--unhandled-rejections", .option }, +}); + +/// Parses `NODE_OPTIONS` env var into command-line tokens filtered by +/// `node_options_allowlist` and inserts them into `args` starting at +/// `offset` (after argv[0]). +/// +/// Splits on whitespace with support for single/double quotes and backslash +/// escapes, matching Node.js's NODE_OPTIONS parsing behavior. Positional +/// (non-flag) tokens and unrecognized flags are dropped to prevent +/// unintended script execution via the environment. +pub fn appendNodeOptionsEnv(env: []const u8, args: *std.array_list.Managed([:0]const u8)) !void { + // First, tokenize the NODE_OPTIONS string (quote- and escape-aware). + var tokens = std.array_list.Managed([]u8).init(bun.default_allocator); + defer { + for (tokens.items) |t| bun.default_allocator.free(t); + tokens.deinit(); + } + + var buf = std.array_list.Managed(u8).init(bun.default_allocator); + defer buf.deinit(); + + var in_quote: ?u8 = null; + var escape = false; + var has_token = false; + + var i: usize = 0; + while (i < env.len) : (i += 1) { + const ch = env[i]; + + if (escape) { + try buf.append(ch); + escape = false; + has_token = true; + continue; + } + + if (ch == '\\') { + escape = true; + continue; + } + + if (in_quote) |q| { + if (ch == q) { + in_quote = null; + } else { + try buf.append(ch); + has_token = true; + } + continue; + } + + if (ch == '\'' or ch == '"') { + in_quote = ch; + has_token = true; + continue; + } + + if (std.ascii.isWhitespace(ch)) { + if (has_token) { + try tokens.append(try buf.toOwnedSlice()); + has_token = false; + } + continue; + } + + try buf.append(ch); + has_token = true; + } + if (has_token) { + try tokens.append(try buf.toOwnedSlice()); + } + + // Now filter tokens through the allowlist and insert into args. + var offset: usize = 1; + var j: usize = 0; + while (j < tokens.items.len) : (j += 1) { + const token = tokens.items[j]; + if (token.len == 0 or token[0] != '-') { + // Positional args are not allowed in NODE_OPTIONS. + continue; + } + + // Split --flag=value into name and value. + const eq_idx = std.mem.indexOfScalar(u8, token, '='); + const flag_name = if (eq_idx) |idx| token[0..idx] else token; + + const kind = node_options_allowlist.get(flag_name) orelse continue; + + const token_z = try bun.default_allocator.dupeZ(u8, token); + try args.insert(offset, token_z); + offset += 1; + + // If this is a value-taking option and the value wasn't inline + // (`--flag value` form), consume the next token as its value. + if (kind == .option and eq_idx == null and j + 1 < tokens.items.len) { + const next_tok = tokens.items[j + 1]; + if (next_tok.len > 0 and next_tok[0] != '-') { + const next_z = try bun.default_allocator.dupeZ(u8, next_tok); + try args.insert(offset, next_z); + offset += 1; + j += 1; + } + } + } +} + pub fn initArgv() !void { if (comptime Environment.isPosix) { argv = try bun.default_allocator.alloc([:0]const u8, std.os.argv.len); @@ -2215,6 +2366,16 @@ pub fn initArgv() !void { argv = argv_list.items; bun_options_argc = argv.len - original_len; } + + // NODE_OPTIONS: Node.js-compatible env var for injecting CLI flags. + // Filtered through an allowlist — unknown flags are dropped. + if (bun.env_var.NODE_OPTIONS.get()) |opts| { + if (opts.len > 0) { + var argv_list = std.array_list.Managed([:0]const u8).fromOwnedSlice(bun.default_allocator, argv); + try appendNodeOptionsEnv(opts, &argv_list); + argv = argv_list.items; + } + } } pub const spawn = @import("./runtime/api/bun/spawn.zig").PosixSpawn; diff --git a/src/bun_core/env_var.zig b/src/bun_core/env_var.zig index 661e4c535f61..86372c25e700 100644 --- a/src/bun_core/env_var.zig +++ b/src/bun_core/env_var.zig @@ -118,6 +118,7 @@ pub const JENKINS_URL = New(kind.string, "JENKINS_URL", .{}); pub const MI_VERBOSE = New(kind.boolean, "MI_VERBOSE", .{ .default = false }); pub const NO_COLOR = New(kind.boolean, "NO_COLOR", .{ .default = false }); pub const NODE_CHANNEL_FD = New(kind.string, "NODE_CHANNEL_FD", .{}); +pub const NODE_OPTIONS = New(kind.string, "NODE_OPTIONS", .{}); /// Set by HostProcess.zig when spawning the WebView host subprocess. The /// child's cli.zig checks this before anything else and hands off to C++ /// Bun__WebView__hostMain. Never returns — no JSC, no VM. diff --git a/test/regression/issue/28817.test.ts b/test/regression/issue/28817.test.ts new file mode 100644 index 000000000000..c3824b518a6b --- /dev/null +++ b/test/regression/issue/28817.test.ts @@ -0,0 +1,119 @@ +// Regression test for https://github.com/oven-sh/bun/issues/28817 +// Bun should honor NODE_OPTIONS=--dns-result-order and other Node-compatible +// flags set via the NODE_OPTIONS environment variable. +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +async function run(nodeOptions: string, script: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: { ...bunEnv, NODE_OPTIONS: nodeOptions }, + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + +describe("NODE_OPTIONS", () => { + test("--dns-result-order=ipv4first sets default order via env", async () => { + const { stdout, exitCode } = await run( + "--dns-result-order=ipv4first", + 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', + ); + expect(stdout.trim()).toBe("ipv4first"); + expect(exitCode).toBe(0); + }); + + test("--dns-result-order ipv6first (space separated) also works", async () => { + const { stdout, exitCode } = await run( + "--dns-result-order ipv6first", + 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', + ); + expect(stdout.trim()).toBe("ipv6first"); + expect(exitCode).toBe(0); + }); + + test("default order is verbatim without NODE_OPTIONS", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());'], + env: { ...bunEnv, NODE_OPTIONS: "" }, + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + expect(stdout.trim()).toBe("verbatim"); + expect(exitCode).toBe(0); + }); + + test("getDefaultResultOrder returns a string, not a function", async () => { + const { stdout, exitCode } = await run( + "--dns-result-order=ipv4first", + 'import dns from "node:dns"; console.log(typeof dns.getDefaultResultOrder());', + ); + expect(stdout.trim()).toBe("string"); + expect(exitCode).toBe(0); + }); + + test("unknown flags are silently ignored", async () => { + const { stdout, exitCode } = await run( + "--unknown-flag --dns-result-order=ipv4first", + 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', + ); + expect(stdout.trim()).toBe("ipv4first"); + expect(exitCode).toBe(0); + }); + + test("positional args cannot inject scripts via NODE_OPTIONS", async () => { + // Even though the user put a positional-looking arg in, the entrypoint + // should still be the -e script, not the injected positional. + const { stdout, exitCode } = await run( + "/etc/passwd", + 'console.log("safe");', + ); + expect(stdout.trim()).toBe("safe"); + expect(exitCode).toBe(0); + }); + + test("--eval is not injectable via NODE_OPTIONS", async () => { + // --eval is not in the allowlist, so this must not execute. + const { stdout, exitCode } = await run( + "--eval console.log('HIJACKED')", + 'console.log("original");', + ); + expect(stdout).not.toContain("HIJACKED"); + expect(stdout.trim()).toBe("original"); + expect(exitCode).toBe(0); + }); + + test("--expose-gc exposes gc()", async () => { + const { stdout, exitCode } = await run( + "--expose-gc", + 'console.log(typeof gc);', + ); + expect(stdout.trim()).toBe("function"); + expect(exitCode).toBe(0); + }); + + test("--title sets process.title", async () => { + const { stdout, exitCode } = await run( + "--title=my-bun-app", + 'console.log(process.title);', + ); + expect(stdout.trim()).toBe("my-bun-app"); + expect(exitCode).toBe(0); + }); + + test("quoted values are parsed correctly", async () => { + const { stdout, exitCode } = await run( + `--dns-result-order='ipv4first'`, + 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', + ); + expect(stdout.trim()).toBe("ipv4first"); + expect(exitCode).toBe(0); + }); +}); From 9aa538e847807dbfc66724af76060a301dd79336 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 3 Apr 2026 19:24:12 +0000 Subject: [PATCH 2/5] [autofix.ci] apply automated fixes --- test/regression/issue/28817.test.ts | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/test/regression/issue/28817.test.ts b/test/regression/issue/28817.test.ts index c3824b518a6b..9d3073cf34c1 100644 --- a/test/regression/issue/28817.test.ts +++ b/test/regression/issue/28817.test.ts @@ -11,11 +11,7 @@ async function run(nodeOptions: string, script: string) { stderr: "pipe", stdout: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); return { stdout, stderr, exitCode }; } @@ -71,39 +67,27 @@ describe("NODE_OPTIONS", () => { test("positional args cannot inject scripts via NODE_OPTIONS", async () => { // Even though the user put a positional-looking arg in, the entrypoint // should still be the -e script, not the injected positional. - const { stdout, exitCode } = await run( - "/etc/passwd", - 'console.log("safe");', - ); + const { stdout, exitCode } = await run("/etc/passwd", 'console.log("safe");'); expect(stdout.trim()).toBe("safe"); expect(exitCode).toBe(0); }); test("--eval is not injectable via NODE_OPTIONS", async () => { // --eval is not in the allowlist, so this must not execute. - const { stdout, exitCode } = await run( - "--eval console.log('HIJACKED')", - 'console.log("original");', - ); + const { stdout, exitCode } = await run("--eval console.log('HIJACKED')", 'console.log("original");'); expect(stdout).not.toContain("HIJACKED"); expect(stdout.trim()).toBe("original"); expect(exitCode).toBe(0); }); test("--expose-gc exposes gc()", async () => { - const { stdout, exitCode } = await run( - "--expose-gc", - 'console.log(typeof gc);', - ); + const { stdout, exitCode } = await run("--expose-gc", "console.log(typeof gc);"); expect(stdout.trim()).toBe("function"); expect(exitCode).toBe(0); }); test("--title sets process.title", async () => { - const { stdout, exitCode } = await run( - "--title=my-bun-app", - 'console.log(process.title);', - ); + const { stdout, exitCode } = await run("--title=my-bun-app", "console.log(process.title);"); expect(stdout.trim()).toBe("my-bun-app"); expect(exitCode).toBe(0); }); From 3b3e802722515a4c9035dc49743f9588a84f5185 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 3 Apr 2026 20:09:57 +0000 Subject: [PATCH 3/5] Consolidate NODE_OPTIONS tests and skip process.title on Windows --- test/regression/issue/28817.test.ts | 121 +++++++++++++--------------- 1 file changed, 54 insertions(+), 67 deletions(-) diff --git a/test/regression/issue/28817.test.ts b/test/regression/issue/28817.test.ts index 9d3073cf34c1..6e518b05449c 100644 --- a/test/regression/issue/28817.test.ts +++ b/test/regression/issue/28817.test.ts @@ -2,102 +2,89 @@ // Bun should honor NODE_OPTIONS=--dns-result-order and other Node-compatible // flags set via the NODE_OPTIONS environment variable. import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isWindows } from "harness"; -async function run(nodeOptions: string, script: string) { +async function runWith(nodeOptions: string, script: string) { await using proc = Bun.spawn({ cmd: [bunExe(), "-e", script], env: { ...bunEnv, NODE_OPTIONS: nodeOptions }, stderr: "pipe", stdout: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - return { stdout, stderr, exitCode }; + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); + return { stdout: stdout.trim(), stderr, exitCode }; } describe("NODE_OPTIONS", () => { - test("--dns-result-order=ipv4first sets default order via env", async () => { - const { stdout, exitCode } = await run( - "--dns-result-order=ipv4first", - 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', - ); - expect(stdout.trim()).toBe("ipv4first"); - expect(exitCode).toBe(0); - }); + test("--dns-result-order honored via env (= and space forms)", async () => { + const getOrder = 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());'; - test("--dns-result-order ipv6first (space separated) also works", async () => { - const { stdout, exitCode } = await run( - "--dns-result-order ipv6first", - 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', - ); - expect(stdout.trim()).toBe("ipv6first"); - expect(exitCode).toBe(0); + // --flag=value form + const eq = await runWith("--dns-result-order=ipv4first", getOrder); + expect(eq.stdout).toBe("ipv4first"); + expect(eq.exitCode).toBe(0); + + // --flag value (space separated) form + const sp = await runWith("--dns-result-order ipv6first", getOrder); + expect(sp.stdout).toBe("ipv6first"); + expect(sp.exitCode).toBe(0); + + // quoted value + const q = await runWith(`--dns-result-order='verbatim'`, getOrder); + expect(q.stdout).toBe("verbatim"); + expect(q.exitCode).toBe(0); }); test("default order is verbatim without NODE_OPTIONS", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());'], - env: { ...bunEnv, NODE_OPTIONS: "" }, - stderr: "pipe", - stdout: "pipe", - }); - const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); - expect(stdout.trim()).toBe("verbatim"); - expect(exitCode).toBe(0); + const r = await runWith("", 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());'); + expect(r.stdout).toBe("verbatim"); + expect(r.exitCode).toBe(0); }); test("getDefaultResultOrder returns a string, not a function", async () => { - const { stdout, exitCode } = await run( + const r = await runWith( "--dns-result-order=ipv4first", - 'import dns from "node:dns"; console.log(typeof dns.getDefaultResultOrder());', + 'import dns from "node:dns"; const v = dns.getDefaultResultOrder(); console.log(typeof v, v);', ); - expect(stdout.trim()).toBe("string"); - expect(exitCode).toBe(0); + expect(r.stdout).toBe("string ipv4first"); + expect(r.exitCode).toBe(0); }); - test("unknown flags are silently ignored", async () => { - const { stdout, exitCode } = await run( + test("unknown flags and positional args are dropped", async () => { + // Unknown flag is ignored; known flag still works. + const r1 = await runWith( "--unknown-flag --dns-result-order=ipv4first", 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', ); - expect(stdout.trim()).toBe("ipv4first"); - expect(exitCode).toBe(0); - }); + expect(r1.stdout).toBe("ipv4first"); + expect(r1.exitCode).toBe(0); - test("positional args cannot inject scripts via NODE_OPTIONS", async () => { - // Even though the user put a positional-looking arg in, the entrypoint - // should still be the -e script, not the injected positional. - const { stdout, exitCode } = await run("/etc/passwd", 'console.log("safe");'); - expect(stdout.trim()).toBe("safe"); - expect(exitCode).toBe(0); - }); + // Positional arg (non-flag) cannot inject a script/entrypoint. + const r2 = await runWith("/etc/passwd", 'console.log("safe");'); + expect(r2.stdout).toBe("safe"); + expect(r2.exitCode).toBe(0); - test("--eval is not injectable via NODE_OPTIONS", async () => { - // --eval is not in the allowlist, so this must not execute. - const { stdout, exitCode } = await run("--eval console.log('HIJACKED')", 'console.log("original");'); - expect(stdout).not.toContain("HIJACKED"); - expect(stdout.trim()).toBe("original"); - expect(exitCode).toBe(0); + // --eval is not in the allowlist, so this must not execute injected code. + const r3 = await runWith("--eval console.log('HIJACKED')", 'console.log("original");'); + expect(r3.stdout).not.toContain("HIJACKED"); + expect(r3.stdout).toBe("original"); + expect(r3.exitCode).toBe(0); }); - test("--expose-gc exposes gc()", async () => { - const { stdout, exitCode } = await run("--expose-gc", "console.log(typeof gc);"); - expect(stdout.trim()).toBe("function"); - expect(exitCode).toBe(0); + test("--expose-gc exposes gc() via env", async () => { + const r = await runWith("--expose-gc", "console.log(typeof gc);"); + expect(r.stdout).toBe("function"); + expect(r.exitCode).toBe(0); }); - test("--title sets process.title", async () => { - const { stdout, exitCode } = await run("--title=my-bun-app", "console.log(process.title);"); - expect(stdout.trim()).toBe("my-bun-app"); - expect(exitCode).toBe(0); - }); - - test("quoted values are parsed correctly", async () => { - const { stdout, exitCode } = await run( - `--dns-result-order='ipv4first'`, - 'import dns from "node:dns"; console.log(dns.getDefaultResultOrder());', - ); - expect(stdout.trim()).toBe("ipv4first"); - expect(exitCode).toBe(0); + test.skipIf(isWindows)("--title sets process.title via env", async () => { + // process.title is unreliable on Windows (varies by OS version); skip there. + const r = await runWith("--title=my-bun-app", "console.log(process.title);"); + expect(r.stdout).toBe("my-bun-app"); + expect(r.exitCode).toBe(0); }); }); From 124b6f140c2d74b07db764a30a0260f0f67747ff Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 3 Apr 2026 20:12:17 +0000 Subject: [PATCH 4/5] [autofix.ci] apply automated fixes --- test/regression/issue/28817.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/regression/issue/28817.test.ts b/test/regression/issue/28817.test.ts index 6e518b05449c..fe6cb452778e 100644 --- a/test/regression/issue/28817.test.ts +++ b/test/regression/issue/28817.test.ts @@ -11,11 +11,7 @@ async function runWith(nodeOptions: string, script: string) { stderr: "pipe", stdout: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); return { stdout: stdout.trim(), stderr, exitCode }; } From 30d9121372c8773fed26a57db754de298c2d6ec7 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 4 May 2026 23:19:04 +0000 Subject: [PATCH 5/5] Address review: fix NODE_OPTIONS argv injection edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Track NODE_OPTIONS-injected flags in bun_options_argc so standalone compiled binaries compute the correct passthrough offset and clap parses the full injected window. - Drop bare required-value flags (e.g. NODE_OPTIONS="--require") when no value follows in the env var itself. Previously the flag was emitted bare and clap bound the user's entrypoint as the missing value — reopening the entrypoint-hijack vector this filter closes. - Preserve empty quoted values (NODE_OPTIONS='--title ""') instead of dropping them. - Backslash has no special meaning inside single quotes (POSIX), so it no longer eats the closing apostrophe. - Add regression tests for bare required-value flag handling. - Drop the prose comment from the test file per review. --- src/bun.zig | 128 ++++++++++++++++++---------- test/regression/issue/28817.test.ts | 28 +++++- 2 files changed, 110 insertions(+), 46 deletions(-) diff --git a/src/bun.zig b/src/bun.zig index f69ce3dcc812..bcc99eaf828e 100644 --- a/src/bun.zig +++ b/src/bun.zig @@ -2148,10 +2148,13 @@ pub fn appendOptionsEnv(env: []const u8, comptime ArgType: type, args: *std.arra } } -/// Kind of flag in `NODE_OPTIONS`: either a boolean flag (no value) or an -/// option that takes a value. Used by `appendNodeOptionsEnv` to decide -/// whether the next token should be consumed as a value. -const NodeOptionKind = enum { bool_flag, option }; +/// Kind of flag in `NODE_OPTIONS`. Controls how the filter treats a bare +/// (no `=value`) occurrence of the flag: +/// - `bool_flag`: no value expected; emit bare. +/// - `required_value`: must have a value. If no value token follows in +/// `NODE_OPTIONS`, the flag is DROPPED — never emitted bare, to avoid +/// binding a later argv token (e.g. the entrypoint) as the missing value. +const NodeOptionKind = enum { bool_flag, required_value }; /// Allowlist of flags supported by Bun that may appear in `NODE_OPTIONS`. /// @@ -2175,34 +2178,38 @@ pub const node_options_allowlist = ComptimeStringMap(NodeOptionKind, .{ .{ "--heap-prof", .bool_flag }, .{ "--heap-prof-md", .bool_flag }, - // Options that take a value - .{ "--conditions", .option }, - .{ "-C", .option }, - .{ "--cpu-prof-dir", .option }, - .{ "--cpu-prof-interval", .option }, - .{ "--cpu-prof-name", .option }, - .{ "--dns-result-order", .option }, - .{ "--heap-prof-dir", .option }, - .{ "--heap-prof-name", .option }, - .{ "--import", .option }, - .{ "--inspect", .option }, - .{ "--inspect-brk", .option }, - .{ "--inspect-wait", .option }, - .{ "--max-http-header-size", .option }, - .{ "--require", .option }, - .{ "-r", .option }, - .{ "--title", .option }, - .{ "--unhandled-rejections", .option }, + // Options that take a value. All treated as required_value: bare form + // (no `=value` and no following value token) is dropped to prevent + // binding the user's entrypoint as the missing value. + .{ "--conditions", .required_value }, + .{ "-C", .required_value }, + .{ "--cpu-prof-dir", .required_value }, + .{ "--cpu-prof-interval", .required_value }, + .{ "--cpu-prof-name", .required_value }, + .{ "--dns-result-order", .required_value }, + .{ "--heap-prof-dir", .required_value }, + .{ "--heap-prof-name", .required_value }, + .{ "--import", .required_value }, + .{ "--inspect", .required_value }, + .{ "--inspect-brk", .required_value }, + .{ "--inspect-wait", .required_value }, + .{ "--max-http-header-size", .required_value }, + .{ "--require", .required_value }, + .{ "-r", .required_value }, + .{ "--title", .required_value }, + .{ "--unhandled-rejections", .required_value }, }); /// Parses `NODE_OPTIONS` env var into command-line tokens filtered by /// `node_options_allowlist` and inserts them into `args` starting at -/// `offset` (after argv[0]). +/// index 1 (after argv[0]). /// /// Splits on whitespace with support for single/double quotes and backslash -/// escapes, matching Node.js's NODE_OPTIONS parsing behavior. Positional -/// (non-flag) tokens and unrecognized flags are dropped to prevent -/// unintended script execution via the environment. +/// escapes (POSIX semantics: backslash has no special meaning inside single +/// quotes). Positional (non-flag) tokens and unrecognized flags are dropped +/// to prevent unintended script execution via the environment. Required-value +/// flags that appear without a value are also dropped, so a bare +/// `NODE_OPTIONS="--require"` cannot hijack the user's entrypoint. pub fn appendNodeOptionsEnv(env: []const u8, args: *std.array_list.Managed([:0]const u8)) !void { // First, tokenize the NODE_OPTIONS string (quote- and escape-aware). var tokens = std.array_list.Managed([]u8).init(bun.default_allocator); @@ -2229,12 +2236,24 @@ pub fn appendNodeOptionsEnv(env: []const u8, args: *std.array_list.Managed([:0]c continue; } - if (ch == '\\') { - escape = true; - continue; - } - + // Single quotes suppress all escaping (POSIX). Handle the in-quote + // branch before the backslash branch so `\` inside `'…'` is literal. if (in_quote) |q| { + if (q == '\'') { + // Single-quoted: no escapes, only the matching quote closes. + if (ch == '\'') { + in_quote = null; + } else { + try buf.append(ch); + has_token = true; + } + continue; + } + // Double-quoted: backslash escapes the next character. + if (ch == '\\') { + escape = true; + continue; + } if (ch == q) { in_quote = null; } else { @@ -2244,8 +2263,15 @@ pub fn appendNodeOptionsEnv(env: []const u8, args: *std.array_list.Managed([:0]c continue; } + if (ch == '\\') { + escape = true; + continue; + } + if (ch == '\'' or ch == '"') { in_quote = ch; + // Mark that we've seen a token even if the quoted body is empty, + // so `--flag ""` preserves the empty string as a distinct token. has_token = true; continue; } @@ -2281,21 +2307,33 @@ pub fn appendNodeOptionsEnv(env: []const u8, args: *std.array_list.Managed([:0]c const kind = node_options_allowlist.get(flag_name) orelse continue; + if (kind == .required_value and eq_idx == null) { + // Required-value flag without inline =value: look for a following + // value token. If there isn't one (or the next token is itself a + // flag), DROP this flag entirely — never emit a bare required-value + // flag, or clap would bind the user's entrypoint as the value. + if (j + 1 >= tokens.items.len) continue; + const next_tok = tokens.items[j + 1]; + // An empty quoted value (`--flag ""`) is legitimate; reject only + // tokens that start with `-`, which would be another flag. + if (next_tok.len > 0 and next_tok[0] == '-') continue; + + const token_z = try bun.default_allocator.dupeZ(u8, token); + try args.insert(offset, token_z); + offset += 1; + + const next_z = try bun.default_allocator.dupeZ(u8, next_tok); + try args.insert(offset, next_z); + offset += 1; + j += 1; + continue; + } + + // bool_flag, or required_value with inline `--flag=value`: safe to + // emit the token as-is. const token_z = try bun.default_allocator.dupeZ(u8, token); try args.insert(offset, token_z); offset += 1; - - // If this is a value-taking option and the value wasn't inline - // (`--flag value` form), consume the next token as its value. - if (kind == .option and eq_idx == null and j + 1 < tokens.items.len) { - const next_tok = tokens.items[j + 1]; - if (next_tok.len > 0 and next_tok[0] != '-') { - const next_z = try bun.default_allocator.dupeZ(u8, next_tok); - try args.insert(offset, next_z); - offset += 1; - j += 1; - } - } } } @@ -2369,11 +2407,15 @@ pub fn initArgv() !void { // NODE_OPTIONS: Node.js-compatible env var for injecting CLI flags. // Filtered through an allowlist — unknown flags are dropped. + // Counted toward `bun_options_argc` so standalone binaries compute the + // correct passthrough offset (see offset_for_passthrough in cli.zig). if (bun.env_var.NODE_OPTIONS.get()) |opts| { if (opts.len > 0) { + const before = argv.len; var argv_list = std.array_list.Managed([:0]const u8).fromOwnedSlice(bun.default_allocator, argv); try appendNodeOptionsEnv(opts, &argv_list); argv = argv_list.items; + bun_options_argc += argv.len - before; } } } diff --git a/test/regression/issue/28817.test.ts b/test/regression/issue/28817.test.ts index fe6cb452778e..505ca02e9d32 100644 --- a/test/regression/issue/28817.test.ts +++ b/test/regression/issue/28817.test.ts @@ -1,6 +1,4 @@ -// Regression test for https://github.com/oven-sh/bun/issues/28817 -// Bun should honor NODE_OPTIONS=--dns-result-order and other Node-compatible -// flags set via the NODE_OPTIONS environment variable. +// https://github.com/oven-sh/bun/issues/28817 import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isWindows } from "harness"; @@ -83,4 +81,28 @@ describe("NODE_OPTIONS", () => { expect(r.stdout).toBe("my-bun-app"); expect(r.exitCode).toBe(0); }); + + test("bare required-value flag does not hijack the entrypoint", async () => { + // Regression guard for a `--require` / `--dns-result-order` etc. with no + // value following in NODE_OPTIONS. A bare required-value flag must be + // DROPPED; otherwise clap would bind the user's -e script (or entrypoint) + // as the flag's value and the script would never run. + const r = await runWith("--dns-result-order", 'console.log("ran");'); + expect(r.stdout).toBe("ran"); + expect(r.exitCode).toBe(0); + + // Same with --require — must not consume the -e script as the path. + const r2 = await runWith("--require", 'console.log("still ran");'); + expect(r2.stdout).toBe("still ran"); + expect(r2.exitCode).toBe(0); + }); + + test("bare required-value flag followed by another flag is dropped", async () => { + // `--dns-result-order` has no value, then `--expose-gc` follows — which + // starts with `-`, so the first flag must not consume it. Result: the + // first flag is dropped; --expose-gc still takes effect. + const r = await runWith("--dns-result-order --expose-gc", "console.log(typeof gc);"); + expect(r.stdout).toBe("function"); + expect(r.exitCode).toBe(0); + }); });