Files
school_compare/docs/superpowers/specs/2026-08-26-school-autosuggest-design.md
T
TudorandClaude Opus 5 0fa1a292c7
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 8s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 10s
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

14 KiB
Raw Blame History

School Autosuggest — Design

Date: 2026-08-26 Status: approved for planning Depends on: the feature-flag layer (PR #125, merged)

Goal

Suggest schools by name as someone types in the site's main search box, so a parent who knows the school they want reaches it in one step instead of searching, scanning a result list, and clicking.

Scope is schools only. Places and postcodes were considered and excluded — see Out of scope.

The finding that shapes everything

The site's rate limiter does not do what it looks like it does.

limiter = Limiter(key_func=get_remote_address) with 60/minute reads request.client.host. In staging and production the backend has no published ports and sits on the internal backend network, so its only caller is the Next proxy — and request.client.host is therefore the Next container, for every browser user on the site.

Measured against staging: 70 concurrent requests to /api/schools returned 60 × 200 and 10 × 429. One machine consumed the whole site's budget for that minute.

Autosuggest is the worst possible feature to build on that. One person typing "st marys primary" produces six to eight debounced requests; eight concurrent searchers would 429 the site. The compare modal's search-as-you-type already shares this bucket, so the exposure exists today — autosuggest makes it certain.

Fixing the keying is therefore part of this work, not a follow-up.

1. Rate-limit keying

Both environments sit behind Cloudflare (server: cloudflare, cf-ray present on staging and production). Cloudflare sets CF-Connecting-IP on every request to the origin and overwrites any client-supplied value, which makes it trustworthy in a way a parsed X-Forwarded-For chain is not.

def client_key(request: Request) -> str:
    """Rate-limit bucket: the real caller, not the proxy in front of them."""
    cf = request.headers.get("cf-connecting-ip")
    if cf:
        return cf.strip()
    xff = request.headers.get("x-forwarded-for")
    if xff:
        return xff.split(",")[0].strip()
    return get_remote_address(request)

nextjs-app/app/api/[...path]/route.ts already forwards every inbound header except host and connection, so CF-Connecting-IP reaches the backend with no proxy change.

This header is trustworthy only for traffic that actually passed through Cloudflare, and nothing in the application can verify that it did. An earlier draft of this section claimed Cloudflare "replaces the header, so a browser cannot forge it", and that only the X-Forwarded-For fallback was forgeable. That was wrong. Cloudflare does overwrite the header on requests it handles — but a caller reaching the origin directly sets whatever it likes, and this process cannot distinguish an edge-set header from an attacker-set one. Both headers are equally forgeable in that scenario.

The consequence is sharper than a weakened defence. An attacker rotating CF-Connecting-IP per request mints a fresh rate-limit bucket every time and evades per-client limits entirely — including on the DataFrame-heavy /api/schools. Against abuse that is worse than the shared bucket it replaced, which at least capped everyone at 60/minute together.

Two mitigations, and they are not interchangeable:

  1. The real fix is at Cloudflare — Authenticated Origin Pulls, or an origin firewall that refuses connections not from Cloudflare's ranges. Only the edge can vouch for its own header. This is infrastructure work and is not part of this change; it is the thing that makes the header mean anything.
  2. The ceiling in §1.1 bounds what evading the keying can achieve while that remains open. It does not make the header trustworthy — it makes trusting it survivable.

The backend being unreachable from outside the Docker network is a real second layer, but it depends on the ingress path in front of the frontend, which this design does not control and should not assume.

1.1 The ceiling, which is back

The shared bucket was acting as an accidental global throttle on a single-process uvicorn backend that filters a 25,000-row DataFrame in-process. Correct per-user keying removes it: the origin becomes reachable at 60/min per user rather than 60/min in total, and — per above — at an unbounded rate by anyone willing to rotate a header.

An earlier draft dropped the in-app ceiling, arguing it belonged at Cloudflare. That argument assumed the keying was sound. It is not, so the ceiling is load-bearing rather than redundant, and it ships here:

GlobalRateLimitMiddleware counts all /api/ requests in a fixed 60-second window against global_rate_limit_per_minute (3000), independent of any client identity, and refuses with a 429 that names capacity rather than the client — an operator has to be able to tell "one noisy client" from "the origin is saturated". It is registered last so it is outermost: a ceiling that applies after the expensive work has run is not a ceiling.

slowapi cannot express this. default_limits and application_limits are both evaluated with the same key_func, making them per-client rather than global, and application_limits only apply with SlowAPIMiddleware installed, which this app does not use. Hence the explicit middleware — about thirty lines, and obviously correct, which is what a backstop needs to be.

Requests from 127.0.0.1 are exempt. The container healthcheck runs curl http://localhost:80/api/data-info from inside the container, and starving it would fail the check, restart the container, and turn a load spike into an outage loop. The exemption keys on the peer address, never the Host header, which the caller sets.

3000/minute is an estimate, not a measurement, and worth revisiting against real traffic.

Per-user limits

Per-user fairness and origin protection are different jobs, and this design now does both separately: the ceiling above for the origin, and per-route limits for fairness. Conflating them is what produced the original behaviour, where one bucket served the whole internet.

The existing 60/minute default is unchanged, and /api/suggest gets 120/minute. Both are estimates rather than measurements, and are a starting point to revisit once the keying is correct enough for real per-user traffic to be visible — which it was not before, because everyone shared one bucket.

2. GET /api/suggest

A dedicated endpoint, not a mode of /api/schools.

The existing search path calls Typesense for URNs and then filters, ranks and sorts the full in-memory DataFrame — a pandas pass per keystroke, holding the GIL and blocking other requests in the same worker. Suggestions need none of it: urn, school_name, phase, school_type, local_authority, postcode and ofsted_rating are all already in the Typesense document (pipeline/scripts/sync_typesense.py).

GET /api/suggest?q=<query>&limit=8
→ 200 {"suggestions": [
      {"urn": 100010, "school_name": "Brecknock Primary School",
       "local_authority": "Camden", "postcode": "NW1 1AA",
       "phase": "Primary", "school_type": "Community school"}
  ]}
  • Under two characters returns {"suggestions": []} with 200. The keystroke path never returns an error for ordinary input.
  • Typesense unavailable returns {"suggestions": []} with 200. There is deliberately no DataFrame fallback: the substring scan /api/schools falls back to is precisely the cost this endpoint exists to avoid, and a silent 25,000-row scan per keystroke is worse than no suggestions.
  • limit is clamped to 20. It is a public endpoint.
  • Rate limit 120/minute per client, not the default 60. A 200 ms debounce tops out near 5 requests/second while someone is actively typing, but averages far below that across a real search; 120 leaves headroom for bursts while still bounding one client.
  • Local authority is part of the payload, not decoration. There are many schools called "St Mary's"; a suggestion list without the authority is unusable for exactly the queries autosuggest is meant to serve.

Caching

CACHE_RULES gains ("/api/suggest", (60, 3600, 86400)). Prefix queries repeat enormously across users and school names change once a year.

The client fetch must not use cache: "no-store". The compare modal does, and copying that pattern would throw away both the browser cache and the ETag 304s the existing CacheAndETagMiddleware already provides.

Both environments currently report cf-cache-status: DYNAMIC — Cloudflare ignores the Cache-Control the API already sends, because it does not cache dynamic paths by default. A Cloudflare Cache Rule for /api/suggest* would let the edge absorb most of this traffic and never reach the origin. That is a dashboard change, it is optional, and nothing here depends on it.

3. The combobox

This is an ARIA combobox, not a text input with a list underneath.

Files. FilterBar.tsx is already long. The work splits three ways: hooks/useSchoolSuggest.ts owns fetching, debouncing and cancellation; components/SuggestList.tsx owns rendering and ARIA; FilterBar.tsx wires them to the existing input and form.

Fetching. 200 ms debounce; minimum two characters; an AbortController cancels the superseded request on every keystroke. Cancellation is not an optimisation — without it, a slow response for "st" can land after the fast one for "st marys" and replace a correct list with a stale one.

Suppressed during postcode entry. The box takes a school name or a postcode, and isValidPostcode already distinguishes them. Suggestions do not appear once the value parses as a postcode.

Keyboard. ArrowDown/ArrowUp move the active option, Escape closes and keeps the typed text, Tab closes. Enter with an option active navigates to that school's page. Enter with none active submits the free-text search exactly as it does today — the existing behaviour is preserved, not replaced.

ARIA. role="combobox" with aria-expanded and aria-controls on the input, aria-activedescendant pointing at the active option, role="listbox" on the list and role="option" on each row.

Both instances get it. HomeView renders FilterBar twice — hero and sticky — from one component, so there is one implementation.

4. Behind a flag

Flag school_autosuggest, declared in backend/flags.py, default off.

This is the most-used control on the site and the first change to it in a while. app/page.tsx is an async server component, so it reads the flag and threads it to FilterBar through HomeView — two prop hops, explicit, no client-side flag read.

Off means the input behaves exactly as it does today: no listener, no fetch, no markup. Not a rendered-then-hidden dropdown.

The rate-limit keying is not flagged. It is a correctness fix that should apply whether or not autosuggest is on, and flagging it would mean shipping a known-wrong limiter into production deliberately.

5. Analytics

search_submitted already carries via: 'input'. Selecting a suggestion fires it with via: 'suggestion' plus the chosen urn, so the obvious question — does this actually help, or do people ignore it — has an answer in the data rather than an opinion.

6. Testing

Backend. client_key prefers CF-Connecting-IP, falls back through X-Forwarded-For to the remote address, and two different values get two different buckets. /api/suggest returns matches, returns empty below two characters, returns empty and 200 when Typesense is unavailable, and clamps limit. That it never touches the DataFrame is asserted by making load_school_data raise and requiring the endpoint to answer anyway.

Frontend. The hook debounces, aborts superseded requests, and drops a late-arriving response for a stale query. The list renders the ARIA attributes. Keyboard navigation moves the active option; Enter on an option navigates; Enter on none submits the search.

E2E. With the flag on, typing a known school name shows it and selecting it lands on that school's page. With the flag off, no combobox markup exists. Gated on the flag the same way the distance journeys are — read the observable effect, since /api/flags is denied to the public.

7. Risks

Removing the accidental throttle. Covered in §1. Correct per-user keying means the origin is reachable at 60/minute per user where it was 60/minute in total, and no in-app global cap replaces it — that job goes to Cloudflare, which is not done as part of this change. Until it is, a determined caller with many source addresses can put more load on a single-process origin than they can today. Against this site's traffic that is a theoretical risk rather than a live one, but it is a real one and it is the price of the fix.

Cloudflare bypass — the open one. If the origin is reachable without passing through Cloudflare, CF-Connecting-IP is attacker-controlled, and rotating it per request defeats per-client limits on every endpoint. The ceiling in §1.1 bounds the damage to the origin's total capacity; it does not restore per-client fairness under attack, and it cannot. Closing this properly means Authenticated Origin Pulls or an origin firewall restricted to Cloudflare's published ranges — infrastructure work, outside this change, and the single most valuable follow-up here.

Typesense becomes user-visible. Today a Typesense outage degrades search to a slow substring match. With autosuggest it also means the dropdown silently stops appearing. That is the correct failure — quiet, not broken — but it makes Typesense health worth monitoring in a way it was not before.

Out of scope

  • Place suggestions. The 2,646 town, authority and outcode pages are a strong candidate and would route people onto the pages W2 built, but they live in the place registry rather than Typesense, so it is a second index and a ranking rule for comparing two kinds of result. Worth its own change.
  • Postcode completion. Would put postcodes.io in the keystroke path, with its own latency and rate limits.
  • The compare modal. It already has search-as-you-type. Converting it to this component is a reasonable follow-up, not part of this.
  • Recent or popular searches. No storage for either, and no evidence yet that they are wanted.