Compare commits
1 Commits
70f0fca5a2
...
d9e8e28846
| Author | SHA1 | Date | |
|---|---|---|---|
| d9e8e28846 |
26
README.md
26
README.md
@@ -107,26 +107,14 @@ 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`. 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 | `""` |
|
||||
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
|
||||
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
|
||||
| `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
|
||||
@@ -1694,7 +1682,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` 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 |
|
||||
| `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 |
|
||||
|
||||
#### API (Planned)
|
||||
|
||||
@@ -1849,8 +1837,7 @@ 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` and
|
||||
`METRICS_PASSWORD` are both set)
|
||||
4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` is set)
|
||||
5. **CORS** — Cross-origin resource sharing headers
|
||||
6. **Timeout** — 60-second request timeout
|
||||
7. **Recoverer** — Panic recovery: one `ERROR` record through
|
||||
@@ -1882,9 +1869,8 @@ 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 the `/metrics`
|
||||
credentials are set and the Metrics middleware is therefore registered
|
||||
at all. The
|
||||
counted in the metrics, on a deployment where `METRICS_USERNAME` is
|
||||
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
|
||||
|
||||
@@ -71,16 +71,6 @@ 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
|
||||
@@ -138,21 +128,6 @@ 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 {
|
||||
@@ -354,30 +329,6 @@ 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) {
|
||||
@@ -455,18 +406,13 @@ 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: metricsUsername,
|
||||
MetricsPassword: metricsPassword,
|
||||
MetricsUsername: envString("METRICS_USERNAME"),
|
||||
MetricsPassword: envString("METRICS_PASSWORD"),
|
||||
Port: port,
|
||||
SentryDSN: envString("SENTRY_DSN"),
|
||||
RetentionSweepInterval: retentionSweepInterval,
|
||||
@@ -566,7 +512,8 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
|
||||
"receiverRateLimit", s.ReceiverRateLimit,
|
||||
"trustedProxies", len(s.TrustedProxies),
|
||||
"hasSentryDSN", s.SentryDSN != "",
|
||||
"hasMetricsAuth", s.MetricsAuthEnabled(),
|
||||
"hasMetricsAuth",
|
||||
s.MetricsUsername != "" && s.MetricsPassword != "",
|
||||
)
|
||||
|
||||
s.warnSharedRateLimitBucket(log)
|
||||
|
||||
@@ -26,12 +26,6 @@ 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) {
|
||||
@@ -732,168 +726,3 @@ 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())
|
||||
}
|
||||
|
||||
@@ -2,7 +2,6 @@ package delivery
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
@@ -17,12 +16,11 @@ const RedactionMarker = "(redacted)"
|
||||
// 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.
|
||||
// It matches literally, against strings taken from the
|
||||
// target's stored configuration, so it guesses nothing about
|
||||
// what a secret looks like. That also bounds what it can
|
||||
// promise: it removes the credential this service handed the
|
||||
// remote, and 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.
|
||||
@@ -32,21 +30,7 @@ type Redactor struct {
|
||||
|
||||
// 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}
|
||||
return Redactor{secrets: targetSecrets(t)}
|
||||
}
|
||||
|
||||
// Redact replaces every occurrence of the target's credential
|
||||
@@ -63,61 +47,24 @@ func (r Redactor) Redact(s string) string {
|
||||
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.
|
||||
// target's configuration carries, longest first so that
|
||||
// replacing one never leaves a fragment of a longer one
|
||||
// behind.
|
||||
//
|
||||
// The destination URL contributes. Its path, query and
|
||||
// Only 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.
|
||||
// Configured HTTP headers are deliberately not included.
|
||||
// Their values are as often routine as secret (Accept,
|
||||
// User-Agent), and redacting them from remote text would
|
||||
// replace ordinary response content with the marker. A remote
|
||||
// that echoes an Authorization header into its response body
|
||||
// is therefore not covered.
|
||||
func targetSecrets(t *database.Target) []string {
|
||||
if t == nil {
|
||||
return nil
|
||||
@@ -137,10 +84,7 @@ func targetSecrets(t *database.Target) []string {
|
||||
return nil
|
||||
}
|
||||
|
||||
return append(
|
||||
urlSecrets(cfg.URL),
|
||||
headerSecrets(cfg.Headers)...,
|
||||
)
|
||||
return urlSecrets(cfg.URL)
|
||||
case database.TargetTypeDatabase, database.TargetTypeLog:
|
||||
// Neither has a destination URL, so neither has
|
||||
// anything to redact.
|
||||
@@ -190,58 +134,3 @@ func urlSecrets(raw string) []string {
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
@@ -29,108 +29,8 @@ func TestRedactor_RemovesSlackWebhookURL(t *testing.T) {
|
||||
|
||||
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))
|
||||
assert.Contains(t, got, delivery.RedactionMarker)
|
||||
assert.Contains(t, got, "no_service for ")
|
||||
}
|
||||
|
||||
// TestRedactor_RemovesBarePath covers a remote that echoes
|
||||
|
||||
@@ -112,24 +112,13 @@ func (r *deliveryResultRow) view(
|
||||
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,
|
||||
ResponseBody: redactor.Redact(string(body)),
|
||||
ResponseBytes: r.ResponseBytes,
|
||||
ResponseShownBytes: len(body),
|
||||
ResponseTruncated: truncated,
|
||||
|
||||
@@ -249,158 +249,6 @@ func TestHandleSourceLogs_RedactsCredentialEchoedInError(
|
||||
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
|
||||
|
||||
@@ -23,10 +23,6 @@ const MaxRenderedBodyBytesForTest = maxRenderedBodyBytes
|
||||
// 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
|
||||
|
||||
@@ -4,7 +4,6 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
@@ -103,20 +102,10 @@ type DeliveryView struct {
|
||||
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 is every recorded attempt at this delivery, in
|
||||
// attempt order. Without it 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
|
||||
@@ -793,16 +782,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
targets := h.loadTargetMap(webhook.ID)
|
||||
page := h.parsePage(r)
|
||||
|
||||
evts, total := h.loadEventsWithDeliveries(
|
||||
@@ -835,50 +815,34 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
|
||||
// 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]eventLogTarget, error) {
|
||||
) map[string]eventLogTarget {
|
||||
var targets []database.Target
|
||||
|
||||
err := h.db.DB().Unscoped().Where(
|
||||
h.db.DB().Where(
|
||||
"webhook_id = ?", webhookID,
|
||||
).Find(&targets).Error
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
).Find(&targets)
|
||||
|
||||
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) {
|
||||
for _, v := range delivery.NewTargetViews(targets) {
|
||||
entry := targetMap[v.ID]
|
||||
entry.View = v
|
||||
targetMap[v.ID] = entry
|
||||
}
|
||||
|
||||
return targetMap, nil
|
||||
return targetMap
|
||||
}
|
||||
|
||||
// parsePage extracts a page number from the query string.
|
||||
@@ -968,57 +932,38 @@ func (h *Handlers) loadEventsWithDeliveries(
|
||||
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.
|
||||
// loadDeliveryResults loads every recorded attempt for the
|
||||
// page's deliveries in one query, 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.
|
||||
// the reason deliveryResultColumns gives. What the cut does
|
||||
// not bound is how many attempts a delivery has: that is the
|
||||
// target's MaxRetries, which the authenticated operator sets
|
||||
// — the same class of operator-chosen dimension as the number
|
||||
// of targets a webhook has, which this page already accepts.
|
||||
// No part of it is chosen by the unauthenticated sender.
|
||||
func (h *Handlers) loadDeliveryResults(
|
||||
webhookDB *gorm.DB,
|
||||
deliveryIDs []string,
|
||||
) map[string][]deliveryResultRow {
|
||||
if len(deliveryIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
var rows []deliveryResultRow
|
||||
|
||||
webhookDB.Model(&database.DeliveryResult{}).Select(
|
||||
deliveryResultColumns, maxRenderedResponseBytes,
|
||||
).Where(
|
||||
"delivery_id IN ?", deliveryIDs,
|
||||
).Order("attempt_num ASC").Find(&rows)
|
||||
|
||||
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],
|
||||
)
|
||||
}
|
||||
for i := range rows {
|
||||
byDelivery[rows[i].DeliveryID] = append(
|
||||
byDelivery[rows[i].DeliveryID], rows[i],
|
||||
)
|
||||
}
|
||||
|
||||
return byDelivery
|
||||
@@ -1038,66 +983,22 @@ func newDeliveryViews(
|
||||
target := targetMap[deliveries[i].TargetID]
|
||||
rows := attempts[deliveries[i].ID]
|
||||
|
||||
results, omitted := renderedAttempts(
|
||||
rows, target.Redactor,
|
||||
)
|
||||
results := make([]DeliveryResultView, len(rows))
|
||||
for j := range rows {
|
||||
results[j] = rows[j].view(target.Redactor)
|
||||
}
|
||||
|
||||
views[i] = DeliveryView{
|
||||
ID: deliveries[i].ID,
|
||||
Status: deliveries[i].Status,
|
||||
Target: target.View,
|
||||
Results: results,
|
||||
AttemptCount: len(rows),
|
||||
AttemptsOmitted: omitted,
|
||||
ID: deliveries[i].ID,
|
||||
Status: deliveries[i].Status,
|
||||
Target: target.View,
|
||||
Results: results,
|
||||
}
|
||||
}
|
||||
|
||||
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) {
|
||||
|
||||
@@ -43,7 +43,10 @@ func (s *Server) serveUntilShutdown() {
|
||||
err := s.httpServer.ListenAndServe()
|
||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||
s.log.Error("listen error", "error", err)
|
||||
s.shutdownOnListenFailure()
|
||||
|
||||
if s.cancelFunc != nil {
|
||||
s.cancelFunc()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,95 +0,0 @@
|
||||
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))
|
||||
}
|
||||
@@ -55,11 +55,8 @@ func (s *Server) setupGlobalMiddleware() {
|
||||
s.router.Use(s.mw.SecurityHeaders())
|
||||
s.router.Use(s.mw.Logging())
|
||||
|
||||
// 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() {
|
||||
// Metrics middleware (only if credentials are configured)
|
||||
if s.params.Config.MetricsUsername != "" {
|
||||
s.router.Use(s.mw.Metrics())
|
||||
}
|
||||
|
||||
@@ -106,14 +103,8 @@ func (s *Server) setupRoutes() {
|
||||
s.h.HandleHealthCheck(),
|
||||
)
|
||||
|
||||
// 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() {
|
||||
// set up authenticated /metrics route:
|
||||
if s.params.Config.MetricsUsername != "" {
|
||||
s.router.Group(func(r chi.Router) {
|
||||
r.Use(s.mw.MetricsAuth())
|
||||
r.Get(
|
||||
|
||||
@@ -34,13 +34,6 @@ 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) {}
|
||||
@@ -76,23 +69,9 @@ 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
|
||||
@@ -105,7 +84,12 @@ func newTestEnvWithConfig(
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
func() *config.Config { return cfg },
|
||||
func() *config.Config {
|
||||
return &config.Config{
|
||||
DataDir: t.TempDir(),
|
||||
Environment: config.EnvironmentDev,
|
||||
}
|
||||
},
|
||||
database.New,
|
||||
database.NewWebhookDBManager,
|
||||
healthcheck.New,
|
||||
@@ -115,7 +99,7 @@ func newTestEnvWithConfig(
|
||||
middleware.New,
|
||||
handlers.New,
|
||||
),
|
||||
fx.Populate(&log, &mw, &hnd, &sess, &db, &dbMgr),
|
||||
fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db, &dbMgr),
|
||||
)
|
||||
app.RequireStart()
|
||||
t.Cleanup(app.RequireStop)
|
||||
@@ -673,119 +657,3 @@ 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,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -50,13 +50,6 @@ 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,
|
||||
@@ -82,13 +75,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
|
||||
@@ -166,12 +159,7 @@ func (s *Server) enableSentry() {
|
||||
s.sentryEnabled = true
|
||||
}
|
||||
|
||||
// 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() {
|
||||
func (s *Server) serve() int {
|
||||
ctx, cancelFunc := context.WithCancel(context.Background())
|
||||
s.cancelFunc = cancelFunc
|
||||
|
||||
@@ -197,30 +185,7 @@ func (s *Server) serve() {
|
||||
<-ctx.Done()
|
||||
// Shutdown is handled by the fx OnStop hook (cleanShutdown).
|
||||
// Do not call cleanShutdown() here to avoid double invocation.
|
||||
}
|
||||
|
||||
// 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()
|
||||
}
|
||||
return s.exitCode
|
||||
}
|
||||
|
||||
func (s *Server) cleanupForExit() {
|
||||
@@ -228,6 +193,9 @@ 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
@@ -53,7 +53,7 @@
|
||||
<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>
|
||||
<span class="text-xs text-gray-400">{{len .Results}} attempt{{if ne (len .Results) 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>
|
||||
@@ -61,9 +61,6 @@
|
||||
</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">
|
||||
|
||||
Reference in New Issue
Block a user