Verify login credentials before spending rate-limit budget (closes #150)

With TRUSTED_PROXIES empty behind the reverse proxy production is
required to run behind, every login POST keyed on the proxy's address
and shared one 5/minute bucket. A stranger sending five POSTs a
minute -- 0.08 requests per second, from anywhere -- kept that bucket
permanently full, and the operator's own correct password was answered
429 indefinitely with no second administrative path.

The login POST no longer has a pre-emptive limiter. The handler
verifies credentials first and spends budget only on a FAILED attempt,
so a correct password is never throttled whatever the counters hold.
Three things follow, and are implemented together because the first is
unsafe without the other two:

- Failures are counted per (client bucket, submitted username), five
  per minute, after which further failures get 429 with a Retry-After.
  A successful login clears the counter, so mistyping and then
  succeeding does not leave the operator throttled.
- Both key sets are capped at 1024 entries. The submitted username is
  attacker-controlled, so past the first cap failures fall back to a
  counter keyed on the client alone, and past both caps a failure is
  answered as throttled without being recorded. Tracked state stays
  under half a megabyte and does not grow with invented usernames.
- Concurrent Argon2id verifications are capped at two, a 128 MB
  ceiling at 64 MB per hash. Every password-hashing endpoint takes a
  slot, including the password-change endpoint, which holds one across
  both its hashes. A request that waits five seconds without a slot is
  answered 503 and no hash runs for it.

An unknown username is verified against a dummy hash instead of
returning early, so a nonexistent account costs the same time as a
real one and the response cannot be used to enumerate usernames.

The password-change limiter is unchanged: RequireAuth runs ahead of
it, so only a request already carrying a valid session reaches its
bucket.

Also adds the missing test for the third bucketKey call site, where
the peer is a trusted proxy but the forwarded chain names no client.
Every existing test of that fallback uses an IPv4 proxy, where
bucketKey is the identity function, so dropping the /64 masking there
left the suite green.

README and the TRUSTED_PROXIES startup warning updated: a shared
bucket now costs precision, not the availability of the admin path.

.gitea/workflows/check.yml

@@ -13,19 +13,39 @@ jobs:
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
         with:
           # The fingerprint step below needs history to find the last commit
-          # that touched the Docker build context, and the superseded-status
-          # step needs it to walk ancestors (it aborts on a shallow clone).
+          # that touched the Docker build context.
           fetch-depth: 0
 
-      - name: Mark superseded run statuses
+      - name: Neutralize superseded run statuses
         # Gitea cancels the in-flight run when another commit is pushed to the
         # same branch and records the cancellation as `failure`, so a commit
-        # that was never tested reads as a test result. The script rewrites
-        # those statuses to say what happened. See its header for why the
-        # state stays `failure` and not `skipped`.
+        # that was never tested reads red. The cancellation is unconditional
+        # server-side for push events and cannot be disabled from a workflow
+        # file, so the superseding run rewrites those statuses to `skipped`.
+        # Only the exact cancellation status is touched; a real failure is
+        # left alone.
         env:
           GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
-        run: script/ci-mark-superseded
+        run: |
+          set -eu
+          api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
+          ctx='check / check (push)'
+          for sha in $(git rev-list --max-count=20 "${GITHUB_SHA}^" || true); do
+            latest="$(curl -sf "${api}/commits/${sha}/status" | jq -r \
+              --arg c "$ctx" \
+              '[.statuses[] | select(.context == $c)][0] // empty
+               | "\(.status)|\(.description)"')" || continue
+            [ "$latest" = 'failure|Has been cancelled' ] || continue
+            curl -sf -X POST "${api}/statuses/${sha}" \
+              -H "Authorization: token ${GITEA_TOKEN}" \
+              -H 'Content-Type: application/json' \
+              -d "$(jq -nc --arg c "$ctx" '{
+                context: $c,
+                state: "skipped",
+                description: "Superseded by a newer commit; never tested"
+              }')" >/dev/null
+            echo "neutralized superseded status on ${sha}"
+          done
 
       - name: Fingerprint the build context
         # `.dockerignore` keeps docs out of the build context, so a docs-only

Dockerfile

@@ -32,9 +32,7 @@ FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a349228
 # Depend on lint stage passing
 COPY --from=lint /src/go.sum /dev/null
 
