Repository navigation
fix: only load allowlisted API keys from shared .env files - #511
Open
ixion36-svg wants to merge 1 commit into
Open
ixion36-svg wants to merge 1 commit into
ixion36-svg wants to merge 1 commit into
Conversation
load_env() copied every key it found into os.environ. Two of the three paths it reads -- ~/.claude/skills/.env and ~/.claude/.env -- are shared with other tools, and these scripts call third-party image APIs, so an unrelated secret parked in one of them entered the process environment. Restrict the import to the four keys these scripts actually read. Also strip the key, without which "KEY = value" yields "KEY " and never matches. Co-Authored-By: Claude Opus 5 <[email protected]>
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
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.
What
load_env()in the threedesigngenerate scripts copies every key it finds intoos.environ:It reads three paths, in order:
Why it is worth narrowing
The last two are not the skill's own files.
~/.claude/.envis a shared location that any Claude Code skill or tool may write to, and the same process then makes outbound calls toapi.muapi.ai,api.atlascloud.aiand the Gemini API.So an unrelated secret parked in a shared
.env— a database password, a cloud key — is loaded into the environment of a process that talks to third-party services. Nothing in this repo sends it anywhere, and this is not a leak report. The point is that the blast radius is set by a file the script does not own, while the script only ever needs four keys.I raised this against my own install, found the three scripts share the pattern, and am sending the fix upstream rather than carrying a local patch.
Change
An explicit allowlist of the keys these scripts actually read:
cip/generate.pyGEMINI_API_KEY,GOOGLE_API_KEYicon/generate.pyGEMINI_API_KEYlogo/generate.pyGEMINI_API_KEY,ATLASCLOUD_API_KEY,MUAPI_API_KEYThe union is used in all three so
load_env()stays identical across the files — happy to switch to per-file lists if you'd prefer them tighter.Also adds
key.strip(): without itKEY = value(spaces around=) yields a key of"KEY ", which would silently fail the allowlist check.Compatibility
No behaviour change for the keys the scripts use — they load exactly as before. Only unrelated keys stop being imported. No new dependencies, no signature changes.
Verification
Lifted
ENV_ALLOWLISTandload_envout of the patched source via AST and ran them against a.envholding two allowlisted keys and three decoys:AWS_SECRET_ACCESS_KEY,ION_DB_PASSWORDandGITHUB_TOKENstayed out. All three files compile; tested with both LF and CRLF.envinput.Note on channel
SECURITY.mdasks for private reporting of vulnerabilities. I judged this out of that scope — the in-scope list is CLI code execution, path traversal and npm supply chain, and there is no attacker-controlled path here. It is defence-in-depth, so a public PR seemed right. Happy to move it if you disagree.