From 4caf8a6d37bf268fc90df7850674f92686846ca8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 6 May 2026 15:48:16 +0000 Subject: [PATCH] fix(cli_driver): allowlist env vars passed to claude CLI subprocess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor security review flagged that run_claude() forwarded the entire parent environment to the externally installed claude CLI binary. In the PR gate flow the binary is dynamically installed from npm, and the surrounding job loads every upstream provider credential (ANTHROPIC_API_KEY, AWS_*, AZURE_FOUNDRY_*, VERTEXAI_CREDENTIALS, GITHUB_TOKEN, ...) into its env so the proxy can route requests. A compromised CLI release would have read access to all of them — even though the CLI itself only ever talks to the proxy via the explicit ANTHROPIC_BASE_URL/ANTHROPIC_AUTH_TOKEN we set. Build the subprocess env from a small allowlist of process-runtime vars (PATH, HOME, NVM_DIR, locale) rather than inheriting all of os.environ. Caller-supplied extra_env still rides on top, which is the sanctioned way for tests to opt-in to passing additional vars (e.g. extended_thinking sets MAX_THINKING_TOKENS). Add unit tests pinning the contract: PATH/HOME flow through, secrets do not, and extra_env can still override anything. Co-authored-by: Mateo Wang --- .../_driver_unit_tests/test_cli_driver.py | 61 +++++++++++++++++-- tests/claude_code/cli_driver.py | 36 ++++++++++- 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/tests/claude_code/_driver_unit_tests/test_cli_driver.py b/tests/claude_code/_driver_unit_tests/test_cli_driver.py index e7725b1116c..1eaa24eb90f 100644 --- a/tests/claude_code/_driver_unit_tests/test_cli_driver.py +++ b/tests/claude_code/_driver_unit_tests/test_cli_driver.py @@ -101,18 +101,71 @@ def test_run_claude_overlays_proxy_env(): assert env["ANTHROPIC_AUTH_TOKEN"] == "sk-abc" -def test_run_claude_extra_env_takes_precedence_over_os_environ(monkeypatch): - monkeypatch.setenv("FOO", "from-os") +def test_run_claude_extra_env_is_added_to_subprocess_env(): + """Caller-supplied extra_env entries land on the subprocess env.""" runner, captured = _make_runner(stdout="") run_claude( prompt="hi", model="claude-opus-4-7", base_url="http://localhost", api_key="sk-abc", - extra_env={"FOO": "from-arg"}, + extra_env={"MAX_THINKING_TOKENS": "4096"}, runner=runner, ) - assert captured["env"]["FOO"] == "from-arg" + assert captured["env"]["MAX_THINKING_TOKENS"] == "4096" + + +def test_run_claude_inherits_only_allowlisted_os_environ(monkeypatch): + """Process-runtime vars (PATH, HOME) flow through; credentials don't. + + The `claude` CLI is a Node binary installed dynamically from npm in + CI. If the package were ever compromised, inheriting the entire + parent environment would hand it every credential the surrounding + proxy job loads (AWS keys, Azure Foundry key, GitHub token, etc.). + Pin the contract: only the small allowlist of runtime vars is + inherited; everything else is dropped unless the caller passes it + explicitly via extra_env. + """ + monkeypatch.setenv("PATH", "/usr/bin:/usr/local/bin") + monkeypatch.setenv("HOME", "/home/runner") + monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "totally-secret") + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-proxy-only") + monkeypatch.setenv("AZURE_FOUNDRY_API_KEY", "azure-secret") + monkeypatch.setenv("VERTEXAI_CREDENTIALS", '{"private_key": "leak"}') + monkeypatch.setenv("GITHUB_TOKEN", "ghs_xxx") + + runner, captured = _make_runner(stdout="") + run_claude( + prompt="hi", + model="claude-opus-4-7", + base_url="http://localhost", + api_key="sk-abc", + runner=runner, + ) + env = captured["env"] + assert env["PATH"] == "/usr/bin:/usr/local/bin" + assert env["HOME"] == "/home/runner" + assert "AWS_SECRET_ACCESS_KEY" not in env + assert "ANTHROPIC_API_KEY" not in env + assert "AZURE_FOUNDRY_API_KEY" not in env + assert "VERTEXAI_CREDENTIALS" not in env + assert "GITHUB_TOKEN" not in env + + +def test_run_claude_extra_env_can_pass_through_otherwise_blocked_var(monkeypatch): + """The allowlist applies to inherited os.environ; extra_env is the + sanctioned way for a test to opt-in to passing something extra.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", "from-os") + runner, captured = _make_runner(stdout="") + run_claude( + prompt="hi", + model="claude-opus-4-7", + base_url="http://localhost", + api_key="sk-abc", + extra_env={"ANTHROPIC_API_KEY": "from-arg"}, + runner=runner, + ) + assert captured["env"]["ANTHROPIC_API_KEY"] == "from-arg" def test_run_claude_parses_stream_json_assistant_text(): diff --git a/tests/claude_code/cli_driver.py b/tests/claude_code/cli_driver.py index 823ae73f86d..388f3655503 100644 --- a/tests/claude_code/cli_driver.py +++ b/tests/claude_code/cli_driver.py @@ -38,6 +38,32 @@ DEFAULT_TIMEOUT_SECONDS = float( os.environ.get("LITELLM_COMPAT_CLI_TIMEOUT_SECONDS") or 120 ) +# Env vars the `claude` Node CLI legitimately needs to function: +# locating its own binary + node, finding HOME for ~/.claude config, +# basic locale/terminal plumbing. Deliberately excludes every +# credential-bearing var that the surrounding CI job sets for the +# proxy (ANTHROPIC_API_KEY, AWS_*, AZURE_*, VERTEXAI_CREDENTIALS, +# GITHUB_TOKEN, OPENAI_API_KEY, DATABASE_URL, ...). The CLI talks to +# the proxy via the explicit ANTHROPIC_BASE_URL/ANTHROPIC_AUTH_TOKEN +# we set below — it has no business reading the proxy's upstream +# credentials, and a compromised CLI release shouldn't be able to +# exfiltrate them out of the CI environment. +_CLI_ENV_ALLOWLIST: tuple = ( + "PATH", + "HOME", + "USER", + "LOGNAME", + "SHELL", + "TERM", + "TMPDIR", + "LANG", + "LC_ALL", + "LC_CTYPE", + "NODE_PATH", + "NVM_DIR", + "NVM_BIN", +) + class ClaudeCLIError(RuntimeError): """Raised when the `claude` CLI cannot be invoked or returns a fatal error.""" @@ -119,9 +145,17 @@ def run_claude( cmd.extend(extra_args) cmd.append(prompt) - env = {**os.environ, **(extra_env or {})} + # Build a minimal env for the CLI subprocess: only the allowlisted + # process-runtime vars from os.environ, plus the explicit proxy + # creds, plus any caller-supplied overrides. See _CLI_ENV_ALLOWLIST + # above for the security rationale. + env: Dict[str, str] = { + key: os.environ[key] for key in _CLI_ENV_ALLOWLIST if key in os.environ + } env["ANTHROPIC_BASE_URL"] = base_url env["ANTHROPIC_AUTH_TOKEN"] = api_key + if extra_env: + env.update(extra_env) # Throttle by provider *before* launching the CLI. Doing this here # (rather than per-test) means every code path that lands on