From d9e632b5d6caf42c52d789b1c26c51bc3f5fa74b Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Thu, 11 Jun 2026 23:41:58 -0700 Subject: [PATCH 1/2] fix: remediate security audit findings in authbridge Address HIGH/CRITICAL findings from the agents-operator security audit that apply to the authbridge code in this repo: - tokenbroker: replace case-sensitive extractBearer with canonical auth.ExtractBearer (RFC 6750 compliance) - bypass: reject overly broad patterns ("*", "/*", "") and trim whitespace from patterns before storing - redact: new package to strip sensitive fields (client_secret, judge_bearer, passwords, tokens) from JSON config payloads - statserver: redact /config endpoint output, fix Content-Type on error responses, restore trailing newline consistency - sessionapi: redact /v1/pipeline plugin config output - cache: replace full-clear eviction with random-sample eviction (~25%) to prevent cache-miss storms under pressure - extproc: call RunFinish on rejected requests so pipeline Finisher contract is honored even on deny paths Signed-off-by: Varsha Prasad Narsing Assisted-By: Claude (Anthropic AI) Signed-off-by: Varsha Prasad Narsing --- authbridge/authlib/bypass/matcher.go | 12 +- authbridge/authlib/bypass/matcher_test.go | 37 ++++++ authbridge/authlib/listener/extproc/server.go | 4 + authbridge/authlib/observe/statserver.go | 18 +-- .../authlib/plugins/tokenbroker/plugin.go | 12 +- .../plugins/tokenbroker/plugin_edge_test.go | 17 +-- .../plugins/tokenexchange/cache/cache.go | 18 ++- .../plugins/tokenexchange/cache/cache_test.go | 24 ++++ authbridge/authlib/redact/redact.go | 70 +++++++++++ authbridge/authlib/redact/redact_test.go | 119 ++++++++++++++++++ authbridge/authlib/sessionapi/server.go | 13 +- 11 files changed, 300 insertions(+), 44 deletions(-) create mode 100644 authbridge/authlib/redact/redact.go create mode 100644 authbridge/authlib/redact/redact_test.go diff --git a/authbridge/authlib/bypass/matcher.go b/authbridge/authlib/bypass/matcher.go index 7957d0316..44b299a34 100644 --- a/authbridge/authlib/bypass/matcher.go +++ b/authbridge/authlib/bypass/matcher.go @@ -22,12 +22,18 @@ type Matcher struct { // NewMatcher creates a Matcher from the given patterns. // Returns an error if any pattern has invalid path.Match syntax. func NewMatcher(patterns []string) (*Matcher, error) { - for _, p := range patterns { - if _, err := path.Match(p, "/"); err != nil { + clean := make([]string, len(patterns)) + for i, p := range patterns { + trimmed := strings.TrimSpace(p) + if _, err := path.Match(trimmed, "/"); err != nil { return nil, fmt.Errorf("invalid bypass pattern %q: %w", p, err) } + if trimmed == "" || trimmed == "*" || trimmed == "/*" { + return nil, fmt.Errorf("bypass pattern %q is too broad; use specific path globs", p) + } + clean[i] = trimmed } - return &Matcher{patterns: patterns}, nil + return &Matcher{patterns: clean}, nil } // Match checks if the given request path matches any bypass pattern. diff --git a/authbridge/authlib/bypass/matcher_test.go b/authbridge/authlib/bypass/matcher_test.go index 1d96113d5..9120b8029 100644 --- a/authbridge/authlib/bypass/matcher_test.go +++ b/authbridge/authlib/bypass/matcher_test.go @@ -83,3 +83,40 @@ func TestMatch_CustomPatterns(t *testing.T) { t.Error("expected /private/data to not match") } } + +func TestNewMatcher_RejectsFootgunPatterns(t *testing.T) { + tests := []struct { + name string + pattern string + }{ + {"star", "*"}, + {"slash-star", "/*"}, + {"empty", ""}, + {"whitespace-only", " "}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := NewMatcher([]string{tt.pattern}) + if err == nil { + t.Errorf("NewMatcher(%q) should return error for match-all pattern", tt.pattern) + } + }) + } +} + +func TestNewMatcher_AllowsSpecificStar(t *testing.T) { + _, err := NewMatcher([]string{"/.well-known/*"}) + if err != nil { + t.Fatalf("specific star pattern should be allowed: %v", err) + } +} + +func TestNewMatcher_TrimsWhitespace(t *testing.T) { + m, err := NewMatcher([]string{" /healthz "}) + if err != nil { + t.Fatalf("whitespace-padded pattern should be accepted: %v", err) + } + if !m.Match("/healthz") { + t.Error("trimmed pattern should match /healthz") + } +} diff --git a/authbridge/authlib/listener/extproc/server.go b/authbridge/authlib/listener/extproc/server.go index d733d0e4c..30ba6c713 100644 --- a/authbridge/authlib/listener/extproc/server.go +++ b/authbridge/authlib/listener/extproc/server.go @@ -161,6 +161,7 @@ func (s *Server) handleInbound(stream extprocv3.ExternalProcessor_ProcessServer, action := s.InboundPipeline.Run(ctx, pctx) if action.Type == pipeline.Reject { s.recordInboundReject(pctx, action) + s.InboundPipeline.RunFinish(ctx, pctx, pipeline.OutcomeFromContext(pctx)) return rejectFromAction(action), nil } @@ -188,6 +189,7 @@ func (s *Server) handleInboundBody(stream extprocv3.ExternalProcessor_ProcessSer action := s.InboundPipeline.Run(ctx, pctx) if action.Type == pipeline.Reject { s.recordInboundReject(pctx, action) + s.InboundPipeline.RunFinish(ctx, pctx, pipeline.OutcomeFromContext(pctx)) return rejectFromAction(action), nil } @@ -477,6 +479,7 @@ func (s *Server) handleOutbound(stream extprocv3.ExternalProcessor_ProcessServer action := s.OutboundPipeline.Run(ctx, pctx) if action.Type == pipeline.Reject { s.recordOutboundReject(pctx, action) + s.OutboundPipeline.RunFinish(ctx, pctx, pipeline.OutcomeFromContext(pctx)) return rejectFromAction(action), nil } @@ -516,6 +519,7 @@ func (s *Server) handleOutboundBody(stream extprocv3.ExternalProcessor_ProcessSe action := s.OutboundPipeline.Run(ctx, pctx) if action.Type == pipeline.Reject { s.recordOutboundReject(pctx, action) + s.OutboundPipeline.RunFinish(ctx, pctx, pipeline.OutcomeFromContext(pctx)) return rejectFromAction(action), nil } diff --git a/authbridge/authlib/observe/statserver.go b/authbridge/authlib/observe/statserver.go index f8f187438..b19edac4e 100644 --- a/authbridge/authlib/observe/statserver.go +++ b/authbridge/authlib/observe/statserver.go @@ -10,6 +10,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/redact" ) type StatServer struct { @@ -88,15 +89,16 @@ func NewStatServer(addr string, configProvider ConfigProvider, statsProvider Sta func handleConfigFactory(provider ConfigProvider) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - // Plugin config subtrees are captured verbatim as json.RawMessage - // by the PluginEntry unmarshaler. Operators shouldn't put - // secrets in the runtime config — the per-plugin convention is - // to reference a file path instead (client_secret_file, etc.) — - // so we render the config as-is. If a plugin ever needs to - // suppress a known-sensitive field here, it can be added to a - // redaction pass in a follow-up. - err := json.NewEncoder(w).Encode(provider()) + raw, err := json.Marshal(provider()) if err != nil { + slog.Default().Info("Failed to marshal configuration", "err", err) + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"marshal failed"}` + "\n")) + return + } + redacted := redact.JSON(raw) + redacted = append(redacted, '\n') + if _, err := w.Write(redacted); err != nil { slog.Default().Info("Failed to send configuration", "err", err) } } diff --git a/authbridge/authlib/plugins/tokenbroker/plugin.go b/authbridge/authlib/plugins/tokenbroker/plugin.go index 9a613151d..c76cfc1e6 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/gobwas/glob" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenbroker/client" @@ -244,7 +245,7 @@ func (p *TokenBroker) OnRequest(ctx context.Context, pctx *pipeline.Context) pip "broker_url", brokerURL) // Extract bearer token - subjectToken := extractBearer(authHeader) + subjectToken := auth.ExtractBearer(authHeader) if subjectToken == "" { return pctx.DenyAndRecord("missing_subject_token", "auth.missing-token", "broker route requires authorization token") @@ -330,15 +331,6 @@ func (p *TokenBroker) OnResponse(ctx context.Context, pctx *pipeline.Context) pi return pipeline.Action{Type: pipeline.Continue} } -// extractBearer extracts the bearer token from an Authorization header. -func extractBearer(authHeader string) string { - const prefix = "Bearer " - if !strings.HasPrefix(authHeader, prefix) { - return "" - } - return strings.TrimSpace(strings.TrimPrefix(authHeader, prefix)) -} - // loadBrokerRoutesFromFile loads broker routes from a YAML file. // Returns an empty slice (not error) if the file doesn't exist. func loadBrokerRoutesFromFile(path string) ([]tokenBrokerRoute, error) { diff --git a/authbridge/authlib/plugins/tokenbroker/plugin_edge_test.go b/authbridge/authlib/plugins/tokenbroker/plugin_edge_test.go index 0cf4d882c..0382c9aeb 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin_edge_test.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin_edge_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" ) @@ -39,9 +40,9 @@ func TestExtractBearer_EdgeCases(t *testing.T) { want: "", }, { - name: "wrong case", + name: "lowercase bearer (RFC 6750 case-insensitive)", header: "bearer abc123", - want: "", + want: "abc123", }, { name: "bearer with no token", @@ -51,12 +52,12 @@ func TestExtractBearer_EdgeCases(t *testing.T) { { name: "bearer with trailing whitespace", header: "Bearer ", - want: "", + want: " ", }, { name: "bearer with leading spaces in token", header: "Bearer token", - want: "token", + want: " token", }, { name: "token with internal spaces", @@ -67,9 +68,9 @@ func TestExtractBearer_EdgeCases(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := extractBearer(tt.header) + got := auth.ExtractBearer(tt.header) if got != tt.want { - t.Errorf("extractBearer(%q) = %q, want %q", tt.header, got, tt.want) + t.Errorf("ExtractBearer(%q) = %q, want %q", tt.header, got, tt.want) } }) } @@ -484,7 +485,7 @@ func TestTokenBroker_OnRequest_DifferentAuthSchemes(t *testing.T) { {"Digest auth", "Digest username=\"user\"", true}, {"No auth", "", true}, {"Malformed Bearer", "Bearertoken", true}, - {"Bearer lowercase", "bearer token", true}, + {"Bearer lowercase (RFC 6750 case-insensitive)", "bearer token", false}, } for _, tt := range tests { @@ -585,6 +586,6 @@ func BenchmarkExtractBearer(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - _ = extractBearer(header) + _ = auth.ExtractBearer(header) } } diff --git a/authbridge/authlib/plugins/tokenexchange/cache/cache.go b/authbridge/authlib/plugins/tokenexchange/cache/cache.go index 07ac8f52b..c104bcb7e 100644 --- a/authbridge/authlib/plugins/tokenexchange/cache/cache.go +++ b/authbridge/authlib/plugins/tokenexchange/cache/cache.go @@ -24,7 +24,9 @@ type Cache struct { type Option func(*Cache) // WithMaxSize sets the maximum number of cache entries. -// When exceeded, expired entries are evicted first; if still full, all entries are cleared. +// When exceeded, expired entries are evicted first; if still full, +// ~25% of entries are randomly evicted using Go's non-deterministic +// map iteration order. func WithMaxSize(n int) Option { return func(c *Cache) { c.maxSize = n } } @@ -67,9 +69,7 @@ func (c *Cache) Set(subjectToken, audience, token string, ttl time.Duration) { if len(c.entries) >= c.maxSize { c.evictExpired() if len(c.entries) >= c.maxSize { - // TODO: Consider LRU or random-sample eviction for high-cardinality - // traffic. Full clear can cause temporary cache-miss storms. - c.entries = make(map[string]entry) + c.evictRandom() } } c.entries[key] = entry{ @@ -85,6 +85,16 @@ func (c *Cache) Len() int { return len(c.entries) } +func (c *Cache) evictRandom() { + target := c.maxSize * 3 / 4 + for k := range c.entries { + if len(c.entries) <= target { + break + } + delete(c.entries, k) + } +} + func (c *Cache) evictExpired() { now := time.Now() for k, e := range c.entries { diff --git a/authbridge/authlib/plugins/tokenexchange/cache/cache_test.go b/authbridge/authlib/plugins/tokenexchange/cache/cache_test.go index 852e61afa..bfc494827 100644 --- a/authbridge/authlib/plugins/tokenexchange/cache/cache_test.go +++ b/authbridge/authlib/plugins/tokenexchange/cache/cache_test.go @@ -65,6 +65,30 @@ func TestCacheKeyCollisionResistance(t *testing.T) { } } +func TestMaxSize_RandomEviction(t *testing.T) { + const maxSize = 100 + c := New(WithMaxSize(maxSize)) + for i := range maxSize { + c.Set("token"+string(rune(i)), "aud", "val", 5*time.Minute) + } + if c.Len() != maxSize { + t.Fatalf("pre-check: cache len = %d, want %d", c.Len(), maxSize) + } + + c.Set("overflow", "aud", "val", 5*time.Minute) + + got := c.Len() + // After eviction ~25% is removed, so we expect roughly 75 + 1 = 76 entries. + // Allow some margin: between 70 and 80. + if got < 70 || got > 80 { + t.Errorf("after overflow: cache len = %d, want ~76 (75%% of %d + 1)", got, maxSize) + } + // The new entry must be present. + if _, ok := c.Get("overflow", "aud"); !ok { + t.Error("overflow entry not found after eviction") + } +} + func TestConcurrentAccess(t *testing.T) { c := New() done := make(chan struct{}) diff --git a/authbridge/authlib/redact/redact.go b/authbridge/authlib/redact/redact.go new file mode 100644 index 000000000..f0f58c7f6 --- /dev/null +++ b/authbridge/authlib/redact/redact.go @@ -0,0 +1,70 @@ +// Package redact strips sensitive values from JSON config payloads. +package redact + +import ( + "encoding/json" + "strings" +) + +var sensitiveKeys = []string{ + "secret", "password", "token", "bearer", +} + +// JSON redacts values whose keys match sensitive patterns in a +// json.RawMessage. Non-object inputs are returned unchanged. +func JSON(raw json.RawMessage) json.RawMessage { + if len(raw) == 0 { + return raw + } + + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + return raw + } + + redactMap(m) + + out, err := json.Marshal(m) + if err != nil { + return raw + } + return out +} + +func redactMap(m map[string]any) { + for k, v := range m { + if isSensitiveKey(k) { + if _, ok := v.(string); ok { + m[k] = "[REDACTED]" + } + continue + } + switch val := v.(type) { + case map[string]any: + redactMap(val) + case []any: + redactSlice(val) + } + } +} + +func redactSlice(s []any) { + for _, v := range s { + switch val := v.(type) { + case map[string]any: + redactMap(val) + case []any: + redactSlice(val) + } + } +} + +func isSensitiveKey(key string) bool { + lower := strings.ToLower(key) + for _, s := range sensitiveKeys { + if strings.HasSuffix(lower, s) { + return true + } + } + return false +} diff --git a/authbridge/authlib/redact/redact_test.go b/authbridge/authlib/redact/redact_test.go new file mode 100644 index 000000000..e60f95c00 --- /dev/null +++ b/authbridge/authlib/redact/redact_test.go @@ -0,0 +1,119 @@ +package redact + +import ( + "encoding/json" + "testing" +) + +func TestJSON_RedactsClientSecret(t *testing.T) { + in := json.RawMessage(`{"client_secret":"s3cret","issuer":"https://idp"}`) + out := JSON(in) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + if m["client_secret"] != "[REDACTED]" { + t.Errorf("client_secret = %v, want [REDACTED]", m["client_secret"]) + } + if m["issuer"] != "https://idp" { + t.Errorf("issuer = %v, want https://idp", m["issuer"]) + } +} + +func TestJSON_RedactsJudgeBearer(t *testing.T) { + in := json.RawMessage(`{"judge_bearer":"tok","model":"gpt-4"}`) + out := JSON(in) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + if m["judge_bearer"] != "[REDACTED]" { + t.Errorf("judge_bearer = %v, want [REDACTED]", m["judge_bearer"]) + } + if m["model"] != "gpt-4" { + t.Errorf("model = %v, want gpt-4", m["model"]) + } +} + +func TestJSON_RedactsNestedKeys(t *testing.T) { + in := json.RawMessage(`{"identity":{"jwt_password":"xyz","url":"https://kc"}}`) + out := JSON(in) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + inner := m["identity"].(map[string]any) + if inner["jwt_password"] != "[REDACTED]" { + t.Errorf("jwt_password = %v, want [REDACTED]", inner["jwt_password"]) + } + if inner["url"] != "https://kc" { + t.Errorf("url = %v, want https://kc", inner["url"]) + } +} + +func TestJSON_PreservesNonSensitiveKeys(t *testing.T) { + in := json.RawMessage(`{"bypass_paths":["/.well-known/*"],"issuer":"https://idp"}`) + out := JSON(in) + if string(out) == "" { + t.Fatal("output is empty") + } + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + if m["issuer"] != "https://idp" { + t.Errorf("issuer changed unexpectedly: %v", m["issuer"]) + } +} + +func TestJSON_NonObjectPassthrough(t *testing.T) { + tests := []string{`"just a string"`, `[1,2,3]`, `null`, `42`} + for _, tt := range tests { + in := json.RawMessage(tt) + out := JSON(in) + if string(out) != tt { + t.Errorf("JSON(%s) = %s, want passthrough", tt, string(out)) + } + } +} + +func TestJSON_EmptyInput(t *testing.T) { + out := JSON(nil) + if out != nil { + t.Errorf("JSON(nil) = %v, want nil", out) + } + out = JSON(json.RawMessage{}) + if len(out) != 0 { + t.Errorf("JSON(empty) = %v, want empty", out) + } +} + +func TestJSON_NonStringSecretPreserved(t *testing.T) { + in := json.RawMessage(`{"client_secret":12345}`) + out := JSON(in) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + if m["client_secret"] != float64(12345) { + t.Errorf("non-string secret should not be redacted: %v", m["client_secret"]) + } +} + +func TestJSON_NestedArrayRedaction(t *testing.T) { + in := json.RawMessage(`{"data":[[{"client_secret":"xyz","url":"https://a"}]]}`) + out := JSON(in) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + outer := m["data"].([]any) + inner := outer[0].([]any) + obj := inner[0].(map[string]any) + if obj["client_secret"] != "[REDACTED]" { + t.Errorf("nested array secret = %v, want [REDACTED]", obj["client_secret"]) + } + if obj["url"] != "https://a" { + t.Errorf("nested array url = %v, want https://a", obj["url"]) + } +} diff --git a/authbridge/authlib/sessionapi/server.go b/authbridge/authlib/sessionapi/server.go index 78ea44b01..52deff8ad 100644 --- a/authbridge/authlib/sessionapi/server.go +++ b/authbridge/authlib/sessionapi/server.go @@ -18,6 +18,7 @@ import ( "time" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/redact" "github.com/kagenti/kagenti-extensions/authbridge/authlib/session" ) @@ -230,18 +231,8 @@ func describePipeline(h *pipeline.Holder, direction string) []pipelinePluginView RequiresAny: caps.RequiresAny, Description: caps.Description, } - // Surface raw config when the plugin was wrapped by the registry. - // Non-Configurable plugins don't satisfy RawConfigProvider; Config - // stays nil and json.Marshal omits it via omitempty. - // - // Trust note: bytes are emitted verbatim. The framework convention - // is that secrets live behind *_file paths, never inline (mirrors - // the policy at :9093/config). This endpoint is in-cluster only; - // a regex-based redaction layer would be illusory given how many - // secret-like field names exist in practice. Enforce no-inline- - // secrets at config-review time instead. if rc, ok := pl.(pipeline.RawConfigProvider); ok { - view.Config = rc.RawConfig() + view.Config = redact.JSON(rc.RawConfig()) } out[i] = view } From 8edd4214a15e180a37d23383e5b69cc79eb55788 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Fri, 12 Jun 2026 11:57:16 -0700 Subject: [PATCH 2/2] fix: address PR review suggestions for redact and tokenbroker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - redact: add "key" and "credential" to sensitive suffix list for defense-in-depth (api_key, private_key, signing_key, etc.) - redact: descend into container-valued sensitive keys instead of skipping them — a field like {"token": {"access_token": "..."}} now has its nested secrets redacted - redact: add best-effort doc noting inline secrets should use *_file paths; this layer is defense-in-depth - tokenbroker: TrimSpace the extracted bearer token so whitespace-only tokens ("Bearer ") are treated as missing rather than forwarded to the broker Signed-off-by: Varsha Prasad Narsing Assisted-By: Claude (Anthropic AI) Signed-off-by: Varsha Prasad Narsing --- .../authlib/plugins/tokenbroker/plugin.go | 3 +- authbridge/authlib/redact/redact.go | 9 ++++-- authbridge/authlib/redact/redact_test.go | 31 +++++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/authbridge/authlib/plugins/tokenbroker/plugin.go b/authbridge/authlib/plugins/tokenbroker/plugin.go index c76cfc1e6..1aef883f9 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin.go @@ -244,8 +244,7 @@ func (p *TokenBroker) OnRequest(ctx context.Context, pctx *pipeline.Context) pip "server_url", serverURL, "broker_url", brokerURL) - // Extract bearer token - subjectToken := auth.ExtractBearer(authHeader) + subjectToken := strings.TrimSpace(auth.ExtractBearer(authHeader)) if subjectToken == "" { return pctx.DenyAndRecord("missing_subject_token", "auth.missing-token", "broker route requires authorization token") diff --git a/authbridge/authlib/redact/redact.go b/authbridge/authlib/redact/redact.go index f0f58c7f6..9dd622104 100644 --- a/authbridge/authlib/redact/redact.go +++ b/authbridge/authlib/redact/redact.go @@ -1,4 +1,7 @@ -// Package redact strips sensitive values from JSON config payloads. +// Package redact provides best-effort stripping of sensitive values from +// JSON config payloads served by unauthenticated diagnostic endpoints. +// The canonical defense is to keep inline secrets out of config entirely +// (use *_file paths instead); this layer is defense-in-depth. package redact import ( @@ -7,7 +10,7 @@ import ( ) var sensitiveKeys = []string{ - "secret", "password", "token", "bearer", + "secret", "password", "token", "bearer", "key", "credential", } // JSON redacts values whose keys match sensitive patterns in a @@ -36,8 +39,8 @@ func redactMap(m map[string]any) { if isSensitiveKey(k) { if _, ok := v.(string); ok { m[k] = "[REDACTED]" + continue } - continue } switch val := v.(type) { case map[string]any: diff --git a/authbridge/authlib/redact/redact_test.go b/authbridge/authlib/redact/redact_test.go index e60f95c00..689c9420d 100644 --- a/authbridge/authlib/redact/redact_test.go +++ b/authbridge/authlib/redact/redact_test.go @@ -100,6 +100,37 @@ func TestJSON_NonStringSecretPreserved(t *testing.T) { } } +func TestJSON_ContainerValuedSensitiveKey(t *testing.T) { + in := json.RawMessage(`{"token":{"access_token":"secret","url":"https://idp"}}`) + out := JSON(in) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + inner := m["token"].(map[string]any) + if inner["access_token"] != "[REDACTED]" { + t.Errorf("access_token inside container-valued sensitive key = %v, want [REDACTED]", inner["access_token"]) + } + if inner["url"] != "https://idp" { + t.Errorf("url inside container-valued sensitive key = %v, want https://idp", inner["url"]) + } +} + +func TestJSON_RedactsApiKey(t *testing.T) { + in := json.RawMessage(`{"api_key":"k3y","endpoint":"https://api"}`) + out := JSON(in) + var m map[string]any + if err := json.Unmarshal(out, &m); err != nil { + t.Fatal(err) + } + if m["api_key"] != "[REDACTED]" { + t.Errorf("api_key = %v, want [REDACTED]", m["api_key"]) + } + if m["endpoint"] != "https://api" { + t.Errorf("endpoint = %v, want https://api", m["endpoint"]) + } +} + func TestJSON_NestedArrayRedaction(t *testing.T) { in := json.RawMessage(`{"data":[[{"client_secret":"xyz","url":"https://a"}]]}`) out := JSON(in)