docs(06-11): refresh Phase 6 security review

.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md

@@ -2,22 +2,31 @@
 phase: 06
 slug: http-routing-auth-groups-and-rate-limiting
 status: verified
+threats_total: 26
+threats_closed: 26
 threats_open: 0
+accepted_risks: 4
 asvs_level: 1
 created: 2026-09-19
-verified: 2026-09-20
+verified: 2026-09-21
 ---
 
 # Phase 6 — Security Review
 
-> Guard registry, dual-group auth, rate limiting, raw-group house-middleware refusal, CORS/body-limit scoping, and the SSRF fetch helper. Every `T-06-01` through `T-06-18`, `T-06-21`, `T-06-22`, and `T-06-SC` from Plans 06-01 through 06-06 is mapped below to a named passing test or a restated accept rationale. Unmapped IDs are a review gap, not an accepted risk.
+> Guard registry, dual-group auth, atomic rate limiting, raw-group house-middleware refusal and transactional panic recovery, CORS/body-limit scoping, transition-aware SSRF protection, and exact personal-token denial serialization. Every reviewed ID from Plans 06-01 through 06-10 is mapped below to a named passing test or a restated accept rationale; Plan 06-11 refreshes the review only after both repositories pass their complete race and vet gates. Unmapped IDs are a review gap, not an accepted risk.
 
-**Date:** 2026-09-20
-**Scope:** Plans 06-01 through 06-06 (implementation, coverage, gap closure, and this review).
+**Date:** 2026-09-21
+**Scope:** Plans 06-01 through 06-10 (implementation, coverage, and corrective gap closure) plus the Plan 06-11 review refresh.
 **Repos grepped:** `summercms.go` and `fonoteka.go` (excluding `.planning/` and `vendor/`).
 
 ---
 
+## Verdict Summary
+
+The post-gap implementation closes all five threats promoted by the Phase 6 verifier and code review. The reviewed register now contains **26 total threats: 26 closed, 0 open, with 4 unchanged accepted risks and no new accepted risk**. This verdict was published only after `go test ./... -count=1 -race -short` and `go vet ./...` passed in both `summercms.go` and `fonoteka.go`.
+
+---
+
 ## Trust Boundaries
 
 | Boundary | Description | Data Crossing |
@@ -28,10 +37,15 @@ verified: 2026-09-20
 | client → X-Forwarded-For / limiter keys | untrusted IP / token id / route param feeds the Store | `RemoteAddr`, XFF, `tok:<id>` |
 | unauthenticated client → personal-token route | missing or invalid bearer traffic must consume a bounded per-IP budget before scope denial returns | bearer status, client IP, limiter counter |
 | inv_token context → limiter key resolver | valid credentials must be resolved before rate limiting to retain independent token budgets | `bouncer.Credential`, `tok:<id>` |
+| concurrent requests → limiter admission | threshold comparison and admitted increment must be one atomic decision | fixed-window count, maximum, retry duration |
+| request metadata → anonymous inline key | caller-controlled throttle text and Host inputs must not select a fresh budget | constant `inline:domainless`, trusted-proxy `ClientIP` |
 | public-share group → anonymous caller | zero-credential surface; 429 bodies must not leak internals | Retry-After, JSON error body |
 | raw group → house middleware | RFC/OAuth surface must never inherit the house envelope | `inv.must-change-password` |
+| handler/middleware → client response | status, headers, and body remain private until successful handler completion | buffered response, success commit, panic discard |
 | request body → handler | unbounded POST is a resource-exhaustion vector | `http.MaxBytesReader` |
 | caller-supplied URL → outbound fetch | user/third-party URL must never reach loopback, RFC1918, CGNAT, or metadata | dial-time IP, host allow-list |
+| IPv6 transition syntax → IPv4 SSRF policy | embedded IPv4 in NAT64 and 6to4 must receive the ordinary reserved/private classification | RFC 6052 `/96` and `/48`, RFC 3056 `2002::/16` |
+| personal-token context → denial serializer | status and exact JSON bytes cross the public compatibility boundary together | `wire.WriteJSON`, raw 401/403 bytes |
 
 ---
 
@@ -59,9 +73,14 @@ verified: 2026-09-20
 | T-06-18 | Spoofing | 06-04 | mitigate | `fetchguard/fetch_test.go:TestFetchAllowHostsRejectsDottedSuffixBypass`; `fetchguard/fetch_coverage_test.go:TestHostAllowedExactAndDottedSuffix` |
 | T-06-21 | Denial of Service | 06-06 | mitigate | `plugins/golem15/fonoteka/routes_isolation_test.go:TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` drives 61 same-IP requests through the real handler returned by `surf.Assemble`: requests 1-60 retain 401 `Invalid token`, while request 61 receives the exact 429 response. The source declaration and runtime-order invariant is `inv_token` -> `throttle:fonoteka-api-token` -> `inv.scope:read`. |
 | T-06-22 | Denial of Service | 06-06 | mitigate | The live route keeps `inv_token` before `throttle:fonoteka-api-token`, so `bouncer.Credential` is populated before the bucket key closure and valid credentials retain `tok:<id>` keying instead of collapsing onto the IP fallback. The exact ordering is covered by `TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` plus the route-source invariant. |
