feat(agents-runtime)!: bash tool runs children with an env allowlist

[msfstef]

Summary

Closes the env-var exfiltration leak in the built-in bash tool. Previously bash.ts passed env: { ...process.env } wholesale to child_process.exec, so the LLM could exfiltrate any credential-shaped variable via echo \$ANTHROPIC_API_KEY. Now bash spawns children with a filtered allowlist constructed once at tool creation.

See plans/sandboxing-investigation.md §1.3, §1.5, §3.5.

Default allowlist

Covers shell/locale/temp/XDG basics plus categories where the absence of a variable would silently break tooling rather than improve security:

• Terminal/UX: COLORTERM, NO_COLOR, FORCE_COLOR, CI
• Proxies: HTTP_PROXY, HTTPS_PROXY, NO_PROXY plus lowercase variants (curl/git/node respect both forms)
• TLS roots: NODE_EXTRA_CA_CERTS, SSL_CERT_FILE, SSL_CERT_DIR (needed in corporate-MITM setups)
• Windows essentials: SYSTEMROOT, COMSPEC, WINDIR, APPDATA, LOCALAPPDATA, USERPROFILE (cmd.exe and Node lookups fail without these)

Deliberately excluded from defaults despite being commonly needed — these are real injection vectors and need an explicit opt-in:

• NPM_CONFIG_* (auth tokens in npmrc-equivalent envs)
• GIT_SSH_COMMAND (arbitrary command injection)
• NODE_OPTIONS (--require ./malicious.js is code-injection)

Extension

createBashTool(cwd, { allowedEnvKeys: ['GITHUB_TOKEN'] })

The list extends (does not replace) the default allowlist. Customers can add keys but cannot accidentally shrink the safe defaults — silently dropping PATH would break tooling without a useful error.

Breaking

Anything relying on env-supplied credentials reaching bash through process.env — ANTHROPIC_API_KEY, OPENAI_API_KEY, GITHUB_TOKEN, BRAVE_SEARCH_API_KEY, custom secrets — silently stops working. Intended behavior. One-line mitigation per call via allowedEnvKeys as above.

Test plan

• Env-leak characterization from test(agents): characterize bash/read/write/edit/fetch_url + Horton composition #4354 flipped: ANTHROPIC_API_KEY no longer appears in child output
• PATH still forwarded
• allowedEnvKeys: ['MY_CUSTOM'] forwards that key to the child
• Extending the allowlist doesn't remove defaults — PATH still passes when other keys are added
• pnpm test, pnpm typecheck, pnpm stylecheck clean

Addressed from review

• Broadened the default allowlist with proxy/TLS/terminal/Windows vars so this PR doesn't break git/pnpm/network calls behind corporate proxies or on Windows by default.
• The merged key Set is now constructed once at createBashTool time instead of per execute() call. process.env is still read per-call so live mutation of the parent env is honored.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.94%. Comparing base (1ab43f5) to head (a9d2786).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4362      +/-   ##
==========================================
+ Coverage   55.84%   55.94%   +0.09%     
==========================================
  Files         245      245              
  Lines       24847    24902      +55     
  Branches     6878     6882       +4     
==========================================
+ Hits        13876    13931      +55     
  Misses      10957    10957              
  Partials       14       14              

Flag	Coverage Δ
packages/agents	70.92% <ø> (ø)
packages/agents-runtime	81.38% <100.00%> (+0.10%)	⬆️
packages/agents-server	73.93% <ø> (ø)
packages/agents-server-ui	6.66% <ø> (ø)
packages/electric-ax	42.61% <ø> (ø)
typescript	55.94% <100.00%> (+0.09%)	⬆️
unit-tests	55.94% <100.00%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude Code Review

Summary

Iteration 2 takes the same allowlist approach as before but absorbs nearly all of the feedback from iteration 1: the default set now covers proxy, TLS, CI/terminal, and Windows-essential vars, and the merged Set is hoisted into createBashTool so it isn't rebuilt per call. The core security property (no ANTHROPIC_API_KEY-style leak via process.env) is unchanged and still well-tested.

What's Working Well

