sneak/webhooker
1
0
Fork 0
Code Issues 62 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

Remove inbound request signature verification (closes #279)
All checks were successful
check / check (push) Successful in 3m14s
Details

Browse Source
This commit was merged in pull request #281.
This commit is contained in:
clawbot
2026-08-24 03:25:09 +02:00
parent ee2276a912
commit 37b59f8822
16 changed files with 59 additions and 2227 deletions
Download Patch File Download Diff File Expand all files Collapse all files

342
internal/handlers/entrypoint_secret_test.go
View File

@@ -1,342 +0,0 @@
package handlers_test
import (
"net/http"
"net/http/httptest"
"net/url"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
// submitEntrypointSecret posts the signature configuration form for
// an entrypoint and returns the recorder.
func submitEntrypointSecret(
t *testing.T,
h *handlers.Handlers,
cookies []*http.Cookie,
webhookID, entrypointID, scheme, secret string,
) *httptest.ResponseRecorder {
t.Helper()
form := url.Values{}
form.Set("signature_scheme", scheme)
form.Set("secret", secret)
req := formRequest(
"/source/"+webhookID+"/entrypoints/"+
entrypointID+"/secret",
cookies,
form,
map[string]string{
paramSourceID: webhookID,
entrypointIDParam: entrypointID,
},
)
w := httptest.NewRecorder()
h.HandleEntrypointSecret().ServeHTTP(w, req)
return w
}
// reloadEntrypoint reads an entrypoint back from the database,
// including the columns the model keeps out of JSON.
func reloadEntrypoint(
t *testing.T,
db *database.Database,
id string,
) database.Entrypoint {
t.Helper()
var ep database.Entrypoint
require.NoError(
t, db.DB().Where("id = ?", id).First(&ep).Error,
)
return ep
}
// TestEntrypointSecretSetRotateAndRemove walks the whole lifecycle
// the UI has to support: turning verification on, rotating the secret
// to a new value, and turning it back off.
func TestEntrypointSecretSetRotateAndRemove(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
cookies := authenticatedCookies(
t, sess, deleteTestUserID, deleteTestUsername,
)
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID, database.SignatureSchemeNone, "",
)
// Set.
w := submitEntrypointSecret(
t, h, cookies, wh.ID, ep.ID, "github", inboundSecret,
)
require.Equal(t, http.StatusSeeOther, w.Code)
stored := reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t, database.SignatureSchemeGitHub, stored.SignatureScheme,
)
assert.Equal(t, inboundSecret, stored.SignatureSecret)
assert.True(t, stored.SignatureConfigured())
// Rotate: a new secret and a different scheme in one submission.
// The new value is submitted with surrounding whitespace, the way
// a secret pasted out of a password manager arrives; storing that
// verbatim would make every later request fail verification with
// nothing visible on either side to explain it.
const rotated = "QQROTATEDSECRETQQ"
w = submitEntrypointSecret(
t, h, cookies, wh.ID, ep.ID, "gitlab", " "+rotated+"\t",
)
require.Equal(t, http.StatusSeeOther, w.Code)
stored = reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t, database.SignatureSchemeGitLab, stored.SignatureScheme,
)
assert.Equal(t, rotated, stored.SignatureSecret)
// Remove. The secret has to go with the scheme: a stored
// credential nothing reads is one more copy to leak.
w = submitEntrypointSecret(t, h, cookies, wh.ID, ep.ID, "", "")
require.Equal(t, http.StatusSeeOther, w.Code)
stored = reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t, database.SignatureSchemeNone, stored.SignatureScheme,
)
assert.Empty(t, stored.SignatureSecret)
assert.False(t, stored.SignatureConfigured())
}
// TestEntrypointSecretRejectsBadInput proves the form cannot create a
// row the receiver would later have to refuse. Both rejections leave
// the stored configuration untouched rather than half-applied.
func TestEntrypointSecretRejectsBadInput(t *testing.T) {
t.Parallel()
cases := []struct {
name string
scheme string
secret string
}{
{
name: "unsupported scheme",
scheme: "stripe",
secret: inboundSecret,
},
{
name: "scheme with no secret",
scheme: "github",
secret: "",
},
{
// Whitespace is stripped, so a secret of spaces is an
// empty one.
name: "scheme with blank secret",
scheme: "github",
secret: " ",
},
}
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
cookies := authenticatedCookies(
t, sess, deleteTestUserID, deleteTestUsername,
)
for _, tc := range cases {
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
w := submitEntrypointSecret(
t, h, cookies, wh.ID, ep.ID, tc.scheme, tc.secret,
)
assert.Equal(
t, http.StatusBadRequest, w.Code, "case %s", tc.name,
)
stored := reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t,
database.SignatureSchemeGitLab,
stored.SignatureScheme,
"case %s", tc.name,
)
assert.Equal(
t, inboundSecret, stored.SignatureSecret,
"case %s", tc.name,
)
}
}
// TestEntrypointSecretRequiresOwnership proves the configuration
// endpoint is bound by the same ownership check as the rest of the
// webhook's pages: another user's entrypoint is a 404, and the secret
// is not touched.
func TestEntrypointSecretRequiresOwnership(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
stranger := authenticatedCookies(
t, sess, "someone-else", "someoneelse",
)
w := submitEntrypointSecret(
t, h, stranger, wh.ID, ep.ID, "github", "hijacked",
)
assert.Equal(t, http.StatusNotFound, w.Code)
assert.Equal(
t,
inboundSecret,
reloadEntrypoint(t, db, ep.ID).SignatureSecret,
)
}
// TestHandleSourceDetail_MasksEntrypointSecret is the regression test
// for the credential on the entrypoint: the page has to say that
// verification is configured and which header carries it, without the
// secret itself ever reaching the rendered HTML.
func TestHandleSourceDetail_MasksEntrypointSecret(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitHub, inboundSecret,
)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.NotContains(t, body, inboundSecret)
assert.Contains(t, body, "GitHub")
assert.Contains(t, body, "X-Hub-Signature-256")
}
// TestEntrypointViewsDropTheSecret pins the projection itself, so the
// barrier survives a template rewrite that stops rendering the field
// the page test above looks at.
func TestEntrypointViewsDropTheSecret(t *testing.T) {
t.Parallel()
views := handlers.NewEntrypointViews([]database.Entrypoint{
{
Path: "p1",
Active: true,
SignatureScheme: database.SignatureSchemeGitHub,
SignatureSecret: inboundSecret,
},
{
Path: "p2",
},
{
// Half a configuration. The receiver 500s every request
// to this row, so the UI must not call it unverified.
Path: "p2a",
SignatureScheme: database.SignatureSchemeGitLab,
},
{
// The other half.
Path: "p2b",
SignatureSecret: inboundSecret,
},
{
// A scheme this build does not know: described as
// unavailable, never echoed back.
Path: "p3",
SignatureScheme: database.SignatureScheme("stripe"),
SignatureSecret: inboundSecret,
},
})
require.Len(t, views, 5)
assert.True(t, views[0].Configured)
assert.Equal(t, "GitHub", views[0].SchemeLabel)
assert.Equal(t, "X-Hub-Signature-256", views[0].SchemeHeader)
assert.False(t, views[1].Configured)
assert.Equal(t, "not verified", views[1].SchemeLabel)
assert.Empty(t, views[1].SchemeHeader)
for _, v := range []handlers.EntrypointView{views[2], views[3]} {
assert.False(t, v.Configured)
assert.Equal(t, "misconfigured", v.SchemeLabel)
assert.Empty(t, v.SchemeHeader)
}
assert.True(t, views[4].Configured)
assert.Equal(t, "(unavailable)", views[4].SchemeLabel)
// The struct has no field that could carry the secret, so this
// fails to compile rather than fails at runtime if one is added
// and populated. The assertion covers the labels it derives.
for _, v := range views {
assert.NotContains(t, v.SchemeLabel, inboundSecret)
assert.NotContains(t, v.SchemeHeader, inboundSecret)
assert.NotContains(t, string(v.Scheme), inboundSecret)
}
}

