Files
summercms/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md
2026-09-20 13:57:07 +02:00

207 lines
14 KiB
Markdown

---
phase: 06-http-routing-auth-groups-and-rate-limiting
verified: 2026-09-20T11:53:11Z
status: gaps_found
score: 7/12 must-haves verified
overrides_applied: 0
mvp_mode_note: "ROADMAP mode is mvp but the goal is not a User Story (user-story.validate valid=false); this requested re-verification uses the technical roadmap contract."
re_verification:
previous_status: gaps_found
previous_score: 11/12
gaps_closed:
- "The live personal-token chain is now inv_token -> throttle:fonoteka-api-token -> inv.scope:read; requests 1-60 return 401 and request 61 returns 429."
gaps_remaining:
- "Rate-limit admission is non-atomic and inline anonymous keys trust r.Host."
- "The SSRF guard misses private IPv4 embedded in NAT64/6to4 addresses."
- "Panic recovery cannot replace a partially committed response."
- "InvScope appends a newline to PHP-compatible 401/403 bodies."
regressions: []
gaps:
- truth: "All five named buckets and inline throttles enforce their limits and documented keys under concurrent traffic."
status: failed
reason: "06-06 fixes middleware order, but TooManyAttempts and Hit remain separately locked, so concurrent requests can all pass the threshold. Inline anonymous keys also include attacker-controlled Host."
artifacts:
- path: "surf/limiter.go"
issue: "Lines 95-108 are check-then-increment; lines 160-164 use r.Host in the key."
- path: "surf/limiter_store.go"
issue: "The Store has no atomic attempt/admission operation."
missing:
- "Atomic threshold check plus increment"
- "Server-controlled inline key prefix instead of r.Host"
- "Concurrent Max=1 and Host-rotation tests"
- truth: "The outbound fetch helper rejects private/reserved destinations in every supported address representation."
status: failed
reason: "Addr.Unmap handles mapped IPv4 only. NAT64 and 6to4 values embedding loopback, RFC1918, or metadata IPv4 miss both current tables."
artifacts:
- path: "fetchguard/ip.go"
issue: "No classification for 64:ff9b::/96, 64:ff9b:1::/48, or 2002::/16."
- path: "fetchguard/fetch.go"
issue: "Dial control only Unmaps before classification."
missing:
- "Decode/recheck embedded IPv4 or reject unsafe transition forms"
- "Transition-address security tests"
- truth: "Panics yield only the promised bare raw 500 or opaque house 500, including after a partial write."
status: failed
reason: "Recovery writes after the wrapped handler. Once status/body is committed, the fallback 500 is ignored and partial data remains. Existing tests panic before writing."
artifacts:
- path: "surf/router.go"
issue: "recoverJSON/recoverBare write directly to the original ResponseWriter."
- path: "surf/router_test.go"
issue: "No partial-write-then-panic coverage."
missing:
- "Buffer/discard responses covered by the opaque recovery contract, or explicitly narrow raw semantics"
- "Partial-write panic tests for both route kinds"
- truth: "Personal-token 401/403 bodies are byte-identical to PHP TokenScope."
status: failed
reason: "InvScope uses json.Encoder.Encode, which appends a newline; the tests hide it with strings.TrimSpace."
artifacts:
- path: "../fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go"
issue: "Line 34 appends a newline."
- path: "../fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope_test.go"
issue: "Line 73 trims before comparison."
missing:
- "Use wire.WriteJSON (or equivalent no-newline writer)"
- "Assert exact 401 and 403 bytes"
deferred:
- truth: "Public/onboarding groups have reachable unauthenticated handlers."
addressed_in: "Phase 13"
evidence: "Phase 13 explicitly ports onboarding/public/invitation routes and public buckets."
- truth: "Unknown and malformed ids on ownership-scoped resources both return 404."
addressed_in: "Phase 12"
evidence: "Phase 12 owns Collections and Albums; Phase 6 supplies the tested constraint primitive."
- truth: "Manual-cover and Discogs production callers use fetchguard."
addressed_in: "Phase 12 / Phase 14"
evidence: "Those phases own cover handling and Discogs integration; 06-04 explicitly shipped helper-only."
---
# Phase 6: HTTP routing, auth groups and rate limiting Verification Report
**Phase Goal:** The three mutually exclusive auth groups share handlers with correct subsets, rate-limit buckets are ported 1:1, OAuth/RFC routes are structurally raw, and the auth registry, limiter, and SSRF fetch helper form secure shared infrastructure.
**Verified:** 2026-09-20T11:53:11Z
**Status:** gaps_found
**Re-verification:** Yes — 06-06 closes the prior middleware-order gap, but current code-review findings expose goal-level defects.
ROADMAP marks this phase `mode: mvp`, but `gsd-sdk query user-story.validate` returns `valid=false`: the goal is not in `As a …, I want …, so that ….` form. User Flow Coverage cannot be generated honestly; this report retains the requested technical verification framing.
## Goal Achievement
### Observable Truths
| # | Truth | Status | Evidence |
| --- | --- | --- | --- |
| 1 | JWT and personal-token groups share the genres handler; subsets are mutually exclusive | ✓ VERIFIED (later groups deferred) | `routes.go:10-16` binds one handler twice; full route-table isolation test passes. |
| 2 | Five named buckets and inline throttles enforce documented limits/keys, including stacking | ✗ FAILED | 06-06 order and request-61 test pass, but `limiter.go:95-108` is non-atomic and inline keys trust `r.Host` (current REVIEW CR-01/CR-02). |
| 3 | Response conventions and raw/house panic behavior hold | ✗ FAILED | Wire helpers and structural raw refusal pass. Partial-write panic breaks the promised 500 boundary, and InvScope adds `\n` while its test trims it (CR-04/WR-11). |
| 4 | Fetch helper is an SSRF boundary with allow-list, private-IP rejection, cap, timeout | ✗ FAILED | Ordinary ranges, cap, timeout, and redirects are covered; NAT64/6to4 embedded private IPv4 bypasses classification (CR-03). |
| 5 | OpenAPI/type validation, path CORS, and production body limits exist | ✓ VERIFIED (warnings) | OpenAPI 3 artifact, CORS wiring, and 134217728-byte config are present. Document omits the second live route/auth schemes (WR-07). |
| 6 | Guard registry unifies JWT/personal-token users and preserves exact error contracts | ✗ FAILED | Registry/accessor are wired; exact personal-token bytes fail due Encoder newline. |
| 7 | `name:param` middleware resolves through factories | ✓ VERIFIED | `strings.Cut` factory path is used by throttle/body.limit/inv.scope. |
| 8 | Fixed-window limiter matches required enforcement semantics | ✗ FAILED | Sequential tests pass; concurrent threshold admission is not atomic. |
| 9 | Raw routes refuse house-tagged middleware through plugin capabilities | ✓ VERIFIED | Central `HasHouseMiddleware` collection and raw refusal remain wired. |
| 10 | Route table excludes JWT middleware from token routes and vice versa | ✓ VERIFIED | Targeted assembled-router test passes. |
| 11 | Planned Phase 6 threat IDs are mapped in the security review | ✓ VERIFIED (stale verdict) | IDs are mapped, but `threats_open: 0` is contradicted by current CR-01..04. |
| 12 | Phase packages and route regressions run | ✓ VERIFIED | Targeted framework and fonoteka checks pass; they omit the adversarial paths above. |
**Score:** 7/12 truths verified
### Deferred Items
| Item | Addressed In | Evidence |
| --- | --- | --- |
| Public/onboarding handlers | Phase 13 | Later goal explicitly names these routes and public buckets. |
| Ownership-resource ID behavior | Phase 12 | Collections/Albums are ported there. |
| Production fetchguard callers | Phase 12 / 14 | Cover handling and Discogs are owned there. |
### Required Artifacts
The SDK reports repo-prefixed PLAN paths missing because CWD is already `summercms.go`; they were resolved manually here and in sibling `../fonoteka.go`.
| Artifact | Expected | Status | Details |
| --- | --- | --- | --- |
| `bouncer/registry.go`, `guard.go` | Guard registry/interfaces | ✓ VERIFIED | Substantive, wired, tested. |
| `../fonoteka.go/.../token_guard.go` | Token credential guard | ✓ VERIFIED | Hash/usability/stamp path wired via Boot. |
| `../fonoteka.go/.../token_scope.go` | PHP scope gate | ✗ DEFECTIVE | Behavior wired; bytes include newline. |
| `surf/limiter.go`, `limiter_store.go` | Fixed-window enforcement | ✗ DEFECTIVE | Data flows, but admission is raceable and inline key is attacker-influenced. |
| `surf/routetable.go`, `pact/capabilities.go` | Route/raw inspection | ✓ VERIFIED | Used by router, route:list, isolation tests. |
| `wire/response.go` | JSON/time/nullable helpers | ✓ VERIFIED | Used by genre controller. |
| `surf/cors.go`, `bodylimit.go` | CORS/body caps | ✓ VERIFIED (warnings) | Current config flows; malformed/missing config and wildcard+credentials remain warnings. |
| `fetchguard/fetch.go`, `policy.go`, `ip.go` | SSRF fetch | ✗ DEFECTIVE | Internally wired; transition targets unclassified. |
| `../fonoteka.go/docs/openapi.json` | Generated OpenAPI | ✓ VERIFIED (incomplete) | Valid document; only one genres route and no security scheme. |
| `06-SECURITY-REVIEW.md` | Threat map | ⚠️ STALE | Mapping exists; zero-open conclusion no longer matches code evidence. |
### Key Link Verification
| From | To | Status | Details |
| --- | --- | --- | --- |
| `routes.go` | shared handler | WIRED | Both prefixes reuse `handler`. |
| `routes.go` | limiter/scope onion | WIRED | 06-06 target order and request-61 regression pass. |
| `plugin.go` | limiter buckets | WIRED-BUT-DEFECTIVE | Five buckets register; limiter correctness fails. |
| `token_scope.go` | bouncer context | WIRED-BUT-DEFECTIVE | Auth decisions work; bytes differ. |
| `routes.go` | `GroupRaw` | WIRED | Structural refusal passes. |
| `fetch.go` | `ip.go` | WIRED-BUT-INCOMPLETE | Dial address checked; transition decoding absent. |
| `genre_controller.go` | `wire.WriteJSON` | WIRED | DB results flow to JSON. |
### Data-Flow Trace (Level 4)
| Artifact | Source | Produces Real Data | Status |
| --- | --- | --- | --- |
| Genres | GORM genre/count queries | Yes | ✓ FLOWING |
| Limiter | MemoryStore by resolved key | Yes, sequentially | ✗ FLOWING BUT RACEABLE |
| CORS/body limits | production YAML | Yes | ✓ FLOWING |
| Fetch | guarded HTTPS transport | Yes in tests | ✗ FLOWING BUT TRANSITION-UNSAFE |
### Behavioral Spot-Checks
| Behavior | Command | Result | Status |
| --- | --- | --- | --- |
| Framework phase packages | `timeout 10s go test ./bouncer ./surf ./wire ./fetchguard -short` | all `ok` | ✓ PASS |
| 06-06 closure/isolation/CORS | `timeout 10s go test ./plugins/golem15/fonoteka -run 'TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited|TestFullRouteTableAuthGroupMutualExclusivity|TestCORSPathScopedOnAssembledRouter' -count=1 -short` | `ok` | ✓ PASS |
| Concurrent threshold | Source trace: separate check then hit; no concurrency test | multiple callers can pass | ✗ FAIL |
| Partial-write panic | Source trace: recovery writes to committed writer | original response cannot be replaced | ✗ FAIL |
### Probe Execution
No probe is declared and no `scripts/*/tests/probe-*.sh` exists.
### Requirements Coverage
All seven PLAN IDs match the Phase 6 mappings; none is orphaned. REQUIREMENTS.md is internally inconsistent: HTTP-04 is checked complete at line 56 but its traceability row says `In Progress`.
| Requirement | Status | Evidence |
| --- | --- | --- |
| HTTP-03 | ✓ SATISFIED (public handlers deferred) | Shared handler and isolation exist. |
| HTTP-04 | ✗ BLOCKED | Route order fixed; atomic admission and stable inline keys remain broken. |
| HTTP-05 | ✓ SATISFIED | Registry/unified accessor exist. |
| HTTP-06 | ✗ BLOCKED | Recovery can retain/leak partial response rather than promised 500. |
| HTTP-07 | ✗ BLOCKED | Transition-address private targets evade the SSRF boundary. |
| HTTP-08 | ✓ SATISFIED (warning) | Generation/type-validation pipeline exists; coverage incomplete. |
| HTTP-09 | ✓ SATISFIED (warnings) | Current PHP-matching CORS/body values are tested. |
### Anti-Patterns Found
| File | Pattern | Severity | Impact |
| --- | --- | --- | --- |
| `surf/limiter.go:95-108` | split check/increment | 🛑 Blocker | concurrent bypass |
| `surf/limiter.go:160-164` | Host in inline key | 🛑 Blocker | caller rotates buckets |
| `fetchguard/ip.go:5-45` | no transition decoding | 🛑 Blocker | SSRF to private infrastructure |
| `surf/router.go:520-541` | recovery after direct writes | 🛑 Blocker | 200/partial-data leak on panic |
| `token_scope.go:31-35` | Encoder newline | 🛑 Blocker | exact PHP contract fails |
| `surf/limiter.go:63-94` | invalid setup fails open | ⚠️ Warning | security control can silently disable |
| `surf/router.go:432-441` | missing body config becomes zero | ⚠️ Warning | request cap can silently disable |
| `docs/openapi.json:40-73` | one route, no auth | ⚠️ Warning | generated clients miss token surface |
No unreferenced `TBD`, `FIXME`, or `XXX` markers were found. Disconfirmation pass: HTTP-04 is only sequentially correct; panic tests cover panic-before-write only; fetch tests omit transition-address targets.
### Human Verification Required
None. The production body-size checkpoint is already recorded. Current failures are programmatically observable and require code/test changes.
### Gaps Summary
Plan 06-06 closes the original middleware-order defect. The phase still fails its security-load-bearing goal: rate limiting is bypassable under concurrency (and inline through Host rotation), fetchguard misses transition-address private targets, recovery cannot uphold the opaque/bare response promise after partial output, and the token scope response is not byte-compatible. These primitives belong to Phase 6 and later phases only consume them, so they are not deferred.
---
_Verified: 2026-09-20T11:53:11Z_
_Verifier: the agent (gsd-verifier)_