Fix/redact sensitive auth logs by Priyanshubhartistm · Pull Request #333 · microcks/microcks-cli

Priyanshubhartistm · 2026-05-08T17:50:58Z

Description

Introduced a new pkg/utils package with sanitization helpers (SanitizeString, SanitizeJSON, SanitizeHeaders, MaskSecret) that redact sensitive values before they are written to logs or stdout.
Fixed DumpRequestIfRequired and DumpResponseIfRequired in pkg/config/config.go to pass all raw HTTP dump output through SanitizeString before printing, ensuring Authorization: Basic/Bearer headers, access_token/refresh_token response bodies, and URL-encoded form secrets (e.g. client_secret=) are always replaced with [REDACTED] — this single change covers all callers across keycloak_client.go and microcks_client.go.
Fixed cmd/login.go to mask the SSO access token and refresh token with MaskSecret before logging, and to sanitize the OAuth2 callback URL with SanitizeString to prevent authorization codes from leaking in log output.
Added 7 unit tests in pkg/utils/sanitize_test.go covering MaskSecret, SanitizeHeaders, nested JSON redaction, case-insensitive key matching, malformed JSON passthrough, HTTP dump with form body, and Basic auth header redaction.

Before (with --verbose):

Authorization: Basic dGVzdGNsaWVudDpzdXBlcnNlY3JldA==
...
{"access_token":"eyJhbGciOiJSUzI1NiJ9...","refresh_token":"eyJhbGc..."}
...
Token: eyJhbGciOiJSUzI1NiJ9...
Refresh Token: eyJhbGc...

After (with --verbose):

Authorization: [REDACTED]
...
{"access_token":"[REDACTED]","refresh_token":"[REDACTED]"}
...
Token: [REDACTED]
Refresh Token: [REDACTED]

Test results

$ go test ./pkg/utils/ -v

=== RUN   TestMaskSecret
--- PASS: TestMaskSecret (0.00s)
=== RUN   TestSanitizeHeaders
--- PASS: TestSanitizeHeaders (0.00s)
=== RUN   TestSanitizeJSONNested
--- PASS: TestSanitizeJSONNested (0.00s)
=== RUN   TestSanitizeJSONCaseInsensitive
--- PASS: TestSanitizeJSONCaseInsensitive (0.00s)
=== RUN   TestSanitizeJSONMalformed
--- PASS: TestSanitizeJSONMalformed (0.00s)
=== RUN   TestSanitizeStringHeadersAndForm
--- PASS: TestSanitizeStringHeadersAndForm (0.00s)
=== RUN   TestSanitizeStringBasicAuth
--- PASS: TestSanitizeStringBasicAuth (0.00s)
PASS
ok  	github.com/microcks/microcks-cli/pkg/utils	0.006s

Related issue(s)

Fixes #265

github-actions · 2026-05-08T17:51:08Z

👋 @Priyanshubhartistm

Welcome to the Microcks community! 💖

Thanks and congrats 🎉 for opening your first pull request here! Be sure to follow the pull request template or please update it accordingly.

Hope you have a great time there!

Priyanshubhartistm · 2026-05-08T22:05:27Z

Hii @lbroudoux, @yada , @Harsh4902 could you please take a look at this PR when you get a chance? Would appreciate your review

Introduce a new pkg/utils/sanitize.go package with utilities to redact sensitive values before they are written to logs or stdout: - MaskSecret: replaces any non-empty secret with '[REDACTED]' - SanitizeHeaders: returns a copy of http.Header with sensitive header values replaced - SanitizeJSON: recursively redacts sensitive keys in JSON payloads - SanitizeString: handles raw HTTP dump strings (CRLF/LF), sanitizing headers, Authorization scheme tokens, and URL-encoded form fields (access_token=, client_secret=, password=, etc.) Sensitive key matching is case-insensitive and covers authorization, proxy-authorization, access_token, refresh_token, id_token, client_secret, password, token, api_key, cookie, set-cookie, x-api-key. Also adds sanitize_test.go with 7 unit tests covering all helpers. Closes microcks#265 Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>

DumpRequestIfRequired and DumpResponseIfRequired previously printed raw httputil dump output directly to stdout, which included Authorization headers (Basic and Bearer), token response bodies, and form-encoded secrets when --verbose was active. Pass the dump through utils.SanitizeString before printing so that all sensitive headers and body values are replaced with '[REDACTED]'. This single fix covers all callers across the codebase: - pkg/connectors/keycloak_client.go (Keycloak auth requests/responses) - pkg/connectors/microcks_client.go (Microcks API requests/responses) Fixes microcks#265 Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>

The oauth2login function logged the raw access token and refresh token directly via log.Printf, making them visible in any terminal or CI/CD log where --verbose is active. - Replace token log calls with utils.MaskSecret so the values are always printed as '[REDACTED]' regardless of --verbose state - Sanitize the OAuth2 callback URL logged on each redirect using utils.SanitizeString to redact any authorization codes or state params that may appear in query strings Fixes microcks#265 Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>

Vaishnav88sk

Resolve the conflicts.

Vaishnav88sk · 2026-08-11T12:47:46Z