+| T-06-23 | Denial of Service | 06-07 | mitigate | `surf/limiter_test.go:TestMemoryStoreConcurrentAttempt`; `surf/limiter_test.go:TestFixedWindowLimiterConcurrentMaxOne`; `surf.MemoryStore.Attempt` owns expiry, threshold comparison, admitted increment, and retry duration under one mutex. |
+| T-06-24 | Denial of Service | 06-07 | mitigate | `surf/limiter_test.go:TestFixedWindowLimiterInlineThrottleKeys/anonymous_same_IP_different_Host`; `TestFixedWindowLimiterInlineThrottleKeys/anonymous_inline_policies_share_a_domainless_key`; `TestFixedWindowLimiterInlineThrottleKeys/principals_differ`; production anonymous key is exactly `inline:domainless|<ClientIP>`. |
+| T-06-25 | Elevation of Privilege / Information Disclosure | 06-08 | mitigate | `fetchguard/ip_test.go:TestIsReservedOrPrivateIPv6Transitions`; `fetchguard/fetch_test.go:TestDialControlRejectsUnsafeIPv6Transitions`; `fetchguard.embeddedTransitionIPv4` decodes both NAT64 forms and 6to4 before the dial decision. |
+| T-06-26 | Information Disclosure | 06-09 | mitigate | `surf/router_test.go:TestRecoverDiscardsPartialResponse/house`; `TestRecoverDiscardsPartialResponse/raw`; `TestBufferedResponseCommitsSuccessfulOutput`; shared `bufferedResponse` commits only after a normal return. |
+| T-06-27 | Tampering | 06-10 | mitigate | `plugins/golem15/fonoteka/middleware/token_scope_test.go:TestInvScope/no-user-401`; `TestInvScope/missing-scope-403`; both denial branches call `wire.WriteJSON` and compare untrimmed bytes. |
 | T-06-SC | Tampering | 06-03 | accept | Both packages are STACK.md-named and pass 06-RESEARCH.md's Package Legitimacy Audit (Approved disposition, no [ASSUMED]/[SUS] verdicts) -- no additional human-verify checkpoint required beyond that prior audit |
 
-*Status: closed. Disposition copied verbatim from the originating plan. Accept rationales copied verbatim.*
+*Status: 26 closed / 0 open. Dispositions are copied from the originating plans; all accept rationales remain verbatim.*
 
 ---
 
@@ -157,6 +176,41 @@ verified: 2026-09-20
 - **Finding:** A valid personal token populates `bouncer.Credential` before the limiter resolves its key, preserving `tok:<id>` keying. Only missing or invalid credentials fall back to `surf.ClientIP`; valid credentials do not collapse onto a shared IP budget.
 - **Disposition:** closed / mitigate.
 
