fix(mcp): normalize env names before secret-pattern matching - #106
Merged
Conversation
isSensitiveEnvVar substring-matched patterns like API_KEY against the raw variable name, so MCP server env overrides named API-KEY, APIKEY, or PRIVATE-KEY - all legal spellings in environment variable names - were forwarded to potentially untrusted MCP subprocesses. Only the override path was affected; the inherited-environment path is allowlist-based. Names are now normalised (uppercased, "-" and "_" stripped) before matching, and the patterns are stored in normalised form, so API-KEY/APIKEY/Api-Key/PRIVATE-KEY/SECRET-KEY/AUTH-TOKEN and similar spellings are all stripped. Regression tests extend TestBuildEnv_OverridesCannotInjectSecrets with non-underscore spellings and add TestIsSensitiveEnvVar_NormalizesSeparators covering blocked spellings and benign names. Docs updated (SECURITY.md section 19, AGENTS.md).
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
odek | 3b085b8 | Commit Preview URL Branch Preview URL |
Jul 26 2026, 09:18 AM |
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
1 participant
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.
Summary
Fixes the reported finding (verified against current code before fixing):
Change
internal/mcpclient/client.go::isSensitiveEnvVarnow normalises each name before matching — uppercased, with-and_stripped — and the patterns are stored in normalised form (APIKEY,TOKEN,SECRET,PASSWORD,CREDENTIAL,CREDS,PRIVATEKEY,ACCESSKEY). This is the report's first mitigation option (normalise before matching).Verified before the fix that
API-KEY,APIKEY, andPRIVATE-KEYreturnedsensitive=false; all spellings are now caught:API-KEY,APIKEY,Api-Key,PRIVATE-KEY,PRIVATEKEY,ACCESS-KEY,SECRET-KEY,SECRETKEY,AUTH-TOKEN,AUTHTOKEN,DB-PASSWORD,AWS-CREDS, …PATH,HOME,EDITOR,NODE_ENV,MCP_SERVER_NAME,KEYSTONE, …Tests
TestBuildEnv_OverridesCannotInjectSecretswith non-underscore override spellings (API-KEY,APIKEY,PRIVATE-KEY,SECRET-KEY) — all dropped from the child environment.TestIsSensitiveEnvVar_NormalizesSeparatorstable test covering blocked spellings and benign names.go test ./... -count=1✅ (28 packages),go vet✅,golangci-lint→ 0 issues ✅Docs updated:
docs/SECURITY.md§19 andAGENTS.md(MCP subprocess environment sanitisation entries).