shell: export rejects invalid bare names, an empty name and options

src/runtime/shell/builtin/export.rs

@@ -1,9 +1,10 @@
-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, ExitCode};
 use bun_collections::index_sort;
+use bun_shell_parser::is_valid_var_name;
 
 #[derive(Default)]
 pub(crate) struct Export {
@@ -15,24 +16,45 @@ enum State {
     #[default]
     Idle,
     WaitingIo,
+    Err,
     Done,
 }
 
 impl Export {
     pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield {
         let argc = Builtin::of(interp, cmd).args_slice().len();
-        if argc == 0 {
-            // No args: print all exported vars.
-            return Self::print_all(interp, cmd);
-        }
+        let mut has_operand = false;
+        let mut errors = Vec::new();
         for i in 0..argc {
             let s = Builtin::of(interp, cmd).arg_bytes(i);
+            if i == 0 && s.len() > 1 && s[0] == b'-' {
+                if s == b"--" {
+                    continue;
+                }
+                // No option is supported. `export -n NAME` must not reach NAME.
+                let end = bun_core::strings::index_of_char_usize(s, b'=').unwrap_or(s.len());
+                let msg = [b"export: ".as_slice(), &s[..end], b": invalid option\n"].concat();
+                return Self::fail(interp, cmd, &msg);
+            }
+            has_operand = true;
             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""[..]),
+                Some(eq) if eq > 0 => (&s[..eq], &s[eq + 1..]),
+                None if is_valid_var_name(s) => {
+                    // A bare NAME stores an empty value (#45371). Do not do that after a refused word.
+                    if !errors.is_empty() {
+                        continue;
+                    }
+                    (s, &b""[..])
+                }
+                refused => {
+                    errors.extend_from_slice(b"export: `");
+                    errors.extend_from_slice(if refused.is_some() { b"=" } else { 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 —
@@ -48,7 +70,20 @@ impl Export {
             label.deref();
             val.deref();
         }
-        Builtin::done(interp, cmd, 0)
+        if !has_operand {
+            // No operand: print all exported vars.
+            return Self::print_all(interp, cmd);
+        }
+        if errors.is_empty() {
+            return Builtin::done(interp, cmd, 0);
+        }
+        Self::fail(interp, cmd, &errors)
+    }
+
+    /// Write `msg` to stderr, then finish with exit code 1.
+    fn fail(interp: &Interpreter, cmd: NodeId, msg: &[u8]) -> Yield {
+        Self::state_mut(interp, cmd).state = State::Err;
+        Builtin::write_failing_error(interp, cmd, msg, 1)
     }
 
     fn print_all(interp: &Interpreter, cmd: NodeId) -> Yield {
@@ -84,7 +119,15 @@ impl Export {
         _: usize,
         err: Option<bun_sys::SystemError>,
     ) -> Yield {
+        let failed = 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,
+            match err {
+                Some(_err) => 1,
+                None => ExitCode::from(failed),
+            },
+        )
     }
 }

src/shell_parser/lib.rs

@@ -17,6 +17,6 @@ pub mod parse;
 pub mod json_fmt;
 
 pub use parse::{
-    LexResult, LexerError, ParseError, Parser, ast, escape_8bit, escape_bun_str,
+    LexResult, LexerError, ParseError, Parser, ast, escape_8bit, escape_bun_str, is_valid_var_name,
     needs_escape_bunstr, needs_escape_utf8_ascii_latin1,
 };

src/shell_parser/parse.rs

@@ -3776,7 +3776,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);
     }

test/js/bun/shell/bunshell.test.ts

@@ -1668,6 +1668,158 @@ describe("deno_task", () => {
     TestBuilder.command`export $(echo "SPLIT=hello world") && echo "SPLIT=[$SPLIT]"`
       .stdout("SPLIT=[hello]\n")
       .runAsTest("export splits a non-assignment word");
+
+    describe("export refuses a name that is not an identifier", () => {
+      const notValid = (name: string) => `export: \`${name}\`: not a valid identifier\n`;
+      const invalidOption = (word: string) => `export: ${word}: invalid option\n`;
+      const env = { A: "one" };
+
+      // [script, stderr, exit code, the exported variables after the script]
+      const rows: [string, string, number, Record<string, string>][] = [
+        ["export a-b", notValid("a-b"), 1, { A: "one" }],
+        ["export 1a", notValid("1a"), 1, { A: "one" }],
+        ['export "a b"', notValid("a b"), 1, { A: "one" }],
+        ["export a.b", notValid("a.b"), 1, { A: "one" }],
+        ["export é", notValid("é"), 1, { A: "one" }],
+        ["export aé", notValid("aé"), 1, { A: "one" }],
+        ["export -", notValid("-"), 1, { A: "one" }],
+        ["X=a-b; export $X", notValid("a-b"), 1, { A: "one" }],
+        ["export $(echo a-b)", notValid("a-b"), 1, { A: "one" }],
+        ["export b{-c,=1}", notValid("b-c"), 1, { A: "one", b: "1" }],
+        ['"export" a-b', notValid("a-b"), 1, { A: "one" }],
+        ["E=export; $E a-b", notValid("a-b"), 1, { A: "one" }],
+        ["(export a-b)", notValid("a-b"), 1, { A: "one" }],
+        ["export a-b | cat", notValid("a-b"), 0, { A: "one" }],
+        ["export a-b 2> /dev/null", "", 1, { A: "one" }],
+
+        // An empty name is refused. The message does not have the value.
+        ["export =x", notValid("="), 1, { A: "one" }],
+        ["export $UNSET=1", notValid("="), 1, { A: "one" }],
+        ['export =$(echo "x B=2")', notValid("="), 1, { A: "one", B: "2" }],
+
+        // A refused word does not stop the `NAME=value` operands around it.
+        ["export B=1 a-b D=2", notValid("a-b"), 1, { A: "one", B: "1", D: "2" }],
+        ["export a-b A=", notValid("a-b"), 1, { A: "" }],
+
+        // A bare NAME stores an empty value. After a refused word it is left alone.
+        ["export a-b A", notValid("a-b"), 1, { A: "one" }],
+        ["export B=1 -n A", notValid("-n"), 1, { A: "one", B: "1" }],
+        ["X=5; export a-b X", notValid("a-b"), 1, { A: "one" }],
+        // Before a refused word it is stored, as it is without one.
+        ["export A a-b", notValid("a-b"), 1, { A: "" }],
+
+        // `export` has no options. A leading option refuses the whole command.
+        ["export -x", invalidOption("-x"), 1, { A: "one" }],
+        ["export -n A", invalidOption("-n"), 1, { A: "one" }],
+        ["export -x B=1", invalidOption("-x"), 1, { A: "one" }],
+        ["export -X=1", invalidOption("-X"), 1, { A: "one" }],
+        ["export -- B=1", "", 0, { A: "one", B: "1" }],
+        ["export -- -n A", notValid("-n"), 1, { A: "one" }],
+
+        // Valid input. The name of a `NAME=value` operand is not checked, only that it is not empty.
+        ["export _b=1 B2=2", "", 0, { A: "one", _b: "1", B2: "2" }],
+        ["export B= C=a=b", "", 0, { A: "one", B: "", C: "a=b" }],
+        ["export a-b=1 a.b=c=d", "", 0, { A: "one", "a-b": "1", "a.b": "c=d" }],
+        ['export "" B=1', "", 0, { A: "one", B: "1" }],
+        ['export ""', "", 0, { A: "one" }],
+        ['export -- ""', "", 0, { A: "one" }],
+      ];
+
+      test.each(rows)("%s", async (script, stderr, exitCode, exported) => {
+        // Without `.quiet()` stderr is a file descriptor: the builtin finishes
+        // in the callback of the write. With `.quiet()` stderr is a buffer.
+        for (const quiet of [false, true]) {
+          const shell = $`${{ raw: script }}`.env(env).nothrow();
+          const result = await (quiet ? shell.quiet() : shell);
+          expect({
+            stdout: result.stdout.toString(),
+            stderr: result.stderr.toString(),
+            exitCode: result.exitCode,
+          }).toEqual({ stdout: "", stderr, exitCode });
+        }
+
+        // `export` with no operand prints each exported variable as `NAME=value`.
+        // PWD is in the list because this file calls `$.cwd()`.
+        const { stdout } = await $`${{ raw: script }}; export`.env(env).nothrow().quiet();
+        const { PWD, ...listed } = Object.fromEntries(
+          stdout
+            .toString()
+            .split("\n")
+            .filter(line => line.length > 0)
+            .map(line => [line.slice(0, line.indexOf("=")), line.slice(line.indexOf("=") + 1)]),
+        );
+        expect(listed).toEqual(exported);
+      });
+
+      test("a child process does not get a refused name", async () => {
+        const printEnv = `console.log(JSON.stringify(["a-b", "a b", "-n", "A", "D"].map(name => [name, process.env[name]])))`;
+        const { stdout, stderr } = await $`export a-b D=1 ${"a b"}; export -n A; ${BUN} -e ${printEnv}`
+          .env({ ...bunEnv, A: "one" })
+          .nothrow()
+          .quiet();
+        expect(stderr.toString()).toBe(notValid("a-b") + notValid("a b") + invalidOption("-n"));
+        expect(JSON.parse(stdout.toString())).toEqual([
+          ["a-b", null],
+          ["a b", null],
+          ["-n", null],
+          ["A", "one"],
+          ["D", "1"],
+        ]);
+      });
+
+      test("the message follows a redirect of stderr", async () => {
+        using dir = tempDir("export-stderr", {});
+        const toStdout = await $`export a-b 2>&1`.env(env).nothrow().quiet();
+        expect({
+          stdout: toStdout.stdout.toString(),
+          stderr: toStdout.stderr.toString(),
+          exitCode: toStdout.exitCode,
+        }).toEqual({ stdout: notValid("a-b"), stderr: "", exitCode: 1 });
+
+        const substituted = await $`echo "$(export a-b 2>&1)"`.env(env).nothrow().quiet();
+        expect(substituted.stdout.toString()).toBe(notValid("a-b"));
+
+        const toFile = await $`export a-b 2> err.txt`.env(env).cwd(String(dir)).nothrow().quiet();
+        expect(await Bun.file(join(String(dir), "err.txt")).text()).toBe(notValid("a-b"));
+        expect(toFile.exitCode).toBe(1);
+      });
+
+      test("an empty word from a JavaScript value is skipped", async () => {
+        const { stdout, stderr } = await $`export ${""} B=1 && echo $B`.env(env).quiet();
+        expect({ stdout: stdout.toString(), stderr: stderr.toString() }).toEqual({ stdout: "1\n", stderr: "" });
+      });
+
+      test("`export --` prints the exported variables", async () => {
+        const { stdout, stderr } = await $`export --`.env(env).quiet();
+        expect(stdout.toString()).toStartWith("A=one\n");
+        expect(stderr.toString()).toBe("");
+      });
+
+      TestBuilder.command`export 1abc`
+        .stderr(notValid("1abc"))
+        .exitCode(1)
+        .testMini({ onlyMini: true })
+        .runAsTest("in a script that `bun run` executes");
+
+      TestBuilder.command`export A=1 2B C=3 3D || echo "failed A=$A C=$C"`
+        .stdout("failed A=1 C=3\n")
+        .stderr(notValid("2B") + notValid("3D"))
+        .runAsTest("the next command of an || list runs");
+
+      TestBuilder.command`export NODE_ENV = test && echo ran`
+        .stderr(notValid("="))
+        .exitCode(1)
+        .runAsTest("spaces around = stop an && list");
+
+      TestBuilder.command`B=2 export -x B; echo "[$B]"`
+        .stdout("[]\n")
+        .stderr(invalidOption("-x"))
+        .runAsTest("a refused option does not apply a prefix assignment");
+
+      TestBuilder.command`export _ok OK2=1 && echo done`
+        .stdout("done\n")
+        .runAsTest("a valid bare name and a valid assignment exit 0");
+    });
   });
 
   describe("pipeline", async () => {