From 44a73ecf2708a425b2622effb2021b05a5118b3a Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Fri, 29 May 2026 18:16:42 +0000 Subject: [PATCH 1/2] fix: add HMAC-SHA256 signing to outbound rendezvous webhook POSTs (PILOT-239) Add crypto/hmac, crypto/sha256, encoding/hex imports. Add secret field to dispatcher struct. Compute HMAC-SHA256 of body in post(); set X-Pilot-Signature-256 header. Add Store.SetSecret() to configure the pre-shared secret. Switch from client.Post to NewRequest+Do to support custom headers. Mirrors existing pattern from pilot-protocol/webhook (X-Pilot-Signature-256). --- webhook/webhook.go | 36 ++++++++++++++++++- webhook/zz_more_test.go | 80 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/webhook/webhook.go b/webhook/webhook.go index 84330b4..300e81c 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -14,6 +14,9 @@ package webhook import ( "bytes" + "crypto/hmac" + "crypto/sha256" + "encoding/hex" "encoding/json" "log/slog" "net/http" @@ -45,6 +48,7 @@ type dispatcher struct { failed atomic.Uint64 delivered atomic.Uint64 initialBackoff time.Duration + secret string // HMAC-SHA256 pre-shared secret (empty = no sig) // Dead letter queue: stores last N failed events for retry/inspection dlqMu sync.Mutex @@ -141,6 +145,15 @@ func (d *dispatcher) post(ev *Event) { return } + // HMAC-SHA256 signature: if a secret is configured, sign the body + // so the receiver can verify authenticity+integrity (PILOT-239). + var sigHeader string + if d.secret != "" { + mac := hmac.New(sha256.New, []byte(d.secret)) + mac.Write(body) + sigHeader = hex.EncodeToString(mac.Sum(nil)) + } + backoff := d.initialBackoff for attempt := 0; attempt < MaxRetries; attempt++ { if attempt > 0 { @@ -148,7 +161,16 @@ func (d *dispatcher) post(ev *Event) { backoff *= 2 } - resp, err := d.client.Post(d.url, "application/json", bytes.NewReader(body)) + req, err := http.NewRequest(http.MethodPost, d.url, bytes.NewReader(body)) + if err != nil { + slog.Warn("registry webhook POST request build failed", "action", ev.Action, "error", err) + continue + } + req.Header.Set("Content-Type", "application/json") + if sigHeader != "" { + req.Header.Set("X-Pilot-Signature-256", sigHeader) + } + resp, err := d.client.Do(req) if err != nil { slog.Warn("registry webhook POST failed", "action", ev.Action, "attempt", attempt+1, "error", err) continue @@ -240,6 +262,18 @@ func (st *Store) SetURL(url string) { } } +// SetSecret sets the HMAC-SHA256 pre-shared secret for outbound webhook +// signatures. When non-empty, every outbound POST includes an +// X-Pilot-Signature-256 header with the hex-encoded HMAC-SHA256 of the +// request body (PILOT-239). No-op when no dispatcher is active. +func (st *Store) SetSecret(secret string) { + st.mu.Lock() + defer st.mu.Unlock() + if st.disp != nil { + st.disp.secret = secret + } +} + // SetInitialBackoff sets the retry backoff. Tests set a short value to avoid // waiting on retry exhaustion. No-op when no dispatcher is active. func (st *Store) SetInitialBackoff(d time.Duration) { diff --git a/webhook/zz_more_test.go b/webhook/zz_more_test.go index 40b2c0f..c020fa8 100644 --- a/webhook/zz_more_test.go +++ b/webhook/zz_more_test.go @@ -3,6 +3,10 @@ package webhook import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "io" "net/http" "net/http/httptest" "sync" @@ -150,6 +154,82 @@ func TestDispatcher_EmitAfterCloseIsNoop(t *testing.T) { } // TestDispatcher_RaceyCloseEmit ensures concurrent Emit+Close doesn't panic. +// TestStore_SetSecret_SignsWebhook verifies that when a secret is set, +// outbound webhook POSTs carry an X-Pilot-Signature-256 header with the +// hex-encoded HMAC-SHA256 of the request body (PILOT-239). +func TestStore_SetSecret_SignsWebhook(t *testing.T) { + t.Parallel() + secret := "test-secret-pilot" + var sigHeader string + var bodyBytes []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sigHeader = r.Header.Get("X-Pilot-Signature-256") + bodyBytes, _ = io.ReadAll(r.Body) + w.WriteHeader(200) + })) + defer srv.Close() + + st := NewStore() + defer st.Close() + st.SetInitialBackoff(time.Millisecond) + st.SetURL(srv.URL) + st.SetSecret(secret) + + st.Emit("test.secret", map[string]interface{}{"k": "v"}) + + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if sigHeader != "" { + break + } + time.Sleep(10 * time.Millisecond) + } + if sigHeader == "" { + t.Fatal("X-Pilot-Signature-256 header not set when secret is configured") + } + + // Verify the HMAC ourselves. + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write(bodyBytes) + expected := hex.EncodeToString(mac.Sum(nil)) + if sigHeader != expected { + t.Fatalf("HMAC mismatch: got %s, want %s", sigHeader, expected) + } +} + +// TestStore_SetSecret_NoSignatureWhenNoSecret verifies that no signature +// header is added when the secret is empty (backward-compatible). +func TestStore_SetSecret_NoSignatureWhenNoSecret(t *testing.T) { + t.Parallel() + var sigHeader string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sigHeader = r.Header.Get("X-Pilot-Signature-256") + w.WriteHeader(200) + })) + defer srv.Close() + + st := NewStore() + defer st.Close() + st.SetInitialBackoff(time.Millisecond) + st.SetURL(srv.URL) + // Do NOT call SetSecret — default is empty. + + st.Emit("test.nosecret", map[string]interface{}{"k": "v"}) + + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + _, delivered, _ := st.disp.stats() + if delivered > 0 { + break + } + time.Sleep(10 * time.Millisecond) + } + + if sigHeader != "" { + t.Fatal("X-Pilot-Signature-256 should NOT be set when no secret configured") + } +} + func TestDispatcher_RaceyCloseEmit(t *testing.T) { t.Parallel() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { From f513651ba0496d80c925e6f8263a86ce8882ead7 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Fri, 29 May 2026 14:23:03 -0700 Subject: [PATCH 2/2] test: protect shared HTTP-handler vars with mutex (PILOT-239) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two new TestStore_SetSecret_* tests had the HTTP handler closure write to plain string/[]byte variables that the test body then read. This is a data race the -race detector caught: - handler goroutine: 'sigHeader = r.Header.Get(...)' at line 166 - test goroutine: 'if sigHeader != ""' at line 182 The race-detector also tripped TestDispatcher_EmitDropsWhenQueueFull (same package, runs after) — that test wasn't itself racy but the detector marks the package failed once it has fired. Fix: protect the shared vars with sync.Mutex; provide getSig/getBody helpers for the test goroutine. Verified locally: go test -race -run TestStore_SetSecret_* + TestDispatcher_EmitDropsWhenQueueFull all PASS. --- webhook/zz_more_test.go | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/webhook/zz_more_test.go b/webhook/zz_more_test.go index c020fa8..9469f3b 100644 --- a/webhook/zz_more_test.go +++ b/webhook/zz_more_test.go @@ -160,11 +160,16 @@ func TestDispatcher_EmitAfterCloseIsNoop(t *testing.T) { func TestStore_SetSecret_SignsWebhook(t *testing.T) { t.Parallel() secret := "test-secret-pilot" - var sigHeader string - var bodyBytes []byte + var ( + mu sync.Mutex + sigHeader string + bodyBytes []byte + ) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() sigHeader = r.Header.Get("X-Pilot-Signature-256") bodyBytes, _ = io.ReadAll(r.Body) + mu.Unlock() w.WriteHeader(200) })) defer srv.Close() @@ -177,23 +182,27 @@ func TestStore_SetSecret_SignsWebhook(t *testing.T) { st.Emit("test.secret", map[string]interface{}{"k": "v"}) + getSig := func() string { mu.Lock(); defer mu.Unlock(); return sigHeader } + getBody := func() []byte { mu.Lock(); defer mu.Unlock(); return bodyBytes } + deadline := time.Now().Add(2 * time.Second) for time.Now().Before(deadline) { - if sigHeader != "" { + if getSig() != "" { break } time.Sleep(10 * time.Millisecond) } - if sigHeader == "" { + gotSig := getSig() + if gotSig == "" { t.Fatal("X-Pilot-Signature-256 header not set when secret is configured") } // Verify the HMAC ourselves. mac := hmac.New(sha256.New, []byte(secret)) - mac.Write(bodyBytes) + mac.Write(getBody()) expected := hex.EncodeToString(mac.Sum(nil)) - if sigHeader != expected { - t.Fatalf("HMAC mismatch: got %s, want %s", sigHeader, expected) + if gotSig != expected { + t.Fatalf("HMAC mismatch: got %s, want %s", gotSig, expected) } } @@ -201,9 +210,14 @@ func TestStore_SetSecret_SignsWebhook(t *testing.T) { // header is added when the secret is empty (backward-compatible). func TestStore_SetSecret_NoSignatureWhenNoSecret(t *testing.T) { t.Parallel() - var sigHeader string + var ( + mu sync.Mutex + sigHeader string + ) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() sigHeader = r.Header.Get("X-Pilot-Signature-256") + mu.Unlock() w.WriteHeader(200) })) defer srv.Close() @@ -225,7 +239,10 @@ func TestStore_SetSecret_NoSignatureWhenNoSecret(t *testing.T) { time.Sleep(10 * time.Millisecond) } - if sigHeader != "" { + mu.Lock() + got := sigHeader + mu.Unlock() + if got != "" { t.Fatal("X-Pilot-Signature-256 should NOT be set when no secret configured") } }