diff --git a/internal/delivery/engine_integration_test.go b/internal/delivery/engine_integration_test.go index 0f4738b..2976a2b 100644 --- a/internal/delivery/engine_integration_test.go +++ b/internal/delivery/engine_integration_test.go @@ -1201,6 +1201,83 @@ func TestDeliverHTTP_TargetTimeout(t *testing.T) { iAssertResultFailed(t, db, del.ID) } +// TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog pins the size +// this engine stores for an oversized response, because the +// event log's redaction is written against it: the row holds +// exactly maxBodyLog bytes and records nothing about how much +// more the remote sent, so a credential echoed across that +// boundary reaches the database already severed and no reader +// of the row can tell the cut happened. +func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog( + t *testing.T, +) { + t.Parallel() + + // Padded so the cut falls five bytes before the end of the + // echoed webhook URL. + const ( + severedTail = 5 + overshoot = 100000 + ) + + sent := strings.Repeat( + "A", + delivery.ExportMaxBodyLog-len(slackWebhookURL)+ + severedTail, + ) + slackWebhookURL + strings.Repeat("Z", overshoot) + + s := newISetup(t) + + ts := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadGateway) + _, _ = io.WriteString(w, sent) + }, + )) + defer ts.Close() + + cfgJSON := iHTTPConfig(ts.URL) + + event := iSeedEvent( + t, s.WebhookDB, s.WebhookID, `{"cut":"test"}`, + ) + targetID := uuid.New().String() + + del := iSeedDelivery( + t, s.WebhookDB, event.ID, targetID, + database.DeliveryStatusPending, + ) + + bodyStr := event.Body + task := iTask( + del, event, s.WebhookID, targetID, + "cut-target", cfgJSON, 0, 1, &bodyStr, + ) + + s.Engine.ExportProcessNewTask(context.TODO(), &task) + + results := iResults(t, s.WebhookDB, del.ID) + require.Len(t, results, 1) + + stored := results[0].ResponseBody + + assert.Len( + t, stored, delivery.ExportMaxBodyLog, + "an oversized response is stored at exactly the cap", + ) + assert.Equal( + t, sent[:delivery.ExportMaxBodyLog], stored, + ) + assert.NotContains( + t, stored, slackWebhookURL, + "the echoed URL is severed by the cut", + ) + assert.Contains( + t, stored, "T00000000", + "the severed prefix still carries the credential", + ) +} + // iSeedEventAndDelivery creates event + delivery // for standalone tests. func iSeedEventAndDelivery( diff --git a/internal/delivery/export_test.go b/internal/delivery/export_test.go index ca37c59..26b4efa 100644 --- a/internal/delivery/export_test.go +++ b/internal/delivery/export_test.go @@ -25,6 +25,12 @@ const ( ExportRetryChannelSize = retryChannelSize ExportDefaultFailureThreshold = defaultFailureThreshold ExportDefaultCooldown = defaultCooldown + + // ExportMaxBodyLog is the cap the engine applies to a + // recorded response body. The event log's handling of a cut + // response is written against this number, so a test has to + // be able to name it. + ExportMaxBodyLog = maxBodyLog ) // ExportIsBlockedIP exposes isBlockedIP for testing. diff --git a/internal/delivery/target_redact.go b/internal/delivery/target_redact.go new file mode 100644 index 0000000..5b2923b --- /dev/null +++ b/internal/delivery/target_redact.go @@ -0,0 +1,256 @@ +package delivery + +import ( + "net/url" + "slices" + "strings" + + "sneak.berlin/go/webhooker/internal/database" +) + +// RedactionMarker stands in for a target credential found in +// text the target's remote peer chose. +const RedactionMarker = "(redacted)" + +// Redactor removes one target's own credential material from +// text that target's remote peer chose: a delivery response +// body, or a delivery error stored before the delivery path +// learned to mask the URLs it embeds. +// +// It removes byte-identical echoes of strings taken from the +// target's stored configuration, and nothing else. Anything +// the remote re-encodes survives: JSON "\/" escaping (what +// PHP's json_encode emits by default), percent-encoding, HTML +// entities, and an echo of only part of a path. It cannot +// remove a secret the remote invented. +// +// The zero Redactor removes nothing, which is what a caller +// holding no target for a delivery gets. +type Redactor struct { + secrets []string +} + +// NewRedactor builds the redactor for one target. +func NewRedactor(t *database.Target) Redactor { + secrets := targetSecrets(t) + + // Longest first, so replacing a secret that is contained + // in a longer one cannot leave a fragment of the longer + // one behind. Configured headers arrive in map order, so + // the sort is also what makes the result deterministic. + slices.SortFunc(secrets, func(a, b string) int { + if d := len(b) - len(a); d != 0 { + return d + } + + return strings.Compare(a, b) + }) + + return Redactor{secrets: secrets} +} + +// Redact replaces every occurrence of the target's credential +// material in s. +func (r Redactor) Redact(s string) string { + if s == "" { + return s + } + + for _, secret := range r.secrets { + s = strings.ReplaceAll(s, secret, RedactionMarker) + } + + return s +} + +// RedactCut redacts s, which its caller has already cut to a +// byte budget, and additionally drops any tail of s that is a +// proper prefix of a secret. +// +// The cut lands wherever the remote's padding puts it, so the +// remote chooses where inside the credential it falls. The +// severed prefix left behind equals no secret, so plain +// Redact would render it verbatim. +func (r Redactor) RedactCut(s string) string { + s = r.Redact(s) + + if n := r.secretPrefixSuffix(s); n > 0 { + return s[:len(s)-n] + RedactionMarker + } + + return s +} + +// secretPrefixSuffix returns the length of the longest suffix +// of s that is a proper prefix of one of the secrets, or 0 +// when there is none. +func (r Redactor) secretPrefixSuffix(s string) int { + longest := 0 + + for _, secret := range r.secrets { + // Proper prefixes only: a whole secret at the tail was + // already replaced by Redact. + n := min(len(secret)-1, len(s)) + for ; n > longest; n-- { + if strings.HasSuffix(s, secret[:n]) { + longest = n + + break + } + } + } + + return longest +} + +// targetSecrets returns the credential-bearing strings a +// target's configuration carries. +// +// The destination URL contributes. Its path, query and +// userinfo are the credential for both target types that have +// one — an incoming-webhook URL is a bearer token, which is +// why MaskURL elides exactly those parts — and they are the +// material this service actually sends, so a remote that +// echoes the request back echoes them. +// +// Configured request headers contribute their values, but +// only for the credential-shaped names isCredentialHeaderName +// picks out. That is the same class-based rule applied to +// URLs: an echoed Accept or User-Agent still renders, an +// echoed Authorization does not. +func targetSecrets(t *database.Target) []string { + if t == nil { + return nil + } + + switch t.Type { + case database.TargetTypeSlack: + cfg, err := parseSlackConfig(t.Config) + if err != nil { + return nil + } + + return urlSecrets(cfg.WebhookURL) + case database.TargetTypeHTTP: + cfg, err := parseHTTPConfig(t.Config) + if err != nil { + return nil + } + + return append( + urlSecrets(cfg.URL), + headerSecrets(cfg.Headers)..., + ) + case database.TargetTypeDatabase, database.TargetTypeLog: + // Neither has a destination URL, so neither has + // anything to redact. + return nil + default: + return nil + } +} + +// urlSecrets returns the substrings of a destination URL that +// must not survive into a rendered page: the whole URL, the +// parts of it MaskURL elides, and any userinfo. +// +// No length floor is applied to the path. A short path is +// treated as a credential exactly like a long one, because +// the field takes an arbitrary URL and no segment can be +// assumed non-secret — the same rule MaskURL applies. +func urlSecrets(raw string) []string { + raw = strings.TrimSpace(raw) + if raw == "" { + return nil + } + + secrets := []string{raw} + + parsed, err := url.Parse(raw) + if err != nil { + return secrets + } + + if parsed.Path != "" && parsed.Path != "/" { + requestURI := parsed.RequestURI() + secrets = append(secrets, requestURI) + + if escaped := parsed.EscapedPath(); escaped != requestURI { + secrets = append(secrets, escaped) + } + } + + if parsed.User != nil { + secrets = append(secrets, parsed.User.String()) + + if pw, ok := parsed.User.Password(); ok && pw != "" { + secrets = append(secrets, pw) + } + } + + return secrets +} + +// minHeaderSecretBytes is the shortest header value treated as +// a credential. Unlike a URL path, a header value can be a +// couple of bytes long, and redacting those would scatter the +// marker through ordinary response text for no gain. +const minHeaderSecretBytes = 4 + +// headerSecrets returns the values of the configured headers +// whose names are credential-shaped. +func headerSecrets(headers map[string]string) []string { + var secrets []string + + for name, value := range headers { + value = strings.TrimSpace(value) + if len(value) < minHeaderSecretBytes { + continue + } + + if isCredentialHeaderName(name) { + secrets = append(secrets, value) + } + } + + return secrets +} + +// isCredentialHeaderName classifies a header by its name. The +// value is never inspected, so the rule is the same +// class-based one MaskURL applies to a destination URL. +// +// The fragments are short on purpose, and match anywhere in +// the name, so abbreviations an operator might use are covered +// too: X-Sig, X-Pass, X-HMAC. That over-matches — a header +// named X-Design contains "sig" — and over-matching is the +// safe direction here: the cost is a marker where an echoed +// header value would have rendered. +func isCredentialHeaderName(name string) bool { + name = strings.ToLower(strings.TrimSpace(name)) + + // Names that carry a credential by definition. + switch name { + case "authorization", "proxy-authorization", "cookie": + return true + } + + // What operators call their own credential headers: + // X-Api-Key, X-Hub-Signature, X-Auth-Token. + for _, fragment := range []string{ + "auth", + "credential", + "hmac", + "key", + "pass", + "secret", + "sig", + "token", + } { + if strings.Contains(name, fragment) { + return true + } + } + + return false +} diff --git a/internal/delivery/target_redact_test.go b/internal/delivery/target_redact_test.go new file mode 100644 index 0000000..f6e46c5 --- /dev/null +++ b/internal/delivery/target_redact_test.go @@ -0,0 +1,246 @@ +package delivery_test + +import ( + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/delivery" +) + +// The secret path segments of a Slack incoming webhook URL. +const ( + redactSecretPath = "/services/T11111111/B11111111/" + + "YYYYYYYYYYYYYYYYYYYYYYYY" + redactWebhookURL = "https://hooks.slack.com" + + redactSecretPath +) + +func TestRedactor_RemovesSlackWebhookURL(t *testing.T) { + t.Parallel() + + r := delivery.NewRedactor(&database.Target{ + Type: database.TargetTypeSlack, + Config: `{"webhookUrl":"` + redactWebhookURL + `"}`, + }) + + got := r.Redact("no_service for " + redactWebhookURL) + + assert.NotContains(t, got, redactSecretPath) + assert.NotContains(t, got, "T11111111") + + // One marker, not a marker with the host left in front of + // it: the whole URL is replaced before the path it + // contains, which is what sorting the secrets longest + // first buys. + assert.Equal( + t, + "no_service for "+delivery.RedactionMarker, + got, + ) +} + +// TestRedactor_RemovesSecretSeveredByACut covers the input the +// redactor exists for: text cut to a byte budget with the +// credential straddling the cut. The remote chooses the +// padding, so it chooses where the cut lands inside the +// credential, and the severed prefix that remains equals no +// secret. +func TestRedactor_RemovesSecretSeveredByACut(t *testing.T) { + t.Parallel() + + r := delivery.NewRedactor(&database.Target{ + Type: database.TargetTypeSlack, + Config: `{"webhookUrl":"` + redactWebhookURL + `"}`, + }) + + // Every cut position inside the credential, not just a + // convenient one. + for n := 1; n < len(redactWebhookURL); n++ { + severed := redactWebhookURL[:n] + cut := "padding " + severed + + got := r.RedactCut(cut) + + assert.Equal( + t, + "padding "+delivery.RedactionMarker, + got, + "cut after %d bytes of the credential", n, + ) + } +} + +// TestRedactor_RedactsCredentialShapedHeaderValues pins the +// class-based header rule: a header whose name says credential +// has its value redacted, and a routine header does not, so +// ordinary response content survives. +func TestRedactor_RedactsCredentialShapedHeaderValues( + t *testing.T, +) { + t.Parallel() + + r := delivery.NewRedactor(&database.Target{ + Type: database.TargetTypeHTTP, + Config: `{"url":"https://example.com/in",` + + `"headers":{` + + `"Authorization":"Bearer AAAAAAAAAAAA",` + + `"Cookie":"session=BBBBBBBBBBBB",` + + `"X-Api-Key":"CCCCCCCCCCCC",` + + `"X-Hub-Signature":"sha256=DDDDDDDDDDDD",` + + `"X-Sig":"EEEEEEEEEEEE",` + + `"X-Pass":"FFFFFFFFFFFF",` + + `"X-HMAC":"GGGGGGGGGGGG",` + + `"X-Credential":"HHHHHHHHHHHH",` + + `"Accept":"application/json",` + + `"User-Agent":"webhooker/1.0"}}`, + }) + + for _, secret := range []string{ + "Bearer AAAAAAAAAAAA", + "session=BBBBBBBBBBBB", + "CCCCCCCCCCCC", + "sha256=DDDDDDDDDDDD", + // Abbreviated names an operator might use. + "EEEEEEEEEEEE", + "FFFFFFFFFFFF", + "GGGGGGGGGGGG", + "HHHHHHHHHHHH", + } { + got := r.Redact("echo: " + secret) + + assert.Equal( + t, + "echo: "+delivery.RedactionMarker, + got, + secret, + ) + } + + const routine = "Accept: application/json, " + + "User-Agent: webhooker/1.0" + + assert.Equal(t, routine, r.Redact(routine)) +} + +// TestRedactor_IgnoresVeryShortHeaderValues pins the floor +// under a header value. Redacting a two-byte value would put +// the marker through every response that happens to contain +// those bytes. +func TestRedactor_IgnoresVeryShortHeaderValues(t *testing.T) { + t.Parallel() + + r := delivery.NewRedactor(&database.Target{ + Type: database.TargetTypeHTTP, + Config: `{"url":"https://example.com/in",` + + `"headers":{"X-Api-Key":"ab"}}`, + }) + + const response = "rabbit" + + assert.Equal(t, response, r.Redact(response)) +} + +// TestRedactor_RemovesBarePath covers a remote that echoes +// only the request path rather than the whole URL. The path +// segments are the credential on their own. +func TestRedactor_RemovesBarePath(t *testing.T) { + t.Parallel() + + r := delivery.NewRedactor(&database.Target{ + Type: database.TargetTypeSlack, + Config: `{"webhookUrl":"` + redactWebhookURL + `"}`, + }) + + got := r.Redact("POST " + redactSecretPath + " 404") + + assert.NotContains(t, got, redactSecretPath) + assert.Equal( + t, + "POST "+delivery.RedactionMarker+" 404", + got, + ) +} + +// TestRedactor_RemovesHTTPURLQueryAndUserinfo covers the HTTP +// target, whose destination is an arbitrary URL: the query +// string and the userinfo carry credentials as readily as the +// path does. +func TestRedactor_RemovesHTTPURLQueryAndUserinfo(t *testing.T) { + t.Parallel() + + // Assembled rather than written out, so the literal is + // not itself a credential-shaped string. + dest := url.URL{ + Scheme: "https", + User: url.UserPassword("user", "hunter2"), + Host: "example.com", + Path: "/in", + RawQuery: "token=s3cr3t", + } + raw := dest.String() + + r := delivery.NewRedactor(&database.Target{ + Type: database.TargetTypeHTTP, + Config: `{"url":"` + raw + `"}`, + }) + + for _, echoed := range []string{ + raw, + "/in?token=s3cr3t", + "hunter2", + } { + got := r.Redact("rejected: " + echoed) + + assert.NotContains(t, got, "s3cr3t", echoed) + assert.NotContains(t, got, "hunter2", echoed) + assert.Contains( + t, got, delivery.RedactionMarker, echoed, + ) + } +} + +// TestRedactor_LeavesUnrelatedTextAlone pins that the +// redactor matches literally: it does not guess at what a +// secret looks like, so ordinary response content survives. +func TestRedactor_LeavesUnrelatedTextAlone(t *testing.T) { + t.Parallel() + + const response = "ok=false error=channel_not_found" + + r := delivery.NewRedactor(&database.Target{ + Type: database.TargetTypeSlack, + Config: `{"webhookUrl":"` + redactWebhookURL + `"}`, + }) + + assert.Equal(t, response, r.Redact(response)) +} + +// TestRedactor_ZeroValueAndConfiglessTargets pins that a +// caller with no target, an unparseable config, or a target +// type with no destination URL gets a redactor that changes +// nothing rather than one that panics. +func TestRedactor_ZeroValueAndConfiglessTargets(t *testing.T) { + t.Parallel() + + const text = "some response body" + + var zero delivery.Redactor + + assert.Equal(t, text, zero.Redact(text)) + assert.Equal(t, text, delivery.NewRedactor(nil).Redact(text)) + + for _, tgt := range []database.Target{ + {Type: database.TargetTypeLog}, + {Type: database.TargetTypeDatabase}, + {Type: database.TargetTypeSlack, Config: "not json"}, + {Type: database.TargetTypeHTTP, Config: ""}, + } { + assert.Equal( + t, text, + delivery.NewRedactor(&tgt).Redact(text), + tgt.Type, + ) + } +} diff --git a/internal/handlers/delivery_result_view.go b/internal/handlers/delivery_result_view.go new file mode 100644 index 0000000..e3bb27b --- /dev/null +++ b/internal/handlers/delivery_result_view.go @@ -0,0 +1,169 @@ +package handlers + +import ( + "sneak.berlin/go/webhooker/internal/delivery" +) + +// maxRenderedResponseBytes caps how many bytes of one stored +// delivery response body reach the event log page. +// +// The bound is enforced here and in SQL, because this page's +// memory profile must not depend on a constant in another +// package staying where it is, and because rows predating the +// delivery engine's own cap or restored from an archive are +// not covered by it at all. +// +// It happens to equal that engine cap, so a row written by the +// current engine reaches this one exactly and is never cut +// twice. Nothing here may assume the two differ: see view. +const maxRenderedResponseBytes = 4096 + +// deliveryResultColumns is the delivery attempt projection. +// The casts to blob are load-bearing for the same reason they +// are in eventLogColumns: they make substr and length count +// bytes rather than characters, and they make SQLite do the +// cut, so an oversized stored response never becomes a Go +// string at all. +const deliveryResultColumns = "delivery_id, attempt_num, success, " + + "status_code, error, duration, " + + "substr(cast(response_body as blob), 1, ?) AS response_body, " + + "length(cast(response_body as blob)) AS response_bytes" + +// DeliveryResultView is the display-safe projection of one +// delivery attempt for the event log page. It carries a +// capped response body plus the true stored size, so the page +// can mark a response as truncated without holding the whole +// thing. +// +// Both Error and ResponseBody have been through the target's +// Redactor. The engine already masks the URL out of the +// errors it stores, so for errors this is a second line +// covering rows written before it did; for response bodies it +// is the only line, and its reach is what +// delivery.Redactor documents. +type DeliveryResultView struct { + AttemptNum int + Success bool + + // StatusCode is 0 when the attempt never got a response, + // which is why the page asks HasStatusCode rather than + // printing the number. + StatusCode int + + // Error is the stored failure message, redacted. + Error string + + // DurationMS is how long the attempt took. + DurationMS int64 + + // ResponseBody holds at most maxRenderedResponseBytes + // bytes of the stored response, redacted. It is remote + // content and must only ever be rendered escaped. + ResponseBody string + + // ResponseBytes is the size of the stored response body, + // before the cut and before redaction. It is what the + // remote sent only when ResponseSizeKnown is set. + ResponseBytes int64 + + // ResponseShownBytes is how much of that the page is + // showing. It is the size of the cut, taken before + // redaction, so the truncation marker reports what SQLite + // returned rather than how much the marker substitution + // then changed the length. + ResponseShownBytes int + + // ResponseTruncated reports that the body shown may be + // incomplete, so the page owes the reader a marker. Every + // body that reaches the cap counts, because one the + // delivery engine cut at its own equal cap is + // indistinguishable from a complete one. + ResponseTruncated bool + + // ResponseSizeKnown reports that ResponseBytes is the whole + // response the remote sent, so the marker may quote it. It + // is false for a body that only reaches the cap, where how + // much came after it was never recorded. + ResponseSizeKnown bool +} + +// HasStatusCode reports whether the attempt got as far as an +// HTTP response. A transport failure stores no status code, +// and rendering that as "0" would read as a real status. +func (v DeliveryResultView) HasStatusCode() bool { + return v.StatusCode != 0 +} + +// deliveryResultRow is one row of the delivery attempt +// projection. Its response body arrives already cut to the +// cap by SQLite, with the true size beside it. +type deliveryResultRow struct { + DeliveryID string + AttemptNum int + Success bool + StatusCode int + Error string + Duration int64 + ResponseBody []byte + ResponseBytes int64 +} + +// view projects a loaded row for rendering, stripping the +// target's own credential out of the two fields a remote peer +// gets to influence. +func (r *deliveryResultRow) view( + redactor delivery.Redactor, +) DeliveryResultView { + body := r.ResponseBody + + // Two different cuts can have shortened this body, and the + // row records only one of them. SQLite cuts here, whenever + // the stored value is larger than the cap. The delivery + // engine cut earlier, whenever the remote sent more than + // its own maxBodyLog — which is this same number, so such a + // row stores the cut length as its whole length and nothing + // in it separates a response that ended at the cap from one + // severed there. + // + // So a body that reaches the cap is treated as cut either + // way. Gating on ResponseBytes alone would assume the two + // caps differ, and they do not: under the current engine + // that gate never opens. + cut := r.ResponseBytes > int64(len(body)) || + len(body) >= maxRenderedResponseBytes + + // The row holds more than the page shows only in the first + // of those cases. In the second the stored row is all there + // is, and its size is a floor rather than the true one. + sizeKnown := r.ResponseBytes > int64(len(body)) + + // Only a cut response can have been left mid-sequence, + // exactly as with an event body. + if cut { + body = trimPartialRune(body) + } + + // A cut body goes through RedactCut: the remote controls + // the padding ahead of a credential it echoes, so it + // controls where the cut falls inside that credential, and + // the severed prefix left behind matches no secret whole. + rendered := string(body) + if cut { + rendered = redactor.RedactCut(rendered) + } else { + rendered = redactor.Redact(rendered) + } + + return DeliveryResultView{ + AttemptNum: r.AttemptNum, + Success: r.Success, + StatusCode: r.StatusCode, + Error: redactor.Redact(r.Error), + DurationMS: r.Duration, + ResponseBody: rendered, + ResponseBytes: r.ResponseBytes, + ResponseShownBytes: len(body), + ResponseTruncated: cut, + ResponseSizeKnown: sizeKnown, + } +} diff --git a/internal/handlers/delivery_result_view_test.go b/internal/handlers/delivery_result_view_test.go new file mode 100644 index 0000000..63f387a --- /dev/null +++ b/internal/handlers/delivery_result_view_test.go @@ -0,0 +1,509 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "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/delivery" + "sneak.berlin/go/webhooker/internal/handlers" + "sneak.berlin/go/webhooker/internal/session" +) + +// responseCap is the number of response bytes the event log +// page is allowed to render for one delivery attempt. +const responseCap = handlers.MaxRenderedResponseBytesForTest + +// failedAttempt describes the failed delivery every test in +// this file seeds. The values are distinctive so that finding +// them in the rendered page cannot be a coincidence. +const ( + attemptStatusCode = 502 + attemptDurationMS = 1234 + attemptNumber = 3 + attemptError = "upstream returned 502 Bad Gateway" +) + +// seedFailedDelivery records an event, a failed delivery +// against targetID, and one delivery result carrying the +// given response body. It returns the delivery. +func seedFailedDelivery( + t *testing.T, + dbMgr *database.WebhookDBManager, + webhookID, targetID, responseBody string, +) *database.Delivery { + t.Helper() + + webhookDB, err := dbMgr.GetDB(webhookID) + require.NoError(t, err) + + event := &database.Event{ + WebhookID: webhookID, + Method: http.MethodPost, + Body: `{"test":true}`, + ContentType: "application/json", + } + + require.NoError(t, webhookDB.Omit( + clause.Associations, + ).Create(event).Error) + + dlv := &database.Delivery{ + EventID: event.ID, + TargetID: targetID, + Status: database.DeliveryStatusFailed, + } + + require.NoError(t, webhookDB.Omit( + clause.Associations, + ).Create(dlv).Error) + + result := &database.DeliveryResult{ + DeliveryID: dlv.ID, + AttemptNum: attemptNumber, + Success: false, + StatusCode: attemptStatusCode, + ResponseBody: responseBody, + Error: attemptError, + Duration: attemptDurationMS, + } + + require.NoError(t, webhookDB.Omit( + clause.Associations, + ).Create(result).Error) + + return dlv +} + +// seedFailureAndRender seeds a failed delivery against a +// target of the given type and config, and returns the +// rendered event log page. +func seedFailureAndRender( + t *testing.T, + targetType database.TargetType, + config, responseBody string, +) string { + t.Helper() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := seedWebhook(t, db) + tgt := seedConfiguredTarget( + t, db, wh.ID, targetType, config, + ) + + seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, responseBody) + + return renderSourceLogsPage(t, h, sess, wh.ID) +} + +// TestHandleSourceLogs_RendersFailedAttempt is the regression +// test for the reported gap: a failed delivery used to render +// as the status word alone, so diagnosing it meant opening the +// per-webhook SQLite file by hand. +func TestHandleSourceLogs_RendersFailedAttempt(t *testing.T) { + t.Parallel() + + body := seedFailureAndRender( + t, + database.TargetTypeHTTP, + `{"url":"https://example.com/hook/abc"}`, + "upstream exploded", + ) + + assert.Contains( + t, body, strconv.Itoa(attemptStatusCode), + "the attempt's status code must reach the page", + ) + assert.Contains( + t, body, attemptError, + "the attempt's error must reach the page", + ) + assert.Contains( + t, body, strconv.Itoa(attemptDurationMS), + "the attempt's duration must reach the page", + ) + assert.Contains( + t, body, "Attempt "+strconv.Itoa(attemptNumber), + "the attempt number must reach the page", + ) + assert.Contains( + t, body, "upstream exploded", + "the attempt's response body must reach the page", + ) +} + +// TestHandleSourceLogs_EscapesResponseBody proves the +// response body is treated as the untrusted remote content it +// is. The remote chooses these bytes and the page is rendered +// inside the operator's authenticated origin, where the +// application's own CSP allows inline script from 'self'. +func TestHandleSourceLogs_EscapesResponseBody(t *testing.T) { + t.Parallel() + + const payload = `` + + body := seedFailureAndRender( + t, + database.TargetTypeHTTP, + `{"url":"https://example.com/hook/abc"}`, + payload, + ) + + assert.NotContains(t, body, payload) + assert.NotContains(t, body, "