fix(security): redact credentials in JSON and form bodies under --verbose - #504
fix(security): redact credentials in JSON and form bodies under --verbose#504gyanranjanpanda wants to merge 3 commits into
Conversation
…bose The regex introduced in 0f8b000 only matched name=value (URL form encoding). The two dump sites that carry secrets both produce JSON bodies, so the pattern never fired. Additionally, the OAuth2 body field names (clientSecret, password, refreshToken) were absent from the alternation entirely. Replace the single catch-all regex with a structured approach: - Parse JSON bodies with encoding/json and mask sensitive keys by name. - Parse form bodies with url.ParseQuery and mask by key name. - Fall back to text-pattern matching for chunked or unparseable bodies. - Match key names after normalising snake_case / camelCase / kebab-case so every spelling variant of the same credential is covered. - Widen the header pattern to cover Proxy-Authorization, X-Auth-Token, Cookie and Set-Cookie in addition to Authorization. Sensitive key set: accessToken, refreshToken, idToken, token, clientSecret, password, secret, code, codeVerifier, authorization, apiKey. Non-sensitive metadata (token_type, expires_in, serviceId, …) is preserved so --verbose output remains useful for debugging. Fixes: microcks#503 (follow-up to 0f8b000, closes microcks#449) Signed-off-by: gyanranjanpanda <sanupanda141@gmail.com>
Resolve the add/add conflict on pkg/config/config_test.go against the suite added by microcks#498, which also defined TestRedactSensitiveContent and captureStdout. Both suites are kept. microcks#498's file is taken as the base; the redaction cases from this branch are appended as TestRedactSensitiveContentInBodies and rewritten in testify style to match the surrounding file. The duplicate captureStdout helper from this branch is dropped in favour of microcks#498's testing.TB variant. Two assertions in microcks#498's TestRedactSensitiveContent caught real defects in this branch, both now fixed in config.go: - Query parameters were no longer redacted. Splitting the dump into head and body moved all key matching into the body, so an OAuth code in a request line or redirect target passed through untouched -- the exact leak 0f8b000 set out to close. redactText now runs over the head as well. The value character class additionally excludes '?' so that the "http://host" portion of a URL cannot swallow the query string before its parameters are examined. - The Authorization header emitted "Bearer [REDACTED]". Preserving the auth scheme was a gratuitous change that broke the output contract microcks#498 pinned; reverted to "Authorization: [REDACTED]". Signed-off-by: gyanranjanpanda <sanupanda141@gmail.com>
a15df70 to
1261ec2
Compare
Caesarsage
left a comment
There was a problem hiding this comment.
This is good.
I left a comment. Also, could you reduce the comments , it is making the code unnecessarily large and noisy. comments is only useful where it is really needed like on the complex regex and should be simple
| } | ||
|
|
||
| func TestRedactSensitiveContentPreservesCRLF(t *testing.T) { | ||
| dump := "GET / HTTP/1.1\r\nAuthorization: Bearer eyJLEAKEDACCESS\r\nAccept: */*\r\n\r\n" |
There was a problem hiding this comment.
One gap here: authToken / auth-token values leak through — authtoken isn't in sensitiveValueKeys, and auth-token is the exact key the CLI's own config uses:
dump := "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\r\n" +
{"authToken":"eyJPROBELEAK"}
redactSensitiveContent(dump) // value survives
Adding "authtoken": {} to the set covers both spellings via the normalizer.
Minor, non-blocking: json.Marshal re-encoding reorders keys and HTML-escapes, so dumped bodies aren't byte-faithful (and won't match Content-Length) — worth a one-line code comment.
- Add authtoken to sensitiveValueKeys to cover authToken and auth-token in JSON and form bodies. - Add one-line note regarding json.Marshal re-encoding. - Reduce comment noise across redaction logic and tests. - Add test coverage for authToken and auth-token redaction. Signed-off-by: gyanranjanpanda <sanupanda141@gmail.com>
Problem
Commit
0f8b000(closes #449) addedredactSensitiveContentto verbose HTTP dumps. It works forAuthorizationheaders but thesensitiveParamPatternregex only matchesname=value(form encoding). Both dump sites that carry secrets produce JSON bodies, so the pattern never fires.Leak 1 —
keycloak_client.go:96: Keycloak token response body is JSON.access_token,refresh_token, andid_tokenprint verbatim.Leak 2 —
microcks_client.go:371:OAuth2ClientContextis marshalled into the POST/api/testsbody.clientSecret,password, andrefreshTokenare not in the pattern at all.The
Authorization: [REDACTED]header line makes the output look safe, masking the residual exposure.Tracked in #503.
Fix
Replace the single catch-all regex with a structured approach:
encoding/json, redact sensitive keys by nameurl.ParseQuery, redact by key nameKey matching normalises
snake_case,camelCase, andkebab-caseso every spelling variant is covered (access_token,accessToken,Access-Tokenall match).Non-sensitive metadata (
token_type,expires_in,serviceId) is preserved so--verbosestays useful.Tests
pkg/config/config_test.go— 13 tests (all new), covering:oAuth2Context(regression for Leak 2)DumpRequestIfRequired/DumpResponseIfRequired