diff --git a/.changeset/bash-env-allowlist.md b/.changeset/bash-env-allowlist.md new file mode 100644 index 0000000000..dcfb8838d5 --- /dev/null +++ b/.changeset/bash-env-allowlist.md @@ -0,0 +1,7 @@ +--- +'@electric-ax/agents-runtime': patch +--- + +The built-in `bash` tool now spawns children with a filtered subset of the parent environment instead of `{...process.env}`. The default allowlist covers shell/locale/temp/XDG basics, terminal hints (`COLORTERM`, `NO_COLOR`, `FORCE_COLOR`, `CI`), proxy vars (`HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` + lowercase), TLS roots (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`), and Windows essentials (`SYSTEMROOT`, `COMSPEC`, `WINDIR`, `APPDATA`, `LOCALAPPDATA`, `USERPROFILE`). Keeps `git`, `npm`, `pnpm`, and most CLI tools functional across dev, CI, and corporate-proxy environments without leaking API keys. + +**Breaking:** anything that relied on env-supplied credentials reaching bash via `process.env` — `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GITHUB_TOKEN`, `BRAVE_SEARCH_API_KEY`, custom secrets — silently stops working. Mitigation: pass `allowedEnvKeys: ['GITHUB_TOKEN', ...]` to `createBashTool`. The list extends the safe defaults; it cannot shrink them. diff --git a/packages/agents-runtime/src/tools/bash.ts b/packages/agents-runtime/src/tools/bash.ts index b9e698b26c..685bfe691e 100644 --- a/packages/agents-runtime/src/tools/bash.ts +++ b/packages/agents-runtime/src/tools/bash.ts @@ -5,7 +5,61 @@ import type { AgentTool } from '@mariozechner/pi-agent-core' const TIMEOUT_MS = 30_000 const MAX_OUTPUT_CHARS = 50_000 -export function createBashTool(workingDirectory: string): AgentTool { +const DEFAULT_ALLOWED_ENV_KEYS: ReadonlyArray = [ + // Shell / identity + `PATH`, + `HOME`, + `USER`, + `LOGNAME`, + `SHELL`, + // Locale + terminal + `LANG`, + `LC_ALL`, + `LC_CTYPE`, + `TERM`, + `COLORTERM`, + `NO_COLOR`, + `FORCE_COLOR`, + `CI`, + // Temp dirs + `TMPDIR`, + `TMP`, + `TEMP`, + // XDG + `XDG_CONFIG_HOME`, + `XDG_CACHE_HOME`, + `XDG_DATA_HOME`, + // Proxies (lower + upper case — curl/git/node respect both forms) + `HTTP_PROXY`, + `HTTPS_PROXY`, + `NO_PROXY`, + `http_proxy`, + `https_proxy`, + `no_proxy`, + // TLS roots (corporate MITM) + `NODE_EXTRA_CA_CERTS`, + `SSL_CERT_FILE`, + `SSL_CERT_DIR`, + // Windows essentials — cmd.exe and Node lookups fail without these. + `SYSTEMROOT`, + `COMSPEC`, + `WINDIR`, + `APPDATA`, + `LOCALAPPDATA`, + `USERPROFILE`, +] + +export function createBashTool( + workingDirectory: string, + opts: { + /** Extends the built-in safe defaults; cannot shrink them. */ + allowedEnvKeys?: ReadonlyArray + } = {} +): AgentTool { + const allowedKeys = new Set([ + ...DEFAULT_ALLOWED_ENV_KEYS, + ...(opts.allowedEnvKeys ?? []), + ]) return { name: `bash`, label: `Bash`, @@ -20,7 +74,7 @@ export function createBashTool(workingDirectory: string): AgentTool { cwd: workingDirectory, timeout: TIMEOUT_MS, maxBuffer: 1024 * 1024, - env: { ...process.env }, + env: filterEnv(process.env, allowedKeys), }) let stdout = `` @@ -66,3 +120,15 @@ export function createBashTool(workingDirectory: string): AgentTool { }, } } + +function filterEnv( + parentEnv: NodeJS.ProcessEnv, + allowedKeys: ReadonlySet +): NodeJS.ProcessEnv { + const out: NodeJS.ProcessEnv = {} + for (const key of allowedKeys) { + const v = parentEnv[key] + if (v !== undefined) out[key] = v + } + return out +} diff --git a/packages/agents-runtime/test/bash-tool.test.ts b/packages/agents-runtime/test/bash-tool.test.ts index 82c84b1a24..b4bcde5eca 100644 --- a/packages/agents-runtime/test/bash-tool.test.ts +++ b/packages/agents-runtime/test/bash-tool.test.ts @@ -28,11 +28,7 @@ describe(`bash tool`, () => { expect(lines).toEqual([await realpath(cwd), process.env.HOME ?? homedir()]) }) - // Characterization: the bash tool currently passes `env: { ...process.env }` - // wholesale to spawned children (`bash.ts:23`). The two tests below capture - // that behavior so the env-scrubbing change planned for a follow-up PR has - // an explicit regression target. - it(`leaks the parent PATH into the child process (no env scrubbing)`, async () => { + it(`forwards PATH to the child process`, async () => { const tool = createBashTool(cwd) const result = await tool.execute(`call-path`, { command: `printf '%s' "$PATH"`, @@ -42,7 +38,7 @@ describe(`bash tool`, () => { ) }) - it(`leaks an ANTHROPIC_API_KEY-style env var to the child process`, async () => { + it(`does NOT forward ANTHROPIC_API_KEY (or other unlisted vars)`, async () => { const sentinel = `sk-test-bash-leak-${Date.now()}` const prev = process.env.ANTHROPIC_API_KEY process.env.ANTHROPIC_API_KEY = sentinel @@ -51,10 +47,39 @@ describe(`bash tool`, () => { const result = await tool.execute(`call-key`, { command: `printf '%s' "$ANTHROPIC_API_KEY"`, }) - expect((result.content[0] as { text: string }).text).toBe(sentinel) + // Empty stdout renders as "(no output)"; check just that the secret didn't appear. + expect((result.content[0] as { text: string }).text).not.toContain( + sentinel + ) } finally { if (prev === undefined) delete process.env.ANTHROPIC_API_KEY else process.env.ANTHROPIC_API_KEY = prev } }) + + it(`forwards variables named in allowedEnvKeys (extending the defaults)`, async () => { + const sentinel = `bash-allowlist-extend-${Date.now()}` + const prev = process.env.MY_CUSTOM + process.env.MY_CUSTOM = sentinel + try { + const tool = createBashTool(cwd, { allowedEnvKeys: [`MY_CUSTOM`] }) + const result = await tool.execute(`call-custom`, { + command: `printf '%s' "$MY_CUSTOM"`, + }) + expect((result.content[0] as { text: string }).text).toBe(sentinel) + } finally { + if (prev === undefined) delete process.env.MY_CUSTOM + else process.env.MY_CUSTOM = prev + } + }) + + it(`allowedEnvKeys extends defaults; PATH still passes through when other keys are added`, async () => { + const tool = createBashTool(cwd, { allowedEnvKeys: [`MY_CUSTOM`] }) + const result = await tool.execute(`call-extend-path`, { + command: `printf '%s' "$PATH"`, + }) + expect((result.content[0] as { text: string }).text).toBe( + process.env.PATH ?? `` + ) + }) })