3 Commits

Include renames

Author	SHA1	Message	Date
TudorandClaude Opus 5	0fa1a292c7	fix(api): bound what a forged CF-Connecting-IP can buy ... Code review, both findings valid. The design doc claimed Cloudflare "replaces the header, so a browser cannot forge it", and that only the X-Forwarded-For fallback was forgeable. That is true only for traffic that actually passed through Cloudflare, and nothing in this process can verify that it did. Reaching the origin directly, both headers are equally attacker-controlled — and rotating CF-Connecting-IP mints a fresh rate-limit bucket per request, defeating per-client limits on every endpoint including the DataFrame-heavy /api/schools. Against abuse that is worse than the shared bucket it replaced, which at least capped everyone together. So the ceiling comes back. I dropped it earlier arguing it belonged at Cloudflare; that argument assumed the keying was sound, and it is not. GlobalRateLimitMiddleware counts all /api/ traffic in a fixed window against a total, independent of client identity, outermost so it refuses before any work happens. Written by hand because slowapi cannot express a global cap: default_limits and application_limits are both keyed by key_func, and the latter needs middleware this app does not install. It does not make the header trustworthy — it makes trusting it survivable. The real fix is Authenticated Origin Pulls or an origin firewall, now documented in DEPLOY.md as the open gap it is. 127.0.0.1 is exempt: the healthcheck curls localhost from inside the container, and starving it would restart the container and turn a load spike into an outage loop. Keyed on the peer address, never the Host header, which the caller sets. Second finding: suggest_schools_typesense promised "never raises" while the parsing loop sat outside the try, so int(None) on a malformed document would have made a keystroke a 500. The loop now skips bad rows rather than dropping the whole list — and a hit with no document no longer becomes a suggestion pointing at /school/0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj	2026-08-26 20:56:32 +01:00
TudorandClaude Opus 5	e651dd0d65	docs(suggest): the in-app global ceiling would not have worked ... Reading slowapi rather than assuming: default_limits and application_limits are both evaluated with the same key_func, so they are per-client across routes, not global. And application_limits only apply 'if in_middleware' — this app installs no SlowAPIMiddleware, so they would never have fired at all. A genuine global cap would need a second Limiter with a constant key plus that middleware. Cloudflare is already in the path on both environments and does this at the right layer, so the ceiling is named as a follow-up there rather than built badly here. The risk that leaves is stated plainly in the risks section instead of being papered over with a mechanism that does not do the job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj	2026-08-26 19:44:21 +01:00
TudorandClaude Opus 5	22c113fc29	docs(suggest): design for school autosuggest ... The load-bearing finding is not about autosuggest. The rate limiter keys on request.client.host, which in staging and prod is the Next container — so all browser users share one 60/min bucket per route. Measured against staging: 70 concurrent requests gave exactly 60 x 200 and 10 x 429. Eight concurrent searchers would 429 the site once each keystroke costs a request, so the keying fix is part of this work. Both environments are behind Cloudflare, which sets CF-Connecting-IP and overwrites any client-supplied value — trustworthy in a way a parsed X-Forwarded-For chain is not, and the backend is unreachable except through the Next proxy. Named honestly: the shared bucket has been an accidental global throttle on a single-process backend, so correct per-user keying removes a protection. A global ceiling ships with it rather than instead of it. Suggestions come from Typesense alone. The existing search path filters a 25,000-row DataFrame per query, which is exactly the cost a keystroke endpoint cannot pay, so there is deliberately no DataFrame fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj	2026-08-26 19:41:00 +01:00