Repository navigation
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? |
santoshkumarradha
added
bug
santoshkumarradha
added
the
sev:critical
…e tests keep to a temp profile A connected service can name an owned CODEAF_ variable for its key, and every owned variable is still read under its former prefix, so that spelling reached a model's shell. It is now stripped with the rest. The new tests re-root the profile in a temporary directory instead of reading the operator's own, check that non-credential variables such as base URLs and proxies survive, and check that a child command cannot grant its own opt-in. The comment that spelled the retired prefix is respelled so the naming law passes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The page said no key reaches a model's shell command. The variables are removed, but a shell with file access can still read a key saved in the profile, and the opt-in only keeps a key that was already exported. The page now says both. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
It carried the issue number, 1484, so the pull-request gate would not find the entry for this change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Collaborator
|
Thanks for this, it's a real fix. I pushed three small commits on top so it can go in:
Checked the behaviour with the real binary against a local model stub: provider keys print as unset in both foreground and background commands, base URL and proxy variables survive, and the opt-in brings the keys back. |
AbirAbbas
approved these changes
Sep 30, 2026
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
4 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.