From 95bda3ee3b0623b131dbcc241ebea7f7ad15f55a Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Wed, 6 May 2026 10:45:29 +0100 Subject: [PATCH] fix: redact sensitive headers in httplog + restore headless logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 #1 — HTTP traffic logger captured Authorization, Cookie, Set-Cookie, X-Api-Key, X-Auth-Token, X-Csrf-Token, Proxy-Authorization, X-Access-Token verbatim into log entries (file 0600 + UI subscribers). Bearer tokens and session cookies were ending up on disk whenever httpLog.includeHeaders was enabled. flattenHeaders now redacts: - the explicit list above (case-insensitive via http.CanonicalHeaderKey) - any header name containing 'token', 'secret', 'password', 'apikey' Header names remain visible; values become [REDACTED]. Redaction is unconditional and on-by-default — no opt-out flag. Users who want raw headers can use tcpdump. P0 #6 — Headless mode without -v silently routed both structured and stdlib logs to io.Discard. A daemon under launchd/systemd had no way to report errors. Headless now defaults log destination to os.Stderr; -v controls only the level (debug vs info). TUI-quiet path is preserved. Tests in internal/httplog/redact_test.go cover all explicit names, substring patterns, and case variants. --- cmd/kportal/main.go | 39 +++++++++--- internal/httplog/proxy.go | 63 +++++++++++++++++++- internal/httplog/redact_test.go | 102 ++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 9 deletions(-) create mode 100644 internal/httplog/redact_test.go diff --git a/cmd/kportal/main.go b/cmd/kportal/main.go index d576603..f1760bd 100644 --- a/cmd/kportal/main.go +++ b/cmd/kportal/main.go @@ -102,16 +102,33 @@ func main() { } // Initialize structured logger + // + // Output destination depends on run mode, NOT on the -v flag: + // - headless mode: always stderr, so daemons under launchd/systemd/journald + // can surface startup and runtime errors. Without this the daemon would + // fail silently and operators have no way to diagnose it. + // - TUI (interactive) mode: stderr would corrupt the bubbletea UI, so we + // route to io.Discard. Verbose TUI is not supported here either; -v in + // interactive mode upgrades to the simple table UI further down. + // + // The -v flag only controls log *level* (debug vs info), never destination. var logLevel logger.Level var logFmt logger.Format var logOutput io.Writer if *verbose { logLevel = logger.LevelDebug - logOutput = os.Stderr } else { logLevel = logger.LevelInfo - logOutput = io.Discard // Silence logger in non-verbose/headless mode to prevent UI corruption + } + + if *headless || *verbose { + // Headless daemons must always emit logs so operators can see failures. + // Verbose mode (with or without TUI) also goes to stderr. + logOutput = os.Stderr + } else { + // Interactive TUI mode: silence logger to avoid corrupting bubbletea UI. + logOutput = io.Discard } switch *logFormat { @@ -179,14 +196,22 @@ func main() { os.Exit(0) } - if !*verbose { - // In interactive mode, disable ALL logging to avoid interfering with bubbletea UI + // Configure stdlib log destination using the same rule as the structured + // logger: only the bubbletea TUI path needs total silence. Headless mode + // keeps stderr so daemonised runs surface errors to journald/launchd. + switch { + case *verbose: + // Verbose mode - enable standard log formatting on stderr (default) + log.SetFlags(log.LstdFlags | log.Lshortfile) + case *headless: + // Headless mode without -v: keep stderr (default writer) but use plain + // timestamps so journald-style log collectors show readable lines. + log.SetFlags(log.LstdFlags) + default: + // Interactive bubbletea mode: silence stdlib log to avoid UI corruption. log.SetOutput(io.Discard) log.SetPrefix("") log.SetFlags(0) - } else { - // Verbose mode - enable standard log formatting - log.SetFlags(log.LstdFlags | log.Lshortfile) } // Load configuration diff --git a/internal/httplog/proxy.go b/internal/httplog/proxy.go index cebfc9f..a231534 100644 --- a/internal/httplog/proxy.go +++ b/internal/httplog/proxy.go @@ -339,11 +339,70 @@ func (p *Proxy) logError(req *http.Request, err error) { _ = p.logger.Log(entry) } -// flattenHeaders converts http.Header to map[string]string. -// Pre-allocates the map with the exact size needed to avoid reallocations. +// redactedHeaderNames is the set of header names whose values are always +// redacted before being captured into log entries. Comparison is +// case-insensitive (canonical MIME header form is used as the key). +// +// Redaction is unconditional and on-by-default as a defense-in-depth measure: +// these headers commonly carry bearer tokens, session cookies, API keys, or +// other credentials that must never be persisted to disk or surfaced to the +// UI. Users who genuinely need raw header capture should use a dedicated +// packet-capture tool (e.g. tcpdump) instead. +var redactedHeaderNames = map[string]struct{}{ + "Authorization": {}, + "Proxy-Authorization": {}, + "Cookie": {}, + "Set-Cookie": {}, + "X-Api-Key": {}, + "X-Auth-Token": {}, + "X-Csrf-Token": {}, + "X-Access-Token": {}, +} + +// redactedHeaderSubstrings is a list of lowercase substrings that, when +// found anywhere in a header name (case-insensitive), trigger redaction. +// This catches custom or vendor-specific sensitive headers without needing +// to enumerate every variant. +var redactedHeaderSubstrings = []string{ + "token", + "secret", + "password", + "apikey", +} + +// redactedValue is the placeholder written in place of any sensitive header +// value. The header name itself is preserved so operators can see which +// sensitive headers were present without leaking their contents. +const redactedValue = "[REDACTED]" + +// shouldRedactHeader reports whether the given header name should have its +// value redacted before being recorded. The check is case-insensitive and +// covers both the explicit name list and the substring patterns. +func shouldRedactHeader(name string) bool { + if _, ok := redactedHeaderNames[http.CanonicalHeaderKey(name)]; ok { + return true + } + lower := strings.ToLower(name) + for _, sub := range redactedHeaderSubstrings { + if strings.Contains(lower, sub) { + return true + } + } + return false +} + +// flattenHeaders converts http.Header to map[string]string, redacting the +// values of any sensitive headers (see redactedHeaderNames / +// redactedHeaderSubstrings) so that credentials never reach the log file or +// UI subscribers. Pre-allocates the map with the exact size needed to avoid +// reallocations. func flattenHeaders(h http.Header) map[string]string { result := make(map[string]string, len(h)) for k, v := range h { + if shouldRedactHeader(k) { + result[k] = redactedValue + continue + } result[k] = strings.Join(v, ", ") } return result diff --git a/internal/httplog/redact_test.go b/internal/httplog/redact_test.go new file mode 100644 index 0000000..5de42e3 --- /dev/null +++ b/internal/httplog/redact_test.go @@ -0,0 +1,102 @@ +package httplog + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestFlattenHeaders_RedactsSensitive verifies that flattenHeaders replaces +// the values of known sensitive headers with the [REDACTED] placeholder while +// preserving the header name and leaving benign headers untouched. Covers +// the explicit name list, case-insensitive matching, and the substring-based +// fallback patterns ("token", "secret", "password", "apikey"). +func TestFlattenHeaders_RedactsSensitive(t *testing.T) { + h := http.Header{ + // Explicit list (canonical casing) + "Authorization": []string{"Bearer eyJhbGciOiJIUzI1NiJ9.payload.sig"}, + "Proxy-Authorization": []string{"Basic dXNlcjpwYXNz"}, + "Cookie": []string{"session=abc123; csrf=xyz"}, + "Set-Cookie": []string{"session=abc123; HttpOnly"}, + "X-Api-Key": []string{"sk_live_deadbeef"}, + "X-Auth-Token": []string{"tok_supersecret"}, + "X-Csrf-Token": []string{"csrf_random_value"}, + "X-Access-Token": []string{"at_anothersecret"}, + + // Substring matches (case-insensitive) + "X-Refresh-Token": []string{"rt_value"}, + "My-Secret-Header": []string{"shh"}, + "X-User-Password": []string{"hunter2"}, + "X-Custom-Apikey": []string{"key_value"}, + + // Benign headers must be preserved verbatim + "Content-Type": []string{"application/json"}, + "Accept": []string{"text/html", "application/json"}, + "User-Agent": []string{"kportal-test/1.0"}, + } + + result := flattenHeaders(h) + + redactedHeaders := []string{ + "Authorization", + "Proxy-Authorization", + "Cookie", + "Set-Cookie", + "X-Api-Key", + "X-Auth-Token", + "X-Csrf-Token", + "X-Access-Token", + "X-Refresh-Token", + "My-Secret-Header", + "X-User-Password", + "X-Custom-Apikey", + } + for _, name := range redactedHeaders { + got, ok := result[name] + assert.Truef(t, ok, "expected redacted header %q to remain present in output", name) + assert.Equalf(t, "[REDACTED]", got, "expected header %q value to be redacted", name) + } + + // Benign headers should be untouched. + assert.Equal(t, "application/json", result["Content-Type"]) + assert.Equal(t, "text/html, application/json", result["Accept"]) + assert.Equal(t, "kportal-test/1.0", result["User-Agent"]) + + // And no benign value should leak the redaction marker (sanity check). + for _, name := range []string{"Content-Type", "Accept", "User-Agent"} { + assert.NotEqualf(t, "[REDACTED]", result[name], "benign header %q must not be redacted", name) + } +} + +// TestShouldRedactHeader_CaseInsensitive verifies that the case-insensitive +// match logic catches lowercased / mixed-case variants of the redaction list. +func TestShouldRedactHeader_CaseInsensitive(t *testing.T) { + cases := []struct { + name string + want bool + }{ + {"authorization", true}, + {"AUTHORIZATION", true}, + {"AuThOrIzAtIoN", true}, + {"cookie", true}, + {"set-cookie", true}, + {"x-api-key", true}, + {"X-CUSTOM-TOKEN", true}, + {"x-app-Secret", true}, + {"My_Password_Header", true}, + {"x-vendor-APIKEY", true}, + + // Non-sensitive + {"Content-Type", false}, + {"Accept", false}, + {"User-Agent", false}, + {"X-Request-Id", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, shouldRedactHeader(tc.name)) + }) + } +}