Compare commits

3 Commits

Author SHA1 Message Date
70f0fca5a2 Render delivery attempt detail in the event log (closes #202)
All checks were successful
check / check (push) Successful in 4m16s
Expanding a delivery on the event log page now shows each recorded
attempt: attempt number, outcome, status code, duration, error and
response body. Previously a failure rendered as "target: failed" and
diagnosing it meant opening the per-webhook SQLite file by hand.

The response body is cut by SQLite via substr over a blob cast, the
same projection the event body uses, so an oversized stored response
never becomes a Go string. The page reports the cut with a marker.

Response bodies and errors are remote content, so both go through a
new delivery.Redactor that strips the target's own destination URL,
path, query and userinfo, plus the values of credential-shaped
request headers, before rendering. A cut body goes through RedactCut
as well, which drops any tail that is a proper prefix of a secret:
the remote chooses the padding in front of a credential it echoes,
so it chooses where the cut falls inside that credential. Target
configuration keeps reaching the template only as a TargetView.

Redactors are built from an unscoped target load. Deleting a target
only soft deletes the row while its deliveries survive, and a scoped
load would leave exactly those deliveries rendering unredacted. The
views the page lists stay scoped.

Attempt loading is chunked so the IN clause cannot exceed SQLite's
bound-parameter limit, its error is reported rather than discarded,
and the page renders at most 20 attempts per delivery, counting what
it leaves out.

static/css/tailwind.css is regenerated with the repo's pinned
tailwindcss for the utility classes the new markup uses.
2026-08-20 05:04:28 +00:00
a13e5b7ded Shut down the app when the listener fails (closes #200) (#218)
All checks were successful
check / check (push) Successful in 3m48s
2026-08-20 06:42:36 +02:00
bb30b3ad64 Fail loudly on half-set metrics auth credentials (closes #205) (#216)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
2026-08-20 06:30:23 +02:00
16 changed files with 1885 additions and 60 deletions

View File

@@ -107,14 +107,26 @@ TTY detection, and security headers are always applied.
| `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` |
| `DEBUG` | Enable debug logging | `false` |
| `MAINTENANCE_MODE` | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false` |
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
| `METRICS_USERNAME` | Basic auth username for `/metrics`. Must be set together with `METRICS_PASSWORD`; one without the other fails startup | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics`. Must be set together with `METRICS_USERNAME`; one without the other fails startup | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket; a correct login password is never throttled either way) | `""` (none) |
#### Metrics credentials
`METRICS_USERNAME` and `METRICS_PASSWORD` are set together or not at
all. With both set, `/metrics` is served behind basic auth. With
neither set, the route is not registered and returns 404. With one set
and the other empty or unset, the process refuses to start and exits
non-zero with an error naming both variables — mounting the endpoint
on the username alone would publish it behind a password that is the
empty string, and quietly withholding it would deny an endpoint that
was asked for. The `hasMetricsAuth` field in the startup log and the
existence of the route are the same value, so they cannot disagree.
#### Trusted proxies
`TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
@@ -1682,7 +1694,7 @@ abuse limit later; they are tracked as future work.
| Method | Path | Description |
| ------ | ---------- | ----------- |
| `GET` | `/metrics` | Prometheus metrics, behind basic auth. The route is registered only when `METRICS_USERNAME` is set; otherwise it does not exist and returns 404 |
| `GET` | `/metrics` | Prometheus metrics, behind basic auth. The route is registered only when `METRICS_USERNAME` and `METRICS_PASSWORD` are both set; with neither set it does not exist and returns 404, and with only one set the process refuses to start |
#### API (Planned)
@@ -1837,7 +1849,8 @@ Applied to all routes in this order:
Permissions-Policy)
3. **Logging** — Structured request logging (method, URL, status,
latency, remote IP, user agent, request ID)
4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` is set)
4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` and
`METRICS_PASSWORD` are both set)
5. **CORS** — Cross-origin resource sharing headers
6. **Timeout** — 60-second request timeout
7. **Recoverer** — Panic recovery: one `ERROR` record through
@@ -1869,8 +1882,9 @@ being read and without reaching CSRF, the route group's remaining
middleware, or the handler. It is not rejected before *any* other
middleware, though: the global entries listed above all run first, so
such a request is still logged and given the security headers — and
counted in the metrics, on a deployment where `METRICS_USERNAME` is
set and the Metrics middleware is therefore registered at all. The
counted in the metrics, on a deployment where the `/metrics`
credentials are set and the Metrics middleware is therefore registered
at all. The
rejection itself is logged at `WARN` with the method, path and
declared length. A chunked request, or
one that lies about its length, is hard-capped by

View File

@@ -71,6 +71,16 @@ var ErrInvalidPort = errors.New("invalid port")
// nor a bare IP address.
var ErrInvalidCIDR = errors.New("invalid CIDR")
// ErrIncompleteMetricsAuth is returned when exactly one of
// METRICS_USERNAME and METRICS_PASSWORD carries a value. Neither
// fallback is acceptable: serving /metrics on the username alone
// publishes an endpoint whose password is the empty string, and
// silently leaving it unmounted withholds an endpoint the operator
// asked for. Half-set is a configuration error, so startup fails.
var ErrIncompleteMetricsAuth = errors.New(
"incomplete metrics credentials",
)
//nolint:revive // ConfigParams is a standard fx naming convention.
type ConfigParams struct {
fx.In
@@ -128,6 +138,21 @@ func (c *Config) IsProd() bool {
return c.Environment == EnvironmentProd
}
// MetricsAuthEnabled reports whether /metrics is served behind basic
// auth. It is the only answer to that question in the codebase: the
// route mount, the Prometheus recording middleware and the startup
// log's hasMetricsAuth field all read this one method, so the log
// cannot report auth as off while the route is mounted.
//
// It requires both credentials rather than the username alone.
// loadFromEnv already rejects a half-set pair, but a Config built in
// code bypasses that, and the failure mode this guards is an endpoint
// mounted with a credential map whose only password is the empty
// string.
func (c *Config) MetricsAuthEnabled() bool {
return c.MetricsUsername != "" && c.MetricsPassword != ""
}
// envString returns the value of the named environment variable,
// or an empty string if not set.
func envString(key string) string {
@@ -329,6 +354,30 @@ func envPrefixList(key string) ([]netip.Prefix, error) {
return prefixes, nil
}
// resolveMetricsAuth reads the /metrics basic-auth credentials and
// rejects a half-set pair, naming both variables either way. The
// error carries neither value: the password is a secret.
func resolveMetricsAuth() (string, string, error) {
username := envString("METRICS_USERNAME")
password := envString("METRICS_PASSWORD")
if (username == "") == (password == "") {
return username, password, nil
}
set, empty := "METRICS_USERNAME", "METRICS_PASSWORD"
if username == "" {
set, empty = empty, set
}
return "", "", fmt.Errorf(
"%w: %s is set but %s is empty; METRICS_USERNAME and "+
"METRICS_PASSWORD must both be set to serve /metrics, "+
"or both be empty to leave it unmounted",
ErrIncompleteMetricsAuth, set, empty,
)
}
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
// dev, and rejects unrecognised values.
func resolveEnvironment() (string, error) {
@@ -406,13 +455,18 @@ func loadFromEnv() (*Config, error) {
return nil, err
}
metricsUsername, metricsPassword, err := resolveMetricsAuth()
if err != nil {
return nil, err
}
return &Config{
DataDir: envString("DATA_DIR"),
Debug: debug,
MaintenanceMode: maintenanceMode,
Environment: environment,
MetricsUsername: envString("METRICS_USERNAME"),
MetricsPassword: envString("METRICS_PASSWORD"),
MetricsUsername: metricsUsername,
MetricsPassword: metricsPassword,
Port: port,
SentryDSN: envString("SENTRY_DSN"),
RetentionSweepInterval: retentionSweepInterval,
@@ -512,8 +566,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"receiverRateLimit", s.ReceiverRateLimit,
"trustedProxies", len(s.TrustedProxies),
"hasSentryDSN", s.SentryDSN != "",
"hasMetricsAuth",
s.MetricsUsername != "" && s.MetricsPassword != "",
"hasMetricsAuth", s.MetricsAuthEnabled(),
)
s.warnSharedRateLimitBucket(log)

View File

@@ -26,6 +26,12 @@ const (
// cidrPrivateV4 is the sample trusted-proxy block the
// TRUSTED_PROXIES cases are built from.
cidrPrivateV4 = "10.0.0.0/8"
// metricsAuthValue is the sample METRICS_PASSWORD the metrics
// credential cases are built from. It is asserted absent from
// the startup error, so it must not be a substring of either
// variable name that error prints.
metricsAuthValue = "s3cret"
)
func TestEnvironmentConfig(t *testing.T) {
@@ -726,3 +732,168 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
})
}
}
// metricsEnv describes what one subtest below puts in the
// environment for a single METRICS_ variable. A variable that is
// set to the empty string and one that is not set at all are
// distinct inputs here, because the reported bug arrived through
// the first of them.
type metricsEnv struct {
set bool
value string
}
// unset leaves the variable out of the environment entirely.
func unset() metricsEnv {
return metricsEnv{set: false, value: ""}
}
// setTo sets the variable, including to the empty string.
func setTo(value string) metricsEnv {
return metricsEnv{set: true, value: value}
}
// metricsAuthCase is one row of the table in TestMetricsAuthConfig,
// named so the table can live in its own function and keep the test
// itself short.
type metricsAuthCase struct {
name string
username metricsEnv
password metricsEnv
expectError bool
expectAuth bool
}
// metricsAuthCases enumerates every combination of the two
// credentials, counting "set to the empty string" and "not set at
// all" as separate inputs on each side.
func metricsAuthCases() []metricsAuthCase {
return []metricsAuthCase{
{
name: "both unset leaves metrics unmounted",
username: unset(),
password: unset(),
},
{
name: "both empty leaves metrics unmounted",
username: setTo(""),
password: setTo(""),
},
{
name: "both set enables metrics auth",
username: setTo("metrics"),
password: setTo(metricsAuthValue),
expectAuth: true,
},
{
name: "username with unset password fails",
username: setTo("metrics"),
password: unset(),
expectError: true,
},
{
name: "username with empty password fails",
username: setTo("metrics"),
password: setTo(""),
expectError: true,
},
{
name: "password with unset username fails",
username: unset(),
password: setTo(metricsAuthValue),
expectError: true,
},
{
name: "password with empty username fails",
username: setTo(""),
password: setTo(metricsAuthValue),
expectError: true,
},
}
}
// TestMetricsAuthConfig covers every combination of METRICS_USERNAME
// and METRICS_PASSWORD. Either both carry a value, in which case
// /metrics is served behind basic auth, or neither does, in which
// case the route is never mounted. One without the other is a
// startup error rather than a fallback: mounting on the username
// alone published /metrics behind a credential map that accepted an
// empty password, which is the defect this test exists to pin. See
// https://git.eeqj.de/sneak/webhooker/issues/205.
func TestMetricsAuthConfig(t *testing.T) {
for _, tt := range metricsAuthCases() {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
if tt.username.set {
t.Setenv("METRICS_USERNAME", tt.username.value)
} else {
require.NoError(
t, os.Unsetenv("METRICS_USERNAME"),
)
}
if tt.password.set {
t.Setenv("METRICS_PASSWORD", tt.password.value)
} else {
require.NoError(
t, os.Unsetenv("METRICS_PASSWORD"),
)
}
if tt.expectError {
assertMetricsAuthRejected(t)
return
}
assertMetricsAuthAccepted(t, tt.expectAuth)
})
}
}
// assertMetricsAuthRejected requires that fx refused to build the
// graph, that the failure is ErrIncompleteMetricsAuth, and that the
// operator is told both variable names — the point of failing here
// rather than degrading is that the message says what to fix.
func assertMetricsAuthRejected(t *testing.T) {
t.Helper()
var cfg *config.Config
app := fx.New(
fx.NopLogger,
fx.Provide(globals.New, logger.New, config.New),
fx.Populate(&cfg),
)
err := app.Err()
require.Error(t, err)
require.ErrorIs(t, err, config.ErrIncompleteMetricsAuth)
assert.Contains(t, err.Error(), "METRICS_USERNAME")
assert.Contains(t, err.Error(), "METRICS_PASSWORD")
// The password is a secret and must not reach a startup error.
assert.NotContains(t, err.Error(), metricsAuthValue)
}
// assertMetricsAuthAccepted requires that startup succeeded and that
// MetricsAuthEnabled — the single value the /metrics mount and the
// startup log both read — reports what the environment asked for.
func assertMetricsAuthAccepted(t *testing.T, expectAuth bool) {
t.Helper()
var cfg *config.Config
app := fxtest.New(
t,
fx.Provide(globals.New, logger.New, config.New),
fx.Populate(&cfg),
)
require.NoError(t, app.Err())
app.RequireStart()
defer app.RequireStop()
assert.Equal(t, expectAuth, cfg.MetricsAuthEnabled())
}

View File

@@ -0,0 +1,247 @@
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.
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",
"key",
"password",
"secret",
"signature",
"token",
} {
if strings.Contains(name, fragment) {
return true
}
}
return false
}

View File

@@ -0,0 +1,237 @@
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",` +
`"Accept":"application/json",` +
`"User-Agent":"webhooker/1.0"}}`,
})
for _, secret := range []string{
"Bearer AAAAAAAAAAAA",
"session=BBBBBBBBBBBB",
"CCCCCCCCCCCC",
"sha256=DDDDDDDDDDDD",
} {
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,
)
}
}

