mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-23 00:41:40 +00:00
fix(cli_driver): allowlist env vars passed to claude CLI subprocess
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 <mateo-berri@users.noreply.github.com>
This commit is contained in:
parent
4494c43fe7
commit
4caf8a6d37
2 changed files with 92 additions and 5 deletions
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue