13 KiB
Purpose: HTTP-04 is a security control, so sequential correctness is insufficient; one atomic store operation must own threshold check plus increment, and request-controlled headers must not select a bucket. Output: an atomic Store API, FixedWindowLimiter wired to it, one server-controlled domainless inline namespace, and deterministic concurrency/Host-rotation/cross-policy regressions.
<execution_context> @/home/jin/.codex/get-shit-done/workflows/execute-plan.md @/home/jin/.codex/get-shit-done/templates/summary.md </execution_context>
@.planning/PROJECT.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-CONTEXT.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-REVIEW.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-06-SUMMARY.md Replace the split Store admission protocol with one operation:Attempt(key string, max int, decay time.Duration) (allowed bool, attempts int, retryAfter time.Duration)
The operation owns lazy expiry, first-hit window creation, threshold comparison, increment of an admitted request, and remaining-window calculation under one lock. attempts is the post-increment count when allowed and the unchanged exhausted count when denied.
In `limiter.go`, call `l.store.Attempt(key, b.Max, b.Decay)` exactly once. On denial, derive `Retry-After`, `X-RateLimit-Reset`, limit/remaining headers, status 429, and the existing exact body from that returned retry duration. On admission, derive remaining as `b.Max-attempts`, set the existing success headers, and invoke `next`. Preserve the exact 60th-allowed/61st-denied semantics, named-bucket stacking, fixed-window duration, and PHP body/headers from D-02.
Change the anonymous inline key closure to `"inline:domainless|" + ClientIP(r, trusted)`. The constant prefix represents the router's server-controlled domainless route identity required by D-02/Laravel parity. Every anonymous inline policy for the same client IP must therefore share this signature: do not include `param`, `r.Host`, `Host`, `X-Forwarded-Host`, URL host, or any other policy-specific or caller-controlled value. Preserve authenticated keys as `u:<principal id>`.
Rewrite existing Store tests around Attempt without deleting expiry, first-hit-wins, sweep, header, stacking, or remaining-count assertions. Add two coordinated concurrency regressions: all goroutines must signal ready and block on a shared start channel before attempting the same key/request; assert exact counts after all complete. The test must use the real MemoryStore and real FixedWindowLimiter, not a serial fake. Extend the anonymous inline-key tests in two independent regressions: (1) request 1 and request 2 use the same RemoteAddr but intentionally different Host values and the second is 429; (2) two requests through `Middleware("2,1")` for one RemoteAddr succeed, then a request from that same RemoteAddr through `Middleware("1,1")` returns 429, proving the inline parameter does not create a new key. Retain the authenticated same-IP/different-principal test proving distinct `u:<id>` buckets. Run the race detector on the package.
cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go test ./surf -run 'Test.*(Atomic|Concurrent|InlineThrottleKeys|MemoryStore|TooManyAttempts|StackedBuckets)' -count=1 -race -short && go vet ./surf && go test ./surf -count=1 -race -short
- `Store` exposes one atomic Attempt operation; production middleware contains no `TooManyAttempts` followed by `Hit` sequence.
- MemoryStore performs expiry check, threshold check, and admitted increment within one mutex critical section and never increments a denied attempt.
- The concurrent Max=1 Store and middleware tests each coordinate all workers with ready/start barriers; the middleware test proves exactly one protected-handler invocation, one success, and N-1 exact 429 responses.
- An anonymous Host-rotation test uses one IP and at least two distinct Host strings, then proves the second request shares the exhausted bucket.
- A cross-inline-policy regression exhausts the shared same-IP counter through `throttle:2,1`, then proves `throttle:1,1` returns 429 instead of receiving a fresh parameter-specific budget.
- Production anonymous inline key construction contains the constant `inline:domainless|` namespace and `ClientIP`, but contains neither `param`, `r.Host`, nor forwarded-host input; authenticated inline keys remain `u:`.
- Existing sequential window, success/429 header, stacked-bucket, authenticated-user-key, sweep, and expiry tests remain present and pass under `-race`.
Concurrent callers cannot over-admit a fixed window, anonymous callers cannot rotate Host or inline policy parameters to rotate buckets, authenticated users retain isolated keys, and the existing PHP-compatible sequential/header semantics remain green.
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
| concurrent clients -> MemoryStore | Many untrusted requests can reach the same key simultaneously and must receive one serialized admission decision |
| request metadata -> inline bucket key | Host and forwarding headers are attacker-controlled; the router contributes one constant domainless-route namespace and only trusted-proxy-aware ClientIP varies the anonymous signature |
STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|---|---|---|---|---|
| T-06-23 | Denial of Service | Store / FixedWindowLimiter.Middleware |
mitigate | Replace split check/increment with atomic Attempt and prove coordinated Max=1 contention admits exactly one request under -race |
| T-06-24 | Denial of Service | inline anonymous key resolver | mitigate | Key only by `inline:domainless |
| T-06-SC | Tampering | package supply chain | accept | No dependency or package-manager change; this plan uses existing stdlib and Phase 6 packages only |
| </threat_model> |
<success_criteria>
- Exactly one concurrent request reaches a Max=1 protected handler.
- Threshold comparison and increment are atomic in the Store contract and implementation.
- Different Host values from one anonymous IP share the same inline bucket.
- Different inline throttle parameters from one anonymous IP share the same domainless/IP key and budget.
- Authenticated principals from one IP retain independent
u:<id>buckets. - Named buckets, authenticated per-user keys, stacking, expiry, headers, and exact 429 body retain their established behavior. </success_criteria>