-# jq is a runtime dependency of script/ci-mark-superseded, which the test
-# suite executes.
-RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates jq && rm -rf /var/lib/apt/lists/*
+RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /build
 

README.md

@@ -284,8 +284,6 @@ are inline commands with no script behind them. We provide:
 - `script/docker` — build the Docker image tagged via `script/projectname`
 - `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
   runs the checks, so a green build implies a green repo)
-- `script/ci-mark-superseded` — CI helper: mark the commits whose run a
-  newer push cancelled (see [CI gate honesty](#ci-gate-honesty))
 - `script/precommit` — pre-commit checks (`go mod tidy` guard, then
   `script/check`)
 - `script/install-precommit` — install the git pre-commit hook that
@@ -999,70 +997,8 @@ requests and has the rest of its aggregate budget rejected there, so
 the aggregate limit is what bounds 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.
-
-What the access log does bound is the _content_ of those lines. A 3xx
-or 4xx response logs the chi route pattern — `/webhook/{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
-`/.well-known/healthcheck` and `/s/*` answer 200 to anyone with no rate
-limiter in front of them, so a query on a fixed 200 URL would otherwise
-buy the same amplification as an invented path. Nothing debuggable is
-lost: `page`, on the authenticated pagination links, is the only query
-parameter this service reads.
-
-The remaining client-supplied fields are truncated rather than dropped,
-each to a fixed budget: 512 bytes for `url`, `useragent` and `referer`,
-128 for `request_id` (chi passes an inbound `X-Request-Id` header
-through), and 32 for `method`. A truncated `User-Agent` is still worth
-reading; an absent one is not. A cut value ends in `[truncated]`, which
-is charged on top of the budget rather than inside it.
-
-Each budget is spent in _encoded_ bytes, not in the bytes the client
-sent. Every rune is charged what the wider of the two log handlers
-emits for it: two bytes for a quotation mark, a backslash or a tab; six
-for a non-printable rune below U+10000; ten for one at or above it,
-which the text handler spells `\UXXXXXXXX`. Go's header parser accepts
-all of them in a header value, so a budget counted raw would buy a
-field several times its nominal size — and the line, not the header, is
-what an operator has to store. Plain ASCII encodes one byte for one, so
-a real browser's `User-Agent` still fits whole; a value built out of
-escapes keeps a proportionally shorter prefix, which is the right
-trade.
-
-Net: **one `INFO` line per request, of at most 2,560 bytes.** That
-ceiling is arithmetic, not an observation: 3 × (512 + 11) for `url`,
-`useragent` and `referer`, plus 128 + 11 for `request_id`, plus 32 + 11
-for `method`, plus a 336-byte fixed portion (the field names, the
-punctuation, both timestamps at their longest, an IPv6 `remoteIP` with
-a zone, the status and the latency) — 2,087 bytes, stated at 2,560 so
-the figure has headroom. `internal/middleware/accesslog_test.go`
-asserts it against 8 KB of client-chosen text in the path, in the
-query, and in each of `User-Agent`, `Referer` and `X-Request-Id`,
-including cases built from the characters the handlers escape, and
-against the widest line the service can be made to write: a 5xx that
-keeps its concrete path while all three header fields are also at their
-budget. Every case runs through both handlers `internal/logger` can
-select — the JSON one and the text one it installs on a tty — since the
-two do not escape alike and the ceiling is quoted unqualified. Measured
-over a real connection, the widest line is 1,972 bytes.
-
-Multiply that ceiling by the request rate to size log storage. Note
-that the rate is not bounded by the limits above on every route:
-`/.well-known/healthcheck` and `/s/*` sit behind no limiter, so there
-the multiplier is whatever the deployment will serve.
+bounded by neither limit: every request is recorded once at `INFO` with
+its full URL, served or rejected alike.
 
 Every limiter here — receiver, login, and password change — identifies
 the client the same way, through one shared key function: the
@@ -1115,79 +1051,33 @@ second administrative path. So the handler inverts the order:
    reachable.
 2. **Failures are counted per (client bucket, submitted username)**,
    five per minute, after which further _failures_ from that pair are
-   answered `429` with a `Retry-After`. That `429` is a label on the
-   response, not a gate in front of the work: the credential check has
-   already run by the time the counter is consulted, so a throttled
-   client's guess is still evaluated. See the guessing rate below. A
-   successful login clears the counter, so mistyping a few times and
-   then getting it right leaves you unthrottled. Because the submitted
-   username is attacker-controlled, at most 1024 username counters and
-   1024 fallback address counters are tracked; past the first cap
-   failures fall back to the address counter, and past both they are
-   answered as throttled without being recorded. Total tracked state
-   is under half a megabyte and does not grow with the number of
-   usernames an attacker invents.
-3. **Concurrent password verifications are capped at two, and the
-   queue for them at 64.** Verifying before counting means every login
-   request costs an Argon2id hash, and Argon2id here is 64 MB per
-   hash — two slots is a 128 MB ceiling on password hashing. Every
-   endpoint that hashes a password takes a slot, including the
-   password-change endpoint, which holds one across both the
-   verification and the new hash. A request that waits five seconds
-   without getting a slot is answered `503 Service Unavailable` and no
-   hash is computed for it. The wait alone does not bound memory, only
-   how long one request holds some: a waiter reaches the guard with
-   its form already parsed, so it holds up to the 1 MB body cap for as
-   long as it waits, and at flood rates an unbounded queue would be
-   worth gigabytes against a 128 MB hashing budget. So the number of
-   waiters is capped as well, at 64 — 1 MB each against 64 MB of
-   committed queue memory, and shallow enough that two slots can drain
-   a full queue inside the five-second deadline. A request arriving
-   past the cap is shed with `503` immediately instead of joining the
-   queue. Peak commitment for the endpoint is therefore 128 MB of
-   hashing plus about 66 MB of parsed request bodies.
+   answered `429` with a `Retry-After`. A successful login clears the
+   counter, so mistyping a few times and then getting it right leaves
+   you unthrottled. Because the submitted username is
+   attacker-controlled, at most 1024 username counters and 1024
+   fallback address counters are tracked; past the first cap failures
+   fall back to the address counter, and past both they are answered
+   as throttled without being recorded. Total tracked state is under
+   half a megabyte and does not grow with the number of usernames an
+   attacker invents.
+3. **Concurrent password verifications are capped at two.** Verifying
+   before counting means every login request costs an Argon2id hash,
+   and Argon2id here is 64 MB per hash — two slots is a 128 MB ceiling
+   on password hashing. Every endpoint that hashes a password takes a
+   slot, including the password-change endpoint, which holds one
+   across both the verification and the new hash. A request that waits
+   five seconds without getting a slot is answered `503 Service
+   Unavailable` and no hash is computed for it.
 
 An unknown username is verified against a dummy hash rather than
 rejected early, so a nonexistent account costs the same time as a real
 one and the response cannot be used to enumerate usernames.
 
-**This raises online guessing throughput by about 300x, and that is
-the trade.** Because the credential check always precedes the counter,
-what bounds online brute force is the semaphore, not the failure
-counter. Two slots at the cost of one Argon2id verification is on the
-order of **27 guesses per second, about 2.3 million per day**, against
-5 per minute under the pre-emptive limiter this replaced. Treat that
-figure as a lower bound rather than a ceiling: it was measured with
-Go's race detector enabled, so real hardware verifies faster and
-guesses faster. Choose the admin password to survive millions of
-online guesses per day — a long random passphrase, not a memorable
-one. Rate-limiting `POST /pages/login` at the reverse proxy, where the
-real client address is visible, is the way to put a cheaper bound back
-on top.
-
-The residual exposure is a bounded, self-clearing loss of login
-**availability** — not merely of latency. A flood can keep both
-verification slots busy, and a request that neither gets a slot within
-five seconds nor finds room in the queue is answered `503`. Above
-roughly 27 requests per second the operator is not served slowly, it
-is shed: its chance per attempt is about the ratio of service rate to
-flood rate, so at 400 requests per second it is roughly one attempt in
-fourteen. A sufficiently determined flood still denies login for as
-long as it runs.
-
-What changed is the price and the aftermath. Denying login used to
-cost an attacker 0.08 requests per second from anywhere; it now costs
-30 or more sustained, about 400 times as much. Nothing accumulates
-while the flood runs, nothing needs resetting when it stops, and the
-operator's correct password succeeds on the first attempt afterwards.
-Restarting the service is **not** a remedy: a restart clears the
-failure counters, which are not what is saturated, and the flood
-re-fills both verification slots on its first two requests. The
-remedies are to block the source at the reverse proxy, or to
-rate-limit `POST /pages/login` there — the one place a limit can be
-applied without reintroducing the lockout, because the proxy sees the
-real client address. Setting `TRUSTED_PROXIES` does not stop the
-saturation, but it makes the source visible in the failure logs.
+The residual exposure is bounded and self-clearing: a flood can keep
+both verification slots busy, so logins queue and some are shed with
+`503` until it stops. That is degraded latency for everyone rather
+than a permanent lockout of the operator, and an operator under one
+can block the source at the reverse proxy or restart the service.
 
 Finer-grained per-webhook rate limits (configured in the web UI and
 enforced in the webhook handler) can layer on top of this env-level
@@ -1209,7 +1099,7 @@ abuse limit later; they are tracked as future work.
 | Method | Path            | Description |
 | ------ | --------------- | ----------- |
 | `GET`  | `/pages/login`  | Login page (not rate limited) |
-| `POST` | `/pages/login`  | Login form submission. Credentials are verified before any limit is consulted, so a correct password is never throttled; 5 FAILED attempts per minute per bucket per submitted username, then `429`. `503` if no verification slot frees up within 5s, or immediately if 64 requests are already queued for one (see [Rate Limiting](#rate-limiting)) |
+| `POST` | `/pages/login`  | Login form submission. Credentials are verified before any limit is consulted, so a correct password is never throttled; 5 FAILED attempts per minute per bucket per submitted username, then `429`. `503` if no verification slot frees up within 5s (see [Rate Limiting](#rate-limiting)) |
 | `POST` | `/pages/logout` | Logout (destroys session) |
 
 #### Authenticated Endpoints
@@ -1217,7 +1107,7 @@ abuse limit later; they are tracked as future work.
 | Method | Path                     | Description |
 | ------ | ------------------------ | ----------- |
 | `GET`  | `/user/{username}`       | User profile page |
-| `POST` | `/user/{username}/password` | Change the user's password (5 per minute per bucket, then `429`; `503` if no verification slot frees up within 5s, or immediately if 64 requests are already queued for one) |
+| `POST` | `/user/{username}/password` | Change the user's password (5 per minute per bucket, then `429`; `503` if no verification slot frees up within 5s) |
 | `GET`  | `/sources`               | List user's webhooks |
 | `GET`  | `/sources/new`           | Create webhook form |
 | `POST` | `/sources/new`           | Create webhook submission |
@@ -1416,11 +1306,9 @@ one that lies about its length, is hard-capped by
 Those same four route groups then apply **CSRF** and **NoCache**
 (`Cache-Control: no-store`, `Pragma: no-cache`), and every group except
 `/pages` applies **RequireAuth**. The rate limiters are per-route
-rather than global: **PasswordChangeRateLimit** on
-`/user/{username}/password` and **ReceiverRateLimit** on
-`/webhook/{uuid}`. There is deliberately none on `/pages/login` — that
-endpoint counts failures inside the handler, after the credential
-check, see [The login endpoint](#the-login-endpoint).
+rather than global: **LoginRateLimit** on `/pages/login`,
+**PasswordChangeRateLimit** on `/user/{username}/password`, and
+**ReceiverRateLimit** on `/webhook/{uuid}`.
 
 ### Authentication
 
@@ -1458,27 +1346,14 @@ check, see [The login endpoint](#the-login-endpoint).
   both at target creation time (URL validation) and at delivery time
   (custom HTTP transport with SSRF-safe dialer that validates resolved
   IPs before connecting, preventing DNS rebinding attacks)
-- **Login limiting is inverted, deliberately.** The login `POST` has
-  no pre-emptive rate limiter in front of it. Credentials are
-  verified first and only a _failed_ attempt spends budget, so a
-  correct password is never throttled and no flood of wrong ones can
-  deny the operator the only administrative path. Failures are
-  counted per (bucket, submitted username), five per minute, after
-  which further failures are answered `429` with a `Retry-After`.
-  What bounds brute force is not that counter but the cap of two
-  concurrent Argon2id verifications: a throttled client's guess is
-  still evaluated, so roughly 27 guesses a second get through and the
-  admin password has to carry that load (see
-  [The login endpoint](#the-login-endpoint)). `GET` requests to the
-  login page are not limited
-- **Password-change rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
-  sliding-window rate limiter, 5 POST attempts per minute per bucket.
-  It runs behind session auth, so only a client already holding a
-  valid session reaches it, and an operator throttled out of changing
-  a password can still log in. The bucket is per client IP only when
+- **Login rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
+  sliding-window rate limiter on the login endpoint, 5 POST attempts
+  per minute per bucket, to slow brute-force attacks. GET requests to
+  the login page are not limited. The password-change endpoint carries
+  the same 5-per-minute limit. The bucket is per client IP only when
   `TRUSTED_PROXIES` names the reverse proxy; unset, every client
-  shares one bucket, which costs precision rather than availability
-  (see [Rate Limiting](#rate-limiting)). webhooker warns at startup
+  shares one bucket and the login becomes remotely deniable (see
+  [Rate Limiting](#rate-limiting)). webhooker warns at startup
   whenever `TRUSTED_PROXIES` is empty
 - Prometheus metrics behind basic auth
 - Static assets embedded in binary (no filesystem access needed at
@@ -1616,32 +1491,11 @@ way.
 A separate workflow step, run before the fingerprint is written, covers
 a second way the gate lied: Gitea cancels an in-flight run when a newer
 commit lands on the same branch and records that cancellation as a
-`failure` status, so a commit nothing ever tested reads as a test
-result. Cancellation is unconditional server-side for push events, so
-the superseding run calls `script/ci-mark-superseded`, which rewrites
-that exact status to `failure` /
-`Superseded by a newer commit; never tested`.
-
-The state stays `failure` on purpose: Gitea's combined status folds
-`skipped` into `success`, so marking a never-tested commit `skipped`
-made the status API report green for it, indistinguishable from a commit
-that passed. Reading a commit's status on this repo therefore goes:
-
-- `success` / `Successful in ...` — the checks ran and passed.
-- `failure` / `Failing after ...` — the checks ran and failed.
-- `failure` / `Superseded by a newer commit; never tested` — the run was
-  cancelled, by a newer push or by hand, and nothing was verified about
-  this commit. Test the commit itself before concluding anything about
-  it.
-
-Genuine failures and successes are never touched, and no status is left
-`pending`, which would block the commit indefinitely. The step derives
-its context string from the workflow name, the job **id** and the event.
-That is deliberately not byte-identical to Gitea's own rule, which uses
-the job's display `name:` where the runner exports the id, so giving the
-job a `name:` — or renaming the workflow — makes the derived context
-stop matching. The step fails loudly when no status on the commit
-carries that context, so no rename can silently disable the rewrite.
+`failure` status, marking a commit red that was never tested.
+Cancellation is unconditional server-side for
+push events, so the superseding run rewrites the exact
+`Has been cancelled` status to `skipped`. Genuine failures are never
+touched.
 
 ## TODO
 

go.mod

@@ -17,7 +17,6 @@ require (
 	github.com/stretchr/testify v1.8.4
 	go.uber.org/fx v1.20.1
 	golang.org/x/crypto v0.38.0
-	gopkg.in/yaml.v3 v3.0.1
 	gorm.io/driver/sqlite v1.5.4
 	gorm.io/gorm v1.25.5
 	modernc.org/sqlite v1.28.0
@@ -53,6 +52,7 @@ require (
 	golang.org/x/text v0.25.0 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 	lukechampine.com/uint128 v1.2.0 // indirect
 	modernc.org/cc/v3 v3.40.0 // indirect
 	modernc.org/ccgo/v3 v3.16.13 // indirect

internal/ciscript/ci_mark_superseded_test.go

@@ -1,387 +0,0 @@
-package ciscript_test
-
-import (
-	"maps"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"slices"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/require"
-	"gopkg.in/yaml.v3"
-)
-
-const (
-	// supersededDesc is the description script/ci-mark-superseded
-	// writes, and the one an earlier revision of it wrote alongside a
-	// `skipped` state.
-	supersededDesc = "Superseded by a newer commit; never tested"
-
-	// liveContext is the commit-status context Gitea uses for this
-	// repository's runs, as seen in its API. The script derives it from
-	// the workflow and job names rather than hardcoding it; the
-	// derivation is checked against this value below.
-	liveContext = "check / check (push)"
-
-	scriptPath = "../../script/ci-mark-superseded"
-	workflow   = "../../.gitea/workflows/check.yml"
-
-	// failure is the only state that neither folds into a combined
-	// `success` (as `skipped` does) nor blocks the commit forever (as
-	// `pending` does).
-	failure = "failure"
-)
-
-// repo is a throwaway git history: parent is the commit a run would be
-// cancelled on, head the commit that superseded it.
-type repo struct {
-	dir    string
-	head   string
-	parent string
-}
-
-// scriptEnv is the run identity the Gitea runner exports and the script
-// builds its context string from.
-type scriptEnv struct {
-	workflow string
-	job      string
-	event    string
-}
-
-func defaultEnv() scriptEnv {
-	return scriptEnv{workflow: "check", job: "check", event: "push"}
-}
-
-func cancelled() commitStatus {
-	return commitStatus{
-		Context:     liveContext,
-		Status:      failure,
-		Description: "Has been cancelled",
-	}
-}
-
-func running() commitStatus {
-	return commitStatus{
-		Context:     liveContext,
-		Status:      "pending",
-		Description: "Has started running",
-	}
-}
-
-func TestMarkSuperseded(t *testing.T) {
-	t.Parallel()
-
-	cases := map[string]struct {
-		parent   commitStatus
-		wantMark bool
-	}{
-		"a cancelled run is marked": {
-			parent:   cancelled(),
-			wantMark: true,
-		},
-		"a laundered skipped status is marked": {
-			parent: commitStatus{
-				Context:     liveContext,
-				Status:      "skipped",
-				Description: supersededDesc,
-			},
-			wantMark: true,
-		},
-		"a genuine failure is left alone": {
-			parent: commitStatus{
-				Context:     liveContext,
-				Status:      failure,
-				Description: "Failing after 3m1s",
-			},
-			wantMark: false,
-		},
-		"a passing run is left alone": {
-			parent: commitStatus{
-				Context:     liveContext,
-				Status:      "success",
-				Description: "Successful in 2m52s",
-			},
-			wantMark: false,
-		},
-		"another context is left alone": {
-			parent: commitStatus{
-				Context:     "other / other (push)",
-				Status:      failure,
-				Description: "Has been cancelled",
-			},
-			wantMark: false,
-		},
-	}
-
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-			requireTools(t)
-
-			history := newRepo(t)
-			fake, api := newFakeGitea(t)
-			fake.setStatus(history.head, running())
-			fake.setStatus(history.parent, tc.parent)
-
-			out, err := runScript(t, history, api, defaultEnv())
-			require.NoError(t, err, out)
-
-			posted := fake.postedFor(history.parent)
-			if !tc.wantMark {
-				require.Empty(t, posted)
-
-				return
-			}
-
-			require.Equal(t, []postedStatus{{
-				Context: liveContext,
-				// Not `skipped`: Gitea's combined status folds
-				// that into `success`, which is what made a
-				// never-tested commit read green.
-				State:       failure,
-				Description: supersededDesc,
-			}}, posted)
-		})
-	}
-}
-
-// A second run must not rewrite what the first one wrote, or every
-// later push would post a duplicate status.
-func TestMarkSupersededIsIdempotent(t *testing.T) {
-	t.Parallel()
-	requireTools(t)
-
-	history := newRepo(t)
-	fake, api := newFakeGitea(t)
-	fake.setStatus(history.head, running())
-	fake.setStatus(history.parent, cancelled())
-
-	for range 2 {
-		out, err := runScript(t, history, api, defaultEnv())
-		require.NoError(t, err, out)
-	}
-
-	require.Len(t, fake.postedFor(history.parent), 1)
-}
-
-// Renaming the workflow or the job changes the context string Gitea
-// uses. The script must say so instead of quietly matching nothing.
-func TestMarkSupersededRejectsAnUnknownContext(t *testing.T) {
-	t.Parallel()
-	requireTools(t)
-
-	history := newRepo(t)
-	fake, api := newFakeGitea(t)
-	fake.setStatus(history.head, running())
-	fake.setStatus(history.parent, cancelled())
-
-	env := defaultEnv()
-	env.job = "renamed"
-
-	out, err := runScript(t, history, api, env)
-	require.Error(t, err)
-	require.Contains(t, out, "renamed")
-	require.Contains(t, out, liveContext)
-	require.Empty(t, fake.postedFor(history.parent))
-}
-
-// ANCESTOR_LIMIT is a documented knob. A value that is set but unusable
-// must abort: handing it to git and discarding the exit status left the
-// walk empty and the step green, marking nothing.
-func TestMarkSupersededRejectsAnUnparseableAncestorLimit(t *testing.T) {
-	t.Parallel()
-	requireTools(t)
-
-	history := newRepo(t)
-	fake, api := newFakeGitea(t)
-	fake.setStatus(history.head, running())
-	fake.setStatus(history.parent, cancelled())
-
-	out, err := runScript(
-		t, history, api, defaultEnv(), "ANCESTOR_LIMIT=twenty",
-	)
-	require.Error(t, err)
-	require.Contains(t, out, "ANCESTOR_LIMIT")
-	require.Contains(t, out, "twenty")
-	require.Empty(t, fake.postedFor(history.parent))
-}
-
-// A status read that fails is not the same as a commit with nothing to
-// do. Losing curl's exit status through a pipe made the two identical
-// and left a laundered commit laundered with no signal.
-func TestMarkSupersededFailsOnAnUnreadableAncestorStatus(t *testing.T) {
-	t.Parallel()
-	requireTools(t)
-
-	history := newRepo(t)
-	fake, api := newFakeGitea(t)
-	fake.setStatus(history.head, running())
-	fake.setStatus(history.parent, cancelled())
-	fake.failStatusRead(history.parent)
-
-	out, err := runScript(t, history, api, defaultEnv())
-	require.Error(t, err)
-	require.Contains(t, out, history.parent)
-	require.Contains(t, out, "cannot read commit statuses")
-	require.Empty(t, fake.postedFor(history.parent))
-}
-
-// A shallow clone cannot resolve the parent, so it is indistinguishable
-// from a root commit to rev-parse and the walk would exit 0 having
-// marked nothing. It must abort instead: dropping `fetch-depth: 0` from
-// the checkout step is one edit, and a silent no-op there restores the
-// false-green bug this script exists to prevent.
-func TestMarkSupersededRejectsAShallowRepository(t *testing.T) {
-	t.Parallel()
-	requireTools(t)
-
-	history := shallowClone(t, newRepo(t))
-	fake, api := newFakeGitea(t)
-	fake.setStatus(history.head, running())
-	fake.setStatus(history.parent, cancelled())
-
-	out, err := runScript(t, history, api, defaultEnv())
-	require.Error(t, err)
-	require.Contains(t, out, "shallow repository")
-	require.Empty(t, fake.postedFor(history.parent))
-	require.Empty(t, fake.postedFor(history.head))
-}
-
-// shallowClone returns the same history as a depth-1 clone. The `file://`
-// URL is required: git ignores --depth for a plain local path.
-func shallowClone(t *testing.T, history repo) repo {
-	t.Helper()
-
-	dir := t.TempDir()
-
-	//nolint:gosec // fixed argv, arguments are test-local paths
-	cmd := exec.CommandContext(t.Context(), "git", "clone", "-q",
-		"--depth=1", "file://"+history.dir, dir)
-
-	out, err := cmd.CombinedOutput()
-	require.NoError(t, err, string(out))
-
-	return repo{dir: dir, head: history.head, parent: history.parent}
-}
-
-// The derived context must equal the one Gitea actually uses, which is
-// built from the same workflow and job names.
-func TestDerivedContextMatchesGitea(t *testing.T) {
-	t.Parallel()
-	requireTools(t)
-
-	name, job := workflowIdentity(t)
-
-	history := newRepo(t)
-	fake, api := newFakeGitea(t)
-	fake.setStatus(history.head, running())
-	fake.setStatus(history.parent, cancelled())
-
-	out, err := runScript(t, history, api, scriptEnv{
-		workflow: name,
-		job:      job,
-		event:    "push",
-	})
-	require.NoError(t, err, out)
-
-	posted := fake.postedFor(history.parent)
-	require.Len(t, posted, 1)
-	require.Equal(t, liveContext, posted[0].Context)
-}
-
-// workflowIdentity reads the workflow name and its single job id out of
-// the checked-in workflow file.
-func workflowIdentity(t *testing.T) (string, string) {
-	t.Helper()
-
-	raw, err := os.ReadFile(workflow)
-	require.NoError(t, err)
-
-	var parsed struct {
-		Name string         `yaml:"name"`
-		Jobs map[string]any `yaml:"jobs"`
-	}
-
-	require.NoError(t, yaml.Unmarshal(raw, &parsed))
-
-	jobs := slices.Collect(maps.Keys(parsed.Jobs))
-	require.Len(t, jobs, 1)
-
-	return parsed.Name, jobs[0]
-}
-
-func runScript(
-	t *testing.T, history repo, api string, env scriptEnv,
-	extra ...string,
-) (string, error) {
-	t.Helper()
-
-	script, err := filepath.Abs(scriptPath)
-	require.NoError(t, err)
-
-	//nolint:gosec // fixed argv, repo-local script under test
-	cmd := exec.CommandContext(t.Context(), "sh", script)
-	cmd.Dir = history.dir
-	cmd.Env = append(os.Environ(),
-		"GITHUB_API_URL="+api,
-		"GITHUB_REPOSITORY=sneak/webhooker",
-		"GITHUB_SHA="+history.head,
-		"GITHUB_WORKFLOW="+env.workflow,
-		"GITHUB_JOB="+env.job,
-		"GITHUB_EVENT_NAME="+env.event,
-		"GITEA_TOKEN=test-token",
-	)
-	cmd.Env = append(cmd.Env, extra...)
-
-	out, err := cmd.CombinedOutput()
-
-	return string(out), err
-}
-
-func newRepo(t *testing.T) repo {
-	t.Helper()
-
-	dir := t.TempDir()
-
-	git := func(args ...string) string {
-		//nolint:gosec // fixed argv, arguments are test constants
-		cmd := exec.CommandContext(t.Context(), "git", args...)
-		cmd.Dir = dir
-
-		out, err := cmd.CombinedOutput()
-		require.NoError(t, err, string(out))
-
-		return strings.TrimSpace(string(out))
-	}
-
-	commit := func(message string) string {
-		git(
-			"-c", "user.email=ci@example.invalid",
-			"-c", "user.name=ci",
-			"-c", "commit.gpgsign=false",
-			"commit", "-q", "--allow-empty", "-m", message,
-		)
-
-		return git("rev-parse", "HEAD")
-	}
-
-	git("init", "-q", "-b", "main")
-
-	parent := commit("parent")
-	head := commit("head")
-
-	return repo{dir: dir, head: head, parent: parent}
-}
-
-func requireTools(t *testing.T) {
-	t.Helper()
-
-	for _, tool := range []string{"sh", "git", "curl", "jq"} {
-		_, err := exec.LookPath(tool)
-		if err != nil {
-			t.Skipf("%s is not installed: %v", tool, err)
-		}
-	}
-}

internal/ciscript/doc.go

@@ -1,10 +0,0 @@
-// Package ciscript holds the tests for the repository's CI shell
-// scripts in script/. It carries no runtime code: the scripts run on
-// the CI runner, not inside the binary, but their behaviour still has
-// to be verified by the test suite.
-//
-// The scripts under test are outside the Go build graph, so `go test`'s
-// result cache serves a stale PASS when only a script changed: run the
-// container build, or GOFLAGS=-count=1, to trust a result here after
-// editing script/.
-package ciscript

internal/ciscript/fakegitea_test.go

@@ -1,162 +0,0 @@
-package ciscript_test
-
-import (
-	"encoding/json"
-	"net/http"
-	"net/http/httptest"
-	"sync"
-	"testing"
-)
-
-// commitStatus is the part of an entry in Gitea's combined-status
-// response that script/ci-mark-superseded reads.
-type commitStatus struct {
-	Context     string `json:"context"`
-	Status      string `json:"status"`
-	Description string `json:"description"`
-}
-
-// postedStatus is the part of a create-status request body the script
-// writes.
-type postedStatus struct {
-	Context     string `json:"context"`
-	State       string `json:"state"`
-	Description string `json:"description"`
-}
-
-// fakeGitea serves the two endpoints the script talks to. Like Gitea,
-// the newest status for a context replaces the previous one, so a
-// second run of the script sees what the first one wrote.
-type fakeGitea struct {
-	mu       sync.Mutex
-	statuses map[string][]commitStatus
-	posted   map[string][]postedStatus
-	// failRead is a commit whose combined-status read answers HTTP
-	// 500, standing in for a status API that is down.
-	failRead string
-}
-
-// newFakeGitea returns the fake and the base URL to hand the script as
-// GITHUB_API_URL.
-func newFakeGitea(t *testing.T) (*fakeGitea, string) {
-	t.Helper()
-
-	fake := &fakeGitea{
-		mu:       sync.Mutex{},
-		statuses: map[string][]commitStatus{},
-		posted:   map[string][]postedStatus{},
-		failRead: "",
-	}
-
-	srv := httptest.NewServer(fake.routes())
-	t.Cleanup(srv.Close)
-
-	return fake, srv.URL
-}
-
-func (f *fakeGitea) routes() http.Handler {
-	mux := http.NewServeMux()
-
-	mux.HandleFunc(
-		"GET /repos/{owner}/{repo}/commits/{sha}/status",
-		f.handleCombined,
-	)
-	mux.HandleFunc(
-		"POST /repos/{owner}/{repo}/statuses/{sha}",
-		f.handleCreate,
-	)
-
-	return mux
-}
-
-func (f *fakeGitea) handleCombined(
-	w http.ResponseWriter, r *http.Request,
-) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	sha := r.PathValue("sha")
-	if f.failRead != "" && f.failRead == sha {
-		http.Error(w, "boom", http.StatusInternalServerError)
-
-		return
-	}
-
-	body := struct {
-		Statuses []commitStatus `json:"statuses"`
-	}{Statuses: f.statuses[sha]}
-
-	payload, err := json.Marshal(body)
-	if err != nil {
-		http.Error(w, err.Error(), http.StatusInternalServerError)
-
-		return
-	}
-
-	w.Header().Set("Content-Type", "application/json")
-
-	_, _ = w.Write(payload)
-}
-
-func (f *fakeGitea) handleCreate(w http.ResponseWriter, r *http.Request) {
-	var got postedStatus
-
-	err := json.NewDecoder(r.Body).Decode(&got)
-	if err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
-
-		return
-	}
-
-	sha := r.PathValue("sha")
-
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	f.posted[sha] = append(f.posted[sha], got)
-	f.replaceLocked(sha, commitStatus{
-		Context:     got.Context,
-		Status:      got.State,
-		Description: got.Description,
-	})
-
-	w.WriteHeader(http.StatusCreated)
-}
-
-// failStatusRead makes the combined-status read for one commit answer
-// HTTP 500.
-func (f *fakeGitea) failStatusRead(sha string) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	f.failRead = sha
-}
-
-// setStatus gives a commit its latest status for a context.
-func (f *fakeGitea) setStatus(sha string, status commitStatus) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	f.replaceLocked(sha, status)
-}
-
-// postedFor returns the statuses the script created for a commit.
-func (f *fakeGitea) postedFor(sha string) []postedStatus {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	return append([]postedStatus(nil), f.posted[sha]...)
-}
-
-// replaceLocked requires f.mu.
-func (f *fakeGitea) replaceLocked(sha string, status commitStatus) {
-	for i, existing := range f.statuses[sha] {
-		if existing.Context == status.Context {
-			f.statuses[sha][i] = status
-
-			return
-		}
-	}
-
-	f.statuses[sha] = append(f.statuses[sha], status)
-}

internal/handlers/event_body.go

@@ -1,199 +0,0 @@
-package handlers
-
-import (
-	"database/sql"
-	"errors"
-	"net/http"
-	"strconv"
-
-	"github.com/go-chi/chi"
-	"github.com/google/uuid"
-	"gorm.io/gorm"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// eventBodyQuery reads one event's stored body as bytes. The cast
-// to blob is what makes the driver hand back the stored bytes
-// rather than a string conversion, so Content-Length taken from
-// the result matches what goes on the wire. The soft-delete
-// predicate is spelled out because Raw bypasses GORM's default
-// scope, and it is what stops a reaped event still being
-// downloadable.
-const eventBodyQuery = "SELECT cast(body as blob) " +
-	"FROM events WHERE id = ? AND webhook_id = ? AND deleted_at IS NULL"
-
-// HandleEventBodyDownload serves one event's stored body in
-// full, which the event log page cannot: it caps each rendered
-// body at maxRenderedBodyBytes.
-//
-// The bytes are attacker-supplied — anyone who can reach the
-// public receiver chooses them — and this route hands them back
-// inside the operator's own authenticated origin, so the
-// 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 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)
-		if !ok {
-			return
-		}
-
-		// Parsing the id before use serves two purposes: a
-		// malformed id can never reach the SQL or the response
-		// header, and the canonical form below is drawn from
-		// uuid's own fixed alphabet rather than from the
-		// request, so the Content-Disposition value cannot be
-		// steered by a client.
-		eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
-		if err != nil {
-			http.NotFound(w, r)
-
-			return
-		}
-
-		h.serveEventBody(w, r, webhook, eventID.String())
-	}
-}
-
-// serveEventBody writes the named event's stored body to w.
-//
-// The event must belong to webhook, which is what keeps this
-// route from reading any event in the system by id alone. Two
-// things enforce that and they are not equally strong. The
-// operative one is that events live in a per-webhook SQLite
-// file, so a sibling webhook's event is not in the database
-// being queried at all. The webhook_id predicate on the query
-// below is the second guard, and it is currently redundant
-// against that isolation; it is there so the scoping survives
-// any future change that puts more than one webhook's events in
-// one file.
-//
-// The body is read in one query and held whole in memory while
-// it is written. That costs roughly two body-sized allocations
-// per concurrent download, not one: the driver's column buffer
-// and the copy database/sql makes in convertAssign when a
-// []byte column is scanned into a *[]byte are live at the same
-// time. Measured allocation is ~2x the body plus ~45 KB, so at
-// the 1 MB ingest cap a download costs ~2 MB of Go heap. On
-// top of that, SQLite's own materialisation of the column
-// value sits in the driver's allocator outside the Go heap, so
-// process peak is higher again: 2x is a floor, not a ceiling.
-// There is no cheaper bound available — database/sql exposes
-// no incremental handle on a SQLite BLOB, and reading byte
-// ranges with substr does not avoid the cost either, because
-// SQLite materialises the whole column value to evaluate each
-// substr call. Range reads only pay for that materialisation
-// once per range.
-//
-// One consequence is worth keeping in view: the read finishes
-// before the client is written to, so no read lock is held for
-// the length of a slow download. These per-webhook databases
-// run in SQLite's default journal mode rather than WAL, so a
-// lock held that long would block the receiver from recording
-// new events.
-func (h *Handlers) serveEventBody(
-	w http.ResponseWriter,
-	r *http.Request,
-	webhook database.Webhook,
-	eventID string,
-) {
-	if !h.dbMgr.DBExists(webhook.ID) {
-		http.NotFound(w, r)
-
-		return
-	}
-
-	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
-	if err != nil {
-		h.serverError(w, "failed to get webhook database", err)
-
-		return
-	}
-
-	body, found, err := eventBody(webhookDB, webhook.ID, eventID)
-	if err != nil {
-		h.serverError(w, "failed to read event body", err)
-
-		return
-	}
-
-	// A miss is a 404 whether the event belongs to another
-	// webhook or does not exist at all, so the response does
-	// not report which. Reading the body before any header is
-	// written is also what keeps an event reaped mid-request
-	// from producing a torn response: either the read finds the
-	// row and the whole body is served, or it does not and the
-	// response is a clean 404.
-	if !found {
-		http.NotFound(w, r)
-
-		return
-	}
-
-	setEventBodyHeaders(w, eventID, int64(len(body)))
-
-	_, err = w.Write(body)
-	if err != nil {
-		// The status and Content-Length are already committed,
-		// so the client sees a short download. There is no way
-		// to report a 500 from here; the log is the record.
-		h.log.Error(
-			"failed to write event body",
-			"webhook_id", webhook.ID,
-			"event_id", eventID,
-			"error", err,
-		)
-	}
-}
-
-// eventBody returns an event's stored body and whether the event
-// exists within the webhook.
-func eventBody(
-	webhookDB *gorm.DB,
-	webhookID, eventID string,
-) ([]byte, bool, error) {
-	var body []byte
-
-	err := webhookDB.Raw(
-		eventBodyQuery, eventID, webhookID,
-	).Row().Scan(&body)
-
-	if errors.Is(err, sql.ErrNoRows) {
-		return nil, false, nil
-	}
-
-	if err != nil {
-		return nil, false, err
-	}
-
-	return body, true, nil
-}
-
-// setEventBodyHeaders applies the response headers that make
-// this route safe to hand attacker-supplied bytes through. See
-// HandleEventBodyDownload for why they are a security control
-// and not a formatting choice.
-//
-// nosniff is also set by the global SecurityHeaders middleware.
-// It is repeated here so the guarantee belongs to the route
-// that needs it rather than to a middleware someone could
-// reorder or scope away.
-func setEventBodyHeaders(
-	w http.ResponseWriter,
-	eventID string,
-	size int64,
-) {
-	w.Header().Set("Content-Type", "application/octet-stream")
-	w.Header().Set("X-Content-Type-Options", "nosniff")
-	w.Header().Set(
-		"Content-Disposition",
-		`attachment; filename="webhooker-event-`+eventID+`.bin"`,
-	)
-	w.Header().Set("Content-Length", strconv.FormatInt(size, 10))
-}

internal/handlers/event_body_test.go

@@ -1,506 +0,0 @@
-package handlers_test
-
-import (
-	"context"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"strconv"
-	"strings"
-	"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/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-// paramEventID is the chi URL parameter the body download
-// handler reads.
-const paramEventID = "eventID"
-
-// otherTestUserID owns webhooks the session user must not be
-// able to read.
-const otherTestUserID = "other-user-id"
-
-// seedWebhookFor inserts a webhook owned by the given user.
-func seedWebhookFor(
-	t *testing.T,
-	db *database.Database,
-	userID string,
-) *database.Webhook {
-	t.Helper()
-
-	wh := &database.Webhook{
-		UserID: userID,
-		Name:   "wh-" + userID,
-	}
-
-	require.NoError(
-		t,
-		db.DB().Omit(clause.Associations).Create(wh).Error,
-	)
-
-	return wh
-}
-
-// fetchEventBody runs the real download handler as the test user
-// for the given source and event ids.
-func fetchEventBody(
-	t *testing.T,
-	h *handlers.Handlers,
-	sess *session.Session,
-	sourceID, eventID string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	// The path is escaped and the raw id goes in the route
-	// context, which is what chi hands a handler: the param is
-	// already percent-decoded by the time it is read.
-	req := httptest.NewRequestWithContext(
-		context.Background(),
-		http.MethodGet,
-		"/source/"+url.PathEscape(sourceID)+
-			"/logs/"+url.PathEscape(eventID)+"/body",
-		nil,
-	)
-
-	for _, c := range authenticatedCookies(
-		t, sess, deleteTestUserID, deleteTestUsername,
-	) {
-		req.AddCookie(c)
-	}
-
-	rctx := chi.NewRouteContext()
-	rctx.URLParams.Add(paramSourceID, sourceID)
-	rctx.URLParams.Add(paramEventID, eventID)
-
-	req = req.WithContext(
-		context.WithValue(
-			req.Context(), chi.RouteCtxKey, rctx,
-		),
-	)
-
-	w := httptest.NewRecorder()
-	h.HandleEventBodyDownload().ServeHTTP(w, req)
-
-	return w
-}
-
-// TestHandleEventBodyDownload_ServesOversizeBodyInFull is the
-// capability the render cap took away: a body far above what the
-// event log page will show comes back whole and byte-identical,
-// with the headers that keep it from being rendered.
-func TestHandleEventBodyDownload_ServesOversizeBodyInFull(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	// Far above the render cap, with multibyte runes and a
-	// distinctive tail, so a body that the log page can only
-	// show a slice of comes back whole and in order.
-	const sentinel = "TAIL-SENTINEL-1f4a9c"
-
-	stored := strings.Repeat("A", 200*1024) +
-		strings.Repeat(snowman, 1000) + sentinel
-
-	wh := seedWebhook(t, db)
-	evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
-
-	w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
-
-	require.Equal(t, http.StatusOK, w.Code)
-	assert.Greater(t, len(stored), bodyCap)
-	assert.Equal(t, stored, w.Body.String())
-	assert.Equal(
-		t, strconv.Itoa(len(stored)),
-		w.Header().Get("Content-Length"),
-	)
-}
-
-// TestHandleEventBodyDownload_BodiesRoundTripByteIdentical
-// covers the sizes and byte values a stored body can actually
-// take: empty, one byte, either side of the render cap, and
-// bytes that are not text at all. Content-Length has to equal
-// the bytes written in every case, since it is derived from the
-// same read that produces them.
-func TestHandleEventBodyDownload_BodiesRoundTripByteIdentical(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	// A NUL, invalid UTF-8 and a multibyte rune, so nothing on
-	// the path can be treating the body as text.
-	binary := "\x00\x01\xff\xfe" + snowman + "\x00tail"
-
-	cases := map[string]string{
-		"empty":         "",
-		"single byte":   "x",
-		"one below cap": strings.Repeat("b", bodyCap-1),
-		"exactly cap":   strings.Repeat("c", bodyCap),
-		"one above cap": strings.Repeat("d", bodyCap+1),
-		"binary":        binary,
-	}
-
-	for name, stored := range cases {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			var (
-				h     *handlers.Handlers
-				sess  *session.Session
-				db    *database.Database
-				dbMgr *database.WebhookDBManager
-			)
-
-			app := newTestApp(t, &h, &sess, &db, &dbMgr)
-			app.RequireStart()
-
-			t.Cleanup(app.RequireStop)
-
-			wh := seedWebhook(t, db)
-			evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
-
-			w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
-
-			require.Equal(t, http.StatusOK, w.Code)
-			assert.Equal(t, stored, w.Body.String())
-			assert.Equal(
-				t, strconv.Itoa(len(stored)),
-				w.Header().Get("Content-Length"),
-			)
-			assert.Equal(
-				t, len(stored), w.Body.Len(),
-				"Content-Length must equal bytes written",
-			)
-		})
-	}
-}
-
-// TestHandleEventBodyDownload_HeadersAreNotRenderable pins the
-// response headers that stop attacker-supplied bytes executing
-// in the operator's own origin. They are a security control, not
-// presentation.
-func TestHandleEventBodyDownload_HeadersAreNotRenderable(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	wh := seedWebhook(t, db)
-	evt := seedEventWithBody(t, dbMgr, wh.ID, `{"small":true}`)
-
-	w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
-
-	require.Equal(t, http.StatusOK, w.Code)
-	assert.Equal(
-		t, "application/octet-stream",
-		w.Header().Get("Content-Type"),
-	)
-	assert.Equal(
-		t, "nosniff",
-		w.Header().Get("X-Content-Type-Options"),
-	)
-
-	disposition := w.Header().Get("Content-Disposition")
-	assert.Equal(
-		t,
-		`attachment; filename="webhooker-event-`+evt.ID+`.bin"`,
-		disposition,
-	)
-}
-
-// TestHandleEventBodyDownload_ScriptBodyStaysInert proves a
-// stored HTML payload is handed back as an attachment of opaque
-// bytes rather than as anything a browser will execute. The
-// bytes themselves are unaltered: this route reports what was
-// delivered.
-func TestHandleEventBodyDownload_ScriptBodyStaysInert(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	const payload = `<html><script>alert(document.cookie)` +
-		`</script></html>`
-
-	wh := seedWebhook(t, db)
-	evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
-
-	w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
-
-	require.Equal(t, http.StatusOK, w.Code)
-	assert.Equal(t, payload, w.Body.String())
-
-	contentType := w.Header().Get("Content-Type")
-	assert.Equal(t, "application/octet-stream", contentType)
-	assert.NotContains(t, contentType, "html")
-	assert.NotContains(t, contentType, "xml")
-	assert.NotContains(t, contentType, "javascript")
-	assert.Contains(
-		t, w.Header().Get("Content-Disposition"), "attachment",
-	)
-	assert.Equal(
-		t, "nosniff",
-		w.Header().Get("X-Content-Type-Options"),
-	)
-}
-
-// TestHandleEventBodyDownload_OtherUsersEvent404s is the
-// authorization test the definition of done asks for: an event
-// stored under a webhook the session user does not own is not
-// readable, and the miss does not distinguish itself from a
-// nonexistent one.
-func TestHandleEventBodyDownload_OtherUsersEvent404s(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	const theirPayload = "OTHER-USERS-PAYLOAD-8b1d"
-
-	theirs := seedWebhookFor(t, db, otherTestUserID)
-	evt := seedEventWithBody(t, dbMgr, theirs.ID, theirPayload)
-
-	w := fetchEventBody(t, h, sess, theirs.ID, evt.ID)
-
-	assert.Equal(t, http.StatusNotFound, w.Code)
-	assert.NotContains(t, w.Body.String(), theirPayload)
-}
-
-// TestHandleEventBodyDownload_EventOfAnotherWebhook404s pins
-// that holding a valid event id is not enough: the event has to
-// belong to the webhook in the path. Both webhooks here are the
-// session user's and both have event databases, so the
-// ownership check cannot be what produces the 404.
-//
-// What does produce it is the per-webhook database file rather
-// than the webhook_id predicate on the query — removing that
-// predicate leaves this test green, because the sibling's event
-// is in a different file. The test is kept as the behavioural
-// guard the route owes; see serveEventBody for which mechanism
-// is load-bearing.
-func TestHandleEventBodyDownload_EventOfAnotherWebhook404s(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	const other = "BELONGS-TO-THE-OTHER-WEBHOOK-3c7e"
-
-	mine := seedWebhook(t, db)
-	seedEventWithBody(t, dbMgr, mine.ID, `{"mine":true}`)
-
-	sibling := seedWebhook(t, db)
-	evt := seedEventWithBody(t, dbMgr, sibling.ID, other)
-
-	w := fetchEventBody(t, h, sess, mine.ID, evt.ID)
-
-	assert.Equal(t, http.StatusNotFound, w.Code)
-	assert.NotContains(t, w.Body.String(), other)
-}
-
-// TestHandleEventBodyDownload_UnknownEvent404s covers the plain
-// miss, including an id that is not a uuid at all and so never
-// reaches the query or the response header.
-func TestHandleEventBodyDownload_UnknownEvent404s(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	wh := seedWebhook(t, db)
-	seedEventWithBody(t, dbMgr, wh.ID, `{"mine":true}`)
-
-	for _, id := range []string{
-		uuid.New().String(),
-		`../../etc/passwd`,
-		"not-a-uuid",
-		`x"; rm -rf /`,
-	} {
-		w := fetchEventBody(t, h, sess, wh.ID, id)
-
-		assert.Equal(
-			t, http.StatusNotFound, w.Code,
-			"event id %q", id,
-		)
-		assert.Empty(
-			t, w.Header().Get("Content-Disposition"),
-			"event id %q must not reach a header", id,
-		)
-	}
-}
-
-// TestHandleEventBodyDownload_ReapedEvent404s pins what happens
-// when the retention reaper takes an event out from under this
-// route. The body is read in one query before any header is
-// written, so a reaped event cannot produce a partial download:
-// it is a clean 404 with no Content-Length and no
-// Content-Disposition. Both removals the codebase performs are
-// covered — the reaper hard-deletes, and a soft-deleted row is
-// excluded by the query's own deleted_at predicate rather than
-// by GORM's default scope, which Raw bypasses.
-func TestHandleEventBodyDownload_ReapedEvent404s(t *testing.T) {
-	t.Parallel()
-
-	for name, hard := range map[string]bool{
-		"soft deleted": false,
-		"hard deleted": true,
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			var (
-				h     *handlers.Handlers
-				sess  *session.Session
-				db    *database.Database
-				dbMgr *database.WebhookDBManager
-			)
-
-			app := newTestApp(t, &h, &sess, &db, &dbMgr)
-			app.RequireStart()
-
-			t.Cleanup(app.RequireStop)
-
-			const payload = "REAPED-PAYLOAD-4d2a"
-
-			wh := seedWebhook(t, db)
-			evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
-
-			webhookDB, err := dbMgr.GetDB(wh.ID)
-			require.NoError(t, err)
-
-			del := webhookDB
-			if hard {
-				del = del.Unscoped()
-			}
-
-			require.NoError(
-				t,
-				del.Delete(&database.Event{}, "id = ?", evt.ID).
-					Error,
-			)
-
-			w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
-
-			assert.Equal(t, http.StatusNotFound, w.Code)
-			assert.NotContains(t, w.Body.String(), payload)
-			assert.Empty(t, w.Header().Get("Content-Length"))
-			assert.Empty(
-				t, w.Header().Get("Content-Disposition"),
-			)
-		})
-	}
-}
-
-// TestHandleSourceLogs_TruncationMarkerLinksToDownload proves
-// the page tells the reader where the rest of the body is, and
-// only when there is a rest to fetch.
-func TestHandleSourceLogs_TruncationMarkerLinksToDownload(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	var (
-		h     *handlers.Handlers
-		sess  *session.Session
-		db    *database.Database
-		dbMgr *database.WebhookDBManager
-	)
-
-	app := newTestApp(t, &h, &sess, &db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	big := seedWebhook(t, db)
-	bigEvt := seedEventWithBody(
-		t, dbMgr, big.ID, strings.Repeat("A", 4*bodyCap),
-	)
-
-	page := renderSourceLogsPage(t, h, sess, big.ID)
-	assert.Contains(
-		t, page,
-		"/source/"+big.ID+"/logs/"+bigEvt.ID+"/body",
-	)
-
-	small := seedWebhook(t, db)
-	smallEvt := seedEventWithBody(
-		t, dbMgr, small.ID, `{"kept":"whole"}`,
-	)
-
-	page = renderSourceLogsPage(t, h, sess, small.ID)
-	assert.NotContains(
-		t, page,
-		"/source/"+small.ID+"/logs/"+smallEvt.ID+"/body",
-	)
-}

internal/handlers/event_log_view_test.go

@@ -25,14 +25,13 @@ const bodyCap = handlers.MaxRenderedBodyBytesForTest
 const snowman = "☃"
 
 // seedEventWithBody records one event with the given body in the
-// webhook's own database and returns it, so a caller that needs
-// the generated event id can have it.
+// webhook's own database.
 func seedEventWithBody(
 	t *testing.T,
 	dbMgr *database.WebhookDBManager,
 	webhookID string,
 	body string,
-) *database.Event {
+) {
 	t.Helper()
 
 	webhookDB, err := dbMgr.GetDB(webhookID)
@@ -48,8 +47,6 @@ func seedEventWithBody(
 	require.NoError(t, webhookDB.Omit(
 		clause.Associations,
 	).Create(event).Error)
-
-	return event
 }
 
 // seedAndProject stores one body and returns the projection the

internal/handlers/source_management.go

@@ -713,55 +713,29 @@ func (h *Handlers) evictArchiveWriterIfUnused(webhookID string) {
 	h.evictArchiveWriter(webhookID)
 }
 
-// ownedWebhook resolves the request's sourceID parameter to a
-// webhook the session's user owns.
-//
-// Ownership and existence are decided by one query, so a
-// webhook belonging to another user is indistinguishable from
-// one that does not exist: both are a 404, and neither confirms
-// the id. Callers that reach further into a webhook's data —
-// the event log page and the event body download — share this
-// one check rather than restating it, so the download cannot
-// come to authorize differently from the page that links to it.
-//
-// It reports false once it has written the response, which is a
-// redirect to the login page for an unauthenticated request and
-// a 404 otherwise. The caller returns without writing more.
-func (h *Handlers) ownedWebhook(
-	w http.ResponseWriter,
-	r *http.Request,
-) (database.Webhook, bool) {
-	var webhook database.Webhook
-
+// HandleSourceLogs shows the request/response logs for a
+// webhook.
+func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
 		userID, ok := h.getUserID(r)
 		if !ok {
 			http.Redirect(
 				w, r, "/pages/login", http.StatusSeeOther,
 			)
 
-		return database.Webhook{}, false
+			return
 		}
 
 		sourceID := chi.URLParam(r, "sourceID")
 
+		var webhook database.Webhook
+
 		err := h.db.DB().Where(
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
 			http.NotFound(w, r)
 
-		return database.Webhook{}, false
-	}
-
-	return webhook, true
-}
-
-// HandleSourceLogs shows the request/response logs for a
-// webhook.
-func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
-		webhook, ok := h.ownedWebhook(w, r)
-		if !ok {
 			return
 		}
 

internal/middleware/accesslog_test.go

@@ -1,658 +0,0 @@
-package middleware_test
-
-import (
-	"bytes"
-	"context"
-	"encoding/json"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-
-	"github.com/go-chi/chi"
-	chimw "github.com/go-chi/chi/middleware"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/config"
-	"sneak.berlin/go/webhooker/internal/middleware"
-)
-
-// floodRequests is the number of distinct invented paths each flood
-// test drives through the access log.
-const floodRequests = 64
-
-// attackerMarker is embedded in every invented path. No access log
-// line for a redirected or rejected request may contain it.
-const attackerMarker = "QQATTACKERTEXTQQ"
-
-// 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
-// send.
-const maxLineBytes = 1024
-
-// maxCappedLineBytes bounds a single access log line when every
-// client-supplied field arrives oversized and is truncated to its
-// budget. This is the number the README quotes as the per-line cost an
-// operator sizes log storage against, and it is a bound on the
-// ENCODED line, which is what the operator's disk holds.
-const maxCappedLineBytes = 2560
-
-// oversizedSegmentBytes is the length of the single attacker-chosen
-// path segment, query string or header used to show line size does not
-// track input size.
-const oversizedSegmentBytes = 8192
-
-// tailMarker is placed at the END of an oversized header value, so its
-// absence from the log proves the value was truncated rather than
-// merely being short.
-const tailMarker = "QQTRUNCATEDTAILQQ"
-
-// These mirror the middleware's own budgets, which are unexported.
-// They are duplicated rather than exported so that widening a budget
-// in the middleware has to be restated here deliberately.
-const (
-	maxFieldBytes         = 512
-	maxRequestIDBytes     = 128
-	maxMethodBytes        = 32
-	truncationSuffix      = "[truncated]"
-	unmatchedRouteLiteral = "(unmatched)"
-)
-
-// capturingMiddleware returns a Middleware whose logger writes JSON
-// lines into the returned buffer, so the access log can be asserted
-// on directly.
-func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
-	t.Helper()
-
-	buf := new(bytes.Buffer)
-	log := slog.New(slog.NewJSONHandler(
-		buf,
-		&slog.HandlerOptions{Level: slog.LevelInfo},
-	))
-
-	cfg := &config.Config{Environment: config.EnvironmentDev}
-
-	return middleware.NewForTest(log, cfg, nil), buf
-}
-
-// capturingTextMiddleware is capturingMiddleware for the other handler
-// internal/logger can select: slog's text handler, which
-// internal/logger/logger.go installs when stderr is a tty. It escapes
-// differently from the JSON one, so the line bound has to be asserted
-// against both.
-func capturingTextMiddleware(
-	t *testing.T,
-) (*middleware.Middleware, *bytes.Buffer) {
-	t.Helper()
-
-	buf := new(bytes.Buffer)
-	log := slog.New(slog.NewTextHandler(
-		buf,
-		&slog.HandlerOptions{Level: slog.LevelInfo},
-	))
-
-	cfg := &config.Config{Environment: config.EnvironmentDev}
-
-	return middleware.NewForTest(log, cfg, nil), buf
-}
-
-// accessLogRouter mirrors the production route shapes that an
-// unauthenticated client can reach: the public receiver, the
-// authenticated profile route (which redirects to login rather than
-// rejecting outright), the health check (which answers 200 to anyone,
-// behind no rate limiter at all), and a plain static route.
-func accessLogRouter(m *middleware.Middleware) *chi.Mux {
-	router := chi.NewRouter()
-	// Production registers RequestID ahead of Logging, and chi's
-	// RequestID passes an inbound X-Request-Id header straight
-	// through, so the request_id field is client-supplied too.
-	router.Use(chimw.RequestID)
-	router.Use(m.Logging())
-
-	router.Get(
-		"/.well-known/healthcheck",
-		func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusOK)
-		},
-	)
-
-	router.HandleFunc(
-		"/webhook/{uuid}",
-		func(w http.ResponseWriter, r *http.Request) {
-			// 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)
-		},
-	)
-
-	router.Route("/user/{username}", func(r chi.Router) {
-		r.Get("/", func(w http.ResponseWriter, r *http.Request) {
-			http.Redirect(
-				w, r, "/pages/login", http.StatusSeeOther,
-			)
-		})
-	})
-
-	boom := func(w http.ResponseWriter, _ *http.Request) {
-		http.Error(w, "boom", http.StatusInternalServerError)
-	}
-
-	router.Get("/boom", boom)
-	// The 5xx branch keeps the concrete path, so it needs a route that
-	// answers 500 to a path of the client's choosing: that is where the
-	// url field and the header fields are both at their budget on the
-	// same line.
-	router.Get("/boom/*", boom)
-
-	return router
-}
-
-// accessLogEntries decodes the captured buffer into one map per
-// logged line, holding every line to maxLineBytes.
-func accessLogEntries(
-	t *testing.T,
-	buf *bytes.Buffer,
-) []map[string]any {
-	t.Helper()
-
-	return accessLogEntriesWithin(t, buf, maxLineBytes)
-}
-
-// accessLogEntriesWithin decodes the captured buffer into one map per
-// logged line, holding every line to bound bytes.
-func accessLogEntriesWithin(
-	t *testing.T,
-	buf *bytes.Buffer,
-	bound int,
-) []map[string]any {
-	t.Helper()
-
-	var entries []map[string]any
-
-	for line := range strings.SplitSeq(
-		strings.TrimSpace(buf.String()), "\n",
-	) {
-		if line == "" {
-			continue
-		}
-
-		require.LessOrEqual(
-			t, len(line), bound,
-			"access log line exceeded its bound",
-		)
-
-		var entry map[string]any
-
-		require.NoError(t, json.Unmarshal([]byte(line), &entry))
-
-		entries = append(entries, entry)
-	}
-
-	return entries
-}
-
-// get drives one GET through the router.
-func get(t *testing.T, router *chi.Mux, target string) int {
-	t.Helper()
-
-	return getWithHeaders(t, router, target, nil)
-}
-
-// getWithHeaders drives one GET through the router with the supplied
-// request headers set.
-func getWithHeaders(
-	t *testing.T,
-	router *chi.Mux,
-	target string,
-	headers map[string]string,
-) int {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodGet, target, nil,
-	)
-
-	for name, value := range headers {
-		req.Header.Set(name, value)
-	}
-
-	w := httptest.NewRecorder()
-	router.ServeHTTP(w, req)
-
-	return w.Code
-}
-
-// assertFloodIsBounded drives floodRequests distinct invented paths
-// built by pathFor and asserts every logged line names wantURL, that
-// none carries the invented text, and that the line count is exactly
-// one per request.
-func assertFloodIsBounded(
-	t *testing.T,
-	pathFor func(i int) string,
-	wantStatus int,
-	wantURL string,
-) {
-	t.Helper()
-
-	m, buf := capturingMiddleware(t)
-	router := accessLogRouter(m)
-
-	for i := range floodRequests {
-		assert.Equal(t, wantStatus, get(t, router, pathFor(i)))
-	}
-
-	assert.NotContains(
-		t, buf.String(), attackerMarker,
-		"access log carried attacker-chosen path text",
-	)
-
-	entries := accessLogEntries(t, buf)
-	require.Len(t, entries, floodRequests)
-
-	for _, entry := range entries {
-		assert.Equal(t, wantURL, entry["url"])
-		assert.InDelta(
-			t, float64(wantStatus), entry["status"], 0,
-		)
-	}
-}
-
-func TestAccessLog_InventedReceiverPathsLogRoutePattern(t *testing.T) {
-	t.Parallel()
-
-	assertFloodIsBounded(
-		t,
-		func(i int) string {
-			return "/webhook/" + attackerMarker +
-				strings.Repeat("x", i) + "?q=" + attackerMarker
-		},
-		http.StatusNotFound,
-		"/webhook/{uuid}",
-	)
-}
-
-func TestAccessLog_InventedProfilePathsLogRoutePattern(t *testing.T) {
-	t.Parallel()
-
-	// The login redirect is a 3xx, not a 4xx, but it is just as free
-	// for an unauthenticated client to drive with invented input.
-	// The doubled slash is what chi's RoutePattern yields for a
-	// mounted subrouter's index route.
-	assertFloodIsBounded(
-		t,
-		func(i int) string {
-			return "/user/" + attackerMarker +
-				strings.Repeat("x", i) + "/"
-		},
-		http.StatusSeeOther,
-		"/user/{username}//",
-	)
-}
-
-func TestAccessLog_UnroutablePathsLogFixedLiteral(t *testing.T) {
-	t.Parallel()
-
-	assertFloodIsBounded(
-		t,
-		func(i int) string {
-			return "/" + attackerMarker + strings.Repeat("x", i)
-		},
-		http.StatusNotFound,
-		"(unmatched)",
-	)
-}
-
-// oversizedValue builds an 8 KB header value out of repetitions of ch,
-// with the tail marker at its end.
-//
-// The leading 'x' is load-bearing for tab: net/textproto strips leading
-// and trailing whitespace from a header value, so a value that were
-// nothing but tabs would arrive empty over a real connection and the
-// case would prove nothing.
-func oversizedValue(ch string) string {
-	return "x" + strings.Repeat(ch, oversizedSegmentBytes) + tailMarker
-}
-
-// oversizedHeaders fills every client-supplied header the access log
-// reads with the same value.
-func oversizedHeaders(value string) map[string]string {
-	return map[string]string{
-		"User-Agent":   value,
-		"Referer":      value,
-		"X-Request-Id": value,
-	}
-}
-
-// sizeCase is one way of pointing 8 KB of client-chosen text at the
-// access log.
-type sizeCase struct {
-	target     string
-	headers    map[string]string
-	wantStatus int
-	wantURL    string
-	bound      int
-}
-
-// lineSizeCases enumerates every part of a request that reaches the
-// access log, at 8 KB apiece.
-func lineSizeCases() map[string]sizeCase {
-	cases := map[string]sizeCase{
-		"oversized path segment": {
-			target: "/webhook/" + attackerMarker +
-				strings.Repeat("x", oversizedSegmentBytes),
-			wantStatus: http.StatusNotFound,
-			wantURL:    "/webhook/{uuid}",
-			bound:      maxLineBytes,
-		},
-		// /.well-known/healthcheck answers 200 to anyone and has no
-		// rate limiter in front of it, so an oversized query appended
-		// to it would otherwise buy the same amplification as an
-		// invented 404 path, unauthenticated and unthrottled.
-		"oversized query on an unauthenticated 200": {
-			target: "/.well-known/healthcheck?q=" + attackerMarker +
-				strings.Repeat("x", oversizedSegmentBytes),
-			wantStatus: http.StatusOK,
-			wantURL:    "/.well-known/healthcheck?(redacted)",
-			bound:      maxLineBytes,
-		},
-		// These reach the line on every request, including one whose
-		// url field is correctly redacted.
-		"oversized headers": {
-			target:     "/" + attackerMarker,
-			headers:    oversizedHeaders(oversizedValue("h")),
-			wantStatus: http.StatusNotFound,
-			wantURL:    unmatchedRouteLiteral,
-			bound:      maxCappedLineBytes,
-		},
-	}
-
-	// The url field on a 5xx keeps the concrete path, so it reaches its
-	// own budget on the same line as the three header fields. That is
-	// the widest line the service can be made to write.
-	longPath := "/boom/" + strings.Repeat("x", oversizedSegmentBytes)
-	wantLongURL := longPath[:maxFieldBytes] + truncationSuffix
-
-	// escapeChars are the runes Go's header parser accepts in a header
-	// value and the log handler then escapes, coming out wider than
-	// they went in. A budget counted in raw bytes lets any of them buy
-	// a field several times its nominal size, so every one of them
-	// gets a case.
-	//
-	// The astral one is the case the JSON handler alone does not
-	// reach: U+1000C is unassigned, so it is non-printable, and
-	// strconv.Quote spells a non-printable rune at or above U+10000
-	// as a ten-byte \UXXXXXXXX. The JSON handler passes it through as
-	// its four UTF-8 bytes, so only the text-handler shape of this
-	// test holds the ten-byte charge honest.
-	escapeChars := map[string]string{
-		"quote":     `"`,
-		"backslash": `\`,
-		"tab":       "\t",
-		"astral":    "\U0001000C",
-	}
-
-	for kind, char := range escapeChars {
-		fill := oversizedValue(char)
-
-		cases["oversized "+kind+" headers"] = sizeCase{
-			target:     "/" + attackerMarker,
-			headers:    oversizedHeaders(fill),
-			wantStatus: http.StatusNotFound,
-			wantURL:    unmatchedRouteLiteral,
-			bound:      maxCappedLineBytes,
-		}
-
-		cases["oversized "+kind+" headers with a 5xx concrete url"] =
-			sizeCase{
-				target:     longPath,
-				headers:    oversizedHeaders(fill),
-				wantStatus: http.StatusInternalServerError,
-				wantURL:    wantLongURL,
-				bound:      maxCappedLineBytes,
-			}
-	}
-
-	return cases
-}
-
-// TestAccessLog_LineSizeDoesNotTrackInputSize drives 8 KB of
-// client-chosen text at the access log through each part of the
-// request that reaches it, and holds the resulting line to a fixed
-// bound in every case.
-//
-// The bound is on the ENCODED line, so the cases built out of
-// characters the handler escapes are the ones that matter: a budget
-// spent in raw bytes passes every plain-ASCII case here and still
-// writes a line half again as long as the stated ceiling.
-func TestAccessLog_LineSizeDoesNotTrackInputSize(t *testing.T) {
-	t.Parallel()
-
-	require.Equal(
-		t, middleware.MaxAccessLogLineBytes, maxCappedLineBytes,
-		"the README quotes this ceiling and the middleware derives "+
-			"it; they have to agree",
-	)
-
-	for name, tc := range lineSizeCases() {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			m, buf := capturingMiddleware(t)
-			router := accessLogRouter(m)
-
-			assert.Equal(
-				t,
-				tc.wantStatus,
-				getWithHeaders(t, router, tc.target, tc.headers),
-			)
-
-			// accessLogEntriesWithin enforces the bound, which is
-			// orders of magnitude smaller than the input just sent.
-			entries := accessLogEntriesWithin(t, buf, tc.bound)
-			require.Len(t, entries, 1)
-			assert.Equal(t, tc.wantURL, entries[0]["url"])
-
-			// The markers sit at the far end of the client-chosen
-			// text, so their absence is what proves the redaction and
-			// the truncation actually ran.
-			assert.NotContains(
-				t, buf.String(), attackerMarker,
-				"access log carried attacker-chosen text",
-			)
-			assert.NotContains(
-				t, buf.String(), tailMarker,
-				"access log carried an untruncated client field",
-			)
-		})
-	}
-}
-
-// TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler runs the
-// same cases through slog's text handler, which internal/logger
-// selects on a tty.
-//
-// MaxAccessLogLineBytes is quoted to operators unqualified, so it has
-// to hold for whichever handler is installed — and the two do not
-// escape alike. The astral case is the one that separates them: the
-// JSON handler emits U+1000C as its four UTF-8 bytes, while
-// strconv.Quote spells it \U0001000C at ten. Charging six for it, as
-// this code did, put a real 2,676-byte line on the wire here while
-// every JSON case stayed comfortably inside the bound.
-//
-// Only the size bound is asserted; the url field's contents are the
-// JSON shape's business above.
-func TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	for name, tc := range lineSizeCases() {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			m, buf := capturingTextMiddleware(t)
-			router := accessLogRouter(m)
-
-			assert.Equal(
-				t,
-				tc.wantStatus,
-				getWithHeaders(t, router, tc.target, tc.headers),
-			)
-
-			line := strings.TrimSpace(buf.String())
-
-			require.NotEmpty(t, line)
-			assert.NotContains(
-				t, line, "\n", "expected exactly one log line",
-			)
-			require.LessOrEqual(
-				t, len(line), tc.bound,
-				"access log line exceeded its bound",
-			)
-			assert.Contains(t, line, "url=")
-			assert.NotContains(
-				t, line, attackerMarker,
-				"access log carried attacker-chosen text",
-			)
-			assert.NotContains(
-				t, line, tailMarker,
-				"access log carried an untruncated client field",
-			)
-		})
-	}
-}
-
-// TestAccessLog_OversizedMethodIsTruncated covers the last term in the
-// MaxAccessLogLineBytes arithmetic that the size cases above cannot
-// reach: Go accepts any RFC 7230 token as a method, and getWithHeaders
-// only ever sends GET.
-func TestAccessLog_OversizedMethodIsTruncated(t *testing.T) {
-	t.Parallel()
-
-	m, buf := capturingMiddleware(t)
-	router := accessLogRouter(m)
-
-	method := strings.Repeat("M", oversizedSegmentBytes) + attackerMarker
-	req := httptest.NewRequestWithContext(
-		context.Background(), method, "/"+attackerMarker, nil,
-	)
-	rec := httptest.NewRecorder()
-	router.ServeHTTP(rec, req)
-
-	entries := accessLogEntriesWithin(t, buf, maxLineBytes)
-	require.Len(t, entries, 1)
-	assert.Equal(
-		t,
-		strings.Repeat("M", maxMethodBytes)+truncationSuffix,
-		entries[0]["method"],
-	)
-	assert.NotContains(
-		t, buf.String(), attackerMarker,
-		"access log carried attacker-chosen text",
-	)
-}
-
-// TestAccessLog_OversizedHeadersKeepATruncatedPrefix checks the other
-// half of the header cap: the fields are cut, not dropped, so a
-// truncated User-Agent is still worth reading.
-func TestAccessLog_OversizedHeadersKeepATruncatedPrefix(t *testing.T) {
-	t.Parallel()
-
-	m, buf := capturingMiddleware(t)
-	router := accessLogRouter(m)
-
-	assert.Equal(
-		t,
-		http.StatusNotFound,
-		getWithHeaders(
-			t, router, "/nope",
-			oversizedHeaders(oversizedValue("h")),
-		),
-	)
-
-	entries := accessLogEntriesWithin(t, buf, maxCappedLineBytes)
-	require.Len(t, entries, 1)
-
-	for key, budget := range map[string]int{
-		"useragent":  maxFieldBytes,
-		"referer":    maxFieldBytes,
-		"request_id": maxRequestIDBytes,
-	} {
-		value, ok := entries[0][key].(string)
-		require.True(t, ok, key)
-		assert.LessOrEqual(
-			t, len(value), budget+len(truncationSuffix), key,
-		)
-		assert.Contains(t, value, truncationSuffix, key)
-		assert.Contains(t, value, "hhhh", key)
-	}
-}
-
-func TestAccessLog_SuccessKeepsConcretePathAndRedactsQuery(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	m, buf := capturingMiddleware(t)
-	router := accessLogRouter(m)
-
-	assert.Equal(
-		t, http.StatusOK, get(t, router, "/webhook/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, "/webhook/known?(redacted)", entries[0]["url"])
-	assert.NotContains(t, buf.String(), "src=ci")
-}
-
-func TestAccessLog_ServerErrorKeepsConcreteURL(t *testing.T) {
-	t.Parallel()
-
-	m, buf := capturingMiddleware(t)
-	router := accessLogRouter(m)
-
-	assert.Equal(
-		t, http.StatusInternalServerError, get(t, router, "/boom"),
-	)
-
-	entries := accessLogEntries(t, buf)
-	require.Len(t, entries, 1)
-	assert.Equal(t, "/boom", entries[0]["url"])
-}
-
-func TestAccessLog_RetainsEveryOtherField(t *testing.T) {
-	t.Parallel()
-
-	m, buf := capturingMiddleware(t)
-	router := accessLogRouter(m)
-
-	assert.Equal(
-		t,
-		http.StatusNotFound,
-		get(t, router, "/webhook/"+attackerMarker),
-	)
-
-	entries := accessLogEntries(t, buf)
-	require.Len(t, entries, 1)
-
-	for _, key := range []string{
-		"request_start", "method", "url", "useragent", "request_id",
-		"referer", "proto", "remoteIP", "status", "latency_ms",
-	} {
-		assert.Contains(t, entries[0], key)
-	}
-
-	assert.Equal(t, http.MethodGet, entries[0]["method"])
-	assert.Equal(t, "HTTP/1.1", entries[0]["proto"])
-}

internal/middleware/export_test.go

@@ -50,10 +50,6 @@ const LoginFailureMaxKeysConst = loginFailureMaxKeys
 // Argon2id verifications.
 const PasswordVerifyConcurrencyConst = passwordVerifyConcurrency
 
-// PasswordVerifyMaxWaitersConst exposes the bound on how many
-// requests may queue for a verification slot.
-const PasswordVerifyMaxWaitersConst = passwordVerifyMaxWaiters
-
 // LoginGuard is the login failure counter and verification
 // semaphore, exposed for direct testing.
 type LoginGuard = loginGuard
@@ -62,20 +58,14 @@ type LoginGuard = loginGuard
 func NewLoginGuardForTest(
 	limit int,
 	interval time.Duration,
-	maxKeys, concurrency, maxWaiters int,
+	maxKeys, concurrency int,
 	wait time.Duration,
 ) *LoginGuard {
 	return newLoginGuard(
-		limit, interval, maxKeys, concurrency, maxWaiters, wait,
+		limit, interval, maxKeys, concurrency, wait,
 	)
 }
 
-// QueuedWaitersForTest reports how many requests are currently
-// queued for a verification slot.
-func (g *LoginGuard) QueuedWaitersForTest() int {
-	return len(g.queue)
-}
-
 // SetNowForTest replaces the guard's clock.
 func (g *LoginGuard) SetNowForTest(now func() time.Time) {
 	g.mu.Lock()

internal/middleware/loginguard.go

@@ -44,32 +44,6 @@ const (
 	// whole. The wait is well inside the 60s request timeout.
 	passwordVerifyWait = 5 * time.Second
 
-	// passwordVerifyMaxWaiters bounds how many requests may be
-	// queued for a slot at once. Past it, acquire sheds immediately
-	// with 503 instead of joining the queue.
-	//
-	// The wait bounds how long one request occupies memory; this
-	// bounds how many do so at the same time, and without it the
-	// 128 MB hashing budget above is the smaller half of the real
-	// footprint. A waiter is not free: by the time it reaches the
-	// guard its form is parsed, so it holds up to maxFormBodySize —
-	// 1 MB — for as long as it waits. At the 400 req/s a saturation
-	// attack can offer, an unbounded queue would hold ~2000 of those
-	// for the full five seconds, which is gigabytes.
-	//
-	// Arithmetic: 1 MB a waiter, and the memory committed to the
-	// queue is 64 MB, so 64 waiters. Cross-check against the
-	// deadline: two slots at the ~27 verifications/s measured on a
-	// review host (with the race detector on, so the real rate is
-	// higher) drain a full 64-deep queue in about 2.4 s, inside
-	// passwordVerifyWait. Queueing deeper would buy memory rather
-	// than throughput, because the extra waiters could not be served
-	// before their deadline anyway.
-	//
-	// Peak commitment is therefore 128 MB of Argon2id plus at most
-	// 66 MB of parsed forms — 64 queued and the 2 being hashed.
-	passwordVerifyMaxWaiters = 64
-
 	// failureKeyHashBytes is how much of the username digest goes
 	// into a failure key. 64 bits over at most loginFailureMaxKeys
 	// live keys makes a collision negligible, and a collision would
@@ -117,12 +91,6 @@ type loginGuard struct {
 
 	slots chan struct{}
 
-	// queue holds one token per request waiting for a slot. A token
-	// is taken non-blockingly, so a request that finds it full is
-	// shed rather than queued, and is given up as soon as the wait
-	// ends however it ends.
-	queue chan struct{}
-
 	limit    int
 	interval time.Duration
 	maxKeys  int
@@ -133,19 +101,17 @@ type loginGuard struct {
 }
 
 // newLoginGuard builds a guard with the given failure limit per
-// interval, key-set cap, verification concurrency, queue depth and
-// slot wait.
+// interval, key-set cap, verification concurrency and slot wait.
 func newLoginGuard(
 	limit int,
 	interval time.Duration,
-	maxKeys, concurrency, maxWaiters int,
+	maxKeys, concurrency int,
 	wait time.Duration,
 ) *loginGuard {
 	return &loginGuard{
 		byUser:   make(map[string]*failureWindow),
 		byAddr:   make(map[string]*failureWindow),
 		slots:    make(chan struct{}, concurrency),
-		queue:    make(chan struct{}, maxWaiters),
 		limit:    limit,
 		interval: interval,
 		maxKeys:  maxKeys,
@@ -155,33 +121,14 @@ func newLoginGuard(
 }
 
 // acquire reserves a verification slot, waiting up to the guard's
-// wait for one. It reports false when the queue of waiters is
-// already full, when no slot became available in time, or when the
+// wait for one. It reports false when none became available or the
 // request was cancelled first; the caller must then answer 503
 // without verifying anything. The returned function releases the
 // slot and must be called exactly once.
 func (g *loginGuard) acquire(ctx context.Context) (func(), bool) {
-	// Shedding past the queue depth is what keeps waiting memory
-	// bounded; the wait alone only bounds how long one waiter holds
-	// its parsed form, not how many hold one at once.
-	select {
-	case g.queue <- struct{}{}:
-	default:
-		return nil, false
-	}
-
-	// Held only for the wait. A request that gets a slot gives its
-	// queue token back before it starts hashing, so the depth is a
-	// bound on waiters rather than on requests in the handler.
-	defer func() { <-g.queue }()
-
 	timer := time.NewTimer(g.wait)
 	defer timer.Stop()
 
-	// The blocking send is deliberate: a receive on a full buffered
-	// channel hands the slot straight to the head of the send queue,
-	// so slots go out in arrival order and a later arrival cannot
-	// barge past a request already waiting.
 	select {
 	case g.slots <- struct{}{}:
 		return func() { <-g.slots }, true
@@ -295,7 +242,6 @@ func (m *Middleware) guard() *loginGuard {
 			loginRateInterval,
 			loginFailureMaxKeys,
 			passwordVerifyConcurrency,
-			passwordVerifyMaxWaiters,
 			passwordVerifyWait,
 		)
 	})
@@ -304,11 +250,10 @@ func (m *Middleware) guard() *loginGuard {
 }
 
 // BeginPasswordVerification reserves one of the bounded Argon2id
-// verification slots. It reports false when the queue of waiting
-// requests is already at passwordVerifyMaxWaiters, or when no slot
-// became free within passwordVerifyWait; in either case the caller
-// must answer 503 and must not verify a password. The returned
-// function releases the slot and must be called exactly once.
+// verification slots. It reports false when none became free within
+// passwordVerifyWait, in which case the caller must answer 503 and
+// must not verify a password. The returned function releases the
+// slot and must be called exactly once.
 //
 // Every endpoint that hashes a password on request must go through
 // this, or the bound has a hole: the memory is committed per hash,

internal/middleware/loginguard_test.go

@@ -10,13 +10,9 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/middleware"
 )
 
-// mib converts the Argon2id memory parameter, which is in KiB, to MB.
-const mib = 1024
-
 const (
 	// guardInterval is the failure window these tests use. It is
 	// long enough that nothing lapses mid-test on its own; tests
@@ -39,7 +35,6 @@ func newGuard(maxKeys, concurrency int) *middleware.LoginGuard {
 		guardInterval,
 		maxKeys,
 		concurrency,
-		middleware.PasswordVerifyMaxWaitersConst,
 		guardWait,
 	)
 }
@@ -288,7 +283,6 @@ func TestLoginGuard_SaturatedSemaphoreRefusesRatherThanQueueing(
 		guardInterval,
 		middleware.LoginFailureMaxKeysConst,
 		1,
-		middleware.PasswordVerifyMaxWaitersConst,
 		10*time.Millisecond,
 	)
 
@@ -335,175 +329,24 @@ func TestLoginGuard_AcquireHonoursCancellation(t *testing.T) {
 }
 
 // TestPasswordVerifyConcurrency_MatchesMemoryBudget pins the
-// concurrency constant to the arithmetic behind it: the number of
-// slots is the hashing budget divided by what one Argon2id hash
-// actually costs.
-//
-// The per-hash figure is read out of the shipped password
-// parameters rather than copied here. A guard that asserts a literal
-// against a literal cannot see the thing it guards: raising
-// argon2Memory would leave it green while the real ceiling doubled.
+// concurrency constant to the arithmetic behind it: Argon2id here is
+// 64 MB per hash, so the number of slots is the number of 64 MB
+// allocations the process is willing to commit to password hashing.
+// Raising it raises peak resident memory by 64 MB a slot.
 func TestPasswordVerifyConcurrency_MatchesMemoryBudget(t *testing.T) {
 	t.Parallel()
 
-	// Memory is the real argon2Memory, in KiB.
-	perHashMB := int(database.DefaultPasswordConfig().Memory) / mib
-
-	require.Positive(
-		t, perHashMB,
-		"the Argon2id memory parameter must be readable in MB",
-	)
-
-	// The memory this service commits to password hashing.
-	const budgetMB = 128
-
-	assert.Equal(
-		t,
-		middleware.PasswordVerifyConcurrencyConst,
-		budgetMB/perHashMB,
-		"the verification concurrency must be the %d MB hashing "+
-			"budget divided by the %d MB one Argon2id hash costs; "+
-			"if the Argon2id parameters changed, the slot count "+
-			"must change with them",
-		budgetMB, perHashMB,
-	)
-}
-
-// TestLoginGuard_ShedsPastTheQueueCap pins the memory bound on
-// waiting, as distinct from the bound on hashing. A waiter arrives
-// with its form already parsed, so an unbounded queue would hold up
-// to maxFormBodySize per waiting request for the whole wait; past
-// the cap the guard must refuse instantly rather than grow.
-func TestLoginGuard_ShedsPastTheQueueCap(t *testing.T) {
-	t.Parallel()
-
 	const (
-		maxWaiters = 2
-
-		// Long enough that a queued waiter never times out on its
-		// own, so anything the test observes leaving the queue left
-		// because it was shed.
-		neverElapses = time.Minute
-
-		// The probe carries its own deadline, so a guard that queues
-		// the probe instead of shedding it fails on the elapsed time
-		// rather than hanging until the package test timeout.
-		probeWait = 200 * time.Millisecond
-
-		// Shedding takes no measurable time; queueing takes the whole
-		// probeWait. Anything under half of it is unambiguous.
-		shedFast = probeWait / 2
+		argon2MemoryMB = 64
+		budgetMB       = 128
 	)
 
-	g := middleware.NewLoginGuardForTest(
-		middleware.LoginRateLimitConst,
-		guardInterval,
-		middleware.LoginFailureMaxKeysConst,
-		1,
-		maxWaiters,
-		neverElapses,
-	)
-
-	// Occupy the only slot, so everything after this queues.
-	release, ok := g.AcquireForTest(context.Background())
-	require.True(t, ok)
-
-	defer release()
-	defer fillQueue(t, g, maxWaiters)()
-
-	got := probeQueueCap(g, probeWait)
-
-	require.NotNil(
-		t, got,
-		"a request arriving past the queue cap is still waiting to "+
-			"be queued; it must have been shed",
-	)
-	assert.False(
-		t, got.ok,
-		"a request arriving past the queue cap must be shed",
-	)
-	assert.Less(
-		t, got.elapsed, shedFast,
-		"shedding must be immediate; waiting for a place in the "+
-			"queue is the memory growth this bounds",
-	)
 	assert.Equal(
-		t, maxWaiters, g.QueuedWaitersForTest(),
-		"a shed request must not have grown the queue",
-	)
-}
-
-// fillQueue starts n waiters on g and returns once all of them are
-// queued for a slot. The returned function releases them and waits
-// for them to exit.
-func fillQueue(
-	t *testing.T,
-	g *middleware.LoginGuard,
-	n int,
-) func() {
-	t.Helper()
-
-	ctx, cancel := context.WithCancel(context.Background())
-
-	var wg sync.WaitGroup
-
-	for range n {
-		wg.Go(func() {
-			done, got := g.AcquireForTest(ctx)
-			if got {
-				done()
-			}
-		})
-	}
-
-	require.Eventually(
 		t,
-		func() bool { return g.QueuedWaitersForTest() == n },
-		time.Second, time.Millisecond,
-		"the waiters must reach the queue before the cap is tested",
+		budgetMB/argon2MemoryMB,
+		middleware.PasswordVerifyConcurrencyConst,
+		"the verification concurrency is %d MB of Argon2id memory "+
+			"divided by %d MB per hash",
+		budgetMB, argon2MemoryMB,
 	)
-
-	return func() {
-		cancel()
-		wg.Wait()
-	}
-}
-
-// probeResult is what the queue-cap probe reports: whether it got a
-// slot, and how long it took to find out.
-type probeResult struct {
-	ok      bool
-	elapsed time.Duration
-}
-
-// probeQueueCap acquires from another goroutine and reports the
-// result, or nil if the call was still blocked after wait.
-//
-// It runs off the test goroutine deliberately. Joining a full queue
-// is not cancellable by context — refusing to join is the property
-// under test — so a guard that fails this would otherwise hang the
-// package until the test timeout instead of failing here.
-func probeQueueCap(
-	g *middleware.LoginGuard,
-	wait time.Duration,
-) *probeResult {
-	probed := make(chan probeResult, 1)
-
-	go func() {
-		start := time.Now()
-
-		release, ok := g.AcquireForTest(context.Background())
-		if ok {
-			release()
-		}
-
-		probed <- probeResult{ok: ok, elapsed: time.Since(start)}
-	}()
-
-	select {
-	case result := <-probed:
-		return &result
-	case <-time.After(wait):
-		return nil
-	}
 }

internal/middleware/middleware.go

@@ -6,14 +6,10 @@ import (
 	"log/slog"
 	"net"
 	"net/http"
-	"strings"
 	"sync"
 	"time"
-	"unicode"
-	"unicode/utf8"
 
 	basicauth "github.com/99designs/basicauth-go"
-	"github.com/go-chi/chi"
 	"github.com/go-chi/chi/middleware"
 	"github.com/go-chi/cors"
 	metrics "github.com/slok/go-http-metrics/metrics/prometheus"
@@ -30,75 +26,6 @@ const (
 	// corsMaxAge is the maximum time (in seconds) that a
 	// preflight response can be cached.
 	corsMaxAge = 300
-
-	// unmatchedRoute is logged in the access log's url field when a
-	// redirected or rejected request matched no route pattern at
-	// all. Every byte of such a path is client-chosen, so none of it
-	// is logged.
-	unmatchedRoute = "(unmatched)"
-
-	// redactedQuery stands in for the query string on the access log
-	// branches that keep the concrete URL. The query is client-chosen
-	// on every route, including the ones that answer an
-	// unauthenticated 200, so logging it verbatim would let a client
-	// pick the size of the line it writes.
-	redactedQuery = "?(redacted)"
-
-	// maxLogFieldBytes bounds each access log field whose value the
-	// client supplies outright: the URL, the User-Agent and the
-	// Referer. The budget is spent in ENCODED bytes (see
-	// truncateLogField), so 512 still holds a real browser's User-Agent
-	// whole — those are plain ASCII, which encodes one byte for one —
-	// while a value built from characters the encoder escapes keeps a
-	// shorter prefix. That is the intended trade: 500 quotation marks
-	// are not a debugging asset.
-	maxLogFieldBytes = 512
-
-	// maxLogRequestIDBytes bounds the request id, which is also
-	// client-supplied: chi's RequestID middleware passes an inbound
-	// X-Request-Id header through verbatim. Its generated form is an
-	// order of magnitude shorter than this.
-	maxLogRequestIDBytes = 128
-
-	// maxLogMethodBytes bounds the method. Go accepts any RFC 7230
-	// token there, bounded only by the header size limit, so it is
-	// client-chosen text like the rest. The longest registered method
-	// is half this.
-	maxLogMethodBytes = 32
-
-	// truncationMarker is appended to any field the access log cut, so
-	// a short value and a truncated one cannot be confused. It is
-	// charged on top of the budget, not inside it.
-	truncationMarker = "[truncated]"
-
-	// MaxAccessLogLineBytes is the ceiling on one JSON access log line,
-	// and the number an operator multiplies by the request rate to size
-	// log storage. It is not an observation of a sample: it is the sum
-	// of the budgets above, each of which truncateLogField enforces in
-	// ENCODED bytes, plus the part of the line no client can influence.
-	//
-	//	url, useragent, referer   3*(512+11) = 1569
-	//	request_id                    128+11 =  139
-	//	method                         32+11 =   43
-	//	fixed portion                        =  336
-	//	                                        ----
-	//	                                        2087
-	//
-	// The fixed portion is the JSON punctuation, the field names, the
-	// level and the message, both timestamps at their longest, an IPv6
-	// remoteIP with a zone, a three-digit status and a full-width int64
-	// latency. Stated at 2560 so the figure carries headroom rather
-	// than sitting on the arithmetic.
-	//
-	// The tty text handler in internal/logger is covered by the same
-	// figure. encodedLogFieldBytes charges every rune at least what
-	// the wider of the two handlers emits for it — including the ten
-	// bytes strconv.Quote spends on a non-printable rune at or above
-	// U+10000, which is four more than the JSON handler ever spends —
-	// so each budget bounds the encoded field under either handler.
-	// The text handler's fixed portion is 286, the smaller of the two,
-	// which puts its worst case at 2037.
-	MaxAccessLogLineBytes = 2560
 )
 
 //nolint:revive // MiddlewareParams is a standard fx naming convention.
@@ -174,178 +101,6 @@ func (lrw *loggingResponseWriter) WriteHeader(code int) {
 	lrw.ResponseWriter.WriteHeader(code)
 }
 
-// encodedLogFieldBytes is what r costs on the line once the log
-// handler has escaped it, taking the worse of the two handlers
-// internal/logger configures.
-//
-// slog's JSON handler escapes quote, backslash, newline, carriage
-// return and tab to two bytes each, and every other C0 control plus
-// LINE SEPARATOR and PARAGRAPH SEPARATOR to a six-byte \u escape; it
-// passes every other rune through as its own UTF-8. Its text handler
-// quotes with strconv.Quote, which spells a non-printable rune below
-// U+10000 as \uXXXX but one at or above U+10000 as \UXXXXXXXX — ten
-// bytes, not six. The text handler is therefore the worse of the two
-// for every non-printable rune, and by four bytes apiece for the
-// 955,086 unassigned, private-use and format code points on planes 1
-// to 16.
-//
-// Charging ten there is what makes MaxAccessLogLineBytes hold for the
-// tty handler as well: U+1000C encodes as F0 90 80 8C, every byte
-// >= 0x80, which httpguts.ValidHeaderFieldValue accepts and
-// net/textproto does not strip, so a header can be filled with them.
-//
-// Both handlers pass printable runes through as their own UTF-8, so
-// unicode.IsPrint separates the escaped cases from the plain ones for
-// either handler.
-func encodedLogFieldBytes(r rune) int {
-	const (
-		// A backslash and the character itself.
-		shortEscapeBytes = 2
-		// \uXXXX, which is also the width of \u00XX.
-		escapedRuneBytes = 6
-		// \UXXXXXXXX, strconv.Quote's spelling of a non-printable
-		// rune outside the basic multilingual plane.
-		escapedAstralRuneBytes = 10
-		// The first code point strconv.Quote spells with \U.
-		firstAstralRune = 0x10000
-	)
-
-	switch {
-	case r == '"' || r == '\\' || r == '\n' || r == '\r' || r == '\t':
-		return shortEscapeBytes
-	case !unicode.IsPrint(r) && r >= firstAstralRune:
-		return escapedAstralRuneBytes
-	case !unicode.IsPrint(r):
-		return escapedRuneBytes
-	default:
-		return utf8.RuneLen(r)
-	}
-}
-
-// truncateLogField caps s at maxBytes of ENCODED output, marking the
-// value when it cuts.
-//
-// Budgeting raw bytes would not bound the line. Escaping only ever
-// grows a value, so a raw budget spent on characters the encoder
-// escapes buys a field several times its nominal size — and the line
-// is the thing an operator is told to multiply by their request rate.
-// Charging each rune what it will actually cost is what makes
-// MaxAccessLogLineBytes true rather than merely larger. The visible
-// consequence is that an escape-heavy value keeps a shorter prefix
-// than a plain one, which is the correct trade.
-//
-// The result is always valid UTF-8. A cut on a byte boundary can split
-// a multi-byte rune, and a header can carry bytes that were never
-// valid UTF-8 to begin with; both are dropped rather than kept, since
-// an encoder would otherwise spend six bytes replacing each one.
-func truncateLogField(s string, maxBytes int) string {
-	// No rune encodes to fewer bytes than it occupies, so nothing past
-	// maxBytes raw can fit the budget. Slicing first bounds the scan
-	// below to the budget rather than to the size of the header the
-	// client sent.
-	window, cut := s, false
-	if len(window) > maxBytes {
-		window, cut = window[:maxBytes], true
-	}
-
-	var (
-		kept  strings.Builder
-		spent int
-	)
-
-	for i := 0; i < len(window); {
-		r, size := utf8.DecodeRuneInString(window[i:])
-		if r == utf8.RuneError && size == 1 {
-			i += size
-
-			continue
-		}
-
-		cost := encodedLogFieldBytes(r)
-		if spent+cost > maxBytes {
-			cut = true
-
-			break
-		}
-
-		spent += cost
-
-		kept.WriteString(window[i : i+size])
-
-		i += size
-	}
-
-	if !cut {
-		return kept.String()
-	}
-
-	return kept.String() + truncationMarker
-}
-
-// concreteLogURL renders the request's own URL for the access log
-// branches that keep it, with the query string replaced by a fixed
-// marker.
-//
-// The path on those branches is bounded by the service's routes or by
-// 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. One route in the service reads a query
-// parameter at all — `page`, on the authenticated pagination links in
-// internal/handlers/source_management.go — and the alternatives that
-// would preserve more (a key count, a key allowlist) all require
-// parsing an attacker-sized query on every request, which is work an
-// unauthenticated client would then be choosing for us.
-func concreteLogURL(r *http.Request) string {
-	path := r.URL.EscapedPath()
-
-	if r.URL.RawQuery == "" && !r.URL.ForceQuery {
-		return path
-	}
-
-	return path + redactedQuery
-}
-
-// accessLogURL returns the value for the access log's url field.
-//
-// 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: 404 or 429
-// on any invented /webhook/ 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 {
-	if status < http.StatusMultipleChoices ||
-		status >= http.StatusInternalServerError {
-		return concreteLogURL(r)
-	}
-
-	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
 // timing and metadata.
 func (s *Middleware) Logging() func(http.Handler) http.Handler {
@@ -370,27 +125,13 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
 					}
 				}
 
-				// Every field below that a client can influence is
-				// truncated to a fixed budget, so the size of this
-				// line does not track the size of the request.
 				s.log.Info("http request",
 					"request_start", start,
-					"method", truncateLogField(
-						r.Method, maxLogMethodBytes,
-					),
-					"url", truncateLogField(
-						accessLogURL(r, lrw.statusCode),
-						maxLogFieldBytes,
-					),
-					"useragent", truncateLogField(
-						r.UserAgent(), maxLogFieldBytes,
-					),
-					"request_id", truncateLogField(
-						requestID, maxLogRequestIDBytes,
-					),
-					"referer", truncateLogField(
-						r.Referer(), maxLogFieldBytes,
-					),
+					"method", r.Method,
+					"url", r.URL.String(),
+					"useragent", r.UserAgent(),
+					"request_id", requestID,
+					"referer", r.Referer(),
 					"proto", r.Proto,
 					"remoteIP", ipFromHostPort(r.RemoteAddr),
 					"status", lrw.statusCode,

internal/server/routes.go

@@ -149,15 +149,6 @@ func (s *Server) setupSourceRoutes() {
 		r.Post("/edit", s.h.HandleSourceEditSubmit())
 		r.Post("/delete", s.h.HandleSourceDelete())
 		r.Get("/logs", s.h.HandleSourceLogs())
-		// The log page renders each body only up to its cap, so
-		// this is the only route that serves a whole one. It
-		// belongs to this group for its RequireAuth and
-		// NoCache; see HandleEventBodyDownload for the headers
-		// that keep the bytes it returns inert.
-		r.Get(
-			"/logs/{eventID}/body",
-			s.h.HandleEventBodyDownload(),
-		)
 		r.Post(
 			"/entrypoints",
 			s.h.HandleEntrypointCreate(),

internal/server/routes_test.go

@@ -7,7 +7,6 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"regexp"
-	"strconv"
 	"strings"
 	"testing"
 
@@ -15,7 +14,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/fx"
 	"go.uber.org/fx/fxtest"
-	"gorm.io/gorm/clause"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/delivery"
@@ -51,7 +49,6 @@ type testEnv struct {
 	router http.Handler
 	sess   *session.Session
 	db     *database.Database
-	dbMgr  *database.WebhookDBManager
 }
 
 // newTestEnv wires the dependency graph with fx and builds the
@@ -67,7 +64,6 @@ func newTestEnv(t *testing.T) *testEnv {
 		hnd  *handlers.Handlers
 		sess *session.Session
 		db   *database.Database
-		dbMgr *database.WebhookDBManager
 	)
 
 	app := fxtest.New(
@@ -90,7 +86,7 @@ func newTestEnv(t *testing.T) *testEnv {
 			middleware.New,
 			handlers.New,
 		),
-		fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db, &dbMgr),
+		fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db),
 	)
 	app.RequireStart()
 	t.Cleanup(app.RequireStop)
@@ -99,7 +95,6 @@ func newTestEnv(t *testing.T) *testEnv {
 		router: server.NewRouterForTest(log.Get(), cfg, mw, hnd),
 		sess:   sess,
 		db:     db,
-		dbMgr:  dbMgr,
 	}
 }
 
@@ -238,49 +233,6 @@ func (e *testEnv) seedUser(
 	return user.ID, hash
 }
 
-// seedWebhook creates a webhook owned by the given user.
-func (e *testEnv) seedWebhook(
-	t *testing.T,
-	userID string,
-) *database.Webhook {
-	t.Helper()
-
-	wh := &database.Webhook{UserID: userID, Name: "routed"}
-
-	require.NoError(
-		t,
-		e.db.DB().Omit(clause.Associations).Create(wh).Error,
-	)
-
-	return wh
-}
-
-// seedEvent records one event with the given body in a webhook's
-// own database.
-func (e *testEnv) seedEvent(
-	t *testing.T,
-	webhookID, body string,
-) *database.Event {
-	t.Helper()
-
-	webhookDB, err := e.dbMgr.GetDB(webhookID)
-	require.NoError(t, err)
-
-	event := &database.Event{
-		WebhookID:   webhookID,
-		Method:      http.MethodPost,
-		Body:        body,
-		ContentType: "application/octet-stream",
-	}
-
-	require.NoError(
-		t,
-		webhookDB.Omit(clause.Associations).Create(event).Error,
-	)
-
-	return event
-}
-
 // storedHash reads the current password hash for a username.
 func (e *testEnv) storedHash(t *testing.T, username string) string {
 	t.Helper()
@@ -420,78 +372,6 @@ func TestPagesLogin_UnderLimit_ValidToken_ReachesHandler(
 	)
 }
 
-// TestPagesLogin_CorrectPasswordSurvivesASpentBudget pins the
-// routing half of the fix, which every other login test misses by
-// driving the handler directly: no pre-emptive limiter sits in front
-// of POST /pages/login on the real route tree.
-//
-// A limiter registered there would answer the last request 429
-// however correct its password is, because the wrong passwords
-// before it have already spent the bucket — which is the lockout
-// this endpoint exists to not have. CSRF and the body cap still run,
-// since every request here carries a harvested token.
-func TestPagesLogin_CorrectPasswordSurvivesASpentBudget(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	const (
-		username = "operator"
-		password = "correct-horse-battery-staple"
-	)
-
-	env := newTestEnv(t)
-	env.seedUser(t, username, password)
-
-	submit := func(t *testing.T, pw string) *httptest.ResponseRecorder {
-		t.Helper()
-
-		token, cookies := env.csrfFrom(t, "/pages/login", nil)
-
-		form := url.Values{}
-		form.Set("csrf_token", token)
-		form.Set("username", username)
-		form.Set("password", pw)
-
-		return env.post("/pages/login", form, cookies)
-	}
-
-	// Spend the failure budget against this username. The exact
-	// limit belongs to the middleware; this waits for the throttle
-	// to appear rather than restating it, under a ceiling well
-	// above it so a broken limiter fails the test instead of
-	// looping.
-	const maxAttempts = 20
-
-	spent := false
-
-	for range maxAttempts {
-		code := submit(t, "wrong").Code
-		if code == http.StatusTooManyRequests {
-			spent = true
-
-			break
-		}
-
-		require.Equal(
-			t, http.StatusUnauthorized, code,
-			"a wrong password must be rejected, not accepted",
-		)
-	}
-
-	require.True(
-		t, spent,
-		"repeated wrong passwords must eventually be throttled",
-	)
-
-	assert.Equal(
-		t, http.StatusSeeOther, submit(t, password).Code,
-		"a correct password must be accepted on the routed "+
-			"endpoint even with the failure budget spent: the "+
-			"operator has no second administrative path",
-	)
-}
-
 // --- /user/{username} group ---
 
 // TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
@@ -552,95 +432,3 @@ func TestPasswordChange_UnderLimit_Succeeds(t *testing.T) {
 		"an under-limit password change should still apply",
 	)
 }
-
-// --- /source/{sourceID} group ---
-
-// TestSourceLogs_TruncationLinkDownloadsTheBody walks the whole
-// feature the way a user does: render the event log page through
-// the production router, take the download URL out of the markup
-// the template emitted, and fetch that URL through the router
-// again. Nothing here is hand-written, so a typo in either the
-// route pattern or the template href fails this test — the
-// handler-level tests cannot catch that, because they forge
-// their own route context and assert a URL string they wrote
-// themselves.
-func TestSourceLogs_TruncationLinkDownloadsTheBody(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "loguser", "somepassword")
-	cookies := env.authCookies(t, userID, "loguser")
-
-	// Comfortably over the event log page's render cap, so the
-	// page truncates the body and renders the download link at
-	// all. The exact cap is the handlers package's business and
-	// is pinned by its own tests; this only needs to exceed it.
-	stored := strings.Repeat("Z", 64*1024)
-
-	wh := env.seedWebhook(t, userID)
-	env.seedEvent(t, wh.ID, stored)
-
-	page := env.get("/source/"+wh.ID+"/logs", cookies)
-	require.Equal(t, http.StatusOK, page.Code)
-
-	link := regexp.MustCompile(
-		`href="(/source/[^"]+/body)"`,
-	).FindStringSubmatch(page.Body.String())
-	require.Len(
-		t, link, 2,
-		"truncated body should render a download link",
-	)
-
-	w := env.get(html.UnescapeString(link[1]), cookies)
-
-	require.Equal(
-		t, http.StatusOK, w.Code,
-		"the link the page emits must be a live route",
-	)
-	assert.Equal(t, stored, w.Body.String())
-	assert.Equal(
-		t, strconv.Itoa(len(stored)),
-		w.Header().Get("Content-Length"),
-	)
-	assert.Equal(
-		t, "application/octet-stream",
-		w.Header().Get("Content-Type"),
-	)
-	assert.Contains(
-		t, w.Header().Get("Content-Disposition"), "attachment",
-	)
-	assert.Equal(
-		t, "nosniff", w.Header().Get("X-Content-Type-Options"),
-	)
-}
-
-// TestSourceLogsBody_OtherUser404s pins that the download route
-// as registered is behind the auth the group provides and the
-// ownership check the handler applies: another logged-in user
-// asking the real router for the same URL gets a 404, and an
-// unauthenticated request never reaches the handler at all.
-func TestSourceLogsBody_OtherUser404s(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	ownerID, _ := env.seedUser(t, "owner", "somepassword")
-	wh := env.seedWebhook(t, ownerID)
-
-	const payload = "OWNERS-PAYLOAD-77c1"
-
-	evt := env.seedEvent(t, wh.ID, payload)
-	path := "/source/" + wh.ID + "/logs/" + evt.ID + "/body"
-
-	intruderID, _ := env.seedUser(t, "intruder", "somepassword")
-	intruder := env.authCookies(t, intruderID, "intruder")
-
-	w := env.get(path, intruder)
-	assert.Equal(t, http.StatusNotFound, w.Code)
-	assert.NotContains(t, w.Body.String(), payload)
-
-	anon := env.get(path, nil)
-	assert.Equal(t, http.StatusSeeOther, anon.Code)
-	assert.Equal(t, "/pages/login", anon.Header().Get("Location"))
-}

script/ci-mark-superseded

@@ -1,152 +0,0 @@
-#!/bin/sh
-# script/ci-mark-superseded: record an honest status on commits whose CI
-# run Gitea cancelled because a newer commit landed on the same branch.
-# Gitea writes `failure` / "Has been cancelled" for such a run, which
-# reads as a test result on a commit nothing ever tested. Cancellation is
-# unconditional server-side for push events, so the superseding run
-# rewrites those statuses to `failure` with a description that says the
-# commit was never tested. `skipped` cannot be used: Gitea's combined
-# status folds `skipped` into `success`, so a never-tested commit would
-# report green. Genuine failures and successes are never touched.
-#
-# Called by the Gitea Actions workflow, which supplies GITHUB_API_URL,
-# GITHUB_REPOSITORY, GITHUB_SHA, GITHUB_WORKFLOW, GITHUB_JOB,
-# GITHUB_EVENT_NAME and GITEA_TOKEN. ANCESTOR_LIMIT (default 20) caps how
-# far back the walk looks; a value that is set but not a positive integer
-# aborts rather than silently disabling the walk.
-set -eu
-
-SUPERSEDED_DESC='Superseded by a newer commit; never tested'
-
-# Gitea builds the commit-status context as
-# "<workflow name> / <job name> (<event>)", so derive it rather than
-# hardcoding the result.
-#
-# The derivation is deliberately not byte-exact with Gitea's own rule and
-# must not be "fixed" into a silent fallback. Gitea uses the job's `name:`
-# (falling back to the job id) and the workflow's `name:` (falling back to
-# the workflow filename), while the runner exports GITHUB_JOB as the job
-# *id* and GITHUB_WORKFLOW as the parsed workflow `name:`. So giving the
-# job a display `name:`, or dropping the workflow's `name:`, makes the
-# derived context stop matching --- and require_own_context below then
-# turns every push red with a message. That loud failure is the point
-# (https://git.eeqj.de/sneak/webhooker/issues/147 item 2); guessing at a
-# fallback would restore the silent no-op it replaced.
-context() {
-    printf '%s / %s (%s)' \
-        "$GITHUB_WORKFLOW" "$GITHUB_JOB" "$GITHUB_EVENT_NAME"
-}
-
-# ANCESTOR_LIMIT is a documented knob, so a value that is set but
-# unusable must fail loudly instead of defaulting
-# (https://git.eeqj.de/sneak/webhooker/issues/80). Passing it straight to
-# git would print `fatal: not an integer` into a discarded exit status
-# and mark nothing.
-ancestor_limit() {
-    # `-` and not `:-`: an explicitly empty value is set-but-unusable
-    # config, so it aborts like any other bad value rather than silently
-    # running at the default.
-    _limit="${ANCESTOR_LIMIT-20}"
-    case "$_limit" in
-    '' | *[!0-9]* | 0*)
-        echo "ANCESTOR_LIMIT must be a positive integer," \
-            "got '${_limit}'" >&2
-
-        return 1
-        ;;
-    esac
-    printf '%s' "$_limit"
-}
-
-# The status Gitea created for this very job proves which context string
-# it uses. If the derived one is missing, the workflow or the job was
-# renamed and the match below would silently stop firing, restoring the
-# false-red bug with no signal. Fail loudly instead.
-require_own_context() {
-    if ! _body="$(curl -sf --retry 3 --retry-delay 2 --max-time 30 \
-        "${1}/commits/${GITHUB_SHA}/status")"; then
-        echo "cannot read commit statuses for ${GITHUB_SHA}" >&2
-        return 1
-    fi
-    _found="$(printf '%s' "$_body" | jq -r '(.statuses // [])[].context')"
-    if printf '%s\n' "$_found" | grep -qxF "$2"; then
-        return 0
-    fi
-    echo "no commit status with context '${2}' on ${GITHUB_SHA}:" >&2
-    echo "workflow or job renamed? contexts present:" >&2
-    printf '%s\n' "$_found" >&2
-    return 1
-}
-
-# Latest status for our context on a commit, as "state|description".
-# The read is retried and bounded, and a read that still fails aborts the
-# step: a laundered commit that cannot be read is not the same as one
-# with nothing to do, and piping curl into jq would discard the
-# difference.
-status_of() {
-    if ! _sbody="$(curl -sf --retry 3 --retry-delay 2 --max-time 30 \
-        "${1}/commits/${2}/status")"; then
-        echo "cannot read commit statuses for ${2}" >&2
-
-        return 1
-    fi
-    printf '%s' "$_sbody" | jq -r --arg c "$3" \
-        '[(.statuses // [])[] | select(.context == $c)][0] // empty
-         | "\(.status)|\(.description)"'
-}
-
-mark_superseded() {
-    curl -sf -X POST "${1}/statuses/${2}" \
-        -H "Authorization: token ${GITEA_TOKEN}" \
-        -H 'Content-Type: application/json' \
-        -d "$(jq -nc --arg c "$3" --arg d "$SUPERSEDED_DESC" \
-            '{context: $c, state: "failure", description: $d}')" \
-        >/dev/null
-}
-
-main() {
-    _api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
-    _ctx="$(context)"
-
-    _limit="$(ancestor_limit)"
-
-    require_own_context "$_api" "$_ctx"
-
-    # A shallow clone cannot resolve the parent, so it looks exactly like
-    # a root commit to rev-parse below and would exit 0 having walked
-    # nothing (or, at depth > 1, only the ancestors that happen to be
-    # present). The workflow checks out with `fetch-depth: 0`; verify
-    # that here rather than depend on it silently.
-    if [ "$(git rev-parse --is-shallow-repository)" = 'true' ]; then
-        echo "shallow repository: the ancestor walk needs full history" >&2
-
-        return 1
-    fi
-
-    # A root commit legitimately has no ancestors and is not an error.
-    # A SHA this repository does not have lands here too, since its
-    # parent is equally unresolvable, but require_own_context above has
-    # already aborted on the 404 for it. The walk itself carries no
-    # `|| true`, so a rev-list failure aborts.
-    if ! git rev-parse -q --verify "${GITHUB_SHA}^" >/dev/null; then
-        echo "no ancestor of ${GITHUB_SHA} to check"
-
-        return 0
-    fi
-
-    _walk="$(git rev-list --max-count="$_limit" "${GITHUB_SHA}^")"
-
-    for _sha in $_walk; do
-        _latest="$(status_of "$_api" "$_sha" "$_ctx")"
-        # A run that was cancelled, or one an earlier revision of this
-        # script laundered into `skipped`. Anything else stands.
-        case "$_latest" in
-        'failure|Has been cancelled' | "skipped|${SUPERSEDED_DESC}") ;;
-        *) continue ;;
-        esac
-        mark_superseded "$_api" "$_sha" "$_ctx"
-        echo "marked superseded: ${_sha}"
-    done
-}
-
-main "$@"

templates/source_logs.html

@@ -38,7 +38,7 @@
                 <div x-show="open" x-cloak class="mt-3 p-3 bg-gray-50 rounded-md">
                     <pre class="text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all">{{.Body}}</pre>
                     {{if .BodyTruncated}}
-                    <p class="mt-2 text-xs text-gray-500">Body truncated for display: showing {{.BodyShownBytes}} of {{.BodyBytes}} bytes. The stored body is unchanged &mdash; <a href="/source/{{$.Webhook.ID}}/logs/{{.ID}}/body" class="text-primary-600 hover:text-primary-700 underline">download the full body</a>.</p>
+                    <p class="mt-2 text-xs text-gray-500">Body truncated for display: showing {{.BodyShownBytes}} of {{.BodyBytes}} bytes. The stored body is unchanged.</p>
                     {{end}}
                 </div>
             </div>