Repository navigation
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Dumbris
approved these changes
Oct 25, 2026
Member
|
Merged. Thank you, @rudycelekli! Current OpenAI keys ( Contributions like this are what keep MCPProxy moving, and they matter a lot to the project. One small ask, only if you feel like it: a short public note about your change (a tweet, a LinkedIn post, a line in a community you are part of) helps more people discover MCPProxy. No pressure at all, and I would be glad to repost it. |
This was referenced Oct 25, 2026
Dumbris
added a commit
that referenced
this pull request
Oct 29, 2026
## Summary
Fixes two secret leaks in `internal/logs/sanitizer.go` that were already
on `main`.
**1. Bearer suffix leak (pattern order).** The patterns run one after
another over the rewritten string, and every mask inserts `***`, which
no pattern's character class accepts. `github_token`, `openai_key` and
`anthropic_key` ran before `bearer_token`. So when a Bearer credential
started with a known key prefix, the key pattern masked it first, and
bearer then stopped at the inserted `***`:
```
before: Bearer sk-AbCdEf1234567890GhIjKlMnOpQrStUvWxYz0123456789.EXTRA_SECRET
-> Bearer sk-A***Ab***89.EXTRA_SECRET
after: -> Bearer sk-A***ET
```
The same thing could happen inside a JWT: a key-shaped run after a `-`
in a segment got masked before `jwt` ran, and the payload and signature
were logged. The fix runs `bearer_token`, then `jwt`, before the
key-specific patterns. A comment above the patterns now states the
ordering rule.
**2. Unsigned JWT (`alg: none`).**
`eyJhbGciOiJub25lIn0.eyJzdWIiOiIxIn0.` has an empty third segment, and
the trailing `\b` could never match after a `.`, so the payload was
logged in clear. The third segment is now `[A-Za-z0-9\-_]*` and the
trailing `\b` is gone. The output is `header.***.***`.
Existing mask formats are unchanged. That includes the tests from #1760,
#1761 and #1762. A credential after `Bearer` now gets the Bearer mask
instead of the key mask.
## Testing
- New `internal/logs/sanitizer_pattern_order_test.go`: Bearer +
sk-/sk-proj-/ghp_/sk-ant- suffix, a JWT with a key-shaped run inside it,
and an unsigned JWT in four contexts. All of these failed before the
fix.
- `go test -race ./internal/logs/` passes.
- golangci-lint v2 (`.github/.golangci.yml`) reports 0 issues, both bare
and with `--build-tags server`.
- Cross-review (opencode gpt-6.1-sol, high): CLEAN.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Bug
The log sanitizer's
openai_keypattern is\b(sk-[A-Za-z0-9]{20,})\b, which only matches the legacy OpenAI key format. Current keys aresk-proj-…,sk-svcacct-…andsk-admin-…, with_and-in the random suffix. The hyphen four characters in breaks the 20-alphanumeric run, so these keys are never matched. In free text, with no=/:before them forhigh_entropyto catch, they are logged in full.The audit redactor (
internal/audit/redact.go) and the security pattern set (patterns/tokens.go) were already updated for these formats; the log sanitizer was not.Trigger
On main the key is written to the log unchanged.
Fix
Extend the pattern with a
(proj|svcacct|admin)-branch that allows_and-in the suffix. The legacy branch stays alphanumeric, sosk-ant-…keys still fall through to the more specificanthropic_keypattern and keep its mask.Test
TestSanitizer_OpenAIProjectAndServiceAccountKeyschecks that the project, service-account, admin and legacy keys are all masked, and that ansk-ant-api03-…key is still masked byanthropic_key(sk-ant-api***).Commands run
CGO_ENABLED=0 go test -count=1 ./internal/logs: okgit stashofsanitizer.go), the new test fails for thesk-proj-,sk-svcacct-andsk-admin-keys withOpenAI key leaked.gofmt -lon the edited files: no output