73
internal/handlers/entrypoint_view.go
View File

@@ -2,56 +2,18 @@ package handlers
import (
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/signature"
)
// signatureUnavailable is what an entrypoint's scheme renders as when
// the stored value is not one this build supports. The stored string
// is never echoed as a fallback: it is operator-supplied and the row
// is already in a state the receiver refuses, so the UI says so
// rather than inventing a description for it.
const signatureUnavailable = "(unavailable)"
// signatureNotVerified is the label for an entrypoint that performs
// no inbound verification.
const signatureNotVerified = "not verified"
// signatureMisconfigured is the label for a row holding one half of
// the scheme/secret pair. The receiver answers every request to such
// an entrypoint 500, so calling it "not verified" would describe a
// receiver that is refusing everything as one that is accepting
// everything. The form cannot create the state; a hand-edited
// database or a downgrade past a scheme can.
const signatureMisconfigured = "misconfigured"
// EntrypointView is the display-safe projection of an entrypoint for
// the UI. It deliberately has no secret field, so no template —
// present or future — can render the shared secret, in the same way
// delivery.TargetView keeps a target's stored credential away from
// one.
// the UI, in the same way delivery.TargetView is one for a target.
type EntrypointView struct {
ID string
Path string
Description string
Active bool
// Configured reports whether inbound requests to this entrypoint
// are verified.
Configured bool
// Scheme is the stored scheme, carried so the form can preselect
// it. It names an algorithm, not a secret.
Scheme database.SignatureScheme
// SchemeLabel and SchemeHeader describe the configured scheme for
// display: the sender's name, and the header its signature
// arrives in.
SchemeLabel string
SchemeHeader string
}
// NewEntrypointViews projects entrypoints for rendering, dropping the
// shared secret on the way.
// NewEntrypointViews projects entrypoints for rendering.
func NewEntrypointViews(
entrypoints []database.Entrypoint,
) []EntrypointView {
@@ -60,31 +22,12 @@ func NewEntrypointViews(
for i := range entrypoints {
e := &entrypoints[i]
view := EntrypointView{
ID: e.ID,
Path: e.Path,
Description: e.Description,
Active: e.Active,
Configured: e.SignatureConfigured(),
Scheme: e.SignatureScheme,
SchemeLabel: signatureNotVerified,
SchemeHeader: "",
}
switch {
case view.Configured:
view.SchemeLabel = signatureUnavailable
info, ok := signature.Info(e.SignatureScheme)
if ok {
view.SchemeLabel = info.Label
view.SchemeHeader = info.Header
}
case e.SignatureHalfConfigured():
view.SchemeLabel = signatureMisconfigured
}
views = append(views, view)
views = append(views, EntrypointView{
ID: e.ID,
Path: e.Path,
Description: e.Description,
Active: e.Active,
})
}
return views

4
internal/handlers/event_resubmit.go
View File

@@ -84,10 +84,6 @@ const resubmitColumns = "id, entrypoint_id, method, headers, " +
// is the stored EVENT. The response bodies and headers the original
// deliveries received stay where they are.
//
// Inbound signature verification is deliberately not re-run. There is
// no inbound signature to check on a copy the operator submits; the
// route is authenticated and CSRF-protected as an operator action.
//
// Resubmitting the same event repeatedly is supported and is the point
// of the feature, so replay's in-flight refusal is deliberately not
// applied here. The route's rate limit is what bounds a held-down

157
internal/handlers/source_management.go
View File

@@ -13,7 +13,6 @@ import (
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/signature"
)
// WebhookListItem holds data for the webhook list view.
@@ -443,16 +442,13 @@ func (h *Handlers) renderSourceDetail(
// receivers; html/template cannot address a value stored in a map.
data := map[string]any{
tmplKeyWebhook: &webhook,
// Entrypoints and targets are both projected to
// display-safe views: an entrypoint carries the shared
// secret its senders sign with and a target's stored
// config blob holds a credential, and neither must ever
// reach a template.
"Entrypoints": NewEntrypointViews(entrypoints),
"Targets": delivery.NewTargetViews(targets),
"SignatureSchemes": signature.Schemes(),
"Events": events,
"BaseURL": scheme + "://" + host,
// Targets are projected to a display-safe view: a
// target's stored config blob holds a credential, and it
// must never reach a template.
"Entrypoints": NewEntrypointViews(entrypoints),
"Targets": delivery.NewTargetViews(targets),
"Events": events,
"BaseURL": scheme + "://" + host,
}
h.renderTemplate(w, r, "source_detail.html", data)
@@ -1265,145 +1261,6 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
}
}
// HandleEntrypointSecret sets, rotates or removes the shared secret
// an entrypoint verifies inbound requests with.
//
// Setting and rotating are the same operation: the form always takes
// the secret afresh and the stored value is never sent to the browser
// to be edited, so there is no path by which the page can display a
// credential it holds. Rotation is therefore "submit the new secret",
// and the operator already has that value — both supported senders
// require them to enter the same string on the sender's side, so
// there is no generated value for webhooker to reveal once.
func (h *Handlers) HandleEntrypointSecret() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
webhook, ok := h.ownedWebhook(w, r)
if !ok {
return
}
// The body size cap is enforced by the MaxBodySize
// middleware, which runs before CSRF parses the form.
err := r.ParseForm()
if err != nil {
http.Error(
w, "Bad request", http.StatusBadRequest,
)
return
}
var entrypoint database.Entrypoint
err = h.db.DB().Where(
"id = ? AND webhook_id = ?",
chi.URLParam(r, "entrypointID"), webhook.ID,
).First(&entrypoint).Error
if err != nil {
http.NotFound(w, r)
return
}
h.applyEntrypointSecret(w, r, &entrypoint)
}
}
// applyEntrypointSecret validates the submitted scheme and secret and
// stores them.
//
// A scheme this build does not support is a 400, never a stored value
// the receiver would later have to interpret: the receiver fails such
// a row closed, so letting one be created would take the entrypoint
// offline through a form that reported success.
func (h *Handlers) applyEntrypointSecret(
w http.ResponseWriter,
r *http.Request,
entrypoint *database.Entrypoint,
) {
// PostFormValue, not FormValue: a credential must come from the
// body. FormValue falls back to the query string, and the request
// line — unlike the body — is what logs, proxies, Referer headers
// and error trackers record.
scheme := database.SignatureScheme(
r.PostFormValue("signature_scheme"),
)
// Surrounding whitespace is stripped, because a secret pasted from
// a password manager routinely carries some and the resulting
// mismatch is undiagnosable from the sender's side. A secret whose
// own first or last character is a space cannot be stored; the
// README says so.
secret := strings.TrimSpace(r.PostFormValue("secret"))
if !signature.Supported(scheme) {
http.Error(
w, "Invalid signature scheme",
http.StatusBadRequest,
)
return
}
if scheme == database.SignatureSchemeNone {
// Turning verification off drops the secret with it: a stored
// credential nothing reads is one more copy to leak, and
// Verify refuses that pairing in any case.
secret = ""
} else if secret == "" {
http.Error(
w,
"A shared secret is required for this signature scheme.",
http.StatusBadRequest,
)
return
}
h.storeEntrypointSecret(w, r, entrypoint, scheme, secret)
}
// storeEntrypointSecret writes a validated scheme and secret to an
// entrypoint and returns the operator to the webhook page.
func (h *Handlers) storeEntrypointSecret(
w http.ResponseWriter,
r *http.Request,
entrypoint *database.Entrypoint,
scheme database.SignatureScheme,
secret string,
) {
// Updates with a map rather than a struct: a struct update skips
// zero values, and the empty pair is exactly what has to be
// written when verification is being turned off.
err := h.db.DB().Model(entrypoint).Updates(map[string]any{
"signature_scheme": scheme,
"signature_secret": secret,
}).Error
if err != nil {
// The error is logged by serverError; GORM's error text
// carries the statement, not the bound values, so the secret
// does not travel with it.
h.serverError(
w, "failed to update entrypoint signature", err,
)
return
}
h.log.Info(
"entrypoint signature configuration updated",
"entrypoint_id", entrypoint.ID,
"webhook_id", entrypoint.WebhookID,
"scheme", string(scheme),
)
http.Redirect(
w, r,
"/source/"+entrypoint.WebhookID,
http.StatusSeeOther,
)
}
// HandleTargetCreate handles adding a new target to a webhook.
func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {

14
internal/handlers/ui_copy_test.go
View File

@@ -13,7 +13,6 @@ import (
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
"sneak.berlin/go/webhooker/internal/signature"
)
// Template data keys the page templates read. The handlers package has
@@ -269,16 +268,15 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
body := renderPage(t, h, sess, "source_detail.html", map[string]any{
dataKeyWebhook: webhook,
// The handler passes projected views, never raw rows — an
// entrypoint carries its shared secret and a target its
// stored credential — so the test data has that same shape.
// The handler passes projected views, never raw rows — a
// target carries its stored credential — so the test data
// has that same shape.
"Entrypoints": handlers.NewEntrypointViews(
[]database.Entrypoint{entrypoint},
),
"Targets": delivery.NewTargetViews(nil),
"SignatureSchemes": signature.Schemes(),
"Events": []database.Event{},
"BaseURL": "https://hooks.example.com",
"Targets": delivery.NewTargetViews(nil),
"Events": []database.Event{},
"BaseURL": "https://hooks.example.com",
})
assert.Contains(

88
internal/handlers/webhook.go
View File

@@ -2,7 +2,6 @@ package handlers
import (
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -12,7 +11,6 @@ import (
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/logfield"
"sneak.berlin/go/webhooker/internal/signature"
)
const (
@@ -72,8 +70,12 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
}
}
// processWebhookRequest reads the body, verifies the sender,
// serializes headers, loads targets, and delivers the event.
// processWebhookRequest reads the body, serializes headers, loads
// targets, and delivers the event.
//
// Nothing about the request itself is authenticated: the entrypoint
// UUID in the path is the credential, and reaching here means it
// matched an active entrypoint.
func (h *Handlers) processWebhookRequest(
w http.ResponseWriter,
r *http.Request,
@@ -84,26 +86,7 @@ func (h *Handlers) processWebhookRequest(
return
}
// Before anything is written. An unverified request must leave no
// event row, no delivery row and no delivery task behind, so this
// sits above every write rather than inside the transaction that
// performs them. It has to sit below the body read because the
// signature is computed over the body; readWebhookBody is what
// bounds that read, so an unauthenticated sender still cannot make
// the process hold more than the 1 MB cap.
if !h.verifyInboundSignature(w, entrypoint, r.Header, body) {
return
}
// These headers are about to be stored verbatim and handed to
// every delivery target, so the scheme's credential comes out
// first. Under GitLab's scheme the header is the shared secret
// itself, and leaving it in would hand the ability to forge
// signed requests to exactly the parties the signature is meant
// to exclude.
headersJSON, err := json.Marshal(
signature.SanitizeHeaders(&entrypoint, r.Header),
)
headersJSON, err := json.Marshal(r.Header)
if err != nil {
h.serverError(w, "failed to serialize headers", err)
@@ -122,63 +105,6 @@ func (h *Handlers) processWebhookRequest(
)
}
// verifyInboundSignature authenticates the request against the
// entrypoint's configured secret, reporting false once it has written
// the response.
//
// An entrypoint with no secret configured is not checked and this
// returns true, which is the unchanged behaviour every existing
// entrypoint keeps.
//
// A configuration that cannot be applied — an unknown scheme, or one
// half of the pair missing — is a 500, not a 401: the request may well
// be authentic, and calling it unauthorized would tell a legitimate
// sender to go fix its own signing. Either way it is refused. Failing
// open here would mean an entrypoint the operator has protected
// quietly accepting anything.
func (h *Handlers) verifyInboundSignature(
w http.ResponseWriter,
entrypoint database.Entrypoint,
header http.Header,
body []byte,
) bool {
err := signature.Verify(&entrypoint, header, body)
if err == nil {
return true
}
if errors.Is(err, signature.ErrConfig) {
h.log.Error(
"entrypoint signature configuration cannot be applied",
"entrypoint_id", entrypoint.ID,
"webhook_id", entrypoint.WebhookID,
"error", err,
)
http.Error(
w, "Internal server error",
http.StatusInternalServerError,
)
return false
}
// Every field here is bounded and none is client-chosen: the ids
// are ours, the scheme is one of a fixed set, and the error is a
// static string carrying no part of the secret or of what the
// client presented. Reaching this line also requires a real
// entrypoint UUID, so it is not a line a stranger can drive.
h.log.Warn(
"inbound signature verification failed",
"entrypoint_id", entrypoint.ID,
"webhook_id", entrypoint.WebhookID,
"scheme", string(entrypoint.SignatureScheme),
"error", err,
)
http.Error(w, "Unauthorized", http.StatusUnauthorized)
return false
}
// loadActiveTargets returns all active targets for a webhook.
func (h *Handlers) loadActiveTargets(
webhookID string,

468
internal/handlers/webhook_signature_test.go
View File

@@ -1,468 +0,0 @@
package handlers_test
import (
"bytes"
"context"
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/go-chi/chi"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/signature"
)
const (
// inboundSecret is the shared secret the signed-receiver tests
// configure on their entrypoint. It doubles as a marker: no log
// line and no rendered page may contain it.
inboundSecret = "QQINBOUNDSECRETQQ"
// inboundBody is the payload the sender signs.
inboundBody = `{"zen":"Non-blocking is better than blocking."}`
// entrypointIDParam is the chi URL parameter naming an entrypoint.
entrypointIDParam = "entrypointID"
)
// hubSignature returns the X-Hub-Signature-256 value a GitHub sender
// holding secret would send for inboundBody.
func hubSignature(secret string) string {
mac := hmac.New(sha256.New, []byte(secret))
_, _ = mac.Write([]byte(inboundBody))
return "sha256=" + hex.EncodeToString(mac.Sum(nil))
}
// seedSignedEntrypoint inserts an active entrypoint for a webhook
// with the given signature configuration and returns it.
func seedSignedEntrypoint(
t *testing.T,
db *database.Database,
webhookID string,
scheme database.SignatureScheme,
secret string,
) *database.Entrypoint {
t.Helper()
ep := &database.Entrypoint{
WebhookID: webhookID,
Path: "path-" + webhookID,
Description: "signed",
Active: true,
SignatureScheme: scheme,
SignatureSecret: secret,
}
require.NoError(
t,
db.DB().Omit(clause.Associations).Create(ep).Error,
)
return ep
}
// postToEntrypoint drives the real receiver handler at an
// entrypoint's path with one optional header set.
func postToEntrypoint(
t *testing.T,
h *handlers.Handlers,
path, body, headerName, headerValue string,
) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodPost,
"/webhook/"+path,
strings.NewReader(body),
)
req.Header.Set("Content-Type", "application/json")
if headerName != "" {
req.Header.Set(headerName, headerValue)
}
rctx := chi.NewRouteContext()
rctx.URLParams.Add("uuid", path)
req = req.WithContext(
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
)
w := httptest.NewRecorder()
h.HandleWebhook().ServeHTTP(w, req)
return w
}
// storedEvents counts the event rows a webhook's per-webhook database
// holds. A database that was never opened holds none, which is the
// state a rejected request has to leave behind.
func storedEvents(
t *testing.T,
mgr *database.WebhookDBManager,
webhookID string,
) int64 {
t.Helper()
if !mgr.DBExists(webhookID) {
return 0
}
db, err := mgr.GetDB(webhookID)
require.NoError(t, err)
var count int64
require.NoError(
t,
db.Model(&database.Event{}).
Where("webhook_id = ?", webhookID).
Count(&count).Error,
)
return count
}
// storedEventHeaders reads back the Headers column of the single
// event row a webhook's per-webhook database holds.
//
// It reads the database rather than an in-memory struct on purpose:
// what matters is what an operator, a backup or the reaper's archive
// would find on disk, not what the handler passed around.
func storedEventHeaders(
t *testing.T,
mgr *database.WebhookDBManager,
webhookID string,
) string {
t.Helper()
require.True(t, mgr.DBExists(webhookID))
db, err := mgr.GetDB(webhookID)
require.NoError(t, err)
var events []database.Event
require.NoError(
t,
db.Where("webhook_id = ?", webhookID).
Find(&events).Error,
)
require.Len(t, events, 1)
return events[0].Headers
}
// signedReceiverCase is one inbound request against an entrypoint
// with a given stored signature configuration.
type signedReceiverCase struct {
name string
scheme database.SignatureScheme
secret string
headerName string
headerValue string
body string
wantStatus int
}
// signedReceiverCases covers each supported scheme with a valid
// signature, an invalid one and none at all, plus the two states that
// are not "a client got it wrong": an entrypoint with nothing
// configured, and one whose stored configuration cannot be applied.
func signedReceiverCases() []signedReceiverCase {
return append(
schemeReceiverCases(), unverifiedReceiverCases()...,
)
}
// schemeReceiverCases covers the two supported schemes.
func schemeReceiverCases() []signedReceiverCase {
return []signedReceiverCase{
{
name: "github valid",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature(inboundSecret),
body: inboundBody,
wantStatus: http.StatusOK,
},
{
name: "github wrong secret",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature("wrong"),
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
{
// A digest that was valid for a different body: the
// check is over the bytes as received.
name: "github body tampered",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature(inboundSecret),
body: inboundBody + " ",
wantStatus: http.StatusUnauthorized,
},
{
name: "github unsigned",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
{
name: "gitlab valid",
scheme: database.SignatureSchemeGitLab,
secret: inboundSecret,
headerName: signature.HeaderGitLab,
headerValue: inboundSecret,
body: inboundBody,
wantStatus: http.StatusOK,
},
{
name: "gitlab wrong token",
scheme: database.SignatureSchemeGitLab,
secret: inboundSecret,
headerName: signature.HeaderGitLab,
headerValue: "wrong",
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
{
name: "gitlab unsigned",
scheme: database.SignatureSchemeGitLab,
secret: inboundSecret,
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
}
}
// unverifiedReceiverCases covers the two entrypoint states that are
// not about a client getting its signature wrong: nothing configured
// at all, and a configuration the receiver cannot apply.
func unverifiedReceiverCases() []signedReceiverCase {
return []signedReceiverCase{
{
// The pass-through case. An entrypoint with nothing
// configured is what every deployment already has, and
// it must keep accepting unsigned requests so that an
// upgrade does not lock an operator out of their own
// receivers.
name: "unconfigured accepts unsigned",
scheme: database.SignatureSchemeNone,
body: inboundBody,
wantStatus: http.StatusOK,
},
{
// A stray signature header changes nothing when nothing
// is configured to check it.
name: "unconfigured ignores a stray header",
scheme: database.SignatureSchemeNone,
headerName: signature.HeaderGitHub,
headerValue: "sha256=deadbeef",
body: inboundBody,
wantStatus: http.StatusOK,
},
{
// A scheme this build cannot apply, reachable only by
// editing the database: refused, not waved through as
// unverified.
name: "unknown scheme fails closed",
scheme: database.SignatureScheme("stripe"),
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature(inboundSecret),
body: inboundBody,
wantStatus: http.StatusInternalServerError,
},
}
}
// TestReceiverVerifiesConfiguredEntrypoints is the load-bearing test
// for the feature: for each supported scheme a correctly signed
// request is accepted and stored, and an incorrectly signed or
// unsigned one is answered 401 having stored nothing.
//
// The event count is the half that matters most. A rejection that
// still wrote a row would leave the receiver a place for a stranger
// who knows a URL to deposit content, which is exactly what the
// signature is there to prevent.
//
// The cases share one application and take a webhook each, rather
// than each standing up its own: every newTestApp seeds an admin user
// and so pays an Argon2id hash at 64 MB, and this package's test
// budget is not large enough to spend one per table row.
func TestReceiverVerifiesConfiguredEntrypoints(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
mgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &mgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
for _, tc := range signedReceiverCases() {
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID, tc.scheme, tc.secret,
)
w := postToEntrypoint(
t, h, ep.Path, tc.body,
tc.headerName, tc.headerValue,
)
assert.Equal(t, tc.wantStatus, w.Code, "case %s", tc.name)
want := int64(0)
if tc.wantStatus == http.StatusOK {
want = 1
}
assert.Equal(
t, want, storedEvents(t, mgr, wh.ID),
"case %s: stored event rows after a %d response",
tc.name, w.Code,
)
}
}
// TestReceiverLogsNoSecret proves the rejection path does not write
// the shared secret, or what the client presented, into the log. A
// GitLab token arrives as the credential itself, so echoing the
// header value would put a live secret in the log of every deployment
// whose sender is briefly misconfigured.
func TestReceiverLogsNoSecret(t *testing.T) {
t.Parallel()
const presented = "QQPRESENTEDVALUEQQ"
var (
h *handlers.Handlers
db *database.Database
)
app := newTestApp(t, &h, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
var buf bytes.Buffer
h.SetLogForTest(slog.New(slog.NewJSONHandler(&buf, nil)))
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
w := postToEntrypoint(
t, h, ep.Path, inboundBody,
signature.HeaderGitLab, presented,
)
require.Equal(t, http.StatusUnauthorized, w.Code)
// The rejection is recorded at all — a silent 401 leaves an
// operator no way to see a sender failing to authenticate.
assert.Contains(t, buf.String(), "verification failed")
assert.NotContains(t, buf.String(), inboundSecret)
assert.NotContains(t, buf.String(), presented)
}
// TestReceiverDoesNotStoreInboundCredential proves an accepted
// request leaves no copy of the shared secret in the event store.
//
// GitLab's X-Gitlab-Token is the credential itself, not a digest
// over the request. Stored headers are read back by the UI, copied
// into every backup and archive, and handed verbatim to every
// delivery target, so a stored token is the entrypoint's only
// authentication control disclosed to precisely the parties it
// exists to exclude.
//
// The two cases share one application: every newTestApp seeds an
// admin user and pays an Argon2id hash at 64 MB, and this package's
// test budget does not stretch to one per case.
func TestReceiverDoesNotStoreInboundCredential(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
mgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &mgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
gitlab := seedWebhook(t, db)
gitlabEP := seedSignedEntrypoint(
t, db, gitlab.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
w := postToEntrypoint(
t, h, gitlabEP.Path, inboundBody,
signature.HeaderGitLab, inboundSecret,
)
require.Equal(t, http.StatusOK, w.Code)
stored := storedEventHeaders(t, mgr, gitlab.ID)
assert.NotContains(
t, stored, inboundSecret,
"the shared secret must not be persisted",
)
assert.NotContains(
t, stored, signature.HeaderGitLab,
"the credential header must not be persisted at all",
)
// Everything else the sender set is still there. A fix that
// stored no headers would satisfy the assertions above while
// discarding the record the receiver exists to keep.
assert.Contains(t, stored, "Content-Type")
// A GitHub digest is an HMAC over the body, so the key cannot be
// recovered from it and it stays: the stripping is scoped to
// what actually carries the secret.
github := seedWebhook(t, db)
githubEP := seedSignedEntrypoint(
t, db, github.ID,
database.SignatureSchemeGitHub, inboundSecret,
)
w = postToEntrypoint(
t, h, githubEP.Path, inboundBody,
signature.HeaderGitHub, hubSignature(inboundSecret),
)
require.Equal(t, http.StatusOK, w.Code)
stored = storedEventHeaders(t, mgr, github.ID)
assert.Contains(t, stored, signature.HeaderGitHub)
assert.NotContains(t, stored, inboundSecret)
}