Strip provider API keys from a model's shell environment - #1486
Open
biggdawg320 wants to merge 1 commit into
Open
biggdawg320 wants to merge 1 commit into
biggdawg320 wants to merge 1 commit into
Conversation
JobShellEnv passed the parent process environment straight through to every bash command a model runs, TMUX floor aside, so an exported OPENROUTER_API_KEY (or OPENAI_API_KEY, a vendored service's own KeyEnv, or a person's own custom-named service key) was readable and printable from inside the sandboxed shell on both belts: the foreground bash tool and the background job registry. JobShellEnv now also strips every provider credential env var codeaf knows about — config.APIKeyEnv, OPENAI_API_KEY, each modelsource.Vendored() source's KeyEnv, and each connected service's custom key_env from the profile's persisted sources — unless CODEAF_ALLOW_PROVIDER_KEYS_IN_SHELL is set in codeaf's own environment, read through env.Get so it carries the same AFORGE_ fallback every owned variable does. A profile-only key was never exported into os.Environ() in the first place, so it needed no change. Registers the new opt-in on settings.go's OperatorEnvPins allowlist, documents it in running-from-the-terminal.md, and adds a changelog entry recording the invalidated belief that shell commands used to inherit these keys. Co-Authored-By: Claude <noreply@anthropic.com>
|
|
Member
|
Hey, thanks for the PR. Could you sign the CLA when you get a chance so we can review it? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both the foreground bash tool and the background job registry built a model's shell environment through JobShellEnv, which stripped the tmux floor but otherwise passed codeaf's whole process environment through — so an exported OPENROUTER_API_KEY, OPENAI_API_KEY, another vendored provider's key, or a person's own custom-named service key was directly readable by any shell command the model ran. JobShellEnv now removes every provider credential env var codeaf knows about, including custom key_env names from the profile's persisted sources, reading the new CODEAF_ALLOW_PROVIDER_KEYS_IN_SHELL opt-in through internal/env's owned-variable door (env.Get) rather than os.Getenv, so it obeys the repository's environment law and carries the standard AFORGE_ legacy fallback. The opt-in is registered on settings.go's OperatorEnvPins allowlist, documented in the running-from-the-terminal manual page, and recorded in a new changelog entry (docs/changes/unreleased/1484-provider-keys-in-shell.md) noting that shell commands used to inherit these keys and no longer do. Added deterministic unit tests covering the strip, the opt-in, and custom persisted-source key variables (internal/exec/bare/shell_env_key_test.go, internal/exec/providerkeyenv_test.go).
Fixes #1484
Testing
internal/exec/bare/shell_env_key_test.go) that fails on the current code and passes with this change.GOTOOLCHAIN=auto go test -timeout 5m -count=1 ./internal/exec/bare/... ./internal/config/... && GOTOOLCHAIN=auto go test -timeout 3m -count=1 -run 'TestJobShellEnvCannotReachTheHostTmux|TestBackgroundJobShellCannotReachTheHostTmux' ./internal/exec/...This change was drafted with AI assistance (Claude) and verified against the project's own test suite before submission.