• Allowlist widening lands the right buckets. HTTP_PROXY/HTTPS_PROXY/NO_PROXY (both cases), NODE_EXTRA_CA_CERTS/SSL_CERT_FILE/SSL_CERT_DIR, COLORTERM/NO_COLOR/FORCE_COLOR/CI, and LOGNAME are all in DEFAULT_ALLOWED_ENV_KEYS (packages/agents-runtime/src/tools/bash.ts:8-50). This directly addresses the corporate-proxy / TLS-MITM concern from the last review and removes the most likely "git/pnpm hangs after upgrade" failure mode.
• Windows essentials present. SYSTEMROOT, COMSPEC, WINDIR, APPDATA, LOCALAPPDATA, USERPROFILE are forwarded, so cmd.exe-driven invocations of bash won't fail in opaque ways on Windows. Node's process.env is already a case-insensitive proxy on Windows, so iterating uppercase keys and reading parentEnv[key] is fine here — no case mismatch to worry about.
• Hoisting applied. allowedKeys is built once inside createBashTool (bash.ts:59-62), not on every execute() call. filterEnv still reads process.env per-call, so live mutation is honored — only the key set is hoisted, which is exactly the right factoring.
• Changeset updated to mention the proxy / TLS / Windows additions, matching what the code now does.

Issues Found

Critical (Must Fix)

None.

Important (Should Fix)

None remaining from the prior review.

Suggestions (Nice to Have)

A few non-secret keys still missing from the defaults

File: packages/agents-runtime/src/tools/bash.ts:8-50

The new defaults are good, but a small number of well-known non-credential keys are still stripped and have historically tripped users in this exact spot:

• GIT_SSH_COMMAND, GIT_TERMINAL_PROMPT — non-interactive git over SSH; without GIT_TERMINAL_PROMPT=0 git will block waiting for a password on hosts with auth misconfigurations.
• NODE_OPTIONS — common for --max-old-space-size tuning in CI.
• npm_config_* family — pnpm and npm consult these for registry config; not expressible as exact keys without a prefix-match. Document the workaround (createBashTool(cwd, { allowedEnvKeys: ['npm_config_registry', ...] })) or skip — your call.

Not blocking; raising because the same "customer-reports-a-broken-tool, root-caused-here" failure mode applies. These can also be added in a follow-up.

Commit message lists the old allowlist

Reference: commit a9d27861734

The commit message body still describes the narrower v1 allowlist (PATH, HOME, USER, SHELL, LANG, LC_ALL, LC_CTYPE, TERM, TMPDIR, TMP, TEMP, XDG_*) — the proxy/TLS/Windows additions that landed in v2 aren't reflected. Cosmetic, but git log archaeology will misrepresent what shipped. Squash-on-merge with a fresh message would fix it.

Changeset doesn't acknowledge what's still in scope

The changeset accurately describes what this PR fixes (env-var leak) and the breaking-change mitigation. It doesn't mention that file-based credential exfiltration (cat ~/.npmrc, cat ~/.aws/credentials, cat ~/.config/gh/hosts.yml, cat ~/.netrc) remains open — see plans/sandboxing-investigation.md §1.3, §1.5, §3.5. Optional, but a one-liner like "process env is one of several leak channels; filesystem access is still unrestricted" prevents users from reading the changeset and assuming this PR fully sandboxes secrets.

Negative test could lock the contract tighter

File: packages/agents-runtime/test/bash-tool.test.ts:50-53

(Carry-over from iteration 1, still applies.) expect(...).not.toContain(sentinel) is correct but loose. expect(text).toBe('(no output)') plus exitCode: 0 would catch silent regressions where the env is forwarded but the output happens to be munged. Lowest priority — the secret-not-present assertion is what matters.

Issue Conformance

No linked issue. PR description still references plans/sandboxing-investigation.md and #4354 as the stacked base. Implementation matches the stated scope — env-var leak, not full sandbox.

Previous Review Status

Addressed:

• Default allowlist widened to cover proxy, TLS, CI/terminal, Windows essentials, and LOGNAME (Important Working with Postgres WAL format from Elixir #1 from iteration 1).
• Windows portability — essential vars now in the allowlist; no platform guard needed because the tool works on Windows now (Important [Merged on #3] Write inserts, updates and deletes to Vaxine #2 from iteration 1).
• Hoisting allowedKeys out of the hot path (Suggestion from iteration 1).

Not addressed (all Suggestion-level, fine to defer):

• ELECTRIC_AGENTS_BASH_ALLOWED_ENV_KEYS env-driven extension.
• Tightening the negative test to strict-equality on (no output).
• Changeset note that file-level exfiltration is still open.

---

Review iteration: 2 | 2026-05-19