docs(06): record gap-closure re-verification
This commit is contained in:
Jakub Zych
@@ -1,47 +1,87 @@
|
||||
---
|
||||
phase: 06-http-routing-auth-groups-and-rate-limiting
|
||||
verified: 2026-09-19T19:45:00Z
|
||||
verified: 2026-09-20T11:53:11Z
|
||||
status: gaps_found
|
||||
score: 11/12 must-haves verified
|
||||
score: 7/12 must-haves verified
|
||||
overrides_applied: 0
|
||||
mvp_mode_note: "ROADMAP mode is mvp but the phase goal is not a User Story (gsd-sdk user-story.validate valid=false). Verification used the technical roadmap contract, not a fabricated user-flow table."
|
||||
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 rate-limit buckets are enforced with the documented keys and limits, including a route stacking two limiters."
|
||||
- truth: "All five named buckets and inline throttles enforce their limits and documented keys under concurrent traffic."
|
||||
status: failed
|
||||
reason: "The only live personal-token route lists inv_token, inv.scope:read, then throttle:fonoteka-api-token. wrap() applies names last-to-first, so InvScope runs before throttle and returns 401 without calling next. Unauthenticated GET /api/v1/fonoteka/genres never consumes the 60/min bucket. PHP attaches throttle on the group and inv.scope on the route (throttle first). Independently confirms 06-REVIEW.md CR-01; no test asserts 429 on the deny path."
|
||||
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: fonoteka.go/plugins/golem15/fonoteka/routes.go
|
||||
issue: "Use(\"inv_token\", \"inv.scope:read\", \"throttle:fonoteka-api-token\") inverts PHP throttle-then-scope; InvScope short-circuits before the bucket"
|
||||
- path: summercms.go/surf/router.go
|
||||
issue: "wrap() last-to-first is the correct onion; the call-site order is wrong"
|
||||
- path: fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go
|
||||
issue: "missing/invalid token writes 401 and returns; next (throttle) never runs"
|
||||
- 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:
|
||||
- "Reorder the personal-token group to inv_token, throttle:fonoteka-api-token, inv.scope:read (inv_token must stay before throttle so tok:<id> is set)"
|
||||
- "Add a test that 61 unauthenticated requests to assembled GET /api/v1/fonoteka/genres return 429 {\"message\":\"Too Many Attempts.\"}"
|
||||
- "Use the same order on any future inv.scope + throttle route"
|
||||
- "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 reachable without auth"
|
||||
- truth: "Public/onboarding groups have reachable unauthenticated handlers."
|
||||
addressed_in: "Phase 13"
|
||||
evidence: "Phase 13 goal ports onboarding/public/invitation routes and their public rate-limit buckets. Phase 6 declared empty group builders per D-15 (zero routes, not 501 shells)."
|
||||
- truth: "unknown and malformed ids on ownership-scoped resources both return 404"
|
||||
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 ports Collections and Albums ownership-scoped endpoints. The router primitive already 404s (surf/params.go constrain, TestTypedIDRouteReturns404)."
|
||||
- truth: "user-supplied cover URL fetch call sites (manual cover URL, Discogs cover)"
|
||||
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: "Phase 12 cover handling; Phase 14 Discogs client. 06-04 D-13 ships fetchguard only — no ManualCoverUrlFetcher or CoverImporter call site this phase."
|
||||
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 (JWT, personal token, public/onboarding) share handlers with correct route subsets, named rate-limit buckets are ported 1:1, and OAuth/RFC routes are structurally exempted from any house envelope or error middleware. Security-load-bearing — auth guard registry, rate limiting and the SSRF-guarded outbound fetch helper all live here; apply the security-review agent.
|
||||
**Verified:** 2026-09-19T19:45:00Z
|
||||
**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:** No — initial verification
|
||||
**Re-verification:** Yes — 06-06 closes the prior middleware-order gap, but current code-review findings expose goal-level defects.
|
||||
|
||||
**MVP mode:** ROADMAP marks this phase `mode: mvp`, but `gsd-sdk query user-story.validate` returns `valid=false` (goal is a technical contract, not `As a …, I want to …, so that ….`). User Flow Coverage is omitted; verification follows the five roadmap success criteria.
|
||||
|
||||
`06-REVIEW.md` (issues_found, 1 critical / 7 warning) is advisory. CR-01 is listed as a gap only because it independently falsifies HTTP-04 / SC2 in the live route table.
|
||||
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
|
||||
|
||||
@@ -49,142 +89,118 @@ deferred:
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Same handler serves JWT `GET /_fonoteka/api/v1/genres` and personal-token `GET /api/v1/fonoteka/genres`; public/onboarding without auth; unknown/malformed ids 404 on ownership-scoped resources | ✓ VERIFIED (deferred subclauses) | `routes.go` binds one `handler := controllers.ListGenres(p.app)` to both groups. `TestGenresSharedHandler` (Postgres) returns equal 200 bodies. Public/onboarding groups exist as empty builders (D-15) — deferred to Phase 13. Router `constrain` 404s malformed/unknown ids (`TestTypedIDRouteReturns404`) — ownership-scoped resources deferred to Phase 12. |
|
||||
| 2 | All five named rate-limit buckets are enforced with the documented keys and limits, including a route stacking two limiters | ✗ FAILED | Buckets exist with PHP names/limits/keys (`plugin.go` `Buckets()`: api-token 60 `tok:<id>` else IP, oauth-token 30 `oauthtok:<ip>`, oauth-register 30 `oauthreg:<ip>`, public-token 60 `pubtok:<token>`, public-ip 120 IP). Stacking is proven on a fixture (`TestFixedWindowLimiterStackedBuckets`). The live token genres route does **not** enforce `fonoteka-api-token` on 401/403: wrap last-to-first + `Use("inv_token", "inv.scope:read", "throttle:fonoteka-api-token")` puts InvScope outside throttle. InvScope returns on missing User. Zero tests fire 429 on that deny path. |
|
||||
| 3 | Response conventions: empty arrays `[]`, timestamps `+00:00`, tri-state booleans keep `null`, conditional keys omitted not nulled; OAuth-group route carries no house envelope, verified by route-registration inspection | ✓ VERIFIED | `wire.Slice` / `Time` / `TriBool` / `WriteJSON` tested (`wire/response_test.go`). ListGenres preallocates `make([]GenreAggregate, 0)` and delegates to `wire.WriteJSON`. Raw panic is bare 500 (`TestRawGroupPanicBare500`); house panic is opaque JSON. `GroupRaw("/", surf.Use(), …)` declared; `TestRawGroupHouseMiddlewareRefusedAtBuild` and `TestRawGroupRefusesHouseMiddlewareOnRealPlugins` fail boot if house MW is attached. Isolation test requires Raw on oauth patterns if present. |
|
||||
| 4 | Guarded outbound fetch helper rejects a non-allow-listed host and enforces a byte cap and timeout on a user-supplied cover URL fetch | ✓ VERIFIED (call sites deferred) | `fetchguard.Fetch`: allow-list before dial (`TestFetchAllowHostsRejectsUnknownHostBeforeDial`, dotted-suffix bypass test), dial-time `Control` + `isReservedOrPrivate` (`TestFetchPrivateIPBlockedInBothModes`), streaming `LimitReader` cap (`TestFetchTooLargeIsStreaming`), https-only, no redirects, 10MiB/10s defaults, typed reasons. No production caller this phase (D-13) — Phase 12/14. |
|
||||
| 5 | OpenAPI from swag annotations; `openapi-typescript` produces valid TS; CORS and JSON body-size limits match the PHP deployment | ✓ VERIFIED | `genre_controller.go` swag annotations → committed `fonoteka.go/docs/openapi.json` (OpenAPI 3.0.3, path `/_fonoteka/api/v1/genres`). `npx openapi-typescript@7.13.0 docs/openapi.json -o /dev/null` exit 0. CORS: `_fonoteka` no ACAO, `/api/v1/fonoteka/genres` `Access-Control-Allow-Origin: *` (`TestCORSPathScopedOnAssembledRouter`). Body limits operator-confirmed 134217728 / 134217728, no INTERIM (`TestProductionBodyLimitsOperatorConfirmed`). |
|
||||
| 6 | Auth guard registry: jwt + inv_token resolve to one `bouncer.User`; 401/403 token bodies match PHP; oauth guard not registered | ✓ VERIFIED | `bouncer.Registry` + `NewJWTGuard` + `TokenGuard`. `TestGenresSharedHandler` missing/unknown token → 401 `{"error":"Invalid token"}`; write-only token → 403 `{"error":"Missing required scope: read"}`. Duplicate/unknown/neither-interface fail boot. Grep of `Register(` finds only `"jwt"` and `"inv_token"`. |
|
||||
| 7 | Parameterized (`name:param`) middleware is a surf factory, not a fixed name table | ✓ VERIFIED | `RegisterMiddlewareFactory` for `throttle`, `body.limit`, plugin `inv.scope`. `strings.Cut` in `wrap()`. |
|
||||
| 8 | Fixed-window limiter matches Laravel tooManyAttempts-before-hit; success headers and 429 headers; PublicShareHeaders 429 body; ClientIP trusted-proxy rules | ✓ VERIFIED | `TestFixedWindowLimiterMemoryStoreWindow`, `FirstHitWins`, `SuccessHeaders`, `TooManyAttemptsHeaders`, `TestPublicShareHeadersRewrites429`, `TestClientIPRejectsSpoofedXFF`. In-process `MemoryStore`; type is `FixedWindowLimiter` (pre-existing `Limiter` interface left in place). |
|
||||
| 9 | Raw group refuses house-envelope middleware at registration; `HasHouseMiddleware` is the only house registration path | ✓ VERIFIED | `inv.must-change-password` only in `HouseMiddlewares()` (plugin.go:73-76), not `Middlewares()`. `TestHouseMiddlewareCapabilityOnRealPlugins` boots; `TestRawGroupRefusesHouseMiddlewareOnRealPlugins` fails boot when a raw group names it. |
|
||||
| 10 | Full assembled route table: zero jwt.auth on `/api/v1/fonoteka`, zero inv_token / inv.scope on `/_fonoteka/api/v1` | ✓ VERIFIED | `TestFullRouteTableAuthGroupMutualExclusivity` walks `BuildRouter` `Routes()` for real `golem15.user` + `golem15.fonoteka`. `route:list` calls `Router.Routes()` (`surf/routelist_command.go`). |
|
||||
| 11 | Every T-06-01 through T-06-18 and T-06-SC is mapped in `06-SECURITY-REVIEW.md` | ✓ VERIFIED | Exactly 19 `\| T-06-` table rows. Mitigate rows name real tests; accept rows restate plan rationales. |
|
||||
| 12 | `go vet` / tests green; both genres routes `status: ported` and parity corpus passing | ✓ VERIFIED | `go vet` and `go test -race -short` green in summercms.go and fonoteka plugin modules. `TestParityCorpus` ok. Manifest: both genres ids `status: ported`. |
|
||||
| 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:** 11/12 truths verified (3 additional clauses deferred to later phases; not counted as failures)
|
||||
**Score:** 7/12 truths verified
|
||||
|
||||
### Deferred Items
|
||||
|
||||
| # | Item | Addressed In | Evidence |
|
||||
|---|------|-------------|----------|
|
||||
| 1 | public/onboarding groups reachable without auth | Phase 13 | Phase 13 ports onboarding/public/invitation routes. Empty group builders in `routes.go:24-29`. |
|
||||
| 2 | unknown/malformed ids 404 on ownership-scoped resources | Phase 12 | Phase 12 Collections/Albums. Router primitive already in `surf/params.go` / `TestTypedIDRouteReturns404`. |
|
||||
| 3 | fetchguard call sites (manual cover URL, Discogs cover) | Phase 12 / 14 | 06-04 D-13: helper and tests only. |
|
||||
| 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
|
||||
|
||||
gsd-sdk `verify.artifacts` reported missing files because PLAN paths keep the `summercms.go/` / `fonoteka.go/` prefix while CWD is already the framework repo. Files were verified at the stripped paths.
|
||||
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` | Named Guard registry | ✓ VERIFIED | Register + Middleware; duplicate/unknown/neither fail with plugin+name |
|
||||
| `bouncer/guard.go` | Guard, CredentialGuard, UnauthorizedWriter | ✓ VERIFIED | Interfaces as planned |
|
||||
| `fonoteka.go/.../token_guard.go` | inv_token hash lookup, expiry, revocation, last-used | ✓ VERIFIED | SHA-256 lookup, `IsUsable()`, stamps last_used once. Wired from plugin Boot |
|
||||
| `fonoteka.go/.../token_scope.go` | InvScope 401/403 PHP bodies | ✓ VERIFIED | Wired via `MiddlewareFactories()` `inv.scope` |
|
||||
| `surf/limiter.go` | FixedWindowLimiter, Bucket, throttle factory | ✓ VERIFIED | Wired in `BuildRouter` `RegisterMiddlewareFactory("surf", "throttle", …)` |
|
||||
| `surf/limiter_store.go` | Store + MemoryStore | ✓ VERIFIED | Hit / TooManyAttempts / AvailableIn; sweep tested |
|
||||
| `surf/clientip.go` | ClientIP + TrustedProxies | ✓ VERIFIED | Used by bucket Key closures |
|
||||
| `fonoteka.go/.../public_share_headers.go` | 429 rewrite + robots/cache headers | ✓ VERIFIED | Registered as `public.share-headers` |
|
||||
| `surf/routetable.go` | RouteInfo + Routes() | ✓ VERIFIED | Used by isolation test and route:list |
|
||||
| `pact/capabilities.go` | GroupRaw, HasHouseMiddleware | ✓ VERIFIED | Implemented by fonoteka Plugin |
|
||||
| `wire/response.go` | WriteJSON, Time, TriBool, Slice | ✓ VERIFIED | ListGenres delegates WriteJSON |
|
||||
| `surf/cors.go` | Path-scoped CORS | ✓ VERIFIED | Applied in `compile()` via `pathScopedCORS` |
|
||||
| `fonoteka.go/docs/openapi.json` | Generated OpenAPI 3 | ✓ VERIFIED | 3.0.3; genres GET; openapi-typescript 0 |
|
||||
| `fetchguard/fetch.go` | Fetch entry point | ✓ VERIFIED (intentionally unwired to app) | D-13: helper only |
|
||||
| `fetchguard/policy.go` | Policy, Reason, Defaults | ✓ VERIFIED | Closed Reason set |
|
||||
| `fetchguard/ip.go` | PHP CIDR table including CGNAT + metadata | ✓ VERIFIED | 100.64.0.0/10, 169.254.0.0/16 |
|
||||
| `06-SECURITY-REVIEW.md` | Threat-to-test map | ✓ VERIFIED | 19 rows |
|
||||
| --- | --- | --- | --- |
|
||||
| `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 | Via | Status | Details |
|
||||
| ---- | --- | --- | ------ | ------- |
|
||||
| `fonoteka/.../routes.go` | `genre_controller.go` | same `ListGenres(p.app)` value on both groups | WIRED | lines 10-16, two `g.Get("/genres", handler)` |
|
||||
| `token_scope.go` | `bouncer/context.go` | `bouncer.User` / `Credential` | WIRED | lines 16-21 |
|
||||
| `token_guard.go` | `models/api_token.go` | `token_hash = ?`, `IsUsable()` | WIRED | lines 46-47 |
|
||||
| `fonoteka/plugin.go` | `surf/limiter.go` | `Buckets() map[string]surf.Bucket` | WIRED | lines 102-148; `_ surf.BucketProvider` |
|
||||
| `surf/router.go` | `surf/limiter.go` | `RegisterMiddlewareFactory(..., "throttle", ...)` | WIRED | BuildRouter lines 419-424 |
|
||||
| `routes.go` | `surf/router.go` | `GroupRaw(` | WIRED | line 33 |
|
||||
| `plugin.go` | `pact/capabilities.go` | `HouseMiddlewares()` | WIRED | lines 73-76 |
|
||||
| `routelist_command.go` | `routetable.go` | `.Routes()` | WIRED | line 24 |
|
||||
| `genre_controller.go` | `wire/response.go` | `wire.WriteJSON` | WIRED | writeJSON wrapper line 149 |
|
||||
| `fetch.go` | `ip.go` | DialContext Control → `isReservedOrPrivate` | WIRED | fetch.go:67-69, 145-163 |
|
||||
| `routes_isolation_test.go` | `routetable.go` | `rt.Routes()` | WIRED | isolation test line 33 |
|
||||
| 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 | Data Variable | Source | Produces Real Data | Status |
|
||||
| -------- | ------------- | ------ | ------------------ | ------ |
|
||||
| ListGenres | `GenreList.Data` | GORM query on `golem15_fonoteka_genres` + tenant album counts | Yes — `TestGenresSharedHandler` non-empty equal bodies | ✓ FLOWING |
|
||||
| FixedWindowLimiter | Store counters | `MemoryStore.Hit` / `TooManyAttempts` | Yes — limiter tests increment and 429 | ✓ FLOWING |
|
||||
| CORS | `http.cors.paths` | `fonoteka.go/config/http.yaml` via `LoadCORSConfig` | Yes — assembled-router CORS test | ✓ FLOWING |
|
||||
| Body limits | `default_bytes` | config 134217728 | Yes — operator-confirmed test | ✓ FLOWING |
|
||||
| fetchguard | response body | guarded HTTPS GET | Yes in tests (httptest TLS); no app caller | ✓ FLOWING (tests) / deferred prod |
|
||||
| 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 vet+test | `go vet ./bouncer/... ./surf/... ./wire/... ./fetchguard/...` and `go test … -race -short` | exit 0 | ✓ PASS |
|
||||
| Full summercms.go `-race -short` | `go test ./... -race -short` | all ok | ✓ PASS |
|
||||
| Shared handler (Postgres) | `go test ./plugins/golem15/fonoteka -run TestGenresSharedHandler` | ok 4.7s | ✓ PASS |
|
||||
| Isolation + CORS + buckets boot | `-run TestFullRouteTableAuthGroupMutualExclusivity\|TestCORSPathScopedOnAssembledRouter\|TestAllRouteGroupsBoot` | ok | ✓ PASS |
|
||||
| Token guard (Postgres) | `go test ./plugins/golem15/fonoteka/classes/auth/` | ok | ✓ PASS |
|
||||
| Body limits config | `go test . -run TestProductionBodyLimitsOperatorConfirmed` | ok | ✓ PASS |
|
||||
| Parity corpus | `go test ./parity/... -run TestParityCorpus` | ok 5.3s | ✓ PASS |
|
||||
| openapi-typescript | `npx openapi-typescript@7.13.0 docs/openapi.json -o /dev/null` | ✨ 7.13.0, 38.3ms, exit 0 | ✓ PASS |
|
||||
| Unauthenticated token-route 429 | code trace of wrap + InvScope; no test exists | throttle not reached on 401 | ✗ FAIL |
|
||||
| --- | --- | --- | --- |
|
||||
| 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 `scripts/*/tests/probe-*.sh` in this phase. `fonoteka.go/scripts/check-openapi.sh` is the OpenAPI pipeline (swag → swagger2openapi → openapi-typescript); the TypeScript step was run directly against the committed document (PASS above).
|
||||
|
||||
| Probe | Command | Result | Status |
|
||||
| ----- | ------- | ------ | ------ |
|
||||
| _(none declared)_ | — | — | SKIP |
|
||||
No probe is declared and no `scripts/*/tests/probe-*.sh` exists.
|
||||
|
||||
### Requirements Coverage
|
||||
|
||||
Phase plans declare HTTP-03, HTTP-04, HTTP-05, HTTP-06, HTTP-07, HTTP-08, HTTP-09. REQUIREMENTS.md maps those seven IDs to Phase 6. HTTP-01/HTTP-02 are not in this phase's plan `requirements:` (HTTP-01 404 primitive exists from routing; HTTP-02 pipeline order is pre-existing). No orphaned Phase 6 IDs.
|
||||
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 | Source Plan | Description | Status | Evidence |
|
||||
| ----------- | ---------- | ----------- | ------ | -------- |
|
||||
| HTTP-03 | 06-01, 06-05 | Three mutually exclusive auth groups share handlers | ✓ SATISFIED (public routes deferred) | Shared ListGenres; isolation test; empty public groups D-15 |
|
||||
| HTTP-04 | 06-02, 06-05 | Named buckets, stacking, 1:1 PHP port | ✗ BLOCKED | Buckets+stacking exist; live token route skips throttle on 401 |
|
||||
| HTTP-05 | 06-01 | Guard registry → one current-user accessor | ✓ SATISFIED | Registry + jwt + inv_token; oauth not registered |
|
||||
| HTTP-06 | 06-03 | Response conventions; no house envelope on OAuth | ✓ SATISFIED | wire helpers; GroupRaw + refusal tests |
|
||||
| HTTP-07 | 06-04 | Guarded outbound fetch | ✓ SATISFIED | fetchguard tests; call sites later |
|
||||
| HTTP-08 | 06-03 | swag → OpenAPI → openapi-typescript | ✓ SATISFIED | committed openapi.json; npx exit 0 |
|
||||
| HTTP-09 | 06-03 | CORS and JSON body size match PHP | ✓ SATISFIED | path-scoped CORS tests; 134217728 confirmed. (MaxBytesError→413 is latent; no POST body handler this phase — see anti-patterns) |
|
||||
| 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 | Line | Pattern | Severity | Impact |
|
||||
| ---- | ---- | ------- | -------- | ------ |
|
||||
| `fonoteka.go/.../routes.go` | 14-16 | throttle after InvScope on live route | 🛑 Blocker | Rate-limit bypass for unauthenticated personal-token traffic (SC2 / HTTP-04 / CR-01) |
|
||||
| `surf/limiter.go` | 91-93 | fail-open if Key/store/resolve nil | ⚠️ Warning | All five production buckets set Key; latent if a plugin registers a nil Key (06-REVIEW WR-02) |
|
||||
| `surf/bodylimit.go` | 10-18 | MaxBytesReader without mapping `*http.MaxBytesError` to 413 | ⚠️ Warning | Current handlers are GET; next POST will 500 unless the handler converts (WR-03). `upload_bytes` loaded and unread |
|
||||
| `surf/clientip.go` | 75-78 | TrustedProxies skips bare IPs / malformed CIDRs | ⚠️ Warning | Config list is empty today; a copied `127.0.0.1` would silently no-op (WR-04) |
|
||||
| `fetchguard/ip.go` | 5-45 | PHP-identical table; no NAT64/6to4 unwrap | ℹ️ Info | Matches PHP 1:1 (plan requirement). CONTEXT allowed stricter; not looser. No caller this phase (WR-05) |
|
||||
| `fonoteka.go/.../token_guard.go` | 51 | last_used_ip from RemoteAddr, not ClientIP | ℹ️ Info | Parity/audit quality; not an SC (WR-01) |
|
||||
| `fonoteka.go/.../routes.go` | 18-33 | Empty group builders | ℹ️ Info | Intentional D-15; not stubs — zero routes registered |
|
||||
| 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 `TBD` / `FIXME` / `XXX` debt markers in phase packages.
|
||||
|
||||
**Confirmation-bias pass:** (1) HTTP-04 is only partially met — buckets register, deny-path does not consume them. (2) `TestAllRouteGroupsBoot` proves Assemble, not 429. (3) No test covers unauthenticated 429 on `/api/v1/fonoteka/genres`.
|
||||
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 remaining. Plan 06-03's body-size `<human-check>` was closed 2026-09-19 (128M / 128M / 128M → 134217728); config and `TestProductionBodyLimitsOperatorConfirmed` record that.
|
||||
None. The production body-size checkpoint is already recorded. Current failures are programmatically observable and require code/test changes.
|
||||
|
||||
### Gaps Summary
|
||||
|
||||
The phase delivers the guard registry, dual-group shared genres handler, five named buckets, raw-group enforcement, wire helpers, path-scoped CORS, operator-confirmed body limits, OpenAPI pipeline, and the SSRF fetch helper. The phase **goal** is not fully achieved because HTTP-04's 1:1 rate-limit port fails on the one live personal-token route: `inv.scope` sits outside `throttle` in the onion, so missing/invalid bearers never hit `fonoteka-api-token`. That is a security control bypass (unlimited 401 spray plus a SHA-256 + SQL lookup per request), not a later-phase item.
|
||||
|
||||
Fix is a middleware-order change plus a deny-path 429 test. Public/onboarding handlers, ownership-scoped 404 resources, and fetchguard call sites are explicitly later-phase work and are listed under deferred, not gaps.
|
||||
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-19T19:45:00Z_
|
||||
_Verifier: Claude (gsd-verifier)_
|
||||
_Verified: 2026-09-20T11:53:11Z_
|
||||
_Verifier: the agent (gsd-verifier)_
|
||||
|
||||
Reference in New Issue
Block a user