From 6e9188b5c7feaae0d6cffc8a558d6de65fa1ffb3 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 21 Sep 2026 14:49:52 +0200 Subject: [PATCH] docs(06): record verification gaps --- .../06-VERIFICATION.md | 272 ++++++++++-------- 1 file changed, 145 insertions(+), 127 deletions(-) diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md index af10213..db00beb 100644 --- a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md @@ -1,87 +1,90 @@ --- phase: 06-http-routing-auth-groups-and-rate-limiting -verified: 2026-09-20T11:53:11Z +verified: 2026-09-21T12:44:44Z status: gaps_found -score: 7/12 must-haves verified +score: 8/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." +mvp_mode_note: "ROADMAP mode is mvp but the technical goal is not a valid User Story (user-story.validate valid=false); this requested re-verification retains the prior technical goal contract." re_verification: previous_status: gaps_found - previous_score: 11/12 + previous_score: 7/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." + - "Fixed-window admission is atomic and anonymous inline keys no longer trust Host or throttle parameters." + - "NAT64 /96, local-use NAT64 /48, and 6to4 embedded-private addresses are classified at dial time." + - "House and raw recovery buffer and discard partial responses before emitting clean 500 contracts." + - "InvScope 401/403 bodies use wire.WriteJSON and contain no trailing newline." + gaps_remaining: [] regressions: [] gaps: - - truth: "All five named buckets and inline throttles enforce their limits and documented keys under concurrent traffic." + - truth: "The configured JSON body-size limit constrains the entire non-raw request pipeline before any named middleware can consume the body." 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." + reason: "Router.wrap installs bodyLimit around only the terminal constrained handler, then wraps authentication/plugin middleware outside it. A named middleware can read an oversized body in full before MaxBytesReader is installed." 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." + issue: "Lines 353-394 apply bodyLimit before the named-middleware loop, making the cap inner at runtime." + - path: "surf/bodylimit_test.go" + issue: "Existing tests read only in the terminal handler and do not cover body-consuming middleware." 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." + - "Place the selected body limit outside all named middleware while keeping it inside recovery." + - "Add an oversized-body regression whose named middleware reads r.Body." + - truth: "Named and inline rate-limit definitions fail boot when their store, key, maximum, or duration cannot enforce a real limit." status: failed - reason: "InvScope uses json.Encoder.Encode, which appends a newline; the tests hide it with strings.TrimSpace." + reason: "RegisterBucket accepts nil keys, non-positive maxima, and non-positive decay; inline minute multiplication can overflow; Middleware explicitly passes through on nil Key/store. The adversarial probe registered a zero-decay Max=1 bucket with no error and both requests returned 204." 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." + - path: "surf/limiter.go" + issue: "Lines 51-60, 63-79, 84-96, and 139-153 accept or fail open on invalid enforcement state." + - path: "surf/limiter_test.go" + issue: "Unknown/duplicate names are covered, but invalid named definitions, nil store, and duration overflow are not." missing: - - "Use wire.WriteJSON (or equivalent no-newline writer)" - - "Assert exact 401 and 403 bytes" + - "Reject nil store and named buckets with nil Key, Max < 1, or Decay <= 0." + - "Overflow-check inline minute-to-duration conversion and remove runtime fail-open behavior." + - "Add boot-failure tests for every invalid definition." + - truth: "The outbound fetch helper rejects all private and non-public special-use destinations, including scoped IPv6 link-local addresses, at the actual dial boundary." + status: failed + reason: "The hand-written table allows special-use IPv4 ranges such as 198.18.0.0/15, 192.0.0.0/24, and 240.0.0.0/4. Zoned fe80::/10 addresses also evade Prefix.Contains. PublicOnlyMode probes reached dialing and returned network_error, not private_ip, for all four examples." + artifacts: + - path: "fetchguard/ip.go" + issue: "Lines 5-23 cover only a subset of non-public IPv4/IPv6 space; zoned IPv6 is not normalized before prefix checks." + - path: "fetchguard/fetch.go" + issue: "Lines 145-163 pass the parsed zoned address into the incomplete classifier." + - path: "fetchguard/ip_test.go" + issue: "No IANA special-use boundary table or scoped-link-local regression exists." + missing: + - "Classify the complete intended non-public/special-use IPv4 and IPv6 ranges." + - "Reject or safely normalize scoped IPv6 before classification." + - "Add dial-time tests for benchmark/reserved ranges and fe80:: addresses with zones." + - truth: "The Phase 6 security review truthfully reports every open security threat." + status: failed + reason: "06-SECURITY-REVIEW.md says verified with 26/26 closed and zero open, but independent source review and executable probes demonstrate open body-limit, limiter fail-open, and SSRF-bypass threats." + artifacts: + - path: ".planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md" + issue: "Frontmatter and verdict claim zero open threats despite reproducible open security gaps." + missing: + - "Keep the review open until the implementation gaps and adversarial tests are resolved." + - "Add threat rows/findings for body-consuming middleware, invalid limiter definitions, and remaining non-public IP representations." 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." + evidence: "Phase 13 goal explicitly ports onboarding/public/invitation routes with their public buckets; Phase 6 currently declares empty builders only." - 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." + evidence: "Phase 12 owns Collections and Albums; Phase 6 only supplies and tests the constraint primitive." + - truth: "A production OAuth/RFC route is present in a raw group and can be inspected for absence of house middleware." + addressed_in: "Phase 8" + evidence: "Phase 8 goal owns the OAuth2.1 endpoints; Phase 6 has an empty GroupRaw and synthetic framework/route-table tests." - 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." + evidence: "Phase 12 owns cover handling and Phase 14 owns the Discogs integration; no production caller imports fetchguard yet." --- # 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 +**Verified:** 2026-09-21T12:44:44Z **Status:** gaps_found -**Re-verification:** Yes — 06-06 closes the prior middleware-order gap, but current code-review findings expose goal-level defects. +**Re-verification:** Yes — all four prior implementation gaps are closed; new adversarial checks expose four blocking concerns. -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. +ROADMAP marks this phase `mode: mvp`, but `gsd-sdk query user-story.validate` reports `valid=false` because the goal is not in User Story form. Consistent with the prior technical verification and the explicit re-verification request, this report evaluates the supplied technical contract and records the metadata discrepancy. ## Goal Achievement @@ -89,118 +92,133 @@ ROADMAP marks this phase `mode: mvp`, but `gsd-sdk query user-story.validate` re | # | 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. | +| 1 | JWT and personal-token groups share the genres handler; implemented subsets are mutually exclusive | ✓ VERIFIED (later public groups deferred) | `routes.go:10-16` binds one `handler` value twice; `TestGenresSharedHandler` and `TestFullRouteTableAuthGroupMutualExclusivity` pass. Public/onboarding builders remain empty and are deferred to Phase 13. | +| 2 | Five named buckets and inline throttles enforce documented live limits/keys, including stacking | ✓ VERIFIED for valid definitions | `Plugin.Buckets()` matches PHP's five names, values, and key composition; atomic, stacking, Host-rotation, shared-inline-budget, and request-61 tests pass. Invalid definitions still fail open under truth 8. | +| 3 | Response conventions and raw/house panic behavior hold | ✓ VERIFIED | `wire` tests cover `[]`, `+00:00`, tri-state null, no-newline JSON; partial-write panic tests prove clean opaque/bare 500s. Actual OAuth endpoints are deferred to Phase 8. | +| 4 | Fetch helper is an SSRF boundary with allow-list, non-public-IP rejection, cap, and timeout | ✗ FAILED | Transition fixes pass, but `198.18.0.1`, `192.0.0.1`, `240.0.0.1`, and `[fe80::1%eth0]` reach dialing and map to `network_error`, not `private_ip`. | +| 5 | OpenAPI/type validation, path CORS, and production body limits exist and enforce the deployment contract | ✗ FAILED | OpenAPI 3 artifact and CORS/config values exist; body-consuming middleware bypasses the 134217728-byte cap because the limiter is installed inside named middleware. | +| 6 | Guard registry unifies JWT/personal-token users and preserves exact error contracts | ✓ VERIFIED | One `bouncer.User` accessor; JWT legacy equivalence, token hash/usability/stamp, and exact InvScope 401/403 tests pass. | +| 7 | `name:param` middleware resolves through factories | ✓ VERIFIED | `Router.wrap` uses `strings.Cut`; inv.scope, throttle, and body.limit are wired through factories and factory tests pass. | +| 8 | Fixed-window limiter is a safe enforcement primitive | ✗ FAILED | Atomic admission and stable keys are fixed, but malformed named buckets, nil store/key, and overflowing durations can disable enforcement without boot failure. | +| 9 | Raw routes refuse house-tagged middleware and use bare recovery | ✓ VERIFIED | Capability registration, raw refusal, sticky raw inheritance, and bare panic recovery tests pass. | +| 10 | Route table excludes JWT middleware from token routes and vice versa | ✓ VERIFIED | Real assembled-router isolation tests pass. | +| 11 | Phase 6 security review accurately maps and closes current threats | ✗ FAILED | Threat IDs are mapped, but the zero-open verdict is contradicted by reproduced security gaps. | +| 12 | Phase packages and route regressions run cleanly | ✓ VERIFIED | Both repository gates plus nested fonoteka/user module tests pass under `-race -short`; vet passes. Tests omit the failing adversarial paths above. | -**Score:** 7/12 truths verified +**Score:** 8/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. | +| # | Item | Addressed In | Evidence | +| --- | --- | --- | --- | +| 1 | Reachable public/onboarding handlers | Phase 13 | Later goal explicitly names onboarding/public/invitation routes and public buckets. | +| 2 | Ownership-resource malformed/unknown ID parity | Phase 12 | Collections and Albums are implemented there. | +| 3 | Real OAuth/RFC raw routes | Phase 8 | OAuth2.1 endpoint implementation belongs there; Phase 6 supplies the raw-group contract. | +| 4 | Production fetchguard callers | Phase 12 / 14 | Manual cover and Discogs caller implementation belongs to those phases. | ### 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`. +The SDK's PLAN paths include repository prefixes and therefore report false missing files from this repository root; artifacts were resolved manually in `summercms.go` and 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. | +| `bouncer/registry.go`, `guard.go`, `context.go` | Named guards and unified request identity | ✓ VERIFIED | Substantive, plugin-boot wired, and tested; typed-nil registration remains a warning. | +| `../fonoteka.go/.../token_guard.go` | Real inv_token verification | ✓ VERIFIED | SHA-256 lookup, usability checks, one last-used stamp, user lookup, and credential propagation. | +| `../fonoteka.go/.../token_scope.go` | Exact PHP 401/403 scope gate | ✓ VERIFIED | Uses `wire.WriteJSON`; untrimmed byte tests pass. | +| `surf/limiter.go`, `limiter_store.go` | Named/inline fixed-window enforcement | ✗ DEFECTIVE | Real buckets work and admission is atomic; malformed enforcement configuration fails open. | +| `surf/routetable.go`, `pact/capabilities.go` | Route/raw inspection | ✓ VERIFIED | Used by router, route:list, and isolation tests. | +| `wire/response.go` | JSON/time/nullable helpers | ✓ VERIFIED | Used by genre controller and independently tested. | +| `surf/cors.go`, `bodylimit.go` | PHP CORS/body caps | ✗ DEFECTIVE | CORS and values flow; body cap is installed after named middleware has already run. | +| `fetchguard/fetch.go`, `policy.go`, `ip.go` | Guarded outbound fetch | ✗ DEFECTIVE | Host/scheme/cap/timeout/redirect and transition checks exist; remaining non-public representations are allowed to dial. | +| `../fonoteka.go/docs/openapi.json` | Generated OpenAPI 3 artifact | ✓ VERIFIED (limited surface) | Valid OpenAPI 3.0.3 with the annotated live JWT genres route; personal-token path/security schemes are absent. | +| `06-SECURITY-REVIEW.md` | Current threat verdict | ✗ STALE | Claims 26/26 closed despite currently reproducible open threats. | ### 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. | +| From | To | Via | Status | Details | +| --- | --- | --- | --- | --- | +| `routes.go` | `controllers.ListGenres` | Same handler variable on JWT/token groups | WIRED | Both live routes share the handler. | +| `routes.go` | guard → limiter → scope | Ordered middleware list | WIRED | 1-60 unauthenticated requests are 401; request 61 is exact 429. | +| `plugin.go` | five limiter buckets | `surf.BucketProvider` | WIRED-BUT-UNSAFE | Five valid definitions register; invalid definitions are not rejected. | +| `token_guard.go` | `models.ApiToken` | Hash lookup/usability/stamp | WIRED | Real DB-backed tests cover the data path. | +| `token_scope.go` | bouncer context / wire | user+credential reads and WriteJSON | WIRED | Exact denial and allowed-scope tests pass. | +| `routes.go` | `GroupRaw` | Empty OAuth landing group | PARTIAL / DEFERRED | Framework contract is wired; there is no production OAuth route until Phase 8. | +| `fetch.go` | `ip.go` | DialControl classification | WIRED-BUT-INCOMPLETE | Actual dial target is checked, but the classifier is not complete and mishandles zones. | +| `router.go` | `bodylimit.go` | MaxBytesReader wrapper | MISORDERED | Cap wraps only the terminal handler, not named middleware. | +| `genre_controller.go` | `wire.WriteJSON` / GORM | DB query to response DTO | WIRED | Real dynamic genre data flows 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 | +| Artifact | Data Variable | Source | Produces Real Data | Status | +| --- | --- | --- | --- | --- | +| Genres handler | `rows` | GORM genre/count query scoped by user/collection | Yes | ✓ FLOWING | +| Token guard | `token`, `user` | GORM hash/user queries | Yes | ✓ FLOWING | +| Limiter | counter entry | MemoryStore keyed by resolver | Yes for valid definitions | ⚠️ FLOWING, CONFIG FAIL-OPEN | +| CORS/body config | `corsCfg`, `defaultBytes` | production YAML through compass | Yes | ✗ BODY CAP MISORDERED | +| Fetch helper | dial address / response stream | HTTPS transport | Yes in tests; no production caller yet | ✗ CLASSIFIER INCOMPLETE / CALLER DEFERRED | ### 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 | +| Prior gap repairs | Targeted `go test` for atomic limiter, inline keys, transitions, partial panic recovery, raw refusal, and InvScope under `-race` | All selected packages `ok` | ✓ PASS | +| Framework repository | `go test ./... -count=1 -race -short` | All packages pass | ✓ PASS | +| Fonoteka root repository | `go test ./... -count=1 -race -short` | Root/parity packages pass | ✓ PASS | +| Nested app modules | `go test ./plugins/golem15/fonoteka/... ./plugins/golem15/user/... -count=1 -race -short` | All packages pass | ✓ PASS | +| Vet | `go vet ./...` in both roots plus nested app modules | Exit 0 | ✓ PASS | +| Body-limit boundary | `/tmp/phase06_bodylimit_probe.go` with limit 4 and 10-byte body | middleware consumed 10; status 200; no handler error | ✗ FAIL | +| Invalid limiter definition | `/tmp/phase06_adversarial_probe.go` zero-decay Max=1 bucket | registration error nil; statuses `[204 204]` | ✗ FAIL | +| Special-use/scoped IP rejection | `/tmp/phase06_fetch_probe.go` | all four inputs returned `network_error`, not `private_ip` | ✗ FAIL | +| Semantic route conflict | `/tmp/phase06_adversarial_probe.go` | ServeMux conflict panicked instead of returning an error | ⚠️ WARNING | ### Probe Execution -No probe is declared and no `scripts/*/tests/probe-*.sh` exists. +No phase probe is declared and no `scripts/*/tests/probe-*.sh` exists. Step 7c is not applicable. ### 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`. +All seven requirement IDs declared across Phase 6 plans are present in REQUIREMENTS.md and mapped to Phase 6. No additional Phase 6 requirement ID is orphaned. -| 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. | +| Requirement | Source Plan | Description | Status | Evidence | +| --- | --- | --- | --- | --- | +| HTTP-03 | 06-01, 06-05, 06-11 | Three auth groups share handlers with subsets | ◐ PARTIAL / DEFERRED | JWT/token sharing and exclusivity pass; public/onboarding handlers are Phase 13. | +| HTTP-04 | 06-02, 06-05..07, 06-11 | Named/inline rate limiting and stacking | ✗ BLOCKED | Five live definitions and atomic traffic behavior pass; invalid definitions can silently disable enforcement. | +| HTTP-05 | 06-01, 06-05, 06-10, 06-11 | Named guards share current-user accessor | ✓ SATISFIED | JWT and inv_token use one registry/context accessor; exact denial tests pass. | +| HTTP-06 | 06-03, 06-05, 06-09, 06-10, 06-11 | Response conventions and raw OAuth boundary | ✓ SATISFIED / DEFERRED ROUTES | Helpers and raw infrastructure pass; actual OAuth handlers are Phase 8. | +| HTTP-07 | 06-04, 06-05, 06-08, 06-11 | Guarded outbound fetch | ✗ BLOCKED | Host/cap/timeout/redirect and transition checks exist, but additional non-public/scoped targets are dialed. | +| HTTP-08 | 06-03, 06-05, 06-11 | Swag/OpenAPI and TypeScript validation | ✓ SATISFIED (warning) | Generation/validation script and valid OpenAPI 3 artifact exist; document covers only one live route. | +| HTTP-09 | 06-03, 06-05, 06-11 | PHP-matching CORS and body limits | ✗ BLOCKED | Values and path CORS match; named middleware can bypass the body cap. | + +REQUIREMENTS.md remains internally inconsistent for HTTP-04 and HTTP-06: their checklist entries are checked while traceability still says `In Progress`. This report does not modify requirements metadata. ### 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 | +| File | Line | Pattern | Severity | Impact | +| --- | --- | --- | --- | --- | +| `surf/router.go` | 353-394 | body cap inside named middleware | 🛑 Blocker | auth/plugin middleware can consume unbounded input | +| `surf/limiter.go` | 63-96 | invalid bucket accepted; runtime pass-through | 🛑 Blocker | security control silently disables | +| `surf/limiter.go` | 139-153 | unchecked duration multiplication | 🛑 Blocker | overflow can produce an ineffective window | +| `fetchguard/ip.go` | 5-54 | incomplete non-public table; zone-insensitive prefix checks | 🛑 Blocker | user URL can dial non-public destinations | +| `06-SECURITY-REVIEW.md` | frontmatter/verdict | zero-open assertion contradicted by code | 🛑 Blocker | security sign-off is not auditable | +| `surf/router.go` | 338-349 | ServeMux semantic conflict can panic | ⚠️ Warning | plugin route input can crash assembly | +| `bouncer/registry.go` | 28-47 | typed-nil guard is accepted | ⚠️ Warning | later authentication can panic | +| `bouncer/jwt.go` | 146-159 | fractional float subject truncation | ⚠️ Warning | signed numeric subject can resolve another ID | +| `surf/router.go` | 489-493 | middleware factories constructed twice | ⚠️ Warning | allocation/side effects can duplicate at boot | +| `surf/router.go` | 440-443 | missing/malformed body config becomes zero | ⚠️ Warning | typo or missing config disables the cap | -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. +No unreferenced `TBD`, `FIXME`, or `XXX` marker was found in the Phase 6 implementation files. Scaffolding-generated placeholder text outside this phase is not a runtime stub. + +Disconfirmation pass: the passing body-limit tests exercise only the terminal handler; the passing fetchguard tests omit multiple non-public and zoned forms; the passing limiter tests cover unknown/duplicate names but not invalid definitions. These are precisely the cases where the green suite overstates the security contract. ### Human Verification Required -None. The production body-size checkpoint is already recorded. Current failures are programmatically observable and require code/test changes. +None. Production body-size values were previously operator-confirmed and are present in config. Current failures are programmatically reproducible and need implementation/test changes, not subjective UAT. ### 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. +All four gaps from the prior verification were genuinely repaired. Phase 6 still cannot pass because three security-load-bearing primitives remain unsafe at their boundaries: body limits do not constrain named middleware, limiter misconfiguration fails open, and fetchguard allows additional non-public/scoped destinations to reach the dial attempt. Consequently the zero-open security review is stale. Later-phase route and caller work remains deferred only where the roadmap explicitly owns it; it does not excuse these shared-infrastructure failures. --- -_Verified: 2026-09-20T11:53:11Z_ +_Verified: 2026-09-21T12:44:44Z_ _Verifier: the agent (gsd-verifier)_