Compare commits
3 Commits
d9e8e28846
...
70f0fca5a2
| Author | SHA1 | Date | |
|---|---|---|---|
| 70f0fca5a2 | |||
| a13e5b7ded | |||
| bb30b3ad64 |
26
README.md
26
README.md
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
|
||||
247
internal/delivery/target_redact.go
Normal file
247
internal/delivery/target_redact.go
Normal 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
|
||||
}
|
||||
237
internal/delivery/target_redact_test.go
Normal file
237
internal/delivery/target_redact_test.go
Normal 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,
|
||||
)
|
||||
}
|
||||
}
|
||||
137
internal/handlers/delivery_result_view.go
Normal file
137
internal/handlers/delivery_result_view.go
Normal 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,
|
||||
}
|
||||
}
|
||||
458
internal/handlers/delivery_result_view_test.go
Normal file
458
internal/handlers/delivery_result_view_test.go
Normal 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",
|
||||
)
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
95
internal/server/listen_failure_test.go
Normal file
95
internal/server/listen_failure_test.go
Normal 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))
|
||||
}
|
||||
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
@@ -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 — <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}}— (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}}
|
||||
|
||||
Reference in New Issue
Block a user