View File

@@ -0,0 +1,137 @@
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.
//
// It matches the cap the delivery engine applies when it
// records a result, so nothing written by the current engine
// is cut twice. The bound is enforced here anyway, and in
// SQL: this page's memory profile must not depend on a
// constant in another package staying where it is, and rows
// predating that cap or restored from an archive are not
// covered by it at all.
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 true size of the stored response
// body, before the cut and before redaction.
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 stored response was
// larger than the cap, so the page owes the reader a
// marker.
ResponseTruncated 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
truncated := r.ResponseBytes > int64(len(body))
// Only a cut response can have been left mid-sequence by
// this query, exactly as with an event body.
if truncated {
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 truncated {
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: truncated,
}
}

View File

@@ -0,0 +1,458 @@
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 = `<script>alert("xss")</script>`
body := seedFailureAndRender(
t,
database.TargetTypeHTTP,
`{"url":"https://example.com/hook/abc"}`,
payload,
)
assert.NotContains(t, body, payload)
assert.NotContains(t, body, "<script>alert")
assert.Contains(t, body, "alert")
}
// TestHandleSourceLogs_RedactsCredentialEchoedInResponse
// covers the case that makes rendering a response body a
// disclosure question at all: the remote echoes back the
// credential the request carried, and the page would then put
// it on the operator's screen.
func TestHandleSourceLogs_RedactsCredentialEchoedInResponse(
t *testing.T,
) {
t.Parallel()
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
"no_service: "+slackWebhookURL,
)
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.Contains(t, body, delivery.RedactionMarker)
// The rest of the response is still shown, or the
// redaction would have cost the operator the diagnosis.
assert.Contains(t, body, "no_service")
}
// TestHandleSourceLogs_RedactsCredentialEchoedInError covers
// the same disclosure through the error field. The delivery
// engine masks the URL out of the errors it stores, so this
// holds the read path to the rows written before it did.
func TestHandleSourceLogs_RedactsCredentialEchoedInError(
t *testing.T,
) {
t.Parallel()
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,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
)
dlv := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, "")
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
// An unmasked transport error, exactly as Go's HTTP
// client renders one.
require.NoError(t, webhookDB.Model(
&database.DeliveryResult{},
).Where(
"delivery_id = ?", dlv.ID,
).Update(
"error",
`Post "`+slackWebhookURL+`": dial tcp: i/o timeout`,
).Error)
body := renderSourceLogsPage(t, h, sess, wh.ID)
assert.NotContains(t, body, slackSecretPath)
assert.Contains(t, body, delivery.RedactionMarker)
assert.Contains(t, body, "i/o timeout")
}
// TestHandleSourceLogs_RedactsCredentialSeveredByTheCut is the
// regression test for a redactor that ran after the cut. The
// remote chooses the padding in front of the credential it
// echoes, so it chooses where the 4096-byte cut lands inside
// that credential; the severed prefix matches no secret whole
// and used to render verbatim.
func TestHandleSourceLogs_RedactsCredentialSeveredByTheCut(
t *testing.T,
) {
t.Parallel()
// Padding sized so the cut falls five bytes before the end
// of the webhook URL, leaving the workspace ID, the bot ID
// and all but the last few token characters in the cut
// body.
const severedTail = 5
pad := strings.Repeat(
"A", responseCap-len(slackWebhookURL)+severedTail,
)
stored := pad + slackWebhookURL + strings.Repeat("Z", 128)
require.Greater(
t, len(stored), responseCap,
"the stored body must exceed the cap or nothing is cut",
)
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
stored,
)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.NotContains(
t, body, slackWebhookURL[:len(slackWebhookURL)-10],
)
assert.Contains(t, body, delivery.RedactionMarker)
}
// TestHandleSourceLogs_RedactsForSoftDeletedTarget covers the
// target most likely to have a bad destination URL in its
// history: deleting and recreating is how an operator fixes a
// mistyped one. The row is only soft deleted and its
// deliveries survive, so its redactor has to survive with it.
func TestHandleSourceLogs_RedactsForSoftDeletedTarget(
t *testing.T,
) {
t.Parallel()
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,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
)
seedFailedDelivery(
t, dbMgr, wh.ID, tgt.ID,
"no_service: "+slackWebhookURL,
)
require.NoError(t, db.DB().Delete(tgt).Error)
body := renderSourceLogsPage(t, h, sess, wh.ID)
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.Contains(t, body, delivery.RedactionMarker)
assert.Contains(t, body, "no_service")
}
// TestHandleSourceLogs_BoundsRenderedAttempts pins the ceiling
// on how many of one delivery's attempts reach the page, and
// that what it drops is counted rather than hidden.
func TestHandleSourceLogs_BoundsRenderedAttempts(t *testing.T) {
t.Parallel()
const extraAttempts = 7
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, database.TargetTypeLog, "",
)
dlv := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, "")
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
total := handlers.MaxRenderedAttemptsForTest + extraAttempts
// seedFailedDelivery already recorded one attempt.
for i := range total - 1 {
require.NoError(t, webhookDB.Omit(
clause.Associations,
).Create(&database.DeliveryResult{
DeliveryID: dlv.ID,
AttemptNum: attemptNumber + 1 + i,
Error: attemptError,
}).Error)
}
views := h.LoadEventLogViewsForTest(
httptest.NewRecorder(), *wh, 1,
)
require.Len(t, views, 1)
require.Len(t, views[0].Deliveries, 1)
dv := views[0].Deliveries[0]
assert.Equal(t, total, dv.AttemptCount)
assert.Len(
t, dv.Results, handlers.MaxRenderedAttemptsForTest,
)
assert.Equal(t, extraAttempts, dv.AttemptsOmitted)
page := renderSourceLogsPage(t, h, sess, wh.ID)
assert.Contains(t, page, "attempts omitted")
assert.Contains(
t, page, strconv.Itoa(total)+" attempts",
"the header must count every recorded attempt",
)
}
// TestHandleSourceLogs_BoundsOversizeResponse proves the
// rendered page is bounded by the response cap rather than by
// the stored response size. The cut happens in SQLite, so the
// oversized value never becomes a Go string; this asserts the
// observable consequence, that neither the page nor the
// projection carries the tail.
func TestHandleSourceLogs_BoundsOversizeResponse(t *testing.T) {
t.Parallel()
const tail = "QQRESPONSETAILQQ"
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, database.TargetTypeLog, "",
)
stored := strings.Repeat("A", responseCap*4) + tail
seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, stored)
views := h.LoadEventLogViewsForTest(
httptest.NewRecorder(), *wh, 1,
)
require.Len(t, views, 1)
require.Len(t, views[0].Deliveries, 1)
require.Len(t, views[0].Deliveries[0].Results, 1)
attempt := views[0].Deliveries[0].Results[0]
assert.LessOrEqual(
t, len(attempt.ResponseBody), responseCap,
)
assert.Equal(
t, int64(len(stored)), attempt.ResponseBytes,
)
assert.True(t, attempt.ResponseTruncated)
page := renderSourceLogsPage(t, h, sess, wh.ID)
assert.NotContains(t, page, tail)
assert.Contains(
t, page, "Response truncated for display",
)
}

