Stop a slow host turning a login-guard test into a segfault (closes #186)

CI run 232 failed
TestLoginGuard_SaturatedSemaphoreRefusesRatherThanQueueing and then
took the whole internal/middleware test binary down with a SIGSEGV, on
a commit whose own gates were green. acquire returns (nil, false) on
every refusal path, the assertion on ok was non-fatal, and the next
line called the nil release. Two defects sit behind that, and the
second one is not confined to the test.

acquire selected over a slot send and an already-armed wait timer. Go
picks among ready cases uniformly at random, so a process descheduled
for longer than the wait sheds a request with slots standing free —
under load, which is exactly when shedding a login is least
defensible. A non-blocking preamble now takes a free slot before any
timer is armed, the same shape lifecycle.waitDone uses to settle its
own both-ready race. It cannot let a late arrival barge past a queued
waiter: a waiter can only be parked on a full buffer, and a release
refills that buffer from the head of the send queue under the channel
lock, so the preamble's send fails whenever anyone is waiting. Placing
it ahead of the queue admission also stops a request that never waits
from occupying a waiter's place.

The test's third acquire is therefore settled by construction rather
than by the wait being long enough, and
TestLoginGuard_FreeSlotBeatsAnExpiredWait pins that: 1000 passes with
a wait already elapsed on arrival, the worst case scheduling can
produce. Without the preamble it fails on pass 1.

Assertions whose value is dereferenced afterwards are require, not
assert. A sweep of every test file found one sibling of the same
shape: webhook_db_manager_test.go checked a slice length non-fatally
and indexed it on the next line, so the regression it guards would
have surfaced as an index-out-of-range panic through
internal/database rather than as a failing test.

TestLoginGuard_SemaphoreBoundsConcurrentVerifications used a 10 ms
sleep to make two workers overlap and asserted the observed maximum
was exactly two. A sleep only makes overlap likely; on a host that can
deschedule a goroutine for longer than the sleep the workers serialise
and the maximum comes back as one. The slot holders now rendezvous, so
the overlap the assertion needs is a fact rather than a race won, and
the test no longer sleeps at all.

The queue-cap test in the same file held the two tightest wall-clock
margins in the repo: it asserted a shed request returned in under
100 ms, timed across a goroutine hand-off, and gave the probe 200 ms
to return at all. Both bounded host latency rather than guard
behaviour. The elapsed-time assertion is gone, since a shed request is
told from a queued one by the queue depth, which is a state fact; the
probe's budget is now five seconds against a queue wait of a minute,
which only a guard that queues can exhaust. Mutating the queue
admission back to a blocking send still fails the test.

TODO.md

@@ -24,9 +24,8 @@ event retention (#63), the database archiving target (#43), the admin
 password change flow (#65), policy compliance (#6), pinned lint tooling
 (#55), and fail-loud configuration parsing (#80).
 
-`next` holds the 1.0.0 milestone less its final four issues (#176, #178,
-#186, #187 — all in review or held on merge order), and is verified
-green by cache-defeated container runs
+`next` holds the completed 1.0.0 milestone: every issue in it is closed,
+and it is verified green by cache-defeated container runs
 (`docker build --no-cache-filter=lint --no-cache-filter=builder`). The
 CI status is not independently claimed here: a superseded run is
 recorded as `skipped` and still rolls up green, so a commit status on
@@ -40,78 +39,15 @@ of 2026-07-06.
 
 # Next Step
 
-Land the last four 1.0.0 issues, then merge the milestone PR to `main`
-and tag 1.0.0 from it. Merge order is forced by a real conflict on
-`README.md` and `internal/middleware/middleware.go`: #186, then #176,
-then #178, then #187.
+Merge the milestone PR to `main` and tag 1.0.0 from it.
 
-Two items belong to the owner, neither blocking the tag. #150 was
-decided by the manager rather than left to stall the queue and is
-flagged on the issue for reversal if that call was wrong. #112 (whether
-`Completed Steps` should exist at all, given it once conflicted on every
-unit) is unanswered; the provisional ruling in force is that issue
-branches do not touch this file.
+Two decisions are open and belong to the owner, neither blocking the
+tag: #115 (mask the `http` target's destination URL, implemented
+speculatively and awaiting a yes or no) and #125 (whether IPv6
+rate-limit keys should bucket by `/64`).
 
 # Completed Steps
 
-- 2026-08-18 Send the chi route pattern to Sentry rather than the
-  concrete path. The receiver's path carries the entrypoint capability
-  token, so every Sentry event from `/webhook/{uuid}` shipped a live
-  credential to a third party. Request `Data`, `QueryString`, `Cookies`
-  and `Env` are dropped and headers reduced to an allowlist (#179)
-- 2026-08-18 Read form fields from the POST body only. `r.FormValue`
-  merges the query string, so a login could be driven by URL parameters
-  — putting the password somewhere that lands in access logs, proxy
-  logs and browser history (#160)
-- 2026-08-18 Verify login credentials before spending rate-limit
-  budget, so a flood of wrong passwords cannot lock out the account it
-  is guessing at. The manager took this decision rather than stall the
-  queue; it is flagged on the issue for reversal (#150)
-- 2026-08-18 Run all linting in Docker via `Dockerfile.lint`. Host lint
-  was wrong in both directions from version skew and shared caches.
-  `script/lint` asserts the summary line, because `--no-cache-filter`
-  silently ignores a stage name it does not match — the flag that makes
-  the gate meaningful fails open (#109)
-- 2026-08-18 Serve an event's full stored body over HTTP. The list
-  query truncates for rendering, and that truncated value was the only
-  way to read a body, so the full payload was unreachable (#157)
-- 2026-08-18 Bound the access log line against client-chosen text.
-  `internal/logfield` budgets by *encoded* bytes, not runes, so a
-  handler's JSON escaping cannot multiply a field past its allowance
-  (#146)
-- 2026-08-18 Mark superseded CI commits `failure` rather than
-  `skipped`. A skipped run rolls up green, so a commit that was never
-  tested reported success (#152)
-- 2026-08-18 Set `fx.StopTimeout` inside the container stop grace, so
-  shutdown hooks are bounded by a deadline the orchestrator will
-  actually honour rather than being killed mid-flush (#134)
-- 2026-08-17 Bucket IPv6 rate-limit keys by `/64`. A single allocation
-  hands out 2^64 addresses, so per-address keying let one client mint
-  unlimited buckets. Manager decision, recorded on the issue (#125)
-- 2026-08-17 Correct release-blocking README and startup-warning
-  inaccuracies, including claims about behaviour the code does not have
-  (#151)
-- 2026-08-17 Fetch and verify Alpine.js at build time against
-  `static/vendor.sha256` instead of committing the minified blob, so
-  the dependency is pinned by hash rather than by trust (#145)
-- 2026-08-17 Bound the event log's rendered bodies in the query itself,
-  so a large stored payload cannot be read into memory just to be
-  truncated for display (#135)
-- 2026-08-17 Mask the `http` target's destination URL in the UI: it can
-  carry a bearer credential in its path or query, and was rendered
-  verbatim. Manager decision to mask unconditionally (#115)
-- 2026-08-14 Bound shutdown hooks by their stop context, so a hook that
-  hangs cannot hold the process past its grace period (#102)
-- 2026-08-14 Render templates via a buffer rather than the
-  `ResponseWriter`, so a template error part-way through cannot commit
-  a 200 and then fail — the response is written only once it is whole
-  (#123)
-- 2026-08-14 Align the session codec's max-age with the 7-day absolute
-  cap. The codec accepted cookies the session layer considered expired,
-  so the cap was enforced in one place and not the other (#108)
-- 2026-08-12 Warn when `TRUSTED_PROXIES` is empty in production, where
-  the safe default silently discards forwarded headers and every client
-  rate-limits as the proxy's address (#149)
 - 2026-08-12 Bound the receiver rate limit per client IP across the
   whole `/webhook/*` route. The existing limiter keyed on the request
   path and `/webhook/{uuid}` matches any single segment, so a client

internal/middleware/loginguard.go

@@ -173,17 +173,9 @@ 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
-// request was cancelled while waiting; the caller must then answer
-// 503 without verifying anything. The returned function releases 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.
-//
-// ctx is consulted only once the request has to wait: a slot that is
-// free on arrival is handed out without looking at it, so an
-// already-cancelled request can be granted one. That is deliberate
-// and matches lifecycle.waitDone — the caller abandons the work on
-// its own ctx and releases the slot immediately, so nothing is spent
-// on it, and refusing instead would mean shedding a request with
-// capacity standing free.
 func (g *loginGuard) acquire(ctx context.Context) (func(), bool) {
 	// A free slot is taken before any timer is armed, and before a
 	// queue place is claimed: a request that never waits is not a

internal/middleware/loginguard_test.go

@@ -31,12 +31,10 @@ const (
 	guardUser   = "admin"
 
 	// racePasses is how many times a both-cases-ready select race is
-	// run. A pass can only go the wrong way once the zero-duration
-	// timer has fired, so the per-pass detection probability is
-	// somewhere below 1/2 rather than exactly it; the bound that
-	// matters is that passes are independent, so a regression that
-	// survives is exponentially unlikely in N. The test still waits
-	// on nothing.
+	// run. Each pass is an independent coin flip if the code under
+	// test does not settle the race itself, so at this N a
+	// regression is caught with probability 1 - 2^-N and the test
+	// still waits on nothing.
 	racePasses = 1000
 )
 
@@ -249,55 +247,33 @@ func TestLoginGuard_SemaphoreBoundsConcurrentVerifications(
 		inside  int
 		highest int
 		wg      sync.WaitGroup
-		recorded sync.WaitGroup
 		once    sync.Once
 	)
 
-	// Slot holders rendezvous instead of sleeping, and they hold until
-	// every worker has been answered. A sleep only makes overlap
-	// likely — on a host loaded enough to deschedule a goroutine for
-	// longer than the sleep the workers serialise and the maximum
-	// observed comes back as 1 — so the rendezvous is what makes the
-	// overlap a fact rather than a race won.
-	//
-	// The barrier must not open at the concurrency-th holder, which
-	// would fix the lower bound at the cost of the upper one this test
-	// exists to enforce: holders would leave as soon as the count
-	// reached concurrency, so a guard admitting extra requests would
-	// let them arrive after the first holders had already left and
-	// highest would report concurrency however many were really let
-	// in. It opens instead once every worker's acquire has returned
-	// and any slot it won has been counted, so under a broken guard
-	// every admitted worker is inside simultaneously and highest is
-	// the true maximum. Under a correct guard the refused workers
-	// return within the guard's own wait, which decides nothing beyond
-	// how long that takes.
+	// Slot holders rendezvous instead of sleeping. A sleep only makes
+	// overlap likely — on a host loaded enough to deschedule a
+	// goroutine for longer than the sleep, the workers can serialise
+	// and the maximum observed comes back as 1. Holding until the
+	// concurrency-th holder arrives makes the overlap the assertion
+	// needs a fact rather than a race won: the first holder cannot
+	// leave until a second one is inside with it.
 	overlapped := make(chan struct{})
 	closeOverlapped := func() {
 		once.Do(func() { close(overlapped) })
 	}
 
-	// Deadlock guard, not a timing margin: no assertion depends on its
-	// length, and the only way to reach it is a worker that never
-	// returns from acquire at all. It is here so that such a wedge
-	// fails legibly on the assertion below instead of hanging until
-	// the package test timeout.
+	// Deadlock guard, not a timing margin: no assertion depends on
+	// its length and the healthy path closes overlapped in
+	// microseconds. It is here so that a guard which never admits two
+	// requests at once fails legibly on the assertion below instead
+	// of hanging until the package test timeout.
 	abandon := time.AfterFunc(rendezvousDeadlock, closeOverlapped)
 	defer abandon.Stop()
 
-	recorded.Add(workers)
-
-	go func() {
-		recorded.Wait()
-		closeOverlapped()
-	}()
-
 	for range workers {
 		wg.Go(func() {
 			release, ok := g.AcquireForTest(context.Background())
 			if !ok {
-				recorded.Done()
-
 				return
 			}
 
@@ -310,12 +286,13 @@ func TestLoginGuard_SemaphoreBoundsConcurrentVerifications(
 				highest = inside
 			}
 
+			reached := inside == concurrency
+
 			mu.Unlock()
 
-			// Counted before signalling, so the barrier can never open
-			// while an admitted worker is still on its way to being
-			// counted.
-			recorded.Done()
+			if reached {
+				closeOverlapped()
+			}
 
 			<-overlapped
 
@@ -535,15 +512,15 @@ func TestLoginGuard_ShedsPastTheQueueCap(t *testing.T) {
 	defer release()
 	defer fillQueue(t, g, maxWaiters)()
 
-	granted, answered := probeQueueCap(g, probePatience)
+	got := probeQueueCap(g, probePatience)
 
-	require.True(
-		t, answered,
+	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, granted,
+		t, *got,
 		"a request arriving past the queue cap must be shed",
 	)
 	assert.Equal(
@@ -592,10 +569,8 @@ func fillQueue(
 	}
 }
 
-// probeQueueCap acquires from another goroutine. It reports, in
-// order, whether the call was granted a slot and whether it was
-// answered at all within wait; a call that never returned reports
-// false for both.
+// probeQueueCap acquires from another goroutine and reports whether
+// it got a slot, 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
@@ -608,7 +583,7 @@ func fillQueue(
 func probeQueueCap(
 	g *middleware.LoginGuard,
 	wait time.Duration,
-) (bool, bool) {
+) *bool {
 	probed := make(chan bool, 1)
 
 	go func() {
@@ -622,8 +597,8 @@ func probeQueueCap(
 
 	select {
 	case result := <-probed:
-		return result, true
+		return &result
 	case <-time.After(wait):
-		return false, false
+		return nil
 	}
 }