36 KiB
36 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 06-http-routing-auth-groups-and-rate-limiting | 02 | execute | 2 |
|
|
true |
|
|
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.FixedWindowLimiter/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.
<execution_context> @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md </execution_context>
@.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 IMPORTANT -- naming collision this plan must avoid: summercms.go/surf/router.go (as of Phase 3, unchanged by 06-01) already declares:package surf
// 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) }
This plan's concrete rate limiter is therefore named FixedWindowLimiter, NOT
Limiter -- declaring type Limiter struct{...} in the same package would be
an illegal redeclaration. Resolution (CONTEXT D-03: the real limiter lives
"behind the existing surf.Limiter seam"):
- The existing
Limiterinterface, its doc comment, and its single-argumentWrap(http.Handler) http.Handlershape are left EXACTLY as they are -- they are not deleted, not renamed, not reused as a base type. They remain the Phase 3 seam per D-03, available to a future call site that wants a single default limiter with no per-route bucket name. FixedWindowLimiter(below) does NOT implementLimiter'sWrapmethod. Its shape is fundamentally different (parameterized by a bucket-name-or- inline-throttleparam string, not a bare wrap), because D-05 requires rate limiting to be expressed exclusively through named/parameterized "throttle:..." middleware entries at call sites, not a blanket per-route wrapper -- there is no meaningful "default limiter with no name" in this design forWrapto represent.noopLimiterandnoOpLimitare REMOVED in this plan, along with their one call site (h = noOpLimit(h)inwrap()). They existed only to satisfy that one blanket call site; once it is removed (rate limiting is now exclusively via named "throttle:..." entries), they have no remaining purpose and are deleted rather than left as dead code.
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/BuildRouter, since pact cannot import surf without a cycle). type BucketProvider interface { Buckets() map[string]Bucket }
// FixedWindowLimiter is the concrete rate limiter (distinct from the // pre-existing surf.Limiter interface -- see the naming-collision note // above). It owns the Store, the named-bucket table, and the inline-throttle // parser, and produces the "throttle" middleware factory's per-route // pact.Middleware. type FixedWindowLimiter struct{ /* unexported: store Store; trusted []netip.Prefix; buckets map[string]Bucket */ }
// NewFixedWindowLimiter is the ONE constructor signature for this type -- // trusted is required at construction (not a later setter) because both // named-bucket Key closures (registered later via RegisterBucket) and the // inline "N,M" throttle's own key resolver need the same trusted-proxy list, // and threading it through every call site individually would risk two // different trusted lists disagreeing within one Router. func NewFixedWindowLimiter(store Store, trusted []netip.Prefix) *FixedWindowLimiter
func (l *FixedWindowLimiter) 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(r, l.trusted)) // 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 *FixedWindowLimiter) Middleware(param string) pact.Middleware
// ValidateThrottle is called once per route at Assemble/BuildRouter 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 *FixedWindowLimiter) 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 FixedWindowLimiter field (nil-safe: BuildRouter always sets it, so this is only nil for a Router built directly via New() outside BuildRouter, e.g. in an existing router_test.go unit test that never registers "throttle:..." -- such a route simply never reaches the throttle factory). BuildRouter (the 06-03-introduced split of Assemble, or Assemble itself if 06-03 has not run yet in this working tree -- Task 1's action names the exact call site either way) constructs one surf.NewFixedWindowLimiter(surf.NewMemoryStore(2time.Minute), surf.TrustedProxies(app.Config)) 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, and noopLimiter/noOpLimit are deleted (see the naming-collision note above) -- rate limiting is now expressed exclusively via named "throttle:..." middleware entries at call sites, matching D-05.
Task 1 (summercms.go): Fixed-window Store, FixedWindowLimiter, 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, the existing Limiter interface/noopLimiter/noOpLimit at the bottom of the file, 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 above -- read the naming-collision note first: Bucket, BucketProvider, FixedWindowLimiter (NOT Limiter -- that name is already the pre-existing interface in router.go), NewFixedWindowLimiter(store Store, trusted []netip.Prefix) *FixedWindowLimiter (the ONE constructor signature -- trusted is a required constructor argument, never a setter, never a second optional overload), 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, l.trusted) (l.trusted is the slice captured at NewFixedWindowLimiter construction time -- do not re-read config per request); (3) else ValidateThrottle/Middleware returns an error (surfaced at Assemble/BuildRouter 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 *FixedWindowLimiter field to Router. In Assemble() (before 06-03 introduces the BuildRouter split -- if 06-03 has already executed in this working tree when this task runs, make the equivalent change in BuildRouter instead, since Assemble will then just call BuildRouter+compile), after constructing r := New(corsOrigins(app)), build trusted := TrustedProxies(app.Config), lim := NewFixedWindowLimiter(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(). Delete the now-dead noopLimiter type and noOpLimit function entirely, and keep the existing Limiter interface declaration as the Phase 3 seam, changing only its doc comment (which currently claims "Phase 6 replaces the no-op with named buckets" and would be stale) to state that Limiter is a retained seam with no implementer after Phase 6 and that rate limiting is provided by FixedWindowLimiter through the parameterized throttle middleware (D-03, D-05) (per the interfaces block's naming-collision note; confirm via grep that noOpLimit/noopLimiter have no other call sites in the repo before deleting -- there are none as of Phase 3/06-01).
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).
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.
<threat_model>
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 |
| </threat_model> |
<success_criteria>
- surf.FixedWindowLimiter reproduces Laravel's fixed-window ThrottleRequests contract, including success-response headers and stacking, alongside the untouched pre-existing surf.Limiter interface.
- 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. </success_criteria>