diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 149edd8..37b128b 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -223,7 +223,7 @@ Plans: 4. The guarded outbound fetch helper rejects a non-allow-listed host and enforces a byte cap and timeout on a user-supplied cover URL fetch (manual cover URL, Discogs cover). 5. OpenAPI is generated from swaggo/swag annotations on handlers and `openapi-typescript` produces valid TypeScript types from it; CORS and JSON body-size limits match the PHP deployment. -**Plans**: 5 plans +**Plans**: 6 plans Plans: **Wave 1** *(parallel)* @@ -243,6 +243,10 @@ Plans: - [x] 06-05-PLAN.md — Full unit coverage across both repos, full route-table mutual-exclusivity test, 06-SECURITY-REVIEW.md +**Wave 5** *(gap closure; blocked on 06-05)* + +- [ ] 06-06-PLAN.md — Repair personal-token middleware order and prove unauthenticated request 61 is rate-limited + ### Phase 7: User plugin and authentication **Goal**: The user plugin is ported with registration, login, JWT issue/refresh, organizations, personal API tokens and the must-change-password lock. Security-load-bearing — password auth, token scope ceilings and the session lock all live here; apply the security-review agent. diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-06-PLAN.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-06-PLAN.md new file mode 100644 index 0000000..b8f0534 --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-06-PLAN.md @@ -0,0 +1,159 @@ +--- +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 +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" + 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" + 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" +--- + + +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. + +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 and an isolated assembled-router regression proving request 61 is denied with the PHP-compatible 429 body. + + + +@/home/jin/.codex/get-shit-done/workflows/execute-plan.md +@/home/jin/.codex/get-shit-done/templates/summary.md + + + +@.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-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 `UnauthorizedWriter` continues the chain without `bouncer.User` or `bouncer.Credential`, allowing `InvScope` to own the exact 401 body per D-08. + +From `fonoteka.go/plugins/golem15/fonoteka/plugin.go`: +- `fonoteka-api-token` uses `tok:` when `bouncer.Credential` contains `*models.ApiToken`; otherwise it falls back to `surf.ClientIP` and enforces Max 60 over one minute. + + + + + + + Task 1: Repair the personal-token middleware onion and prove the unauthenticated 61st request is throttled + fonoteka.go/plugins/golem15/fonoteka/routes.go, fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go + + fonoteka.go/plugins/golem15/fonoteka/routes.go (the live `/api/v1/fonoteka` genres group and its current middleware declaration) + fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go (real-plugin activation and BuildRouter isolation pattern) + fonoteka.go/plugins/golem15/fonoteka/routes_cors_test.go (`stubInvToken` and fresh assembled-router test setup patterns) + fonoteka.go/plugins/golem15/fonoteka/plugin.go (`fonoteka-api-token` Max/Decay/Key closure) + fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go (InvScope 401/403 short-circuit behavior) + summercms.go/surf/router.go (`Router.wrap` reverse loop and `Assemble`/`BuildRouter` fresh limiter construction) + summercms.go/surf/limiter.go (`TooManyAttempts` before `Hit`, 60th-allowed/61st-denied semantics, exact 429 body) + summercms.go/bouncer/registry.go (guard-error continuation and request-context behavior) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-CONTEXT.md (D-01, D-02, D-05, D-06, D-08) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md (Common Pitfall 5) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VERIFICATION.md (HTTP-04 failed truth and required target order) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-REVIEW.md (CR-01) + + + In `Plugin.Routes`, change only the live personal-token genres group's `surf.Use` arguments from `inv_token`, `inv.scope:read`, `throttle:fonoteka-api-token` to `inv_token`, `throttle:fonoteka-api-token`, `inv.scope:read`. This exact source order is required because `surf.Router.wrap` composes last-to-first: `inv_token` remains outermost and resolves a valid credential before the bucket key is computed (preserving `tok:` per D-01/D-06), the throttle then consumes both authenticated and unauthenticated traffic (D-02), and `InvScope` remains the innermost scope/401/403 gate (D-08). Do not move throttle before `inv_token`, do not alter bucket limits or key logic, and do not address unrelated REVIEW.md warnings. + + 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. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && go test ./plugins/golem15/fonoteka -run 'TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited|TestFullRouteTableAuthGroupMutualExclusivity' -count=1 -short + + + - `routes.go` contains `surf.Use("inv_token", "throttle:fonoteka-api-token", "inv.scope:read")` on the live `/api/v1/fonoteka/genres` group and does not contain the old `inv_token`, `inv.scope:read`, `throttle:fonoteka-api-token` order. + - `TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited` calls the handler returned by `surf.Assemble` for the real two-plugin set; it does not construct a limiter or middleware chain directly. + - The test uses one fresh assembled handler and a stable explicit RemoteAddr for all 61 requests, with no `t.Parallel`, global handler, or shared limiter state. + - Requests 1-60 assert HTTP 401 plus trimmed body `{"error":"Invalid token"}`; request 61 asserts HTTP 429 plus raw body exactly `{"message":"Too Many Attempts."}`, `X-RateLimit-Limit: 60`, and `X-RateLimit-Remaining: 0`. + - The targeted command exits 0 under `-short`, proving the regression does not require Docker, Postgres, or shared test state. + + The live personal-token genres route consumes unauthenticated requests in the 60/minute bucket before InvScope short-circuits, while valid tokens remain resolvable to tok:; an assembled-route regression fails if the middleware order is inverted again. + + + + + +## 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:` 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 | + + + +| Source | ID | Coverage | Status | +|--------|----|----------|--------| +| GOAL | Phase 6 success criterion 2 | Plans 06-01 through 06-05 provide the limiter/buckets; this plan repairs the failed live deny path | COVERED | +| REQ | HTTP-04 | Task 1 makes the live named bucket enforceable on unauthenticated traffic and adds regression proof | 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 | 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 | 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 | + + + +From `fonoteka.go`, run `go test ./plugins/golem15/fonoteka -run 'TestPersonalTokenGenresUnauthenticatedRequestsAreRateLimited|TestFullRouteTableAuthGroupMutualExclusivity' -count=1 -short`, then `go test ./plugins/golem15/fonoteka/... -count=1 -short`. Confirm a source grep shows only the target live order for the personal-token genres route. + + + +- The live personal-token genres group declares `inv_token`, `throttle:fonoteka-api-token`, `inv.scope:read` in 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 `-short` with no database or external service and cannot inherit limiter counters from another test. +- HTTP-04's verification blocker and REVIEW CR-01 are directly closed without changing unrelated warnings or deferred work. + + + +Create `.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-06-SUMMARY.md` when done. +