View File

@@ -19,6 +19,14 @@ func (s *Handlers) SetLogForTest(log *slog.Logger) {
// to the handlers_test package.
const MaxRenderedBodyBytesForTest = maxRenderedBodyBytes
// MaxRenderedResponseBytesForTest exposes the event log's
// delivery response cap to the handlers_test package.
const MaxRenderedResponseBytesForTest = maxRenderedResponseBytes
// MaxRenderedAttemptsForTest exposes the event log's
// per-delivery attempt ceiling to the handlers_test package.
const MaxRenderedAttemptsForTest = maxRenderedAttempts
// DummyVerificationsForTest reports how many equivalent-cost
// verifications were charged for usernames that do not exist. It
// lets a test prove the anti-enumeration path ran without timing

View File

@@ -4,11 +4,13 @@ import (
"encoding/json"
"errors"
"net/http"
"slices"
"strconv"
"strings"
"github.com/go-chi/chi"
"github.com/google/uuid"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
@@ -100,6 +102,32 @@ type DeliveryView struct {
ID string
Status database.DeliveryStatus
Target delivery.TargetView
// Results is this delivery's attempts in attempt order,
// bounded by maxRenderedAttempts. Without them a failure
// renders as the status word alone and says nothing about
// why.
Results []DeliveryResultView
// AttemptCount is how many attempts were recorded, which
// is more than len(Results) once the middle was dropped.
AttemptCount int
// AttemptsOmitted is how many attempts were dropped from
// the middle of Results. The page must show it, or the
// bound would hide history rather than fold it.
AttemptsOmitted int
}
// eventLogTarget is what the event log needs to know about
// one target: the display-safe view its template renders, and
// the redactor that keeps that target's own credential out of
// the text its remote peer chose. The two are kept together
// so a caller cannot pick up one without the other, and apart
// from TargetView so the secrets never reach a template.
type eventLogTarget struct {
View delivery.TargetView
Redactor delivery.Redactor
}
// HandleSourceList shows a list of user's webhooks.
@@ -765,7 +793,16 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
return
}
targets := h.loadTargetMap(webhook.ID)
targets, err := h.loadTargetMap(webhook.ID)
if err != nil {
// Without the map every delivery renders through a
// zero redactor, so failing the page is the only
// safe answer.
h.serverError(w, "failed to load targets", err)
return
}
page := h.parsePage(r)
evts, total := h.loadEventsWithDeliveries(
@@ -794,29 +831,54 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
}
// loadTargetMap loads targets into a map of display-safe
// views keyed by target ID. The projection happens here so
// that no caller can hand a raw target, configuration blob
// and all, to a template.
// views keyed by target ID, each paired with its redactor.
// The projection happens here so that no caller can hand a
// raw target, configuration blob and all, to a template: the
// raw rows do not leave this function.
//
// The load is Unscoped because deleting a target only soft
// deletes the row while its deliveries survive in the
// per-webhook database: a scoped load leaves those deliveries
// with a zero redactor, which renders their response bodies
// unredacted. Only the redactor half of the map is built from
// deleted rows. The view half, which is what the page lists,
// stays scoped.
func (h *Handlers) loadTargetMap(
webhookID string,
) map[string]delivery.TargetView {
) (map[string]eventLogTarget, error) {
var targets []database.Target
h.db.DB().Where(
err := h.db.DB().Unscoped().Where(
"webhook_id = ?", webhookID,
).Find(&targets)
views := delivery.NewTargetViews(targets)
targetMap := make(
map[string]delivery.TargetView, len(views),
)
for _, v := range views {
targetMap[v.ID] = v
).Find(&targets).Error
if err != nil {
return nil, err
}
return targetMap
targetMap := make(
map[string]eventLogTarget, len(targets),
)
live := make([]database.Target, 0, len(targets))
for i := range targets {
targetMap[targets[i].ID] = eventLogTarget{
Redactor: delivery.NewRedactor(&targets[i]),
}
if !targets[i].DeletedAt.Valid {
live = append(live, targets[i])
}
}
// The views come from NewTargetViews rather than being
// rebuilt here, so the masking rules stay in one place.
for _, v := range delivery.NewTargetViews(live) {
entry := targetMap[v.ID]
entry.View = v
targetMap[v.ID] = entry
}
return targetMap, nil
}
// parsePage extracts a page number from the query string.
@@ -840,7 +902,7 @@ func (h *Handlers) parsePage(r *http.Request) int {
func (h *Handlers) loadEventsWithDeliveries(
w http.ResponseWriter,
webhook database.Webhook,
targetMap map[string]delivery.TargetView,
targetMap map[string]eventLogTarget,
page int,
) ([]EventLogView, int64) {
var totalEvents int64
@@ -877,43 +939,165 @@ func (h *Handlers) loadEventsWithDeliveries(
).Find(&rows)
result = make([]EventLogView, len(rows))
eventDeliveries := make([][]database.Delivery, len(rows))
var deliveryIDs []string
for i := range rows {
result[i] = rows[i].view()
var deliveries []database.Delivery
webhookDB.Where(
"event_id = ?", rows[i].ID,
).Find(&deliveries)
).Find(&eventDeliveries[i])
for j := range eventDeliveries[i] {
deliveryIDs = append(
deliveryIDs, eventDeliveries[i][j].ID,
)
}
}
attempts := h.loadDeliveryResults(webhookDB, deliveryIDs)
for i := range rows {
result[i].Deliveries = newDeliveryViews(
deliveries, targetMap,
eventDeliveries[i], targetMap, attempts,
)
}
return result, totalEvents
}
// deliveryIDChunkSize bounds how many delivery IDs go into one
// IN clause. SQLite refuses a statement carrying more than
// SQLITE_MAX_VARIABLE_NUMBER (32766) bound parameters, and a
// page holds one delivery per target per event, so a webhook
// with enough targets would turn the whole query into an error
// and the page into zero attempts.
const deliveryIDChunkSize = 500
// loadDeliveryResults loads the recorded attempts for the
// page's deliveries, keyed by delivery ID.
//
// Each response body is cut by SQLite rather than in Go, for
// the reason deliveryResultColumns gives. How many attempts a
// delivery has is the target's MaxRetries, which the
// authenticated operator sets; how many of them reach the page
// is bounded again by maxRenderedAttempts.
func (h *Handlers) loadDeliveryResults(
webhookDB *gorm.DB,
deliveryIDs []string,
) map[string][]deliveryResultRow {
byDelivery := make(map[string][]deliveryResultRow)
for chunk := range slices.Chunk(
deliveryIDs, deliveryIDChunkSize,
) {
var rows []deliveryResultRow
err := webhookDB.Model(
&database.DeliveryResult{},
).Select(
deliveryResultColumns, maxRenderedResponseBytes,
).Where(
"delivery_id IN ?", chunk,
).Order("attempt_num ASC").Find(&rows).Error
if err != nil {
// A discarded error here renders as a delivery that
// never ran, which is indistinguishable from one
// that really never ran.
h.log.Error(
"failed to load delivery attempts",
"error", err,
)
return byDelivery
}
for i := range rows {
byDelivery[rows[i].DeliveryID] = append(
byDelivery[rows[i].DeliveryID], rows[i],
)
}
}
return byDelivery
}
// newDeliveryViews projects deliveries for rendering,
// resolving each one's target to its display-safe view.
// resolving each one's target to its display-safe view and
// each one's attempts through that target's redactor.
func newDeliveryViews(
deliveries []database.Delivery,
targetMap map[string]delivery.TargetView,
targetMap map[string]eventLogTarget,
attempts map[string][]deliveryResultRow,
) []DeliveryView {
views := make([]DeliveryView, len(deliveries))
for i := range deliveries {
target := targetMap[deliveries[i].TargetID]
rows := attempts[deliveries[i].ID]
results, omitted := renderedAttempts(
rows, target.Redactor,
)
views[i] = DeliveryView{
ID: deliveries[i].ID,
Status: deliveries[i].Status,
Target: targetMap[deliveries[i].TargetID],
Target: target.View,
Results: results,
AttemptCount: len(rows),
AttemptsOmitted: omitted,
}
}
return views
}
// maxRenderedAttempts bounds how many of one delivery's
// attempts the page renders. Past it the middle is dropped and
// counted, keeping the first attempts and the last ones: how
// the delivery started failing and how it ended are what a
// reader needs, and the count says plainly that the rest was
// dropped rather than never recorded.
const (
renderedAttemptsHead = 10
renderedAttemptsTail = 10
maxRenderedAttempts = renderedAttemptsHead +
renderedAttemptsTail
)
// renderedAttempts projects a delivery's attempts through the
// target's redactor, at most maxRenderedAttempts of them, and
// reports how many it dropped.
func renderedAttempts(
rows []deliveryResultRow,
redactor delivery.Redactor,
) ([]DeliveryResultView, int) {
omitted := 0
if len(rows) > maxRenderedAttempts {
omitted = len(rows) - maxRenderedAttempts
kept := make(
[]deliveryResultRow, 0, maxRenderedAttempts,
)
kept = append(kept, rows[:renderedAttemptsHead]...)
kept = append(
kept, rows[len(rows)-renderedAttemptsTail:]...,
)
rows = kept
}
views := make([]DeliveryResultView, len(rows))
for i := range rows {
views[i] = rows[i].view(redactor)
}
return views, omitted
}
// HandleEntrypointCreate handles adding a new entrypoint.
func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {

View File

@@ -43,10 +43,7 @@ func (s *Server) serveUntilShutdown() {
err := s.httpServer.ListenAndServe()
if err != nil && !errors.Is(err, http.ErrServerClosed) {
s.log.Error("listen error", "error", err)
if s.cancelFunc != nil {
s.cancelFunc()
}
s.shutdownOnListenFailure()
}
}

View File

@@ -0,0 +1,95 @@
package server_test
import (
"context"
"net"
"testing"
"time"
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/server"
)
// listenFailureDeadline is how long the app gets to give up after a
// listen it cannot satisfy. The defect this pins left the process
// reporting RUNNING for 183 seconds with nothing bound; a bind error
// is known instantly, so anything past a moment here is that defect
// back.
const listenFailureDeadline = 2 * time.Second
// lifecycleTimeout bounds the app's start and stop sequences so a
// wedged hook fails the test instead of hanging it.
const lifecycleTimeout = 15 * time.Second
// TestListenFailure_ShutsDownTheApp pins that a listener the server
// cannot bind terminates the application with a non-zero status.
//
// The fx OnStart hook returns as soon as the serving goroutine is
// spawned, so a bind failure is discovered after fx has already
// reported RUNNING. Nothing else in the graph observes it, and the
// process used to stay alive with no listener: down, but indis-
// tinguishable from healthy to systemd's Restart=on-failure and to
// Docker's restart policies, which is the state this test exists to
// keep from returning.
//
// The port is occupied by a listener this test holds open, on a
// kernel-chosen port, so the failure is the real EADDRINUSE the
// operator hits when a second instance starts. Loopback is enough to
// collide with the server's wildcard bind: a listening socket on a
// specific address blocks the wildcard from claiming the same port.
func TestListenFailure_ShutsDownTheApp(t *testing.T) {
t.Parallel()
var listenCfg net.ListenConfig
occupied, err := listenCfg.Listen(
t.Context(), "tcp", "127.0.0.1:0",
)
require.NoError(t, err)
t.Cleanup(func() { _ = occupied.Close() })
addr, ok := occupied.Addr().(*net.TCPAddr)
require.True(t, ok, "listener is not TCP")
// The collaborators come from the wired graph rather than stubs,
// so the Server under test is the one that ships. Only the port
// is test-specific.
env := newTestEnv(t)
env.cfg.Port = addr.Port
app := fx.New(
fx.NopLogger,
fx.Supply(env.log, env.cfg, env.mw, env.hnd),
fx.Provide(globals.New, server.New),
fx.Invoke(func(*server.Server) {}),
)
startCtx, cancelStart := context.WithTimeout(
context.Background(), lifecycleTimeout,
)
defer cancelStart()
require.NoError(t, app.Start(startCtx))
select {
case sig := <-app.Wait():
require.Equal(
t, server.ListenFailureExitCode, sig.ExitCode,
"listen failure must exit non-zero",
)
case <-time.After(listenFailureDeadline):
t.Fatal("listen failure left the app running")
}
// The stop sequence still has to complete: the fix must reach
// shutdown through fx rather than around it.
stopCtx, cancelStop := context.WithTimeout(
context.Background(), lifecycleTimeout,
)
defer cancelStop()
require.NoError(t, app.Stop(stopCtx))
}

View File

@@ -55,8 +55,11 @@ func (s *Server) setupGlobalMiddleware() {
s.router.Use(s.mw.SecurityHeaders())
s.router.Use(s.mw.Logging())
// Metrics middleware (only if credentials are configured)
if s.params.Config.MetricsUsername != "" {
// Metrics recording middleware, registered only when the
// endpoint that exposes what it records is served. The
// condition is the same MetricsAuthEnabled the /metrics mount
// in setupRoutes reads.
if s.params.Config.MetricsAuthEnabled() {
s.router.Use(s.mw.Metrics())
}
@@ -103,8 +106,14 @@ func (s *Server) setupRoutes() {
s.h.HandleHealthCheck(),
)
// set up authenticated /metrics route:
if s.params.Config.MetricsUsername != "" {
// Authenticated /metrics route. The condition is
// Config.MetricsAuthEnabled and never the username alone: a
// username with an empty password would otherwise mount the
// endpoint behind a credential map that accepts an empty
// password. Config rejects that combination at startup, and
// this reads the same value the startup log reports, so the
// two cannot disagree about whether the route exists.
if s.params.Config.MetricsAuthEnabled() {
s.router.Group(func(r chi.Router) {
r.Use(s.mw.MetricsAuth())
r.Get(

View File

@@ -34,6 +34,13 @@ import (
// the CSRF middleware executed.
const csrfCookieName = "_gorilla_csrf"
const (
// metricsUser and metricsAuthValue are the /metrics basic-auth
// credentials the metrics routing tests below configure.
metricsUser = "metrics"
metricsAuthValue = "s3cret"
)
type noopNotifier struct{}
func (n *noopNotifier) Notify([]delivery.Task) {}
@@ -69,9 +76,23 @@ type testEnv struct {
func newTestEnv(t *testing.T) *testEnv {
t.Helper()
return newTestEnvWithConfig(t, &config.Config{
DataDir: t.TempDir(),
Environment: config.EnvironmentDev,
})
}
// newTestEnvWithConfig is newTestEnv over a caller-supplied Config,
// for the routes whose existence the configuration decides. The same
// pointer reaches the router and every middleware, so a test cannot
// accidentally configure one and not the other.
func newTestEnvWithConfig(
t *testing.T, cfg *config.Config,
) *testEnv {
t.Helper()
var (
log *logger.Logger
cfg *config.Config
mw *middleware.Middleware
hnd *handlers.Handlers
sess *session.Session
@@ -84,12 +105,7 @@ func newTestEnv(t *testing.T) *testEnv {
fx.Provide(
globals.New,
logger.New,
func() *config.Config {
return &config.Config{
DataDir: t.TempDir(),
Environment: config.EnvironmentDev,
}
},
func() *config.Config { return cfg },
database.New,
database.NewWebhookDBManager,
healthcheck.New,
@@ -99,7 +115,7 @@ func newTestEnv(t *testing.T) *testEnv {
middleware.New,
handlers.New,
),
fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db, &dbMgr),
fx.Populate(&log, &mw, &hnd, &sess, &db, &dbMgr),
)
app.RequireStart()
t.Cleanup(app.RequireStop)
@@ -657,3 +673,119 @@ func TestSourceLogsBody_OtherUser404s(t *testing.T) {
assert.Equal(t, http.StatusSeeOther, anon.Code)
assert.Equal(t, "/pages/login", anon.Header().Get("Location"))
}
// metricsConfig is a Config differing from the routing default only
// in the two /metrics credentials.
func metricsConfig(
t *testing.T, username, password string,
) *config.Config {
t.Helper()
return &config.Config{
DataDir: t.TempDir(),
Environment: config.EnvironmentDev,
MetricsUsername: username,
MetricsPassword: password,
}
}
// metricsRequest asks the real router for /metrics with the given
// basic-auth credentials, or with no Authorization header when
// username is empty.
func (e *testEnv) metricsRequest(
username, password string,
) *httptest.ResponseRecorder {
req := httptest.NewRequestWithContext(
context.Background(), http.MethodGet, "/metrics", nil,
)
if username != "" {
req.SetBasicAuth(username, password)
}
w := httptest.NewRecorder()
e.router.ServeHTTP(w, req)
return w
}
// TestMetricsRouteUnmountedWithoutCredentials pins that with neither
// credential configured the route does not exist, which is the
// documented behaviour and the only valid way for /metrics to be
// absent.
func TestMetricsRouteUnmountedWithoutCredentials(t *testing.T) {
t.Parallel()
env := newTestEnvWithConfig(t, metricsConfig(t, "", ""))
assert.Equal(
t, http.StatusNotFound,
env.metricsRequest("", "").Code,
)
}
// TestMetricsRouteRequiresCredentials pins that with both credentials
// configured the route exists and every request that does not carry
// the configured pair is refused — including the empty password that
// a half-set configuration used to make sufficient.
func TestMetricsRouteRequiresCredentials(t *testing.T) {
t.Parallel()
env := newTestEnvWithConfig(
t, metricsConfig(t, metricsUser, metricsAuthValue),
)
assert.Equal(
t, http.StatusUnauthorized,
env.metricsRequest("", "").Code,
"no credentials must not reach the metrics handler",
)
assert.Equal(
t, http.StatusUnauthorized,
env.metricsRequest(metricsUser, "").Code,
"an empty password must not reach the metrics handler",
)
assert.Equal(
t, http.StatusUnauthorized,
env.metricsRequest(metricsUser, "wrong").Code,
)
ok := env.metricsRequest(metricsUser, metricsAuthValue)
assert.Equal(t, http.StatusOK, ok.Code)
assert.Contains(t, ok.Body.String(), "go_goroutines")
}
// TestMetricsRouteUnmountedOnHalfSetConfig pins the defect from
// https://git.eeqj.de/sneak/webhooker/issues/205 at the routing
// layer. Config rejects a half-set pair at startup, so this Config
// cannot be reached from the environment; the assertion is that the
// route tree does not publish an endpoint accepting an empty
// password even when handed one anyway, because the mount and the
// startup log's hasMetricsAuth read the same value.
func TestMetricsRouteUnmountedOnHalfSetConfig(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
username string
password string
}{
{name: "username only", username: metricsUser},
{name: "password only", password: metricsAuthValue},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
cfg := metricsConfig(t, tc.username, tc.password)
env := newTestEnvWithConfig(t, cfg)
assert.False(t, cfg.MetricsAuthEnabled())
assert.Equal(
t, http.StatusNotFound,
env.metricsRequest(
tc.username, tc.password,
).Code,
)
})
}
}

View File

@@ -50,6 +50,13 @@ const (
minSentryFlush = 250 * time.Millisecond
)
// ListenFailureExitCode is the status the process exits with when the
// HTTP listener cannot be established, or dies for a reason other
// than a requested shutdown. It must stay non-zero: systemd
// `Restart=on-failure` and Docker's restart policies key off it, and a
// zero exit would read as a deliberate stop.
const ListenFailureExitCode = 1
// SentryFlushBudget reports how long the Sentry flush may run when
// remaining is the time left on the fx stop context after the HTTP
// drain. sentry.Flush takes a bare duration and honours no context,
@@ -75,13 +82,13 @@ type ServerParams struct {
Config *config.Config
Middleware *middleware.Middleware
Handlers *handlers.Handlers
Shutdowner fx.Shutdowner
}
// Server is the main HTTP server that wires up routes and manages
// graceful shutdown.
type Server struct {
startupTime time.Time
exitCode int
sentryEnabled bool
log *slog.Logger
cancelFunc context.CancelFunc
@@ -159,7 +166,12 @@ func (s *Server) enableSentry() {
s.sentryEnabled = true
}
func (s *Server) serve() int {
// serve installs the signal watcher, starts the listener and blocks
// until the server's context is cancelled. The process exit status is
// fx's to decide — from a signal, or from the code
// shutdownOnListenFailure hands the Shutdowner — so this reports
// nothing back to its caller.
func (s *Server) serve() {
ctx, cancelFunc := context.WithCancel(context.Background())
s.cancelFunc = cancelFunc
@@ -185,7 +197,30 @@ func (s *Server) serve() int {
<-ctx.Done()
// Shutdown is handled by the fx OnStop hook (cleanShutdown).
// Do not call cleanShutdown() here to avoid double invocation.
return s.exitCode
}
// shutdownOnListenFailure ends the application after the HTTP
// listener failed. The fx OnStart hook returns as soon as the serving
// goroutine is spawned, so nothing downstream of it ever learns that
// the listen failed: fx reports RUNNING and the process sits alive
// with nothing bound, which is invisible to systemd and Docker
// restart policies. Asking the Shutdowner to stop the app with a
// non-zero code is what turns that into a visible failure.
//
// The context cancel that follows only unwinds serve()'s own wait.
// The shutdown itself runs through fx's normal stop sequence, so the
// clean-shutdown drain in cleanShutdown is reached unchanged.
func (s *Server) shutdownOnListenFailure() {
err := s.params.Shutdowner.Shutdown(
fx.ExitCode(ListenFailureExitCode),
)
if err != nil {
s.log.Error("shutdown request failed", "error", err)
}
if s.cancelFunc != nil {
s.cancelFunc()
}
}
func (s *Server) cleanupForExit() {
@@ -193,9 +228,6 @@ func (s *Server) cleanupForExit() {
}
func (s *Server) cleanShutdown(ctx context.Context) {
// initiate clean shutdown
s.exitCode = 0
ctxShutdown, shutdownCancel := context.WithTimeout(
ctx, ShutdownTimeout,
)

File diff suppressed because one or more lines are too long

View File

@@ -40,6 +40,57 @@
{{if .BodyTruncated}}
<p class="mt-2 text-xs text-gray-500">Body truncated for display: showing {{.BodyShownBytes}} of {{.BodyBytes}} bytes. The stored body is unchanged &mdash; <a href="/source/{{$.Webhook.ID}}/logs/{{.ID}}/body" class="text-primary-600 hover:text-primary-700 underline">download the full body</a>.</p>
{{end}}
{{if .Deliveries}}
<div class="mt-4 border-t border-gray-200 pt-3">
<h3 class="text-xs font-medium uppercase tracking-wide text-gray-500">Deliveries</h3>
<div class="mt-2 divide-y divide-gray-200">
{{range .Deliveries}}
<div class="py-2" x-data="{ attempts: false }">
<div class="flex items-center justify-between cursor-pointer" @click="attempts = !attempts">
<div class="flex items-center gap-3">
<span class="text-sm text-gray-700">{{.Target.Name}}</span>
<span class="text-xs {{if eq .Status "delivered"}}text-green-600{{else if eq .Status "failed"}}text-red-600{{else if eq .Status "retrying"}}text-yellow-600{{else}}text-gray-400{{end}}">{{.Status}}</span>
</div>
<div class="flex items-center gap-2">
<span class="text-xs text-gray-400">{{.AttemptCount}} attempt{{if ne .AttemptCount 1}}s{{end}}</span>
<svg class="w-3 h-3 text-gray-400 transition-transform" :class="{ 'rotate-180': attempts }" fill="none" stroke="currentColor" viewBox="0 0 24 24">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7"/>
</svg>
</div>
</div>
<div x-show="attempts" x-cloak class="mt-2 space-y-2">
{{if .AttemptsOmitted}}
<p class="text-xs text-gray-500">{{.AttemptsOmitted}} attempts omitted between the first and last shown.</p>
{{end}}
{{range .Results}}
<div class="rounded-md bg-white border border-gray-200 p-2">
<div class="flex flex-wrap items-center gap-3 text-xs">
<span class="text-gray-500">Attempt {{.AttemptNum}}</span>
<span class="{{if .Success}}text-green-600{{else}}text-red-600{{end}}">{{if .Success}}success{{else}}failure{{end}}</span>
<span class="text-gray-500">Status: {{if .HasStatusCode}}{{.StatusCode}}{{else}}&mdash; (no response){{end}}</span>
<span class="text-gray-500">Duration: {{.DurationMS}} ms</span>
</div>
{{if .Error}}
<p class="mt-2 text-xs text-red-700 break-all">Error: {{.Error}}</p>
{{end}}
{{if .ResponseBody}}
<pre class="mt-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all">{{.ResponseBody}}</pre>
{{end}}
{{if .ResponseTruncated}}
<p class="mt-1 text-xs text-gray-500">Response truncated for display: showing {{.ResponseShownBytes}} of {{.ResponseBytes}} bytes.</p>
{{end}}
</div>
{{else}}
<p class="text-xs text-gray-500">No attempts recorded yet.</p>
{{end}}
</div>
</div>
{{end}}
</div>
</div>
{{end}}
</div>
</div>
{{else}}