20 KiB
20 KiB
phase: 06-http-routing-auth-groups-and-rate-limiting
plan: 06
type: execute
wave: 5
depends_on: ["06-05"]
files_modified:
- fonoteka.go/plugins/golem15/fonoteka/routes.go
- fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go
- .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md
autonomous: true
gap_closure: true
requirements: [HTTP-04]
must_haves:
truths:
- "An unauthenticated request to GET /api/v1/fonoteka/genres passes through inv_token, then throttle:fonoteka-api-token, then inv.scope:read; InvScope can no longer short-circuit before the limiter consumes the request"
- "The first 60 same-IP requests without credentials retain the PHP-compatible 401 Invalid token response, while request 61 returns HTTP 429 with exactly {"message":"Too Many Attempts."}"
- "A valid personal token is still resolved before the limiter, so the fonoteka-api-token bucket can retain its tok: key instead of collapsing valid credentials onto the IP fallback"
- "The phase security review covers Plan 06-06 and maps both newly identified rate-limit threats to the assembled-route regression and exact middleware-order invariant"
artifacts:
- path: "fonoteka.go/plugins/golem15/fonoteka/routes.go"
provides: "Correct live personal-token middleware declaration"
contains: 'surf.Use("inv_token", "throttle:fonoteka-api-token", "inv.scope:read")'
- path: "fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go"
provides: "Assembled-route unauthenticated deny-path rate-limit regression"
contains: "TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited"
- path: ".planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md"
provides: "Final Phase 6 threat register and evidence map including Plan 06-06"
contains: "T-06-21"
key_links:
- from: "fonoteka.go/plugins/golem15/fonoteka/routes.go"
to: "summercms.go/surf/router.go"
via: "surf.Use declaration consumed by Router.wrap's last-to-first middleware composition"
pattern: 'inv_token.*throttle:fonoteka-api-token.*inv.scope:read'
- from: "fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go"
to: "/api/v1/fonoteka/genres"
via: "surf.Assemble of the real golem15.user and golem15.fonoteka plugins followed by 61 same-IP requests"
pattern: "surf.Assemble|/api/v1/fonoteka/genres"
- from: ".planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md"
to: "fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go"
via: "T-06-21/T-06-22 proof entries naming the assembled-route regression and exact inv_token -> throttle -> inv.scope invariant"
pattern: "T-06-2[12].*TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited|inv_token.*throttle:fonoteka-api-token.*inv.scope:read"
Close the HTTP-04 verification gap by correcting the live personal-token genres middleware onion and proving the unauthenticated deny path consumes the 60/minute bucket.
From `/media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go`, run `go test ./plugins/golem15/fonoteka -run 'TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited|TestFullRouteTableAuthGroupMutualExclusivity' -count=1 -short`, then the commit-level gates `go vet ./...` and `go test ./... -short`. Confirm a source grep shows only the target live order for the personal-token genres route. Before commit and summary, verify `06-SECURITY-REVIEW.md` includes Plan 06-06, T-06-21/T-06-22, the named assembled-route test, the exact middleware invariant, valid-token `tok:` preservation, and an audit-trail total of 21 closed / 0 open.
Purpose: remove the rate-limit bypass that currently allows unlimited missing/invalid personal-token requests to reach InvScope's 401 path without incrementing the limiter.
Output: the exact inv_token -> throttle:fonoteka-api-token -> inv.scope:read declaration, an isolated assembled-router regression proving request 61 is denied with the PHP-compatible 429 body, and a final security-review map for T-06-21/T-06-22.
<execution_context> @/home/jin/.codex/get-shit-done/workflows/execute-plan.md @/home/jin/.codex/get-shit-done/templates/summary.md </execution_context>
@.planning/PROJECT.md @.planning/ROADMAP.md @.planning/REQUIREMENTS.md @.planning/STATE.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-CONTEXT.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-REVIEW.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-02-SUMMARY.md @.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-SUMMARY.md From `summercms.go/surf/router.go`: - `Router.wrap` iterates `rt.middleware` from the last name to the first, so the first listed middleware becomes the outermost runtime wrapper. - `Assemble(app, plugins) (http.Handler, error)` calls `BuildRouter`, which creates a new `FixedWindowLimiter` and `MemoryStore` for that assembled handler.From summercms.go/bouncer/registry.go:
- A guard error without
UnauthorizedWritercontinues the chain withoutbouncer.Userorbouncer.Credential, allowingInvScopeto own the exact 401 body per D-08.
From fonoteka.go/plugins/golem15/fonoteka/plugin.go:
fonoteka-api-tokenusestok:<id>whenbouncer.Credentialcontains*models.ApiToken; otherwise it falls back tosurf.ClientIPand enforces Max 60 over one minute.
In `routes_isolation_test.go`, add `TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` using the existing real-plugin activation pattern: create a fresh `bootConfig` and `backpack.App`, publish a fresh `bouncer.Registry`, activate exactly `golem15.user` and `golem15.fonoteka`, and register a test-local `inv_token` guard that always returns unauthenticated without implementing `UnauthorizedWriter`. Assemble once with `surf.Assemble`; this construction must be local to the test and not shared or parallelized, so `BuildRouter` supplies a fresh `FixedWindowLimiter`/`MemoryStore` and no other test can consume its bucket.
Send 61 independent `httptest.NewRequest` calls to `GET /api/v1/fonoteka/genres` through that assembled handler, with no Authorization header and the same explicit `RemoteAddr` on every request. Use a fresh recorder for every call. Assert requests 1 through 60 return 401 and, after `strings.TrimSpace`, the existing `{"error":"Invalid token"}` body, proving the throttle does not replace the normal deny response before exhaustion. Assert request 61 returns 429 and its raw body is exactly `{"message":"Too Many Attempts."}`, with `X-RateLimit-Limit: 60` and `X-RateLimit-Remaining: 0`. The test must exercise the production route, real middleware factory, real named bucket, and real limiter; do not hand-compose middleware or call `FixedWindowLimiter` directly.
Search production route declarations under `fonoteka.go/plugins/golem15/fonoteka` for any other route containing both `inv.scope:` and `throttle:`. Apply this same order only if another live production route exists in the current tree; do not change test-only boot probes, empty future groups, or unrelated route stacks speculatively.
Add T-06-21 to the Threat Register as the unauthenticated personal-token rate-limit bypass mitigation. Its proof must name `TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited`, state that it drives 61 same-IP requests through the real handler returned by `surf.Assemble`, and record that requests 1-60 keep the 401 Invalid token response while request 61 receives the exact 429 response. Record the exact runtime-order invariant as source declaration `inv_token` -> `throttle:fonoteka-api-token` -> `inv.scope:read`; do not summarize it as merely "middleware reordered."
Add T-06-22 to the Threat Register as the valid-credential bucket-isolation mitigation. Its proof must state that `inv_token` remains before `throttle:fonoteka-api-token`, so `bouncer.Credential` is populated before the bucket key closure and valid credentials retain `tok:<id>` keying rather than collapsing onto the IP fallback. Add corresponding Findings by Threat prose for T-06-21/T-06-22 that cites the executed route and regression evidence. Update the closed-threat total and Security Audit Trail to reflect all 21 reviewed IDs (`T-06-01` through `T-06-18`, `T-06-21`, `T-06-22`, and `T-06-SC`) with zero open; do not create new accepted risks because both new threats are mitigated. Keep this documentation-only update in Task 2's separate atomic commit after Task 1's implementation-and-test commit.
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
| unauthenticated client -> personal-token route | Missing or invalid bearer traffic is attacker-controlled and must consume a bounded per-IP budget before credential/scope denial returns |
| inv_token context -> limiter key resolver | Valid credentials must be available before rate limiting so users receive independent tok: budgets; unauthenticated traffic must safely fall back to ClientIP |
STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|---|---|---|---|---|
| T-06-21 | Denial of Service | /api/v1/fonoteka/genres middleware chain |
mitigate | Put throttle:fonoteka-api-token between inv_token and inv.scope:read; the assembled-route 61-request regression proves missing credentials cannot bypass the 60/minute deny-path budget |
| T-06-22 | Denial of Service | fonoteka-api-token key selection |
mitigate | Keep inv_token outermost so valid credentials populate bouncer.Credential before the bucket key closure runs and retain per-token tok:<id> isolation rather than sharing an IP bucket |
| T-06-SC | Tampering | package supply chain | accept | This gap closure performs no npm, pip, cargo, or Go module install and changes no dependency manifest; it uses only already-audited Phase 6 code |
| </threat_model> |
<source_coverage_audit>
| Source | ID | Coverage | Status |
|---|---|---|---|
| GOAL | Phase 6 success criterion 2 | Plans 06-01 through 06-05 provide the limiter/buckets; Task 1 repairs the failed live deny path and Task 2 records its final security evidence | COVERED |
| REQ | HTTP-04 | Task 1 makes the live named bucket enforceable on unauthenticated traffic and adds regression proof; Task 2 maps the closed threats | COVERED |
| REQ | HTTP-03, HTTP-05, HTTP-06, HTTP-07, HTTP-08, HTTP-09 | Already satisfied by executed Plans 06-01 through 06-05; no verification gap requests further work | COVERED (existing) |
| RESEARCH | Common Pitfall 5 | Exact inv_token -> throttle -> InvScope order plus no-double-lookup assembled behavior in Task 1, with T-06-21/T-06-22 evidence recorded by Task 2 | COVERED |
| CONTEXT | D-01, D-02, D-05, D-06, D-08 | Exact named bucket, 429 body, parameterized middleware, credential context, and InvScope ownership preserved in Task 1 and audited in Task 2 | COVERED |
| CONTEXT | D-03, D-04, D-07, D-09 through D-18 | Already implemented by executed Plans 06-01 through 06-05 and not reopened by the authoritative gap | COVERED (existing) |
| CONTEXT | Deferred Ideas | Explicitly excluded from this gap plan | EXCLUDED |
| </source_coverage_audit> |
<success_criteria>
- The live personal-token genres group declares
inv_token,throttle:fonoteka-api-token,inv.scope:readin that exact order. - A fresh assembled-router test proves requests 1-60 without credentials return the existing 401 body and request 61 from the same IP returns the exact PHP-compatible 429 body and exhausted-limit headers.
- The regression runs under
-shortwith no database or external service and cannot inherit limiter counters from another test. go vet ./...and repository-widego test ./... -shortpass from thefonoteka.gorepository root before commit/summary.06-SECURITY-REVIEW.mdcovers Plan 06-06 and closes T-06-21/T-06-22 with concrete links to the assembled-route regression, exact middleware order, and preservedtok:<id>keying.- HTTP-04's verification blocker and REVIEW CR-01 are directly closed without changing unrelated warnings or deferred work. </success_criteria>