fix(bash): pass per-exec env to nested sh/bash and stop leaking parent vars

[trieloff]

Fixes #438.

Problem

Bash.exec(…, { env, replaceEnv: true }) sets the per-exec variables for directly run commands, but a nested sh/bash never sees them, while the constructor's env (e.g. SECRET) still reaches it. There were two halves, as the issue describes:

1. Bash.exec added per-exec keys to env but not to exportedVars. It also kept the parent's export set under replaceEnv, and shared that Set by reference, so an export in one exec() leaked into every later one.
2. sh/bash (executeScript) called the nested exec without replaceEnv, so the child merged onto the persistent shell's full env. That made unexported variables and parent secrets visible.

Fix

• Exports (Bash.ts): the export set is copied per exec. Only the host Bash.exec API and new shells treat env as the environment and export its keys, and only valid names count, so positional parameters (0, #, 1) stay local. Internal callers such as env and time pass the full variable map, so they keep the persistent export set as before.
• New shells (newShell on the internal CommandExecOptions): nested sh/bash passes only the parent's exported env plus positional parameters, and asks for a new shell. Bash.exec then initializes what bash sets up at startup, all unexported:
  • PATH and the host variables (OSTYPE, MACHTYPE, HOSTTYPE, HOSTNAME) get fixed defaults when the environment lacks them
  • IFS, OPTIND, SHELLOPTS and BASHOPTS are always reset
• cd -: under replaceEnv the previous directory comes only from the provided OLDPWD, not the parent's state. A child that didn't inherit OLDPWD reports cd: OLDPWD not set.

Top-level replaceEnv still starts from exactly the given env. printenv with { env: { A: "1" }, replaceEnv: true } prints only A=1.

Behaviour (checked against /bin/bash 3.2.57 and bash 5.3)

script	before	after / bash
sh -c 'echo [$MARKER]; printenv SECRET' with { env: {MARKER:"YES"}, replaceEnv: true }	[] / leak	[YES] / exit 1
export FOO=bar in one exec, then FOO=noleak; sh -c 'echo [$FOO]'	[noleak]	[]
FOO=secret; env sh -c 'echo [$FOO]'	[]	[]
export -n HOME; sh -c 'echo [$HOME]'	[/home/user]	[]
HOSTNAME=secret; export -n HOSTNAME; sh -c 'echo $HOSTNAME'	secret	fresh default
unset PATH; sh -c 'IFS=:; export -p'	—	lists none of PATH/IFS/OPTIND/…
export IFS=: OPTIND=5; sh -c '…'	IFS=:…	IFS=' \t\n', OPTIND=1
cd /tmp; export -n OLDPWD; sh -c 'cd -'	/home/user	OLDPWD not set, exit 1

(OSTYPE/HOSTNAME defaults are the sandbox's linux-gnu/localhost rather than the host's values.)

Out of scope, unchanged from main

• FOO=x env sh -c 'echo $FOO' prints [] (bash: [x]), because the env command doesn't mark its assignments exported.
• env -i printenv prints PWD=….
• env/printenv list all shell variables, not just exported ones.

Testing

• src/commands/bash/bash.env.test.ts (24 tests, one per scenario above). 6 of them cover the review findings and fail on the first revision of this PR.
• pnpm typecheck, pnpm lint:fix, pnpm lint:banned, pnpm knip are clean. There is a changeset.
• pnpm test:run: everything passes except 6 Python/WASM tests that time out loading CPython locally (they pass in CI).

🤖 Generated with Claude Code

[vercel]

@claude is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

[auto-maintain]

🤖 auto-maintain review

Automated, advisory triage for @trieloff's PR. Facts below are read from the GitHub API.

Check	Result
Author's merged PRs (this repo)	18
Account established	✅ (age 6491d · 135 followers · 204 public repos)
Commits signed/verified	✅ 2/2
Changeset included	✅ (.changeset/nested-shell-environment.md)

Review panel: 🔴 high highest severity

just-bash maintainer code review: 🟡 medium

The environment isolation fix has two compatibility gaps in nested-shell inheritance that should be corrected before merge.

• packages/just-bash/src/Bash.ts:787 — Nested shells unconditionally discard and overwrite inherited SHELLOPTS/BASHOPTS. After `export SHELLOPTS; set -x`, `bash -c ...` therefore loses the exported option state instead of enabling xtrace, contrary to Bash startup semantics.
• packages/just-bash/src/Bash.ts:766 — Filtering per-exec environment keys through shell-identifier syntax drops valid process-environment entries such as `A-B=x` from nested shells. The same key supplied to the constructor is inherited, so per-exec env remains behaviorally inconsistent; exclude only synthetic positional keys instead.

General code review: 🔴 high

The direct nested-shell path is fixed, but wrapper-mediated execution still bypasses `replaceEnv` and leaks parent secrets.

• packages/just-bash/src/Bash.ts:740 — Nested shells invoked through wrappers can still recover constructor secrets. Non-`newShell` recursive execs seed from `this.state.env`; for example, `time` merges its clean `ctx.env` without `replaceEnv`, so `new Bash({env:{SECRET:'leak'}}).exec("time sh -c 'printenv SECRET' 2>/dev/null", {env:{MARKER:'YES'}, replaceEnv:true})` exposes `SECRET`. Recursive execution must preserve the current isolated environment instead of rebuilding from persistent state.

Adversarial security: 🟡 medium

Environment isolation remains bypassable through the shared command-resolution cache.

• packages/just-bash/src/Bash.ts:810 — New shells replace PATH but reuse the persistent command hash. Resolution consults this cache before PATH, so a script cached from a parent or earlier exec can run inside an isolated nested shell despite being absent from its PATH, potentially exposing that execution's secrets. Use a fresh hash table for newShell/replaceEnv contexts.

Adversarial security (second opinion): 🟡 medium

The change itself is a legitimate env-isolation fix with no backdoor, obfuscation, or new external effect, but it introduces a silent regression (exported vars lost for shells nested under env/time) and leaves the advertised replaceEnv isolation bypassable through timeout/xargs/find.

• packages/just-bash/src/Bash.ts:759 — Regression: exports made inside the running script no longer reach shells nested under wrapper commands. At execDepth >= 1 `exportsEnv` is false, so the child's export set is rebuilt from `this.state.exportedVars` — the pristine instance-level set — and, because the Set is now copied per exec instead of shared, `export FOO=bar` in the script never lands there. `export FOO=bar; env sh -c 'echo [$FOO]'` (and the same with `time`) printed `[bar]` on base and prints `[]` now; real bash prints `bar`. The new test only covers the unexported case (`FOO=secret; env sh -c ...`), so this silently drops legitimately exported variables.
• packages/just-bash/src/Bash.ts:740 — The per-exec env isolation this PR advertises is still bypassable: commands that re-enter `ctx.exec` without passing env (`timeout`, `xargs`, `find -exec`, `rg --pre`) land in the non-replaceEnv branch, so the child env is rebuilt from `this.state.env` — the constructor environment — discarding the caller's `replaceEnv: true`. With `new Bash({env:{SECRET:'leak'}})`, `bash.exec("timeout 5 printenv SECRET", {env:{}, replaceEnv:true})` (or `echo x | xargs printenv SECRET`) still prints the secret, while the newly-added `sh -c` test asserts it must not. Worth closing or documenting alongside the sh/bash fix, since the changeset states the guarantee unconditionally.

Standard Bash and host portability: 🟡 medium

The environment isolation fix breaks standard Bash startup handling for inherited special option variables.

• packages/just-bash/src/Bash.ts:787 — Nested shells unconditionally discard inherited SHELLOPTS/BASHOPTS export state and values. Standard Bash imports these variables, enables the listed options, and preserves their export attribute; e.g. `set -u; export SHELLOPTS; bash -c 'echo $missing'` must fail, but this implementation resets `nounset` and succeeds. IFS/OPTIND should reset their values but likewise retain export status when inherited.

_{Posted by auto-maintain. This automated code review is advisory; a human maintainer makes the call.}

[trieloff]

Thanks. All findings are addressed in 2780bf9:

• Seeded variables exported / copied from the parent map (bash.ts:159/165/169, both reviewers): nested sh/bash no longer seeds anything through env. It passes only the exported env plus positional parameters and sets an internal newShell flag. Bash.exec then initializes PATH and the host variables from fixed defaults (only when not exported), and always resets IFS/OPTIND/SHELLOPTS/BASHOPTS, all unexported. HOSTNAME=secret; export -n HOSTNAME no longer reaches the child, and export -p there doesn't list any of these.
• env/time re-export the whole variable map (Bash.ts:749): only the host Bash.exec API and new shells export their env keys. Internal callers keep the persistent export set, so FOO=secret; env sh -c 'echo [$FOO]' prints [] again.
• SHELLOPTS/BASHOPTS injected into env -i (Bash.ts:761): these are now set only for new shells.
• previousDir leaks through replaceEnv (Bash.ts:777): replaceEnv takes the previous directory only from the provided OLDPWD, and cd - without one reports OLDPWD not set.
• Changeset: added .changeset/nested-shell-environment.md.

Each finding has a regression test in bash.env.test.ts.

[auto-maintain]

⚠️ auto-maintain: review severity raised to 🔴 high

A new push changed this PR and the review now contains a higher-severity finding. See the updated review comment above.