Compare commits
1
Commits
next
..
3f8afa2f28
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3f8afa2f28 |
+23
-4
@@ -76,6 +76,8 @@ RUN --network=none node_modules/.bin/prettier --check '**/*.md'
|
||||
# depends on it.
|
||||
#
|
||||
# golangci/golangci-lint:v2.14.0 (Debian-based), 2026-09-24
|
||||
# Using Debian-based image because mattn/go-sqlite3 (CGO) does not
|
||||
# compile on Alpine musl (off64_t is a glibc type).
|
||||
FROM golangci/golangci-lint:v2.14.0@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint
|
||||
|
||||
WORKDIR /src
|
||||
@@ -165,7 +167,9 @@ RUN --mount=type=tmpfs,target=/tmp/tests,size=512m \
|
||||
|
||||
# Build stage
|
||||
# golang:1.26.1-bookworm (Debian-based), 2026-03-17
|
||||
# The image ships git and make, which the version step below uses.
|
||||
# Using Debian-based image because gorm.io/driver/sqlite pulls in
|
||||
# mattn/go-sqlite3 (CGO), which does not compile on Alpine musl. The image
|
||||
# ships git and make, which the version step below uses.
|
||||
FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a3492282a6c820bf4755fd64a4 AS builder
|
||||
|
||||
# Nothing is wanted from the lint and test phases or from the stylesheet and
|
||||
@@ -207,11 +211,26 @@ RUN version="$(make version VERSION="$VERSION")"; \
|
||||
fi
|
||||
|
||||
# Builds through the Makefile's build target, which runs script/assets
|
||||
# (Alpine.js, extracted from its tarball in 3p/) first. The target builds
|
||||
# with cgo off, so the binary is statically linked and runs on the Alpine
|
||||
# runtime image below as it is.
|
||||
# (Alpine.js, extracted from its tarball in 3p/) first.
|
||||
RUN make build VERSION="$VERSION"
|
||||
|
||||
# Rebuild with static linking for the Alpine runtime. The cgo binary the
|
||||
# step above builds is dynamically linked against glibc, which Alpine
|
||||
# (musl) does not have.
|
||||
#
|
||||
# A statically linked glibc still loads shared libraries at run time for
|
||||
# getaddrinfo and dlopen, and Alpine has none to load, so the build tags
|
||||
# keep both out of the binary: netgo and osusergo select Go's own DNS
|
||||
# resolver and user lookup instead of glibc's, and
|
||||
# sqlite_omit_load_extension leaves out SQLite extension loading, which
|
||||
# webhooker never uses.
|
||||
#
|
||||
# The static flags and tags go in through GO_LDFLAGS and GO_TAGS rather
|
||||
# than a -ldflags of their own: the build target composes them with the
|
||||
# -X that stamps the version, so this relink cannot silently drop the
|
||||
# stamp.
|
||||
RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_TAGS=netgo,osusergo,sqlite_omit_load_extension GO_LDFLAGS='-extldflags "-static"'
|
||||
|
||||
# Runtime stage
|
||||
# alpine:3.21, 2026-03-17
|
||||
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
|
||||
|
||||
@@ -16,6 +16,17 @@ VERSION ?= $(shell script/version)
|
||||
# exists to correct.
|
||||
override VERSION := $(or $(strip $(VERSION)),$(shell script/version))
|
||||
|
||||
# Extra linker flags and build tags for the build target. The static
|
||||
# relink in the Dockerfile passes -extldflags and its build tags through
|
||||
# these rather than a -ldflags of its own, so composing flags cannot drop
|
||||
# the version stamp.
|
||||
#
|
||||
# The build target itself always passes -trimpath and -s -w, as the Go
|
||||
# Dockerfile in REPO_POLICIES.md does: no build paths, symbol table or
|
||||
# debug information in the binary.
|
||||
GO_LDFLAGS ?=
|
||||
GO_TAGS ?=
|
||||
|
||||
bootstrap:
|
||||
@script/bootstrap
|
||||
|
||||
@@ -43,12 +54,8 @@ fmt-check:
|
||||
check:
|
||||
@script/check
|
||||
|
||||
# Built with cgo off, -trimpath and -s -w, as the Go Dockerfile in
|
||||
# REPO_POLICIES.md does: the binary carries no build paths, symbol table or
|
||||
# debug information, and on Linux, the image's included, it is statically
|
||||
# linked.
|
||||
build: assets
|
||||
CGO_ENABLED=0 go build -trimpath -ldflags '-s -w -X main.version=$(VERSION)' -o bin/webhooker ./cmd/webhooker
|
||||
go build -trimpath -tags '$(GO_TAGS)' -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
|
||||
|
||||
run: build
|
||||
./bin/webhooker
|
||||
|
||||
@@ -804,12 +804,7 @@ comes up, serves pages, and is broken in a way nothing reports.
|
||||
proxy's log names it regardless. nginx's default `combined` format already
|
||||
logs `$remote_addr`; do not replace it with one that drops the client
|
||||
address, and retain those logs as long as you would want to answer a question
|
||||
about traffic. That log also records the full request path, so it holds the
|
||||
entrypoint UUID of every event a sender posts. webhooker's own log does not,
|
||||
apart from the line a `log` target writes. Protect the proxy's log as you
|
||||
would the entrypoint URLs themselves: anyone who can read it can post events
|
||||
to every entrypoint in it (see
|
||||
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret)).
|
||||
about traffic.
|
||||
|
||||
### nginx
|
||||
|
||||
@@ -835,9 +830,7 @@ server {
|
||||
client_max_body_size 1m;
|
||||
|
||||
# $remote_addr is the client. webhooker's own log names it, as
|
||||
# clientIP, only while TRUSTED_PROXIES covers this proxy. The
|
||||
# request path in this log holds the entrypoint UUID, so protect
|
||||
# the file like the entrypoint URL itself.
|
||||
# clientIP, only while TRUSTED_PROXIES covers this proxy.
|
||||
access_log /var/log/nginx/webhooker.access.log combined;
|
||||
|
||||
location / {
|
||||
@@ -1169,16 +1162,7 @@ What that means for an operator:
|
||||
|
||||
- **The URL is a capability, so treat it as a secret.** Keep it out of logs,
|
||||
ticket bodies, chat messages and screenshots. Anyone who reads it anywhere can
|
||||
post events as that sender. Apart from a `log` target's line, webhooker's own
|
||||
log lines never carry the UUID: they name a request to the receiver by the
|
||||
route pattern `/h/{uuid}`, the `webhook request received` line names the
|
||||
entrypoint by its `entrypoint_id`, and the access log keeps only the scheme
|
||||
and host of a `Referer`. A `log` target writes the request's headers as
|
||||
received, so a `Referer` naming the entrypoint URL appears in its line, as it
|
||||
does when a sender has followed a redirect from `http://` to `https://`. The
|
||||
reverse proxy's access log does carry the UUID, in the request path, so it
|
||||
must be protected like the URL itself (see
|
||||
[Deployment behind a reverse proxy](#deployment-behind-a-reverse-proxy)).
|
||||
post events as that sender.
|
||||
- **Rotating means minting a new entrypoint, not changing a key.** There is no
|
||||
way to rotate the UUID in place. To retire one, delete the entrypoint (or
|
||||
deactivate it, which answers `410`) and create a new one, then point the
|
||||
@@ -1253,37 +1237,34 @@ markup. The CSP build runs no expressions, so every Alpine directive in
|
||||
`static/js/app.js`: `x-data="collapsible"` and `@click="toggle"`, never
|
||||
`x-data="{ open: false }"` or `@click="open = !open"`.
|
||||
|
||||
A browser test in `internal/server` loads the sign-in page, the webhook page,
|
||||
its edit pages, the event log, the webhook list, the settings page, the profile
|
||||
page and the `404` error page shown for a path that does not exist, under the
|
||||
real policy and checks that: the add entrypoint form stays hidden until Add is
|
||||
clicked; for every target type, the targets section's Add shows only a choice of
|
||||
type with Next and Cancel, Next shows only that type's fields (no url field for
|
||||
`database` or `log`), Cancel at either step closes the form, and saving adds the
|
||||
target; a refused target comes back with its form open, the values entered and
|
||||
the reason, and after Cancel the next Add starts with an empty form and no
|
||||
reason; a refused save on the target edit page and on the webhook edit page
|
||||
comes back with the reason and every value entered; the Copy button beside an
|
||||
entrypoint URL reads "Copied" once clicked; an entrypoint's Edit button shows
|
||||
its edit form in place of its description and hides until the form closes,
|
||||
Cancel hides the form and drops what was typed, as does leaving the page and
|
||||
going back to it, and Save changes the description; of the recent events on the
|
||||
webhook page only the newest starts expanded, each expands and collapses, and
|
||||
Open leads to the event's own page; of the events in the event log only the
|
||||
newest starts expanded, and an event there expands and collapses when its row's
|
||||
caret or its ID is clicked, and from the keyboard, but not when its ID is
|
||||
selected with the mouse, and a delivery's attempts inside it expand and
|
||||
collapse; at phone width the menu button opens and closes the mobile menu, and
|
||||
neither the webhook page nor the event log, with a delivery's attempts open,
|
||||
scrolls sideways or cuts anything off at the page's or a card's edge; and a
|
||||
target's Delete asks first, naming the target, sends nothing when the answer is
|
||||
no and deletes the target when it is yes. It also fails if the browser reports a
|
||||
console warning or error, an uncaught exception, or anything the policy refused.
|
||||
`make check` and the image build lint it but do not run it, and `make test`
|
||||
leaves it out (its file is built only with the `browser` build tag). Run it with
|
||||
`make test-browser` after changing `templates/` or `static/js/`: that builds
|
||||
`Dockerfile.browser`, which runs the test in a digest-pinned headless browser
|
||||
image, so the host needs no browser.
|
||||
A browser test in `internal/server` loads the webhook page, its edit pages and
|
||||
the event log under the real policy and checks that: the add entrypoint form
|
||||
stays hidden until Add is clicked; for every target type, the targets section's
|
||||
Add shows only a choice of type with Next and Cancel, Next shows only that
|
||||
type's fields (no url field for `database` or `log`), Cancel at either step
|
||||
closes the form, and saving adds the target; a refused target comes back with
|
||||
its form open, the values entered and the reason, and after Cancel the next Add
|
||||
starts with an empty form and no reason; a refused save on the target edit page
|
||||
and on the webhook edit page comes back with the reason and every value entered;
|
||||
the Copy button beside an entrypoint URL reads "Copied" once clicked; an
|
||||
entrypoint's Edit button shows its edit form in place of its description and
|
||||
hides until the form closes, Cancel hides the form and drops what was typed, as
|
||||
does leaving the page and going back to it, and Save changes the description; of
|
||||
the recent events on the webhook page only the newest starts expanded, each
|
||||
expands and collapses, and Open leads to the event's own page; of the events in
|
||||
the event log only the newest starts expanded, and an event there expands and
|
||||
collapses when its row's caret or its ID is clicked, and from the keyboard, but
|
||||
not when its ID is selected with the mouse, and a delivery's attempts inside it
|
||||
expand and collapse; and at phone width the menu button opens and closes the
|
||||
mobile menu, and neither the webhook page nor the event log, with a delivery's
|
||||
attempts open, scrolls sideways or cuts anything off at the page's or a card's
|
||||
edge. It also fails if the browser reports a console warning or error, an
|
||||
uncaught exception, or anything the policy refused. `make check` and the image
|
||||
build lint it but do not run it, and `make test` leaves it out (its file is
|
||||
built only with the `browser` build tag). Run it with `make test-browser` after
|
||||
changing `templates/` or `static/js/`: that builds `Dockerfile.browser`, which
|
||||
runs the test in a digest-pinned headless browser image, so the host needs no
|
||||
browser.
|
||||
|
||||
The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
|
||||
byte as the npm registry publishes it. It is a dependency, not this repo's build
|
||||
@@ -1316,9 +1297,7 @@ and commit the regenerated file with the change. Tailwind takes classes only
|
||||
from the files that `input.css` names in its `@source` lines; a class written in
|
||||
any other file is not generated until that file is named there too. `make check`
|
||||
and the image build fail when the committed file differs from what `make css`
|
||||
generates. `static/css/style.css` is hand-written and is not generated. The
|
||||
pages' Content-Security-Policy refuses inline styles, so a template styles an
|
||||
element with a class, never a `style` attribute.
|
||||
generates. `static/css/style.css` is hand-written and is not generated.
|
||||
|
||||
`make css` runs the Tailwind standalone CLI in Docker, at the version and sha256
|
||||
pinned in the Dockerfile's stylesheet stages; it is never installed on the host.
|
||||
@@ -1393,10 +1372,10 @@ It uses:
|
||||
management
|
||||
- **[go-chi](https://github.com/go-chi/chi)** for HTTP routing
|
||||
- **[GORM](https://gorm.io)** for database access with
|
||||
**[modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite)**, which is pure
|
||||
Go, as the SQLite driver. `gorm.io/driver/sqlite` also imports
|
||||
`mattn/go-sqlite3`, whose stub for builds without cgo is linked into the
|
||||
binary but never used
|
||||
**[modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite)** as the runtime
|
||||
SQLite driver. Note: `gorm.io/driver/sqlite` transitively depends on
|
||||
`mattn/go-sqlite3`, which requires CGO at build time (see [Docker](#docker)
|
||||
section)
|
||||
- **[slog](https://pkg.go.dev/log/slog)** (stdlib) for structured logging with
|
||||
TTY detection (text for dev, JSON for prod)
|
||||
- **[gorilla/sessions](https://github.com/gorilla/sessions)** for encrypted
|
||||
@@ -1591,27 +1570,29 @@ did not arrive on the URL and counts in neither.
|
||||
A delivery destination for events. Each target defines where and how events
|
||||
should be forwarded.
|
||||
|
||||
| Field | Type | Description |
|
||||
| -------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `id` | UUID | Primary key |
|
||||
| `webhook_id` | UUID | Foreign key → Webhook |
|
||||
| `name` | string | Human-readable name |
|
||||
| `type` | TargetType | One of: `http`, `slack`, `database`, `log` |
|
||||
| `active` | boolean | Whether deliveries are enabled (default: true) |
|
||||
| `config` | JSON text | Type-specific configuration |
|
||||
| `max_attempts` | integer | How many times an `http` or `slack` target attempts each delivery in all: at least 1, and 1 by default. Ignored by `database` and `log` targets. The web UI labels it Delivery attempts |
|
||||
| Field | Type | Description |
|
||||
| ------------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `id` | UUID | Primary key |
|
||||
| `webhook_id` | UUID | Foreign key → Webhook |
|
||||
| `name` | string | Human-readable name |
|
||||
| `type` | TargetType | One of: `http`, `slack`, `database`, `log` |
|
||||
| `active` | boolean | Whether deliveries are enabled (default: true) |
|
||||
| `config` | JSON text | Type-specific configuration |
|
||||
| `max_retries` | integer | Total delivery attempts for `http` and `slack` targets, not retries on top of the first: 0 is a single fire-and-forget attempt with no retries and no circuit breaker, and a value of N makes N attempts in all, with exponential backoff and a per-target circuit breaker. Ignored by `database` and `log` targets. The web UI labels it Delivery attempts |
|
||||
|
||||
**Relations:** Belongs to Webhook. Has many Deliveries.
|
||||
|
||||
**Target types:**
|
||||
|
||||
- **`http`** — Forward the event as an HTTP POST to a configured URL. With
|
||||
`max_attempts` at 1 the target makes a single attempt; above 1 it retries a
|
||||
failed delivery with exponential backoff, up to that many attempts in all,
|
||||
behind a per-target circuit breaker.
|
||||
- **`http`** — Forward the event as an HTTP POST to a configured URL.
|
||||
`max_retries` is the total number of delivery attempts, not retries on top of
|
||||
the first: when `max_retries` is 0 (the default), the target operates in
|
||||
fire-and-forget mode, a single attempt with no retries and no circuit breaker;
|
||||
a value of N makes up to N attempts in all, retrying failed deliveries with
|
||||
exponential backoff and protecting them with a per-target circuit breaker.
|
||||
- **`slack`** — Post the event as a formatted message to a Slack-compatible
|
||||
incoming webhook URL (`webhookUrl` in `config`). It is built on the same HTTP
|
||||
core as `http` and honours `max_attempts` identically, circuit breaker
|
||||
core as `http` and honours `max_retries` identically, circuit breaker
|
||||
included. See the Slack target section under "Per-Webhook Event Databases" for
|
||||
the message format.
|
||||
- **`database`** — Archive the full event as a row into the target's own archive
|
||||
@@ -2133,7 +2114,9 @@ endpoint. That is the JSON key; the error text for a missing one reads
|
||||
`webhook_url is required`, which is the message, not the key.
|
||||
|
||||
The database uses the
|
||||
[modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite) driver.
|
||||
[modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite) driver at runtime,
|
||||
though CGO is required at build time due to the transitive `mattn/go-sqlite3`
|
||||
dependency from `gorm.io/driver/sqlite`.
|
||||
|
||||
#### Schema
|
||||
|
||||
@@ -2184,11 +2167,11 @@ External Service
|
||||
▼ ▼ ▼
|
||||
┌────────────┐ ┌────────────┐ ┌────────────┐
|
||||
│ HTTP Target│ │ HTTP Target│ │ Log Target │
|
||||
│max_attempts│ │max_attempts│ │ (stdout) │
|
||||
│ == 1, │ │ > 1, │ └────────────┘
|
||||
│ one attempt│ │ backoff + │
|
||||
│(max_retries│ │(max_retries│ │ (stdout) │
|
||||
│ == 0) │ │ > 0, │ └────────────┘
|
||||
│ fire+forget│ │ backoff + │
|
||||
└────────────┘ │ circuit │
|
||||
│ breaker │
|
||||
│ breaker) │
|
||||
└────────────┘
|
||||
```
|
||||
|
||||
@@ -2265,7 +2248,7 @@ re-inject it to every currently active target.
|
||||
|
||||
### Circuit Breaker (HTTP and Slack Targets with Retries)
|
||||
|
||||
`http` and `slack` targets with `max_attempts` > 1 are protected by a
|
||||
`http` and `slack` targets with `max_retries` > 0 are protected by a
|
||||
**per-target circuit breaker** that prevents hammering a down target with
|
||||
repeated failed delivery attempts. The circuit breaker is in-memory only and
|
||||
resets on restart (which is fine — startup recovery rescans the database
|
||||
@@ -2306,10 +2289,10 @@ anyway).
|
||||
- **Cooldown:** 30 seconds in open state before probing
|
||||
|
||||
**Scope:** Circuit breakers apply to **`http` and `slack` targets with
|
||||
`max_attempts` > 1**. The Slack target is built on the same HTTP core and hands
|
||||
its own `max_attempts` to the same retry path, so it gets a breaker with the
|
||||
same 5-failure / 30-second defaults. Single-attempt targets of either type
|
||||
(`max_attempts` == 1), database targets (local operations), and log targets
|
||||
`max_retries` > 0**. The Slack target is built on the same HTTP core and hands
|
||||
its own `max_retries` to the same retry path, so it gets a breaker with the same
|
||||
5-failure / 30-second defaults. Fire-and-forget targets of either type
|
||||
(`max_retries` == 0), database targets (local operations), and log targets
|
||||
(stdout) do not use circuit breakers.
|
||||
|
||||
When a circuit is open and a new delivery arrives, the engine marks the delivery
|
||||
@@ -2441,32 +2424,27 @@ log volume it caps rather than eliminates. A path that names no entrypoint is
|
||||
recorded by the handler at `DEBUG`, and the aggregate limiter logs its own
|
||||
rejections at `DEBUG` and without the path, so neither appears at all under the
|
||||
default level. The per-entrypoint limiter is the loud one: it logs every
|
||||
rejection at `WARN`, naming the route pattern `/h/{uuid}` rather than the
|
||||
request path. A client hammering a single invented path is served
|
||||
rejection at `WARN` with the request path, which on this route is
|
||||
attacker-controlled text. A client hammering a single invented path is served
|
||||
`RECEIVER_RATE_LIMIT` requests and has the rest of its aggregate budget rejected
|
||||
there, so the aggregate limit is what bounds the _number_ of those `WARN` lines
|
||||
— to under ten times `RECEIVER_RATE_LIMIT` per minute per client IP, 1080 at the
|
||||
defaults, where before it there was no bound at all. The access log is bounded
|
||||
by neither limit: every request is recorded once at `INFO`, served or rejected
|
||||
alike.
|
||||
defaults, where before it there was no bound at all. Their _width_ is bounded by
|
||||
the field budgets below, the same ones the access log spends. The access log is
|
||||
bounded by neither limit: every request is recorded once at `INFO`, served or
|
||||
rejected alike.
|
||||
|
||||
What the access log does bound is the _content_ of those lines. On the receiver
|
||||
route it logs the route pattern `/h/{uuid}` whatever the status, because the
|
||||
concrete path carries the entrypoint UUID, which is the entrypoint's credential
|
||||
(see
|
||||
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret)).
|
||||
A request chi never routed, because it matched no route or because a middleware
|
||||
answered it first, logs the literal `(unmatched)` whatever the status. In the
|
||||
`dev` environment the CORS middleware answers a preflight that way, so a
|
||||
preflight to the receiver does not log its UUID either. On every other route, a
|
||||
3xx or 4xx response logs the chi route pattern, such as `/user/{username}//`, in
|
||||
place of the concrete URL. Those are the outcomes an unauthenticated client can
|
||||
drive for free: a 404 on any invented path, a login redirect on any invented
|
||||
profile path. Logging the URL there would let a flood write text of its own
|
||||
choosing, at a length of its own choosing, into the log. 2xx and 5xx responses
|
||||
keep the concrete path — a success resolved against a static route or against
|
||||
the operator's own data, and a 5xx is a bug in this service, where the exact
|
||||
path is the evidence and no client can provoke one at will.
|
||||
What the access log does bound is the _content_ of those lines. A 3xx or 4xx
|
||||
response logs the chi route pattern — `/h/{uuid}`, `/user/{username}//`, or the
|
||||
literal `(unmatched)` when the request hit no route at all — in place of the
|
||||
concrete URL. Those are the outcomes an unauthenticated client can drive for
|
||||
free: 404 and 429 on any invented receiver path, a login redirect on any
|
||||
invented profile path. Logging the URL there would let a flood write text of its
|
||||
own choosing, at a length of its own choosing, into the log. 2xx and 5xx
|
||||
responses keep the concrete path — a success resolved against a static route or
|
||||
against the operator's own data (on the receiver, against a stored entrypoint
|
||||
UUID), and a 5xx is a bug in this service, where the exact path is the evidence
|
||||
and no client can provoke one at will.
|
||||
|
||||
The query string is never logged; it is replaced by the fixed marker
|
||||
`?(redacted)`. It is client-chosen on every route, and
|
||||
@@ -2478,11 +2456,6 @@ to, `notice`, which names the line a page shows after an action, and the event
|
||||
log's `show`, which picks the events it lists. A query string sent to an
|
||||
entrypoint is not lost either: the event stores it, and the event log shows it.
|
||||
|
||||
The `referer` field keeps only the scheme and host of the request's `Referer`,
|
||||
and logs `(redacted)` for one without them, because its path can be an
|
||||
entrypoint URL: a sender that follows the `http://` to `https://` redirect in
|
||||
the nginx example sends the URL it was redirected from.
|
||||
|
||||
Client-supplied request content does not leave the host by the other route
|
||||
either. The Sentry SDK attaches the request to every event it captures,
|
||||
independently of the access log, and `SendDefaultPII=false` does not cover all
|
||||
@@ -2492,18 +2465,14 @@ are both taken unconditionally, the body precisely because these handlers call
|
||||
body with `(redacted)`, drops cookies and the remote-address environment, and
|
||||
reduces the headers to a fixed allowlist — `Accept`, `Content-Length`,
|
||||
`Content-Type`, `Host`, `Origin`, `Referer`, `User-Agent` and `X-Request-Id`.
|
||||
`Referer` is sent as its scheme and host only, because its path can be an
|
||||
entrypoint URL; `internal/referer.SchemeAndHost` does that reduction for both
|
||||
the access log's `referer` field and this hook.
|
||||
|
||||
The same hook rewrites the request URL. The SDK builds it as
|
||||
`scheme://host/path` from the concrete path, which on the receiver route is
|
||||
`/h/<uuid>` in full — and that UUID is a write capability, not an identifier:
|
||||
anyone holding it can post events this service accepts and its targets then
|
||||
deliver. What is sent is the chi route pattern instead: `http://host/h/{uuid}`.
|
||||
A tracker has its own retention, access control and deletion policy, so the
|
||||
pattern is sent on every route, including the 2xx and 5xx responses outside the
|
||||
receiver for which the access log keeps the concrete path.
|
||||
deliver. A tracker has its own retention, access control and deletion policy, so
|
||||
the rule the access log follows above does not carry across that boundary. What
|
||||
is sent is the chi route pattern instead: `http://host/h/{uuid}`.
|
||||
|
||||
The scheme and the host are kept, and everything else in the URL is discarded
|
||||
rather than edited, so a future SDK version that starts appending a query string
|
||||
@@ -2565,9 +2534,8 @@ receiver route. What survives still names the failing route — scheme, host,
|
||||
route pattern, method — and `X-Request-Id` ties the event to the local access
|
||||
log line that holds the rest. Nothing dropped is needed for the likeliest use,
|
||||
debugging a CSRF rejection. Its three inputs are the TLS decision, `Origin` and
|
||||
`Referer`; the latter two are kept (`Referer` as its scheme and host, which is
|
||||
all the check compares), and the first is the scheme of the retained URL,
|
||||
because the SDK derives that scheme from
|
||||
`Referer`; the latter two are kept, and the first is the scheme of the retained
|
||||
URL, because the SDK derives that scheme from
|
||||
`r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"`. That predicate
|
||||
is the SDK's own and is stricter than `internal/reqtls.IsTLS`, which this
|
||||
service now uses everywhere it decides transport: the SDK reports `http` for the
|
||||
@@ -2633,7 +2601,9 @@ than it:
|
||||
| ------------------------------------------ | ------- | ------------------- | ------------------------------------------ |
|
||||
| `request body exceeds limit` (413) | `WARN` | path, method | yes — `MaxBodySize` precedes `RequireAuth` |
|
||||
| `csrf: token validation failed` (403) | `WARN` | path, method | yes — `CSRF` precedes `RequireAuth` |
|
||||
| `... rate limit exceeded` (429) | `WARN` | path | yes, on the receiver |
|
||||
| `auth middleware: unauthenticated request` | `DEBUG` | path, method | yes, by definition |
|
||||
| `entrypoint not found` | `DEBUG` | entrypoint UUID | yes, on the receiver |
|
||||
| `user not found` / `invalid password` | `DEBUG` | username | yes, on the sign-in form |
|
||||
| `login failure limit exceeded` (429) | `WARN` | path | yes, on the sign-in form |
|
||||
| `password verification capacity exhausted` | `WARN` | path | yes, on the sign-in form |
|
||||
@@ -2661,12 +2631,13 @@ wider of the two handlers spends more on than the client spent sending them.
|
||||
Every case holds each line to the 2,560-byte ceiling. That per-line ceiling is
|
||||
what the figure above states, and every row establishes it.
|
||||
|
||||
Two of the sites go further and bound the whole flood's output — the total bytes
|
||||
a run of distinct invented values wrote, which is the shape an operator sizing
|
||||
storage cares about. They are `request body exceeds limit`
|
||||
(`TestMaxBodySize_FloodOfOversizePathsDoesNotGrowTheLog`) and `user not found`
|
||||
(through `assertBoundedFlood`). The other rows carry no aggregate assertion; the
|
||||
per-line ceiling is what they establish.
|
||||
Three of the sites go further and bound the whole flood's output — the total
|
||||
bytes a run of distinct invented values wrote, which is the shape an operator
|
||||
sizing storage cares about. They are `request body exceeds limit`
|
||||
(`TestMaxBodySize_FloodOfOversizePathsDoesNotGrowTheLog`),
|
||||
`entrypoint not found` and `user not found` (the last two through
|
||||
`assertBoundedFlood`). The other rows carry no aggregate assertion; the per-line
|
||||
ceiling is what they establish.
|
||||
|
||||
`internal/logfield/logfield_test.go` measures the per-rune charge against what
|
||||
the handlers really emit, over roughly 3,000 code points on each, so an
|
||||
@@ -2684,7 +2655,7 @@ everything else, so its lines take the level the operator set and the handler
|
||||
`internal/logger` selected, and every value it emits is spent through the same
|
||||
`internal/logfield` budget. A record-not-found is not logged as an error: it is
|
||||
the expected outcome on both of those paths, and each handler already records
|
||||
its own miss at `DEBUG`, without the SQL and within the ceiling above. Slow
|
||||
its own miss at `DEBUG` — bounded, per the table above — without the SQL. Slow
|
||||
statements are kept, at `WARN`, above the same 200 ms threshold GORM used and
|
||||
with the statement bounded, because that report is the one thing GORM's logger
|
||||
gave an operator that nothing else here does. The adapter orders its cases
|
||||
@@ -3099,8 +3070,6 @@ webhooker/
|
||||
│ │ ├── ratelimit.go # Per-IP rate limiting middleware (go-chi/httprate)
|
||||
│ │ ├── loginguard.go # Login failure counters and the Argon2id verification semaphore
|
||||
│ │ └── middlewaretest/ # Test support: a Middleware for tests in other packages
|
||||
│ ├── referer/
|
||||
│ │ └── referer.go # SchemeAndHost: cuts a Referer down to its scheme and host, for the access log and the Sentry hook
|
||||
│ ├── reqtls/
|
||||
│ │ └── reqtls.go # IsTLS: the one TLS predicate, r.TLS or X-Forwarded-Proto
|
||||
│ ├── server/
|
||||
@@ -3115,8 +3084,8 @@ webhooker/
|
||||
│ ├── static.go # //go:embed directive
|
||||
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
|
||||
│ ├── css/tailwind.css # Generated stylesheet the pages load
|
||||
│ ├── css/style.css # Hand-written, loaded after tailwind.css: btn-small, the pointer cursor for input.css's buttons, the webhook list cards' focus outline, the rule hiding x-cloak elements
|
||||
│ ├── js/app.js # Copy-to-clipboard, the delete prompts, and the Alpine.js components
|
||||
│ ├── css/style.css # Hand-written, loaded after tailwind.css: btn-small, the pointer cursor for input.css's buttons, the webhook list cards' focus outline
|
||||
│ ├── js/app.js # Copy-to-clipboard, and the Alpine.js components
|
||||
│ └── js/alpine.min.js # Alpine.js CSP build, extracted from 3p/ by make assets, not committed
|
||||
├── templates/ # Go HTML templates (base, login, sources, etc.)
|
||||
├── script/ # Scripts to Rule Them All entrypoints
|
||||
@@ -3170,9 +3139,8 @@ Applied to all routes in this order:
|
||||
|
||||
1. **RequestID** — Generate unique request IDs (chi built-in)
|
||||
2. **SecurityHeaders** — Production security headers on every response (HSTS
|
||||
unless `HSTS_MAX_AGE` is `0`, X-Content-Type-Options, X-Frame-Options, CSP
|
||||
`default-src 'self'; script-src 'self'; style-src 'self'`, Referrer-Policy,
|
||||
Permissions-Policy)
|
||||
unless `HSTS_MAX_AGE` is `0`, X-Content-Type-Options, X-Frame-Options, CSP,
|
||||
Referrer-Policy, Permissions-Policy)
|
||||
3. **Logging** — Structured request logging (method, URL, status, latency,
|
||||
remote IP, client IP, user agent, request ID)
|
||||
4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` and
|
||||
@@ -3257,12 +3225,6 @@ credential check, see [The login endpoint](#the-login-endpoint).
|
||||
- Production security headers on all responses: HSTS (unless `HSTS_MAX_AGE` is
|
||||
`0`), X-Content-Type-Options (`nosniff`), X-Frame-Options (`DENY`),
|
||||
Content-Security-Policy, Referrer-Policy, and Permissions-Policy
|
||||
- The Content-Security-Policy is
|
||||
`default-src 'self'; script-src 'self'; style-src 'self'`: a page takes
|
||||
scripts and styles only from files webhooker serves, and the browser runs no
|
||||
inline script or event handler and ignores any `<style>` element or `style`
|
||||
attribute in a page. The pages need none: their scripts are in `static/js/`
|
||||
and their styles in `static/css/`
|
||||
- Request body size limits (1 MB) on all form POST endpoints, enforced by
|
||||
middleware that runs before CSRF parses the form
|
||||
- **CSRF protection** via [gorilla/csrf](https://github.com/gorilla/csrf) on all
|
||||
@@ -3470,12 +3432,16 @@ fixed independently of the compiler's:
|
||||
a step's log, and then fails. `make test` builds this stage alone.
|
||||
6. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint and test
|
||||
phases and the `css-check` and `markdown-check` stages passing (it copies a
|
||||
file from each), and runs `make build` (which extracts Alpine.js from `3p/`
|
||||
first). `make build` compiles with cgo off, so the binary is statically
|
||||
linked and runs on the Alpine runtime image. The version is the `VERSION`
|
||||
build arg if one is given, otherwise derived from the `.git` in the context,
|
||||
and the stage fails if a context with `.git` would stamp an empty version,
|
||||
`dev` or `unknown` (see [Version stamping](#version-stamping)).
|
||||
file from each), runs `make build` (which extracts Alpine.js from `3p/`
|
||||
first), and then rebuilds the binary with `CGO_ENABLED=1` and static linking
|
||||
so it runs on musl, with the build tags `netgo`, `osusergo` and
|
||||
`sqlite_omit_load_extension` so it needs none of glibc's shared libraries at
|
||||
run time. Both builds go through `make build`, the relink adding its
|
||||
`-extldflags` via `GO_LDFLAGS` and its tags via `GO_TAGS`, so neither can
|
||||
drop the `-X` that stamps the version. The version is the `VERSION` build arg
|
||||
if one is given, otherwise derived from the `.git` in the context, and the
|
||||
stage fails if a context with `.git` would stamp an empty version, `dev` or
|
||||
`unknown` (see [Version stamping](#version-stamping)).
|
||||
7. **Runtime stage** (`alpine:3.21`) — copies the static binary and
|
||||
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` directory for
|
||||
all SQLite databases, exposes port 8080, and includes a health check against
|
||||
@@ -3487,6 +3453,11 @@ The lint and test phases invoke `gofmt`, `golangci-lint` and `go test` directly
|
||||
rather than `make fmt-check`, `make lint` and `make test`: those targets build
|
||||
docker stages, which would need a docker daemon inside this build.
|
||||
|
||||
The lint phase and the Go stages use Debian rather than Alpine because
|
||||
`gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO and does
|
||||
not compile against musl. Only the final binary is statically linked, which is
|
||||
what lets it run on the Alpine runtime image.
|
||||
|
||||
`script/cibuild` is the CI gate: it runs `script/bootstrap`, then
|
||||
`script/check`, then builds the image, whose build runs the lint and test phases
|
||||
and the stylesheet and Markdown checks again. A build that succeeds is a repo
|
||||
|
||||
@@ -25,11 +25,10 @@ func TestEventDBCreateOmitsAssociations(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
target := database.Target{
|
||||
WebhookID: webhookID,
|
||||
Name: "leaky-target",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"https://hooks.example/secret"}`,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: webhookID,
|
||||
Name: "leaky-target",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"https://hooks.example/secret"}`,
|
||||
}
|
||||
target.ID = uuid.New().String()
|
||||
|
||||
|
||||
@@ -23,9 +23,8 @@ func TestPreloadedModelsMarshalWithoutTheirParent(t *testing.T) {
|
||||
Name: testWebhookName,
|
||||
Entrypoints: []database.Entrypoint{{Path: uuid.New().String()}},
|
||||
Targets: []database.Target{{
|
||||
Name: "log",
|
||||
Type: database.TargetTypeLog,
|
||||
MaxAttempts: 1,
|
||||
Name: "log",
|
||||
Type: database.TargetTypeLog,
|
||||
}},
|
||||
}
|
||||
require.NoError(t, db.Create(&stored).Error)
|
||||
|
||||
@@ -46,10 +46,9 @@ func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
|
||||
{
|
||||
name: "target config",
|
||||
model: database.Target{
|
||||
Name: keptField,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Name: keptField,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -98,9 +97,8 @@ func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
|
||||
encoded := marshalModel(t, database.Webhook{
|
||||
Name: keptField,
|
||||
Targets: []database.Target{{
|
||||
Name: "slack",
|
||||
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Name: "slack",
|
||||
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
||||
}},
|
||||
})
|
||||
|
||||
|
||||
@@ -29,12 +29,9 @@ type Target struct {
|
||||
// handler that marshals the model itself.
|
||||
Config string `gorm:"type:text" json:"-"` // JSON configuration
|
||||
|
||||
// MaxAttempts is how many times an HTTP or Slack target attempts
|
||||
// each delivery in all, at least 1. 1 is a single attempt with no
|
||||
// circuit breaker; above 1, a failed attempt is retried with
|
||||
// backoff behind the breaker. The table's check constraint refuses
|
||||
// a target saved with less than 1, whatever its type.
|
||||
MaxAttempts int `gorm:"not null;check:max_attempts >= 1" json:"maxAttempts,omitempty"`
|
||||
// For HTTP targets (max_retries=0 means fire-and-forget,
|
||||
// >0 enables retries with backoff)
|
||||
MaxRetries int `json:"maxRetries,omitempty"`
|
||||
|
||||
// Relations. No model marshals the record it belongs to:
|
||||
// Webhook.Targets leads back here, and the JSON could loop.
|
||||
|
||||
@@ -1,44 +0,0 @@
|
||||
package database_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
)
|
||||
|
||||
// newHTTPTarget returns an http target that makes maxAttempts attempts
|
||||
// at each delivery.
|
||||
func newHTTPTarget(maxAttempts int) *database.Target {
|
||||
return &database.Target{
|
||||
WebhookID: uuid.New().String(),
|
||||
Name: "target",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"https://example.com/"}`,
|
||||
MaxAttempts: maxAttempts,
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_RefusesZeroAttempts relies on the targets table
|
||||
// alone: nothing in the model checks the number, so the check
|
||||
// constraint is what keeps a target with 0 attempts out of the
|
||||
// database.
|
||||
func TestTargetCreate_RefusesZeroAttempts(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := startedTestDB(t)
|
||||
|
||||
err := db.Create(newHTTPTarget(0)).Error
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "CHECK constraint failed")
|
||||
}
|
||||
|
||||
func TestTargetCreate_AcceptsOneAttempt(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := startedTestDB(t)
|
||||
|
||||
require.NoError(t, db.Create(newHTTPTarget(1)).Error)
|
||||
}
|
||||
@@ -87,13 +87,13 @@ func TestMainSchemaHoldsEveryModel(t *testing.T) {
|
||||
Active: true,
|
||||
})
|
||||
assertRoundTrip(t, db, &database.Target{
|
||||
BaseModel: newBaseModel(),
|
||||
WebhookID: uuid.New().String(),
|
||||
Name: "target",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Active: true,
|
||||
Config: `{"url":"https://example.com/"}`,
|
||||
MaxAttempts: 3,
|
||||
BaseModel: newBaseModel(),
|
||||
WebhookID: uuid.New().String(),
|
||||
Name: "target",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Active: true,
|
||||
Config: `{"url":"https://example.com/"}`,
|
||||
MaxRetries: 3,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -80,11 +80,10 @@ CREATE TABLE targets (
|
||||
type text NOT NULL,
|
||||
active numeric DEFAULT true,
|
||||
config text,
|
||||
max_attempts integer NOT NULL,
|
||||
max_retries integer,
|
||||
PRIMARY KEY (id),
|
||||
CONSTRAINT fk_webhooks_targets
|
||||
FOREIGN KEY (webhook_id) REFERENCES webhooks (id),
|
||||
CONSTRAINT chk_targets_max_attempts CHECK (max_attempts >= 1)
|
||||
FOREIGN KEY (webhook_id) REFERENCES webhooks (id)
|
||||
);
|
||||
|
||||
CREATE INDEX idx_targets_deleted_at ON targets (deleted_at);
|
||||
|
||||
@@ -124,12 +124,11 @@ func (env *archiveEnv) addDatabaseTarget(
|
||||
t.Helper()
|
||||
|
||||
tgt := &database.Target{
|
||||
WebhookID: webhookID,
|
||||
Name: "Archive",
|
||||
Type: database.TargetTypeDatabase,
|
||||
Active: true,
|
||||
Config: configJSON,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: webhookID,
|
||||
Name: "Archive",
|
||||
Type: database.TargetTypeDatabase,
|
||||
Active: true,
|
||||
Config: configJSON,
|
||||
}
|
||||
require.NoError(
|
||||
t,
|
||||
|
||||
@@ -107,7 +107,7 @@ type Task struct {
|
||||
TargetName string
|
||||
TargetType database.TargetType
|
||||
TargetConfig string
|
||||
MaxAttempts int
|
||||
MaxRetries int
|
||||
|
||||
Method string
|
||||
RawQuery string
|
||||
@@ -1769,10 +1769,10 @@ func buildEventFromTask(task *Task) database.Event {
|
||||
|
||||
func buildTargetFromTask(task *Task) database.Target {
|
||||
target := database.Target{
|
||||
Name: task.TargetName,
|
||||
Type: task.TargetType,
|
||||
Config: task.TargetConfig,
|
||||
MaxAttempts: task.MaxAttempts,
|
||||
Name: task.TargetName,
|
||||
Type: task.TargetType,
|
||||
Config: task.TargetConfig,
|
||||
MaxRetries: task.MaxRetries,
|
||||
}
|
||||
|
||||
target.ID = task.TargetID
|
||||
@@ -2028,7 +2028,7 @@ func (e *Engine) countAttemptsBatch(
|
||||
// One delivery id per recorded attempt, tallied here rather than
|
||||
// grouped in SQL: internal/gormlog forbids (*gorm.DB).Scan, which
|
||||
// a GROUP BY into a struct would need, and an attempt row per
|
||||
// delivery is bounded by the target's MaxAttempts.
|
||||
// delivery is bounded by the target's MaxRetries.
|
||||
var attemptIDs []string
|
||||
|
||||
err := webhookDB.
|
||||
@@ -2106,7 +2106,7 @@ func buildRecoveryTask(
|
||||
TargetName: target.Name,
|
||||
TargetType: target.Type,
|
||||
TargetConfig: target.Config,
|
||||
MaxAttempts: target.MaxAttempts,
|
||||
MaxRetries: target.MaxRetries,
|
||||
Method: event.Method,
|
||||
RawQuery: event.RawQuery,
|
||||
Headers: event.Headers,
|
||||
|
||||
@@ -170,7 +170,7 @@ func iTask(
|
||||
d database.Delivery,
|
||||
event database.Event,
|
||||
webhookID, targetID, name, config string,
|
||||
maxAttempts, attemptNum int,
|
||||
maxRetries, attemptNum int,
|
||||
body *string,
|
||||
) delivery.Task {
|
||||
return delivery.Task{
|
||||
@@ -181,7 +181,7 @@ func iTask(
|
||||
TargetName: name,
|
||||
TargetType: database.TargetTypeHTTP,
|
||||
TargetConfig: config,
|
||||
MaxAttempts: maxAttempts,
|
||||
MaxRetries: maxRetries,
|
||||
Method: event.Method,
|
||||
Headers: event.Headers,
|
||||
ContentType: event.ContentType,
|
||||
@@ -242,7 +242,7 @@ func TestProcessNewTask_InlineBody(t *testing.T) {
|
||||
cfg := iHTTPConfig(ts.URL)
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"test-target", cfg, 1, 1, &bodyStr,
|
||||
"test-target", cfg, 0, 1, &bodyStr,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(
|
||||
@@ -293,7 +293,7 @@ func TestProcessNewTask_LargeBody_FetchFromDB(
|
||||
cfg := iHTTPConfig(ts.URL)
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"test-large", cfg, 1, 1, nil,
|
||||
"test-large", cfg, 0, 1, nil,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(
|
||||
@@ -320,7 +320,7 @@ func TestProcessNewTask_InvalidWebhookID(t *testing.T) {
|
||||
TargetName: "test",
|
||||
TargetType: database.TargetTypeHTTP,
|
||||
TargetConfig: iHTTPConfig("http://localhost:9999"),
|
||||
MaxAttempts: 1,
|
||||
MaxRetries: 0,
|
||||
Body: nil,
|
||||
AttemptNum: 1,
|
||||
}
|
||||
@@ -506,7 +506,7 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"lifecycle-test", "",
|
||||
1, 1, &bodyStr,
|
||||
0, 1, &bodyStr,
|
||||
)
|
||||
task.TargetType = database.TargetTypeLog
|
||||
|
||||
@@ -645,7 +645,7 @@ func TestRecoverPendingDeliveries(t *testing.T) {
|
||||
|
||||
iCreateTarget(t, s.MainDB, targetID,
|
||||
s.WebhookID, "recovery-target",
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
iSeedPendingDeliveries(
|
||||
@@ -686,17 +686,17 @@ func iCreateTarget(
|
||||
targetID, webhookID, name string,
|
||||
targetType database.TargetType,
|
||||
config string,
|
||||
maxAttempts int,
|
||||
maxRetries int,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
target := database.Target{
|
||||
WebhookID: webhookID,
|
||||
Name: name,
|
||||
Type: targetType,
|
||||
Active: true,
|
||||
Config: config,
|
||||
MaxAttempts: maxAttempts,
|
||||
WebhookID: webhookID,
|
||||
Name: name,
|
||||
Type: targetType,
|
||||
Active: true,
|
||||
Config: config,
|
||||
MaxRetries: maxRetries,
|
||||
}
|
||||
target.ID = targetID
|
||||
|
||||
@@ -1077,7 +1077,7 @@ func TestRecoverInFlight_WithPendingDeliveries(
|
||||
|
||||
iCreateTarget(t, s.MainDB, targetID,
|
||||
s.WebhookID, "recover-target",
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
event := iSeedEvent(
|
||||
@@ -1232,7 +1232,7 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"auth-target", string(cfgJSON),
|
||||
1, 1, &bodyStr,
|
||||
0, 1, &bodyStr,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(
|
||||
@@ -1277,7 +1277,7 @@ func TestDeliverHTTP_TargetTimeout(t *testing.T) {
|
||||
|
||||
task, d := iHTTPTaskAndDelivery(
|
||||
event, del, "timeout-target",
|
||||
string(cfgJSON), 1, 1,
|
||||
string(cfgJSON), 0, 1,
|
||||
)
|
||||
|
||||
e.ExportDeliverHTTP(context.TODO(), db, d, task)
|
||||
@@ -1339,7 +1339,7 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
|
||||
bodyStr := event.Body
|
||||
task := iTask(
|
||||
del, event, s.WebhookID, targetID,
|
||||
"cut-target", cfgJSON, 1, 1, &bodyStr,
|
||||
"cut-target", cfgJSON, 0, 1, &bodyStr,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(context.TODO(), &task)
|
||||
@@ -1403,7 +1403,7 @@ func iHTTPTaskAndDelivery(
|
||||
event database.Event,
|
||||
del database.Delivery,
|
||||
name, config string,
|
||||
maxAttempts, attemptNum int,
|
||||
maxRetries, attemptNum int,
|
||||
) (*delivery.Task, *database.Delivery) {
|
||||
task := &delivery.Task{
|
||||
DeliveryID: del.ID,
|
||||
@@ -1413,7 +1413,7 @@ func iHTTPTaskAndDelivery(
|
||||
TargetName: name,
|
||||
TargetType: database.TargetTypeHTTP,
|
||||
TargetConfig: config,
|
||||
MaxAttempts: maxAttempts,
|
||||
MaxRetries: maxRetries,
|
||||
AttemptNum: attemptNum,
|
||||
}
|
||||
|
||||
@@ -1423,10 +1423,9 @@ func iHTTPTaskAndDelivery(
|
||||
Status: database.DeliveryStatusPending,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: name,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: config,
|
||||
MaxAttempts: maxAttempts,
|
||||
Name: name,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: config,
|
||||
},
|
||||
}
|
||||
d.ID = del.ID
|
||||
@@ -1467,7 +1466,7 @@ func TestDeliverHTTP_InvalidConfig(t *testing.T) {
|
||||
)
|
||||
|
||||
task, d := iHTTPTaskAndDelivery(
|
||||
event, del, "bad-config", `not-json`, 1, 1,
|
||||
event, del, "bad-config", `not-json`, 0, 1,
|
||||
)
|
||||
|
||||
e.ExportDeliverHTTP(context.TODO(), db, d, task)
|
||||
@@ -1491,7 +1490,7 @@ func TestDeliverHTTP_InvalidConfigUnrecordedStaysPending(t *testing.T) {
|
||||
)
|
||||
|
||||
task, d := iHTTPTaskAndDelivery(
|
||||
event, del, "bad-config", `not-json`, 1, 1,
|
||||
event, del, "bad-config", `not-json`, 0, 1,
|
||||
)
|
||||
|
||||
require.NoError(t, db.Exec("drop table delivery_results").Error)
|
||||
|
||||
@@ -149,7 +149,7 @@ func seedLogTask(
|
||||
bodyStr := event.Body
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"hook-context-test", "", 1, 1, &bodyStr,
|
||||
"hook-context-test", "", 0, 1, &bodyStr,
|
||||
)
|
||||
task.TargetType = database.TargetTypeLog
|
||||
|
||||
@@ -278,7 +278,7 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
|
||||
}
|
||||
|
||||
// seedHeldDelivery gives the setup's webhook an http target with
|
||||
// maxAttempts, pointed at a receiver that keeps every request open until
|
||||
// maxRetries, pointed at a receiver that keeps every request open until
|
||||
// the client gives up on it, and seeds one delivery to that target at
|
||||
// status. It returns the task that sends attempt attemptNum of the
|
||||
// delivery, and a channel that is closed when a request reaches the
|
||||
@@ -286,7 +286,7 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
|
||||
func seedHeldDelivery(
|
||||
t *testing.T,
|
||||
s iSetup,
|
||||
maxAttempts int,
|
||||
maxRetries int,
|
||||
status database.DeliveryStatus,
|
||||
attemptNum int,
|
||||
) (delivery.Task, <-chan struct{}) {
|
||||
@@ -315,7 +315,7 @@ func seedHeldDelivery(
|
||||
iCreateWebhook(t, s.MainDB, s.WebhookID, "hook")
|
||||
iCreateTarget(
|
||||
t, s.MainDB, targetID, s.WebhookID, "held",
|
||||
database.TargetTypeHTTP, cfg, maxAttempts,
|
||||
database.TargetTypeHTTP, cfg, maxRetries,
|
||||
)
|
||||
|
||||
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"held":true}`)
|
||||
@@ -325,7 +325,7 @@ func seedHeldDelivery(
|
||||
|
||||
return iTask(
|
||||
d, event, s.WebhookID, targetID, "held", cfg,
|
||||
maxAttempts, attemptNum, &bodyStr,
|
||||
maxRetries, attemptNum, &bodyStr,
|
||||
), arrived
|
||||
}
|
||||
|
||||
@@ -380,7 +380,7 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
|
||||
lc := startEngineViaHook(t, s.Engine)
|
||||
|
||||
task, arrived := seedHeldDelivery(
|
||||
t, s, 1, database.DeliveryStatusPending, 1,
|
||||
t, s, 0, database.DeliveryStatusPending, 1,
|
||||
)
|
||||
|
||||
s.Engine.Notify([]delivery.Task{task})
|
||||
@@ -404,13 +404,13 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
|
||||
func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const maxAttempts = 2
|
||||
const maxRetries = 2
|
||||
|
||||
s := newISetup(t)
|
||||
lc := startEngineViaHook(t, s.Engine)
|
||||
|
||||
task, arrived := seedHeldDelivery(
|
||||
t, s, maxAttempts, database.DeliveryStatusRetrying, maxAttempts,
|
||||
t, s, maxRetries, database.DeliveryStatusRetrying, maxRetries,
|
||||
)
|
||||
iSeedFailedResult(t, s.WebhookDB, task.DeliveryID)
|
||||
|
||||
@@ -435,10 +435,9 @@ func deliverToArchive(t *testing.T, s iSetup) (string, string) {
|
||||
iCreateWebhook(t, s.MainDB, s.WebhookID, "hook")
|
||||
|
||||
tgt := &database.Target{
|
||||
WebhookID: s.WebhookID,
|
||||
Name: "archive",
|
||||
Type: database.TargetTypeDatabase,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: s.WebhookID,
|
||||
Name: "archive",
|
||||
Type: database.TargetTypeDatabase,
|
||||
}
|
||||
require.NoError(
|
||||
t, s.MainDB.Omit(clause.Associations).Create(tgt).Error,
|
||||
|
||||
@@ -142,7 +142,7 @@ func buildHTTPFixture(
|
||||
dlv database.Delivery,
|
||||
event database.Event,
|
||||
targetID, name, config string,
|
||||
maxAttempts, attemptNum int,
|
||||
maxRetries, attemptNum int,
|
||||
) httpDeliveryFixture {
|
||||
task := &delivery.Task{
|
||||
DeliveryID: dlv.ID,
|
||||
@@ -152,7 +152,7 @@ func buildHTTPFixture(
|
||||
TargetName: name,
|
||||
TargetType: database.TargetTypeHTTP,
|
||||
TargetConfig: config,
|
||||
MaxAttempts: maxAttempts,
|
||||
MaxRetries: maxRetries,
|
||||
AttemptNum: attemptNum,
|
||||
}
|
||||
|
||||
@@ -162,10 +162,10 @@ func buildHTTPFixture(
|
||||
Status: dlv.Status,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: name,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: config,
|
||||
MaxAttempts: maxAttempts,
|
||||
Name: name,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: config,
|
||||
MaxRetries: maxRetries,
|
||||
},
|
||||
}
|
||||
d.ID = dlv.ID
|
||||
@@ -277,7 +277,7 @@ func TestDeliverHTTP_Success(t *testing.T) {
|
||||
|
||||
cfg := newHTTPTargetConfig(ts.URL)
|
||||
fix := buildHTTPFixture(
|
||||
dlv, event, targetID, "test-http", cfg, 1, 1,
|
||||
dlv, event, targetID, "test-http", cfg, 0, 1,
|
||||
)
|
||||
|
||||
e.ExportDeliverHTTP(
|
||||
@@ -324,7 +324,7 @@ func TestDeliverHTTP_Failure(t *testing.T) {
|
||||
cfg := newHTTPTargetConfig(ts.URL)
|
||||
fix := buildHTTPFixture(
|
||||
dlv, event, targetID,
|
||||
"test-http-fail", cfg, 1, 1,
|
||||
"test-http-fail", cfg, 0, 1,
|
||||
)
|
||||
|
||||
e.ExportDeliverHTTP(
|
||||
@@ -473,23 +473,14 @@ func TestDeliverHTTP_WithRetries_Success(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
// TestDeliverHTTP_MaxAttemptsExhausted proves a target with three
|
||||
// delivery attempts makes at most three: the first two failures are
|
||||
// retried, the third is final, and each attempt goes through the
|
||||
// target's circuit breaker.
|
||||
func TestDeliverHTTP_MaxAttemptsExhausted(t *testing.T) {
|
||||
func TestDeliverHTTP_MaxRetriesExhausted(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const maxAttempts = 3
|
||||
|
||||
db := testWebhookDB(t)
|
||||
|
||||
var requests atomic.Int32
|
||||
|
||||
ts := httptest.NewServer(
|
||||
http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
requests.Add(1)
|
||||
w.WriteHeader(http.StatusBadGateway)
|
||||
},
|
||||
),
|
||||
@@ -498,103 +489,26 @@ func TestDeliverHTTP_MaxAttemptsExhausted(t *testing.T) {
|
||||
|
||||
e := testEngine(t, 1)
|
||||
targetID := uuid.New().String()
|
||||
|
||||
// It opens on the last failure only if every attempt reached it.
|
||||
cb := delivery.NewTestCircuitBreaker(maxAttempts, time.Minute)
|
||||
e.ExportSetCircuitBreaker(targetID, cb)
|
||||
|
||||
event := seedEvent(t, db, `{"retry":"exhaust"}`)
|
||||
|
||||
dlv := seedDelivery(
|
||||
t, db, event.ID, targetID,
|
||||
database.DeliveryStatusPending,
|
||||
)
|
||||
|
||||
cfg := newHTTPTargetConfig(ts.URL)
|
||||
sched := &recordingScheduler{}
|
||||
|
||||
for attempt := 1; attempt <= maxAttempts; attempt++ {
|
||||
fix := buildHTTPFixture(
|
||||
dlv, event, targetID,
|
||||
"test-http-exhaust", cfg, maxAttempts, attempt,
|
||||
)
|
||||
|
||||
e.ExportDeliverHTTPWithScheduler(
|
||||
context.TODO(), db, fix.Delivery, fix.Task, sched,
|
||||
)
|
||||
}
|
||||
|
||||
assert.Equal(t, int32(maxAttempts), requests.Load())
|
||||
assert.Len(t, sched.delays, maxAttempts-1,
|
||||
"every failure but the last should be retried",
|
||||
)
|
||||
assertDeliveryStatus(t, db, dlv.ID,
|
||||
database.DeliveryStatusFailed,
|
||||
)
|
||||
assert.Equal(t, delivery.CircuitOpen, cb.State(),
|
||||
"every attempt should be counted by the circuit breaker",
|
||||
)
|
||||
}
|
||||
|
||||
// TestDeliverHTTP_SingleAttemptSkipsCircuitBreaker proves a target
|
||||
// with one delivery attempt makes exactly that attempt, even while
|
||||
// its circuit breaker is open, and that a failure is final.
|
||||
func TestDeliverHTTP_SingleAttemptSkipsCircuitBreaker(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := testWebhookDB(t)
|
||||
|
||||
var requests atomic.Int32
|
||||
|
||||
ts := httptest.NewServer(
|
||||
http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
requests.Add(1)
|
||||
w.WriteHeader(http.StatusBadGateway)
|
||||
},
|
||||
),
|
||||
)
|
||||
defer ts.Close()
|
||||
|
||||
e := testEngine(t, 1)
|
||||
targetID := uuid.New().String()
|
||||
|
||||
cb := e.ExportGetCircuitBreaker(targetID)
|
||||
|
||||
for range delivery.ExportDefaultFailureThreshold {
|
||||
cb.RecordFailure()
|
||||
}
|
||||
|
||||
require.Equal(t, delivery.CircuitOpen, cb.State())
|
||||
|
||||
event := seedEvent(t, db, `{"retry":"none"}`)
|
||||
|
||||
dlv := seedDelivery(
|
||||
t, db, event.ID, targetID,
|
||||
database.DeliveryStatusPending,
|
||||
database.DeliveryStatusRetrying,
|
||||
)
|
||||
|
||||
cfg := newHTTPTargetConfig(ts.URL)
|
||||
fix := buildHTTPFixture(
|
||||
dlv, event, targetID,
|
||||
"test-http-single", cfg, 1, 1,
|
||||
)
|
||||
sched := &recordingScheduler{}
|
||||
|
||||
e.ExportDeliverHTTPWithScheduler(
|
||||
context.TODO(), db, fix.Delivery, fix.Task, sched,
|
||||
"test-http-exhaust", cfg, 3, 3,
|
||||
)
|
||||
|
||||
assert.Equal(t, int32(1), requests.Load(),
|
||||
"the open circuit breaker should not hold the attempt back",
|
||||
e.ExportDeliverHTTP(
|
||||
context.TODO(), db, fix.Delivery, fix.Task,
|
||||
)
|
||||
assert.Empty(t, sched.delays, "no retry should be scheduled")
|
||||
|
||||
assertDeliveryStatus(t, db, dlv.ID,
|
||||
database.DeliveryStatusFailed,
|
||||
)
|
||||
assertDeliveryResult(
|
||||
t, db, dlv.ID, false, http.StatusBadGateway,
|
||||
)
|
||||
}
|
||||
|
||||
func TestDeliverHTTP_SchedulesRetryOnFailure(
|
||||
@@ -813,10 +727,9 @@ func seedConcurrencyTasks(
|
||||
Status: database.DeliveryStatusPending,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: fmt.Sprintf("task-%d", i),
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: targetCfg,
|
||||
MaxAttempts: 1,
|
||||
Name: fmt.Sprintf("task-%d", i),
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: targetCfg,
|
||||
},
|
||||
}
|
||||
tasks[i].ID = dlv.ID
|
||||
@@ -828,7 +741,7 @@ func seedConcurrencyTasks(
|
||||
TargetName: tasks[i].Target.Name,
|
||||
TargetType: tasks[i].Target.Type,
|
||||
TargetConfig: tasks[i].Target.Config,
|
||||
MaxAttempts: 1,
|
||||
MaxRetries: 0,
|
||||
AttemptNum: 1,
|
||||
}
|
||||
}
|
||||
@@ -1775,10 +1688,9 @@ func buildSlackDelivery(
|
||||
Status: dlv.Status,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: name,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: slackCfg,
|
||||
MaxAttempts: 1,
|
||||
Name: name,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: slackCfg,
|
||||
},
|
||||
}
|
||||
d.ID = dlv.ID
|
||||
@@ -1962,10 +1874,9 @@ func TestDeliverSlack_InvalidConfig(t *testing.T) {
|
||||
Status: database.DeliveryStatusPending,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: "test-slack-bad",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"not_webhook_url":"missing"}`,
|
||||
MaxAttempts: 1,
|
||||
Name: "test-slack-bad",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"not_webhook_url":"missing"}`,
|
||||
},
|
||||
}
|
||||
d.ID = dlv.ID
|
||||
@@ -2022,9 +1933,8 @@ func TestProcessDelivery_RoutesToSlack(t *testing.T) {
|
||||
)
|
||||
|
||||
task := &delivery.Task{
|
||||
DeliveryID: dlv.ID,
|
||||
TargetType: database.TargetTypeSlack,
|
||||
MaxAttempts: 1,
|
||||
DeliveryID: dlv.ID,
|
||||
TargetType: database.TargetTypeSlack,
|
||||
}
|
||||
|
||||
e.ExportProcessDelivery(
|
||||
@@ -2141,10 +2051,10 @@ func buildSlackRetryDelivery(
|
||||
Status: database.DeliveryStatusPending,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: "test-slack-retry",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: cfg,
|
||||
MaxAttempts: 5,
|
||||
Name: "test-slack-retry",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: cfg,
|
||||
MaxRetries: 5,
|
||||
},
|
||||
}
|
||||
d.ID = dlv.ID
|
||||
@@ -2179,11 +2089,11 @@ func TestDeliverSlack_WithRetries_SchedulesRetry(
|
||||
)
|
||||
|
||||
task := &delivery.Task{
|
||||
DeliveryID: dlv.ID,
|
||||
TargetID: targetID,
|
||||
TargetType: database.TargetTypeSlack,
|
||||
MaxAttempts: 5,
|
||||
AttemptNum: 1,
|
||||
DeliveryID: dlv.ID,
|
||||
TargetID: targetID,
|
||||
TargetType: database.TargetTypeSlack,
|
||||
MaxRetries: 5,
|
||||
AttemptNum: 1,
|
||||
}
|
||||
|
||||
e.ExportProcessDelivery(context.TODO(), db, d, task)
|
||||
|
||||
@@ -152,7 +152,7 @@ func TestEventDBHoldsNoTargetRowsOnFailedDelivery(t *testing.T) {
|
||||
)
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"leaky-target", cfg, 1, 1, &body,
|
||||
"leaky-target", cfg, 0, 1, &body,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(context.TODO(), &task)
|
||||
|
||||
@@ -147,12 +147,11 @@ func tsSeedTarget(
|
||||
t.Helper()
|
||||
|
||||
target := database.Target{
|
||||
WebhookID: webhookID,
|
||||
Name: "slack-sink",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: config,
|
||||
Active: true,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: webhookID,
|
||||
Name: "slack-sink",
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: config,
|
||||
Active: true,
|
||||
}
|
||||
|
||||
require.NoError(t, mainDB.Create(&target).Error)
|
||||
@@ -177,7 +176,7 @@ func tsTask(
|
||||
TargetName: target.Name,
|
||||
TargetType: database.TargetTypeSlack,
|
||||
TargetConfig: target.Config,
|
||||
MaxAttempts: 1,
|
||||
MaxRetries: 0,
|
||||
Method: event.Method,
|
||||
Headers: event.Headers,
|
||||
ContentType: event.ContentType,
|
||||
|
||||
@@ -142,10 +142,9 @@ func (e *Engine) ExportDeliverSlack(
|
||||
d *database.Delivery,
|
||||
) {
|
||||
task := &Task{
|
||||
DeliveryID: d.ID,
|
||||
TargetID: d.TargetID,
|
||||
MaxAttempts: d.Target.MaxAttempts,
|
||||
AttemptNum: 1,
|
||||
DeliveryID: d.ID,
|
||||
TargetID: d.TargetID,
|
||||
AttemptNum: 1,
|
||||
}
|
||||
|
||||
e.targets[database.TargetTypeSlack].Deliver(
|
||||
|
||||
@@ -32,7 +32,7 @@ func fSweepSetup(
|
||||
|
||||
iCreateTarget(t, s.MainDB, targetID,
|
||||
s.WebhookID, name,
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
require.NoError(t, s.MainDB.Create(&database.Webhook{
|
||||
@@ -229,7 +229,7 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
|
||||
|
||||
iCreateTarget(t, s.MainDB, targetID,
|
||||
s.WebhookID, "released",
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
event := iSeedEvent(
|
||||
|
||||
@@ -291,20 +291,20 @@ func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
|
||||
body := event.Body
|
||||
cfg := iHTTPConfig(ts.URL)
|
||||
|
||||
// An attempt budget above the failure threshold, so the breaker
|
||||
// A retry budget above the failure threshold, so the breaker
|
||||
// rather than the budget is what stops the delivery.
|
||||
maxAttempts := delivery.ExportDefaultFailureThreshold + 5
|
||||
maxRetries := delivery.ExportDefaultFailureThreshold + 5
|
||||
|
||||
// The retries below are only run if their target still exists;
|
||||
// see https://git.eeqj.de/sneak/webhooker/issues/107.
|
||||
iCreateTarget(
|
||||
t, s.MainDB, targetID, s.WebhookID, "metrics-trip",
|
||||
database.TargetTypeHTTP, cfg, maxAttempts,
|
||||
database.TargetTypeHTTP, cfg, maxRetries,
|
||||
)
|
||||
|
||||
first := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"metrics-trip", cfg, maxAttempts, 1, &body,
|
||||
"metrics-trip", cfg, maxRetries, 1, &body,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(context.TODO(), &first)
|
||||
@@ -316,7 +316,7 @@ func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
|
||||
ExportDefaultFailureThreshold; attempt++ {
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"metrics-trip", cfg, maxAttempts, attempt, &body,
|
||||
"metrics-trip", cfg, maxRetries, attempt, &body,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessRetryTask(
|
||||
@@ -364,16 +364,16 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
|
||||
|
||||
body := event.Body
|
||||
cfg := iHTTPConfig(ts.URL)
|
||||
maxAttempts := delivery.ExportDefaultFailureThreshold + 5
|
||||
maxRetries := delivery.ExportDefaultFailureThreshold + 5
|
||||
|
||||
iCreateTarget(
|
||||
t, s.MainDB, targetID, s.WebhookID, "metrics-blocked",
|
||||
database.TargetTypeHTTP, cfg, maxAttempts,
|
||||
database.TargetTypeHTTP, cfg, maxRetries,
|
||||
)
|
||||
|
||||
first := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"metrics-blocked", cfg, maxAttempts, 1, &body,
|
||||
"metrics-blocked", cfg, maxRetries, 1, &body,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(context.TODO(), &first)
|
||||
@@ -382,7 +382,7 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
|
||||
ExportDefaultFailureThreshold; attempt++ {
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"metrics-blocked", cfg, maxAttempts, attempt, &body,
|
||||
"metrics-blocked", cfg, maxRetries, attempt, &body,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessRetryTask(context.TODO(), &task)
|
||||
@@ -405,7 +405,7 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
|
||||
|
||||
blocked := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"metrics-blocked", cfg, maxAttempts,
|
||||
"metrics-blocked", cfg, maxRetries,
|
||||
delivery.ExportDefaultFailureThreshold+1, &body,
|
||||
)
|
||||
|
||||
|
||||
@@ -66,7 +66,7 @@ func TestRecoverySkipsPendingWithSuccessfulResult(
|
||||
|
||||
iCreateTarget(t, s.MainDB, targetID,
|
||||
s.WebhookID, "already-delivered",
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
event := iSeedEvent(
|
||||
@@ -129,7 +129,7 @@ func TestRecoveryContinuesTheAttemptNumbering(t *testing.T) {
|
||||
|
||||
iCreateTarget(t, s.MainDB, targetID,
|
||||
s.WebhookID, "numbering",
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
event := iSeedEvent(
|
||||
@@ -354,17 +354,16 @@ func TestFailedResultWriteLeavesDeliveryRecoverable(
|
||||
Status: database.DeliveryStatusPending,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: "unwritable",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: iHTTPConfig(ts.URL),
|
||||
MaxAttempts: 1,
|
||||
Name: "unwritable",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: iHTTPConfig(ts.URL),
|
||||
},
|
||||
}
|
||||
full.ID = d.ID
|
||||
|
||||
s.Engine.ExportDeliverHTTP(
|
||||
context.Background(), s.WebhookDB, full,
|
||||
&delivery.Task{DeliveryID: d.ID, MaxAttempts: 1, AttemptNum: 1},
|
||||
&delivery.Task{DeliveryID: d.ID, AttemptNum: 1},
|
||||
)
|
||||
|
||||
assert.Equal(
|
||||
@@ -445,10 +444,10 @@ func TestFailedResultWriteWithRetriesLeavesDeliveryRecoverable(
|
||||
Status: database.DeliveryStatusPending,
|
||||
Event: event,
|
||||
Target: database.Target{
|
||||
Name: "unwritable",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: iHTTPConfig(ts.URL),
|
||||
MaxAttempts: 3,
|
||||
Name: "unwritable",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: iHTTPConfig(ts.URL),
|
||||
MaxRetries: 3,
|
||||
},
|
||||
}
|
||||
full.ID = d.ID
|
||||
@@ -458,8 +457,9 @@ func TestFailedResultWriteWithRetriesLeavesDeliveryRecoverable(
|
||||
s.Engine.ExportDeliverHTTPWithScheduler(
|
||||
context.Background(), s.WebhookDB, full,
|
||||
&delivery.Task{
|
||||
DeliveryID: d.ID, TargetID: targetID,
|
||||
MaxAttempts: 3, AttemptNum: 1,
|
||||
DeliveryID: d.ID,
|
||||
TargetID: targetID,
|
||||
AttemptNum: 1,
|
||||
},
|
||||
sched,
|
||||
)
|
||||
|
||||
@@ -96,7 +96,7 @@ func deliverWithProbeHeaders(
|
||||
body := event.Body
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, targetID,
|
||||
"redirect-target", string(cfg), 1, 1, &body,
|
||||
"redirect-target", string(cfg), 0, 1, &body,
|
||||
)
|
||||
|
||||
s.Engine.ExportProcessNewTask(context.TODO(), &task)
|
||||
|
||||
@@ -120,8 +120,8 @@ func unavailableConfigFields() []ConfigField {
|
||||
}
|
||||
|
||||
// slackConfigFields describes a Slack target: its masked
|
||||
// webhook URL and its number of delivery attempts. Only the
|
||||
// masked URL is shown; the full URL is the credential.
|
||||
// webhook URL and its retry count. Only the masked URL is
|
||||
// shown; the full URL is the credential.
|
||||
func slackConfigFields(t *database.Target) []ConfigField {
|
||||
cfg, err := parseSlackConfig(t.Config)
|
||||
if err != nil {
|
||||
@@ -131,7 +131,7 @@ func slackConfigFields(t *database.Target) []ConfigField {
|
||||
return []ConfigField{{
|
||||
Label: "Webhook URL",
|
||||
Value: cfg.MaskedWebhookURL(),
|
||||
}, maxAttemptsField(t)}
|
||||
}, maxRetriesField(t)}
|
||||
}
|
||||
|
||||
// httpConfigFields describes an HTTP target: its destination
|
||||
@@ -178,18 +178,23 @@ func httpConfigFields(t *database.Target) []ConfigField {
|
||||
})
|
||||
}
|
||||
|
||||
fields = append(fields, maxAttemptsField(t))
|
||||
fields = append(fields, maxRetriesField(t))
|
||||
|
||||
return fields
|
||||
}
|
||||
|
||||
// maxAttemptsField describes a target's number of delivery
|
||||
// attempts, which lives on the target row rather than in its
|
||||
// configuration blob.
|
||||
func maxAttemptsField(t *database.Target) ConfigField {
|
||||
// maxRetriesField describes a target's retry count, which lives
|
||||
// on the target row rather than in its configuration blob. A
|
||||
// stored 0 makes a single attempt, so it is shown as 1.
|
||||
func maxRetriesField(t *database.Target) ConfigField {
|
||||
attempts := strconv.Itoa(t.MaxRetries)
|
||||
if t.MaxRetries == 0 {
|
||||
attempts = "1 (fire-and-forget: no retries, no circuit breaker)"
|
||||
}
|
||||
|
||||
return ConfigField{
|
||||
Label: "Delivery attempts",
|
||||
Value: strconv.Itoa(t.MaxAttempts),
|
||||
Value: attempts,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -32,7 +32,7 @@ const (
|
||||
viewMaskedOrigin = viewExampleOrigin + "/..."
|
||||
viewUnavailable = "(unavailable)"
|
||||
viewExpiryNever = "never"
|
||||
viewMaxAttempts = "Delivery attempts"
|
||||
viewMaxRetries = "Delivery attempts"
|
||||
)
|
||||
|
||||
func TestMaskedWebhookURL(t *testing.T) {
|
||||
@@ -171,7 +171,6 @@ func slackTarget() database.Target {
|
||||
Active: true,
|
||||
Config: `{"webhookUrl":"` +
|
||||
slackWebhookURL + `"}`,
|
||||
MaxAttempts: 1,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -190,29 +189,28 @@ func TestNewTargetViews_Slack(t *testing.T) {
|
||||
assert.Equal(
|
||||
t,
|
||||
map[string]string{
|
||||
"Webhook URL": slackMaskedURL,
|
||||
viewMaxAttempts: "1",
|
||||
"Webhook URL": slackMaskedURL,
|
||||
viewMaxRetries: "1 (fire-and-forget: no retries, no circuit breaker)",
|
||||
},
|
||||
fieldMap(view.Config),
|
||||
)
|
||||
}
|
||||
|
||||
// TestNewTargetViews_SlackAttempts proves a Slack target shows
|
||||
// its number of delivery attempts the same way an HTTP target
|
||||
// does.
|
||||
func TestNewTargetViews_SlackAttempts(t *testing.T) {
|
||||
// TestNewTargetViews_SlackRetries proves a Slack target shows
|
||||
// its retry count the same way an HTTP target does.
|
||||
func TestNewTargetViews_SlackRetries(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
target := slackTarget()
|
||||
target.MaxAttempts = 2
|
||||
target.MaxRetries = 2
|
||||
|
||||
view := viewFor(t, target)
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
map[string]string{
|
||||
"Webhook URL": slackMaskedURL,
|
||||
viewMaxAttempts: "2",
|
||||
"Webhook URL": slackMaskedURL,
|
||||
viewMaxRetries: "2",
|
||||
},
|
||||
fieldMap(view.Config),
|
||||
)
|
||||
@@ -227,7 +225,7 @@ func TestNewTargetViews_HTTP(t *testing.T) {
|
||||
`"timeout":30,` +
|
||||
`"headers":{"Authorization":"Bearer sekrit"},` +
|
||||
`"forwardQuery":true}`,
|
||||
MaxAttempts: 5,
|
||||
MaxRetries: 5,
|
||||
})
|
||||
|
||||
fields := fieldMap(view.Config)
|
||||
@@ -239,7 +237,7 @@ func TestNewTargetViews_HTTP(t *testing.T) {
|
||||
"Timeout": "30s",
|
||||
"Headers": "1 configured",
|
||||
"Query string": "passed on to this target",
|
||||
viewMaxAttempts: "5",
|
||||
viewMaxRetries: "5",
|
||||
},
|
||||
fields,
|
||||
)
|
||||
@@ -250,22 +248,19 @@ func TestNewTargetViews_HTTP(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestNewTargetViews_HTTPSingleAttempt proves a single attempt is
|
||||
// shown as the plain number, like any other.
|
||||
func TestNewTargetViews_HTTPSingleAttempt(t *testing.T) {
|
||||
func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
view := viewFor(t, database.Target{
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + viewExampleHook + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + viewExampleHook + `"}`,
|
||||
})
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
map[string]string{
|
||||
"Destination URL": viewMaskedOrigin,
|
||||
viewMaxAttempts: "1",
|
||||
viewMaxRetries: "1 (fire-and-forget: no retries, no circuit breaker)",
|
||||
},
|
||||
fieldMap(view.Config),
|
||||
)
|
||||
@@ -279,9 +274,8 @@ func TestNewTargetViews_HTTPMasksDestinationURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
view := viewFor(t, database.Target{
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + slackWebhookURL + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + slackWebhookURL + `"}`,
|
||||
})
|
||||
|
||||
fields := fieldMap(view.Config)
|
||||
@@ -404,23 +398,19 @@ func TestNewTargetViews_Unpresentable(t *testing.T) {
|
||||
Config: blob,
|
||||
},
|
||||
"unparseable json": {
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: blob,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: blob,
|
||||
},
|
||||
"empty slack config": {
|
||||
Type: database.TargetTypeSlack,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
},
|
||||
"slack config without url": {
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{}`,
|
||||
},
|
||||
"unparseable http json": {
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":`,
|
||||
},
|
||||
"unparseable archive json": {
|
||||
Type: database.TargetTypeDatabase,
|
||||
|
||||
@@ -189,7 +189,6 @@ func TestNewTargetConfigForm(t *testing.T) {
|
||||
Config: `{"url":"https://example.com/h",` +
|
||||
`"headers":{"Authorization":"Bearer abc"},` +
|
||||
`"timeout":9}`,
|
||||
MaxAttempts: 1,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "https://example.com/h", form.URL)
|
||||
@@ -197,9 +196,8 @@ func TestNewTargetConfigForm(t *testing.T) {
|
||||
assert.Equal(t, "9", form.Timeout)
|
||||
|
||||
form, err = delivery.NewTargetConfigForm(&database.Target{
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"https://hooks.example/s"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"https://hooks.example/s"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "https://hooks.example/s", form.URL)
|
||||
@@ -244,9 +242,9 @@ func TestNewTargetConfigForm_UnreadableConfigErrors(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []*database.Target{
|
||||
{Type: database.TargetTypeHTTP, Config: "not json", MaxAttempts: 1},
|
||||
{Type: database.TargetTypeHTTP, Config: `{}`, MaxAttempts: 1},
|
||||
{Type: database.TargetTypeSlack, Config: "", MaxAttempts: 1},
|
||||
{Type: database.TargetTypeHTTP, Config: "not json"},
|
||||
{Type: database.TargetTypeHTTP, Config: `{}`},
|
||||
{Type: database.TargetTypeSlack, Config: ""},
|
||||
{
|
||||
Type: database.TargetTypeDatabase,
|
||||
Config: `{"expiry":"soon"}`,
|
||||
|
||||
@@ -53,10 +53,10 @@ type httpCore struct {
|
||||
}
|
||||
|
||||
// deliver runs one delivery attempt through the retry core.
|
||||
// A maxAttempts of 1 is a single attempt, recorded without
|
||||
// consulting a circuit breaker. A higher maxAttempts gates the
|
||||
// attempt on the circuit breaker and schedules a backed-off retry
|
||||
// on failure, until maxAttempts attempts have been made.
|
||||
// A maxRetries of 0 is fire-and-forget: a single attempt is
|
||||
// recorded and no circuit breaker is consulted. A positive
|
||||
// maxRetries gates the attempt on the circuit breaker and
|
||||
// schedules a backed-off retry on failure.
|
||||
//
|
||||
// ctx is the context attempt sends its request with. Stopping the
|
||||
// engine cancels it; see interruptedByStop.
|
||||
@@ -66,17 +66,17 @@ func (c *httpCore) deliver(
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
maxAttempts int,
|
||||
maxRetries int,
|
||||
attempt func() attemptResult,
|
||||
) {
|
||||
if maxAttempts == 1 {
|
||||
if maxRetries == 0 {
|
||||
c.fireAndForget(ctx, webhookDB, d, attempt())
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
c.withRetry(
|
||||
ctx, webhookDB, d, task, sched, maxAttempts, attempt,
|
||||
ctx, webhookDB, d, task, sched, maxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -153,7 +153,7 @@ func (c *httpCore) withRetry(
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
maxAttempts int,
|
||||
maxRetries int,
|
||||
attempt func() attemptResult,
|
||||
) {
|
||||
cb := c.getCircuitBreaker(task.TargetID)
|
||||
@@ -205,7 +205,7 @@ func (c *httpCore) withRetry(
|
||||
cb.RecordFailure()
|
||||
|
||||
c.handleRetry(
|
||||
webhookDB, d, task, sched, maxAttempts, attemptNum,
|
||||
webhookDB, d, task, sched, maxRetries, attemptNum,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -266,10 +266,10 @@ func (c *httpCore) handleRetry(
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
maxAttempts int,
|
||||
maxRetries int,
|
||||
attemptNum int,
|
||||
) {
|
||||
if attemptNum >= maxAttempts {
|
||||
if attemptNum >= maxRetries {
|
||||
c.eng.settleStatus(
|
||||
webhookDB, d, d.Target.Type,
|
||||
database.DeliveryStatusFailed,
|
||||
@@ -439,7 +439,7 @@ func (t *httpTarget) Deliver(
|
||||
|
||||
t.deliver(
|
||||
ctx, webhookDB, d, task, sched,
|
||||
d.Target.MaxAttempts, attempt,
|
||||
d.Target.MaxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -47,7 +47,7 @@ func deliverWithQuery(t *testing.T, cfg string) database.DeliveryResult {
|
||||
database.DeliveryStatusPending,
|
||||
)
|
||||
task := iTask(
|
||||
d, event, s.WebhookID, d.TargetID, "query", cfg, 1, 1, &event.Body,
|
||||
d, event, s.WebhookID, d.TargetID, "query", cfg, 0, 1, &event.Body,
|
||||
)
|
||||
task.RawQuery = eventQuery
|
||||
|
||||
@@ -144,9 +144,8 @@ func TestDeliverHTTP_ForwardedQueryKeepsTheTargetURLMasked(t *testing.T) {
|
||||
t.Cleanup(ts.Close)
|
||||
|
||||
target := &database.Target{
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: httpTargetConfig(t, ts.URL+"/in?token="+secret, true),
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: httpTargetConfig(t, ts.URL+"/in?token="+secret, true),
|
||||
}
|
||||
|
||||
deliverWithQuery(t, target.Config)
|
||||
|
||||
@@ -22,9 +22,8 @@ func TestRedactor_RemovesSlackWebhookURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := delivery.NewRedactor(&database.Target{
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
})
|
||||
|
||||
got := r.Redact("no_service for " + redactWebhookURL)
|
||||
@@ -53,9 +52,8 @@ func TestRedactor_RemovesSecretSeveredByACut(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := delivery.NewRedactor(&database.Target{
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
})
|
||||
|
||||
// Every cut position inside the credential, not just a
|
||||
@@ -98,7 +96,6 @@ func TestRedactor_RedactsCredentialShapedHeaderValues(
|
||||
`"X-Credential":"HHHHHHHHHHHH",` +
|
||||
`"Accept":"application/json",` +
|
||||
`"User-Agent":"webhooker/1.0"}}`,
|
||||
MaxAttempts: 1,
|
||||
})
|
||||
|
||||
for _, secret := range []string{
|
||||
@@ -139,7 +136,6 @@ func TestRedactor_IgnoresVeryShortHeaderValues(t *testing.T) {
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"https://example.com/in",` +
|
||||
`"headers":{"X-Api-Key":"ab"}}`,
|
||||
MaxAttempts: 1,
|
||||
})
|
||||
|
||||
const response = "rabbit"
|
||||
@@ -154,9 +150,8 @@ func TestRedactor_RemovesBarePath(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := delivery.NewRedactor(&database.Target{
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
})
|
||||
|
||||
got := r.Redact("POST " + redactSecretPath + " 404")
|
||||
@@ -188,9 +183,8 @@ func TestRedactor_RemovesHTTPURLQueryAndUserinfo(t *testing.T) {
|
||||
raw := dest.String()
|
||||
|
||||
r := delivery.NewRedactor(&database.Target{
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + raw + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + raw + `"}`,
|
||||
})
|
||||
|
||||
for _, echoed := range []string{
|
||||
@@ -238,9 +232,8 @@ func TestRedactor_RemovesEchoedQueryOfURLWithoutPath(t *testing.T) {
|
||||
"https://example.com?token=" + secret,
|
||||
} {
|
||||
r := delivery.NewRedactor(&database.Target{
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + dest + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + dest + `"}`,
|
||||
})
|
||||
|
||||
for echoed, want := range echoes {
|
||||
@@ -260,9 +253,8 @@ func TestRedactor_LeavesUnrelatedTextAlone(t *testing.T) {
|
||||
const response = "ok=false error=channel_not_found"
|
||||
|
||||
r := delivery.NewRedactor(&database.Target{
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeSlack,
|
||||
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
|
||||
})
|
||||
|
||||
assert.Equal(t, response, r.Redact(response))
|
||||
@@ -287,9 +279,8 @@ func TestRedactor_EmptyUserinfoDoesNotShredTheBody(t *testing.T) {
|
||||
require.Empty(t, parsed.User.String())
|
||||
|
||||
r := delivery.NewRedactor(&database.Target{
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + dest + `"}`,
|
||||
MaxAttempts: 1,
|
||||
Type: database.TargetTypeHTTP,
|
||||
Config: `{"url":"` + dest + `"}`,
|
||||
})
|
||||
|
||||
const body = "ok=false error=channel_not_found"
|
||||
@@ -323,8 +314,8 @@ func TestRedactor_ZeroValueAndConfiglessTargets(t *testing.T) {
|
||||
for _, tgt := range []database.Target{
|
||||
{Type: database.TargetTypeLog},
|
||||
{Type: database.TargetTypeDatabase},
|
||||
{Type: database.TargetTypeSlack, Config: "not json", MaxAttempts: 1},
|
||||
{Type: database.TargetTypeHTTP, Config: "", MaxAttempts: 1},
|
||||
{Type: database.TargetTypeSlack, Config: "not json"},
|
||||
{Type: database.TargetTypeHTTP, Config: ""},
|
||||
} {
|
||||
assert.Equal(
|
||||
t, text,
|
||||
|
||||
@@ -30,8 +30,9 @@ type SlackTargetConfig struct {
|
||||
// slackTarget delivers events to Slack incoming webhooks. It
|
||||
// formats the event into a Slack message and posts it as
|
||||
// JSON. It shares the retry core with the HTTP target: a
|
||||
// MaxAttempts of 1 is a single attempt, while a higher one
|
||||
// adds backoff and circuit breaking.
|
||||
// MaxRetries of 0 stays single-attempt fire-and-forget
|
||||
// (preserving existing Slack targets), while a positive
|
||||
// MaxRetries adds backoff and circuit breaking.
|
||||
type slackTarget struct {
|
||||
*httpCore
|
||||
|
||||
@@ -82,7 +83,7 @@ func (t *slackTarget) Deliver(
|
||||
|
||||
t.deliver(
|
||||
ctx, webhookDB, d, task, sched,
|
||||
d.Target.MaxAttempts, attempt,
|
||||
d.Target.MaxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -743,7 +743,7 @@ func TestSendRecoveredDeliveries_TargetMissingFromMap(
|
||||
|
||||
iCreateTarget(
|
||||
t, s.MainDB, targetID, s.WebhookID, "found-on-lookup",
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
event := iSeedEvent(
|
||||
@@ -790,7 +790,7 @@ func TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone(
|
||||
|
||||
iCreateTarget(
|
||||
t, s.MainDB, targetID, s.WebhookID, "healthy",
|
||||
database.TargetTypeLog, "", 1,
|
||||
database.TargetTypeLog, "", 0,
|
||||
)
|
||||
|
||||
event := iSeedEvent(
|
||||
|
||||
@@ -146,7 +146,7 @@ func openDB(
|
||||
}
|
||||
|
||||
// assertBounded holds every line the adapter wrote to the stated
|
||||
// ceiling and proves no line carried a whole client value.
|
||||
// ceiling and proves each was cut rather than merely short.
|
||||
func assertBounded(t *testing.T, out string) {
|
||||
t.Helper()
|
||||
|
||||
|
||||
@@ -308,7 +308,7 @@ func createReplayDelivery(
|
||||
TargetName: target.Name,
|
||||
TargetType: target.Type,
|
||||
TargetConfig: target.Config,
|
||||
MaxAttempts: target.MaxAttempts,
|
||||
MaxRetries: target.MaxRetries,
|
||||
Method: event.Method,
|
||||
RawQuery: event.RawQuery,
|
||||
Headers: event.Headers,
|
||||
|
||||
@@ -158,7 +158,8 @@ func TestHandleSourceLogs_RendersFailedAttempt(t *testing.T) {
|
||||
// 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.
|
||||
// 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()
|
||||
|
||||
|
||||
@@ -35,10 +35,11 @@ const eventBodyQuery = "SELECT cast(body as blob) " +
|
||||
// response is deliberately not renderable. Content-Disposition
|
||||
// makes the browser download rather than display it, and the
|
||||
// octet-stream type plus nosniff stop it being interpreted as
|
||||
// HTML or script. Without those a stored payload would be shown
|
||||
// as a page of the operator's own origin. The application's CSP
|
||||
// is a second defence behind them: it allows no inline script,
|
||||
// so such a page could not run script of its own.
|
||||
// HTML or script. Without those a stored payload would execute
|
||||
// as the logged-in operator. The application's CSP does not
|
||||
// help here: script-src allows 'unsafe-inline' from 'self', so
|
||||
// a document served from this origin could run its own inline
|
||||
// script.
|
||||
func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
webhook, ok := h.ownedWebhook(w, r)
|
||||
|
||||
@@ -84,7 +84,7 @@ func TestHandleEventDetail_ShowsEventWholeWithDeliveries(t *testing.T) {
|
||||
assert.Contains(t, page, strconv.Itoa(len(body))+" bytes")
|
||||
assert.Contains(t, page, "{\n "pad": "xxx")
|
||||
assert.Contains(t, page, ""tail": ""+sentinel+""\n}")
|
||||
assert.Contains(t, page, `break-all max-h-[32rem] overflow-y-auto"`)
|
||||
assert.Contains(t, page, `style="max-height: 32rem; overflow-y: auto"`)
|
||||
assert.NotContains(t, page, "Showing the first")
|
||||
assert.Contains(t, page, target.Name)
|
||||
assert.Contains(t, page, ">failed</span>")
|
||||
|
||||
@@ -496,7 +496,7 @@ const deliveryIDChunkSize = 500
|
||||
//
|
||||
// Each response body is cut by SQLite rather than in Go, for
|
||||
// the reason deliveryResultColumns gives. How many attempts a
|
||||
// delivery has is bounded by the target's MaxAttempts, which the
|
||||
// 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(
|
||||
|
||||
@@ -37,10 +37,10 @@ const MaxRenderedResponseBytesForTest = maxRenderedResponseBytes
|
||||
// per-delivery attempt ceiling to the handlers_test package.
|
||||
const MaxRenderedAttemptsForTest = maxRenderedAttempts
|
||||
|
||||
// MaxTargetAttemptsForTest exposes the target max_attempts ceiling to
|
||||
// MaxTargetRetriesForTest exposes the target max_retries ceiling to
|
||||
// the handlers_test package, so the tests assert against the constant
|
||||
// the handlers enforce rather than a number copied beside it.
|
||||
const MaxTargetAttemptsForTest = maxTargetAttempts
|
||||
const MaxTargetRetriesForTest = maxTargetRetries
|
||||
|
||||
// EventDBLeftMsgForTest and SidecarLeftMsgForTest expose the two
|
||||
// messages the webhook delete handler logs when a file of the event
|
||||
|
||||
@@ -1,15 +1,19 @@
|
||||
package handlers_test
|
||||
|
||||
// The handler-side half of the log-field audit. The slog calls in
|
||||
// this package that reach a value an UNAUTHENTICATED client picks
|
||||
// outright, and of a length it picks outright, are the failed-login
|
||||
// DEBUG lines, whose username is a form field.
|
||||
// The handler-side half of the log-field audit. Two slog calls in
|
||||
// this package reach a value an UNAUTHENTICATED client picks outright
|
||||
// and of a length it picks outright:
|
||||
//
|
||||
// They are at DEBUG, which is off in production by default. That is
|
||||
// - the unknown-entrypoint DEBUG line on /h/{uuid}, whose
|
||||
// path segment matched no stored entrypoint and so is bounded by
|
||||
// nothing;
|
||||
// - the failed-login DEBUG lines, whose username is a form field.
|
||||
//
|
||||
// Both are at DEBUG, which is off in production by default. That is
|
||||
// not a bound: an operator turning DEBUG on to diagnose a flood must
|
||||
// not thereby hand the flood an unbounded write. They spend the same
|
||||
// internal/logfield budget as the access log, and are held here to
|
||||
// middleware.MaxAccessLogLineBytes.
|
||||
// not thereby hand the flood an unbounded write. Both spend the same
|
||||
// internal/logfield budget as the access log, and both are held here
|
||||
// to middleware.MaxAccessLogLineBytes.
|
||||
//
|
||||
// The two login lines past the username lookup — "invalid password"
|
||||
// and "user logged in" — carry the same cap without needing it, since
|
||||
@@ -191,12 +195,13 @@ func receiverRouter(h *handlers.Handlers) *chi.Mux {
|
||||
return router
|
||||
}
|
||||
|
||||
// postReceiver sends one POST with an empty body at /h/<segment>
|
||||
// through router and returns the response status.
|
||||
// postReceiver sends one POST at /h/<segment>.
|
||||
//
|
||||
// RawPath is cleared after parsing so chi routes on the decoded path
|
||||
// and the handler sees the raw bytes rather than their percent-escaped
|
||||
// spelling.
|
||||
// spelling. That is the harder case for the budget: the escaped
|
||||
// spelling is plain ASCII, which costs one byte per byte, while the
|
||||
// decoded bytes are what the log handler has to escape.
|
||||
func postReceiver(
|
||||
t *testing.T, router *chi.Mux, segment string,
|
||||
) int {
|
||||
@@ -255,6 +260,43 @@ func postLoginWithPassword(
|
||||
return w.Code
|
||||
}
|
||||
|
||||
// TestUnknownEntrypoint_LogLineDoesNotTrackPathSize drives 8 KB of
|
||||
// client-chosen path at the unauthenticated receiver's
|
||||
// unknown-entrypoint DEBUG line and holds it to the same ceiling the
|
||||
// access log states.
|
||||
func TestUnknownEntrypoint_LogLineDoesNotTrackPathSize(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for handlerName, newHandler := range logHandlers() {
|
||||
for fillName, fill := range escapeFills() {
|
||||
t.Run(handlerName+"/"+fillName, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
h, buf := capturingHandlers(t, newHandler)
|
||||
router := receiverRouter(h)
|
||||
|
||||
for i := range floodRequests {
|
||||
assert.Equal(
|
||||
t,
|
||||
http.StatusNotFound,
|
||||
postReceiver(
|
||||
t, router,
|
||||
oversizedFill(fill)+
|
||||
strings.Repeat("y", i),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
lines := logLines(t, buf)
|
||||
require.Len(t, lines, floodRequests)
|
||||
assertNoClientText(t, buf)
|
||||
|
||||
assertBoundedFlood(t, buf.Len())
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestFailedLogin_LogLineDoesNotTrackUsernameSize drives 8 KB of
|
||||
// client-chosen username at the unauthenticated login endpoint's
|
||||
// DEBUG line and holds it to the same ceiling.
|
||||
|
||||
@@ -64,11 +64,10 @@ func seedTarget(
|
||||
t.Helper()
|
||||
|
||||
tgt := &database.Target{
|
||||
WebhookID: webhookID,
|
||||
Name: "t-" + string(targetType),
|
||||
Type: targetType,
|
||||
Active: true,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: webhookID,
|
||||
Name: "t-" + string(targetType),
|
||||
Type: targetType,
|
||||
Active: true,
|
||||
}
|
||||
|
||||
require.NoError(
|
||||
|
||||
@@ -37,12 +37,11 @@ func seedConfiguredTarget(
|
||||
t.Helper()
|
||||
|
||||
tgt := &database.Target{
|
||||
WebhookID: webhookID,
|
||||
Name: "t-" + string(targetType),
|
||||
Type: targetType,
|
||||
Active: true,
|
||||
Config: config,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: webhookID,
|
||||
Name: "t-" + string(targetType),
|
||||
Type: targetType,
|
||||
Active: true,
|
||||
Config: config,
|
||||
}
|
||||
|
||||
require.NoError(
|
||||
@@ -274,7 +273,7 @@ func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
|
||||
|
||||
assert.Contains(
|
||||
t, body,
|
||||
`<div class="max-w-[108rem] mx-auto px-6 py-8">`,
|
||||
`<div class="mx-auto px-6 py-8" style="max-width: 108rem"`,
|
||||
)
|
||||
assert.Contains(
|
||||
t, body,
|
||||
@@ -287,8 +286,8 @@ func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
|
||||
// target and says what deleting it loses, that the webhook's gives its
|
||||
// number of stored events (5 received, 2 removed by retention, so 3,
|
||||
// the statistics pane's "Within retention" figure), and that an
|
||||
// entrypoint with no description is named by its URL. Each prompt is
|
||||
// its form's data-confirm attribute, where is a line break.
|
||||
// entrypoint with no description is named by its URL. The template
|
||||
// writes the slashes after http: as \/, which the browser reads as /.
|
||||
func TestHandleSourceDetail_DeletePromptsNameWhatIsLost(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -326,28 +325,29 @@ func TestHandleSourceDetail_DeletePromptsNameWhatIsLost(t *testing.T) {
|
||||
body := renderSourceDetailPage(t, h, sess, wh.ID)
|
||||
|
||||
assert.Contains(t, body,
|
||||
`data-confirm="Delete webhook "delete-me"? `+
|
||||
`Delete webhook "delete-me"?\n\n`+
|
||||
`This deletes its stored events (3) and their deliveries. `+
|
||||
`Any archive files it wrote are kept."`)
|
||||
`Any archive files it wrote are kept.`)
|
||||
assert.Contains(t, body,
|
||||
`data-confirm="Delete entrypoint "Stripe"? `+
|
||||
`Delete entrypoint "Stripe"?\n\n`+
|
||||
`Senders using its URL get an error from now on, `+
|
||||
`and the URL cannot be restored."`)
|
||||
`and the URL cannot be restored.`)
|
||||
assert.Contains(t, body,
|
||||
`data-confirm="Delete entrypoint "http://example.com/h/`+
|
||||
unnamed.Path+`"? `)
|
||||
`Delete entrypoint "http:\/\/example.com/h/`+
|
||||
unnamed.Path+`"?`)
|
||||
assert.Contains(t, body,
|
||||
`data-confirm="Delete target "t-log"? `+
|
||||
`Delete target "t-log"?\n\n`+
|
||||
`Nothing more is delivered to it. `+
|
||||
`Its past deliveries stay in the event log."`)
|
||||
`Its past deliveries stay in the event log.`)
|
||||
}
|
||||
|
||||
// TestHandleSourceDetail_DeletePromptKeepsQuotesInName checks that a
|
||||
// webhook name with quotes, a backslash, a closing script tag and a
|
||||
// newline reaches its delete prompt escaped for the attribute, which
|
||||
// the browser reads back as the name typed: each quote and angle
|
||||
// bracket as a character reference, the rest as typed. An unescaped
|
||||
// double quote would end the attribute and cut the prompt short.
|
||||
// newline reaches its delete prompt escaped for the script, which the
|
||||
// browser reads back as the name typed: each quote and angle bracket
|
||||
// as a \u escape, the slash as \/, the newline as \n and the backslash
|
||||
// doubled. An unescaped newline would break the prompt's script, and
|
||||
// the form would then submit without asking.
|
||||
func TestHandleSourceDetail_DeletePromptKeepsQuotesInName(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -373,6 +373,6 @@ func TestHandleSourceDetail_DeletePromptKeepsQuotesInName(t *testing.T) {
|
||||
body := renderSourceDetailPage(t, h, sess, wh.ID)
|
||||
|
||||
assert.Contains(t, body,
|
||||
"Delete webhook "Bob's "best" \\ hook"+
|
||||
"</script>\nline two"?")
|
||||
"Delete webhook "Bob\\u0027s \\u0022best\\u0022 \\\\ hook"+
|
||||
"\\u003c\\/script\\u003e\\nline two"?")
|
||||
}
|
||||
|
||||
@@ -1,81 +0,0 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// maxTargetAttempts bounds a target's max_attempts.
|
||||
//
|
||||
// Both target forms already declare max="20" on the input, so this
|
||||
// enforces server-side what the UI has always advertised rather than
|
||||
// introducing a new limit.
|
||||
//
|
||||
// The number is not cosmetic. Every attempt writes a delivery_results
|
||||
// row that the event log then loads and renders, and the engine backs
|
||||
// off by 2^(n-1) seconds, so attempt 20 is already about six days
|
||||
// after the first. A value beyond this buys no additional durability
|
||||
// and only costs rows.
|
||||
const maxTargetAttempts = 20
|
||||
|
||||
// Errors returned when a max_attempts form value cannot be turned into
|
||||
// a number of attempts.
|
||||
var (
|
||||
// errAttemptsInvalid signals a max_attempts form value that is not
|
||||
// a whole number of at least 1.
|
||||
errAttemptsInvalid = errors.New(
|
||||
"must be a whole number, at least 1",
|
||||
)
|
||||
|
||||
// errAttemptsTooLarge signals a max_attempts form value that is a
|
||||
// whole number but above maxTargetAttempts. It is distinguished
|
||||
// from errAttemptsInvalid so the message can name the ceiling
|
||||
// instead of implying the input was not a number.
|
||||
errAttemptsTooLarge = errors.New("out of range")
|
||||
)
|
||||
|
||||
// parseMaxAttempts interprets a max_attempts form value.
|
||||
//
|
||||
// An ABSENT value — the field empty or not submitted — yields
|
||||
// fallback, which lets the create path apply its default and the edit
|
||||
// path leave the stored value alone. A value that is SET BUT INVALID
|
||||
// is an error: unparseable, below 1, or above maxTargetAttempts.
|
||||
//
|
||||
// The distinction is the whole point of this function. Returning a
|
||||
// default for input the operator typed but that did not parse would
|
||||
// silently turn retries off on a store-and-forward proxy — and on the
|
||||
// edit path destroy a working configuration over a typo. A default
|
||||
// answers a question that was not asked; it never answers one that
|
||||
// was asked badly.
|
||||
func parseMaxAttempts(raw string, fallback int) (int, error) {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
return fallback, nil
|
||||
}
|
||||
|
||||
v, err := strconv.Atoi(raw)
|
||||
if err != nil || v < 1 {
|
||||
return 0, errAttemptsInvalid
|
||||
}
|
||||
|
||||
if v > maxTargetAttempts {
|
||||
return 0, errAttemptsTooLarge
|
||||
}
|
||||
|
||||
return v, nil
|
||||
}
|
||||
|
||||
// attemptsErrorMessage returns the message the create and edit forms
|
||||
// show for a rejected max_attempts value. Any error other than
|
||||
// errAttemptsTooLarge falls back to the generic wording, so an
|
||||
// unrecognised parse failure still produces a sensible 400.
|
||||
func attemptsErrorMessage(err error) string {
|
||||
if errors.Is(err, errAttemptsTooLarge) {
|
||||
return errAttemptsTooLarge.Error() +
|
||||
": at most " + strconv.Itoa(maxTargetAttempts) +
|
||||
" attempts"
|
||||
}
|
||||
|
||||
return errAttemptsInvalid.Error()
|
||||
}
|
||||
@@ -1,429 +0,0 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/handlers"
|
||||
)
|
||||
|
||||
// attemptsTargetURL is the destination the attempts-validation targets
|
||||
// point at. It is a literal public address rather than a hostname so
|
||||
// the SSRF check resolves nothing and a sandbox without DNS cannot
|
||||
// make these cases pass or fail for the wrong reason.
|
||||
const attemptsTargetURL = "https://93.184.216.34/hooks/attempts"
|
||||
|
||||
const (
|
||||
// wayAboveCeiling is the typo'd-extra-zero case from the report.
|
||||
wayAboveCeiling = "999999999"
|
||||
|
||||
// notANumber is the plainest garbage an operator can type, and
|
||||
// the value the report submitted on the edit form.
|
||||
notANumber = "abc"
|
||||
|
||||
// workingAttempts is the number of attempts a seeded target is
|
||||
// already delivering with, which a rejected submission must not
|
||||
// disturb.
|
||||
workingAttempts = 2
|
||||
)
|
||||
|
||||
// aboveCeiling is the smallest rejected whole number.
|
||||
func aboveCeiling() string {
|
||||
return strconv.Itoa(handlers.MaxTargetAttemptsForTest + 1)
|
||||
}
|
||||
|
||||
// overCeilingAttempts is whole-number input past the limit, which is
|
||||
// rejected with the limit named.
|
||||
func overCeilingAttempts() []string {
|
||||
return []string{aboveCeiling(), wayAboveCeiling}
|
||||
}
|
||||
|
||||
// belowOneAttempts is whole-number input under the floor of one
|
||||
// attempt, which is rejected with the floor named.
|
||||
func belowOneAttempts() []string {
|
||||
return []string{"0", "-5"}
|
||||
}
|
||||
|
||||
// unparseableAttempts is input an operator can type into the field
|
||||
// that is not a number of attempts. Each must be REJECTED: silently
|
||||
// reading any of them as a default turns retries off on a
|
||||
// store-and-forward proxy without saying so.
|
||||
//
|
||||
// The twenty-digit case is here because it parses as digits but
|
||||
// overflows int, which is the one failure the field's own min/max
|
||||
// attributes cannot describe.
|
||||
func unparseableAttempts() []string {
|
||||
return []string{
|
||||
notANumber,
|
||||
"2.7",
|
||||
"12345678901234567890",
|
||||
"1e3",
|
||||
}
|
||||
}
|
||||
|
||||
// createAttemptsForm is a complete, otherwise-valid HTTP target
|
||||
// creation, so the only thing any case below varies is max_attempts.
|
||||
func createAttemptsForm(attempts string) url.Values {
|
||||
form := url.Values{}
|
||||
form.Set("name", "attempts-target")
|
||||
form.Set("type", string(database.TargetTypeHTTP))
|
||||
form.Set("url", attemptsTargetURL)
|
||||
|
||||
if attempts != absentField {
|
||||
form.Set("max_attempts", attempts)
|
||||
}
|
||||
|
||||
return form
|
||||
}
|
||||
|
||||
// absentField marks a field the form does not submit at all, which is
|
||||
// the case that legitimately takes a default and must stay distinct
|
||||
// from a field submitted with garbage in it.
|
||||
const absentField = "\x00absent"
|
||||
|
||||
// absentAttempts is every way of saying "the operator did not set
|
||||
// this", each of which takes the default rather than a 400. Blank and
|
||||
// whitespace-only count as absent here because they do in the timeout
|
||||
// and retention controls on the same forms; a rule the fields do not
|
||||
// share would be its own surprise.
|
||||
func absentAttempts() []string {
|
||||
return []string{absentField, "", " "}
|
||||
}
|
||||
|
||||
// createWithAttempts posts the target create form for a fresh webhook
|
||||
// and returns the webhook and the response.
|
||||
func createWithAttempts(
|
||||
t *testing.T,
|
||||
env *sourceTestEnv,
|
||||
attempts string,
|
||||
) (database.Webhook, int, string) {
|
||||
t.Helper()
|
||||
|
||||
webhook := seedWebhookWithRetention(t, env.db, 30)
|
||||
|
||||
w := serveTarget(
|
||||
env, http.MethodPost,
|
||||
"/hook/"+webhook.ID+"/targets",
|
||||
createAttemptsForm(attempts),
|
||||
)
|
||||
|
||||
return webhook, w.Code, w.Body.String()
|
||||
}
|
||||
|
||||
// TestTargetCreate_AttemptsAboveCeilingRejected proves the create form
|
||||
// enforces a ceiling at all, and that the 400 names it — a rejection
|
||||
// that does not say what the limit is leaves the operator guessing.
|
||||
func TestTargetCreate_AttemptsAboveCeilingRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
ceiling := strconv.Itoa(handlers.MaxTargetAttemptsForTest)
|
||||
|
||||
for _, attempts := range overCeilingAttempts() {
|
||||
webhook, code, body := createWithAttempts(t, env, attempts)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, code,
|
||||
"max_attempts=%s should be rejected", attempts)
|
||||
assert.Contains(t, body, ceiling,
|
||||
"the rejection for %s should name the ceiling",
|
||||
attempts)
|
||||
assert.Empty(t,
|
||||
targetsForWebhook(t, env.db, webhook.ID),
|
||||
"no target should be created for %s", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_AttemptsBelowOneRejected proves the create form
|
||||
// refuses 0 and less, naming the floor: every delivery is attempted
|
||||
// at least once.
|
||||
func TestTargetCreate_AttemptsBelowOneRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, attempts := range belowOneAttempts() {
|
||||
webhook, code, body := createWithAttempts(t, env, attempts)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, code,
|
||||
"max_attempts=%s should be rejected", attempts)
|
||||
assert.Contains(t, body, "at least 1",
|
||||
"the rejection for %s should name the floor",
|
||||
attempts)
|
||||
assert.Empty(t,
|
||||
targetsForWebhook(t, env.db, webhook.ID),
|
||||
"no target should be created for %s", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_UnparseableAttemptsRejected is the core of the
|
||||
// defect: each of these was accepted with HTTP 200 and stored as a
|
||||
// default.
|
||||
func TestTargetCreate_UnparseableAttemptsRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, attempts := range unparseableAttempts() {
|
||||
webhook, code, body := createWithAttempts(t, env, attempts)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, code,
|
||||
"max_attempts=%q should be rejected, not coerced",
|
||||
attempts)
|
||||
assert.Contains(t, body, "whole number",
|
||||
"the rejection for %q should say why", attempts)
|
||||
assert.Empty(t,
|
||||
targetsForWebhook(t, env.db, webhook.ID),
|
||||
"no target should be created for %q", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_ValidAttemptsStored covers the accepting half,
|
||||
// including both bounds: a bound that rejects its own limit would
|
||||
// make the advertised range unreachable.
|
||||
func TestTargetCreate_ValidAttemptsStored(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, want := range []int{1, 3, handlers.MaxTargetAttemptsForTest} {
|
||||
webhook, code, body := createWithAttempts(
|
||||
t, env, strconv.Itoa(want),
|
||||
)
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, code, body)
|
||||
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
assert.Equal(t, want, targets[0].MaxAttempts)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_AbsentAttemptsTakesDefault keeps the two cases
|
||||
// distinct. An omitted field is not an operator asking for something
|
||||
// invalid, so it still gets the default of one attempt rather than a
|
||||
// 400 — otherwise the fix above would make the form unusable.
|
||||
func TestTargetCreate_AbsentAttemptsTakesDefault(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, attempts := range absentAttempts() {
|
||||
webhook, code, body := createWithAttempts(t, env, attempts)
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, code, body)
|
||||
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
assert.Equal(t, 1, targets[0].MaxAttempts,
|
||||
"an absent max_attempts should take the default")
|
||||
}
|
||||
}
|
||||
|
||||
// seedAttemptsTarget creates an HTTP target already delivering with
|
||||
// workingAttempts attempts, through the real create handler.
|
||||
func seedAttemptsTarget(
|
||||
t *testing.T,
|
||||
env *sourceTestEnv,
|
||||
) (database.Webhook, database.Target) {
|
||||
t.Helper()
|
||||
|
||||
webhook, code, body := createWithAttempts(
|
||||
t, env, strconv.Itoa(workingAttempts),
|
||||
)
|
||||
require.Equal(t, http.StatusSeeOther, code, body)
|
||||
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
require.Equal(t, workingAttempts, targets[0].MaxAttempts)
|
||||
|
||||
return webhook, targets[0]
|
||||
}
|
||||
|
||||
// editAttemptsForm is a complete edit submission that changes the
|
||||
// target's name as well, so a rejected submission can be shown to
|
||||
// have written nothing at all rather than merely to have left
|
||||
// max_attempts alone.
|
||||
func editAttemptsForm(attempts string) url.Values {
|
||||
form := url.Values{}
|
||||
form.Set("name", "renamed-by-edit")
|
||||
form.Set("url", attemptsTargetURL)
|
||||
|
||||
if attempts != absentField {
|
||||
form.Set("max_attempts", attempts)
|
||||
}
|
||||
|
||||
return form
|
||||
}
|
||||
|
||||
// assertEditRejectedAndUnchanged submits an edit expected to fail and
|
||||
// checks both halves of the requirement: the 400 explains itself, and
|
||||
// the target it was submitted against is untouched.
|
||||
func assertEditRejectedAndUnchanged(
|
||||
t *testing.T,
|
||||
env *sourceTestEnv,
|
||||
attempts, wantReason string,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
webhook, target := seedAttemptsTarget(t, env)
|
||||
|
||||
w := submitTargetEdit(
|
||||
env, webhook.ID, target.ID, editAttemptsForm(attempts),
|
||||
)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, w.Code,
|
||||
"max_attempts=%q should be rejected on edit", attempts)
|
||||
assert.Contains(t, w.Body.String(), wantReason,
|
||||
"the rejection for %q should say why", attempts)
|
||||
|
||||
stored := storedTarget(t, env, target.ID)
|
||||
assert.Equal(t, workingAttempts, stored.MaxAttempts,
|
||||
"a rejected edit must not destroy the working number "+
|
||||
"of attempts with %q", attempts)
|
||||
assert.Equal(t, "attempts-target", stored.Name,
|
||||
"a rejected edit must write nothing at all")
|
||||
}
|
||||
|
||||
// TestTargetEdit_UnparseableAttemptsRejected is the damaging half of
|
||||
// the defect. A target delivering with retries, re-saved with a typo
|
||||
// in the field, returned 200 and was left with retries disabled and
|
||||
// nothing said.
|
||||
func TestTargetEdit_UnparseableAttemptsRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, attempts := range unparseableAttempts() {
|
||||
assertEditRejectedAndUnchanged(
|
||||
t, env, attempts, "whole number",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetEdit_AttemptsBelowOneRejected proves the floor applies
|
||||
// to the edit path too.
|
||||
func TestTargetEdit_AttemptsBelowOneRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, attempts := range belowOneAttempts() {
|
||||
assertEditRejectedAndUnchanged(t, env, attempts, "at least 1")
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetEdit_AttemptsAboveCeilingRejected proves the ceiling
|
||||
// applies to the edit path too, naming itself, so the two paths
|
||||
// cannot disagree about what is storable.
|
||||
func TestTargetEdit_AttemptsAboveCeilingRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
ceiling := strconv.Itoa(handlers.MaxTargetAttemptsForTest)
|
||||
|
||||
for _, attempts := range overCeilingAttempts() {
|
||||
assertEditRejectedAndUnchanged(t, env, attempts, ceiling)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetEdit_ValidAttemptsStored covers the accepting half of the
|
||||
// edit path, so the bounds cannot be enforced by simply refusing
|
||||
// every submission.
|
||||
func TestTargetEdit_ValidAttemptsStored(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, want := range []int{1, 9, handlers.MaxTargetAttemptsForTest} {
|
||||
webhook, target := seedAttemptsTarget(t, env)
|
||||
|
||||
w := submitTargetEdit(
|
||||
env, webhook.ID, target.ID,
|
||||
editAttemptsForm(strconv.Itoa(want)),
|
||||
)
|
||||
require.Equal(t,
|
||||
http.StatusSeeOther, w.Code, w.Body.String(),
|
||||
)
|
||||
|
||||
assert.Equal(t, want,
|
||||
storedTarget(t, env, target.ID).MaxAttempts)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetEdit_AbsentAttemptsLeavesStoredValue is the edit path's
|
||||
// absent-versus-invalid case. The field is only offered by the forms
|
||||
// for types that retry, so a submission without it must leave the
|
||||
// stored count alone rather than be rejected or reset.
|
||||
func TestTargetEdit_AbsentAttemptsLeavesStoredValue(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, attempts := range absentAttempts() {
|
||||
webhook, target := seedAttemptsTarget(t, env)
|
||||
|
||||
w := submitTargetEdit(
|
||||
env, webhook.ID, target.ID,
|
||||
editAttemptsForm(attempts),
|
||||
)
|
||||
require.Equal(t,
|
||||
http.StatusSeeOther, w.Code, w.Body.String(),
|
||||
)
|
||||
|
||||
assert.Equal(t, workingAttempts,
|
||||
storedTarget(t, env, target.ID).MaxAttempts,
|
||||
"an absent max_attempts must leave the stored "+
|
||||
"count alone (%q)", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetAttempts_CreateAndEditAgreeOnEveryCase proves the two
|
||||
// paths cannot disagree, which is what let the create form and the
|
||||
// edit form drift apart in the first place. Every input is submitted
|
||||
// to both and the accept/reject verdicts are compared.
|
||||
func TestTargetAttempts_CreateAndEditAgreeOnEveryCase(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
accepted := []string{
|
||||
"1", "2",
|
||||
strconv.Itoa(handlers.MaxTargetAttemptsForTest),
|
||||
}
|
||||
overCeiling := overCeilingAttempts()
|
||||
belowOne := belowOneAttempts()
|
||||
unparseable := unparseableAttempts()
|
||||
|
||||
cases := make(
|
||||
[]string, 0,
|
||||
len(accepted)+len(overCeiling)+len(belowOne)+
|
||||
len(unparseable),
|
||||
)
|
||||
cases = append(cases, accepted...)
|
||||
cases = append(cases, overCeiling...)
|
||||
cases = append(cases, belowOne...)
|
||||
cases = append(cases, unparseable...)
|
||||
|
||||
for _, attempts := range cases {
|
||||
_, createCode, _ := createWithAttempts(t, env, attempts)
|
||||
|
||||
webhook, target := seedAttemptsTarget(t, env)
|
||||
editCode := submitTargetEdit(
|
||||
env, webhook.ID, target.ID,
|
||||
editAttemptsForm(attempts),
|
||||
).Code
|
||||
|
||||
assert.Equal(t,
|
||||
createCode == http.StatusBadRequest,
|
||||
editCode == http.StatusBadRequest,
|
||||
"create and edit must agree on max_attempts=%q "+
|
||||
"(create %d, edit %d)",
|
||||
attempts, createCode, editCode,
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -102,10 +102,9 @@ func (h *Handlers) newTarget(
|
||||
in targetFormInput,
|
||||
) (*database.Target, string, error) {
|
||||
target := &database.Target{
|
||||
WebhookID: webhookID,
|
||||
Type: in.Type,
|
||||
Active: true,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: webhookID,
|
||||
Type: in.Type,
|
||||
Active: true,
|
||||
}
|
||||
|
||||
errMsg, err := h.setTargetFromForm(ctx, target, in)
|
||||
@@ -118,10 +117,9 @@ func (h *Handlers) newTarget(
|
||||
|
||||
// setTargetFromForm validates a target form against the target's type
|
||||
// and, when it accepts it, sets the target's name, configuration and
|
||||
// number of delivery attempts from it. It returns the message the form
|
||||
// shows for anything it refuses, an unknown type among them, and then
|
||||
// leaves the target unchanged; an error is the server's fault, as for
|
||||
// newTarget.
|
||||
// retry count from it. It returns the message the form shows for
|
||||
// anything it refuses, an unknown type among them, and then leaves the
|
||||
// target unchanged; an error is the server's fault, as for newTarget.
|
||||
// The add target form and the target edit form both go through here,
|
||||
// so the two cannot come to disagree about what a target may be.
|
||||
func (h *Handlers) setTargetFromForm(
|
||||
@@ -138,20 +136,20 @@ func (h *Handlers) setTargetFromForm(
|
||||
return errMsg, err
|
||||
}
|
||||
|
||||
// An empty max_attempts keeps the target's count: the default of
|
||||
// 1 for a new target, and the stored count for an edited one,
|
||||
// since the forms for target types that do not retry have no such
|
||||
// field. A value that is filled in but invalid is refused rather
|
||||
// than becoming that count, so a typo cannot destroy the count a
|
||||
// target is delivering with.
|
||||
maxAttempts, err := parseMaxAttempts(in.MaxAttempts, target.MaxAttempts)
|
||||
// An empty max_retries keeps the target's count: the
|
||||
// fire-and-forget default of 0 for a new target, and the stored
|
||||
// count for an edited one, since the forms for target types that
|
||||
// do not retry have no such field. A value that is filled in but
|
||||
// invalid is refused rather than becoming that count, so a typo
|
||||
// cannot destroy the count a target is delivering with.
|
||||
maxRetries, err := parseMaxRetries(in.MaxRetries, target.MaxRetries)
|
||||
if err != nil {
|
||||
return "Invalid delivery attempts: " + attemptsErrorMessage(err), nil
|
||||
return "Invalid delivery attempts: " + retriesErrorMessage(err), nil
|
||||
}
|
||||
|
||||
target.Name = in.Name
|
||||
target.Config = configJSON
|
||||
target.MaxAttempts = maxAttempts
|
||||
target.MaxRetries = maxRetries
|
||||
|
||||
return "", nil
|
||||
}
|
||||
@@ -178,8 +176,8 @@ type targetFormInput struct {
|
||||
// ForwardQuery is an HTTP target's checkbox that passes each
|
||||
// event's query string on to it.
|
||||
ForwardQuery bool
|
||||
// MaxAttempts is an HTTP or Slack target's max_attempts.
|
||||
MaxAttempts string
|
||||
// MaxRetries is an HTTP or Slack target's max_retries.
|
||||
MaxRetries string
|
||||
// Expiry is a database (archive) target's row expiry.
|
||||
Expiry string
|
||||
// Rotation is a database (archive) target's rotation.
|
||||
@@ -206,7 +204,7 @@ func targetFormInputFrom(r *http.Request) targetFormInput {
|
||||
Headers: r.PostFormValue("headers"),
|
||||
Timeout: r.PostFormValue("timeout"),
|
||||
ForwardQuery: r.PostFormValue("forward_query") != "",
|
||||
MaxAttempts: r.PostFormValue("max_attempts"),
|
||||
MaxRetries: r.PostFormValue("max_retries"),
|
||||
Expiry: r.PostFormValue("expiry"),
|
||||
Rotation: r.PostFormValue("rotation"),
|
||||
}
|
||||
|
||||
@@ -196,7 +196,7 @@ func TestHandleTargetCreate_QueryStringCannotSupplyNameOrType(
|
||||
|
||||
w, _ := postTargetCreate(
|
||||
t, env, webhook.ID,
|
||||
"name=leaky&type=slack&max_attempts=9&expiry=30d&url="+
|
||||
"name=leaky&type=slack&max_retries=9&expiry=30d&url="+
|
||||
url.QueryEscape(targetSecretURL),
|
||||
url.Values{},
|
||||
)
|
||||
|
||||
@@ -21,24 +21,24 @@ func TestHandleTargetCreate_EveryType(t *testing.T) {
|
||||
|
||||
// fields is the rest of each submission, as a query string.
|
||||
cases := []struct {
|
||||
targetType database.TargetType
|
||||
fields string
|
||||
wantConfig string
|
||||
wantAttempts int
|
||||
targetType database.TargetType
|
||||
fields string
|
||||
wantConfig string
|
||||
wantRetries int
|
||||
}{
|
||||
{
|
||||
database.TargetTypeHTTP, "timeout=12&max_attempts=3",
|
||||
database.TargetTypeHTTP, "timeout=12&max_retries=3",
|
||||
`{"url":"` + editOriginalURL + `","timeout":12}`, 3,
|
||||
},
|
||||
{
|
||||
database.TargetTypeSlack, "max_attempts=4",
|
||||
database.TargetTypeSlack, "max_retries=4",
|
||||
`{"webhookUrl":"` + editOriginalURL + `"}`, 4,
|
||||
},
|
||||
{
|
||||
database.TargetTypeDatabase, "expiry=720h",
|
||||
`{"expiry":"720h"}`, 1,
|
||||
`{"expiry":"720h"}`, 0,
|
||||
},
|
||||
{database.TargetTypeLog, "", "", 1},
|
||||
{database.TargetTypeLog, "", "", 0},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
@@ -62,7 +62,7 @@ func TestHandleTargetCreate_EveryType(t *testing.T) {
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
assert.Equal(t, tc.targetType, targets[0].Type)
|
||||
assert.Equal(t, tc.wantAttempts, targets[0].MaxAttempts)
|
||||
assert.Equal(t, tc.wantRetries, targets[0].MaxRetries)
|
||||
|
||||
if tc.wantConfig == "" {
|
||||
assert.Empty(t, targets[0].Config)
|
||||
@@ -90,11 +90,11 @@ func TestHandleTargetCreate_RefusedFormComesBack(t *testing.T) {
|
||||
{
|
||||
database.TargetTypeHTTP,
|
||||
"name=private&url=" + editBlockedURL +
|
||||
"&timeout=12&max_attempts=3",
|
||||
"&timeout=12&max_retries=3",
|
||||
"Invalid target URL",
|
||||
},
|
||||
{
|
||||
database.TargetTypeSlack, "name=no-url&max_attempts=4",
|
||||
database.TargetTypeSlack, "name=no-url&max_retries=4",
|
||||
"Webhook URL is required for Slack targets",
|
||||
},
|
||||
{
|
||||
@@ -133,8 +133,8 @@ func TestHandleTargetCreate_RefusedFormComesBack(t *testing.T) {
|
||||
assert.Contains(t, page, html.EscapeString(tc.reason))
|
||||
|
||||
// Each value comes back in a data attribute of the targets
|
||||
// section named after its field (max_attempts as
|
||||
// data-max-attempts), except url, which comes back in
|
||||
// section named after its field (max_retries as
|
||||
// data-max-retries), except url, which comes back in
|
||||
// data-destination; templates/source_detail.html says why.
|
||||
for field := range typed {
|
||||
attr := "data-" + strings.ReplaceAll(field, "_", "-")
|
||||
|
||||
@@ -82,7 +82,7 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
|
||||
Headers: cfg.Headers,
|
||||
Timeout: cfg.Timeout,
|
||||
ForwardQuery: cfg.ForwardQuery,
|
||||
MaxAttempts: strconv.Itoa(target.MaxAttempts),
|
||||
MaxRetries: strconv.Itoa(target.MaxRetries),
|
||||
Expiry: cfg.Expiry,
|
||||
Rotation: cfg.Rotation,
|
||||
}
|
||||
|
||||
@@ -117,7 +117,7 @@ func seedHTTPTarget(
|
||||
form.Set("url", editOriginalURL)
|
||||
form.Set("headers", headers)
|
||||
form.Set("timeout", timeout)
|
||||
form.Set("max_attempts", "3")
|
||||
form.Set("max_retries", "3")
|
||||
|
||||
w := serveTarget(
|
||||
env, http.MethodPost,
|
||||
@@ -179,7 +179,7 @@ func editForm(targetURL, headers, timeout string) url.Values {
|
||||
form.Set("url", targetURL)
|
||||
form.Set("headers", headers)
|
||||
form.Set("timeout", timeout)
|
||||
form.Set("max_attempts", "5")
|
||||
form.Set("max_retries", "5")
|
||||
|
||||
return form
|
||||
}
|
||||
@@ -280,9 +280,9 @@ func TestHandleTargetEditSubmit_Saves(t *testing.T) {
|
||||
assertEditClearingRemovesThem(t, env)
|
||||
})
|
||||
|
||||
t.Run("absent max_attempts is kept", func(t *testing.T) {
|
||||
t.Run("absent max_retries is not zeroed", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
assertEditKeepsAbsentMaxAttempts(t, env)
|
||||
assertEditKeepsAbsentMaxRetries(t, env)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -307,7 +307,7 @@ func assertEditChangesDestination(
|
||||
|
||||
reloaded := storedTarget(t, env, target.ID)
|
||||
assert.Equal(t, "edited-name", reloaded.Name)
|
||||
assert.Equal(t, 5, reloaded.MaxAttempts)
|
||||
assert.Equal(t, 5, reloaded.MaxRetries)
|
||||
assert.Equal(
|
||||
t, database.TargetTypeHTTP, reloaded.Type,
|
||||
"the edit form must not change a target's type",
|
||||
@@ -368,25 +368,25 @@ func assertEditClearingRemovesThem(
|
||||
assert.Zero(t, cfg.Timeout)
|
||||
}
|
||||
|
||||
// Delivery attempts are offered only by the forms for target types
|
||||
// that retry. An absent field means the form does not edit them, not
|
||||
// that retries should be turned off.
|
||||
func assertEditKeepsAbsentMaxAttempts(
|
||||
// Retries are offered only by the forms for target types that retry.
|
||||
// An absent field means the form does not edit retries, not that
|
||||
// they should be turned off.
|
||||
func assertEditKeepsAbsentMaxRetries(
|
||||
t *testing.T, env *sourceTestEnv,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
webhook, target := seedHTTPTarget(t, env, "", "")
|
||||
require.Equal(t, 3, target.MaxAttempts)
|
||||
require.Equal(t, 3, target.MaxRetries)
|
||||
|
||||
form := editForm(editOriginalURL, "", "")
|
||||
form.Del("max_attempts")
|
||||
form.Del("max_retries")
|
||||
|
||||
w := submitTargetEdit(env, webhook.ID, target.ID, form)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
||||
|
||||
assert.Equal(
|
||||
t, 3, storedTarget(t, env, target.ID).MaxAttempts,
|
||||
t, 3, storedTarget(t, env, target.ID).MaxRetries,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -659,12 +659,12 @@ func TestHandleTargetEditSubmit_RefusedFormComesBack(t *testing.T) {
|
||||
{
|
||||
database.TargetTypeHTTP,
|
||||
"name=edited&url=" + editBlockedURL +
|
||||
"&headers=X-Edited:+kept&timeout=12&max_attempts=3",
|
||||
"&headers=X-Edited:+kept&timeout=12&max_retries=3",
|
||||
"Invalid target URL",
|
||||
},
|
||||
{
|
||||
database.TargetTypeSlack,
|
||||
"name=edited&url=" + editOriginalURL + "&max_attempts=25",
|
||||
"name=edited&url=" + editOriginalURL + "&max_retries=25",
|
||||
"Invalid delivery attempts",
|
||||
},
|
||||
{
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// maxTargetRetries bounds a target's max_retries.
|
||||
//
|
||||
// Both target forms already declare max="20" on the input, so this
|
||||
// enforces server-side what the UI has always advertised rather than
|
||||
// introducing a new limit.
|
||||
//
|
||||
// The number is not cosmetic. Every attempt writes a delivery_results
|
||||
// row that the event log then loads and renders, and the engine backs
|
||||
// off by 2^(n-1) seconds, so attempt 20 is already about six days
|
||||
// after the first. A value beyond this buys no additional durability
|
||||
// and only costs rows.
|
||||
const maxTargetRetries = 20
|
||||
|
||||
// Errors returned when a max_retries form value cannot be turned into
|
||||
// a retry count.
|
||||
var (
|
||||
// errRetriesInvalid signals a max_retries form value that is not
|
||||
// a non-negative whole number.
|
||||
errRetriesInvalid = errors.New(
|
||||
"must be a whole number",
|
||||
)
|
||||
|
||||
// errRetriesTooLarge signals a max_retries form value that is a
|
||||
// whole number but above maxTargetRetries. It is distinguished
|
||||
// from errRetriesInvalid so the message can name the ceiling
|
||||
// instead of implying the input was not a number.
|
||||
errRetriesTooLarge = errors.New("out of range")
|
||||
)
|
||||
|
||||
// parseMaxRetries interprets a max_retries form value.
|
||||
//
|
||||
// An ABSENT value — the field empty or not submitted — yields
|
||||
// fallback, which lets the create path apply its default and the edit
|
||||
// path leave the stored value alone. A value that is SET BUT INVALID
|
||||
// is an error: unparseable, negative, or above maxTargetRetries.
|
||||
//
|
||||
// The distinction is the whole point of this function. max_retries=0
|
||||
// means fire-and-forget, so returning 0 for input the operator typed
|
||||
// but that did not parse silently disables retries on a
|
||||
// store-and-forward proxy — and on the edit path it destroys a
|
||||
// working retry configuration over a typo. A default answers a
|
||||
// question that was not asked; it never answers one that was asked
|
||||
// badly.
|
||||
func parseMaxRetries(raw string, fallback int) (int, error) {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
return fallback, nil
|
||||
}
|
||||
|
||||
v, err := strconv.Atoi(raw)
|
||||
if err != nil || v < 0 {
|
||||
return 0, errRetriesInvalid
|
||||
}
|
||||
|
||||
if v > maxTargetRetries {
|
||||
return 0, errRetriesTooLarge
|
||||
}
|
||||
|
||||
return v, nil
|
||||
}
|
||||
|
||||
// retriesErrorMessage returns the message the create and edit forms
|
||||
// show for a rejected max_retries value. Any error other than
|
||||
// errRetriesTooLarge falls back to the generic wording, so an
|
||||
// unrecognised parse failure still produces a sensible 400.
|
||||
func retriesErrorMessage(err error) string {
|
||||
if errors.Is(err, errRetriesTooLarge) {
|
||||
return errRetriesTooLarge.Error() +
|
||||
": at most " + strconv.Itoa(maxTargetRetries) +
|
||||
" attempts"
|
||||
}
|
||||
|
||||
return errRetriesInvalid.Error() +
|
||||
", or 0 for fire-and-forget"
|
||||
}
|
||||
@@ -0,0 +1,385 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/handlers"
|
||||
)
|
||||
|
||||
// retriesTargetURL is the destination the retry-validation targets
|
||||
// point at. It is a literal public address rather than a hostname so
|
||||
// the SSRF check resolves nothing and a sandbox without DNS cannot
|
||||
// make these cases pass or fail for the wrong reason.
|
||||
const retriesTargetURL = "https://93.184.216.34/hooks/retries"
|
||||
|
||||
const (
|
||||
// wayAboveCeiling is the typo'd-extra-zero case from the report.
|
||||
wayAboveCeiling = "999999999"
|
||||
|
||||
// notANumber is the plainest garbage an operator can type, and
|
||||
// the value the report submitted on the edit form.
|
||||
notANumber = "abc"
|
||||
|
||||
// workingRetries is the retry count a seeded target is already
|
||||
// delivering with, which a rejected submission must not disturb.
|
||||
workingRetries = 2
|
||||
)
|
||||
|
||||
// aboveCeiling is the smallest rejected whole number.
|
||||
func aboveCeiling() string {
|
||||
return strconv.Itoa(handlers.MaxTargetRetriesForTest + 1)
|
||||
}
|
||||
|
||||
// overCeilingRetries is whole-number input past the limit, which is
|
||||
// rejected with the limit named.
|
||||
func overCeilingRetries() []string {
|
||||
return []string{aboveCeiling(), wayAboveCeiling}
|
||||
}
|
||||
|
||||
// unparseableRetries is input an operator can type into the field
|
||||
// that is not a retry count. Each must be REJECTED: silently reading
|
||||
// any of them as 0 turns a store-and-forward proxy into
|
||||
// fire-and-forget without saying so.
|
||||
//
|
||||
// The twenty-digit case is here because it parses as digits but
|
||||
// overflows int, which is the one failure the field's own min/max
|
||||
// attributes cannot describe.
|
||||
func unparseableRetries() []string {
|
||||
return []string{
|
||||
notANumber,
|
||||
"2.7",
|
||||
"-5",
|
||||
"12345678901234567890",
|
||||
"1e3",
|
||||
}
|
||||
}
|
||||
|
||||
// createRetriesForm is a complete, otherwise-valid HTTP target
|
||||
// creation, so the only thing any case below varies is max_retries.
|
||||
func createRetriesForm(retries string) url.Values {
|
||||
form := url.Values{}
|
||||
form.Set("name", "retries-target")
|
||||
form.Set("type", string(database.TargetTypeHTTP))
|
||||
form.Set("url", retriesTargetURL)
|
||||
|
||||
if retries != absentField {
|
||||
form.Set("max_retries", retries)
|
||||
}
|
||||
|
||||
return form
|
||||
}
|
||||
|
||||
// absentField marks a field the form does not submit at all, which is
|
||||
// the case that legitimately takes a default and must stay distinct
|
||||
// from a field submitted with garbage in it.
|
||||
const absentField = "\x00absent"
|
||||
|
||||
// absentRetries is every way of saying "the operator did not set
|
||||
// this", each of which takes the default rather than a 400. Blank and
|
||||
// whitespace-only count as absent here because they do in the timeout
|
||||
// and retention controls on the same forms; a rule the fields do not
|
||||
// share would be its own surprise.
|
||||
func absentRetries() []string {
|
||||
return []string{absentField, "", " "}
|
||||
}
|
||||
|
||||
// createWithRetries posts the target create form for a fresh webhook
|
||||
// and returns the webhook and the response.
|
||||
func createWithRetries(
|
||||
t *testing.T,
|
||||
env *sourceTestEnv,
|
||||
retries string,
|
||||
) (database.Webhook, int, string) {
|
||||
t.Helper()
|
||||
|
||||
webhook := seedWebhookWithRetention(t, env.db, 30)
|
||||
|
||||
w := serveTarget(
|
||||
env, http.MethodPost,
|
||||
"/hook/"+webhook.ID+"/targets",
|
||||
createRetriesForm(retries),
|
||||
)
|
||||
|
||||
return webhook, w.Code, w.Body.String()
|
||||
}
|
||||
|
||||
// TestTargetCreate_RetriesAboveCeilingRejected proves the create form
|
||||
// enforces a ceiling at all, and that the 400 names it — a rejection
|
||||
// that does not say what the limit is leaves the operator guessing.
|
||||
func TestTargetCreate_RetriesAboveCeilingRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
ceiling := strconv.Itoa(handlers.MaxTargetRetriesForTest)
|
||||
|
||||
for _, retries := range overCeilingRetries() {
|
||||
webhook, code, body := createWithRetries(t, env, retries)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, code,
|
||||
"max_retries=%s should be rejected", retries)
|
||||
assert.Contains(t, body, ceiling,
|
||||
"the rejection for %s should name the ceiling",
|
||||
retries)
|
||||
assert.Empty(t,
|
||||
targetsForWebhook(t, env.db, webhook.ID),
|
||||
"no target should be created for %s", retries)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_UnparseableRetriesRejected is the core of the
|
||||
// defect: each of these was accepted with HTTP 200 and stored as 0.
|
||||
func TestTargetCreate_UnparseableRetriesRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, retries := range unparseableRetries() {
|
||||
webhook, code, body := createWithRetries(t, env, retries)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, code,
|
||||
"max_retries=%q should be rejected, not coerced",
|
||||
retries)
|
||||
assert.Contains(t, body, "whole number",
|
||||
"the rejection for %q should say why", retries)
|
||||
assert.Empty(t,
|
||||
targetsForWebhook(t, env.db, webhook.ID),
|
||||
"no target should be created for %q", retries)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_ValidRetriesStored covers the accepting half,
|
||||
// including the ceiling itself: a bound that rejects its own limit
|
||||
// would make the advertised maximum unreachable.
|
||||
func TestTargetCreate_ValidRetriesStored(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, want := range []int{0, 3, handlers.MaxTargetRetriesForTest} {
|
||||
webhook, code, body := createWithRetries(
|
||||
t, env, strconv.Itoa(want),
|
||||
)
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, code, body)
|
||||
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
assert.Equal(t, want, targets[0].MaxRetries)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetCreate_AbsentRetriesTakesDefault keeps the two cases
|
||||
// distinct. An omitted field is not an operator asking for something
|
||||
// invalid, so it still gets the fire-and-forget default rather than a
|
||||
// 400 — otherwise the fix above would make the form unusable.
|
||||
func TestTargetCreate_AbsentRetriesTakesDefault(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, retries := range absentRetries() {
|
||||
webhook, code, body := createWithRetries(t, env, retries)
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, code, body)
|
||||
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
assert.Equal(t, 0, targets[0].MaxRetries,
|
||||
"an absent max_retries should take the default")
|
||||
}
|
||||
}
|
||||
|
||||
// seedRetriesTarget creates an HTTP target already delivering with
|
||||
// workingRetries retries, through the real create handler.
|
||||
func seedRetriesTarget(
|
||||
t *testing.T,
|
||||
env *sourceTestEnv,
|
||||
) (database.Webhook, database.Target) {
|
||||
t.Helper()
|
||||
|
||||
webhook, code, body := createWithRetries(
|
||||
t, env, strconv.Itoa(workingRetries),
|
||||
)
|
||||
require.Equal(t, http.StatusSeeOther, code, body)
|
||||
|
||||
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||
require.Len(t, targets, 1)
|
||||
require.Equal(t, workingRetries, targets[0].MaxRetries)
|
||||
|
||||
return webhook, targets[0]
|
||||
}
|
||||
|
||||
// editRetriesForm is a complete edit submission that changes the
|
||||
// target's name as well, so a rejected submission can be shown to
|
||||
// have written nothing at all rather than merely to have left
|
||||
// max_retries alone.
|
||||
func editRetriesForm(retries string) url.Values {
|
||||
form := url.Values{}
|
||||
form.Set("name", "renamed-by-edit")
|
||||
form.Set("url", retriesTargetURL)
|
||||
|
||||
if retries != absentField {
|
||||
form.Set("max_retries", retries)
|
||||
}
|
||||
|
||||
return form
|
||||
}
|
||||
|
||||
// assertEditRejectedAndUnchanged submits an edit expected to fail and
|
||||
// checks both halves of the requirement: the 400 explains itself, and
|
||||
// the target it was submitted against is untouched.
|
||||
func assertEditRejectedAndUnchanged(
|
||||
t *testing.T,
|
||||
env *sourceTestEnv,
|
||||
retries, wantReason string,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
webhook, target := seedRetriesTarget(t, env)
|
||||
|
||||
w := submitTargetEdit(
|
||||
env, webhook.ID, target.ID, editRetriesForm(retries),
|
||||
)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, w.Code,
|
||||
"max_retries=%q should be rejected on edit", retries)
|
||||
assert.Contains(t, w.Body.String(), wantReason,
|
||||
"the rejection for %q should say why", retries)
|
||||
|
||||
stored := storedTarget(t, env, target.ID)
|
||||
assert.Equal(t, workingRetries, stored.MaxRetries,
|
||||
"a rejected edit must not destroy the working retry "+
|
||||
"count with %q", retries)
|
||||
assert.Equal(t, "retries-target", stored.Name,
|
||||
"a rejected edit must write nothing at all")
|
||||
}
|
||||
|
||||
// TestTargetEdit_UnparseableRetriesRejected is the damaging half of
|
||||
// the defect. A target delivering with two retries, re-saved with a
|
||||
// typo in the field, returned 200 and was left with retries disabled
|
||||
// and nothing said.
|
||||
func TestTargetEdit_UnparseableRetriesRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, retries := range unparseableRetries() {
|
||||
assertEditRejectedAndUnchanged(
|
||||
t, env, retries, "whole number",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetEdit_RetriesAboveCeilingRejected proves the ceiling
|
||||
// applies to the edit path too, naming itself, so the two paths
|
||||
// cannot disagree about what is storable.
|
||||
func TestTargetEdit_RetriesAboveCeilingRejected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
ceiling := strconv.Itoa(handlers.MaxTargetRetriesForTest)
|
||||
|
||||
for _, retries := range overCeilingRetries() {
|
||||
assertEditRejectedAndUnchanged(t, env, retries, ceiling)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetEdit_ValidRetriesStored covers the accepting half of the
|
||||
// edit path, so the ceiling cannot be enforced by simply refusing
|
||||
// every submission.
|
||||
func TestTargetEdit_ValidRetriesStored(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, want := range []int{0, 9, handlers.MaxTargetRetriesForTest} {
|
||||
webhook, target := seedRetriesTarget(t, env)
|
||||
|
||||
w := submitTargetEdit(
|
||||
env, webhook.ID, target.ID,
|
||||
editRetriesForm(strconv.Itoa(want)),
|
||||
)
|
||||
require.Equal(t,
|
||||
http.StatusSeeOther, w.Code, w.Body.String(),
|
||||
)
|
||||
|
||||
assert.Equal(t, want,
|
||||
storedTarget(t, env, target.ID).MaxRetries)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetEdit_AbsentRetriesLeavesStoredValue is the edit path's
|
||||
// absent-versus-invalid case. Retries are only offered by the forms
|
||||
// for types that retry, so a submission without the field must leave
|
||||
// the stored count alone rather than be rejected or zeroed.
|
||||
func TestTargetEdit_AbsentRetriesLeavesStoredValue(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
for _, retries := range absentRetries() {
|
||||
webhook, target := seedRetriesTarget(t, env)
|
||||
|
||||
w := submitTargetEdit(
|
||||
env, webhook.ID, target.ID,
|
||||
editRetriesForm(retries),
|
||||
)
|
||||
require.Equal(t,
|
||||
http.StatusSeeOther, w.Code, w.Body.String(),
|
||||
)
|
||||
|
||||
assert.Equal(t, workingRetries,
|
||||
storedTarget(t, env, target.ID).MaxRetries,
|
||||
"an absent max_retries must leave the stored "+
|
||||
"count alone (%q)", retries)
|
||||
}
|
||||
}
|
||||
|
||||
// TestTargetRetries_CreateAndEditAgreeOnEveryCase proves the two
|
||||
// paths cannot disagree, which is what let the create form and the
|
||||
// edit form drift apart in the first place. Every input is submitted
|
||||
// to both and the accept/reject verdicts are compared.
|
||||
func TestTargetRetries_CreateAndEditAgreeOnEveryCase(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
|
||||
accepted := []string{
|
||||
"0", "1",
|
||||
strconv.Itoa(handlers.MaxTargetRetriesForTest),
|
||||
}
|
||||
overCeiling := overCeilingRetries()
|
||||
unparseable := unparseableRetries()
|
||||
|
||||
cases := make(
|
||||
[]string, 0,
|
||||
len(accepted)+len(overCeiling)+len(unparseable),
|
||||
)
|
||||
cases = append(cases, accepted...)
|
||||
cases = append(cases, overCeiling...)
|
||||
cases = append(cases, unparseable...)
|
||||
|
||||
for _, retries := range cases {
|
||||
_, createCode, _ := createWithRetries(t, env, retries)
|
||||
|
||||
webhook, target := seedRetriesTarget(t, env)
|
||||
editCode := submitTargetEdit(
|
||||
env, webhook.ID, target.ID,
|
||||
editRetriesForm(retries),
|
||||
).Code
|
||||
|
||||
assert.Equal(t,
|
||||
createCode == http.StatusBadRequest,
|
||||
editCode == http.StatusBadRequest,
|
||||
"create and edit must agree on max_retries=%q "+
|
||||
"(create %d, edit %d)",
|
||||
retries, createCode, editCode,
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -59,7 +59,7 @@ func TestHandleTargetToggle_DoesNotUndoAnEdit(t *testing.T) {
|
||||
stored := storedTarget(t, env, tgt.ID)
|
||||
assert.False(t, stored.Active)
|
||||
assert.Equal(t, "edited-name", stored.Name)
|
||||
assert.Equal(t, 5, stored.MaxAttempts)
|
||||
assert.Equal(t, 5, stored.MaxRetries)
|
||||
assert.Equal(
|
||||
t, editReplacedURL, storedHTTPConfig(t, env, tgt.ID).URL,
|
||||
)
|
||||
|
||||
@@ -348,19 +348,21 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
// maxAttemptsHelp is the wording both target forms must carry. The
|
||||
// delivery core makes max_attempts attempts in total, not that many
|
||||
// maxRetriesHelp is the wording both target forms must carry. The
|
||||
// delivery core makes max_retries attempts in total, not that many
|
||||
// retries on top of a first try (a fresh delivery starts at attempt 1
|
||||
// and target_http gives up once the attempt number reaches
|
||||
// max_attempts).
|
||||
const maxAttemptsHelp = "How many times each delivery is attempted in " +
|
||||
"all, the first attempt included."
|
||||
// max_retries), and 0 is special-cased to a single fire-and-forget
|
||||
// attempt with no circuit breaker.
|
||||
const maxRetriesHelp = "How many times each delivery is attempted in " +
|
||||
"all, the first attempt included. 0 means a single attempt with no " +
|
||||
"retries and no circuit breaker."
|
||||
|
||||
// TestTargetFormMaxAttemptsCopyMatchesBehaviour pins the max_attempts
|
||||
// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries
|
||||
// label, "Delivery attempts", and help text on both the create form
|
||||
// (the add-target form on the webhook detail page) and the edit form,
|
||||
// so the copy cannot drift back to calling the number a retry count.
|
||||
func TestTargetFormMaxAttemptsCopyMatchesBehaviour(t *testing.T) {
|
||||
func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var h *handlers.Handlers
|
||||
@@ -392,11 +394,11 @@ func TestTargetFormMaxAttemptsCopyMatchesBehaviour(t *testing.T) {
|
||||
|
||||
assert.Contains(t, createBody, "Delivery attempts:</label>")
|
||||
assert.Contains(
|
||||
t, createBody, maxAttemptsHelp,
|
||||
"the add-target form must explain max_attempts as total attempts",
|
||||
t, createBody, maxRetriesHelp,
|
||||
"the add-target form must explain max_retries as total attempts",
|
||||
)
|
||||
|
||||
// A slack target exercises the same max_attempts field while needing
|
||||
// A slack target exercises the same max_retries field while needing
|
||||
// only a URL from the edit template, so the test data stays
|
||||
// minimal. The Target and TargetForm keys mirror the field names
|
||||
// the template reads off the handler's values.
|
||||
@@ -410,8 +412,8 @@ func TestTargetFormMaxAttemptsCopyMatchesBehaviour(t *testing.T) {
|
||||
"Active": true,
|
||||
},
|
||||
"TargetForm": map[string]any{
|
||||
"URL": "https://hooks.slack.com/services/x",
|
||||
"MaxAttempts": "3",
|
||||
"URL": "https://hooks.slack.com/services/x",
|
||||
"MaxRetries": "3",
|
||||
},
|
||||
dataKeyError: "",
|
||||
},
|
||||
@@ -419,8 +421,8 @@ func TestTargetFormMaxAttemptsCopyMatchesBehaviour(t *testing.T) {
|
||||
|
||||
assert.Contains(t, editBody, `class="label">Delivery attempts</label>`)
|
||||
assert.Contains(
|
||||
t, editBody, maxAttemptsHelp,
|
||||
"the target edit form must explain max_attempts as total attempts",
|
||||
t, editBody, maxRetriesHelp,
|
||||
"the target edit form must explain max_retries as total attempts",
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"gorm.io/gorm"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/delivery"
|
||||
"sneak.berlin/go/webhooker/internal/logfield"
|
||||
"sneak.berlin/go/webhooker/internal/middleware"
|
||||
)
|
||||
|
||||
@@ -48,13 +49,14 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// Logged only once the lookup has found the entrypoint, so
|
||||
// an invented path writes no INFO line; a miss is logged at
|
||||
// DEBUG. The entrypoint is named by its ID: the UUID is its
|
||||
// credential and is never logged.
|
||||
// Logged only once the UUID is known to name a real
|
||||
// entrypoint. The UUID comes straight out of the path on
|
||||
// the one unauthenticated endpoint, so logging it before
|
||||
// the lookup let a client write an INFO line per invented
|
||||
// path; the request itself is already in the access log
|
||||
// and a miss is already logged at DEBUG.
|
||||
h.log.Info("webhook request received",
|
||||
"entrypoint_id", entrypoint.ID,
|
||||
"webhook_id", entrypoint.WebhookID,
|
||||
"entrypoint_uuid", entrypointUUID,
|
||||
"method", r.Method,
|
||||
"remoteIP", middleware.RemoteIP(r),
|
||||
"clientIP", middleware.ClientIP(r),
|
||||
@@ -131,10 +133,17 @@ func (h *Handlers) lookupEntrypoint(
|
||||
"path = ?", entrypointUUID,
|
||||
).First(&entrypoint)
|
||||
if result.Error != nil {
|
||||
// Not even an unknown UUID is logged. The receiver's log
|
||||
// lines never carry the path segment, which is a credential
|
||||
// whenever it names an entrypoint.
|
||||
h.log.Debug("entrypoint not found")
|
||||
// The receiver is unauthenticated and /h/{uuid}
|
||||
// matches any single segment, so this value is entirely
|
||||
// client-chosen on exactly the branch where the lookup
|
||||
// failed. DEBUG is off by default; the cap is what keeps
|
||||
// turning it on from restoring an unbounded write.
|
||||
h.log.Debug(
|
||||
"entrypoint not found",
|
||||
"path", logfield.Truncate(
|
||||
entrypointUUID, logfield.MaxBytes,
|
||||
),
|
||||
)
|
||||
http.NotFound(w, r)
|
||||
|
||||
return entrypoint, false
|
||||
@@ -433,7 +442,7 @@ func buildDeliveryTasks(
|
||||
TargetName: targets[i].Name,
|
||||
TargetType: targets[i].Type,
|
||||
TargetConfig: targets[i].Config,
|
||||
MaxAttempts: targets[i].MaxAttempts,
|
||||
MaxRetries: targets[i].MaxRetries,
|
||||
Method: event.Method,
|
||||
RawQuery: event.RawQuery,
|
||||
Headers: event.Headers,
|
||||
|
||||
@@ -12,15 +12,12 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi"
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gorm.io/gorm/clause"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/handlers"
|
||||
"sneak.berlin/go/webhooker/internal/middleware"
|
||||
"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
|
||||
)
|
||||
|
||||
// TestHandleWebhook_LogsClientNextToThePeer checks that the
|
||||
@@ -102,87 +99,6 @@ func TestHandleWebhook_LogsClientNextToThePeer(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleWebhook_LogsNoEntrypointUUID sends an accepted event, a
|
||||
// request over the receiver rate limit and a request for an unknown
|
||||
// UUID through the receiver as routes.go mounts it, and checks that
|
||||
// nothing logged, at any level, carries either UUID.
|
||||
func TestHandleWebhook_LogsNoEntrypointUUID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
db *database.Database
|
||||
)
|
||||
|
||||
cfg := &config.Config{DataDir: t.TempDir(), ReceiverRateLimit: 1}
|
||||
|
||||
app := newTestAppWithConfig(t, cfg, &h, &db)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
buf := new(bytes.Buffer)
|
||||
log := slog.New(slog.NewJSONHandler(
|
||||
buf, &slog.HandlerOptions{Level: slog.LevelDebug},
|
||||
))
|
||||
h.SetLogForTest(log)
|
||||
mw := middlewaretest.New(t, log, cfg, nil)
|
||||
|
||||
webhook := seedWebhook(t, db)
|
||||
entrypoint := &database.Entrypoint{
|
||||
WebhookID: webhook.ID,
|
||||
Path: uuid.New().String(),
|
||||
Active: true,
|
||||
}
|
||||
require.NoError(t, db.DB().Omit(clause.Associations).
|
||||
Create(entrypoint).Error)
|
||||
|
||||
unknownUUID := uuid.New().String()
|
||||
|
||||
router := chi.NewRouter()
|
||||
router.Use(mw.Logging())
|
||||
router.With(mw.ReceiverRateLimit()).HandleFunc(
|
||||
middleware.ReceiverRoute, h.HandleWebhook(),
|
||||
)
|
||||
|
||||
// In order: the limit is one request per minute per path, so the
|
||||
// second request for the known UUID is the rejected one.
|
||||
for _, step := range []struct {
|
||||
entrypointUUID string
|
||||
wantStatus int
|
||||
}{
|
||||
{entrypoint.Path, http.StatusOK},
|
||||
{entrypoint.Path, http.StatusTooManyRequests},
|
||||
{unknownUUID, http.StatusNotFound},
|
||||
} {
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost,
|
||||
"/h/"+step.entrypointUUID, strings.NewReader("{}"),
|
||||
)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
require.Equal(t, step.wantStatus, w.Code)
|
||||
}
|
||||
|
||||
// The lines this test is about were all written, so the checks
|
||||
// below are not passing on an empty log.
|
||||
for _, msg := range []string{
|
||||
"webhook request received",
|
||||
"webhook receiver rate limit exceeded",
|
||||
"entrypoint not found",
|
||||
} {
|
||||
assert.Contains(t, buf.String(), `"msg":"`+msg+`"`)
|
||||
}
|
||||
|
||||
assert.NotContains(t, buf.String(), entrypoint.Path)
|
||||
assert.NotContains(t, buf.String(), unknownUUID)
|
||||
|
||||
line := receivedLine(t, buf)
|
||||
assert.Equal(t, entrypoint.ID, line["entrypoint_id"])
|
||||
assert.Equal(t, webhook.ID, line["webhook_id"])
|
||||
}
|
||||
|
||||
// receivedLine returns the one "webhook request received" line in the
|
||||
// captured JSON log.
|
||||
func receivedLine(t *testing.T, buf *bytes.Buffer) map[string]any {
|
||||
|
||||
@@ -7,7 +7,6 @@ import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -28,13 +27,6 @@ const floodRequests = 64
|
||||
// line for a redirected or rejected request may contain it.
|
||||
const attackerMarker = "QQATTACKERTEXTQQ"
|
||||
|
||||
// knownEntrypointUUID and failingEntrypointUUID are the UUIDs the
|
||||
// stand-in receiver in accessLogRouter answers 200 and 500 to.
|
||||
const (
|
||||
knownEntrypointUUID = "6f1e2a9c-4b7d-4e3a-9c2f-1d8b5a7e3c60"
|
||||
failingEntrypointUUID = "0b7c3e5a-2d9f-4a61-8e4b-7c1d6f2a9e35"
|
||||
)
|
||||
|
||||
// maxLineBytes bounds a single access log line whose client-supplied
|
||||
// fields are of ordinary size. Well above what the fixed fields need,
|
||||
// well below the length of the oversized input the amplification tests
|
||||
@@ -140,21 +132,17 @@ func accessLogRouter(m *middleware.Middleware) *chi.Mux {
|
||||
)
|
||||
|
||||
router.HandleFunc(
|
||||
middleware.ReceiverRoute,
|
||||
"/h/{uuid}",
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
// Stands in for the real handler: a known entrypoint
|
||||
// UUID succeeds, a failing one is a server error, and
|
||||
// any other 404s.
|
||||
switch chi.URLParam(r, "uuid") {
|
||||
case knownEntrypointUUID:
|
||||
w.WriteHeader(http.StatusOK)
|
||||
case failingEntrypointUUID:
|
||||
http.Error(
|
||||
w, "boom", http.StatusInternalServerError,
|
||||
)
|
||||
default:
|
||||
// Stands in for the real handler: an unknown entrypoint
|
||||
// UUID 404s, a known one succeeds.
|
||||
if chi.URLParam(r, "uuid") != "known" {
|
||||
http.Error(w, "not found", http.StatusNotFound)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
},
|
||||
)
|
||||
|
||||
@@ -347,13 +335,11 @@ func oversizedValue(ch string) string {
|
||||
}
|
||||
|
||||
// oversizedHeaders fills every client-supplied header the access log
|
||||
// reads with the same value. The Referer carries it as the host of an
|
||||
// absolute URL, because the access log keeps only a Referer's scheme
|
||||
// and host.
|
||||
// reads with the same value.
|
||||
func oversizedHeaders(value string) map[string]string {
|
||||
return map[string]string{
|
||||
"User-Agent": value,
|
||||
"Referer": "http://" + value,
|
||||
"Referer": value,
|
||||
"X-Request-Id": value,
|
||||
}
|
||||
}
|
||||
@@ -663,111 +649,17 @@ func TestAccessLog_SuccessKeepsConcretePathAndRedactsQuery(
|
||||
router := accessLogRouter(m)
|
||||
|
||||
assert.Equal(
|
||||
t, http.StatusOK,
|
||||
get(t, router, "/.well-known/healthcheck?src=ci"),
|
||||
t, http.StatusOK, get(t, router, "/h/known?src=ci"),
|
||||
)
|
||||
|
||||
// The path resolved against a stored entrypoint, so it stays. The
|
||||
// query never does: see TestAccessLog_UnauthenticatedSuccess...
|
||||
entries := accessLogEntries(t, buf)
|
||||
require.Len(t, entries, 1)
|
||||
assert.Equal(
|
||||
t, "/.well-known/healthcheck?(redacted)", entries[0]["url"],
|
||||
)
|
||||
assert.Equal(t, "/h/known?(redacted)", entries[0]["url"])
|
||||
assert.NotContains(t, buf.String(), "src=ci")
|
||||
}
|
||||
|
||||
// TestAccessLog_ReceiverNeverLogsTheUUID covers the two statuses that
|
||||
// keep the concrete path on every other route. On the receiver that
|
||||
// path carries the entrypoint UUID, so the line names the route
|
||||
// pattern instead.
|
||||
func TestAccessLog_ReceiverNeverLogsTheUUID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for entrypointUUID, wantStatus := range map[string]int{
|
||||
knownEntrypointUUID: http.StatusOK,
|
||||
failingEntrypointUUID: http.StatusInternalServerError,
|
||||
} {
|
||||
t.Run(strconv.Itoa(wantStatus), func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m, buf := capturingMiddleware(t)
|
||||
router := accessLogRouter(m)
|
||||
|
||||
assert.Equal(
|
||||
t, wantStatus,
|
||||
get(t, router, "/h/"+entrypointUUID+"?src=ci"),
|
||||
)
|
||||
|
||||
entries := accessLogEntries(t, buf)
|
||||
require.Len(t, entries, 1)
|
||||
assert.Equal(t, middleware.ReceiverRoute, entries[0]["url"])
|
||||
assert.NotContains(t, buf.String(), entrypointUUID)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestAccessLog_ReceiverPreflightNeverLogsTheUUID sends a CORS
|
||||
// preflight to the receiver in dev, in the production middleware
|
||||
// order. CORS answers it with 200 before chi routes it, so the line
|
||||
// names no route and carries no UUID.
|
||||
func TestAccessLog_ReceiverPreflightNeverLogsTheUUID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m, buf := capturingMiddleware(t)
|
||||
|
||||
router := chi.NewRouter()
|
||||
router.Use(m.Logging())
|
||||
router.Use(m.CORS())
|
||||
router.HandleFunc(
|
||||
middleware.ReceiverRoute,
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
},
|
||||
)
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodOptions,
|
||||
"/h/"+knownEntrypointUUID, nil,
|
||||
)
|
||||
req.Header.Set("Origin", "https://sender.example")
|
||||
req.Header.Set("Access-Control-Request-Method", http.MethodPost)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
|
||||
entries := accessLogEntries(t, buf)
|
||||
require.Len(t, entries, 1)
|
||||
assert.Equal(t, unmatchedRouteLiteral, entries[0]["url"])
|
||||
assert.NotContains(t, buf.String(), knownEntrypointUUID)
|
||||
}
|
||||
|
||||
// TestAccessLog_RefererNeverLogsTheUUID sends receiver requests whose
|
||||
// Referer is the entrypoint URL, as a sender's is after it follows a
|
||||
// redirect from http:// to https://. The referer field keeps only the
|
||||
// scheme and host, and a Referer without them is logged as a marker.
|
||||
func TestAccessLog_RefererNeverLogsTheUUID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m, buf := capturingMiddleware(t)
|
||||
router := accessLogRouter(m)
|
||||
|
||||
for _, referer := range []string{
|
||||
"http://webhooker.example/h/" + knownEntrypointUUID + "?src=ci",
|
||||
"/h/" + knownEntrypointUUID,
|
||||
} {
|
||||
assert.Equal(t, http.StatusOK, getWithHeaders(
|
||||
t, router, "/h/"+knownEntrypointUUID,
|
||||
map[string]string{"Referer": referer},
|
||||
))
|
||||
}
|
||||
|
||||
entries := accessLogEntries(t, buf)
|
||||
require.Len(t, entries, 2)
|
||||
assert.Equal(t, "http://webhooker.example", entries[0]["referer"])
|
||||
assert.Equal(t, "(redacted)", entries[1]["referer"])
|
||||
assert.NotContains(t, buf.String(), knownEntrypointUUID)
|
||||
}
|
||||
|
||||
func TestAccessLog_ServerErrorKeepsConcreteURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -10,10 +10,8 @@ package middleware_test
|
||||
// the client's choosing.
|
||||
// - CSRF's 403 rejection, at WARN, also registered ahead of
|
||||
// RequireAuth.
|
||||
// - The rate limiters' 429 rejection, at WARN. On the receiver
|
||||
// route it names the route pattern rather than the path, and
|
||||
// the limiters that log a path sit behind RequireAuth, so its cap
|
||||
// is defensive.
|
||||
// - The rate limiters' 429 rejection, at WARN, on the
|
||||
// unauthenticated receiver among others.
|
||||
// - RequireAuth's own unauthenticated-request line, at DEBUG.
|
||||
// - RecordLoginFailure's throttle rejection, at WARN. Its cap is
|
||||
// defensive rather than load-bearing today: chi pins the one
|
||||
@@ -270,14 +268,10 @@ func logSites() map[string]logSite {
|
||||
send: postNoToken,
|
||||
wantStatus: http.StatusForbidden,
|
||||
},
|
||||
// The 429 line every limiter but the aggregate one shares.
|
||||
// Mounted outside any route, as here, it logs the path, as
|
||||
// it does for the limiters behind RequireAuth; on
|
||||
// ReceiverRoute it names the pattern instead. The
|
||||
// per-entrypoint receiver limiter's bucket is keyed on the
|
||||
// path, so the first request through a fresh path is served
|
||||
// and only the ones after it are rejected; sendUntilLimited
|
||||
// absorbs that.
|
||||
// The per-entrypoint receiver limiter, unauthenticated. Its
|
||||
// bucket is keyed on the path, so the first request through a
|
||||
// fresh path is served and only the ones after it are
|
||||
// rejected; sendUntilLimited absorbs that.
|
||||
"receiver rate limit 429": {
|
||||
build: func(
|
||||
t *testing.T, m *middleware.Middleware,
|
||||
|
||||
@@ -41,8 +41,7 @@ const unmatchedMethod = unmatchedRoute
|
||||
// after /h/ is client-chosen, so labelling by path lets any
|
||||
// unauthenticated client mint permanent series at will, and publishes
|
||||
// the entrypoint UUID (the receiver's only credential) in the scrape
|
||||
// while doing it. The access log and the 429 line read it for the
|
||||
// same reason: see ReceiverRoute.
|
||||
// while doing it.
|
||||
//
|
||||
// chi populates the route context during routeHTTP, so this is only
|
||||
// valid once routing has run. Every caller below is on the recording
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
"time"
|
||||
|
||||
basicauth "github.com/99designs/basicauth-go"
|
||||
"github.com/go-chi/chi"
|
||||
"github.com/go-chi/chi/middleware"
|
||||
"github.com/go-chi/cors"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
@@ -22,7 +23,6 @@ import (
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/globals"
|
||||
"sneak.berlin/go/webhooker/internal/logfield"
|
||||
"sneak.berlin/go/webhooker/internal/referer"
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
)
|
||||
|
||||
@@ -96,14 +96,14 @@ const (
|
||||
// It is also the ceiling on every OTHER line this service writes
|
||||
// THROUGH SLOG that carries text an UNAUTHENTICATED client
|
||||
// supplies. Those lines — the MaxBodySize rejection, the CSRF
|
||||
// rejection, the unauthenticated-request DEBUG line, the
|
||||
// failed-login DEBUG lines, and the two login-throttle WARN
|
||||
// lines ("login failure limit exceeded" in loginguard.go and
|
||||
// "password verification capacity exhausted" in
|
||||
// internal/handlers/auth.go) — spend the same per-field budgets,
|
||||
// and each carries strictly fewer client-supplied fields than the
|
||||
// access log does, so none of them can reach a width the access
|
||||
// log cannot. That is
|
||||
// rejection, the rate-limit rejection, the unauthenticated-request
|
||||
// and unknown-entrypoint DEBUG lines, the failed-login DEBUG
|
||||
// lines, and the two login-throttle WARN lines ("login failure
|
||||
// limit exceeded" in loginguard.go and "password verification
|
||||
// capacity exhausted" in internal/handlers/auth.go) — spend the
|
||||
// same per-field budgets, and each carries strictly fewer
|
||||
// client-supplied fields than the access log does,
|
||||
// so none of them can reach a width the access log cannot. That is
|
||||
// asserted directly, per line and under both handlers, rather than
|
||||
// left to the reasoning: see logbound_test.go in this package and
|
||||
// in internal/handlers.
|
||||
@@ -115,15 +115,16 @@ const (
|
||||
// a caller on a parameterised route would supply, which is the
|
||||
// only way those caps can be pinned at all.
|
||||
//
|
||||
// The GORM adapter in internal/gormlog writes through slog as well.
|
||||
// It logs each statement with its placeholders rather than the
|
||||
// values bound to them (see its ParamsFilter), so the key a client
|
||||
// sends to the receiver or login lookup does not reach its lines.
|
||||
// Its widest line spends two logfield.MaxBytes budgets — the
|
||||
// statement and the driver error — against a fixed portion smaller
|
||||
// than this one's, and internal/gormlog/gormlog_test.go asserts
|
||||
// every line it emits against this constant directly rather than
|
||||
// leaving it as arithmetic.
|
||||
// One writer in that set is not a handler's own slog call. The
|
||||
// GORM adapter in internal/gormlog logs the statement with the
|
||||
// client-chosen parameter already interpolated into it, which on
|
||||
// the receiver and login lookups is exactly the value the budgets
|
||||
// above exist for. Its widest line spends two logfield.MaxBytes
|
||||
// budgets — the statement and the driver error — against a fixed
|
||||
// portion smaller than this one's, and
|
||||
// internal/gormlog/gormlog_test.go asserts every line it emits
|
||||
// against this constant directly rather than leaving it as
|
||||
// arithmetic.
|
||||
//
|
||||
// What it does NOT cover, so that the figure above is not read as
|
||||
// more than it is:
|
||||
@@ -150,12 +151,6 @@ const (
|
||||
MaxAccessLogLineBytes = 2560
|
||||
)
|
||||
|
||||
// ReceiverRoute is the route pattern the webhook receiver is served
|
||||
// at. Its path carries the entrypoint UUID, which is the receiver's
|
||||
// only credential, so the log lines this package writes name a
|
||||
// request on this route by the pattern, never by its path.
|
||||
const ReceiverRoute = "/h/{uuid}"
|
||||
|
||||
//nolint:revive // MiddlewareParams is a standard fx naming convention.
|
||||
type MiddlewareParams struct {
|
||||
fx.In
|
||||
@@ -273,14 +268,14 @@ func (lrw *loggingResponseWriter) Unwrap() http.ResponseWriter {
|
||||
// marker.
|
||||
//
|
||||
// The path on those branches is bounded by the service's routes or by
|
||||
// the operator's data — a 2xx under /s means the file is in the
|
||||
// embedded tree. The query is not bounded by anything:
|
||||
// /.well-known/healthcheck and /s/* take no authentication and sit
|
||||
// behind no rate limiter, and /pages/login behind only the login
|
||||
// limiter, so any of them will answer 200 to a URL carrying an
|
||||
// arbitrary number of arbitrary bytes after the '?'. Keeping the path
|
||||
// and dropping the query is what makes this branch as bounded as the
|
||||
// pattern branches below.
|
||||
// the operator's data — a 2xx on the receiver means the UUID named a
|
||||
// stored entrypoint, a 2xx under /s means the file is in the embedded
|
||||
// tree. The query is not bounded by anything: /.well-known/healthcheck
|
||||
// and /s/* take no authentication and sit behind no rate limiter, and
|
||||
// /pages/login behind only the login limiter, so any of them will
|
||||
// answer 200 to a URL carrying an arbitrary number of arbitrary bytes
|
||||
// after the '?'. Keeping the path and dropping the query is what makes
|
||||
// this branch as bounded as the pattern branches below.
|
||||
//
|
||||
// Nothing debuggable is lost. The only query parameters the service
|
||||
// reads are the login page's `next`, the page to return to,
|
||||
@@ -302,40 +297,36 @@ func concreteLogURL(r *http.Request) string {
|
||||
|
||||
// accessLogURL returns the value for the access log's url field.
|
||||
//
|
||||
// On ReceiverRoute it is the route pattern whatever the status, so
|
||||
// the entrypoint UUID in the path is never logged. A request chi never
|
||||
// routed gets unmatchedRoute whatever the status, for the same reason:
|
||||
// in dev, CORS answers a preflight to the receiver before routing.
|
||||
//
|
||||
// On every other route, 2xx and 5xx responses get the concrete path
|
||||
// (see concreteLogURL). A success resolved against a static route or
|
||||
// against the operator's own data, and a server error is our own bug,
|
||||
// where the exact URL is the primary evidence and which no client can
|
||||
// provoke at will.
|
||||
// 2xx and 5xx responses get the concrete path (see concreteLogURL). A
|
||||
// success resolved against a static route or against the operator's
|
||||
// own data — on the receiver, a 2xx means the UUID named a stored
|
||||
// entrypoint — and a server error is our own bug, where the exact URL
|
||||
// is the primary evidence and which no client can provoke at will.
|
||||
//
|
||||
// 3xx and 4xx responses get the chi route pattern instead. Those are
|
||||
// the outcomes an unauthenticated client drives for free, such as a
|
||||
// 303 to the login page on any invented /user/ path. Logging the
|
||||
// concrete URL there lets a flood write attacker-chosen text, of
|
||||
// attacker-chosen length, into the operator's log at one line per
|
||||
// request. The pattern comes from the router's own table, so it is
|
||||
// bounded by the service's routes while still naming which class of
|
||||
// request was rejected.
|
||||
// the outcomes an unauthenticated client drives for free: 404 or 429
|
||||
// on any invented /h/ path, 303 to the login page on any
|
||||
// invented /user/ path. Logging the concrete URL there lets a flood
|
||||
// write attacker-chosen text, of attacker-chosen length, into the
|
||||
// operator's log at one line per request. The pattern comes from the
|
||||
// router's own table, so it is bounded by the service's routes while
|
||||
// still naming which class of request was rejected.
|
||||
//
|
||||
// The pattern is only populated once routing has run, so this must be
|
||||
// called after the handler returns, not before.
|
||||
func accessLogURL(r *http.Request, status int) string {
|
||||
pattern := routePatternID(r.Context())
|
||||
if pattern == ReceiverRoute || pattern == unmatchedRoute {
|
||||
return pattern
|
||||
}
|
||||
|
||||
if status < http.StatusMultipleChoices ||
|
||||
status >= http.StatusInternalServerError {
|
||||
return concreteLogURL(r)
|
||||
}
|
||||
|
||||
return pattern
|
||||
if rc := chi.RouteContext(r.Context()); rc != nil {
|
||||
if pattern := rc.RoutePattern(); pattern != "" {
|
||||
return pattern
|
||||
}
|
||||
}
|
||||
|
||||
return unmatchedRoute
|
||||
}
|
||||
|
||||
// Logging returns middleware that logs each HTTP request with
|
||||
@@ -388,8 +379,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
|
||||
requestID, maxLogRequestIDBytes,
|
||||
),
|
||||
"referer", logfield.Truncate(
|
||||
referer.SchemeAndHost(r.Referer()),
|
||||
logfield.MaxBytes,
|
||||
r.Referer(), logfield.MaxBytes,
|
||||
),
|
||||
"proto", r.Proto,
|
||||
"remoteIP", RemoteIP(r),
|
||||
@@ -548,9 +538,7 @@ func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler {
|
||||
// SecurityHeaders returns middleware that sets production security
|
||||
// headers on every response: HSTS (unless HSTS_MAX_AGE is zero),
|
||||
// X-Content-Type-Options, X-Frame-Options, CSP, Referrer-Policy, and
|
||||
// Permissions-Policy. The CSP takes scripts and styles only from
|
||||
// files on this origin, so a template may hold no inline script,
|
||||
// event handler or style attribute.
|
||||
// Permissions-Policy.
|
||||
func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
||||
maxAge := s.params.Config.HSTSMaxAge
|
||||
hsts := "max-age=" + strconv.FormatInt(int64(maxAge/time.Second), 10) +
|
||||
@@ -572,8 +560,8 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
||||
w.Header().Set(
|
||||
"Content-Security-Policy",
|
||||
"default-src 'self'; "+
|
||||
"script-src 'self'; "+
|
||||
"style-src 'self'",
|
||||
"script-src 'self' 'unsafe-inline'; "+
|
||||
"style-src 'self' 'unsafe-inline'",
|
||||
)
|
||||
w.Header().Set(
|
||||
"Referrer-Policy",
|
||||
|
||||
@@ -747,10 +747,10 @@ func TestNoCache_SetsHeaders(t *testing.T) {
|
||||
|
||||
// --- SecurityHeaders Middleware Tests ---
|
||||
|
||||
// securityHeaders serves one request through SecurityHeaders built
|
||||
// over a Config with the given HSTSMaxAge, and returns the response's
|
||||
// headers.
|
||||
func securityHeaders(t *testing.T, maxAge time.Duration) http.Header {
|
||||
// hstsHeaders serves one request through SecurityHeaders built over
|
||||
// a Config with the given HSTSMaxAge, and returns every
|
||||
// Strict-Transport-Security value on the response.
|
||||
func hstsHeaders(t *testing.T, maxAge time.Duration) []string {
|
||||
t.Helper()
|
||||
|
||||
cfg := &config.Config{HSTSMaxAge: maxAge}
|
||||
@@ -772,7 +772,7 @@ func securityHeaders(t *testing.T, maxAge time.Duration) http.Header {
|
||||
|
||||
handler.ServeHTTP(w, req)
|
||||
|
||||
return w.Header()
|
||||
return w.Header().Values("Strict-Transport-Security")
|
||||
}
|
||||
|
||||
func TestSecurityHeaders_HSTS(t *testing.T) {
|
||||
@@ -804,28 +804,11 @@ func TestSecurityHeaders_HSTS(t *testing.T) {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, tt.expected,
|
||||
securityHeaders(t, tt.maxAge).Values(
|
||||
"Strict-Transport-Security",
|
||||
),
|
||||
)
|
||||
assert.Equal(t, tt.expected, hstsHeaders(t, tt.maxAge))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSecurityHeaders_CSP pins the Content-Security-Policy: a page
|
||||
// takes its scripts and styles only from files on its own origin,
|
||||
// never from an inline script, event handler or style.
|
||||
func TestSecurityHeaders_CSP(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, "default-src 'self'; script-src 'self'; style-src 'self'",
|
||||
securityHeaders(t, 0).Get("Content-Security-Policy"),
|
||||
)
|
||||
}
|
||||
|
||||
// --- MaxBodySize Middleware Tests ---
|
||||
|
||||
const testBodyLimit int64 = 64
|
||||
|
||||
@@ -259,28 +259,26 @@ func (m *Middleware) clientAddr(r *http.Request) (netip.Addr, bool) {
|
||||
return peer, true
|
||||
}
|
||||
|
||||
// tooManyRequests returns the 429 handler used by the postRateLimit
|
||||
// limiters and the per-entrypoint receiver limiter: it logs the
|
||||
// tooManyRequests returns the 429 handler used by the
|
||||
// password-change and per-entrypoint receiver limiters: it logs the
|
||||
// rejection with logMessage and answers with responseMessage.
|
||||
// httprate adds the Retry-After header (RFC 6585). The aggregate
|
||||
// receiver limiter uses floodTooManyRequests instead.
|
||||
//
|
||||
// On ReceiverRoute the line names the route pattern, so the
|
||||
// entrypoint UUID in the path is never logged. Elsewhere it names the
|
||||
// path, capped against the same budget as the access log's url field;
|
||||
// the limiters there sit behind RequireAuth, so the cap is defensive.
|
||||
// The path is capped against the same budget as the access log's url
|
||||
// field. The per-entrypoint receiver limiter is unauthenticated and
|
||||
// its path is a client-chosen segment of client-chosen length, so at
|
||||
// WARN an uncapped path would let a sender pick the size of the line
|
||||
// it writes — the same defect the access log capping closed.
|
||||
func (m *Middleware) tooManyRequests(
|
||||
logMessage, responseMessage string,
|
||||
) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
path := r.URL.Path
|
||||
if routePatternID(r.Context()) == ReceiverRoute {
|
||||
path = ReceiverRoute
|
||||
}
|
||||
|
||||
m.log.Warn(
|
||||
logMessage,
|
||||
"path", logfield.Truncate(path, logfield.MaxBytes),
|
||||
"path", logfield.Truncate(
|
||||
r.URL.Path, logfield.MaxBytes,
|
||||
),
|
||||
"remoteIP", RemoteIP(r),
|
||||
"clientIP", ClientIP(r),
|
||||
)
|
||||
|
||||
@@ -1,32 +0,0 @@
|
||||
// Package referer reduces a Referer header to its scheme and host, so
|
||||
// that a URL path, which on the receiver route carries the entrypoint
|
||||
// UUID, never reaches a log line or an error report.
|
||||
package referer
|
||||
|
||||
import "net/url"
|
||||
|
||||
// redacted stands in for a Referer that cannot be reduced to its
|
||||
// scheme and host.
|
||||
const redacted = "(redacted)"
|
||||
|
||||
// SchemeAndHost returns a Referer's scheme and host, never its path or
|
||||
// query. The path can be an entrypoint URL, which carries the UUID: a
|
||||
// sender that follows a redirect from http:// to https:// sends the
|
||||
// URL it was redirected from, and a browser sends the 404 page's URL
|
||||
// under /h/ when it fetches that page's stylesheet. A Referer without
|
||||
// both a scheme and a host comes back as "(redacted)".
|
||||
//
|
||||
// It is the access log's referer field, and the Referer header the
|
||||
// Sentry hook sends.
|
||||
func SchemeAndHost(referer string) string {
|
||||
if referer == "" {
|
||||
return ""
|
||||
}
|
||||
|
||||
parsed, err := url.Parse(referer)
|
||||
if err != nil || parsed.Scheme == "" || parsed.Host == "" {
|
||||
return redacted
|
||||
}
|
||||
|
||||
return parsed.Scheme + "://" + parsed.Host
|
||||
}
|
||||
@@ -1,67 +0,0 @@
|
||||
package referer_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"sneak.berlin/go/webhooker/internal/referer"
|
||||
)
|
||||
|
||||
// entrypointUUID is the secret part of an entrypoint URL; no case may
|
||||
// return it.
|
||||
const entrypointUUID = "0b6f7d3e-6a39-4e3c-9f0a-2c4d8e1b5a77"
|
||||
|
||||
// redacted is what SchemeAndHost returns for a Referer it cannot
|
||||
// reduce to a scheme and host.
|
||||
const redacted = "(redacted)"
|
||||
|
||||
func TestSchemeAndHost(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
referer string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "no Referer",
|
||||
referer: "",
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "entrypoint URL with a query",
|
||||
referer: "http://webhooker.example/h/" + entrypointUUID +
|
||||
"?src=ci",
|
||||
want: "http://webhooker.example",
|
||||
},
|
||||
{
|
||||
name: "host with a port",
|
||||
referer: "https://webhooker.example:8443/h/" + entrypointUUID,
|
||||
want: "https://webhooker.example:8443",
|
||||
},
|
||||
{
|
||||
name: "path without a scheme or host",
|
||||
referer: "/h/" + entrypointUUID,
|
||||
want: redacted,
|
||||
},
|
||||
{
|
||||
name: "host without a scheme",
|
||||
referer: "webhooker.example/h/" + entrypointUUID,
|
||||
want: redacted,
|
||||
},
|
||||
{
|
||||
name: "not a URL",
|
||||
referer: "http://[::1/h/" + entrypointUUID,
|
||||
want: redacted,
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
got := referer.SchemeAndHost(tc.referer)
|
||||
|
||||
assert.Equal(t, tc.want, got)
|
||||
assert.NotContains(t, got, entrypointUUID)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -60,8 +60,8 @@ const (
|
||||
// TestAlpineRunsUnderTheSecurityPolicy loads the webhook page and the
|
||||
// event log in a headless browser, served by the real router and so
|
||||
// under the real Content-Security-Policy, and checks that the pages'
|
||||
// Alpine.js directives, the copy control and the delete prompts work,
|
||||
// and that a target's row shows its delivery counts.
|
||||
// Alpine.js directives and the copy control work, and that a target's
|
||||
// row shows its delivery counts.
|
||||
func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -74,10 +74,6 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
|
||||
userID, _ := env.seedUser(t, "browser", "browser-password")
|
||||
webhook, older, event, target := seedBrowserWebhook(t, env, userID)
|
||||
|
||||
// The sign-in page is loaded before the browser signs in, since a
|
||||
// signed-in visit to it is sent on to another page.
|
||||
checkOtherPages(ctx, t, srv.URL+"/pages/login")
|
||||
|
||||
require.NoError(t, chromedp.Run(
|
||||
ctx, setCookies(srv.URL, env.authCookies(t, userID, "browser")),
|
||||
))
|
||||
@@ -103,11 +99,6 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
|
||||
checkEventLog(ctx, t, page+"/events", event.ID, older.ID, target.Name)
|
||||
checkMobileMenu(ctx, t, page)
|
||||
checkPhoneWidth(ctx, t, page, page+"/events", target.Name)
|
||||
checkOtherPages(ctx, t,
|
||||
srv.URL+"/hooks", srv.URL+"/settings", srv.URL+"/user/browser",
|
||||
// A path that does not exist, which gets the 404 error page.
|
||||
srv.URL+"/no-such-page")
|
||||
checkDeletePrompt(ctx, t, page, target)
|
||||
|
||||
assert.Empty(t, problems(), "the browser reported problems")
|
||||
}
|
||||
@@ -363,11 +354,11 @@ func checkAddEachTargetType(ctx context.Context, t *testing.T, url string) {
|
||||
}{
|
||||
{
|
||||
"http", "http",
|
||||
"csrf_token name type url headers timeout max_attempts",
|
||||
"csrf_token name type url headers timeout max_retries",
|
||||
map[string]string{"url": publicTargetURL},
|
||||
},
|
||||
{
|
||||
"slack", "slack", "csrf_token name type url max_attempts",
|
||||
"slack", "slack", "csrf_token name type url max_retries",
|
||||
map[string]string{"url": publicTargetURL},
|
||||
},
|
||||
{
|
||||
@@ -639,11 +630,11 @@ func checkRefusedEdits(
|
||||
values map[string]string
|
||||
}{
|
||||
{page + "/targets/" + targetID + "/edit", map[string]string{
|
||||
"#name": "edited-target",
|
||||
"#url": "http://127.0.0.1/hook",
|
||||
"#headers": "X-Edited: kept",
|
||||
"#timeout": "12",
|
||||
"#max_attempts": "3",
|
||||
"#name": "edited-target",
|
||||
"#url": "http://127.0.0.1/hook",
|
||||
"#headers": "X-Edited: kept",
|
||||
"#timeout": "12",
|
||||
"#max_retries": "3",
|
||||
}},
|
||||
{page + "/edit", map[string]string{
|
||||
"#name": "edited-webhook",
|
||||
@@ -1397,75 +1388,3 @@ func checkPhoneWidth(
|
||||
assert.False(t, sideways, "the event log scrolls sideways on a phone")
|
||||
assert.Empty(t, cutOff, "the event log cuts these off on a phone")
|
||||
}
|
||||
|
||||
// checkOtherPages loads the pages at urls, which no other check opens,
|
||||
// so that anything the browser reports going wrong on them fails the
|
||||
// test too.
|
||||
func checkOtherPages(ctx context.Context, t *testing.T, urls ...string) {
|
||||
t.Helper()
|
||||
|
||||
for _, url := range urls {
|
||||
require.NoError(t, chromedp.Run(ctx, loadPage(url)))
|
||||
}
|
||||
}
|
||||
|
||||
// answerPrompt returns a script that stands in for the user at the
|
||||
// browser's prompt: it makes window.confirm record the question in
|
||||
// window.asked and give answer. It also records in window.sent whether
|
||||
// a form is sent once its submit event is handled, from a listener on
|
||||
// window, which the event reaches after the listener static/js/app.js
|
||||
// adds to document.
|
||||
func answerPrompt(answer bool) string {
|
||||
return fmt.Sprintf(`
|
||||
window.asked = "";
|
||||
window.sent = false;
|
||||
window.confirm = (question) => {
|
||||
window.asked = question;
|
||||
return %t;
|
||||
};
|
||||
window.addEventListener("submit", (event) => {
|
||||
window.sent = !event.defaultPrevented;
|
||||
});`, answer)
|
||||
}
|
||||
|
||||
// checkDeletePrompt loads a webhook page and clicks Delete beside
|
||||
// target. It checks that the browser asks first, naming the target,
|
||||
// that answering no leaves the form unsent, and that answering yes
|
||||
// sends it and the target is deleted.
|
||||
func checkDeletePrompt(
|
||||
ctx context.Context, t *testing.T, url string, target *database.Target,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
form := `form[action$="/targets/` + target.ID + `/delete"]`
|
||||
question := `Delete target "` + target.Name + `"?` + "\n\n" +
|
||||
"Nothing more is delivered to it. " +
|
||||
"Its past deliveries stay in the event log."
|
||||
|
||||
var (
|
||||
asked string
|
||||
sent bool
|
||||
listed bool
|
||||
)
|
||||
|
||||
require.NoError(t, chromedp.Run(
|
||||
ctx, loadPage(url), chromedp.Evaluate(answerPrompt(false), nil),
|
||||
))
|
||||
click(ctx, t, form+" button")
|
||||
require.NoError(t, chromedp.Run(
|
||||
ctx,
|
||||
chromedp.Evaluate(`window.asked`, &asked),
|
||||
chromedp.Evaluate(`window.sent`, &sent),
|
||||
))
|
||||
assert.Equal(t, question, asked, "Delete does not ask about the target")
|
||||
assert.False(t, sent, "answering no still sends the delete form")
|
||||
|
||||
require.NoError(t, chromedp.Run(
|
||||
ctx, chromedp.Evaluate(answerPrompt(true), nil),
|
||||
))
|
||||
clickAndLoad(ctx, t, form+" button")
|
||||
require.NoError(t, chromedp.Run(ctx, chromedp.Evaluate(
|
||||
`document.querySelector('`+form+`') !== null`, &listed,
|
||||
)))
|
||||
assert.False(t, listed, "answering yes does not delete the target")
|
||||
}
|
||||
|
||||
@@ -27,11 +27,10 @@ func TestHook_DownloadArchive(t *testing.T) {
|
||||
env.seedTarget(t, wh.ID)
|
||||
|
||||
archive := &database.Target{
|
||||
WebhookID: wh.ID,
|
||||
Name: "kept",
|
||||
Type: database.TargetTypeDatabase,
|
||||
Active: true,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: wh.ID,
|
||||
Name: "kept",
|
||||
Type: database.TargetTypeDatabase,
|
||||
Active: true,
|
||||
}
|
||||
require.NoError(t,
|
||||
env.db.DB().Omit(clause.Associations).Create(archive).Error,
|
||||
|
||||
@@ -6,8 +6,7 @@ import (
|
||||
|
||||
sentryhttp "github.com/getsentry/sentry-go/http"
|
||||
"github.com/go-chi/chi"
|
||||
chimw "github.com/go-chi/chi/middleware"
|
||||
"sneak.berlin/go/webhooker/internal/middleware"
|
||||
"github.com/go-chi/chi/middleware"
|
||||
"sneak.berlin/go/webhooker/static"
|
||||
)
|
||||
|
||||
@@ -61,7 +60,7 @@ func (s *Server) SetupRoutes() {
|
||||
}
|
||||
|
||||
func (s *Server) setupGlobalMiddleware() {
|
||||
s.router.Use(chimw.RequestID)
|
||||
s.router.Use(middleware.RequestID)
|
||||
s.router.Use(s.mw.SecurityHeaders())
|
||||
s.router.Use(s.mw.Logging())
|
||||
|
||||
@@ -346,7 +345,7 @@ func (s *Server) setupWebhookRoutes() {
|
||||
// unauthenticated endpoint's body; TestReceiver_OversizeBodyRefused
|
||||
// pins it.
|
||||
s.router.With(s.mw.ReceiverRateLimit()).HandleFunc(
|
||||
middleware.ReceiverRoute,
|
||||
"/h/{uuid}",
|
||||
s.h.HandleWebhook(),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -402,12 +402,11 @@ func (e *testEnv) seedTarget(
|
||||
t.Helper()
|
||||
|
||||
tgt := &database.Target{
|
||||
WebhookID: webhookID,
|
||||
Name: "routed-target",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Active: true,
|
||||
Config: `{"url":"http://93.184.216.34/hook"}`,
|
||||
MaxAttempts: 1,
|
||||
WebhookID: webhookID,
|
||||
Name: "routed-target",
|
||||
Type: database.TargetTypeHTTP,
|
||||
Active: true,
|
||||
Config: `{"url":"http://93.184.216.34/hook"}`,
|
||||
}
|
||||
|
||||
require.NoError(
|
||||
|
||||
@@ -8,7 +8,6 @@ import (
|
||||
|
||||
"github.com/getsentry/sentry-go"
|
||||
"github.com/go-chi/chi"
|
||||
"sneak.berlin/go/webhooker/internal/referer"
|
||||
)
|
||||
|
||||
// sentryRedacted stands in for a withheld field on every event shipped
|
||||
@@ -254,7 +253,6 @@ func sentryTransactionName(name, pattern string) string {
|
||||
// gorilla/csrf accepts in place of the form field — and the shared
|
||||
// secrets senders put on the receiver route (X-Gitlab-Token and the
|
||||
// per-provider signature headers) would otherwise ship verbatim.
|
||||
// The Referer it keeps is cut down to its scheme and host.
|
||||
func keptSentryHeaders(headers map[string]string) map[string]string {
|
||||
if len(headers) == 0 {
|
||||
return headers
|
||||
@@ -263,15 +261,9 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
|
||||
kept := make(map[string]string, len(headers))
|
||||
|
||||
for name, value := range headers {
|
||||
if !sentryKeepsHeader(name) {
|
||||
continue
|
||||
if sentryKeepsHeader(name) {
|
||||
kept[name] = value
|
||||
}
|
||||
|
||||
if http.CanonicalHeaderKey(name) == "Referer" {
|
||||
value = referer.SchemeAndHost(value)
|
||||
}
|
||||
|
||||
kept[name] = value
|
||||
}
|
||||
|
||||
return kept
|
||||
@@ -279,14 +271,11 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
|
||||
|
||||
// sentryKeepsHeader reports whether a request header is routing or
|
||||
// content metadata rather than client-chosen payload. Referer is kept
|
||||
// only as its scheme and host, through referer.SchemeAndHost, which
|
||||
// the access log uses too: its path can be an entrypoint URL, which
|
||||
// carries the UUID, as when a sender follows a redirect from http://
|
||||
// to https:// or a browser fetches the 404 page's stylesheet from
|
||||
// under /h/. The scheme and host are all gorilla/csrf compares a
|
||||
// Referer by, so a CSRF rejection is still readable from the event.
|
||||
// X-Request-Id ties the event to the local access log line, which
|
||||
// holds the rest of the detail.
|
||||
// on the reasoning that it is browser-set, that the only query
|
||||
// parameters in this service's own URLs are the login page's `next`,
|
||||
// `notice` and the event log's `show`, and that Referrer-Policy is set to
|
||||
// strict-origin-when-cross-origin. X-Request-Id ties the event to the
|
||||
// local access log line, which holds the rest of the detail.
|
||||
func sentryKeepsHeader(name string) bool {
|
||||
switch http.CanonicalHeaderKey(name) {
|
||||
case "Accept",
|
||||
|
||||
@@ -325,37 +325,6 @@ func TestSentryScrub_ReplacesTheCapabilityPathWithTheRoutePattern(
|
||||
)
|
||||
}
|
||||
|
||||
// TestSentryScrub_KeepsOnlyTheRefererSchemeAndHost sends a receiver
|
||||
// request whose Referer is the entrypoint URL, as a sender's is after
|
||||
// it follows the README's redirect from http:// to https://. The event
|
||||
// keeps the Referer's scheme and host, and carries the UUID nowhere.
|
||||
func TestSentryScrub_KeepsOnlyTheRefererSchemeAndHost(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
event := onlyEvent(t, sentryCase{
|
||||
scrub: true,
|
||||
panics: true,
|
||||
request: func(client *sentry.Client) *http.Request {
|
||||
req := sentryReceiverRequest(client)
|
||||
req.Header.Set(
|
||||
"Referer",
|
||||
"http://webhooker.example/h/"+sentryReceiverUUID,
|
||||
)
|
||||
|
||||
return req
|
||||
},
|
||||
}.capture(t))
|
||||
|
||||
assert.NotContains(
|
||||
t, marshalEvent(t, event), sentryReceiverUUID,
|
||||
)
|
||||
assert.Equal(
|
||||
t,
|
||||
"http://webhooker.example",
|
||||
event.Request.Headers["Referer"],
|
||||
)
|
||||
}
|
||||
|
||||
// TestSentryScrub_KeepsTheRoutingContext checks the hook does not cost
|
||||
// the debugging signal: the route, its scheme and host, the method and
|
||||
// the metadata headers still identify what failed. On a static route
|
||||
|
||||
@@ -153,15 +153,19 @@ func TestVersion_IsStableAcrossInvocations(t *testing.T) {
|
||||
require.Equal(t, first, second)
|
||||
}
|
||||
|
||||
// The build target is the only place that stamps the version. If a
|
||||
// future edit drops the -X flag, the binary silently reports "dev"
|
||||
// again, the defect this package exists for.
|
||||
func TestMakefile_BuildStampsTheVersion(t *testing.T) {
|
||||
// The build target is the only place composing linker flags. If a
|
||||
// future edit drops either half, the binary silently reports "dev"
|
||||
// again (the defect this package exists for) or fails to link on
|
||||
// Alpine. If it stops passing GO_TAGS, the image's static binary links
|
||||
// glibc's getaddrinfo and dlopen again.
|
||||
func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
makefile := read(t, makefilePath)
|
||||
|
||||
require.Contains(t, makefile, "-X main.version=$(VERSION)")
|
||||
require.Contains(t, makefile, "$(GO_LDFLAGS)")
|
||||
require.Contains(t, makefile, "-tags '$(GO_TAGS)'")
|
||||
require.Contains(t, makefile, "VERSION ?= $(shell script/version)")
|
||||
}
|
||||
|
||||
@@ -220,8 +224,8 @@ func requireMake(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The image compiles through the build target, so it carries the
|
||||
// target's version stamp.
|
||||
// Every compile in the image goes through the build target, so the
|
||||
// static relink cannot replace the flags that carry the stamp.
|
||||
func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -230,7 +234,10 @@ func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
|
||||
require.NotContains(t, dockerfile, "go build",
|
||||
"a raw go build bypasses the Makefile's -X flag")
|
||||
require.Contains(t, dockerfile, "ARG VERSION")
|
||||
require.Contains(t, dockerfile, `RUN make build VERSION="$VERSION"`)
|
||||
require.Contains(t, dockerfile,
|
||||
`make build VERSION="$VERSION"`+
|
||||
` GO_TAGS=netgo,osusergo,sqlite_omit_load_extension`+
|
||||
` GO_LDFLAGS='-extldflags "-static"'`)
|
||||
}
|
||||
|
||||
func read(t *testing.T, path string) string {
|
||||
|
||||
+3
-11
@@ -1,7 +1,7 @@
|
||||
/*
|
||||
* The two shared styles for the controls a user clicks, and the rule that
|
||||
* hides an element marked x-cloak. Every page loads this file after
|
||||
* tailwind.css. It is plain CSS: make css does not build it.
|
||||
* The two shared styles for the controls a user clicks. Every page loads
|
||||
* this file after tailwind.css. It is plain CSS: make css does not build
|
||||
* it.
|
||||
*
|
||||
* A button is btn-primary, btn-secondary or btn-danger, from input.css.
|
||||
* A secondary or inline action, such as Copy beside an entrypoint URL or
|
||||
@@ -46,11 +46,3 @@
|
||||
outline-offset: 2px;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Alpine.js removes x-cloak from an element once it has started, so
|
||||
* something Alpine hides is not seen on the page while it loads.
|
||||
*/
|
||||
[x-cloak] {
|
||||
display: none !important;
|
||||
}
|
||||
|
||||
File diff suppressed because one or more lines are too long
+3
-15
@@ -58,18 +58,6 @@
|
||||
}
|
||||
})();
|
||||
|
||||
// Asks before a form with a data-confirm attribute is sent, such as a
|
||||
// delete form on the webhook page. The browser's own prompt shows the
|
||||
// attribute's text, and the form is sent only if the user accepts.
|
||||
document.addEventListener("submit", function (event) {
|
||||
"use strict";
|
||||
|
||||
const message = event.target.getAttribute("data-confirm");
|
||||
if (message !== null && !window.confirm(message)) {
|
||||
event.preventDefault();
|
||||
}
|
||||
});
|
||||
|
||||
// Alpine.js components.
|
||||
//
|
||||
// The pages' Content-Security-Policy forbids eval, so the UI loads
|
||||
@@ -151,7 +139,7 @@ document.addEventListener("alpine:init", function () {
|
||||
headers: "",
|
||||
timeout: "",
|
||||
forwardQuery: false,
|
||||
maxAttempts: "",
|
||||
maxRetries: "",
|
||||
expiry: "",
|
||||
rotation: "",
|
||||
init() {
|
||||
@@ -165,7 +153,7 @@ document.addEventListener("alpine:init", function () {
|
||||
this.timeout = refused.timeout;
|
||||
this.forwardQuery =
|
||||
this.$root.hasAttribute("data-forward-query");
|
||||
this.maxAttempts = refused.maxAttempts;
|
||||
this.maxRetries = refused.maxRetries;
|
||||
this.expiry = refused.expiry;
|
||||
this.rotation = refused.rotation;
|
||||
},
|
||||
@@ -185,7 +173,7 @@ document.addEventListener("alpine:init", function () {
|
||||
this.headers = "";
|
||||
this.timeout = "";
|
||||
this.forwardQuery = false;
|
||||
this.maxAttempts = "";
|
||||
this.maxRetries = "";
|
||||
this.expiry = "";
|
||||
this.rotation = "";
|
||||
this.$refs.form.reset();
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
{{if .Binary}}
|
||||
<p class="text-xs text-gray-500">This body is binary ({{.Size}} bytes) and is not shown. <a href="{{.EventURL}}/body" class="btn-small">Download the body</a></p>
|
||||
{{else if .Text}}
|
||||
<pre class="rounded-md border border-gray-200 bg-white p-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all{{if .Scroll}} max-h-[32rem] overflow-y-auto{{end}}">{{.Text}}</pre>
|
||||
<pre class="rounded-md border border-gray-200 bg-white p-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all"{{if .Scroll}} style="max-height: 32rem; overflow-y: auto"{{end}}>{{.Text}}</pre>
|
||||
{{if .Cut}}
|
||||
<p class="mt-2 text-xs text-gray-500">Showing the first {{.ShownBytes}} of {{.Size}} bytes, unformatted. <a href="{{.EventURL}}" class="btn-small">Show the whole body</a> <a href="{{.EventURL}}/body" class="btn-small">Download the body</a></p>
|
||||
{{end}}
|
||||
|
||||
@@ -4,5 +4,6 @@
|
||||
<title>{{block "title" .}}Webhooker{{end}}</title>
|
||||
<link rel="stylesheet" href="/s/css/tailwind.css">
|
||||
<link rel="stylesheet" href="/s/css/style.css">
|
||||
<style>[x-cloak] { display: none !important; }</style>
|
||||
{{block "head" .}}{{end}}
|
||||
{{end}}
|
||||
|
||||
@@ -13,11 +13,13 @@
|
||||
<div class="p-4">
|
||||
<!-- A value too wide to sit beside its name moves to the
|
||||
next line, where a list breaks only at the spaces
|
||||
between its entries. wrap-anywhere breaks inside a
|
||||
value only when it alone is wider than the line. -->
|
||||
between its entries. overflow-wrap: anywhere breaks
|
||||
inside a value only when it alone is wider than the
|
||||
line; an inline style, because the committed
|
||||
tailwind.css has no class for it. -->
|
||||
<div class="flex flex-wrap justify-between items-start gap-4">
|
||||
<code class="text-sm font-medium text-gray-900">{{.Name}}</code>
|
||||
<code class="text-sm text-gray-900 wrap-anywhere">{{.Value}}</code>
|
||||
<code class="text-sm text-gray-900" style="overflow-wrap: anywhere">{{.Value}}</code>
|
||||
</div>
|
||||
<p class="text-sm text-gray-500 mt-1">{{.Description}}</p>
|
||||
</div>
|
||||
|
||||
@@ -5,10 +5,12 @@
|
||||
{{define "content"}}
|
||||
<!-- 108rem, half again the 72rem (max-w-6xl) of the webhook list, the
|
||||
event log, the navbar and the footer, so an entrypoint URL fits on
|
||||
one line. wrap-anywhere goes only on names and descriptions: a
|
||||
row too wide for a phone must still run past the edge, where the
|
||||
browser test sees it, rather than break its controls mid-word. -->
|
||||
<div class="max-w-[108rem] mx-auto px-6 py-8">
|
||||
one line. An inline style, because the committed tailwind.css has
|
||||
no class this wide. wrap-anywhere goes only on names and
|
||||
descriptions: a row too wide for a phone must still run past the
|
||||
edge, where the browser test sees it, rather than break its
|
||||
controls mid-word. -->
|
||||
<div class="mx-auto px-6 py-8" style="max-width: 108rem">
|
||||
<div class="mb-6">
|
||||
<a href="/hooks" class="btn-small">← Back to webhooks</a>
|
||||
<div class="flex flex-wrap justify-between items-center gap-2 mt-2">
|
||||
@@ -21,12 +23,11 @@
|
||||
<div class="flex gap-2">
|
||||
<a href="/hook/{{.Webhook.ID}}/events" class="btn-secondary">Full Event Log</a>
|
||||
<a href="/hook/{{.Webhook.ID}}/edit" class="btn-secondary">Edit</a>
|
||||
<!-- Before a delete form is sent, static/js/app.js
|
||||
asks in the browser's own prompt, with the form's
|
||||
data-confirm text; is a line break. The
|
||||
template escapes each name for the attribute, so a
|
||||
quote in it shows as typed. -->
|
||||
<form method="POST" action="/hook/{{.Webhook.ID}}/delete" data-confirm="Delete webhook "{{.Webhook.Name}}"? This deletes its stored events{{with .Stats}} ({{.WithinRetention.Events}}){{end}} and their deliveries. Any archive files it wrote are kept.">
|
||||
<!-- The delete prompts are the browser's own, so they
|
||||
work without the page's scripts. The template
|
||||
escapes each name for the script, so a quote or a
|
||||
backslash in it shows as typed. -->
|
||||
<form method="POST" action="/hook/{{.Webhook.ID}}/delete" onsubmit="return confirm('Delete webhook "{{.Webhook.Name}}"?\n\nThis deletes its stored events{{with .Stats}} ({{.WithinRetention.Events}}){{end}} and their deliveries. Any archive files it wrote are kept.')">
|
||||
<input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
|
||||
<button type="submit" class="btn-danger">Delete</button>
|
||||
</form>
|
||||
@@ -89,7 +90,7 @@
|
||||
{{if .Active}}Deactivate{{else}}Activate{{end}}
|
||||
</button>
|
||||
</form>
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/delete" data-confirm="Delete entrypoint "{{if .Description}}{{.Description}}{{else}}{{$.BaseURL}}/h/{{.Path}}{{end}}"? Senders using its URL get an error from now on, and the URL cannot be restored." class="inline">
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/entrypoints/{{.ID}}/delete" onsubmit="return confirm('Delete entrypoint "{{if .Description}}{{.Description}}{{else}}{{$.BaseURL}}/h/{{.Path}}{{end}}"?\n\nSenders using its URL get an error from now on, and the URL cannot be restored.')" class="inline">
|
||||
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
|
||||
<button type="submit" class="btn-small text-red-600" title="Delete">Delete</button>
|
||||
</form>
|
||||
@@ -135,7 +136,7 @@
|
||||
data-headers="{{.TargetForm.Headers}}"
|
||||
data-timeout="{{.TargetForm.Timeout}}"
|
||||
{{if .TargetForm.ForwardQuery}}data-forward-query{{end}}
|
||||
data-max-attempts="{{.TargetForm.MaxAttempts}}"
|
||||
data-max-retries="{{.TargetForm.MaxRetries}}"
|
||||
data-expiry="{{.TargetForm.Expiry}}"
|
||||
data-rotation="{{.TargetForm.Rotation}}">
|
||||
<div class="p-4 border-b border-gray-200 flex justify-between items-center">
|
||||
@@ -193,9 +194,9 @@
|
||||
<div>
|
||||
<div class="flex gap-2 items-center">
|
||||
<label class="text-sm text-gray-700">Delivery attempts:</label>
|
||||
<input type="number" name="max_attempts" :value="maxAttempts" placeholder="1" min="1" max="20" class="input text-sm w-24">
|
||||
<input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
|
||||
</div>
|
||||
<p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included.</p>
|
||||
<p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included. 0 means a single attempt with no retries and no circuit breaker.</p>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
@@ -209,9 +210,9 @@
|
||||
<div>
|
||||
<div class="flex gap-2 items-center">
|
||||
<label class="text-sm text-gray-700">Delivery attempts:</label>
|
||||
<input type="number" name="max_attempts" :value="maxAttempts" placeholder="1" min="1" max="20" class="input text-sm w-24">
|
||||
<input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
|
||||
</div>
|
||||
<p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included.</p>
|
||||
<p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included. 0 means a single attempt with no retries and no circuit breaker.</p>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
@@ -277,7 +278,7 @@
|
||||
{{if .Active}}Deactivate{{else}}Activate{{end}}
|
||||
</button>
|
||||
</form>
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/delete" data-confirm="Delete target "{{.Name}}"? Nothing more is delivered to it. Its past deliveries stay in the event log." class="inline">
|
||||
<form method="POST" action="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/delete" onsubmit="return confirm('Delete target "{{.Name}}"?\n\nNothing more is delivered to it. Its past deliveries stay in the event log.')" class="inline">
|
||||
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
|
||||
<button type="submit" class="btn-small text-red-600" title="Delete">Delete</button>
|
||||
</form>
|
||||
|
||||
@@ -89,9 +89,9 @@
|
||||
|
||||
{{if or (eq .Target.Type "http") (eq .Target.Type "slack")}}
|
||||
<div class="form-group">
|
||||
<label for="max_attempts" class="label">Delivery attempts</label>
|
||||
<input type="number" id="max_attempts" name="max_attempts" value="{{.TargetForm.MaxAttempts}}" min="1" max="20" class="input">
|
||||
<p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included.</p>
|
||||
<label for="max_retries" class="label">Delivery attempts</label>
|
||||
<input type="number" id="max_retries" name="max_retries" value="{{.TargetForm.MaxRetries}}" min="0" max="20" class="input">
|
||||
<p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included. 0 means a single attempt with no retries and no circuit breaker.</p>
|
||||
</div>
|
||||
{{end}}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user