From d92ab6ea4e4cbc35213d667d72abcb781758a0fc Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Sat, 30 May 2026 23:43:09 +0000 Subject: [PATCH] fix: redact secrets in DLQ read API (PILOT-314) HandleGetWebhookDLQ previously returned event Details verbatim. If the audit-level redaction (redactKey) missed a secret field, the DLQ became a credential-disclosure surface for any caller holding the admin token. - Export RedactMap in audit package (applies same redactKey rules) - Apply audit.RedactMap to DLQ entry details on retrieval - Add tests for RedactMap and DLQ redaction path --- audit/audit.go | 23 ++++++++++++++++ audit/zz_redaction_test.go | 45 +++++++++++++++++++++++++++++++ webhook/webhook.go | 6 ++++- webhook/zz_webhook_test.go | 54 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/audit/audit.go b/audit/audit.go index eeba718..4e65eb1 100644 --- a/audit/audit.go +++ b/audit/audit.go @@ -324,3 +324,26 @@ func redactKey(k string) bool { } return false } + +// RedactMap returns a shallow copy of m with any values whose keys +// match redactKey replaced by "". Nil-safe (returns nil). +// +// PILOT-314: the DLQ read API (HandleGetWebhookDLQ) returns event +// Details verbatim. If audit redaction wasn't exhaustive, the DLQ +// becomes a credential-disclosure surface for anyone holding the +// admin token. RedactMap allows callers outside package audit to +// apply the same redaction rules on retrieval. +func RedactMap(m map[string]interface{}) map[string]interface{} { + if m == nil { + return nil + } + out := make(map[string]interface{}, len(m)) + for k, v := range m { + if redactKey(k) { + out[k] = "" + } else { + out[k] = v + } + } + return out +} diff --git a/audit/zz_redaction_test.go b/audit/zz_redaction_test.go index e86996e..149cccf 100644 --- a/audit/zz_redaction_test.go +++ b/audit/zz_redaction_test.go @@ -59,3 +59,48 @@ func TestBuildEntryDoesNotRedactNonSecretKeys(t *testing.T) { } } } + +// TestRedactMap verifies the exported redaction helper used by the +// webhook DLQ read API (PILOT-314). +func TestRedactMap(t *testing.T) { + t.Parallel() + + t.Run("redacts sensitive keys", func(t *testing.T) { + in := map[string]interface{}{ + "token": "abc123", + "hostname": "public-host", + "api_key": "sk-live", + "reason": "ok", + "db_secret": "pw", // suffix _secret + } + got := RedactMap(in) + if got["token"] != "" { + t.Errorf("token = %v, want ", got["token"]) + } + if got["api_key"] != "" { + t.Errorf("api_key = %v, want ", got["api_key"]) + } + if got["db_secret"] != "" { + t.Errorf("db_secret = %v, want ", got["db_secret"]) + } + if got["hostname"] != "public-host" { + t.Errorf("hostname = %v, want public-host", got["hostname"]) + } + if got["reason"] != "ok" { + t.Errorf("reason = %v, want ok", got["reason"]) + } + }) + + t.Run("nil-safe", func(t *testing.T) { + if got := RedactMap(nil); got != nil { + t.Errorf("RedactMap(nil) = %v, want nil", got) + } + }) + + t.Run("empty map", func(t *testing.T) { + got := RedactMap(map[string]interface{}{}) + if len(got) != 0 { + t.Errorf("RedactMap({}) len = %d, want 0", len(got)) + } + }) +} diff --git a/webhook/webhook.go b/webhook/webhook.go index 300e81c..93a6bda 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -24,6 +24,7 @@ import ( "sync/atomic" "time" + "github.com/pilot-protocol/rendezvous/audit" "github.com/pilot-protocol/rendezvous/events" ) @@ -379,6 +380,9 @@ func (st *Store) HandleGetWebhook() map[string]interface{} { } // HandleGetWebhookDLQ requires admin-token verification by the caller. +// PILOT-314: applies audit.RedactMap to each entry's Details so that +// any secret material not caught at audit-build time is stripped before +// the DLQ payload reaches the caller. func (st *Store) HandleGetWebhookDLQ() map[string]interface{} { st.mu.RLock() d := st.disp @@ -393,7 +397,7 @@ func (st *Store) HandleGetWebhookDLQ() map[string]interface{} { "timestamp": ev.Timestamp.Format("2006-01-02T15:04:05Z"), } if len(ev.Details) > 0 { - entry["details"] = ev.Details + entry["details"] = audit.RedactMap(ev.Details) } evts = append(evts, entry) } diff --git a/webhook/zz_webhook_test.go b/webhook/zz_webhook_test.go index 90f461e..db6338c 100644 --- a/webhook/zz_webhook_test.go +++ b/webhook/zz_webhook_test.go @@ -104,6 +104,60 @@ func TestHandleGetWebhookDLQ(t *testing.T) { } } +// TestHandleGetWebhookDLQRedactSecrets verifies that PILOT-314 is fixed: +// sensitive keys (token, password, etc.) in DLQ event Details are redacted +// when retrieved via HandleGetWebhookDLQ. +func TestHandleGetWebhookDLQRedactSecrets(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(400) // client error → DLQ + })) + defer srv.Close() + + st := webhook.NewStore() + defer st.Close() + st.SetInitialBackoff(time.Millisecond) + st.HandleSetWebhook(srv.URL) + + // Emit an event with a secret-bearing details map. + st.Emit("dlq.secret", map[string]interface{}{ + "admin_token": "supersecret", + "hostname": "public-host", + "password": "hunter2", + "reason": "test", + }) + + deadline := time.Now().Add(2 * time.Second) + var dlqResp map[string]interface{} + for time.Now().Before(deadline) { + dlqResp = st.HandleGetWebhookDLQ() + if count, _ := dlqResp["count"].(int); count >= 1 { + break + } + time.Sleep(20 * time.Millisecond) + } + + evts, _ := dlqResp["events"].([]map[string]interface{}) + if len(evts) < 1 { + t.Fatal("expected at least 1 DLQ event") + } + details, _ := evts[0]["details"].(map[string]interface{}) + + // Secret keys must be redacted. + for _, key := range []string{"admin_token", "password"} { + if v, ok := details[key]; !ok || v != "" { + t.Errorf("details[%q] = %v, want ", key, v) + } + } + // Non-secret keys must pass through. + if v := details["hostname"]; v != "public-host" { + t.Errorf("details[hostname] = %v, want public-host", v) + } + if v := details["reason"]; v != "test" { + t.Errorf("details[reason] = %v, want test", v) + } +} + // TestSubscribeFansOutFromEventBus verifies that Store.Subscribe causes // events published on the bus to be delivered to the webhook endpoint. func TestSubscribeFansOutFromEventBus(t *testing.T) {