+### T-06-23 — fixed-window admission is atomic under contention
+
+- **Source:** `surf/limiter_store.go` (`Store.Attempt`, `MemoryStore.Attempt`); `surf/limiter.go` (`FixedWindowLimiter.Middleware`).
+- **Test evidence:** `TestMemoryStoreConcurrentAttempt` and `TestFixedWindowLimiterConcurrentMaxOne` coordinate 32 workers behind ready/start barriers with `Max=1`.
+- **Finding:** `Attempt` reads time once and performs lazy expiry, threshold comparison, the admitted increment, and retry-duration calculation while holding one mutex. Exactly one contender is admitted, the protected handler runs once, and the other 31 requests receive the exact 429 body without incrementing the exhausted counter.
+- **Disposition:** closed / mitigate.
+
+### T-06-24 — anonymous inline keys are server-controlled and domainless
+
+- **Source:** `surf/limiter.go` (`FixedWindowLimiter.resolve`), whose anonymous signature is exactly `inline:domainless|<ClientIP>` and whose authenticated signature remains `u:<id>`.
+- **Test evidence:** `TestFixedWindowLimiterInlineThrottleKeys/anonymous_same_IP_different_Host` proves Host rotation shares the exhausted bucket; `TestFixedWindowLimiterInlineThrottleKeys/anonymous_inline_policies_share_a_domainless_key` proves different inline throttle parameters from the same IP share one budget; `TestFixedWindowLimiterInlineThrottleKeys/principals_differ` proves authenticated principals retain independent `u:<id>` buckets.
+- **Finding:** The anonymous key explicitly excludes the throttle `param`, `r.Host`, the `Forwarded` host parameter, and `X-Forwarded-Host`. Only the constant router-owned namespace and trusted-proxy-aware `ClientIP` participate, so neither policy text nor any request/forwarded Host input can rotate anonymous buckets.
+- **Disposition:** closed / mitigate.
+
+### T-06-25 — transition-address SSRF representations receive the IPv4 policy
+
+- **Source:** `fetchguard/ip.go` (`isReservedOrPrivate`, `embeddedTransitionIPv4`); `fetchguard/fetch.go` (`dialControl`).
+- **Test evidence:** `TestIsReservedOrPrivateIPv6Transitions` covers loopback, RFC1918, metadata, and public controls for RFC 6052 `64:ff9b::/96`, RFC 6052 local-use `64:ff9b:1::/48`, and RFC 3056 6to4 `2002::/16`, including fail-closed non-zero `/48` `u` octet handling. `TestDialControlRejectsUnsafeIPv6Transitions` proves all unsafe forms return `errPrivateIP` / `ReasonPrivateIP` at the production connection boundary before the raw connection is used.
+- **Finding:** Supported transition formats extract an IPv4 value and recursively apply the ordinary IPv4 reserved/private table; public `8.8.8.8` controls remain allowed rather than blanket-blocking the prefixes.
+- **Disposition:** closed / mitigate.
+
+### T-06-26 — partial route output is discarded on panic
+
+- **Source:** `surf/router.go` (`bufferedResponse`, `recoverJSON`, `recoverBare`).
+- **Test evidence:** `TestRecoverDiscardsPartialResponse/house` and `TestRecoverDiscardsPartialResponse/raw` each write status 202, `X-Partial: secret`, and `secret-partial` before panicking. `TestBufferedResponseCommitsSuccessfulOutput` covers explicit status, repeated `WriteHeader`, implicit 200, headers, and body on success.
+- **Finding:** Both recovery wrappers pass only the private buffer to the route. A panic discards buffered status, headers, and body: house returns exact `{"error":true,"message":"Internal server error"}` with status 500, while raw returns a header-clean, bodyless 500. A normal return commits once and replaces only route-owned header keys, preserving unrelated outer-wrapper headers.
+- **Disposition:** closed / mitigate.
+
+### T-06-27 — InvScope denial bytes match the PHP contract
+
+- **Source:** `plugins/golem15/fonoteka/middleware/token_scope.go`, where both denial branches call `wire.WriteJSON`.
+- **Test evidence:** `TestInvScope/no-user-401` compares raw bytes exactly to `{"error":"Invalid token"}`; `TestInvScope/missing-scope-403` compares raw bytes exactly to `{"error":"Missing required scope: write"}`. Both assert final `}` and reject every CR/LF byte before the secondary JSON-shape check.
+- **Finding:** The prior `json.Encoder.Encode` newline is gone; status, Content-Type, and untrimmed body bytes are one locked response contract. The valid-scope path still reaches the handler, and the wrong-credential path remains fail-closed.
+- **Disposition:** closed / mitigate.
+
 ### T-06-SC — OpenAPI toolchain packages (accept)
 
 - **Rationale (verbatim from 06-03):** Both packages are STACK.md-named and pass 06-RESEARCH.md's Package Legitimacy Audit (Approved disposition, no [ASSUMED]/[SUS] verdicts) -- no additional human-verify checkpoint required beyond that prior audit.
@@ -184,7 +238,14 @@ That line is inside `HouseMiddlewares()`. `Middlewares()` registers `public.shar
 
 ## Accepted Risks Log
 
-Four accepts (06-05's "three" list omitted T-06-05, which 06-01 already accepted). Plan 06-06 adds two mitigated threats and no new accepts. Rationales are copied verbatim in the Threat Register `Proof` column for each accept row.
+Four accepts (06-05's "three" list omitted T-06-05, which 06-01 already accepted). Plans 06-06 through 06-11 add seven mitigated threats and no new accepts. Rationales are copied verbatim in the Threat Register `Proof` column for each accept row.
+
+## Post-Gap Verification Gates
+
+- `summercms.go`: `go test ./... -count=1 -race -short` — pass; `go vet ./...` — pass.
+- `fonoteka.go`: `go test ./... -count=1 -race -short` — pass; `go vet ./...` — pass.
+- Source assertion: anonymous inline keys contain `inline:domainless|<ClientIP>` and no throttle-parameter or request/forwarded-Host contribution — pass.
+- Evidence assertion: exactly one Threat Register row and one substantive finding exist for each of T-06-23 through T-06-27 — pass.
 
 ---
 
@@ -194,3 +255,4 @@ Four accepts (06-05's "three" list omitted T-06-05, which 06-01 already accepted
 |------------|---------------|--------|------|--------|
 | 2026-09-19 | 19 | 19 | 0 | gsd-executor (06-05) |
 | 2026-09-20 | 21 | 21 | 0 | gsd-executor (06-06) |
+| 2026-09-21 | 26 | 26 | 0 | gsd-executor (06-11 post-gap refresh) |