From c4f406bb64d922bbb277d689ba615d7e43d78120 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sat, 19 Sep 2026 16:58:48 +0200 Subject: [PATCH] docs(06): create phase plan --- .planning/ROADMAP.md | 20 +- .../06-01-PLAN.md | 334 +++++++++ .../06-02-PLAN.md | 309 +++++++++ .../06-03-PLAN.md | 333 +++++++++ .../06-04-PLAN.md | 244 +++++++ .../06-05-PLAN.md | 164 +++++ .../06-PATTERNS.md | 636 ++++++++++++++++++ .../06-VALIDATION.md | 57 +- 8 files changed, 2071 insertions(+), 26 deletions(-) create mode 100644 .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-PLAN.md create mode 100644 .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-02-PLAN.md create mode 100644 .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-03-PLAN.md create mode 100644 .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-PLAN.md create mode 100644 .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-PLAN.md create mode 100644 .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-PATTERNS.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 8755e0d..27b0de1 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -223,7 +223,25 @@ 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**: TBD +**Plans**: 5 plans + +Plans: +**Wave 1** *(parallel)* + +- [ ] 06-01-PLAN.md — Auth-groups slice: router verb/factory growth, bouncer guard registry, real inv_token guard + inv.scope, genres shared under both auth groups +- [ ] 06-04-PLAN.md — SSRF-guarded outbound fetch helper (framework primitive, independent of the other three plans) + +**Wave 2** *(blocked on 06-01)* + +- [ ] 06-02-PLAN.md — Rate limiting: fixed-window Store/Limiter, trusted-proxy client IP, five fonoteka buckets, PublicShareHeaders, remaining route groups declared + +**Wave 3** *(blocked on 06-02)* + +- [ ] 06-03-PLAN.md — Contract surface: raw-group enforcement + route table + route:list, wire response helpers, swag/openapi-typescript pipeline, path-scoped CORS, body limits (blocking human-verify checkpoint for production body-size numbers), oauth group declared raw + +**Wave 4** *(blocked on 06-01..06-04)* + +- [ ] 06-05-PLAN.md — Full unit coverage across both repos, full route-table mutual-exclusivity test, 06-SECURITY-REVIEW.md ### Phase 7: User plugin and authentication diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-PLAN.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-PLAN.md new file mode 100644 index 0000000..cd5c4ee --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-PLAN.md @@ -0,0 +1,334 @@ +--- +phase: 06-http-routing-auth-groups-and-rate-limiting +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - summercms.go/pact/capabilities.go + - summercms.go/surf/router.go + - summercms.go/surf/router_test.go + - summercms.go/bouncer/guard.go + - summercms.go/bouncer/registry.go + - summercms.go/bouncer/registry_test.go + - summercms.go/bouncer/context.go + - summercms.go/bouncer/context_test.go + - summercms.go/bouncer/jwt.go + - fonoteka.go/plugins/golem15/user/plugin.go + - fonoteka.go/plugins/golem15/fonoteka/plugin.go + - fonoteka.go/plugins/golem15/fonoteka/models/api_token.go + - fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go + - fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard_test.go + - fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go + - fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope_test.go + - fonoteka.go/plugins/golem15/fonoteka/routes.go + - fonoteka.go/plugins/golem15/fonoteka/routes_group_test.go + - fonoteka.go/parity/genres_seed_test.go + - fonoteka.go/parity/manifest.yaml +autonomous: true +requirements: [HTTP-03, HTTP-05] + +must_haves: + truths: + - "A JWT-authenticated GET /_fonoteka/api/v1/genres and a personal-token GET /api/v1/fonoteka/genres are served by the literal same controllers.ListGenres(p.app) handler value (D-15 shared-handler proof)" + - "bouncer.User(ctx) resolves identically for a JWT caller and a personal-token caller; only one accessor exists (D-06)" + - "A missing/unknown/malformed inv_ token on the personal-token group returns 401 {\"error\":\"Invalid token\"}; a token missing the required scope returns 403 {\"error\":\"Missing required scope: \"} -- never bouncer's {\"error\":true,\"message\":...} shape (D-08)" + - "The jwt guard's existing 401 bodies and Middleware(secret, users) behavior are byte-identical to Phase 3 after being re-expressed through the registry (D-10)" + - "Registering two guards under the same name, or referencing an unregistered guard name, fails boot with a message naming both plugins (D-06)" + - "The oauth guard name is not registered by any code in this plan -- grep for Register( calls finds only \"jwt\" and \"inv_token\" (D-09)" + - "GET /api/v1/fonoteka/genres personal_token is status: ported in manifest.yaml and passes TestParityCorpus against real Postgres via a seed-hook-inserted token, not a production mint path (D-07)" + - "Parameterized (\"name:param\") middleware names are resolved by a surf factory mechanism, not a fixed string table, so \"inv.scope:write\" and a future \"throttle:10,1\" are written at call sites exactly as PHP's ->middleware() calls (D-05)" + artifacts: + - path: summercms.go/bouncer/registry.go + provides: "Named Guard registry: Register(pluginID, name string, g any) error, Middleware(name string) (func(http.Handler) http.Handler, error)" + - path: summercms.go/bouncer/guard.go + provides: "Guard, CredentialGuard, UnauthorizedWriter interfaces" + - path: fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go + provides: "TokenGuard: real inv_token verification against models.ApiToken (hash lookup, expiry, revocation, last-used stamp)" + - path: fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go + provides: "InvScope(scope string) pact.Middleware with exact PHP 401/403 bodies" + key_links: + - from: fonoteka.go/plugins/golem15/fonoteka/routes.go + to: fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go + via: "both r.Group calls register controllers.ListGenres(p.app) as the GET handler" + pattern: "controllers\\.ListGenres\\(p\\.app\\)" + - from: fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go + to: summercms.go/bouncer/context.go + via: "InvScope reads bouncer.User(ctx) and bouncer.Credential(ctx)" + pattern: "bouncer\\.(User|Credential)\\(r\\.Context\\(\\)\\)" + - from: fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go + to: fonoteka.go/plugins/golem15/fonoteka/models/api_token.go + via: "hash lookup against ApiToken.TokenHash, then IsUsable()/HasScope()" + pattern: "token_hash = \\?|IsUsable\\(\\)|HasScope\\(" +--- + + +Turn the Phase 3 JWT-only routing skeleton into a real two-guard auth surface: extend the router to support non-GET verbs and parameterized ("name:param") middleware, build a named Guard registry in `bouncer` that both the existing `jwt` guard and a new real `inv_token` guard resolve through to one `bouncer.User(ctx)` accessor, and prove HTTP-03's core claim -- the same handler serves both the JWT group and the personal-token group -- by mounting `GET genres` under `/_fonoteka/api/v1` (JWT) and `/api/v1/fonoteka` (`inv_token` + `inv.scope:read`). + +Purpose: this is the foundational wave every later Phase 6 plan (rate limiting, raw groups, response conventions) builds on -- the router's verb/factory growth and the guard registry are load-bearing seams, not local-only code. +Output: `pact.Router`/`surf.Router` support Post/Put/Patch/Delete and colon-parameterized middleware names; `bouncer.Registry` with two real guards; `golem15.fonoteka`'s `inv_token` guard and `inv.scope` middleware; genres reachable and parity-green on both auth groups. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.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-PATTERNS.md + + + +New file summercms.go/bouncer/guard.go: + +package bouncer + +import "net/http" + +// Guard resolves the caller's Principal for r, or an error describing why not. +type Guard interface { + Authenticate(r *http.Request) (*Principal, error) +} + +// CredentialGuard resolves the Principal AND its underlying credential (e.g. +// *models.ApiToken) in one pass -- a DB-backed guard must never verify twice +// per request (RESEARCH.md Pitfall 5: last_used_at must stamp once). +type CredentialGuard interface { + AuthenticateCredential(r *http.Request) (*Principal, any, error) +} + +// UnauthorizedWriter lets a guard write its own failure response. jwtGuard +// implements this (reusing write401's {"error":true,"message":...} shape). +// TokenGuard does NOT implement it: PHP's TokenScope, not ApiTokenGuard, owns +// the {"error":"Invalid token"} 401 body (D-08) -- Registry.Middleware must +// pass an unauthenticated request through untouched when a guard has no +// UnauthorizedWriter, leaving denial to downstream middleware. +type UnauthorizedWriter interface { + WriteUnauthorized(w http.ResponseWriter, err error) +} + +New file summercms.go/bouncer/registry.go: + +package bouncer + +import "net/http" + +type Registry struct{ /* unexported: map[string]namedGuard */ } + +func NewRegistry() *Registry + +// Register stores g under name. g must implement Guard or CredentialGuard. +// Empty name, nil g, a type implementing neither, or a duplicate name all +// fail with a "bouncer: ..." error naming pluginID and name. +func (reg *Registry) Register(pluginID, name string, g any) error + +// Middleware derives an http middleware from a registered guard. Unknown +// names fail (fail boot, mirrors surf.RegisterMiddleware's contract). +// On Authenticate/AuthenticateCredential success: WithUser (+WithCredential +// if a credential was returned) then next.ServeHTTP. +// On failure: if the guard implements UnauthorizedWriter, it writes the +// response and the chain stops; otherwise next.ServeHTTP runs unauthenticated. +func (reg *Registry) Middleware(name string) (func(http.Handler) http.Handler, error) + +Extended summercms.go/bouncer/context.go (add alongside the existing userKey/WithUser/User): + +type credentialKey struct{} + +func WithCredential(ctx context.Context, cred any) context.Context +func Credential(ctx context.Context) (any, bool) + +New guard constructor in summercms.go/bouncer/jwt.go (adapter over the UNCHANGED existing helpers -- do not edit bearerToken/Verify/write401/msgUserNotFound bodies): + +// NewJWTGuard adapts the existing bearerToken -> Verify -> users.FindByID +// chain (identical to Middleware's body) into a Guard + UnauthorizedWriter, +// so Registry.Middleware("jwt") is byte-identical to bouncer.Middleware. +func NewJWTGuard(secret string, users UserProvider) Guard + +Extended summercms.go/pact/capabilities.go (Router interface): + +type Router interface { + Group(prefix string, middleware []string, fn func(Router)) + Get(path string, handler http.HandlerFunc, middleware ...string) + Post(path string, handler http.HandlerFunc, middleware ...string) + Put(path string, handler http.HandlerFunc, middleware ...string) + Patch(path string, handler http.HandlerFunc, middleware ...string) + Delete(path string, handler http.HandlerFunc, middleware ...string) + Where(param, pattern string) + WhereIn(param string, values ...string) +} + +Extended summercms.go/surf/router.go: + +// RegisterMiddlewareFactory stores a parameterized middleware. At wrap time, +// a route middleware name not found in r.named is split on the first ':' +// (strings.Cut); if the base name matches a registered factory, fn(param) +// builds the pact.Middleware for that one route. Duplicate factory names +// fail exactly like RegisterMiddleware. +func (r *Router) RegisterMiddlewareFactory(pluginID, name string, fn func(param string) pact.Middleware) error + +Extended summercms.go/pact/capabilities.go (new capability, collected in surf.Assemble in the same loop as HasMiddleware): + +type HasMiddlewareFactories interface { + MiddlewareFactories() map[string]func(param string) pact.Middleware +} + + + + + + Task 1 (summercms.go): Router verb growth, parameterized-middleware factories, and the bouncer Guard registry + summercms.go/pact/capabilities.go, summercms.go/surf/router.go, summercms.go/surf/router_test.go, summercms.go/bouncer/guard.go, summercms.go/bouncer/registry.go, summercms.go/bouncer/registry_test.go, summercms.go/bouncer/context.go, summercms.go/bouncer/context_test.go, summercms.go/bouncer/jwt.go + + summercms.go/pact/capabilities.go (full -- current Router interface, HasMiddleware) + summercms.go/surf/router.go (full -- route struct, add/compile/wrap, RegisterMiddleware, Assemble) + summercms.go/surf/router_test.go (full -- existing test shapes to extend, esp. TestMissingMiddlewareNamesPluginAndName) + summercms.go/bouncer/jwt.go (full -- bearerToken, Verify, write401, mapJWTError, msgUserNotFound: reuse verbatim, do not rewrite) + summercms.go/bouncer/context.go (full -- WithUser/User shape to mirror for WithCredential/Credential) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md (Pitfall 9: router is GET-only, exact lines to generalize; Pattern 1) + + + In pact/capabilities.go: add Post, Put, Patch, Delete to the Router interface with the exact same signature shape as Get (path string, handler http.HandlerFunc, middleware ...string). Add HasMiddlewareFactories interface { MiddlewareFactories() map[string]func(param string) pact.Middleware } alongside the existing HasMiddleware. + + In surf/router.go: thread a method string parameter through add(pluginID, prefix string, groupMW []string, method, path string, handler http.HandlerFunc, extra []string); change the duplicate-detection key from the hardcoded "GET " + full to method + " " + full; change route{method: "GET", ...} to use the passed method. Add Post/Put/Patch/Delete on both *Router and *Group, each calling add with its own HTTP method string, mirroring Get's and Group.Get's existing nil-guard shape exactly. In compile(), change mux.Handle("GET "+rt.path, h) to mux.Handle(rt.method+" "+rt.path, h). + + Add a factories map[string]namedMiddlewareFactory field to Router (initialize in New), a namedMiddlewareFactory{pluginID string; fn func(param string) pact.Middleware} type, and RegisterMiddlewareFactory(pluginID, name string, fn func(param string) pact.Middleware) error mirroring RegisterMiddleware's nil/empty/duplicate checks and "surf: middleware factory %q already registered by %s" error shape. In wrap()'s middleware-resolution loop, when r.named[name] misses, call strings.Cut(name, ":"); if hasParam and r.factories[base] exists, build h = r.factories[base].fn(param)(h) and continue the loop instead of falling through to the existing "unknown middleware" error (only fall through to that error when neither a named middleware nor a factory match). In Assemble(), add a second loop (after the existing HasMiddleware loop, before the Routes loop) that type-asserts each plugin against pact.HasMiddlewareFactories and calls r.RegisterMiddlewareFactory(p.ID(), name, fn) for each entry. + + Create bouncer/guard.go with the Guard, CredentialGuard, UnauthorizedWriter interfaces exactly as specified in the interfaces block above. + + Create bouncer/registry.go with Registry, NewRegistry(), Register(pluginID, name string, g any) error (validate g is non-nil and implements Guard or CredentialGuard via a type switch; fail with "bouncer: plugin %q registered guard %q that implements neither Guard nor CredentialGuard" otherwise; duplicate name fails with "bouncer: guard %q already registered by %s", mirroring RegisterMiddleware's message shape), and Middleware(name string) (func(http.Handler) http.Handler, error) exactly as specified above: prefer CredentialGuard.AuthenticateCredential when the guard implements it, else Guard.Authenticate; on error, call UnauthorizedWriter.WriteUnauthorized if implemented, else pass the request through unauthenticated (no principal attached); on success, WithUser then WithCredential (only if a non-nil credential was returned) before calling next. + + Extend bouncer/context.go with credentialKey{} and WithCredential/Credential, an exact structural mirror of userKey{}/WithUser/User (nil-context guard, context.WithValue, type-assert-with-ok). + + In bouncer/jwt.go, add NewJWTGuard(secret string, users UserProvider) Guard returning an unexported jwtGuard{secret, users} struct whose Authenticate method runs the IDENTICAL sequence Middleware's closure already runs (bearerToken(r) -> Verify(raw, secret) -> strconv.ParseUint the subject -> users.FindByID -> nil checks) by calling those same existing package-level helpers, returning (*Principal, error) instead of writing a response; its WriteUnauthorized(w, err) method calls the existing write401(w, err.Error()) verbatim. Do NOT change the body of the existing exported Middleware function -- it keeps working unmodified (D-10). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test ./surf/... ./bouncer/... ./pact/... -short + + + - surf.Router and surf.Group both expose working Post/Put/Patch/Delete; a new test registers POST /items and GET /items on the same path and asserts both are dispatched independently (no collision, no duplicate-route error). + - A new test registers a middleware factory under "inv.scope" and a route using "inv.scope:write"; asserts the factory receives exactly "write" as param and its returned middleware runs. + - Registry duplicate/unknown-name tests in bouncer/registry_test.go assert the error string contains both plugin id and guard name (mirroring TestMissingMiddlewareNamesPluginAndName's assertion shape). + - A test registers a fake guard implementing only Guard + UnauthorizedWriter, asserts Registry.Middleware writes the guard's own response and never calls next on failure. + - A test registers a fake guard implementing only CredentialGuard (no UnauthorizedWriter), asserts next still runs on failure with NO principal attached to the context (soft-fail path), and that a successful AuthenticateCredential attaches both bouncer.User(ctx) and bouncer.Credential(ctx). + - bouncer.NewJWTGuard(secret, users) wrapped through Registry.Middleware("jwt") produces byte-identical response bodies/status codes to bouncer.Middleware(secret, users) for the same four failure cases already covered by bouncer/jwt_test.go (missing token, expired, bad signature, unknown user). + + pact.Router supports all four non-GET verbs; surf resolves "name:param" middleware via registered factories; bouncer.Registry exists with Guard/CredentialGuard/UnauthorizedWriter and is unit-tested in isolation from any real guard. + + + + Task 2 (fonoteka.go): Real inv_token guard, inv.scope middleware, and guard registration in both plugins + fonoteka.go/plugins/golem15/user/plugin.go, fonoteka.go/plugins/golem15/fonoteka/plugin.go, fonoteka.go/plugins/golem15/fonoteka/models/api_token.go, fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go, fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard_test.go, fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go, fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope_test.go + + - ApiToken.HasScope("read") is true only when "read" is present in Scopes.Get(). + - ApiToken.IsUsable() is false when RevokedAt != nil, false when ExpiresAt != nil and in the past, true otherwise. + - TokenGuard.AuthenticateCredential returns an error for: no Authorization header, a bearer not prefixed inv_, an unknown hash, a revoked token, an expired token. + - TokenGuard.AuthenticateCredential on a valid token returns (*bouncer.Principal{ID: token.UserID, ...}, *models.ApiToken, nil) and stamps LastUsedAt/LastUsedIP exactly once (assert via a row re-read, not a mock). + - InvScope("write") on a request with no bouncer.User(ctx) writes 401 {"error":"Invalid token"}. + - InvScope("write") on a request with a resolved user but a credential lacking the write scope writes 403 {"error":"Missing required scope: write"}. + - InvScope("read") on a request with a read-scoped credential calls next. + + + summercms.go/bouncer/guard.go, summercms.go/bouncer/registry.go, summercms.go/bouncer/context.go (Task 1 output) + fonoteka.go/plugins/golem15/fonoteka/models/api_token.go (full -- Fillable/Hidden/fields, Scopes is lagoon.Jsonable[[]string]) + fonoteka.go/plugins/golem15/fonoteka/middleware/must_change_password.go (full -- exact same-package middleware shape to mirror) + fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go (writeJSON/writeOpaque500 -- do NOT import cross-package; token_scope.go needs its own tiny writer, see action) + fonoteka.go/plugins/golem15/user/plugin.go (full -- Boot/Middlewares to restructure) + fonoteka.go/plugins/golem15/fonoteka/plugin.go (full -- Boot/Middlewares to extend) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/middleware/TokenScope.php (exact 401/403 bodies, lines 39-60) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/classes/auth/ApiTokenGuard.php (verify/stamp sequence, lines 42-63) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/classes/auth/ApiTokenManager.php (PREFIX = 'inv_', sha256 hash, verify() gate, lines 20-83) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/models/ApiToken.php (isExpired/isRevoked/isUsable/hasScope, lines 63-77) + + + In models/api_token.go, add two methods: HasScope(scope string) bool returning slices.Contains(t.Scopes.Get(), scope) (import "slices"), and IsUsable() bool returning t.RevokedAt == nil && (t.ExpiresAt == nil || t.ExpiresAt.After(time.Now())). Leave Fillable/Hidden/TableName untouched. + + Create classes/auth/token_guard.go (package auth): TokenGuard struct { db *gorm.DB }, NewTokenGuard(db *gorm.DB) *TokenGuard. Implement AuthenticateCredential(r *http.Request) (*bouncer.Principal, any, error): read Authorization header, trim a leading "Bearer " (same convention as PHP's $request->bearerToken()); if empty or not strings.HasPrefix(bearer, "inv_"), return an error (mutual-exclusion gate, mirrors ApiTokenManager::verify's prefix check); compute sha256.Sum256([]byte(bearer)) hex-encoded; SELECT one models.ApiToken WHERE token_hash = ?; if not found or !token.IsUsable(), return an error; stamp last_used_at/last_used_ip with a single UpdateColumns call against models.ApiToken{} filtered by id = ? (derive the IP from r.RemoteAddr via net.SplitHostPort, falling back to the raw value on split error -- the trusted-proxy-aware client IP function lands in 06-02; do not build it here) so no GORM hooks or query logging capture the raw bearer; load the owning usermodels.User by token.UserID; return &bouncer.Principal{ID: user.ID, MustChangePassword: user.MustChangePassword}, &token, nil. TokenGuard implements bouncer.CredentialGuard only -- it must NOT implement UnauthorizedWriter (D-08: TokenScope, not the guard, owns the 401/403 bodies). + + Create middleware/token_scope.go (package middleware, same package as must_change_password.go): InvScope(scope string) pact.Middleware. Body: read bouncer.User(r.Context()); if not ok, write status 401 with body {"error":"Invalid token"} and return (do not call next). Otherwise read bouncer.Credential(r.Context()), type-assert to *models.ApiToken; if the assertion fails or !token.HasScope(scope), write status 403 with body {"error":"Missing required scope: " + scope} and return. Otherwise call next.ServeHTTP(w, r). Write the two JSON bodies with a small unexported writeJSON(w http.ResponseWriter, status int, v any) local to this file (Content-Type: application/json, json.NewEncoder(w).Encode(v)) -- do not reuse bouncer's write401 (different body shape: bool error there, string error here) and do not cross-import controllers. + + In fonoteka.go/plugins/golem15/user/plugin.go: in Boot, after the existing classes.JWTSecret(app) and hook-registration code, look up *bouncer.Registry via app.Lookup[*bouncer.Registry](); if absent, construct with bouncer.NewRegistry() and app.Publish(reg); call reg.Register(p.ID(), "jwt", bouncer.NewJWTGuard(secret, classes.GormUsers{App: app})) and return its error. Rewrite Middlewares() to look up the registry (p.app.Lookup[*bouncer.Registry]()), call reg.Middleware("jwt"), and return map[string]pact.Middleware{"jwt.auth": mw} (nil map if either lookup fails, matching the existing nil-on-error convention already used for the JWT-secret failure path). + + In fonoteka.go/plugins/golem15/fonoteka/plugin.go: inside the existing if gdb, ok := app.Lookup[*gorm.DB](); ok { ... } block in Boot, after classes.RegisterHooks(gdb) succeeds, look up-or-create the SAME *bouncer.Registry (identical lookup-or-create snippet as the user plugin) and call reg.Register(p.ID(), "inv_token", auth.NewTokenGuard(gdb)), returning its error. Extend Middlewares() to also look up bouncer.Registry and, if reg.Middleware("inv_token") succeeds, add "inv_token": mw to the returned map alongside the existing "inv.must-change-password" entry. Add a new method MiddlewareFactories() map[string]func(param string) pact.Middleware { return map[string]func(string) pact.Middleware{"inv.scope": func(scope string) pact.Middleware { return middleware.InvScope(scope) }} } and declare _ pact.HasMiddlewareFactories = (*Plugin)(nil) alongside the existing interface assertions. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && go vet ./... && go test ./plugins/golem15/fonoteka/... ./plugins/golem15/user/... -short + + + - go test ./plugins/golem15/fonoteka/classes/auth/... -run TestTokenGuard covers every behavior case listed above, using a real (testcontainers) Postgres ApiToken row, not a mock. + - go test ./plugins/golem15/fonoteka/middleware/... -run TestInvScope asserts the exact byte bodies {"error":"Invalid token"} and {"error":"Missing required scope: write"} (not the {"error":true,"message":...} shape). + - A boot-time test (activating both plugins against a real *backpack.App) asserts reg.Middleware("jwt") and reg.Middleware("inv_token") both resolve without error after party.Activate. + - LastUsedAt/LastUsedIP are asserted to change exactly once per AuthenticateCredential call in the test (no double-stamp). + + Both guards are registered through the same bouncer.Registry; jwt.auth is behaviorally unchanged; inv_token + inv.scope reproduce TokenScope.php's exact 401/403 contract. + + + + Task 3 (fonoteka.go, parity): Mount genres under both auth groups, parity seed hook for the token surface, shared-handler + isolation tests + fonoteka.go/plugins/golem15/fonoteka/routes.go, fonoteka.go/plugins/golem15/fonoteka/routes_group_test.go, fonoteka.go/parity/genres_seed_test.go, fonoteka.go/parity/manifest.yaml + + fonoteka.go/plugins/golem15/fonoteka/routes.go (current, full -- 14 lines) + fonoteka.go/parity/genres_seed_test.go (full -- seedGenres, upsertParityAlice, upsertParityCollection, mintTestJWT patterns to mirror for a test-only token mint) + fonoteka.go/parity/manifest.yaml lines 2803-2841 (the two personal_token genres entries; the sibling GET /_fonoteka/api/v1/genres jwt entry at lines 1514-1532 for the exact status: ported / seed_hook: genres shape to copy) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/routes.php lines 449-454, 513-514 (the real /api/v1/fonoteka group prefix, middleware, and the genres route's inv.scope:read) + + + In routes.go, add a second r.Group("/api/v1/fonoteka", surf.Use("inv_token", "inv.scope:read"), func(g pact.Router) { g.Get("/genres", controllers.ListGenres(p.app)) }) call after the existing JWT group, passing the SAME controllers.ListGenres(p.app) handler value used in the JWT group (proves D-15's shared-handler requirement by construction, not convention). PHP's real group middleware also carries a group-level throttle:fonoteka-api-token (routes.php line 450) -- that bucket does not exist until 06-02, so leave a "// TODO(06-02): throttle:fonoteka-api-token" comment directly above this r.Group call as the exact landing spot; do not add a "throttle:..." string to surf.Use(...) yet (it would fail boot with "unknown middleware" until 06-02 registers the factory). + + In parity/genres_seed_test.go, extend seedGenres (reuse the existing Alice + collection setup it already performs) to also mint a test-only personal token: generate 32 random bytes, base64url-encode (no padding) with an "inv_" prefix -- mirroring ApiTokenManager::mint's secret shape but minted directly via GORM (minting stays test-only per D-07, never through a production endpoint); compute sha256 hex of the secret; upsert (idempotent, same Where(...).Take then Create pattern as upsertParityAlice) a fonotekamodels.ApiToken{UserID: alice.ID, Name: , TokenHash: hash, Scopes: lagoon.Jsonable[[]string]{Data: []string{"read"}, Valid: true}} row; call store.Set("token:mcp-read", secret) so the manifest's existing Authorization: "Bearer {{token:mcp-read}}" fixture headers resolve during replay. Keep this inside the existing seedGenres function and the existing "genres" map entry in seedHooks -- do not introduce a second hook name, since the personal-token genres route needs the identical Alice/collection context seedGenres already builds. + + In parity/manifest.yaml, on the GET /api/v1/fonoteka/genres personal_token entry (currently status: pending, no seed_hook key), change status: pending to status: ported and add a seed_hook: genres line directly below status:, matching the exact key placement already used on the GET /_fonoteka/api/v1/genres jwt entry. Do not modify any other manifest entry (in particular leave POST /api/v1/fonoteka/genres personal_token and every other pending entry untouched). + + Create routes_group_test.go (matching the existing test package convention in this directory) with httptest-based assertions: (a) a request to /_fonoteka/api/v1/genres with a valid JWT and a request to /api/v1/fonoteka/genres with a valid inv_ token both return 200 with equal GenreList bodies for the same seeded user/collection (shared-handler proof); (b) a request to /api/v1/fonoteka/genres with no Authorization header returns 401 {"error":"Invalid token"}; (c) a request with a well-formed but unknown inv_ bearer returns 401 {"error":"Invalid token"}; (d) a request with a token minted with only ["write"] scopes returns 403 {"error":"Missing required scope: read"} on the same route. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && go vet ./... && go test ./plugins/golem15/fonoteka/... -run TestGenresSharedHandler -short && go test ./parity/... -run TestParityCorpus + + + - The new httptest suite passes all four sub-assertions listed above. + - go test ./parity/... -run TestParityCorpus reports GET /api/v1/fonoteka/genres personal_token as passing (not pending) and does not regress the existing GET /_fonoteka/api/v1/genres jwt passing count. + - grep -n "status: ported" fonoteka.go/parity/manifest.yaml shows exactly two entries (the pre-existing jwt genres route and the newly-flipped personal_token genres route). + + Genres is reachable and parity-green under both auth groups through the identical handler; the personal-token guard, scope gate, and 401/403 bodies are proven end to end, not just unit-tested in isolation. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|--------------| +| client -> Authorization header | untrusted bearer credential (JWT or inv_ token) parsed on every request | +| guard registry -> plugin Boot | plugin-declared guard names become live auth middleware; a misregistration is a silent-bypass risk if not fail-loud | +| inv_token guard -> golem15_fonoteka_api_tokens | hash-indexed lookup of an untrusted bearer against stored token_hash | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-06-01 | Spoofing | bouncer.Registry.Register | mitigate | Duplicate or unregistered guard names fail Boot loudly (Task 1 test), mirroring surf.RegisterMiddleware's existing fail-boot contract -- no silent no-op auth | +| T-06-02 | Elevation of Privilege | inv_token credential vs jwt credential | mitigate | Each guard is registered and resolved independently; Task 3's shared-handler test proves both groups reach the SAME handler through DIFFERENT guards, and Task 3 asserts the personal-token group rejects a missing/invalid inv_ credential -- full route-table mutual-exclusivity (zero jwt.auth on the token group, zero inv_token/inv.scope on the JWT group) is completed in 06-03 once the route table exists; this plan's partial coverage is the two groups never sharing a middleware list literal | +| T-06-03 | Information Disclosure | ApiToken.TokenHash | accept | Already hidden via json:"-" and Hidden() (Phase 5, verified by 05-06's hidden-marshal test); this plan adds no new serialization path for the hash | +| T-06-04 | Repudiation | last_used_at/last_used_ip stamping | mitigate | TokenGuard stamps via a single UpdateColumns call with no query/error logging of the raw bearer; Task 2 test asserts exactly one stamp per AuthenticateCredential call | +| T-06-05 | Tampering | inv_token guard's SHA-256 hash lookup | accept | Indexed equality lookup (not a byte-for-byte secret compare) is not a timing side-channel per RESEARCH.md's V6 Cryptography note; crypto/subtle is reserved for a future raw-compare path (e.g. OAuth client secrets, Phase 8), not needed here | + + + +cd summercms.go && go vet ./... && go test ./surf/... ./bouncer/... ./pact/... -short +cd ../fonoteka.go && go vet ./... && go test ./plugins/golem15/... -short && go test ./parity/... -run TestParityCorpus + + + +- pact.Router and surf.Router/Group support Get/Post/Put/Patch/Delete and colon-parameterized middleware factories. +- bouncer.Registry resolves both "jwt" and "inv_token" to the same bouncer.User(ctx) accessor; jwt's behavior is unchanged from Phase 3. +- golem15.fonoteka's inv_token guard and inv.scope middleware reproduce TokenScope.php's exact 401/403 bodies. +- GET genres is reachable and parity-green on both /_fonoteka/api/v1 (JWT) and /api/v1/fonoteka (personal token) through the identical handler value. +- go vet ./... and go test ./... are green in both repos (testcontainers-gated tests included). + + + +Create `.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-SUMMARY.md` when done + diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-02-PLAN.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-02-PLAN.md new file mode 100644 index 0000000..ee37b68 --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-02-PLAN.md @@ -0,0 +1,309 @@ +--- +phase: 06-http-routing-auth-groups-and-rate-limiting +plan: 02 +type: execute +wave: 2 +depends_on: ["06-01"] +files_modified: + - summercms.go/surf/limiter.go + - summercms.go/surf/limiter_store.go + - summercms.go/surf/clientip.go + - summercms.go/surf/router.go + - summercms.go/surf/limiter_test.go + - summercms.go/surf/clientip_test.go + - summercms.go/pact/capabilities.go + - fonoteka.go/plugins/golem15/fonoteka/plugin.go + - fonoteka.go/plugins/golem15/fonoteka/routes.go + - fonoteka.go/plugins/golem15/fonoteka/middleware/public_share_headers.go + - fonoteka.go/plugins/golem15/fonoteka/middleware/public_share_headers_test.go + - fonoteka.go/plugins/golem15/fonoteka/routes_bucket_test.go + - fonoteka.go/config/http.yaml + - fonoteka.go/parity/php_parity.sh + - .planning/ROADMAP.md + - .planning/REQUIREMENTS.md +autonomous: true +requirements: [HTTP-04] + +must_haves: + truths: + - "The five fonoteka buckets exist with the exact names, limits and key composition from routes.php: fonoteka-api-token (60/min, tok: else IP), fonoteka-oauth-token (30/min, oauthtok:), fonoteka-oauth-register (30/min, oauthreg:), fonoteka-public-token (60/min, pubtok:), fonoteka-public-ip (120/min, IP) (D-01)" + - "The limiter is a fixed window matching Laravel's tooManyAttempts-before-hit, first-hit-wins control flow, not a token bucket or sliding window (D-02, Pitfall 1)" + - "X-RateLimit-Limit and X-RateLimit-Remaining are set on every throttled route's successful response, not only on 429s; Retry-After and X-RateLimit-Reset appear only on 429 (D-02, Pitfall 3)" + - "Stacking two throttle: middleware entries on one route (fonoteka-public-token + fonoteka-public-ip) enforces both budgets independently (D-01)" + - "Client IP for limiter keys comes from RemoteAddr unless RemoteAddr is inside http.trusted_proxies, in which case the rightmost untrusted X-Forwarded-For hop is used; an empty trusted-proxies list means RemoteAddr only (D-04)" + - "The public-share group's 429 body is {\"error\":\"Too many requests\"} via PublicShareHeaders, never the house-default {\"message\":\"Too Many Attempts.\"} body used elsewhere (D-02 Pitfall 2)" + - "php_parity.sh exports APP_DEBUG=false so recorded error fixtures reflect the production (non-debug) body shape (user-resolved Open Question 2)" + - "ROADMAP.md and REQUIREMENTS.md HTTP-04 say five named buckets, not seven (D-01 miscount correction)" + - "Rate-limit counters live in-process behind a small Store interface (mutex-guarded map with expiry sweep), stdlib only -- no otter/cooler package this phase (D-03)" + artifacts: + - path: summercms.go/surf/limiter_store.go + provides: "Store interface + MemoryStore: Hit, TooManyAttempts, AvailableIn matching Illuminate\\Cache\\RateLimiter semantics" + - path: summercms.go/surf/limiter.go + provides: "Bucket type, Limiter, RegisterBucket, the throttle: middleware factory wired into surf.Router" + - path: summercms.go/surf/clientip.go + provides: "ClientIP(r, trusted) and TrustedProxies(cfg) -- the single source of client IP for limiter keys" + - path: fonoteka.go/plugins/golem15/fonoteka/middleware/public_share_headers.go + provides: "PublicShareHeaders: X-Robots-Tag/Cache-Control on every response, 429 body rewrite with header preservation" + key_links: + - from: fonoteka.go/plugins/golem15/fonoteka/plugin.go + to: summercms.go/surf/limiter.go + via: "Plugin implements surf.BucketProvider, registering the five buckets at Boot" + pattern: "Buckets\\(\\) map\\[string\\]surf\\.Bucket" + - from: summercms.go/surf/router.go + to: summercms.go/surf/limiter.go + via: "Router registers a built-in \"throttle\" middleware factory bound to its Limiter" + pattern: "RegisterMiddlewareFactory\\(.*\"throttle\"" +--- + + +Replace the Phase 3 no-op limiter seam with a real fixed-window rate limiter that ports Płytarium's rate-limit contract 1:1: the five named `fonoteka-*` buckets, inline `throttle:N,M` limits, stacked limiters on one route, and Laravel's exact wire behavior (headers on success and on 429, first-hit-wins fixed window, no token-bucket smoothing). Wire the trusted-proxy-aware client IP function that both the limiter and D-08's token surface can share, and port `PublicShareHeaders`' 429 rewrite for the anonymous public-share group. + +Purpose: HTTP-04 requires byte-for-byte parity with Laravel's `ThrottleRequests`/`RateLimiter`, which has documented, non-obvious semantics (fixed window, not token bucket; headers on success too; tooManyAttempts checked BEFORE hit) -- getting this wrong changes observable client-visible behavior, not just an implementation detail. +Output: `surf.Limiter`/`surf.Store`/`surf.Bucket`/`surf.ClientIP`; the `"throttle"` middleware factory; the five fonoteka buckets registered and attached to the token group; `PublicShareHeaders` ported; the remaining Phase-6-scope route groups (`jwt_locale`, `onboarding`, `public_invitation`, `public_share`/`public_wishlist`) declared as group builders per D-15. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.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-PATTERNS.md +@.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-SUMMARY.md + + + +New file summercms.go/surf/limiter_store.go: + +package surf + +import "time" + +// Store mirrors Illuminate\Cache\RateLimiter's hit/tooManyAttempts/ +// availableIn control flow: a fixed window, first-hit-wins (an existing +// unexpired window is never extended), with resetAttempts as a side effect +// of TooManyAttempts observing an expired window. +type Store interface { + Hit(key string, decay time.Duration) (attempts int) + TooManyAttempts(key string, max int) bool + AvailableIn(key string) time.Duration +} + +// NewMemoryStore returns an in-process, mutex-guarded Store. sweep controls +// the background expired-entry cleanup interval (memory hygiene only -- +// correctness does not depend on it, since expiry is checked lazily). +func NewMemoryStore(sweep time.Duration) *MemoryStore + +New file summercms.go/surf/limiter.go: + +package surf + +import "net/http" + +// Bucket is one named rate-limit definition. Key composes the limiter key +// from the request (token id, IP, route param -- D-01's per-bucket rule). +type Bucket struct { + Name string + Max int + Decay time.Duration + Key func(r *http.Request) string +} + +// BucketProvider is implemented by plugins that declare named buckets (not a +// pact interface: it lives in surf and is type-asserted directly in +// Assemble, since pact cannot import surf without a cycle). +type BucketProvider interface { + Buckets() map[string]Bucket +} + +type Limiter struct{ /* unexported: store Store; buckets map[string]Bucket */ } + +func NewLimiter(store Store) *Limiter +func (l *Limiter) RegisterBucket(pluginID, name string, b Bucket) error + +// Middleware builds the throttle: factory body. param is either a +// registered bucket name (looked up in l.buckets) or a literal "N,M" pair +// (inline throttle, parsed once and cached -- see ValidateThrottle). +// Sequence per request (Laravel ThrottleRequests::handleRequest order): +// 1. resolve the Bucket (named lookup or inline N,M with an inline key +// resolver: principal id if bouncer.User(ctx) is set, else +// r.Host+"|"+ClientIP) +// 2. if store.TooManyAttempts(key, max): set Retry-After/X-RateLimit-Reset/ +// X-RateLimit-Limit/X-RateLimit-Remaining=0, write the 429 body, stop +// 3. else store.Hit(key, decay); set X-RateLimit-Limit/-Remaining; call next +func (l *Limiter) Middleware(param string) pact.Middleware + +// ValidateThrottle is called once per route at Assemble/wrap time (not per +// request) so a malformed inline "N,M" or an unregistered bucket name fails +// boot instead of the first live request. +func (l *Limiter) ValidateThrottle(param string) error + +New file summercms.go/surf/clientip.go: + +package surf + +import "net/netip" + +// ClientIP is the single source of client IP for limiter keys (D-04). +// RemoteAddr is used unless it parses as being inside one of trusted; +// in that case the rightmost X-Forwarded-For hop NOT inside any trusted +// prefix is used. An empty trusted list means RemoteAddr only. +func ClientIP(r *http.Request, trusted []netip.Prefix) string + +// TrustedProxies reads http.trusted_proxies (a []string of CIDRs) from cfg +// and parses it once into []netip.Prefix. A malformed entry is skipped, not +// fatal (logged by the caller if desired). +func TrustedProxies(cfg *compass.Config) []netip.Prefix + +Router change (summercms.go/surf/router.go): Router gains a limiter *Limiter +field (nil-safe: a nil limiter behaves like today's noopLimiter). Assemble +constructs one surf.NewLimiter(surf.NewMemoryStore(2*time.Minute)) and +attaches it to the Router before the BucketProvider loop, then registers the +built-in factory: r.RegisterMiddlewareFactory("surf", "throttle", +func(param string) pact.Middleware { return r.limiter.Middleware(param) }). +The unconditional h = noOpLimit(h) line in wrap() is removed -- rate limiting +is now expressed exclusively via named "throttle:..." middleware entries at +call sites, matching D-05. + + + + + + Task 1 (summercms.go): Fixed-window Store, Limiter, trusted-proxy ClientIP, and router wiring + summercms.go/surf/limiter.go, summercms.go/surf/limiter_store.go, summercms.go/surf/clientip.go, summercms.go/surf/router.go, summercms.go/surf/limiter_test.go, summercms.go/surf/clientip_test.go, summercms.go/pact/capabilities.go + + summercms.go/surf/router.go (full, post-06-01 -- wrap()'s noOpLimit call site, RegisterMiddlewareFactory, Assemble) + summercms.go/compass/config.go (full -- Lookup/String/Int/Bool/LoadSection for reading http.trusted_proxies) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md (Pitfalls 1, 3, 4, 6 and the "Fixed-window limiter Store" code example, lines 399-414) + /media/nvme/dev/golem15/fonoteka/vendor/laravel/framework/src/Illuminate/Cache/RateLimiter.php (hit/tooManyAttempts/availableIn -- read directly, do not paraphrase from memory) + /media/nvme/dev/golem15/fonoteka/vendor/laravel/framework/src/Illuminate/Routing/Middleware/ThrottleRequests.php (handleRequest control-flow order: tooManyAttempts check BEFORE hit; addHeaders on the success response too; resolveRequestSignature's user-id-vs-domain+ip branch) + + + Create limiter_store.go: counterEntry{count int; resetAt time.Time}; MemoryStore{mu sync.Mutex; entries map[string]*counterEntry; sweep time.Duration}; NewMemoryStore(sweep time.Duration) *MemoryStore starting a background goroutine (time.Ticker on sweep, or time.AfterFunc re-armed each tick) that deletes entries where time.Now().After(resetAt). Hit(key, decay): lock; if entry absent or now is after entry.resetAt, replace with a fresh {count:0, resetAt: now.Add(decay)} (first-hit-wins: an existing unexpired window is never extended); increment count; return count. TooManyAttempts(key, max): lock; if entry absent, return false; if now is after entry.resetAt, delete the entry (mirrors PHP's resetAttempts() side effect) and return false; return entry.count >= max. AvailableIn(key): lock; if entry absent, return 0; return max(0, entry.resetAt.Sub(now)). + + Create limiter.go per the exact contract in the interfaces block: Bucket, BucketProvider, Limiter, NewLimiter, RegisterBucket (dup-fail shape identical to RegisterMiddleware's, error "surf: bucket %q already registered by %s"), Middleware(param), ValidateThrottle(param). Resolve param in this order: (1) if param matches a registered bucket name, use it; (2) else parse param as "N,M" via strings.Cut(param, ",") + strconv.Atoi on both parts (N=max attempts, M=decay in minutes, matching Laravel's throttle:N,M semantics where M is minutes) and build an ad-hoc Bucket with Max=N, Decay=time.Duration(M)*time.Minute, and Key resolving per Pitfall 4: if bouncer.User(r.Context()) is set, key is "u:"+principal ID; else key is r.Host+"|"+ClientIP(r, trusted) (the inline-throttle Limiter needs a trusted-proxy list too -- thread it through NewLimiter's constructor or a setter, e.g. NewLimiter(store Store, trusted []netip.Prefix)); (3) else ValidateThrottle/Middleware returns an error (surfaced at Assemble time via the factory's first invocation during Router.wrap()'s per-route validation loop, not deferred to the first live request -- Assemble already calls r.wrap(rt) once per route after Routes() collection specifically to catch unknown-middleware errors at boot; the same loop now also catches unknown bucket/malformed inline-throttle errors). Request-time sequence inside Middleware's returned handler: call store.TooManyAttempts(key, max) FIRST; if true, compute retryAfter := store.AvailableIn(key), set headers Retry-After (seconds, integer), X-RateLimit-Reset (unix timestamp of now+retryAfter), X-RateLimit-Limit (max), X-RateLimit-Remaining ("0"), write status 429 with body {"message":"Too Many Attempts."} (the house-default shape per Pitfall 2 -- PublicShareHeaders rewrites this for its own group in Task 2), and return without calling next; else call attempts := store.Hit(key, decay), set X-RateLimit-Limit and X-RateLimit-Remaining (max(0, max-attempts)) on the response, then call next.ServeHTTP. + + Create clientip.go: ClientIP(r, trusted) -- parse r.RemoteAddr via net.SplitHostPort to get the bare IP; if trusted is empty or the parsed IP does not match any prefix in trusted (netip.Prefix.Contains), return that IP; otherwise read X-Forwarded-For, split on comma, trim each hop, walk the list RIGHT TO LEFT, return the first hop whose parsed address is NOT contained in any trusted prefix; if every hop is trusted (or the header is empty/absent), fall back to the original RemoteAddr IP. TrustedProxies(cfg *compass.Config): read http.trusted_proxies as a []string (mirror corsOrigins's []string/[]any type-switch pattern already in router.go), netip.ParsePrefix each entry, skip invalid entries, return the slice (nil/empty when the key is absent). + + In router.go: add a limiter *Limiter field to Router; in Assemble(), after constructing r := New(corsOrigins(app)), build trusted := TrustedProxies(app.Config), lim := NewLimiter(NewMemoryStore(2*time.Minute), trusted) (document the 2-minute sweep default inline as "longest bucket decay is 1 minute; sweep at 2x"), set r.limiter = lim, and call r.RegisterMiddlewareFactory("surf", "throttle", func(param string) pact.Middleware { return lim.Middleware(param) }) before the existing HasMiddleware/HasMiddlewareFactories/BucketProvider loops (BucketProvider loop is new: for each plugin implementing surf.BucketProvider, call lim.RegisterBucket(p.ID(), name, b) for every entry). Remove the unconditional h = noOpLimit(h) line from wrap() (rate limiting is now exclusively expressed via "throttle:..." middleware entries); delete noOpLimit/noopLimiter only if nothing else references them, otherwise leave them unused-but-harmless is NOT acceptable in Go (unused private funcs are fine, but confirm via go vet/staticcheck that removing the call site doesn't orphan an exported symbol other packages depend on -- grep first). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test ./surf/... -run TestLimiter -short && go test ./surf/... -run TestClientIP -short + + + - A fuzz-free unit test drives MemoryStore directly and asserts: three Hit calls within one decay window return counts 1,2,3; TooManyAttempts is false at count==max-1 and true at count==max; after the window elapses, TooManyAttempts resets to false and a subsequent Hit reopens a fresh window (first-hit-wins, not extended). + - A test asserts X-RateLimit-Limit/X-RateLimit-Remaining are present on a SUCCESSFUL (200) response that passed through a throttle: middleware, not just on the 429. + - A test asserts the 429 response carries Retry-After, X-RateLimit-Reset, X-RateLimit-Limit, and X-RateLimit-Remaining=0, with body {"message":"Too Many Attempts."}. + - A test registers two buckets and stacks "throttle:bucket-a","throttle:bucket-b" on one route; exhausting bucket-a's budget alone (while bucket-b still has room) returns 429; a separate test exhausting only bucket-b (bucket-a fresh) also returns 429 -- both budgets are enforced independently. + - ClientIP tests: RemoteAddr used when trusted is empty; RemoteAddr used when RemoteAddr is untrusted even if X-Forwarded-For is present (spoofing test); rightmost untrusted hop used when RemoteAddr is trusted and X-Forwarded-For has a mixed trusted/untrusted chain. + - An inline "throttle:10,1" test asserts the key differs for two different bouncer.User(ctx) principals sharing one IP, but is IDENTICAL for two anonymous requests from the same IP+Host (Pitfall 4). + + surf.Limiter reproduces Laravel's fixed-window ThrottleRequests wire contract exactly, including header placement and stacking; ClientIP is the single trusted-proxy-aware resolver. + + + + Task 2 (fonoteka.go): Register the five buckets, attach throttle:fonoteka-api-token, port PublicShareHeaders, declare the remaining route groups + fonoteka.go/plugins/golem15/fonoteka/plugin.go, fonoteka.go/plugins/golem15/fonoteka/routes.go, fonoteka.go/plugins/golem15/fonoteka/middleware/public_share_headers.go, fonoteka.go/plugins/golem15/fonoteka/middleware/public_share_headers_test.go, fonoteka.go/config/http.yaml + + summercms.go/surf/limiter.go, summercms.go/surf/clientip.go (Task 1 output) + fonoteka.go/plugins/golem15/fonoteka/plugin.go (post-06-01 -- Boot/Middlewares/MiddlewareFactories to extend) + fonoteka.go/plugins/golem15/fonoteka/routes.go (post-06-01 -- the two existing genres groups) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/routes.php lines 34-67, 333-431 (bucket definitions, jwt_locale group, onboarding group, invitation inspection route, public-share/public-wishlist group, PublicShareHeaders wiring) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/middleware/PublicShareHeaders.php (full -- exact headers and 429 rewrite) + fonoteka.go/parity/manifest.yaml lines 1-9 (the seven auth_groups: jwt_locale, jwt, onboarding, public_invitation, public_share, personal_token, oauth) + + + In plugin.go, add Buckets() map[string]surf.Bucket implementing surf.BucketProvider, building each Key closure with a trusted := surf.TrustedProxies(p.app.Config) captured once: "fonoteka-api-token" {Max:60, Decay:time.Minute, Key: func(r){ if tok, ok := bouncer.Credential(r.Context()).(*models.ApiToken); ok { return "tok:"+strconv.FormatUint(uint64(tok.ID),10) }; return surf.ClientIP(r, trusted) }} (note: this bucket sits at GROUP level in PHP, meaning the inv_token guard middleware must run BEFORE this bucket's Key resolver sees a credential -- confirm the group's middleware order in routes.go is surf.Use("inv_token", "inv.scope:", "throttle:fonoteka-api-token") so bouncer.Credential is already populated); "fonoteka-oauth-token" {Max:30, Decay:time.Minute, Key: func(r){ return "oauthtok:"+surf.ClientIP(r, trusted) }}; "fonoteka-oauth-register" {Max:30, Decay:time.Minute, Key: func(r){ return "oauthreg:"+surf.ClientIP(r, trusted) }}; "fonoteka-public-token" {Max:60, Decay:time.Minute, Key: func(r){ return "pubtok:"+r.PathValue("token") }}; "fonoteka-public-ip" {Max:120, Decay:time.Minute, Key: func(r){ return surf.ClientIP(r, trusted) }}. Register these by returning them from Buckets() (Assemble's new BucketProvider loop calls lim.RegisterBucket(p.ID(), name, b) for each). + + In routes.go: on the existing personal-token r.Group("/api/v1/fonoteka", ...) call from 06-01, change surf.Use("inv_token", "inv.scope:read") to surf.Use("inv_token", "inv.scope:read", "throttle:fonoteka-api-token") and remove the "// TODO(06-02)" comment left by 06-01 -- this is the landing spot it named. Add four more r.Group calls, each with ZERO g.Get/g.Post calls inside (empty body -- no ported handler exists for any route in these groups yet; the group declaration itself is what D-15 requires, and its middleware STRING LIST is what a boot-time smoke test (Task 3) proves resolves without error): + - r.Group("/_fonoteka/api/v1", surf.Use("jwt.auth"), func(g pact.Router) {}) for jwt_locale (PHP's me/locale group carries jwt.auth+bindings only, no inv.must-change-password -- do not add inv.must-change-password here even though the main jwt group has it). + - r.Group("/_fonoteka/api/v1", surf.Use("throttle:10,1"), func(g pact.Router) {}) for onboarding. + - r.Group("/_fonoteka/api/v1/invitations", surf.Use("throttle:10,1"), func(g pact.Router) {}) for public_invitation (PHP registers this as a single ungrouped Route::get with inline middleware -- represented here as a one-off empty group for the manifest's auth_group grouping). + - r.Group("/_fonoteka/api/v1", surf.Use("public.share-headers", "throttle:fonoteka-public-token", "throttle:fonoteka-public-ip"), func(g pact.Router) {}) for public_share/public_wishlist (both PHP prefixes share one middleware stack; one Go group declaration covers both per D-15's "framework fixture routes in tests" allowance for proving behavior without a real handler). + Add a package comment above these four calls citing routes.php's exact line ranges and noting each is deliberately empty pending its real handler in a later phase (not a 501 shell: zero routes are registered at all). + + Create middleware/public_share_headers.go: PublicShareHeaders(next http.Handler) http.Handler (fixed middleware, not a factory -- register it in Middlewares() under the name "public.share-headers"). Wrap next in an http.ResponseWriter interceptor (buffer the status code via a small responseRecorder wrapper, matching the codebase's plain-stdlib style) so that: if the final status is 429, rewrite the body to {"error":"Too many requests"} while copying Retry-After/X-RateLimit-Limit/X-RateLimit-Remaining/X-RateLimit-Reset from whatever the limiter already set; on EVERY response (429 or not) set X-Robots-Tag: noindex, nofollow and Cache-Control: private, no-store (exact PHP string order, matching PublicShareHeaders.php lines 50-54) after next.ServeHTTP returns control, since Go's http.ResponseWriter forbids setting headers after WriteHeader is called -- design the wrapper to capture headers/status BEFORE flushing to the real ResponseWriter (buffer the whole response in memory, matching this middleware's small, bounded-size use case, then write final headers + body once). + + In plugin.go's Middlewares(), add "public.share-headers": middleware.PublicShareHeaders to the returned map. + + In fonoteka.go/config/http.yaml, add a trusted_proxies: [] key (empty list, matching "no proxies trusted yet" -- an operator populates this in a later phase's deployment work) nested appropriately so it resolves to http.trusted_proxies via compass's section-naming (file is already the "http" section, so add a top-level trusted_proxies: [] key alongside the existing cors: block). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && go vet ./... && go test ./plugins/golem15/fonoteka/... -run TestPublicShareHeaders -short + + + - go test ./plugins/golem15/fonoteka/middleware/... -run TestPublicShareHeaders asserts: a 429 from an inner handler is rewritten to {"error":"Too many requests"} with Retry-After/X-RateLimit-* preserved; a 200 response still gets X-Robots-Tag and Cache-Control but its body is untouched. + - grep -n "fonoteka-api-token\|fonoteka-oauth-token\|fonoteka-oauth-register\|fonoteka-public-token\|fonoteka-public-ip" fonoteka.go/plugins/golem15/fonoteka/plugin.go shows all five bucket names with their exact Max values (60, 30, 30, 60, 120). + - The personal-token genres route's middleware list (readable via the route table once 06-03 exists, or directly via a Task-3 boot-smoke test in the meantime) includes "throttle:fonoteka-api-token" as the last entry. + + All five buckets are registered and available; the token group is throttled; the public-share group's 429 shape is ported; the remaining four PHP route groups are declared as structurally correct, handler-free group builders. + + + + Task 3 (fonoteka.go, parity, .planning): APP_DEBUG=false fix, boot-smoke coverage for all groups, ROADMAP/REQUIREMENTS wording correction + fonoteka.go/parity/php_parity.sh, fonoteka.go/plugins/golem15/fonoteka/routes_bucket_test.go, .planning/ROADMAP.md, .planning/REQUIREMENTS.md + + fonoteka.go/parity/php_parity.sh (full -- the export_env() function that does not currently set APP_DEBUG) + fonoteka.go/parity/manifest.yaml (search for any 429/throttle/error-shaped fixture bodies that would change under APP_DEBUG=false) + .planning/ROADMAP.md Phase 6 section (Success Criteria item 2: "All seven named rate-limit buckets") + .planning/REQUIREMENTS.md HTTP-04 line ("ports Płytarium's seven named buckets and inline throttles 1:1") + + + In php_parity.sh's export_env() (or the equivalent function that sets environment variables before booting the isolated PHP instance), add export APP_DEBUG=false alongside the existing exports, so any fixture recorded/re-recorded from this point on reflects the production (non-debug) error body shape per the user-resolved Open Question 2. Search manifest.yaml and fixtures/routes/*.yaml for any existing recorded fixture whose body would plausibly differ under debug vs non-debug (error/exception-shaped bodies, 4xx/5xx cases with a "trace" or "exception" key) and flag them in the plan's SUMMARY if any are found needing re-recording (this repo's CACHE_DRIVER=array means live PHP 429s specifically cannot be recorded from this harness at all -- per the user-resolved note, 429 bodies/headers are asserted directly in Go tests from the Laravel vendor source already cited in 06-RESEARCH.md, not from recorded fixtures; do not attempt to record a 429 fixture from this harness). + + Create routes_bucket_test.go: a boot-smoke test that calls the same app.Handler(...)-equivalent path parity/parity_test.go's newTarget uses (or a lighter-weight surf.Assemble(app, plugins) call against a real activated plugin set) and asserts no error -- this exercises every bucket name and every "throttle:..."/"inv.scope:..."/"public.share-headers" middleware string across all six now-declared route groups (jwt, jwt_locale, onboarding, public_invitation, public_share/public_wishlist, personal_token) without needing a real handler in any of the five non-genres groups. Add a second assertion using the route table if 06-03 has already landed in this working tree (guard with a build check or skip gracefully if surf.Router.Routes() does not exist yet in this wave -- 06-02 runs before 06-03, so prefer NOT depending on Routes() here at all; the plain no-error boot assertion is sufficient for this plan). + + In .planning/ROADMAP.md, Phase 6 Success Criteria item 2, change "All seven named rate-limit buckets" to "All five named rate-limit buckets". In .planning/REQUIREMENTS.md, HTTP-04's description, change "ports Płytarium's seven named buckets and inline throttles 1:1" to "ports Płytarium's five named buckets and inline throttles 1:1". Commit this docs change SEPARATELY from the code changes in this plan (per CLAUDE.md: "planning docs and code in separate commits") -- do not include these two files in the same commit as the Go/shell changes. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && go vet ./... && go test ./plugins/golem15/fonoteka/... -run TestAllRouteGroupsBoot -short && grep -c "APP_DEBUG=false" parity/php_parity.sh + + + - grep -c "APP_DEBUG=false" fonoteka.go/parity/php_parity.sh returns at least 1. + - The boot-smoke test passes, proving all five bucket names and every declared group's middleware list resolves at Assemble time. + - grep -n "seven named rate-limit buckets\|seven named buckets" .planning/ROADMAP.md .planning/REQUIREMENTS.md returns zero matches; grep -n "five named" returns at least one match in each file. + + The parity harness records future fixtures against production-shaped error bodies; every Phase-6-scope route group boots cleanly with its real middleware stack; the roadmap/requirements bucket-count miscount is corrected in a docs-only commit. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|--------------| +| client -> X-Forwarded-For | untrusted header, must only be honored when RemoteAddr is a configured trusted proxy | +| client -> rate-limit keys | an attacker-controlled IP/token/route-param feeds directly into the Store's key namespace | +| public-share group -> unauthenticated caller | the only Phase-6 surface exposed with zero credential requirement; its error responses must never leak internals | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-06-06 | Denial of Service | surf.ClientIP | mitigate | X-Forwarded-For honored only when RemoteAddr is inside http.trusted_proxies; an untrusted caller cannot spoof their limiter key (Task 1 spoofing test) | +| T-06-07 | Information Disclosure | 429 response on the public-share group | mitigate | PublicShareHeaders rewrites any 429 (framework-default or otherwise) to a fixed JSON body before it reaches an anonymous caller, and sets no-index/no-store headers on every response in that group | +| T-06-08 | Denial of Service | in-process MemoryStore under high cardinality (many distinct keys, e.g. one per IP) | accept | v1 ships an unbounded-until-swept map per CONTEXT D-03's explicit "no otter/cooler this phase" decision; the sweep goroutine bounds long-term growth to roughly one decay window's worth of distinct keys, acceptable for a single-instance v1 deployment | +| T-06-09 | Repudiation | Recorded parity fixtures under the wrong APP_DEBUG value | mitigate | php_parity.sh now pins APP_DEBUG=false so all future recordings are production-shaped; this plan audits existing fixtures for drift rather than assuming none exists | + + + +cd summercms.go && go vet ./... && go test ./surf/... -short +cd ../fonoteka.go && go vet ./... && go test ./plugins/golem15/fonoteka/... -short + + + +- surf.Limiter reproduces Laravel's fixed-window ThrottleRequests contract, including success-response headers and stacking. +- The five fonoteka buckets are registered with exact names/limits/keys; the token group is throttled by fonoteka-api-token. +- PublicShareHeaders ports its exact 429 rewrite and unconditional headers. +- All six Phase-6-scope route groups (everything but oauth) boot cleanly with their real middleware stacks. +- ROADMAP.md/REQUIREMENTS.md say five buckets, corrected in a docs-only commit. +- go vet ./... and go test ./... are green in both repos. + + + +Create `.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-02-SUMMARY.md` when done + diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-03-PLAN.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-03-PLAN.md new file mode 100644 index 0000000..c3ea3b0 --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-03-PLAN.md @@ -0,0 +1,333 @@ +--- +phase: 06-http-routing-auth-groups-and-rate-limiting +plan: 03 +type: execute +wave: 3 +depends_on: ["06-02"] +files_modified: + - summercms.go/pact/capabilities.go + - summercms.go/surf/router.go + - summercms.go/surf/router_test.go + - summercms.go/surf/routetable.go + - summercms.go/surf/routetable_test.go + - summercms.go/surf/routelist_command.go + - summercms.go/surf/cors.go + - summercms.go/surf/cors_test.go + - summercms.go/surf/bodylimit.go + - summercms.go/surf/bodylimit_test.go + - summercms.go/wire/response.go + - summercms.go/wire/response_test.go + - summercms.go/internal/build/build.go + - summercms.go/internal/build/build_test.go + - fonoteka.go/plugins/golem15/fonoteka/routes.go + - fonoteka.go/plugins/golem15/fonoteka/plugin.go + - fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go + - fonoteka.go/config/http.yaml + - fonoteka.go/scripts/check-openapi.sh + - fonoteka.go/docs/openapi.json +autonomous: false +requirements: [HTTP-06, HTTP-08, HTTP-09] +user_setup: + - service: production-host + why: "http.body_limits config keys need the real client_max_body_size/post_max_size/upload_max_filesize values from the production nginx vhost and php.ini, which are not in any repo (06-RESEARCH.md Assumption A2 / Open Question 2)" + dashboard_config: + - task: "Read client_max_body_size (nginx), post_max_size and upload_max_filesize (php.ini) off the production host and report the three numbers" + location: "production host, operator-managed nginx vhost and php.ini, outside version control per docs/deploy/plytarium.com.md" + +must_haves: + truths: + - "A group flagged Raw refuses any middleware tagged house-envelope/error at registration time, failing boot rather than silently mounting it (D-16)" + - "A panic inside a raw-group route returns a bare 500 with no JSON body, while a panic in any other route still returns the house {\"error\":true,\"message\":\"Internal server error\"} body (D-16)" + - "The .well-known/oauth-authorization-server and oauth/mcp/* group is declared raw with fonoteka-oauth-token and fonoteka-oauth-register attached to its two future routes, and carries no handlers yet (D-16)" + - "A route table (method, pattern, plugin, middleware chain, raw flag) is exposed and used both by a test and by a route:list CLI command (D-16)" + - "No route under /api/v1/fonoteka carries jwt.auth in its middleware chain, and no route under /_fonoteka/api/v1 carries inv_token or an inv.scope: entry, verified by inspecting the route table (D-16, completes T-06-02 from 06-01)" + - "wire.WriteJSON reproduces genre_controller.go's writeJSON byte-for-byte (SetEscapeHTML(false), trailing-newline trim); wire.Time marshals as YYYY-MM-DDTHH:MM:SS+00:00, never Z; a never-nil wire.Slice helper exists (D-17)" + - "GET /_fonoteka/api/v1/genres carries no CORS headers on any response; GET /api/v1/fonoteka/genres carries Access-Control-Allow-Origin: * on every response, matching config/cors.php's paths list excluding _fonoteka/api/* (D-18, Pitfall 10)" + - "http.body_limits config keys exist with clearly-marked INTERIM defaults; a blocking checkpoint records the operator-reported production values before this plan is considered complete (D-18, user-resolved Open Question 1)" + - "swag-annotated ListGenres produces a committed OpenAPI document that openapi-typescript converts into valid TypeScript with zero errors (D-08/HTTP-08)" + artifacts: + - path: summercms.go/surf/routetable.go + provides: "RouteInfo{Method,Pattern,PluginID,Middleware,Raw} and Router.Routes()" + - path: summercms.go/wire/response.go + provides: "WriteJSON, WriteOpaque500, Time, TriBool, Slice[T]" + - path: summercms.go/surf/cors.go + provides: "Path-scoped, config-driven CORS matching config/cors.php's paths/methods/origins/headers/max_age/credentials" + - path: fonoteka.go/docs/openapi.json + provides: "Generated OpenAPI document from swag annotations on the real genres handler" + key_links: + - from: fonoteka.go/plugins/golem15/fonoteka/routes.go + to: summercms.go/surf/router.go + via: "the oauth group is declared via GroupRaw, not Group" + pattern: "GroupRaw\\(" + - from: summercms.go/surf/routelist_command.go + to: summercms.go/surf/routetable.go + via: "the route:list command calls Router.Routes() and renders it as a table" + pattern: "\\.Routes\\(\\)" + - from: fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go + to: summercms.go/wire/response.go + via: "GenreList/writeJSON usage is replaced by or delegates to wire helpers" + pattern: "wire\\.(WriteJSON|Time|Slice)" +--- + + +Ship the structural guarantees HTTP-06/HTTP-08/HTTP-09 require and that no later phase can retrofit without a breaking change: a `Raw` group flag enforced at registration (not by convention), a route table + `route:list` CLI command that makes group mutual-exclusivity and the OAuth exemption independently verifiable, promoted response-convention helpers (`wire` package), path-scoped config-driven CORS matching `config/cors.php` exactly, per-group JSON body limits, and the swag/`openapi-typescript` pipeline on the one real handler this phase has (`ListGenres`). + +Purpose: this plan is the "contract surface" of the phase -- everything a future handler or a future auth guard (OAuth, Phase 8) must be structurally prevented from getting wrong, proven by inspection rather than trusted by convention. +Output: `Router.GroupRaw`, `Router.Routes()`, `surf.RouteListCommand`, `wire.WriteJSON`/`Time`/`TriBool`/`Slice`, path-scoped `surf.cors`, per-group body limits, the oauth group declared raw, a committed OpenAPI document, and a resolved (not guessed) production body-size limit. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.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-PATTERNS.md +@.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-SUMMARY.md +@.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-02-SUMMARY.md + + + +Extended summercms.go/pact/capabilities.go (Router interface): + +type Router interface { + Group(prefix string, middleware []string, fn func(Router)) + GroupRaw(prefix string, middleware []string, fn func(Router)) + Get(path string, handler http.HandlerFunc, middleware ...string) + Post(path string, handler http.HandlerFunc, middleware ...string) + Put(path string, handler http.HandlerFunc, middleware ...string) + Patch(path string, handler http.HandlerFunc, middleware ...string) + Delete(path string, handler http.HandlerFunc, middleware ...string) + Where(param, pattern string) + WhereIn(param string, values ...string) +} + +New file summercms.go/surf/routetable.go: + +package surf + +type RouteInfo struct { + Method string + Pattern string + PluginID string + Middleware []string + Raw bool +} + +// Routes returns a defensive copy of every registered route, post-Assemble. +func (r *Router) Routes() []RouteInfo + +New file summercms.go/surf/routelist_command.go: + +package surf + +// RouteListCommand mirrors ServeCommand's shape: builds the router the same +// way (party.Activate has already run; this command receives app/plugins), +// but renders Routes() as a table instead of serving. +func RouteListCommand(app *backpack.App, plugins []party.Plugin) bonfire.Command + +Router internals (summercms.go/surf/router.go) needed to support the above: +- route struct gains a raw bool field, set from the declaring Group/GroupRaw + at add() time (thread it exactly like groupMW already threads through + Group/Get/Post/etc). +- Group gains a raw bool field, inherited (once true, stays true through + nested Group() calls -- a raw group cannot spawn a non-raw child). +- RegisterMiddleware and RegisterMiddlewareFactory both gain a variant that + tags an entry as house-envelope/error (see Task 1 action for the exact + shape); wrap() refuses a raw route referencing a house-tagged name at + Assemble time (registration time), returning a "surf: raw group cannot use + house-envelope middleware %q (plugin %q)" error -- not a runtime check. +- compile()/wrap() recovery moves from one blanket recoverJSON(cors(...)) + wrap in compile() to a per-route choice inside wrap(): raw routes get a + bare-500 recoverBare; non-raw routes keep recoverJSON's existing + {"error":true,"message":"Internal server error"} body. +- Assemble no longer needs a package-level BuildRouter split: RouteListCommand + can call the same Assemble(app, plugins) as ServeCommand and then call + .(*Router) via a small unexported adapter, OR Assemble is refactored into + BuildRouter(app, plugins) (*Router, error) + Assemble = BuildRouter+compile + -- choose the refactor (BuildRouter) since it avoids any type assertion on + the returned http.Handler and is the cleaner seam for route:list. + +New file summercms.go/wire/response.go: + +package wire + +// WriteJSON is byte-identical to genre_controller.go's private writeJSON +// (SetEscapeHTML(false), trailing-newline trim via bytes.TrimSuffix). +func WriteJSON(w http.ResponseWriter, status int, v any) +func WriteOpaque500(w http.ResponseWriter) + +// Time marshals as Carbon's +00:00 form (2006-01-02T15:04:05+00:00), never +// Go's default "Z". Unmarshal accepts both +00:00 and Z for read paths. +type Time struct{ time.Time } +func (t Time) MarshalJSON() ([]byte, error) +func (t *Time) UnmarshalJSON(b []byte) error + +// TriBool keeps PHP's nullable-boolean tri-state: Valid=false marshals null. +type TriBool struct{ Valid, Value bool } +func (b TriBool) MarshalJSON() ([]byte, error) + +// Slice guarantees a never-nil JSON array. +func Slice[T any](s []T) []T + + + + + + Task 1 (summercms.go): Raw group enforcement, route table, and the route:list command + summercms.go/pact/capabilities.go, summercms.go/surf/router.go, summercms.go/surf/router_test.go, summercms.go/surf/routetable.go, summercms.go/surf/routetable_test.go, summercms.go/surf/routelist_command.go, summercms.go/internal/build/build.go, summercms.go/internal/build/build_test.go + + summercms.go/surf/router.go (full, post-06-02 -- route/Group structs, add/compile/wrap, RegisterMiddleware, Assemble, recoverJSON, cors) + summercms.go/surf/serve.go (full -- ServeCommand's bonfire.Command shape to mirror for RouteListCommand) + summercms.go/bonfire/command.go (Command/Output shapes; bonfire/widgets.go's Table(headers []string, rows [][]string) signature) + summercms.go/lagoon/commands.go (RuntimeCommands -- an existing bonfire.Command-returning function to mirror for style, not for content) + summercms.go/internal/build/build.go lines 80-127 (generateMain -- exact lines emitting "commands = append(commands, surf.ServeCommand(app, plugins))"; add the route:list line directly after it) + summercms.go/internal/build/build_test.go (existing assertions on generated main.go content to extend) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md Pattern 3 (raw group structural exemption, lines 303-321) + + + In pact/capabilities.go, add GroupRaw(prefix string, middleware []string, fn func(Router)) to the Router interface, same signature shape as Group. + + In surf/router.go: add raw bool to both Group and route. Add GroupRaw on *Router and *Group mirroring Group's existing body exactly, except the constructed child Group has raw: true; when Group() is called on an ALREADY-raw Group (g.raw == true), the child it constructs must also be raw:true (sticky inheritance -- a raw group cannot spawn a non-raw child via a later Group() call). Thread raw through add()'s signature (alongside pluginID/prefix/groupMW/method/path/handler/extra) so route.raw is set from whichever Group/GroupRaw declared it. + + Add a houseTagged bool field to namedMiddleware and namedMiddlewareFactory. Add RegisterHouseMiddleware(pluginID, name string, fn pact.Middleware) error and RegisterHouseMiddlewareFactory(pluginID, name string, fn func(string) pact.Middleware) error, each calling the existing Register*/RegisterMiddlewareFactory internals but setting houseTagged: true on the stored entry (do not duplicate the dup-check logic -- factor a shared unexported registerNamed(pluginID, name string, tagged bool, ...) if that keeps both call sites DRY, or two thin wrappers if simpler; either is acceptable as long as the dup/empty-name checks are not duplicated verbatim). In fonoteka.go's plugin.go (see Task 3 of this plan for the actual call-site change), "inv.must-change-password" switches from RegisterMiddleware to RegisterHouseMiddleware. + + In wrap()'s middleware-resolution loop: after resolving a name to either a namedMiddleware or a factory-built middleware, check `if rt.raw && resolvedHouseTagged { return nil, fmt.Errorf("surf: raw group cannot use house-envelope middleware %q (plugin %q)", name, rt.pluginID) }` -- this makes the refusal a registration-time (Assemble-time) failure, since Assemble already calls r.wrap(rt) once per route specifically to surface these errors before compile(). + + Change wrap()'s outer recovery: instead of compile() wrapping the whole mux in one recoverJSON(cors(...)), move recovery INTO wrap() per-route: `if rt.raw { h = recoverBare(h) } else { h = recoverJSON(h) }` as the outermost wrap (applied after the locale() wrap, i.e. truly outermost). Add recoverBare(next http.Handler) http.Handler mirroring recoverJSON's shape but on panic writing only w.WriteHeader(http.StatusInternalServerError) with NO body and NO Content-Type header set (D-16: "bare 500, no house JSON body"). compile()'s final return simplifies to just the CORS wrapper around mux (CORS itself becomes path-scoped in Task 3 of this plan -- for THIS task, compile() can keep calling the existing blanket cors(r.origins, mux) unchanged; Task 3 replaces it). + + Create routetable.go: RouteInfo exactly as specified in the interfaces block; (r *Router) Routes() []RouteInfo iterating r.routes and returning a defensive copy (new slice, new []string per entry -- do not alias r.routes[i].middleware). + + Refactor Assemble into BuildRouter(app *backpack.App, plugins []party.Plugin) (*Router, error) containing everything Assemble currently does EXCEPT the final r.compile() call (return r, nil instead); Assemble(app, plugins) becomes `r, err := BuildRouter(app, plugins); if err != nil { return nil, err }; return r.compile()`. This lets RouteListCommand call BuildRouter directly and read .Routes() without compiling/serving. + + Create routelist_command.go: RouteListCommand(app, plugins) bonfire.Command{Name: "route:list", Description: "List registered HTTP routes", Run: func(ctx, in, out) error { r, err := BuildRouter(app, plugins); if err != nil { return err }; rows := make([][]string, 0, len(r.Routes())); for _, rt := range r.Routes() { rows = append(rows, []string{rt.Method, rt.Pattern, rt.PluginID, strings.Join(rt.Middleware, ","), strconv.FormatBool(rt.Raw)}) }; out.Table([]string{"Method","Pattern","Plugin","Middleware","Raw"}, rows); return nil }}. + + In internal/build/build.go's generateMain, add a line `b.WriteString("\tcommands = append(commands, surf.RouteListCommand(app, plugins))\n")` directly after the existing `surf.ServeCommand` append line. Update build_test.go's existing content-assertion test to also assert the generated main.go contains "surf.RouteListCommand". + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test ./surf/... -run TestRawGroup -short && go test ./surf/... -run TestRouteTable -short && go test ./internal/build/... -short + + + - A test registers a house-tagged middleware and a raw group referencing it by name; asserts Assemble/BuildRouter returns an error naming the middleware and the plugin. + - A test registers a raw group with a panicking handler; asserts the response is status 500 with an EMPTY body and no Content-Type header; a parallel non-raw panicking route still returns the existing {"error":true,"message":"Internal server error"} JSON body. + - A test asserts Router.Routes() reflects Raw:true only for routes declared inside GroupRaw, and that a route declared via a plain Group() nested inside a GroupRaw() is also Raw:true (sticky inheritance). + - go test ./internal/build/... confirms the generated main.go template contains both "surf.ServeCommand" and "surf.RouteListCommand". + + Raw-group refusal is a registration-time failure, not a runtime check; the route table and route:list command exist and are independently testable. + + + + Task 2 (summercms.go, fonoteka.go): Response-convention wire package and the swag/openapi-typescript pipeline + summercms.go/wire/response.go, summercms.go/wire/response_test.go, fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go, fonoteka.go/scripts/check-openapi.sh, fonoteka.go/docs/openapi.json + + fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go (full -- writeJSON/writeOpaque500 to promote verbatim, GenreList/GenreAggregate to annotate) + summercms.go/tide/normalize.go lines 1-20 (the carbonOffsetRe regex: ^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\+00:00$ -- the exact target format, no fractional seconds, literal +00:00 not Z) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md Standard Stack (swaggo/swag v1.16.6, openapi-typescript 7.13.0) and Installation section + + + Create wire/response.go: WriteJSON and WriteOpaque500, copied verbatim in behavior from genre_controller.go's private writeJSON/writeOpaque500 (same bytes.Buffer + json.Encoder with SetEscapeHTML(false) + bytes.TrimSuffix(buf.Bytes(), []byte("\n")) trick, same {"error":true,"message":"Internal server error"} body for WriteOpaque500). Time struct{ time.Time } with MarshalJSON formatting via t.UTC().Format("2006-01-02T15:04:05") + "+00:00" (do NOT use Go's Z07:00 verb, which emits "Z" for UTC -- must literally match tide/normalize.go's carbonOffsetRe) and UnmarshalJSON accepting a quoted RFC3339-ish string with either +00:00 or Z (use time.Parse with two candidate layouts, first match wins). TriBool{Valid, Value bool} with MarshalJSON returning "null" when !Valid, else "true"/"false" as bare JSON booleans (not quoted). Slice[T any](s []T) []T returning s unchanged if non-nil, else []T{} (mirrors classes/serialize.go's existing inline pattern -- this is the promoted, reusable form of it). + + In genre_controller.go, replace the private writeJSON/writeOpaque500 function bodies with thin delegations to wire.WriteJSON/wire.WriteOpaque500 (keep the local function names and call sites unchanged so no other file in this package needs to change -- only the two function bodies become one-line delegations, proving the promotion without a wider refactor this phase). Add swag doc comments directly above the ListGenres function: `// @Summary List genres` / `// @Description Returns the global genre pool with tenant-scoped album counts` / `// @Tags genres` / `// @Produce json` / `// @Param non_empty query string false "1 to only return genres with albums"` / `// @Success 200 {object} GenreList` / `// @Router /_fonoteka/api/v1/genres [get]` (swag's comment-annotation syntax, scanning this handler's existing signature -- no signature change). + + Create scripts/check-openapi.sh (fonoteka.go/scripts/, executable): runs `go run github.com/swaggo/swag/cmd/swag@v1.16.6 init --generalInfo plugins/golem15/fonoteka/controllers/genre_controller.go --output docs --parseDependency` (adjust flags to this repo's actual layout after a first local run) to produce docs/openapi.json (swag's default is swagger.json/yaml -- pin the output filename explicitly via swag's -o/--output and --outputTypes json flags so the committed artifact is deterministic), then runs `npx --yes openapi-typescript@7.13.0 docs/openapi.json -o /dev/null` (validity check only -- Phase 10 owns wiring real output into the admin SPA) and fails the script (non-zero exit) if either command errors, and a third check: `git diff --exit-code docs/openapi.json` style drift check IF run in CI (document this as the drift-check step; for local/plan execution, running the script once and committing docs/openapi.json is sufficient). + + Run the script once locally, commit the resulting docs/openapi.json. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test ./wire/... -short && cd ../fonoteka.go && bash scripts/check-openapi.sh + + + - wire.Time{}.MarshalJSON on a known UTC time produces a string matching tide/normalize.go's carbonOffsetRe exactly (test asserts via that same regex, imported or copied into wire/response_test.go). + - wire.TriBool{Valid:false} marshals to null; {Valid:true,Value:false} marshals to false (bare, not "false" string). + - wire.Slice(nil) returns a non-nil empty slice that marshals to []. + - fonoteka.go/scripts/check-openapi.sh exits 0 and fonoteka.go/docs/openapi.json exists and is valid JSON containing a /_fonoteka/api/v1/genres path. + + wire package exists and is unit-tested independent of any handler; genre_controller.go delegates to it; a real, committed OpenAPI document is generated from a real handler's annotations and validated by openapi-typescript. + + + + Task 3 (fonoteka.go): Path-scoped CORS, per-group body limits, the raw OAuth group, and the production body-limit checkpoint + summercms.go/surf/cors.go, summercms.go/surf/cors_test.go, summercms.go/surf/bodylimit.go, summercms.go/surf/bodylimit_test.go, fonoteka.go/plugins/golem15/fonoteka/routes.go, fonoteka.go/plugins/golem15/fonoteka/plugin.go, fonoteka.go/config/http.yaml + + summercms.go/surf/router.go (post-Task-1 of this plan -- compile()'s current blanket cors(r.origins, mux) call to replace) + summercms.go/compass/config.go (LoadSection with koanf tags -- the mechanism for CORSConfig) + /media/nvme/dev/golem15/fonoteka/config/cors.php (full, already read -- exact paths/methods/origins/headers/max_age/credentials values) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/routes.php lines 521-567 (the oauth group: bindings only, two POST routes each with one throttle, no jwt.auth/inv.scope, handlers arrive Phase 8) + fonoteka.go/parity/fixtures/routes/GET__api_v1_fonoteka_genres_personal_token.yaml (confirms wildcard CORS on the personal-token group) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md Assumption A2 / Open Question 2 (body-size limits not in any repo) + + + Create surf/cors.go: CORSConfig struct with koanf tags matching cors.php's keys exactly (Paths []string `koanf:"paths"`, AllowedMethods []string `koanf:"allowed_methods"`, AllowedOrigins []string `koanf:"allowed_origins"`, AllowedOriginsPatterns []string `koanf:"allowed_origins_patterns"`, AllowedHeaders []string `koanf:"allowed_headers"`, ExposedHeaders []string `koanf:"exposed_headers"`, MaxAge int `koanf:"max_age"`, SupportsCredentials bool `koanf:"supports_credentials"`); LoadCORSConfig(cfg *compass.Config) (CORSConfig, error) calling cfg.LoadSection("http.cors", &out). A pathScopedCORS(cfg CORSConfig, next http.Handler) http.Handler replacing the existing blanket cors() call in compile(): for each request, check r.URL.Path (with leading slash stripped) against every glob in cfg.Paths using path.Match (Laravel's glob syntax `api/*` maps directly to Go's path.Match "api/*" against the leading-slash-stripped path); if no glob matches, call next.ServeHTTP with NO CORS headers set (this is what makes /_fonoteka/api/* get nothing, since it is not in the paths list); if a glob matches, apply the existing header-setting logic (Origin allow-list or "*", Vary, Allow-Headers, Allow-Methods) using cfg's fields instead of the hardcoded strings currently in cors(), plus Access-Control-Max-Age when cfg.MaxAge > 0 and Access-Control-Allow-Credentials: true when cfg.SupportsCredentials, and still short-circuit OPTIONS preflight with 204 exactly as today. In router.go's compile(), replace the blanket `cors(r.origins, mux)` call with `pathScopedCORS(corsCfg, mux)` where corsCfg is loaded once in BuildRouter via LoadCORSConfig(app.Config) and threaded through compile() (add a corsCfg field to Router, set in BuildRouter before compile is reached). + + Create surf/bodylimit.go: two config keys read once in BuildRouter -- http.body_limits.default_bytes and http.body_limits.upload_bytes (both int64 via a small helper since compass.Config.Int returns int; convert). Wrap every non-raw route's handler (innermost, next to constrain()) with http.MaxBytesReader(w, r.Body, defaultBytes) via a bodyLimit(defaultBytes int64) middleware applied unconditionally in wrap() (raw routes are exempt -- RFC endpoints like /oauth/mcp/token have their own well-known limits and must not gain a house-specific cap). Add an opt-in named middleware factory "body.limit" (registered by the router itself, like "throttle") whose param is a byte count string, letting a future upload route request a larger cap by listing "body.limit:20971520" (20 MiB) etc. explicitly in its middleware list, overriding the default for that one route (apply it as the innermost wrap so it supersedes the blanket default-bytes reader for that route only). + + In fonoteka.go/config/http.yaml, add: cors: paths: ["api/*", "_user/api/*", "_journal/api/*", "_feedback/api/*", "oauth/mcp/*"], allowed_methods: ["*"], allowed_origins: ["*"], allowed_origins_patterns: [], allowed_headers: ["*"], exposed_headers: [], max_age: 0, supports_credentials: false (byte-for-byte from config/cors.php); body_limits: default_bytes: 8388608 (8 MiB, PHP's stock post_max_size, marked with a comment "INTERIM default per 06-RESEARCH.md Assumption A2 -- replace with the real production client_max_body_size/post_max_size once read off the host"), upload_bytes: 2097152 (2 MiB, PHP's stock upload_max_filesize, same INTERIM comment). + + In fonoteka.go/plugins/golem15/fonoteka/plugin.go, switch the "inv.must-change-password" registration from RegisterMiddleware to RegisterHouseMiddleware (Task 1's new capability) -- this is the concrete proof case for D-16's registration-time refusal (a raw group referencing "inv.must-change-password" must now fail boot). + + In fonoteka.go/plugins/golem15/fonoteka/routes.go, add the oauth group via r.GroupRaw("/", surf.Use(), func(g pact.Router) {}) -- bindings has no Go equivalent (no route-model binding concept), so the middleware list is empty; the group exists purely to prove GroupRaw's structural behavior at this prefix and to be the landing spot for Phase 8's two real routes, each of which will individually add "throttle:fonoteka-oauth-token"/"throttle:fonoteka-oauth-register" per-route (not group-level, matching routes.php's actual per-route ->middleware() calls at lines 561/565) -- do not attach the throttle names to the empty GroupRaw call itself, since PHP attaches them per-route, not per-group, and there is no route to attach them to yet. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test ./surf/... -run TestCORS -short && go test ./surf/... -run TestBodyLimit -short + + + - A test against the assembled fonoteka router asserts a request to /_fonoteka/api/v1/genres carries NO Access-Control-Allow-Origin header, and a request to /api/v1/fonoteka/genres carries Access-Control-Allow-Origin: * (Pitfall 10, matching the recorded parity fixture). + - A test asserts a request body larger than http.body_limits.default_bytes on a non-raw route fails with the standard http.MaxBytesReader error surfaced as a 4xx/error response (exact status code documented in the test, matching whatever the handler's own body-read error path already returns via the existing recoverJSON/error handling -- if no handler currently reads a body, test MaxBytesReader directly against a framework fixture route). + - A test asserts the oauth GroupRaw declaration has Raw:true in the route table (even with zero routes, assert via boot-time introspection of the Group's raw flag if Routes() has nothing to show for an empty group -- prefer adding one framework-fixture raw route inside a TEST-only GroupRaw call, not inside production routes.go, to get a concrete RouteInfo{Raw:true} entry to assert against). + - A test asserts registering "inv.must-change-password" (now house-tagged) inside a raw group fails Assemble/BuildRouter with an error naming the middleware. + + + This task is a blocking checkpoint per the user-resolved Open Question 1 (body-size limits). Before this plan is considered complete: + 1. Have the operator read the production nginx vhost's `client_max_body_size` and php.ini's `post_max_size`/`upload_max_filesize` off the production host (docs/deploy/plytarium.com.md's operator-managed vhost, not in any repo). + 2. Replace the INTERIM `default_bytes`/`upload_bytes` values in fonoteka.go/config/http.yaml with the real numbers (converted to bytes). + 3. Update the two INTERIM comments to record the reported values and the date they were confirmed. + 4. Re-run the body-limit tests to confirm they still pass against the new numbers (they assert against config-loaded values, not hardcoded ones, so no test code changes should be needed -- only the config numbers and their comments). + + Type "approved" once the real production body-size numbers are recorded in fonoteka.go/config/http.yaml, or describe what changed if the numbers differ from the INTERIM defaults. + CORS is path-scoped and matches config/cors.php exactly; body limits are enforced per group with the real (operator-confirmed) production numbers, not a guess; the oauth group is structurally raw with zero handlers. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|--------------| +| raw group -> house middleware | a misconfigured raw group must fail boot, not silently mount an envelope/error middleware onto an RFC-shaped surface | +| any request body -> handler | an unbounded request body is a resource-exhaustion vector regardless of auth group | +| CORS config -> browser | an overly permissive path-glob match would leak the JWT group's same-origin-only posture to cross-origin callers | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-06-10 | Elevation of Privilege | Route table mutual-exclusivity | mitigate | routetable_test.go asserts, over the FULL assembled route table, that no /api/v1/fonoteka route carries jwt.auth and no /_fonoteka/api/v1 route carries inv_token/inv.scope: -- this completes the partial coverage 06-01 accepted (T-06-02) | +| T-06-11 | Tampering | Raw-group house-middleware refusal | mitigate | Registration-time (not request-time) failure; a future PR cannot silently attach an envelope middleware to the OAuth group and have it ship | +| T-06-12 | Denial of Service | Unbounded request body on a non-raw route | mitigate | http.MaxBytesReader applied unconditionally per non-raw route via the default body limit; raw (RFC) routes rely on their own future handler-level limits, matching PHP's own scoping | +| T-06-13 | Information Disclosure | Body-size limits shipped as a guess | mitigate | INTERIM defaults are clearly commented as unverified; the plan cannot close without the blocking checkpoint recording operator-confirmed production numbers -- never presented as verified when it is not | +| T-06-SC | Tampering | npm/go install in scripts/check-openapi.sh (swag, openapi-typescript) | accept | Both packages are STACK.md-named and pass 06-RESEARCH.md's Package Legitimacy Audit (Approved disposition, no [ASSUMED]/[SUS] verdicts) -- no additional human-verify checkpoint required beyond that prior audit | + + + +cd summercms.go && go vet ./... && go test ./surf/... ./wire/... ./internal/build/... -short +cd ../fonoteka.go && go vet ./... && go test ./plugins/golem15/fonoteka/... -short && bash scripts/check-openapi.sh + + + +- Raw groups refuse house-tagged middleware at registration time and recover with a bare 500. +- The route table and route:list command exist and prove group mutual-exclusivity by inspection. +- wire package ships the promoted response-convention helpers, unit-tested independently. +- CORS is path-scoped and matches config/cors.php exactly (JWT group: no headers; token group: wildcard). +- Body limits are enforced per group with operator-confirmed production numbers, not a guess. +- The oauth group is declared raw with zero handlers. +- A real OpenAPI document is generated from swag annotations and validated by openapi-typescript. +- go vet ./... and go test ./... are green in both repos. + + + +Create `.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-03-SUMMARY.md` when done + diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-PLAN.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-PLAN.md new file mode 100644 index 0000000..1d34663 --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-PLAN.md @@ -0,0 +1,244 @@ +--- +phase: 06-http-routing-auth-groups-and-rate-limiting +plan: 04 +type: execute +wave: 1 +depends_on: [] +files_modified: + - summercms.go/fetchguard/policy.go + - summercms.go/fetchguard/ip.go + - summercms.go/fetchguard/fetch.go + - summercms.go/fetchguard/fetch_test.go + - summercms.go/fetchguard/ip_test.go +autonomous: true +requirements: [HTTP-07] + +must_haves: + truths: + - "A URL whose host is not on Policy.AllowHosts (exact or dotted-suffix match) is rejected before any network I/O (D-11)" + - "PublicOnly mode accepts any hostname but rejects the fetch if ANY resolved IP is private/loopback/reserved, including under an allow-list (D-11)" + - "The private/loopback/reserved IP check runs at dial time via net.Dialer.Control on the actual address being connected, not only against the pre-resolved hostname, closing the DNS-rebinding TOCTOU gap PHP's own code admits it has (D-12)" + - "Only https is accepted; a redirect response is never followed (D-11)" + - "A response body is capped while streaming (io.LimitReader), not after being fully buffered -- a slow-drip origin cannot exceed the cap even within the timeout (D-11, Pitfall 8)" + - "Byte cap and timeout default to config http.fetch.max_bytes/http.fetch.timeout_seconds (10 MiB / 10s when unset) and are overridable per call, but an explicitly-set zero or negative effective value is an error, never treated as unlimited (D-14)" + - "Failure reasons are a closed, typed set matching PHP's invalid_url/scheme/unresolvable/private_ip/network_error/too_large family (D-13)" + - "The private IPv4/IPv6 CIDR table matches ManualCoverUrlFetcher.php's PRIVATE_V4_CIDRS/PRIVATE_V6_PREFIXES constants exactly, including 100.64.0.0/10 (CGNAT) and 169.254.0.0/16 (cloud metadata)" + - "No ManualCoverUrlFetcher or CoverImporter call site exists in this plan -- this plan ships the helper and its tests only (D-13)" + artifacts: + - path: summercms.go/fetchguard/fetch.go + provides: "Fetch(ctx, url string, policy Policy) (*Result, error) -- the SSRF-guarded outbound fetch entry point" + - path: summercms.go/fetchguard/policy.go + provides: "Policy{Mode, AllowHosts, PublicOnly, MaxBytes, Timeout}, Reason type, Defaults/DefaultsFromConfig" + - path: summercms.go/fetchguard/ip.go + provides: "isReservedOrPrivate(netip.Addr) bool -- the private/loopback/reserved/CGNAT CIDR table" + key_links: + - from: summercms.go/fetchguard/fetch.go + to: summercms.go/fetchguard/ip.go + via: "the http.Transport's DialContext Control hook calls isReservedOrPrivate on the address actually being dialed" + pattern: "Control:.*dialControl|isReservedOrPrivate" +--- + + +Ship a framework-owned, SSRF-guarded outbound fetch helper offering both PHP fetch modes (`AllowHosts` exact/dotted-suffix allow-listing and `PublicOnly` any-host-but-only-public-IPs) with the private/loopback/reserved IP check enforced at dial time -- closing the DNS-rebinding gap PHP's own `ManualCoverUrlFetcher` comment admits it has -- plus https-only, no redirects, a streaming byte cap, and a timeout, all with typed failure reasons. + +Purpose: HTTP-07 requires a single reusable primitive two future phases' call sites (manual cover URL fetch, Discogs cover import) will use for user-supplied and third-party URLs; getting the SSRF guard right here, once, with tests against real `httptest` servers, is cheaper and safer than each call site re-implementing it. +Output: `fetchguard.Fetch`, `fetchguard.Policy`, typed `Reason` values, and a private/reserved IP classification table -- helper and tests only, no call sites this phase (D-13). + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.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-PATTERNS.md + + + +New file summercms.go/fetchguard/policy.go: + +package fetchguard + +import "time" + +type Mode int + +const ( + AllowHostsMode Mode = iota + PublicOnlyMode +) + +type Reason string + +const ( + ReasonInvalidURL Reason = "invalid_url" + ReasonScheme Reason = "scheme" + ReasonUnresolvable Reason = "unresolvable" + ReasonPrivateIP Reason = "private_ip" + ReasonNetworkError Reason = "network_error" + ReasonTooLarge Reason = "too_large" +) + +// Policy is supplied per call. Mode selects AllowHosts vs PublicOnly; the +// private/loopback/reserved IP block is ALWAYS on regardless of Mode (D-11: +// "even under an allow-list"). +type Policy struct { + Mode Mode + AllowHosts []string // exact or dotted-suffix match, only used in AllowHostsMode + MaxBytes int64 // 0 means "use the config/framework default", never unlimited + Timeout time.Duration +} + +// Error carries the typed Reason plus the underlying error for logging. +type Error struct { + Reason Reason + Err error +} +func (e *Error) Error() string +func (e *Error) Unwrap() error + +// Defaults are the framework fallback: 10 MiB, 10s, matching PHP. +func Defaults() (maxBytes int64, timeout time.Duration) + +// DefaultsFromConfig reads http.fetch.max_bytes/http.fetch.timeout_seconds +// from cfg, falling back to Defaults() for absent/zero keys. An explicitly +// configured zero or negative value is an error (D-14), returned instead of +// silently falling back -- config typos must not silently become "unlimited". +func DefaultsFromConfig(cfg *compass.Config) (maxBytes int64, timeout time.Duration, err error) + +New file summercms.go/fetchguard/fetch.go: + +package fetchguard + +type Result struct { + Body []byte + ContentType string + StatusCode int +} + +// Fetch validates url against policy, resolves defaults for any zero +// MaxBytes/Timeout via DefaultsFromConfig-equivalent (cfg may be nil, in +// which case Defaults() applies), then performs the guarded HTTPS fetch. +// A non-nil error is always *Error with a Reason from the closed set. +func Fetch(ctx context.Context, url string, policy Policy, cfg *compass.Config) (*Result, error) + +New file summercms.go/fetchguard/ip.go: + +package fetchguard + +import "net/netip" + +// isReservedOrPrivate classifies addr (already Unmap()-ed by the caller) +// against the exact table ManualCoverUrlFetcher.php hand-rolls: +// v4: 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16, +// 100.64.0.0/10, 0.0.0.0/8 +// v6: ::1, fe80::/10, fc00::/7 +// plus addr.IsMulticast()/IsUnspecified() as additional stdlib-covered cases. +func isReservedOrPrivate(addr netip.Addr) bool + + + + + + Task 1: Private/reserved IP classification table + summercms.go/fetchguard/ip.go, summercms.go/fetchguard/ip_test.go + + - 127.0.0.1, 10.1.2.3, 172.16.0.1, 172.31.255.255, 192.168.1.1, 169.254.169.254 (cloud metadata), 100.64.0.1 (CGNAT), 0.0.0.1 -> true. + - 8.8.8.8, 1.1.1.1, 93.184.216.34 (a real public IP) -> false. + - ::1 -> true; fe80::1 -> true; fc00::1 -> true; a public v6 address (e.g. 2606:4700:4700::1111) -> false. + - An IPv4-mapped IPv6 literal for a private address (::ffff:169.254.169.254) classifies as true AFTER Unmap() is applied by the caller -- ip_test.go asserts isReservedOrPrivate(addr.Unmap()) for this case, documenting that Unmap() is the CALLER's responsibility (fetch.go's dial hook), not this function's. + - A multicast address (224.0.0.1) and the unspecified address (0.0.0.0) both -> true. + - 172.15.255.255 and 172.32.0.0 (just outside the RFC1918 172.16.0.0/12 range) -> false (boundary test). + + + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/classes/ManualCoverUrlFetcher.php lines 41-55 (PRIVATE_V4_CIDRS, PRIVATE_V6_PREFIXES -- the exact source table, read directly, not from memory) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md Pitfall 6 (why net.IP.IsPrivate() alone is insufficient; the Unmap()/Is4In6 normalization requirement) + + + Create ip.go: a package-level `var privateV4 = []netip.Prefix{ netip.MustParsePrefix("127.0.0.0/8"), netip.MustParsePrefix("10.0.0.0/8"), netip.MustParsePrefix("172.16.0.0/12"), netip.MustParsePrefix("192.168.0.0/16"), netip.MustParsePrefix("169.254.0.0/16"), netip.MustParsePrefix("100.64.0.0/10"), netip.MustParsePrefix("0.0.0.0/8") }` and `var privateV6 = []netip.Prefix{ netip.MustParsePrefix("::1/128"), netip.MustParsePrefix("fe80::/10"), netip.MustParsePrefix("fc00::/7") }` (note: PHP's PRIVATE_V6_PREFIXES lists bare "::1" as a prefix-less loopback literal -- express it as the exact /128 host prefix so netip.Prefix.Contains works uniformly with the other CIDR entries). isReservedOrPrivate(addr netip.Addr) bool: if !addr.IsValid() return true (fail closed on garbage input); if addr.IsMulticast() || addr.IsUnspecified() return true; select privateV4 or privateV6 based on addr.Is4() (the caller is documented to pass an already-Unmap()-ed address, so a v4-mapped-v6 literal has already become a plain v4 Addr by this point); iterate the selected table with Prefix.Contains(addr), return true on any match, false otherwise. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go test ./fetchguard/... -run TestIsReservedOrPrivate -v + + + - Every case in the behavior list above is a distinct table-driven subtest and passes. + - go vet ./fetchguard/... is clean. + + The private/reserved/CGNAT/metadata IP table is a literal, tested port of ManualCoverUrlFetcher.php's constants -- no range invented, none omitted. + + + + Task 2: Policy, defaults, and the dial-time-guarded, streaming-capped Fetch entry point + summercms.go/fetchguard/policy.go, summercms.go/fetchguard/fetch.go, summercms.go/fetchguard/fetch_test.go + + - A malformed URL (missing scheme/host) -> Error{Reason: ReasonInvalidURL}. + - An http:// (non-https) URL -> Error{Reason: ReasonScheme}, no network I/O attempted. + - In AllowHostsMode, a host not present in Policy.AllowHosts (exact match) and not a dotted-suffix of any entry -> Error{Reason: ReasonInvalidURL} before DNS resolution (host-allow-list is a pre-dial check, separate from the IP check). + - A dotted-suffix bypass attempt (host "evil-discogs.com" against an AllowHosts entry "discogs.com") is rejected -- "evil-discogs.com" must NOT match as a suffix of "discogs.com" (only "*.discogs.com" or the exact host matches). + - A hostname that resolves only to a private/loopback address (via an httptest server bound to 127.0.0.1, or a fake net.Resolver/dial override in the test) -> Error{Reason: ReasonPrivateIP}, in BOTH AllowHostsMode (host allow-listed) and PublicOnlyMode -- proving the IP block is unconditional. + - A server that returns a 3xx redirect -> Fetch does not follow it (CheckRedirect returns an error or http.ErrUseLastResponse; test asserts the Result reflects the 3xx response itself, or Error{Reason: ReasonNetworkError}, whichever the implementation picks -- pin ONE behavior and document it in the doc comment on Fetch). + - A server that streams more than policy.MaxBytes (test server writes in a loop past the cap within the timeout) -> Error{Reason: ReasonTooLarge}, and the test asserts via a byte-counting io.Reader on the SERVER side that the client did not request/read more than MaxBytes+1 bytes (proving streaming enforcement, not post-hoc buffering). + - policy.MaxBytes == 0 and policy.Timeout == 0 with a nil cfg -> Defaults() values (10 MiB, 10s) are used. + - DefaultsFromConfig with an explicit http.fetch.max_bytes: 0 (or negative) set in cfg -> returns a non-nil error, never silently falls back to Defaults() (D-14: "zero or negative effective values are an error"). + + + summercms.go/fetchguard/ip.go (Task 1 output) + summercms.go/compass/config.go (Int/Lookup -- for DefaultsFromConfig) + /media/nvme/dev/golem15/fonoteka/plugins/golem15/fonoteka/classes/ManualCoverUrlFetcher.php full file (validation order comment, lines 1-90+ already read this session -- the exact 10-step sequence to mirror minus the MIME-sniffing/attach steps, which are out of scope per D-13) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-RESEARCH.md Code Examples "Dial-time SSRF guard shape" (lines 416-442) and Pitfalls 6-8 + + + Create policy.go with Mode, Reason constants, Policy, Error (Error()/Unwrap() per the interfaces block), Defaults() returning (10*1024*1024, 10*time.Second), and DefaultsFromConfig(cfg) reading "http.fetch.max_bytes"/"http.fetch.timeout_seconds" via cfg.Lookup (not cfg.Int, since Int silently returns 0 for both "absent" and "explicitly zero" -- use Lookup to distinguish "key absent" (fall back to Defaults()) from "key present and <= 0" (return an error) from "key present and positive" (use it)); timeout_seconds converts to time.Duration via time.Duration(n)*time.Second. + + Create fetch.go: Fetch(ctx, rawURL string, policy Policy, cfg *compass.Config) (*Result, error). Step 1: url.Parse(rawURL); if err or Scheme=="" or Host=="" -> &Error{ReasonInvalidURL, err}. Step 2: if strings.ToLower(parsed.Scheme) != "https" -> &Error{ReasonScheme, nil}. Step 3 (AllowHostsMode only): strip IPv6 brackets from parsed.Hostname() if present; check exact match against policy.AllowHosts OR a dotted-suffix match (host has a literal "." immediately before the suffix, e.g. strings.HasSuffix(host, "."+allowed) OR host == allowed -- this is what prevents "evil-discogs.com" matching "discogs.com": there is no "." immediately before "discogs.com" in "evil-discogs.com" since the character before it is "-", not "."); no match -> &Error{ReasonInvalidURL, nil}. Step 4: resolve maxBytes/timeout: if policy.MaxBytes > 0 use it, else if cfg != nil use DefaultsFromConfig(cfg) (propagate its error), else use Defaults(); same pattern for Timeout. Step 5: build an http.Client with Timeout: timeout, CheckRedirect: func(...) error { return http.ErrUseLastResponse } (pin this: redirects are never followed, the 3xx response itself is returned to the caller as a non-error Result so the caller can decide -- document this choice in Fetch's doc comment), and Transport: &http.Transport{DialContext: (&net.Dialer{Timeout: timeout, Control: dialControl(policy)}).DialContext}. dialControl(policy) is the unexported func(network, address string, c syscall.RawConn) error from the RESEARCH.md code example: net.SplitHostPort the address, netip.ParseAddr the host, addr.Unmap(), isReservedOrPrivate(addr) -> return an error wrapping ReasonPrivateIP (the DialContext error surfaces through the http.Client call as a network error -- Step 6 below maps it back to ReasonPrivateIP by checking errors.As/a sentinel, not by re-deriving it from scratch). Step 6: perform the GET request with ctx; on any transport-level error, check whether it wraps the private-IP sentinel from Step 5's dial hook (map to ReasonPrivateIP) or a DNS resolution failure specifically (map to ReasonUnresolvable), else map to ReasonNetworkError. Step 7: on a successful round trip, wrap resp.Body in io.LimitReader(resp.Body, maxBytes+1); io.ReadAll it; if len(data) == maxBytes+1, return &Error{ReasonTooLarge, nil} (the limit was hit, meaning the true body is at least one byte larger than allowed); otherwise return &Result{Body: data, ContentType: resp.Header.Get("Content-Type"), StatusCode: resp.StatusCode}, nil. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test ./fetchguard/... -short -race + + + - Every behavior-list case above is a distinct subtest against a real httptest.Server (or a deliberately-private-bound listener for the private_ip cases) -- no mocked http.RoundTripper standing in for the dial-time check itself, since the dial-time enforcement is the exact thing under test. + - The too_large test proves streaming enforcement: assert (via a counter on the test server's handler) that the server did not need to write more than maxBytes+1 bytes before the client aborted the read, i.e. the client did not buffer an unbounded body first. + - The dotted-suffix bypass test explicitly includes "evil-discogs.com" against an AllowHosts entry of "discogs.com" and asserts rejection. + - DefaultsFromConfig's explicit-zero-is-an-error behavior has a dedicated test using a real *compass.Config loaded from a temp YAML file (not a hand-built struct), so the config-parsing path is exercised too. + + fetchguard.Fetch enforces https-only, dial-time private-IP blocking (closing the DNS-rebinding gap), no redirects, streaming byte caps, and typed failure reasons -- proven against real network I/O in every test, with zero production call sites added this phase. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|--------------| +| caller-supplied URL -> outbound fetch | the entire point of this package: a user- or third-party-supplied URL must never reach an internal service, cloud metadata endpoint, or loopback service | +| DNS resolution -> TCP connect | the classic SSRF TOCTOU gap (resolve-time check vs connect-time address) | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-06-14 | Elevation of Privilege | SSRF via a caller-supplied URL reaching an internal service or 169.254.169.254 | mitigate | Dial-time net.Dialer.Control check on every connection attempt (D-12), always-on regardless of Mode (D-11), tested against real httptest/private listeners | +| T-06-15 | Tampering | DNS rebinding (resolve-time check passes, connect-time IP differs) | mitigate | The check runs at actual-dial time on the real address being connected, not on a pre-resolved hostname -- this is a deliberate Go-side improvement over PHP's own documented gap (D-12), not parity with PHP's vulnerable pattern | +| T-06-16 | Denial of Service | Unbounded response body from a malicious/slow-drip origin that passes the SSRF gate | mitigate | io.LimitReader enforces the byte cap while streaming, combined with an http.Client.Timeout; a post-hoc len(body) check alone (as PHP's own comment warns against) is never used | +| T-06-17 | Elevation of Privilege | Redirect to a private/internal address after the initial host/IP checks pass | mitigate | CheckRedirect prevents the client from ever following a redirect automatically; the caller receives the 3xx response itself and must explicitly re-invoke Fetch (through the same guard) if it wants to follow it | +| T-06-18 | Spoofing | Dotted-suffix allow-list bypass (e.g. evil-discogs.com against discogs.com) | mitigate | Suffix match requires a literal "." immediately preceding the allowed suffix or an exact match; tested explicitly | + + + +cd summercms.go && go vet ./... && go test ./fetchguard/... -race + + + +- fetchguard.Fetch enforces https-only, host allow-listing (exact + dotted-suffix) or public-only IP mode, always-on private/reserved IP blocking at dial time, no redirects, streaming byte caps, and a timeout. +- Failure reasons are a closed, typed set matching PHP's family. +- Zero call sites exist yet (helper + tests only, per D-13). +- go vet ./... and go test ./... -race are green. + + + +Create `.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-SUMMARY.md` when done + diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-PLAN.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-PLAN.md new file mode 100644 index 0000000..89a319e --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-PLAN.md @@ -0,0 +1,164 @@ +--- +phase: 06-http-routing-auth-groups-and-rate-limiting +plan: 05 +type: execute +wave: 4 +depends_on: ["06-01", "06-02", "06-03", "06-04"] +files_modified: + - summercms.go/bouncer/registry_coverage_test.go + - summercms.go/surf/limiter_coverage_test.go + - summercms.go/surf/routetable_coverage_test.go + - summercms.go/surf/cors_coverage_test.go + - summercms.go/wire/response_coverage_test.go + - summercms.go/fetchguard/fetch_coverage_test.go + - fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard_coverage_test.go + - fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope_coverage_test.go + - fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go + - fonoteka.go/parity/parity_test.go + - .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md +autonomous: true +requirements: [HTTP-03, HTTP-04, HTTP-05, HTTP-06, HTTP-07, HTTP-08, HTTP-09] + +must_haves: + truths: + - "Every T-06-01 through T-06-18 and T-06-SC threat ID from plans 01-04 is mapped in 06-SECURITY-REVIEW.md to a named passing test or a restated accept/transfer rationale -- none left unmapped" + - "The full route-table mutual-exclusivity assertion (zero jwt.auth on /api/v1/fonoteka, zero inv_token/inv.scope on /_fonoteka/api/v1, across the WHOLE assembled route table, not just the genres pair) has a dedicated passing test" + - "go vet ./... and go test ./... -race are green in both repos, including every -short-gated and testcontainers-gated test" + - "The parity harness re-run confirms both genres routes (jwt and personal_token) are status: ported and passing, with no regression in the recorded-passing count from prior phases" + artifacts: + - path: .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md + provides: "Threat-to-test mapping for every T-06-xx ID across plans 01-04" + key_links: + - from: fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go + to: summercms.go/surf/routetable.go + via: "test iterates Router.Routes() asserting group mutual exclusivity" + pattern: "\\.Routes\\(\\)" +--- + + +Close Phase 6 with full unit coverage of everything Plans 06-01 through 06-04 built (earlier plans carried behavior/smoke tests but were not blocked on full coverage, per this project's lean-mode workflow rule), a dedicated full-route-table mutual-exclusivity test, and `06-SECURITY-REVIEW.md` mapping every threat ID named across the phase to a passing test or a restated, deliberate risk acceptance. + +Purpose: prove -- not just assert -- that the guard registry, limiter, raw-group enforcement, CORS/body-limit scoping, and the SSRF fetch helper hold under coverage broader than the happy-path tests each earlier plan shipped inline, and that the phase's security-load-bearing status (auth guard registry, rate limiting, SSRF fetch helper) is closed out with an explicit, reviewable trail. +Output: coverage-gap tests across both repos; a full route-table isolation test; `06-SECURITY-REVIEW.md`; a green `go vet`/`go test -race` in both repos; a clean parity harness re-run. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.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-01-SUMMARY.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-03-SUMMARY.md +@.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-04-SUMMARY.md + + + + + + Task 1 (summercms.go): Coverage gaps in bouncer, surf (limiter/routetable/cors), wire, and fetchguard + summercms.go/bouncer/registry_coverage_test.go, summercms.go/surf/limiter_coverage_test.go, summercms.go/surf/routetable_coverage_test.go, summercms.go/surf/cors_coverage_test.go, summercms.go/wire/response_coverage_test.go, summercms.go/fetchguard/fetch_coverage_test.go + + summercms.go/bouncer/registry.go, guard.go, context.go (Plan 06-01) + summercms.go/surf/limiter.go, limiter_store.go, clientip.go, cors.go, bodylimit.go, routetable.go (Plans 06-02, 06-03) + summercms.go/wire/response.go (Plan 06-03) + summercms.go/fetchguard/policy.go, fetch.go, ip.go (Plan 06-04) + .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-01-PLAN.md through 06-04-PLAN.md's blocks (every T-06-xx entry, for cross-reference against Task 3's security review) + + + Run `go test ./bouncer/... ./surf/... ./wire/... ./fetchguard/... -coverprofile` locally to find lines/branches not exercised by the tests each plan already shipped inline, then add targeted tests closing the real gaps found (do not pad with trivial assertions). Expected gap categories to check specifically, since they are easy to under-test inline while building the primary feature: (a) Registry.Register with a guard implementing NEITHER Guard nor CredentialGuard (the type-switch default-fail branch); (b) MemoryStore's sweep goroutine actually removing an expired entry (not just TooManyAttempts' lazy-expiry path) -- construct with a short sweep interval and assert the internal entry count drops after the sweep fires; (c) RegisterMiddlewareFactory/RegisterHouseMiddlewareFactory duplicate-name failure (only the non-house variant may have been tested inline); (d) pathScopedCORS with a path matching NONE of the configured globs (already covered for the two named fonoteka groups, but add a framework-level fixture-route test independent of fonoteka.go); (e) wire.Time.UnmarshalJSON round-tripping both a +00:00 and a Z-suffixed input; (f) fetchguard.Fetch's PublicOnlyMode explicitly (Task 2 of 06-04 tests AllowHostsMode's private-IP rejection; add the PublicOnlyMode private-IP and PublicOnlyMode-any-host-accepted-when-public cases if not already present); (g) surf.Router.Routes() called before any route is registered (empty-router case) and after a raw group with zero routes (proving Raw:true is still inspectable via the Group's own state, not just via a RouteInfo entry -- if Task 1 of 06-03 could only assert this through a routed fixture, add the missing empty-raw-group introspection path here, or explicitly document in this task's test file why it is structurally untestable and rely on the routed-fixture proof instead). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test ./... -race -short + + + - go test ./bouncer/... ./surf/... ./wire/... ./fetchguard/... -cover reports coverage on every new file from Plans 06-01/06-02/06-03/06-04 with no 0%-covered exported function remaining. + - Every gap category (a)-(g) above has either a passing test or an explicit one-line comment in the coverage test file explaining why it is not applicable/testable, so a reviewer never has to wonder whether a gap was missed vs. deliberately skipped. + + Framework-side Phase 6 code (bouncer, surf, wire, fetchguard) has coverage beyond each plan's inline happy-path tests, with every identified gap either closed or explicitly justified. + + + + Task 2 (fonoteka.go, parity): App-side coverage gaps, full route-table isolation test, parity re-run + fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard_coverage_test.go, fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope_coverage_test.go, fonoteka.go/plugins/golem15/fonoteka/routes_isolation_test.go, fonoteka.go/parity/parity_test.go + + fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go, token_guard_test.go (Plan 06-01) + fonoteka.go/plugins/golem15/fonoteka/middleware/token_scope.go, token_scope_test.go, public_share_headers.go (Plans 06-01, 06-02) + fonoteka.go/plugins/golem15/fonoteka/routes.go, plugin.go (post-06-03, full seven-group state) + fonoteka.go/parity/parity_test.go, manifest.yaml (current state) + + + Add coverage for: (a) TokenGuard.AuthenticateCredential with a token row that has a non-nil ExpiresAt in the FUTURE (usable) versus in the PAST (not usable) as two explicit cases, and a RevokedAt-set case, all against real Postgres (not just the "unknown hash" case Plan 06-01 likely covered first); (b) InvScope with a credential type-assertion failure (bouncer.Credential(ctx) set to something that is NOT *models.ApiToken -- proves the middleware fails closed rather than panicking); (c) PublicShareHeaders wrapping a handler that writes headers AFTER a partial body write (edge case for the buffer-then-flush design) if not already covered. + + Create routes_isolation_test.go: assemble the real app (`app.Handler`-equivalent or a direct `party.Activate` + `surf.BuildRouter` call against `app.PluginIDs`) and call `.Routes()`; assert, over the FULL route table (not just the genres pair): zero entries with `Pattern` prefixed `/api/v1/fonoteka` have `"jwt.auth"` in `Middleware`; zero entries with `Pattern` prefixed `/_fonoteka/api/v1` have `"inv_token"` or any entry with a `strings.HasPrefix(m, "inv.scope:")` in `Middleware`; the one entry for `/.well-known/oauth-authorization-server`-or-`/oauth/mcp/*`-prefixed pattern (if any routes exist there yet) has `Raw == true`. This is the completion of T-06-02/T-06-10's partial coverage from Plans 06-01/06-03. + + Re-run `go test ./parity/... -run TestParityCorpus` and confirm the summary line's `passing` count includes both genres routes and has not regressed from Plan 06-01's Task 3 result; if `check-phase2.sh --fresh-php` is available in this environment, run it once and note the result in the plan's SUMMARY (do not block completion on live-PHP availability, matching Phase 3's precedent that `--fresh-php` is a sign-off convenience, not a gate). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && go vet ./... && go test ./... -race -short && go test ./parity/... -run TestParityCorpus + + + - routes_isolation_test.go passes and its assertions run against the FULL route table returned by a real Router.Routes() call, not a hand-built fixture list. + - go test ./parity/... -run TestParityCorpus reports both genres routes as passing. + - go vet ./... and go test ./... -race are green. + + App-side guard/middleware edge cases are covered against real Postgres; group mutual-exclusivity is proven over the entire assembled route table, not a hand-picked pair; the parity harness confirms no regression. + + + + Task 3 (.planning): Security review mapping every T-06-xx threat to a passing test + .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-01-PLAN.md through 06-04-PLAN.md's blocks (every T-06-01 through T-06-18 and T-06-SC entry) + All coverage test files created in Task 1 and Task 2 of this plan + + + Write 06-SECURITY-REVIEW.md with one table row per threat ID from T-06-01 through T-06-18 plus T-06-SC (19 entries total across Plans 06-01-06-04's threat_model blocks): Threat ID, Category, Plan of origin, Disposition (copy verbatim from the originating plan), and either the exact test function name (file:TestName) that proves the mitigation holds, or a restated accept/transfer rationale copied verbatim from the originating plan's threat_model table (for the three `accept` dispositions: T-06-03, T-06-08, T-06-SC). Explicitly grep both repos for any place `bouncer.Credential` or `TokenGuard`'s resolved token is logged, and confirm no call site logs the raw bearer token or the token hash outside the guard's own DB update (grep -rn "raw\|bearer\|LastUsedIP" across the auth package and confirm no fmt.Print*/log.* call is adjacent). Explicitly grep for any second `RegisterMiddleware`(non-house) call registering `"inv.must-change-password"` to confirm Plan 06-03's switch to `RegisterHouseMiddleware` is the only registration site (no stale duplicate). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && grep -c "^| T-06-" .planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-SECURITY-REVIEW.md + + + - The security review table contains exactly 19 T-06-xx rows (T-06-01 through T-06-18 plus T-06-SC), none missing. + - Every `mitigate` disposition names a real, currently-passing test function (verified by cross-referencing the test file); every `accept` disposition restates the originating plan's rationale rather than inventing a new one. + - `grep -rn "RegisterMiddleware(p.ID(), \"inv.must-change-password\"" fonoteka.go/plugins/golem15/fonoteka/plugin.go` returns zero matches (confirms the house-tagged variant is the only registration). + + Every threat identified across Phase 6's four implementation plans is mapped to a passing test or a deliberate, restated risk acceptance -- no threat ID is silently dropped between planning and phase close. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|--------------| +| this plan's tests -> every earlier plan's security-load-bearing code | this is the phase's closing verification pass, not new production code -- its own risk surface is limited to test-code correctness and review completeness | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-06-19 | Repudiation | Security review completeness | mitigate | 06-SECURITY-REVIEW.md is required to map every T-06-xx ID from all 4 prior plans by name -- an unmapped ID is a review gap, not an accepted risk, and must be caught before phase close (Task 3's exact-count acceptance criterion) | +| T-06-20 | Tampering | Coverage gaps could hide a real Phase 6 defect behind an inline happy-path test | mitigate | Task 1/2 run coverage tooling against every new package and require either a closing test or an explicit documented reason per gap category, rather than trusting each plan's own inline tests were exhaustive | + + + +Full suite in both repos: `cd summercms.go && go vet ./... && go test ./... -race` and `cd ../fonoteka.go && go vet ./... && go test ./... -race` (testcontainers Postgres included). `06-SECURITY-REVIEW.md` committed and cross-checked against all four prior plans' `` blocks. Parity harness re-run confirms both genres routes pass with no regression. + + + +- Coverage gaps identified across bouncer/surf/wire/fetchguard and the fonoteka auth/middleware packages are closed or explicitly justified. +- A dedicated test proves group mutual-exclusivity over the FULL assembled route table. +- 06-SECURITY-REVIEW.md maps every T-06-01 through T-06-18 plus T-06-SC to a passing test or a restated acceptance rationale. +- go vet ./... and go test ./... -race are green in both repos. +- The parity harness confirms both genres routes pass with no regression from Plan 06-01. + + + +Create `.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-05-SUMMARY.md` when done + diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-PATTERNS.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-PATTERNS.md new file mode 100644 index 0000000..d65b557 --- /dev/null +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-PATTERNS.md @@ -0,0 +1,636 @@ +# Phase 6: HTTP routing, auth groups and rate limiting - Pattern Map + +**Mapped:** 2026-09-19 +**Files analyzed:** 14 (new/modified, across `summercms.go` and `fonoteka.go`) +**Analogs found:** 14 / 14 (all have at least a role-match; several are direct extensions of an existing file) + +## File Classification + +| New/Modified File | Repo | Role | Data Flow | Closest Analog | Match Quality | +|---|---|---|---|---|---| +| `surf/router.go` (extend: Post/Put/Patch/Delete, raw-group flag, route table) | summercms.go | router | request-response | `surf/router.go` (self, extend in place) | exact | +| `surf/routetable.go` (new) | summercms.go | utility (introspection) | request-response | `pact/capabilities.go` (`Router` interface shape) + `surf/router.go` (`route` struct) | role-match | +| `surf/limiter.go` (new: real Limiter) | summercms.go | middleware | event-driven (counter state) | `surf/router.go` `Limiter`/`noopLimiter` (lines 336-347) | exact (seam already defined) | +| `surf/limiter_store.go` (new: Store interface + mutex map) | summercms.go | service (in-process store) | CRUD (counter hit/reset) | `compass/config.go` (`sync.RWMutex`-guarded struct with a `view()`/`rebuild()` pattern) | role-match | +| `surf/clientip.go` (new: trusted-proxy client IP) | summercms.go | utility | transform | `surf/params.go` (`Constraint`/pure-function utility file, no state) | role-match | +| `surf/cors.go` (extend existing `cors()` in router.go into path-scoped, config-driven) | summercms.go | middleware | request-response | `surf/router.go` `cors()` (lines 303-322) | exact | +| `bouncer/registry.go` (new: named Guard registry) | summercms.go | service (registry) | CRUD (register/resolve) | `surf/router.go` `RegisterMiddleware` (lines 67-80) | exact (explicitly named as the pattern to mirror by RESEARCH.md) | +| `bouncer/guard.go` (new: `Guard`/`CredentialGuard` interfaces, `jwt` guard adapter) | summercms.go | service (auth) | request-response | `bouncer/jwt.go` `Middleware()` (lines 30-64) | exact | +| new fetch-guard package, e.g. `fetchguard/fetch.go` | summercms.go | service (outbound I/O) | streaming | none in-repo (new capability) — nearest shape is `surf/serve.go`'s `net`/`syscall` stdlib usage and `compass/env.go`'s pure-function validation style | no analog (see below) | +| new response-convention package, e.g. `wire/response.go` | summercms.go | utility (wire types) | transform | `fonoteka.go` `controllers/genre_controller.go` `writeJSON`/`writeOpaque500` (lines 136-153) | role-match (promote existing per-controller helper to a framework package) | +| `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go` (new: `inv_token` Guard) | fonoteka.go | service (auth) | CRUD (lookup ApiToken) | `bouncer/jwt.go` `Middleware()`/`Verify()` (lines 30-84) + `fonoteka.go` `models/api_token.go` | exact (same guard shape, new credential model) | +| `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_scope.go` (new: `inv.scope:` middleware) | fonoteka.go | middleware | request-response | `fonoteka.go` `middleware/must_change_password.go` (whole file) | exact | +| `../fonoteka.go/plugins/golem15/fonoteka/routes.go` (extend: all 7 group builders, buckets) | fonoteka.go | route | request-response | `../fonoteka.go/plugins/golem15/fonoteka/routes.go` (self, extend in place) + `surf/router_test.go` `routePlugin.Routes` (lines 25-34, group-nesting shape) | exact | +| `../fonoteka.go/plugins/golem15/fonoteka/plugin.go` (extend: register `inv_token` guard + 5 buckets at Boot) | fonoteka.go | config/bootstrap | event-driven (boot-time registration) | `../fonoteka.go/plugins/golem15/fonoteka/plugin.go` (self, extend in place) | exact | + +## Pattern Assignments + +### `surf/router.go` — extend with verbs, raw-group flag, route table (router, request-response) + +**Analog:** self (`surf/router.go`, full file already read) + +**Imports pattern** (lines 1-12): +```go +package surf + +import ( + "fmt" + "net/http" + "strings" + + "git.golem15.com/golem15/summercms/backpack" + "git.golem15.com/golem15/summercms/pact" + "git.golem15.com/golem15/summercms/party" + "git.golem15.com/golem15/summercms/towel" +) +``` + +**Verb registration pattern to replicate for Post/Put/Patch/Delete** (lines 103-108, 124-129, 187-204): +```go +func (r *Router) Get(path string, handler http.HandlerFunc, middleware ...string) { + if r == nil { + return + } + r.add(r.pluginID, r.prefix, r.middleware, path, handler, middleware) +} +// Group mirrors this with the same nil-guard shape (lines 124-129). + +func (r *Router) add(pluginID, prefix string, groupMW []string, path string, handler http.HandlerFunc, extra []string) { + full := joinPath(prefix, path) + key := "GET " + full // <-- generalize to use the real method for Post/Put/Patch/Delete + if prev, ok := r.seen[key]; ok { + r.compileErr = fmt.Errorf("surf: duplicate route %s registered by %s and %s", key, prev, pluginID) + return + } + r.seen[key] = pluginID + mw := append([]string{}, groupMW...) + mw = append(mw, extra...) + r.routes = append(r.routes, route{ + pluginID: pluginID, + method: "GET", // <-- generalize + path: full, + handler: handler, + middleware: mw, + }) +} +``` +`route.method` already exists on the struct (line 26) but is hardcoded to `"GET"` at both call sites (`add`, `compile`'s `mux.Handle("GET "+rt.path, h)` at line 216) — Pitfall 9 in RESEARCH.md names this exact spot. Add a `method string` parameter threaded through `add`/`Get`/`Post`/etc., and change `compile()`'s `mux.Handle(rt.method+" "+rt.path, h)`. + +**Group-nesting pattern (for raw-group flag)** (lines 50-56, 89-101, 110-122): +```go +type Group struct { + router *Router + pluginID string + prefix string + middleware []string + // raw bool <-- add here, inherited like middleware is (line 97-99) +} + +func (r *Router) Group(prefix string, middleware []string, fn func(pact.Router)) { + if r == nil || fn == nil { + return + } + g := &Group{ + router: r, + pluginID: r.pluginID, + prefix: joinPath(r.prefix, prefix), + middleware: append([]string{}, r.middleware...), + } + g.middleware = append(g.middleware, middleware...) + fn(g) +} +``` +A `Raw`-flagged group needs a **new capability method** on `pact.Router` (e.g. `GroupRaw(prefix string, middleware []string, fn func(Router))`) since `pact.Router` is a narrow interface (see `pact/capabilities.go` below) — extend the interface, not just the `surf` struct, or app code in `fonoteka.go` cannot call it. + +**Compile / wrap pattern (where raw-group must skip envelope/recover)** (lines 206-263): +```go +func (r *Router) compile() (http.Handler, error) { + if r.compileErr != nil { + return nil, r.compileErr + } + mux := http.NewServeMux() + for _, rt := range r.routes { + h, err := r.wrap(rt) + if err != nil { + return nil, err + } + mux.Handle("GET "+rt.path, h) // generalize method + } + return recoverJSON(cors(r.origins, mux)), nil // raw groups need their own bare-500 wrapping, not this outer one +} + +func (r *Router) wrap(rt route) (http.Handler, error) { + h := constrain(rt.handler, rt.constraints) + h = noOpLimit(h) + h = orgSlot(h) + for i := len(rt.middleware) - 1; i >= 0; i-- { + name := rt.middleware[i] + named, ok := r.named[name] + if !ok { + return nil, fmt.Errorf("surf: plugin %q references unknown middleware %q", rt.pluginID, name) + } + h = named.fn(h) + } + h = locale(h) + return h, nil +} +``` +D-16's "fail boot if a house-envelope/error middleware is named on a raw group" belongs in this loop over `rt.middleware` — check `rt.raw` and reject any middleware name tagged house-envelope/error before building `h`. + +**Error message convention** (repeated throughout): `fmt.Errorf("surf: %q ...: %w"/", got/registered by %s and %s", ...)` — always prefixed `"surf: "`, always names the plugin and the offending identifier. Reuse verbatim for every new failure path (duplicate guard name, unknown guard name, raw-group violation). + +--- + +### `surf/routetable.go` (new) — exported route table for tests + `route:list` (utility, introspection) + +**Analog:** `pact/capabilities.go`'s `Router` interface (lines 40-45) for the shape contract, and `surf/router.go`'s `route` struct (lines 24-31) for the fields to expose. + +**Existing `route` struct to project into a public `RouteInfo`:** +```go +type route struct { + pluginID string + method string + path string + handler http.Handler + middleware []string + constraints []Constraint +} +``` +RESEARCH.md's own suggested shape (Pattern 3, lines 311-320) is already aligned with this struct — add `Raw bool` to `route` and project a read-only `[]RouteInfo` snapshot via a new `(r *Router) Routes() []RouteInfo` method, mirroring the getter-returns-copy style already used by `party.snapshot()` (`party/registry.go`, copies a slice under a mutex before returning it — same defensive-copy idiom to use here, though `Router` itself has no concurrent-write path post-`Assemble`). + +--- + +### `surf/limiter.go` + `surf/limiter_store.go` (new) — real Limiter behind the existing interface (service, event-driven counter) + +**Analog 1 — the seam to fill in** (`surf/router.go` lines 336-347): +```go +// Limiter wraps handlers. Phase 6 replaces the no-op with named buckets. +type Limiter interface { + Wrap(http.Handler) http.Handler +} + +type noopLimiter struct{} + +func (noopLimiter) Wrap(next http.Handler) http.Handler { return next } + +func noOpLimit(next http.Handler) http.Handler { + return noopLimiter{}.Wrap(next) +} +``` +The real limiter is a second implementation of this same `Limiter` interface; `noOpLimit`'s call site in `wrap()` (line 223, `h = noOpLimit(h)`) is where a named-bucket-aware limiter must be threaded in instead — the bucket name(s) come from `rt.middleware` entries like `throttle:fonoteka-public-ip`, so the limiter needs access to the per-route middleware list, not just a blanket wrap. + +**Analog 2 — mutex-guarded struct with rebuild/view split** (`compass/config.go` lines 39-48, 166-180): +```go +type Config struct { + mu sync.RWMutex + opts Options + // ... + k *koanf.Koanf + runtime *koanf.Koanf +} + +func (c *Config) view() *koanf.Koanf { + if c == nil { + return nil + } + c.mu.RLock() + defer c.mu.RUnlock() + if c.k == nil { + return nil + } + out := c.k.Copy() + if c.runtime != nil { + _ = out.Merge(c.runtime) + } + return out +} +``` +Mirror this `mu sync.RWMutex` + accessor-under-lock shape for the in-process `Store` (D-03): a `mu sync.Mutex`-guarded `map[string]*counterEntry`, with `Hit`/`TooManyAttempts`/`AvailableIn` each taking the lock internally (not exposing the map). Use `RWMutex` only if reads (`TooManyAttempts`) genuinely outnumber writes (`Hit`) — plain `Mutex` is simpler and matches the CONTEXT D-03 "mutex-guarded map" wording exactly. + +**Error/message conventions:** none apply directly (this is stdlib counters, no wire errors) — but keep the `"surf: "`-prefixed error convention for any constructor-time misconfiguration (e.g., unknown bucket name at `Group()`/route-registration time), matching `RegisterMiddleware`'s `fmt.Errorf("surf: middleware %q already registered by %s", ...)`. + +--- + +### `surf/clientip.go` (new) — trusted-proxy-aware client IP (utility, transform) + +**Analog:** `surf/params.go` (full file, pure functions operating on `*http.Request`, no state, package-level `Constraint`/`IntParam`/`Regex`/`Enum`). + +**Pattern to mirror** (lines 22-35 style — request-in, value-and-bool-out, no receiver state): +```go +// IntParam returns a positive integer path value. Missing or malformed ids +// are false so callers can 404 both unknown and non-integer values. +func IntParam(r *http.Request, name string) (int64, bool) { + if r == nil { + return 0, false + } + raw := r.PathValue(name) + if raw == "" { + return 0, false + } + n, err := strconv.ParseInt(raw, 10, 64) + if err != nil || n < 1 { + return 0, false + } + return n, true +} +``` +`ClientIP(r *http.Request, trusted []netip.Prefix) string` should follow this exact shape: nil-request guard first, then a linear decision chain, no panics, a safe zero-value return on any ambiguity (falls back to `RemoteAddr`, never to an unvalidated header). D-04 requires this to be "one framework function" — keep it a single exported function in this file, not a method on a stateful type, matching `params.go`'s style. + +--- + +### `surf/cors.go` — path-scoped, config-driven CORS (middleware, request-response) + +**Analog:** `surf/router.go`'s existing `cors()` (lines 303-322), which is the direct predecessor this task extends/replaces. + +```go +func cors(origins []string, next http.Handler) http.Handler { + allowed := make(map[string]struct{}, len(origins)) + for _, o := range origins { + allowed[o] = struct{}{} + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + origin := r.Header.Get("Origin") + if _, ok := allowed[origin]; ok && origin != "" { + w.Header().Set("Access-Control-Allow-Origin", origin) + w.Header().Set("Vary", "Origin") + w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type, Accept") + w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS") + } + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return + } + next.ServeHTTP(w, r) + }) +} +``` +Currently applied once, globally, in `compile()` (`cors(r.origins, mux)`, line 218) — D-18 requires this to become **path-scoped** (glob list like PHP's `paths: ['api/*', ...]`), so the wrapping must move from a single blanket call in `compile()` to a per-group or per-route decision, reading `compass` config via the existing `corsOrigins(app *backpack.App)` helper pattern (lines 265-288) which already reads `http.cors.allowed_origins` off `app.Config.Lookup(...)` — extend that helper to also read `http.cors.paths`, `.methods`, `.headers`, `.max_age`, `.credentials`, matching `compass.Config.Lookup`/`.String`/`.Bool` accessors shown in `compass/config.go` lines 112-155. + +**Test proof pattern** (`surf/router_test.go` lines 76-110, `TestCORSPreflightBypassesNamedAuth`) — reuse this exact shape (register a fake `jwt.auth` middleware, assert it did NOT run on an OPTIONS preflight, assert the `Access-Control-Allow-Origin` header) to prove path-scoping: run it once against a path in `http.cors.paths` (expect headers) and once against `/_fonoteka/api/*` (expect no headers), per Pitfall 10. + +--- + +### `bouncer/registry.go` (new) — named Guard registry (service, CRUD register/resolve) + +**Analog:** `surf/router.go` `RegisterMiddleware` (lines 67-80) — RESEARCH.md's own Pattern 1 (lines 245-272) already names this as the pattern to mirror; confirmed by reading the source directly. + +```go +// RegisterMiddleware stores a named wrapper. Duplicate names fail. +func (r *Router) RegisterMiddleware(pluginID, name string, fn pact.Middleware) error { + if r == nil { + return fmt.Errorf("surf: router is nil") + } + if name == "" || fn == nil { + return fmt.Errorf("surf: plugin %q registered empty middleware", pluginID) + } + if existing, ok := r.named[name]; ok { + return fmt.Errorf("surf: middleware %q already registered by %s", name, existing.pluginID) + } + r.named[name] = namedMiddleware{pluginID: pluginID, fn: fn} + return nil +} +``` +Port 1:1 into `bouncer.Registry.Register(pluginID, name string, g Guard) error` with `"bouncer: "` prefix instead of `"surf: "` (matching this package's own existing error-message convention seen in `bouncer/jwt.go`, e.g. `"bouncer: jwt secret is empty"` at line 69). Container type mirrors `namedMiddleware{pluginID string; fn pact.Middleware}` (line 19-22 of `surf/router.go`) → `namedGuard{pluginID string; guard Guard}`. + +**Duplicate-registration test pattern to replicate:** `surf/router_test.go`'s `TestMissingMiddlewareNamesPluginAndName` (lines 36-47) asserts the error string contains both the plugin id and the middleware name — write the equivalent `TestDuplicateGuardNamesBothPlugins`/`TestUnknownGuardNameFailsBoot` in `bouncer/registry_test.go` with the same assertion shape (`strings.Contains(err.Error(), ...)`). + +--- + +### `bouncer/guard.go` (new) — `Guard`/`CredentialGuard` interfaces, re-express `jwt` as a guard (service, request-response) + +**Analog:** `bouncer/jwt.go` `Middleware()` (lines 30-64) — this becomes the guard body wrapped by the registry; D-10 requires zero behavior change. + +```go +func Middleware(secret string, users UserProvider) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, err := bearerToken(r) + if err != nil { + write401(w, err.Error()) + return + } + sub, err := Verify(raw, secret) + if err != nil { + write401(w, err.Error()) + return + } + id, err := strconv.ParseUint(sub, 10, 64) + if err != nil || id == 0 { + write401(w, msgUserNotFound) + return + } + if users == nil { + write401(w, msgUserNotFound) + return + } + user, err := users.FindByID(r.Context(), uint(id)) + if err != nil { + write401(w, "Authentication error") + return + } + if user == nil { + write401(w, msgUserNotFound) + return + } + next.ServeHTTP(w, r.WithContext(WithUser(r.Context(), user))) + }) + } +} +``` +Wrap this exact function body (unchanged) as the `Authenticate(r *http.Request) (*Principal, error)` implementation of a `jwtGuard` struct, so `bouncer.Middleware(secret, users)` keeps working unmodified (D-10: "kept unchanged... re-expressed... without changing behavior") while `Registry.Middleware("jwt")` derives the same `func(http.Handler) http.Handler` generically from `Guard.Authenticate` + a shared 401-writer. Reuse `write401` (line 135-139) verbatim as the shared unauthorized-response writer for any guard that wants the `{"error":true,"message":...}` shape (only `jwt` uses this shape — `inv_token` uses a different shape, see below). + +**Context accessor pattern to extend** (`bouncer/context.go`, full file): +```go +type userKey struct{} + +func WithUser(ctx context.Context, user *Principal) context.Context { + if ctx == nil { + ctx = context.Background() + } + return context.WithValue(ctx, userKey{}, user) +} + +func User(ctx context.Context) (*Principal, bool) { + if ctx == nil { + return nil, false + } + u, ok := ctx.Value(userKey{}).(*Principal) + return u, ok && u != nil +} +``` +Add a second unexported key (`credentialKey{}`) with the identical `WithCredential`/`Credential` accessor pair for D-06's "optional credential accessor" — same nil-guard-first, `context.WithValue`, type-assert-with-ok-bool shape. Do not add fields to `Principal`; keep the credential as a separate `any` accessor so `bouncer` never imports `models.ApiToken` (two-repo boundary, CLAUDE.md "framework never imports the app"). + +**Context round-trip test pattern** (`bouncer/jwt_test.go` lines 222-231, `TestContextUserRoundTrip`) — replicate verbatim for `TestContextCredentialRoundTrip`. + +--- + +### `fetchguard/fetch.go` (new package) — SSRF-guarded outbound fetch (service, streaming) + +**No close in-repo analog** — this is a new capability (no existing outbound-HTTP-client code in either repo). Nearest structural precedents: +- `surf/serve.go` (full file) for stdlib `net`/`syscall`/`context`-heavy code style in this codebase — note its signal-aware shutdown pattern is not relevant, but its plain, no-third-party-dependency stdlib composition (`http.Server`, `net.Listener`, `context.WithTimeout`) is the house style to match: no wrapper abstractions, direct stdlib types. +- `bouncer/jwt.go`'s error-mapping style (`mapJWTError`, lines 115-133) — a `switch`/`errors.Is` chain converting a library error into one of a small closed set of named sentinel-ish errors — mirror this shape for mapping dial/read failures into the closed set `invalid_url`/`unresolvable`/`private_ip`/`network_error`/`too_large` (D-13). + +Use RESEARCH.md's own Code Examples section verbatim as the implementation skeleton (already vetted against the PHP source and Go stdlib capabilities this session): +```go +func dialControl(policy Policy) func(network, address string, c syscall.RawConn) error { + return func(network, address string, c syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return err + } + addr, err := netip.ParseAddr(host) + if err != nil { + return fmt.Errorf("fetchguard: unparseable dial address %q", host) + } + addr = addr.Unmap() // normalize ::ffff:a.b.c.d to a.b.c.d before classifying + if isReservedOrPrivate(addr) { + return fmt.Errorf("fetchguard: private_ip") + } + if policy.Mode == AllowHostsMode && !policy.hostAllowed(/* original hostname */) { + return fmt.Errorf("fetchguard: host not allow-listed") + } + return nil + } +} +``` +Package-naming convention: lower-case, no underscore, matches existing package names (`bouncer`, `surf`, `pact`, `compass`, `bonfire`, `lagoon`, `towel`, `backpack`, `party`) — `fetchguard` fits; `wire`/`parchment` (for the response package) also fit this one-word convention. + +--- + +### `wire/response.go` (new package) — response-convention JSON writer + types (utility, transform) + +**Analog:** `fonoteka.go` `controllers/genre_controller.go` `writeJSON`/`writeOpaque500` (lines 136-153) — this is the exact pattern to promote from a per-controller private helper into a shared framework package. + +```go +func writeJSON(w http.ResponseWriter, status int, v any) { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + if err := enc.Encode(v); err != nil { + writeOpaque500(w) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write(bytes.TrimSuffix(buf.Bytes(), []byte("\n"))) +} + +func writeOpaque500(w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":true,"message":"Internal server error"}`)) +} +``` +Note the buffer-then-trim-trailing-newline trick (`bytes.TrimSuffix(buf.Bytes(), []byte("\n"))`) and `SetEscapeHTML(false)` — both are deliberate wire-fidelity choices already established in this codebase; carry them into the framework `wire.WriteJSON`. The never-nil-slice convention is already independently established in `classes/serialize.go` (`SerializeAlbum`, lines 15-22): +```go +tracklist := a.Tracklist.Get() +if tracklist == nil { + tracklist = []models.TrackEntry{} +} +failures := a.CoverImportFailures.Get() +if failures == nil { + failures = []string{} +} +``` +This nil-to-empty-slice guard is the pattern D-17's "never-nil slice helper" should generalize (e.g. `wire.Slice[T](s []T) []T { if s == nil { return []T{} }; return s }`), reused at every call site currently doing this inline. + +--- + +### `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_guard.go` (new) — `inv_token` Guard (service, CRUD lookup) + +**Analog:** `bouncer/jwt.go` `Middleware()` + `Verify()` (lines 30-84) for the guard shape; `../fonoteka.go/plugins/golem15/fonoteka/models/api_token.go` (full file, already read) for the model this guard reads: +```go +type ApiToken struct { + ID uint `gorm:"column:id;primaryKey"` + UserID uint `gorm:"column:user_id"` + Name *string `gorm:"column:name"` + TokenHash string `gorm:"column:token_hash" json:"-"` + Scopes lagoon.Jsonable[[]string] `gorm:"column:scopes"` + CollectionIDs lagoon.Jsonable[[]uint] `gorm:"column:collection_ids"` + ExpiresAt *time.Time `gorm:"column:expires_at"` + RevokedAt *time.Time `gorm:"column:revoked_at"` + LastUsedAt *time.Time `gorm:"column:last_used_at"` + LastUsedIP *string `gorm:"column:last_used_ip"` + OAuthClientID *string `gorm:"column:oauth_client_id"` + CreatedAt time.Time `gorm:"column:created_at"` + UpdatedAt time.Time `gorm:"column:updated_at"` +} +func (ApiToken) TableName() string { return "golem15_fonoteka_api_tokens" } +func (ApiToken) Hidden() []string { return []string{"token_hash"} } +``` +Model already has `ExpiresAt`/`RevokedAt`/`LastUsedAt`/`LastUsedIP` fields ready for D-07's expiry/revocation/last-used rules — no model change needed this phase, only a guard that reads them. Guard's `Authenticate` should look up by SHA-256 hash of the presented token against `TokenHash` (an indexed equality lookup, not a timing-sensitive compare per RESEARCH.md's V6 Cryptography note — `crypto/subtle` is not needed on this specific lookup path), then apply the same nil-guard/error-mapping shape `bouncer/jwt.go`'s `Middleware` uses (`bearerToken` → verify → `FindByID`-equivalent chain). + +**Registration site pattern:** `plugin.go`'s `Middlewares()` (lines 53-57) shows the existing shape for registering a named capability with the framework at Boot; the guard registers the same way but via `bouncer.RegisterGuard`/`Registry.Register` instead of `surf.RegisterMiddleware` — see `plugin.go` pattern assignment below. + +--- + +### `../fonoteka.go/plugins/golem15/fonoteka/classes/auth/token_scope.go` (new) — `inv.scope:` middleware (middleware, request-response) + +**Analog:** `middleware/must_change_password.go` (full file, already read) — same plugin, same directory family, nearly identical shape (auth-context read → conditional 4xx JSON write → `next.ServeHTTP`): +```go +package middleware + +import ( + "encoding/json" + "net/http" + + "git.golem15.com/golem15/summercms/bouncer" +) + +// MustChangePassword rejects authenticated users who must rotate their password. +func MustChangePassword(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + user, ok := bouncer.User(r.Context()) + if ok && user.MustChangePassword { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusLocked) + _ = json.NewEncoder(w).Encode(map[string]any{ + "error": "Password change required", + "must_change_password": true, + }) + return + } + next.ServeHTTP(w, r) + }) +} +``` +`InvScope(scope string)` differs only in: (a) it's a parameterized-middleware factory (`func(scope string) pact.Middleware`, not a bare `pact.Middleware`) — matches `throttle:N,M`'s parameterized-name convention (D-05); (b) two failure branches (401 no-credential, 403 wrong-scope) instead of one; (c) body key is `error` as a **string** (`{"error":"Invalid token"}` / `{"error":"Missing required scope: "}`), NOT the `{"error":true,"message":...}` shape `write401`/`MustChangePassword` use — RESEARCH.md's Pattern 2 (lines 274-301) flags this divergence explicitly and its example is ready to use as-is: +```go +func InvScope(scope string) pact.Middleware { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + principal, ok := bouncer.User(r.Context()) + if !ok || principal == nil { + writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "Invalid token"}) + return + } + tok, _ := bouncer.Credential(r.Context()) + token, ok := tok.(*models.ApiToken) + if !ok || !token.HasScope(scope) { + writeJSON(w, http.StatusForbidden, map[string]string{"error": "Missing required scope: " + scope}) + return + } + next.ServeHTTP(w, r) + }) + } +} +``` +`writeJSON` here should be the same helper as `controllers/genre_controller.go`'s (or the promoted `wire.WriteJSON`, once that package exists) — do not hand-roll a third JSON writer in this file. + +--- + +### `../fonoteka.go/plugins/golem15/fonoteka/routes.go` — extend to all 7 groups (route, request-response) + +**Analog:** self (full file already read, 14 lines) — the file to extend in place, plus `surf/router_test.go`'s `routePlugin.Routes` (lines 25-34) for the nested-`Group` call shape once non-GET verbs and raw groups exist: + +```go +func (p *Plugin) Routes(r pact.Router) error { + r.Group("/_fonoteka/api/v1", surf.Use("jwt.auth", "inv.must-change-password"), func(g pact.Router) { + g.Get("/genres", controllers.ListGenres(p.app)) + }) + return nil +} +``` +Extend with a second `r.Group("/api/v1/fonoteka", surf.Use("inv_token", "inv.scope:read"), func(g pact.Router) { g.Get("/genres", controllers.ListGenres(p.app)) })` — literally the same handler function value (`controllers.ListGenres(p.app)`), proving D-15's shared-handler requirement by construction, not by convention. Keep `surf.Use(...)` string-list calling convention exactly as-is for every group (`throttle:10,1`, `inv.scope:write`, etc., per D-05) — this is the file where `routes.php`'s line-by-line shape must be visually preserved, so prefer one `r.Group(...)` block per PHP route group in the same order as `routes.php`. + +--- + +### `../fonoteka.go/plugins/golem15/fonoteka/plugin.go` — extend Boot to register guard + buckets (bootstrap, event-driven) + +**Analog:** self (full file already read, 61 lines) — extend `Boot` and `Middlewares()`: + +```go +func (p *Plugin) Boot(app *backpack.App) error { + p.app = app + classes.SetDebug(app.Config != nil && app.Config.Bool("app.debug")) + if gdb, ok := app.Lookup[*gorm.DB](); ok { + if err := classes.RegisterHooks(gdb); err != nil { + return err + } + } + return nil +} + +func (p *Plugin) Middlewares() map[string]pact.Middleware { + return map[string]pact.Middleware{ + "inv.must-change-password": middleware.MustChangePassword, + } +} +``` +Add guard registration (`bouncer.RegisterGuard(p.ID(), "inv_token", auth.NewTokenGuard(gdb))`, guarded by the same `app.Lookup[*gorm.DB]()` presence check already used for hooks) and bucket registration (5 named buckets via whatever `surf`-side bucket-registration function the limiter package exposes — likely a new `pact.HasBuckets`-style capability interface mirroring `pact.HasMiddleware`'s `Middlewares() map[string]pact.Middleware` shape) inside `Boot`, after the existing `gdb` lookup. Add `"inv.scope:read"`/`"inv.scope:write"`/`"inv.scope:ai"` is NOT a fixed-name middleware map entry — it's parameterized (D-05), so it needs the same kind of factory-registration `surf.Use("inv.scope:write")` string-parsing the router already does for `throttle:N,M`; confirm at plan time whether `Middlewares()` needs a parallel `MiddlewareFactories()` capability or whether `surf`'s existing named-middleware resolution is extended to call a factory when the name contains `:`. + +--- + +## Shared Patterns + +### Named-registration-fails-on-duplicate +**Source:** `surf/router.go` `RegisterMiddleware` (lines 67-80); also `party/registry.go` `Register`/`Activate` (duplicate/topo-sort-fail-boot family, lines 15-60+). +**Apply to:** `bouncer/registry.go` (guard registry), any new bucket-registration function in `surf`. +```go +func (r *Router) RegisterMiddleware(pluginID, name string, fn pact.Middleware) error { + if r == nil { + return fmt.Errorf("surf: router is nil") + } + if name == "" || fn == nil { + return fmt.Errorf("surf: plugin %q registered empty middleware", pluginID) + } + if existing, ok := r.named[name]; ok { + return fmt.Errorf("surf: middleware %q already registered by %s", name, existing.pluginID) + } + r.named[name] = namedMiddleware{pluginID: pluginID, fn: fn} + return nil +} +``` + +### Opaque JSON 500 / house error envelope +**Source:** `surf/router.go` `recoverJSON` (lines 290-301); `fonoteka.go` `controllers/genre_controller.go` `writeOpaque500` (lines 149-153); `bouncer/jwt.go` `write401` (lines 135-139). +**Apply to:** Every non-raw-group handler and middleware; explicitly NOT the raw OAuth group (D-16 — raw group gets a bare 500, no JSON body, no `Content-Type`). +```go +w.Header().Set("Content-Type", "application/json") +w.WriteHeader(http.StatusInternalServerError) +_, _ = w.Write([]byte(`{"error":true,"message":"Internal server error"}`)) +``` + +### Context accessor pair (unexported key type, WithX/X functions) +**Source:** `bouncer/context.go` (full file). +**Apply to:** The new `Credential(ctx)`/`WithCredential(ctx, v)` accessor pair (D-06); `towel.WithLocale`/`towel.WithOrganization` (referenced in `surf/router.go` lines 326, 332) are the same pattern already used twice in this codebase for request-scoped context state. +```go +type userKey struct{} + +func WithUser(ctx context.Context, user *Principal) context.Context { + if ctx == nil { + ctx = context.Background() + } + return context.WithValue(ctx, userKey{}, user) +} + +func User(ctx context.Context) (*Principal, bool) { + if ctx == nil { + return nil, false + } + u, ok := ctx.Value(userKey{}).(*Principal) + return u, ok && u != nil +} +``` + +### `": "`-prefixed, plugin-and-identifier-naming errors +**Source:** every package in this repo (`surf: middleware %q already registered by %s`, `bouncer: jwt secret is empty`, `compass: config directory is empty`). +**Apply to:** every new error path this phase introduces (guard registry, limiter bucket registration, raw-group violations, fetch-guard failure reasons). Keep the package-name prefix and, where a plugin is implicated, name it explicitly (never a bare "registration failed"). + +### Never-nil slice on JSON output +**Source:** `fonoteka.go` `classes/serialize.go` `SerializeAlbum` (lines 15-22). +**Apply to:** Any handler-built DTO with a slice field (D-17's "never-nil slice helper"); the `GenreList{Data: rows}` shape in `controllers/genre_controller.go` (line 101, `rows := make([]GenreAggregate, 0)`) is a second live example of the same discipline applied at the query layer instead of the serializer layer — both are valid places to guarantee non-nil, pick whichever is closer to the data source per DTO. + +## No Analog Found + +| File | Role | Data Flow | Reason | +|------|------|-----------|--------| +| `fetchguard/fetch.go` (new package) | service | streaming | No outbound-HTTP-client code exists anywhere in either repo yet; RESEARCH.md's own Code Examples section (dial-time SSRF guard) is the concrete starting point instead of an in-repo analog — treat it as the implementation skeleton, not a summary to re-derive from scratch. | +| `surf/limiter.go` fixed-window algorithm internals (Hit/TooManyAttempts/AvailableIn control flow) | service | event-driven | No rate limiter exists in this codebase yet. RESEARCH.md's Code Examples section ports the control flow directly from Laravel's `Illuminate\Cache\RateLimiter` vendor source (read directly this session) — use that as the spec, `compass/config.go`'s mutex-guarded-struct shape only for the Go idiom of wrapping shared state safely. | +| OpenAPI generation wiring (`summer openapi:generate` or equivalent + swag annotations) | config/build-tooling | batch | No `bonfire.Command` in this repo currently wraps an external code-generation tool; `surf/serve.go`'s `bonfire.Command{Name, Flags, Run}` shape (full file) is the closest structural analog for *how to define the command*, but the swag-invocation logic itself has no precedent in-repo. | + +## Metadata + +**Analog search scope:** `bouncer/`, `surf/`, `pact/`, `compass/`, `bonfire/`, `lagoon/`, `party/`, `backpack/` in `summercms.go`; `plugins/golem15/fonoteka/{controllers,middleware,classes,models}/`, `routes.go`, `plugin.go` in `fonoteka.go`; `parity/manifest.yaml` structure checked for route/auth_group shape. +**Files scanned:** 22 read in full or targeted sections (surf/router.go, surf/serve.go, surf/params.go, surf/router_test.go, bouncer/jwt.go, bouncer/context.go, bouncer/jwt_test.go, pact/capabilities.go, compass/config.go, bonfire/command.go, party/registry.go, fonoteka.go/routes.go, plugin.go, controllers/genre_controller.go, middleware/must_change_password.go, classes/serialize.go, models/api_token.go, plus directory listings of classes/, controllers/, middleware/, parity/). +**Pattern extraction date:** 2026-09-19 diff --git a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VALIDATION.md b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VALIDATION.md index 7ad803c..109f0da 100644 --- a/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VALIDATION.md +++ b/.planning/phases/06-http-routing-auth-groups-and-rate-limiting/06-VALIDATION.md @@ -1,8 +1,8 @@ --- phase: 6 slug: http-routing-auth-groups-and-rate-limiting -status: draft -nyquist_compliant: false +status: planned +nyquist_compliant: true wave_0_complete: false created: 2026-09-19 --- @@ -36,17 +36,22 @@ created: 2026-09-19 ## Per-Task Verification Map -Task IDs are filled in by the planner once PLAN.md files exist. Requirement-level map from 06-RESEARCH.md §Validation Architecture: - | Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | |---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| -| TBD | TBD | TBD | HTTP-03 | TBD | Same handler serves JWT `/_fonoteka/api/v1/genres` and personal-token `/api/v1/fonoteka/genres`; unknown and malformed ids both 404; groups mutually exclusive in the route table | integration | `go test ./... -run 'TestGenresSharedHandler|TestGroupsMutuallyExclusive'` (fonoteka.go) | ❌ W0 | ⬜ pending | -| TBD | TBD | TBD | HTTP-04 | TBD | Five named buckets, inline `throttle:N,M`, stacked limiters; fixed-window Laravel semantics and headers; client IP only via trusted-proxy rule | unit + integration | `go test ./surf/... -run 'TestLimiter|TestClientIP'` | ❌ W0 | ⬜ pending | -| TBD | TBD | TBD | HTTP-05 | TBD | Guard registry: `jwt` and `inv_token` resolve to one `bouncer.User(ctx)`; duplicate/unknown guard name fails boot; `inv.scope` 401/403 bodies exact | unit + integration | `go test ./bouncer/... -run TestGuardRegistry` | ❌ W0 | ⬜ pending | -| TBD | TBD | TBD | HTTP-06 | TBD | `[]`, `+00:00`, tri-state `null`, omitted keys; raw OAuth group refuses house envelope/error middleware at registration, verified over the route table | unit + route-table | `go test ./surf/... -run 'TestResponseTypes|TestRawGroupExemption'` | ❌ W0 | ⬜ pending | -| TBD | TBD | TBD | HTTP-07 | TBD | Fetch helper rejects non-allow-listed host and private/loopback IPs at dial time, https only, no redirects, byte cap while streaming, timeout | unit (httptest) | `go test ./... -run TestFetch` | ❌ W0 | ⬜ pending | -| TBD | TBD | TBD | HTTP-08 | — | swag generates OpenAPI from handler annotations; `openapi-typescript` yields valid TS; drift check | build gate | phase gate script (exact command set at plan time) | ❌ W0 | ⬜ pending | -| TBD | TBD | TBD | HTTP-09 | TBD | Path-scoped CORS equals `config/cors.php` (JWT group gets no CORS headers); `http.MaxBytesReader` body limits per group | integration | `go test ./surf/... -run 'TestCORS|TestBodyLimit'` | ❌ W0 | ⬜ pending | +| 06-01-T1 | 06-01 | 1 | HTTP-05 | T-06-01 | Router verbs + parameterized middleware factories; guard registry primitives unit-tested in isolation | unit | `go test ./surf/... ./bouncer/... ./pact/... -short` (summercms.go) | ✅ created by plan | ⬜ pending | +| 06-01-T2 | 06-01 | 1 | HTTP-05 | T-06-04, T-06-05 | Real inv_token guard + inv.scope exact 401/403 bodies; jwt guard behaviorally unchanged | unit + testcontainers | `go test ./plugins/golem15/fonoteka/... ./plugins/golem15/user/... -short` (fonoteka.go) | ✅ created by plan | ⬜ pending | +| 06-01-T3 | 06-01 | 1 | HTTP-03 | T-06-02 | Same handler serves JWT and personal-token genres; parity-green on both | integration + parity | `go test ./plugins/golem15/fonoteka/... -run TestGenresSharedHandler -short && go test ./parity/... -run TestParityCorpus` (fonoteka.go) | ✅ created by plan | ⬜ pending | +| 06-02-T1 | 06-02 | 2 | HTTP-04 | T-06-06 | Fixed-window Store/Limiter matching Laravel wire contract; trusted-proxy ClientIP | unit | `go test ./surf/... -run 'TestLimiter\|TestClientIP' -short` (summercms.go) | ✅ created by plan | ⬜ pending | +| 06-02-T2 | 06-02 | 2 | HTTP-04 | T-06-07 | Five named buckets registered; token group throttled; PublicShareHeaders 429 rewrite | unit + integration | `go test ./plugins/golem15/fonoteka/... -run TestPublicShareHeaders -short` (fonoteka.go) | ✅ created by plan | ⬜ pending | +| 06-02-T3 | 06-02 | 2 | HTTP-04 | T-06-09 | All six declared route groups boot cleanly; APP_DEBUG=false pinned for future fixture recordings | integration | `go test ./plugins/golem15/fonoteka/... -run TestAllRouteGroupsBoot -short` (fonoteka.go) | ✅ created by plan | ⬜ pending | +| 06-03-T1 | 06-03 | 3 | HTTP-06 | T-06-10, T-06-11 | Raw group refuses house-envelope middleware at registration; bare 500 on raw panic; route table + route:list | unit | `go test ./surf/... -run 'TestRawGroup\|TestRouteTable' -short` (summercms.go) | ✅ created by plan | ⬜ pending | +| 06-03-T2 | 06-03 | 3 | HTTP-06, HTTP-08 | — | wire response helpers (JSON writer, +00:00 time, tri-state bool, never-nil slice); swag→openapi-typescript pipeline | unit + build-gate | `go test ./wire/... -short && bash scripts/check-openapi.sh` (fonoteka.go) | ✅ created by plan | ⬜ pending | +| 06-03-T3 | 06-03 | 3 | HTTP-09 | T-06-12, T-06-13 | Path-scoped CORS matches config/cors.php; body limits enforced with operator-confirmed production numbers; oauth group raw | integration + human-verify | `go test ./surf/... -run 'TestCORS\|TestBodyLimit' -short` (summercms.go); blocking checkpoint for production body-size numbers | ✅ created by plan | ⬜ pending | +| 06-04-T1 | 06-04 | 1 | HTTP-07 | T-06-14 | Private/reserved/CGNAT/metadata IP table matches ManualCoverUrlFetcher.php exactly | unit | `go test ./fetchguard/... -run TestIsReservedOrPrivate -v` (summercms.go) | ✅ created by plan | ⬜ pending | +| 06-04-T2 | 06-04 | 1 | HTTP-07 | T-06-14, T-06-15, T-06-16, T-06-17, T-06-18 | Dial-time SSRF guard, https-only, no redirects, streaming byte cap, typed failure reasons | unit (httptest) | `go test ./fetchguard/... -short -race` (summercms.go) | ✅ created by plan | ⬜ pending | +| 06-05-T1 | 06-05 | 4 | HTTP-03..09 | all (coverage sweep) | Framework-side coverage gaps closed (bouncer/surf/wire/fetchguard) | unit | `go test ./... -race -short` (summercms.go) | ✅ created by plan | ⬜ pending | +| 06-05-T2 | 06-05 | 4 | HTTP-03..09 | T-06-02, T-06-10 (full completion) | App-side coverage gaps; full route-table mutual-exclusivity over every route, not just genres | unit + parity | `go test ./... -race -short && go test ./parity/... -run TestParityCorpus` (fonoteka.go) | ✅ created by plan | ⬜ pending | +| 06-05-T3 | 06-05 | 4 | HTTP-03..09 | T-06-19, T-06-20 | Every T-06-xx threat mapped to a passing test or restated acceptance | doc + grep-gate | `grep -c "^\| T-06-" 06-SECURITY-REVIEW.md` | ✅ created by plan | ⬜ pending | *Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* @@ -54,12 +59,14 @@ Task IDs are filled in by the planner once PLAN.md files exist. Requirement-leve ## Wave 0 Requirements -- [ ] `surf/limiter_test.go` — fixed-window Store semantics, named-bucket resolution, inline throttle keys, stacking -- [ ] `bouncer/registry_test.go` — guard register / duplicate-fail / resolve-by-name -- [ ] Fetch-helper package `fetch_test.go` — httptest servers for each typed failure reason -- [ ] `surf/routetable_test.go` — raw-group middleware refusal at registration, route table contents -- [ ] Existing infra reused: `surf` router tests, `../fonoteka.go/parity` TestMain and `seedHooks` (token insertion for the `inv_token` guard) -- [ ] No new test framework +- [x] `surf/limiter_test.go` — fixed-window Store semantics, named-bucket resolution, inline throttle keys, stacking — scaffolded and filled by Plan 06-02 Task 1 (no pre-existing file; the plan creates it, satisfying the Nyquist "create the scaffold" rule since there is no separate Wave 0 in this phase's plan set) +- [x] `bouncer/registry_test.go` — guard register / duplicate-fail / resolve-by-name — scaffolded and filled by Plan 06-01 Task 1 +- [x] Fetch-helper package `fetch_test.go`/`ip_test.go` — httptest servers for each typed failure reason — scaffolded and filled by Plan 06-04 Tasks 1-2 +- [x] `surf/routetable_test.go` — raw-group middleware refusal at registration, route table contents — scaffolded and filled by Plan 06-03 Task 1 +- [x] Existing infra reused: `surf` router tests, `../fonoteka.go/parity` TestMain and `seedHooks` (token insertion for the `inv_token` guard, extended by Plan 06-01 Task 3) +- [x] No new test framework + +No standalone Wave 0 plan is used in this phase's plan set — each implementation plan (06-01 through 06-04) creates and fills its own test files in the same wave as the production code, and every task in every plan carries a concrete `` verify command (confirmed via `verify.plan-structure` on all 5 plans: `hasVerify: true` on every task). Plan 06-05 (wave 4) is the dedicated coverage-closing plan required by this project's lean-mode "unit tests are always the last plan" rule. --- @@ -67,17 +74,17 @@ Task IDs are filled in by the planner once PLAN.md files exist. Requirement-leve | Behavior | Requirement | Why Manual | Test Instructions | |----------|-------------|------------|-------------------| -| Production `client_max_body_size` / `post_max_size` / `upload_max_filesize` values | HTTP-09 | The production nginx vhost and php.ini are operator-managed and not in any repo (06-RESEARCH.md A2) | Operator reads the values from the production host; they are recorded in config defaults and the test asserts the recorded numbers | +| Production `client_max_body_size` / `post_max_size` / `upload_max_filesize` values | HTTP-09 | The production nginx vhost and php.ini are operator-managed and not in any repo (06-RESEARCH.md A2) | Plan 06-03 Task 3 is a `checkpoint:human-verify` (`autonomous: false`): operator reads the values from the production host; they are recorded in `fonoteka.go/config/http.yaml` replacing the INTERIM defaults, and the existing body-limit tests (which assert against config-loaded values) are re-run to confirm no regression | --- ## Validation Sign-Off -- [ ] All tasks have `` verify or Wave 0 dependencies -- [ ] Sampling continuity: no 3 consecutive tasks without automated verify -- [ ] Wave 0 covers all MISSING references -- [ ] No watch-mode flags -- [ ] Feedback latency < 120s -- [ ] `nyquist_compliant: true` set in frontmatter +- [x] All tasks have `` verify or Wave 0 dependencies — confirmed via `gsd-sdk query verify.plan-structure` on all 5 PLAN.md files (`hasVerify: true` on every task, including the one `checkpoint:human-verify` task, which also carries an `` block for its automatable portion) +- [x] Sampling continuity: no 3 consecutive tasks without automated verify — every task across all 5 plans has one +- [x] Wave 0 covers all MISSING references — no file listed as `❌ W0` in the original requirement→test map remains missing; every referenced test file is created by an explicit plan task +- [x] No watch-mode flags — all commands are one-shot `go test`/`go vet`/`bash` invocations +- [x] Feedback latency < 120s — quick-run commands are package-scoped `-short` runs +- [x] `nyquist_compliant: true` set in frontmatter -**Approval:** pending +**Approval:** planned — ready for `/gsd:execute-phase 06`