From 0fd335f722e612a7d4b233f46e83d5a64fd9fd84 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 05:23:29 +0000 Subject: [PATCH 1/7] shell: export rejects invalid identifiers The export builtin inserted every argument into the exported environment without a check on the name. `export 1abc a-b=5` put `1abc=` and `a-b=5` into the environment of child processes. Validate each name with is_valid_var_name. An invalid word is reported on stderr as "export: ``: not a valid identifier" and is not exported. The valid words are still exported. The exit code is 1 when any word was rejected, like bash. --- src/runtime/shell/builtin/export.rs | 21 ++++++++++++++++++--- src/runtime/shell/mod.rs | 2 +- src/shell_parser/lib.rs | 2 +- src/shell_parser/parse.rs | 2 +- test/js/bun/shell/bunshell.test.ts | 18 ++++++++++++++++++ 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 29d6b0e5c71b..984e5f1f47e6 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -1,8 +1,8 @@ -use crate::shell::EnvStr; use crate::shell::builtin::{Builtin, BuiltinState, IoKind}; use crate::shell::interpreter::{Interpreter, NodeId}; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; +use crate::shell::{EnvStr, is_valid_var_name}; use bun_collections::index_sort; #[derive(Default)] @@ -15,6 +15,7 @@ enum State { #[default] Idle, WaitingIo, + Err, Done, } @@ -25,6 +26,9 @@ impl Export { // No args: print all exported vars. return Self::print_all(interp, cmd); } + // Like bash: every invalid word is reported, the valid ones are still + // exported, and the exit code is 1 if any word was rejected. + let mut errors = Vec::new(); for i in 0..argc { let s = Builtin::of(interp, cmd).arg_bytes(i); if s.is_empty() { @@ -34,6 +38,12 @@ impl Export { Some(eq) => (&s[..eq], &s[eq + 1..]), None => (s, &b""[..]), }; + if !is_valid_var_name(name) { + errors.extend_from_slice(b"export: `"); + errors.extend_from_slice(s); + errors.extend_from_slice(b"`: not a valid identifier\n"); + continue; + } // The argv backing is freed when the Cmd retires, // so the key/value MUST be duplicated into ref-counted storage — // `init_slice` here would leave dangling EnvStr in `export_env`. @@ -45,7 +55,11 @@ impl Export { label.deref(); val.deref(); } - Builtin::done(interp, cmd, 0) + if errors.is_empty() { + return Builtin::done(interp, cmd, 0); + } + Self::state_mut(interp, cmd).state = State::Err; + Builtin::write_failing_error(interp, cmd, &errors, 1) } fn print_all(interp: &Interpreter, cmd: NodeId) -> Yield { @@ -81,7 +95,8 @@ impl Export { _: usize, err: Option, ) -> Yield { + let failed = err.is_some() || matches!(Self::state_mut(interp, cmd).state, State::Err); Self::state_mut(interp, cmd).state = State::Done; - Builtin::done(interp, cmd, err.map_or(0, |_| 1)) + Builtin::done(interp, cmd, if failed { 1 } else { 0 }) } } diff --git a/src/runtime/shell/mod.rs b/src/runtime/shell/mod.rs index 6a509b2fb00e..85a0399fb426 100644 --- a/src/runtime/shell/mod.rs +++ b/src/runtime/shell/mod.rs @@ -132,7 +132,7 @@ pub mod subproc; // ─── shell escaping (canonical impl lives in bun_shell_parser) ─────────────── // Re-export so `crate::shell::*` callers resolve without duplicating the table. -pub use bun_shell_parser::{escape_8bit, needs_escape_utf8_ascii_latin1}; +pub use bun_shell_parser::{escape_8bit, is_valid_var_name, needs_escape_utf8_ascii_latin1}; // ─── AST surface (lifetime-erased aliases over `bun_shell_parser::ast`) ────── // State nodes hold `*const ast::*` raw pointers into the bumpalo-allocated AST diff --git a/src/shell_parser/lib.rs b/src/shell_parser/lib.rs index 5ed358c87c2c..b9c5e0ce0100 100644 --- a/src/shell_parser/lib.rs +++ b/src/shell_parser/lib.rs @@ -20,5 +20,5 @@ pub mod json_fmt; pub use parse::{ JSValueRaw, LexResult, LexerError, ParseError, Parser, ast, escape_8bit, escape_bun_str, - needs_escape_bunstr, needs_escape_utf8_ascii_latin1, + is_valid_var_name, needs_escape_bunstr, needs_escape_utf8_ascii_latin1, }; diff --git a/src/shell_parser/parse.rs b/src/shell_parser/parse.rs index 5c285b84c372..5f6ff3c2357e 100644 --- a/src/shell_parser/parse.rs +++ b/src/shell_parser/parse.rs @@ -3785,7 +3785,7 @@ impl<'a, const ENCODING: StringEncoding> ShellCharIter<'a, ENCODING> { /// - a-zA-Z /// - _ /// - 0-9 (but can't be first char) -pub(crate) fn is_valid_var_name(var_name: &[u8]) -> bool { +pub fn is_valid_var_name(var_name: &[u8]) -> bool { if is_all_ascii(var_name) { return is_valid_var_name_ascii(var_name); } diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 2d57ca0587c9..721f66823a16 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1398,6 +1398,24 @@ describe("deno_task", () => { TestBuilder.command`export VAR=1 VAR2=testing VAR3="test this out" && echo $VAR $VAR2 $VAR3` .stdout("1 testing test this out\n") .runAsTest("exported vars 2"); + + TestBuilder.command`export 1abc a-b=5 =x ok=1; ${BUN} -e ${"console.log(JSON.stringify([process.env['1abc'], process.env['a-b'], process.env.ok]))"}` + .stdout('[null,null,"1"]\n') + .stderr( + "export: `1abc`: not a valid identifier\n" + + "export: `a-b=5`: not a valid identifier\n" + + "export: `=x`: not a valid identifier\n", + ) + .testMini() + .runAsTest("export rejects invalid identifiers and keeps the valid ones"); + + TestBuilder.command`export 1abc` + .stderr("export: `1abc`: not a valid identifier\n") + .exitCode(1) + .testMini() + .runAsTest("export exits 1 on an invalid identifier"); + + TestBuilder.command`export _ok OK2=1 && echo done`.stdout("done\n").runAsTest("export accepts valid identifiers"); }); describe("pipeline", async () => { From 425b825f31aedb592e16d8abbaead8a5e2932d0b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 05:42:59 +0000 Subject: [PATCH 2/7] shell: export rejects an empty word too An empty argument is not a valid identifier either. Drop the skip so the same check reports it, like bash. --- src/runtime/shell/builtin/export.rs | 3 --- test/js/bun/shell/bunshell.test.ts | 5 +++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 984e5f1f47e6..a9f7cf018998 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -31,9 +31,6 @@ impl Export { let mut errors = Vec::new(); for i in 0..argc { let s = Builtin::of(interp, cmd).arg_bytes(i); - if s.is_empty() { - continue; - } let (name, value) = match bun_core::strings::index_of_char_usize(s, b'=') { Some(eq) => (&s[..eq], &s[eq + 1..]), None => (s, &b""[..]), diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 721f66823a16..914c3e146454 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1399,12 +1399,13 @@ describe("deno_task", () => { .stdout("1 testing test this out\n") .runAsTest("exported vars 2"); - TestBuilder.command`export 1abc a-b=5 =x ok=1; ${BUN} -e ${"console.log(JSON.stringify([process.env['1abc'], process.env['a-b'], process.env.ok]))"}` + TestBuilder.command`export 1abc a-b=5 =x "" ok=1; ${BUN} -e ${"console.log(JSON.stringify([process.env['1abc'], process.env['a-b'], process.env.ok]))"}` .stdout('[null,null,"1"]\n') .stderr( "export: `1abc`: not a valid identifier\n" + "export: `a-b=5`: not a valid identifier\n" + - "export: `=x`: not a valid identifier\n", + "export: `=x`: not a valid identifier\n" + + "export: ``: not a valid identifier\n", ) .testMini() .runAsTest("export rejects invalid identifiers and keeps the valid ones"); From 1da25baaf1fbb30fe4ec583846dd7afcfd975242 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 05:44:34 +0000 Subject: [PATCH 3/7] shell: drop a redundant comment in the export builtin --- src/runtime/shell/builtin/export.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index a9f7cf018998..bdd069f3bb44 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -26,8 +26,6 @@ impl Export { // No args: print all exported vars. return Self::print_all(interp, cmd); } - // Like bash: every invalid word is reported, the valid ones are still - // exported, and the exit code is 1 if any word was rejected. let mut errors = Vec::new(); for i in 0..argc { let s = Builtin::of(interp, cmd).arg_bytes(i); From 710ad934a97a5db5782f20b3e6f064bbd08564c0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:01:21 +0000 Subject: [PATCH 4/7] shell: export skips a leading -- before the identifier check POSIX scripts write `export -- NAME=value`. The identifier check would report `--` as an invalid name and exit 1. Treat a leading `--` as the end of options, like bash. --- src/runtime/shell/builtin/export.rs | 6 ++++-- test/js/bun/shell/bunshell.test.ts | 9 +++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index bdd069f3bb44..03fa61e5bdc0 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -22,12 +22,14 @@ enum State { impl Export { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let argc = Builtin::of(interp, cmd).args_slice().len(); - if argc == 0 { + // POSIX end-of-options marker: `export -- NAME=value`. + let start = usize::from(argc > 0 && Builtin::of(interp, cmd).arg_bytes(0) == b"--"); + if start >= argc { // No args: print all exported vars. return Self::print_all(interp, cmd); } let mut errors = Vec::new(); - for i in 0..argc { + for i in start..argc { let s = Builtin::of(interp, cmd).arg_bytes(i); let (name, value) = match bun_core::strings::index_of_char_usize(s, b'=') { Some(eq) => (&s[..eq], &s[eq + 1..]), diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 914c3e146454..63bc44a8bd40 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1417,6 +1417,15 @@ describe("deno_task", () => { .runAsTest("export exits 1 on an invalid identifier"); TestBuilder.command`export _ok OK2=1 && echo done`.stdout("done\n").runAsTest("export accepts valid identifiers"); + + TestBuilder.command`export -- FOO=bar && echo $FOO && export -- && echo done` + .stdout(stdout => { + expect(stdout).toStartWith("bar\n"); + expect(stdout).toContain("FOO=bar\n"); + expect(stdout).not.toContain("--="); + expect(stdout).toEndWith("done\n"); + }) + .runAsTest("export treats a leading -- as the end of options"); }); describe("pipeline", async () => { From 51ed49570f7939c07163a7354a3ba72293c0c5f6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:54:45 +0000 Subject: [PATCH 5/7] shell: cover the exit code of export for an empty word and for mixed arguments --- test/js/bun/shell/bunshell.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 63bc44a8bd40..c32b71d475b7 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1426,6 +1426,21 @@ describe("deno_task", () => { expect(stdout).toEndWith("done\n"); }) .runAsTest("export treats a leading -- as the end of options"); + + TestBuilder.command`export ""` + .stderr("export: ``: not a valid identifier\n") + .exitCode(1) + .runAsTest("export exits 1 on an empty word"); + + TestBuilder.command`export A=1 2B C=3 3D || echo "failed A=$A C=$C"` + .stdout("failed A=1 C=3\n") + .stderr("export: `2B`: not a valid identifier\nexport: `3D`: not a valid identifier\n") + .runAsTest("export exits 1 when a bare name is invalid and still exports the valid ones"); + + TestBuilder.command`export a-b=5 1FOO=bar OK=1 || echo "failed OK=$OK"` + .stdout("failed OK=1\n") + .stderr("export: `a-b=5`: not a valid identifier\nexport: `1FOO=bar`: not a valid identifier\n") + .runAsTest("export exits 1 when an assignment name is invalid and still exports the valid ones"); }); describe("pipeline", async () => { From 584bdc0960dc5d59fe1c90e4c03c396aa61c591e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 10:02:35 +0000 Subject: [PATCH 6/7] shell: cover the synchronous stderr path of export with a quiet run --- test/js/bun/shell/bunshell.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index c32b71d475b7..ad7fe61cd79b 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1416,6 +1416,14 @@ describe("deno_task", () => { .testMini() .runAsTest("export exits 1 on an invalid identifier"); + // `.quiet()` keeps stderr as an in-memory buffer, so the builtin takes the + // synchronous write path instead of the async fd write the other runs use. + TestBuilder.command`export 1abc` + .stderr("export: `1abc`: not a valid identifier\n") + .exitCode(1) + .quiet() + .runAsTest("export exits 1 on an invalid identifier with quiet output"); + TestBuilder.command`export _ok OK2=1 && echo done`.stdout("done\n").runAsTest("export accepts valid identifiers"); TestBuilder.command`export -- FOO=bar && echo $FOO && export -- && echo done` From 510edee36b3e2c29cdbe5b3eda49c5a845a1a997 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 12:36:45 +0000 Subject: [PATCH 7/7] shell: consume the write error in Export::on_io_writer_chunk clippy denies needless_pass_by_value on the err argument. A write error keeps precedence over the identifier failure. --- src/runtime/shell/builtin/export.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 03fa61e5bdc0..f862d61ed568 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -2,7 +2,7 @@ use crate::shell::builtin::{Builtin, BuiltinState, IoKind}; use crate::shell::interpreter::{Interpreter, NodeId}; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; -use crate::shell::{EnvStr, is_valid_var_name}; +use crate::shell::{EnvStr, ExitCode, is_valid_var_name}; use bun_collections::index_sort; #[derive(Default)] @@ -92,8 +92,15 @@ impl Export { _: usize, err: Option, ) -> Yield { - let failed = err.is_some() || matches!(Self::state_mut(interp, cmd).state, State::Err); + let failed = matches!(Self::state_mut(interp, cmd).state, State::Err); Self::state_mut(interp, cmd).state = State::Done; - Builtin::done(interp, cmd, if failed { 1 } else { 0 }) + Builtin::done( + interp, + cmd, + match err { + Some(_err) => 1, + None => ExitCode::from(failed), + }, + ) } }