+	log.Printf("Token: %s\n", utils.MaskSecret(tokenString))
+	log.Printf("Refresh Token: %s\n", utils.MaskSecret(refreshToken))


These two log.Printf lines are added unconditionally (outside any --verbose guard). The original code didn't log tokens at all after the server shutdown. Now every SSO login prints [REDACTED] tokens to the terminal. Not a security leak since you're masking them, but it's adding new output that wasn't there before. Is that intentional? Might confuse users or break scripts that parse stdout.

Vaishnav88sk · 2026-08-11T12:49:41Z

+	if sanitized, ok := sanitizeHTTPDump(input); ok {
+		return sanitized
+	}
+


SanitizeJSON uses json.Marshal which produces compact JSON. If the original dump was indented/prettified for debugging, the sanitized output comes back minified. Could be disorienting when comparing before/after. Consider using json.MarshalIndent to preserve formatting.

Vaishnav88sk · 2026-08-11T12:51:58Z

No tests for empty/nil inputs in SanitizeString or SanitizeHeaders. SanitizeString("") should return "" but it's not verified.

Vaishnav88sk · 2026-08-11T12:56:20Z

+func TestSanitizeStringHeadersAndForm(t *testing.T) {
+	input := "Authorization: Bearer abc\nX-Api-Key: key\nContent-Type: text/plain\n\nclient_secret=secret&grant_type=password"
+	sanitized := SanitizeString(input)
+
+	if !containsLine(sanitized, "Authorization: "+redactedValue) {
+		t.Fatalf("expected authorization header redacted")
+	}
+	if !containsLine(sanitized, "X-Api-Key: "+redactedValue) {
+		t.Fatalf("expected api key header redacted")
+	}
+	if !containsLine(sanitized, "Content-Type: text/plain") {
+		t.Fatalf("expected content-type preserved")
+	}
+	if !containsLine(sanitized, "client_secret="+redactedValue+"&grant_type=password") {
+		t.Fatalf("expected form secret redacted")
+	}
+}


containsLine uses plain strings.Contains, not an actual line-boundary check. The name is misleading, a match inside a longer string would pass. Fine for these tests but worth renaming to containsSubstr or similar for clarity.

Priyanshubhartistm · 2026-08-14T10:35:52Z

@Vaishnav88sk ,sure, I will resolve the merge conflicts as soon as possible

Caesarsage · 2026-08-16T19:42:26Z

Fixed on master — verbose dumps now pass through redaction that masks the Authorization header and token query params (access_token/refresh_token/id_token/code). One residual nit: client_secret= isn't in the param regex yet, worth a tiny follow-up PR, but the leak reported here is closed.

I believe this can be closed (happy to review ones you file the small follow-up from updated upstream master).

Priyanshubhartistm requested review from Harsh4902, lbroudoux and yada as code owners May 8, 2026 17:50

Priyanshubhartistm force-pushed the fix/redact-sensitive-auth-logs branch from d9a0239 to 572d7d0 Compare May 8, 2026 18:21

Priyanshubhartistm added 3 commits June 6, 2026 14:46

Priyanshubhartistm force-pushed the fix/redact-sensitive-auth-logs branch from 572d7d0 to 453b88e Compare June 6, 2026 09:18

Priyanshubhartistm temporarily deployed to Build June 6, 2026 09:18 — with GitHub Actions Inactive

Vaishnav88sk suggested changes Aug 11, 2026

View reviewed changes

Vaishnav88sk mentioned this pull request Aug 11, 2026

feat: add global --verbose flag for HTTP debugging #370

Closed

Caesarsage mentioned this pull request Aug 17, 2026

fix(watcher): skip context when Microcks client cannot be created #482

Open

Navigation Menu

Uh oh!

Uh oh!

Fix/redact sensitive auth logs - #333

Fix/redact sensitive auth logs#333
Priyanshubhartistm wants to merge 3 commits into
microcks:masterfrom
Priyanshubhartistm:fix/redact-sensitive-auth-logs

Priyanshubhartistm commented May 8, 2026

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

Priyanshubhartistm commented May 8, 2026 •

edited

Loading

Uh oh!

Vaishnav88sk left a comment

Uh oh!

Vaishnav88sk Aug 11, 2026

Uh oh!

Vaishnav88sk Aug 11, 2026

Uh oh!

Vaishnav88sk Aug 11, 2026

Uh oh!

Vaishnav88sk Aug 11, 2026

Uh oh!

Priyanshubhartistm commented Aug 14, 2026

Uh oh!

Caesarsage commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		log.Printf("Token: %s\n", utils.MaskSecret(tokenString))
		log.Printf("Refresh Token: %s\n", utils.MaskSecret(refreshToken))

Uh oh!

Uh oh!

Conversation

Priyanshubhartistm commented May 8, 2026

Description

Test results

Related issue(s)

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

Priyanshubhartistm commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vaishnav88sk left a comment

Choose a reason for hiding this comment

Uh oh!

Vaishnav88sk Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Vaishnav88sk Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Vaishnav88sk Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Vaishnav88sk Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Priyanshubhartistm commented Aug 14, 2026

Uh oh!

Caesarsage commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Priyanshubhartistm commented May 8, 2026 •

edited

Loading