diff --git a/.env.example b/.env.example index 897629f400f..f0b98b2d794 100644 --- a/.env.example +++ b/.env.example @@ -57,6 +57,13 @@ PORT=8000 # requests with a valid JWT but an unlisted email return 403. # ADMIN_ALLOWED_EMAILS=alice@example.com,bob@example.com +# Shared secret a Cloudflare Transform Rule stamps as X-Origin-Secret on every +# request it proxies for api.anyplot.ai; api/origin_gate.py refuses anything +# without it, which closes the direct *.run.app door. LEAVE THIS UNSET locally +# and in tests — unset means the gate is off, and that is also the production +# rollback. Set only on the Cloud Run service, from Secret Manager. +# ORIGIN_SECRET= + # ============================================================================ # AI Services (optional) # ============================================================================ diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c1d6086789..0475c20e969 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,41 @@ aggregate instead: an italic *Catalog* line at the end of the version section an ### Added +- **A shared-secret origin gate closes the direct `*.run.app` door, and the apex Worker's + source moves into the repository** — the API runs on Cloud Run with `ingress=all`, so it + answers on two addresses: `api.anyplot.ai`, which Cloudflare proxies, and the raw + `*.run.app` URL, which it does not. Everything the edge enforces — the bot challenge, the + WAF, the cache that makes the `max-age=300` reads free — was one URL away from being + bypassed, and `api/request_context.py` already documented callers doing it. A Cloudflare + Transform Rule stamps `X-Origin-Secret` on everything it proxies for the API host, and + `api/origin_gate.py` refuses anything without it with 403 before the request costs + anything. **Unset means off**, which is what makes the rollback a single variable and + keeps local development and the test suite untouched: the code can ship long before the + rule and the secret exist. `/health` reports `origin_gate` (`off` · `off-seen` · `ok` · + `missing` · `mismatch`) for the request it was asked with — never the value — so every + route into the service can be measured *before* the switch is thrown; `off-seen` is the + state every path that must keep working has to reach first. Exempt, as exact paths with no + prefixes: `/health` (the deploy smoke reaches the candidate on its `run.app` tag URL, which + never passes the edge) and `/debug/cache/invalidate` (`sync-postgres.yml` posts to the + direct URL by design, because Cloudflare's bot challenge answers an unauthenticated curl + POST with a 403 HTML page; that endpoint carries its own constant-time token), plus + `OPTIONS`, which a browser cannot attach a custom header to. `/seo-proxy/…` is deliberately + **not** exempt although the sibling repo exempts it: the site's nginx fetches those pages + over `api.anyplot.ai` and so carries the header, while an exemption would leave the API's + most expensive reads open on the direct URL — a cache miss or an unknown id queries the + repositories, and a crawler user agent schedules an outbound Plausible event per request. + Every header secret is now compared through one byte-wise comparator + (`api/secret_compare.py`, used by the gate and by both `/debug/*` locks), because + `secrets.compare_digest` raises `TypeError` on a non-ASCII `str` while a header arrives + latin-1-decoded from the wire: comparing strings handed any caller a one-byte way to turn a + cheap 401 or 403 into an unhandled, logged 500 — including on `/debug/cache/invalidate`, + which is exempt from the gate precisely because it has its own lock. The Cloudflare Worker + behind `anyplot.ai/api/*` now has + its source in `infra/cloudflare/`, because a Worker subrequest to a host in the same zone + bypasses that zone's Transform Rules — so the Worker stamps the header itself, deleting + any inbound one first so a caller cannot supply it. The pre-traffic smoke reads the secret + at run time and sends it, accepting `off`/`off-seen` so the pipeline keeps working before + the gate is armed and after a rollback. (#11208) - **The agent instructions are pinned by a test, and the drift it found is fixed** — `CLAUDE.md` and `.github/copilot-instructions.md` both open with the claim that they stay in sync, and both are read as binding shorthand, but nothing checked either claim. `tests/unit/test_agent_instructions.py` @@ -73,7 +108,6 @@ aggregate instead: an italic *Catalog* line at the end of the version section an branch, so a dispatch from a feature branch can never raise or close a production incident. Timeout recomputed to 62 min by the file's own formula (36 checks x 90 s + five non-retried probes). (#11209) - - **The API image is built and its container smoke-tested before merge, not after** — the first build attempt of a changed Dockerfile used to happen in Cloud Build, once the PR was already on `main`; that is how the deploy-api trigger sat red from 2026-08-30 until #10821 @@ -256,6 +290,25 @@ aggregate instead: an italic *Catalog* line at the end of the version section an idle window — so the instance is in practice never reclaimed and visitors keep the same time to first byte. `anyplot-api` keeps `min-instances=1`: its cold start is ~11.6 s and its traffic does leave gaps over 15 minutes. (#10812) +- **The API deploy configures the revision additively — `--update-secrets` and + `--update-env-vars`, not the `--set-` forms** — both `--set-` flags replace their whole set, + so anything attached to the service out of band is stripped from every revision the pipeline + creates. `ORIGIN_SECRET` is exactly such a binding — attached by hand to arm the origin gate, + removed by hand to roll back — and a secret-backed variable lives in the same revision + environment as a literal one, so either flag was a way to silently disarm the gate on the + next deploy. It cannot simply be listed in the flag instead: Cloud Run refuses a deploy + naming a secret that does not exist, which would break every build until the rollout creates + it. The cost is that a variable dropped from either line is no longer removed automatically. + (#11208) +- **The analytics middleware moves inside `CORSMiddleware`** — a consequence of where the + origin gate has to sit. The gate belongs inside CORS, so its 403 still carries the headers + a browser needs to read it as a 403 rather than as an opaque network error, and outside + the bot counter, so a refused request can never fire an outbound Plausible event — + `track_asset_fetch` fires per request for anything with a crawler user agent, so a caller + on the direct URL could otherwise turn each of its own refusals into one. Those two are + only simultaneously possible with the counter inside CORS. The cache-header middleware + stays outside CORS, where its `setdefault` for the /og/ cards depends on being. `api/main.py` + now carries the stack order and the reason for each position. (#11208) - **The frontend declares the Node version it is actually built with, and something enforces it** — `app/package.json` asked for `node >=20` while the image that produces the deployed bundle builds on Node 22 and CI tests on Node 24, so the only version the diff --git a/agentic/docs/project-guide.md b/agentic/docs/project-guide.md index 30836b2c724..41c19456986 100644 --- a/agentic/docs/project-guide.md +++ b/agentic/docs/project-guide.md @@ -202,6 +202,8 @@ Example: `plots/scatter-basic/` contains everything for the basic scatter plot. - **`agentic/workflows/`**: Click CLI scripts (plan, build, test, review + orchestrators) - **`agentic/commands/`**: Markdown prompt templates - **`automation/`**: CI/CD helper scripts (workflow_cli, label_manager, sync_to_postgres) +- **`infra/`**: Infrastructure that would otherwise live only in a dashboard + - **`infra/cloudflare/`**: Source of the apex `anyplot.ai/api/*` Worker, plus the origin gate's rollout and measuring procedure - **`tests/`**: Unit, integration, and e2e tests mirroring source structure - **`docs/`**: Architecture and workflow documentation diff --git a/api/cloudbuild.yaml b/api/cloudbuild.yaml index 34ec5fdb51e..0e4042b8e7f 100644 --- a/api/cloudbuild.yaml +++ b/api/cloudbuild.yaml @@ -68,10 +68,31 @@ steps: # /insights/visitors on the public stats page). The Secret Manager # entry must exist before the first deploy that includes this line — # create it with: gcloud secrets create PLAUSIBLE_API_KEY --data-file=- - - "--set-secrets=DATABASE_URL=DATABASE_URL:latest,CACHE_INVALIDATE_TOKEN=CACHE_INVALIDATE_TOKEN:latest,ADMIN_TOKEN=ADMIN_TOKEN:latest,PLAUSIBLE_API_KEY=PLAUSIBLE_API_KEY:latest" + # + # `--update-secrets`, NOT `--set-secrets`: the latter replaces the whole + # binding set, so it would strip any secret attached out of band from + # every revision this pipeline creates. ORIGIN_SECRET (api/origin_gate.py) + # is exactly such a binding — it is attached by hand when the gate is + # armed and removed by hand to roll back, and `--set-secrets` would + # silently disarm the gate on the next deploy. It cannot be listed here + # instead: Cloud Run refuses a deploy that names a secret which does not + # exist, which would break every build until the rollout reaches the step + # that creates it. The cost of `--update-secrets` is that a binding + # dropped from this line is no longer removed automatically — worth it + # against a gate that turns itself off. + - "--update-secrets=DATABASE_URL=DATABASE_URL:latest,CACHE_INVALIDATE_TOKEN=CACHE_INVALIDATE_TOKEN:latest,ADMIN_TOKEN=ADMIN_TOKEN:latest,PLAUSIBLE_API_KEY=PLAUSIBLE_API_KEY:latest" - "--execution-environment=gen2" # ^|^ alt delimiter: values contain @ (emails) and may contain , (multi-email lists) - - "--set-env-vars=^|^ENVIRONMENT=production|GOOGLE_CLOUD_PROJECT=$PROJECT_ID|GCS_BUCKET=anyplot-images|CF_ACCESS_TEAM_DOMAIN=${_CF_ACCESS_TEAM_DOMAIN}|CF_ACCESS_AUD=${_CF_ACCESS_AUD}|ADMIN_ALLOWED_EMAILS=${_ADMIN_ALLOWED_EMAILS}" + # + # `--update-env-vars` for the same reason as `--update-secrets` above, and + # belt and braces on top of it: a secret-backed variable lives in the same + # revision environment as a literal one, so a destructive `--set-env-vars` + # is a second way this pipeline could drop the hand-attached + # ORIGIN_SECRET and silently disarm the gate (Copilot review). Additive on + # both flags means one deploy cannot undo an out-of-band change; the cost + # is that a variable dropped from this line is no longer removed by the + # next deploy, which is the trade already accepted for the secrets. + - "--update-env-vars=^|^ENVIRONMENT=production|GOOGLE_CLOUD_PROJECT=$PROJECT_ID|GCS_BUCKET=anyplot-images|CF_ACCESS_TEAM_DOMAIN=${_CF_ACCESS_TEAM_DOMAIN}|CF_ACCESS_AUD=${_CF_ACCESS_AUD}|ADMIN_ALLOWED_EMAILS=${_ADMIN_ALLOWED_EMAILS}" - "--cpu-throttling" - "--concurrency=15" - "--timeout=600" @@ -112,18 +133,51 @@ steps: # _MIN_INSTANCES says, so the first call that touches the DB may be the # first real DB request of that container's life and can fail once. RETRY="--retry 5 --retry-delay 5 --retry-all-errors" + # The candidate is probed on its `run.app` tag URL, which by definition + # never passes the Cloudflare edge — so once ORIGIN_SECRET is set on the + # service (api/origin_gate.py) every probe but /health needs the header + # the edge would have stamped. Read here rather than through + # `availableSecrets` on purpose: that resolves at build start and would + # fail every build until the secret exists, which is precisely the first + # step of the rollout. Missing secret or missing permission => empty => + # the probes run bare, which is correct while the gate is off and fails + # loudly at /libraries once it is on. The value is captured, never + # echoed; the step runs without `set -x`. + ORIGIN_SECRET=$$(gcloud secrets versions access latest --secret=ORIGIN_SECRET 2>/dev/null || true) + HDR=() + if [ -n "$$ORIGIN_SECRET" ]; then HDR=(-H "X-Origin-Secret: $$ORIGIN_SECRET"); fi + # /health stays bare: it is exempt from the gate, and that is what makes + # it the probe that always reaches a cold candidate. curl -fsS $$RETRY "$$URL/health" | grep -q '"healthy"' + # …and that the secret this BUILD can read is the one the SERVICE was + # given. /health reports the verdict for the request it was asked with + # (never the value), so a rotation applied to only one of the two shows + # up here instead of as a mysterious 403 after the promote. + # `off`/`off-seen` are ACCEPTED, not failures: they are the gate before + # it is armed and after a rollback, and a build that refused to run then + # would take the deploy pipeline down exactly when it is needed most. + # Only `mismatch` is a real disagreement. + if [ -n "$$ORIGIN_SECRET" ]; then + gate=$$(curl -fsS $$RETRY "$${HDR[@]}" "$$URL/health" | python3 -c "import json,sys; print(json.load(sys.stdin).get('origin_gate'))") + case "$$gate" in + ok) echo "origin gate: armed, and this build's secret matches" ;; + off|off-seen) echo "origin gate: $$gate (not armed on this revision)" ;; + *) echo "origin gate says '$$gate' for this build's secret — service and build disagree"; exit 1 ;; + esac + fi # /libraries and /languages fall back to static metadata when the DB is # unreachable (optional_db), so they prove the app serves but not the # database. /plots/filter takes require_db — it is the probe that fails # when the Cloud SQL connection is broken. - curl -fsS $$RETRY "$$URL/libraries" | grep -q '"libraries"' - curl -fsS $$RETRY "$$URL/languages" | grep -q '"languages"' - curl -fsS $$RETRY "$$URL/plots/filter" >/dev/null + curl -fsS $$RETRY "$${HDR[@]}" "$$URL/libraries" | grep -q '"libraries"' + curl -fsS $$RETRY "$${HDR[@]}" "$$URL/languages" | grep -q '"languages"' + curl -fsS $$RETRY "$${HDR[@]}" "$$URL/plots/filter" >/dev/null # Fail-closed admin gate. 401 is the answer with ADMIN_TOKEN present and # no header sent; a 503 here would mean the secret never arrived, which - # is exactly the misconfiguration worth failing the build over. - code=$$(curl -s $$RETRY -o /dev/null -w '%{http_code}' "$$URL/debug/status") + # is exactly the misconfiguration worth failing the build over. With the + # gate armed the origin header has to be sent too, or this reads 403 and + # says "admin gate" about something that never reached it. + code=$$(curl -s $$RETRY "$${HDR[@]}" -o /dev/null -w '%{http_code}' "$$URL/debug/status") test "$$code" = "401" || { echo "admin gate expected 401, got $$code"; exit 1; } echo "smoke OK" id: "smoke" diff --git a/api/main.py b/api/main.py index a6ba3c335ad..d69e48b8f80 100644 --- a/api/main.py +++ b/api/main.py @@ -25,6 +25,7 @@ http_exception_handler, ) from api.mcp.server import mcp_server # noqa: E402 +from api.origin_gate import OriginSecretMiddleware # noqa: E402 from api.routers import ( # noqa: E402 debug_router, download_router, @@ -161,26 +162,35 @@ async def lifespan(app: FastAPI): app.add_exception_handler(HTTPException, http_exception_handler) app.add_exception_handler(Exception, generic_exception_handler) +# The middleware stack, written innermost-first because `add_middleware` and +# `@app.middleware` both wrap what is already there — so reading this file from +# here down gives the order a request actually travels, in reverse: +# +# cache headers → CORS → origin gate → bot counter → gzip → router +# +# (`HeadAsGetMiddleware` and `MCPTrailingSlashMiddleware` wrap the whole app +# further out still; both only rewrite the scope.) +# +# Two of those positions are load-bearing: +# +# * The origin gate directly inside CORS, so a 403 from it still carries the +# headers a browser needs to read it as a 403 rather than as an opaque +# network error — and OUTSIDE the bot counter, so a refused request can never +# fire an outbound Plausible event. That second one is why the counter moved +# in here from outside CORS: `track_asset_fetch` fires per request for +# anything with a crawler user agent, so a caller on the direct `run.app` URL +# could otherwise turn each of its own refusals into one. +# * The cache-header middleware stays OUTSIDE CORS, because its `setdefault` +# for the /og/ cards is what keeps CORSMiddleware's own header when the +# request came from an allowlisted origin — it has to run after CORS on the +# way out. + # Enable GZip compression for responses > 500 bytes # This significantly reduces payload size for JSON API responses # (e.g., /plots/filter: 301KB -> ~40KB with gzip) # Note: GZip must be added before CORS so compression happens before CORS headers are added app.add_middleware(GZipMiddleware, minimum_size=500) -# Configure CORS. Origins come from settings.cors_origins (single source of -# truth — a hardcoded list here previously left https://www.anyplot.ai out -# even though config promised it); the regex additionally allows any -# localhost port for local dev servers. -app.add_middleware( - CORSMiddleware, - allow_origins=settings.cors_origins, - allow_origin_regex=r"http://localhost:\d+", - allow_credentials=True, - allow_methods=["*"], - allow_headers=["*"], - expose_headers=["Mcp-Session-Id"], # MCP session tracking -) - # Record which AI or search agent requested which catalogue page. # @@ -214,6 +224,26 @@ async def record_bot_fetch(request: Request, call_next): return response +# Close the direct `*.run.app` door: require the header the Cloudflare edge +# stamps. Dormant until ORIGIN_SECRET is set on the service, which is both the +# rollout order and the rollback (api/origin_gate.py). +app.add_middleware(OriginSecretMiddleware) + +# Configure CORS. Origins come from settings.cors_origins (single source of +# truth — a hardcoded list here previously left https://www.anyplot.ai out +# even though config promised it); the regex additionally allows any +# localhost port for local dev servers. +app.add_middleware( + CORSMiddleware, + allow_origins=settings.cors_origins, + allow_origin_regex=r"http://localhost:\d+", + allow_credentials=True, + allow_methods=["*"], + allow_headers=["*"], + expose_headers=["Mcp-Session-Id"], # MCP session tracking +) + + # Add cache headers middleware @app.middleware("http") async def add_cache_headers(request: Request, call_next): diff --git a/api/origin_gate.py b/api/origin_gate.py new file mode 100644 index 00000000000..423b5847267 --- /dev/null +++ b/api/origin_gate.py @@ -0,0 +1,172 @@ +"""Shared-secret gate that closes the direct `*.run.app` door. + +Both Cloud Run services stand with `ingress=all` — there is no load balancer, +and putting one in front would cost more per month than the services do. So the +API answers on two addresses: `https://api.anyplot.ai`, which is proxied by +Cloudflare, and the raw `*.run.app` URL, which is not. Everything Cloudflare +enforces for this site — the bot challenge, the WAF, the cache that makes the +`public, max-age=300` reads free — is one URL away from being bypassed, and +`api/request_context.py` already documents callers doing it. + +A Cloudflare Transform Rule stamps `X-Origin-Secret: ` onto every +request it proxies for `api.anyplot.ai`. This middleware requires that header, +so a caller who skips the edge is refused with 403 before the request costs +anything. It is not authentication — it says "you came through the front door", +nothing about who you are; `api/dependencies.py` still decides what a caller may +do on the `/debug/*` routes. + +One path stamps it itself rather than getting it from the rule: `anyplot.ai/api/*` +runs through a Cloudflare Worker, and a Worker subrequest to a host in the SAME +zone skips that zone's Transform Rules (measured during kurrentschrift's rollout; +the Worker's source now lives in `infra/cloudflare/`). + +**The check is OFF unless `ORIGIN_SECRET` is set.** That is the rollback: take +the variable off the Cloud Run service, then promote the resulting revision — +this service pins traffic to a named revision, so a new one serves nothing until +it is promoted. Local development and the test suite therefore never see the +gate, and the rollout can put the code in production long before the rule and +the secret exist. + +Exempt by path, and only these two: + +* `/health` — the deploy's pre-traffic smoke probes the candidate revision on + its `run.app` tag URL, which by definition never passes the edge. Gating it + would make every deploy fail closed. +* `/debug/cache/invalidate` — the one legitimate caller that has no front door. + `sync-postgres.yml` flushes the cache from a GitHub runner at the end of each + sync, and it posts to the direct `*.run.app` URL *on purpose*: Cloudflare's + bot challenge answers an unauthenticated curl POST against `api.anyplot.ai` + with a 403 HTML page. The endpoint carries its own shared secret + (`CACHE_INVALIDATE_TOKEN`, constant-time compared) and returns 503 when none + is configured, so it is gated — just by a different lock. Sending the origin + secret from CI instead would let this exemption go; that needs a repository + secret and a workflow change, and is named as the follow-up in the PR that + introduced this file. + +`/seo-proxy/…` is deliberately NOT exempt, though the sibling repo exempts it +belt-and-braces. The site's nginx fetches the prerendered pages over +`https://api.anyplot.ai` (`app/nginx.conf` `@seo_proxy` and `@seo_proxy_python`), +so that path does pass the edge and does carry the header — while an exemption +would leave the most expensive reads in the API open on the direct URL: a cache +miss or an unknown id queries `SpecRepository`/`ImplRepository`, and a crawler +user agent schedules an outbound Plausible event per request. That is precisely +the cost this gate exists to refuse (Copilot review). The rollout measures the +crawler path end to end before arming, and `bot-serving-check.yml` runs daily, +so being wrong here is loud rather than silent. + +`OPTIONS` never reaches the gate in the shipped stack — `CORSMiddleware` sits +outside it (`api/main.py`) and answers a preflight itself — but it is let +through explicitly anyway: a browser cannot attach a custom header to a +preflight, so a gate that refused one would break every cross-origin call on the +site rather than protecting anything. The exemption is what makes that +independent of where the middleware ends up in the stack. + +**What this gate does not close.** It protects the API service's own door. The +APP service (`anyplot-app`) also stands with `ingress=all`, and its nginx +relays a crawler user agent through `@seo_proxy` to `https://api.anyplot.ai` — +where the edge stamps the header legitimately. So a caller who sends a crawler +user agent to the app's raw `*.run.app` URL still reaches the prerendered +render, and its DB queries and Plausible event, without having passed the edge +himself (Copilot review). That is a second door on a second service, not a hole +in this one: the request this process sees genuinely came through the edge. +Closing it means gating the app service or refusing to proxy for `run.app` +hosts in `app/nginx.conf` — and `bot-serving-check.yml` deliberately probes that +exact flow on the app origin, so it is a change with its own blast radius and +its own PR. +""" + +from __future__ import annotations + +from fastapi import Request +from fastapi.responses import JSONResponse + +from api.secret_compare import secret_matches +from core.config import settings + + +ORIGIN_SECRET_HEADER = "x-origin-secret" + +# Exact paths only, no prefixes: a prefix exemption is how a gate quietly grows +# a hole. See the module docstring for what each of these two buys, and why +# `/seo-proxy/…` is not among them. +EXEMPT_PATHS = frozenset({"/health", "/debug/cache/invalidate"}) + + +def gate_is_armed() -> bool: + """Whether a secret is configured at all. + + Read per call, not at import: unsetting the variable is the rollback, and + the tests flip the setting and expect the next request to notice. + """ + return bool(settings.origin_secret) + + +def is_exempt(path: str, method: str) -> bool: + """Paths and methods the gate never refuses.""" + if method == "OPTIONS": + return True + return path in EXEMPT_PATHS + + +def header_verdict(request: Request) -> str: + """What the gate makes of this request. Five values, in two groups: + + * armed — `ok` · `missing` · `mismatch` + * not armed — `off` (no header arrived) · `off-seen` (one did) + + `off-seen` is what makes the rollout measurable rather than brave: put the + Transform Rule live while the gate is still off, then ask each path in turn + — the `api.` host, the apex `/api/*` Worker, the site's nginx, the raw + `run.app` — and only arm the gate once every path that must keep working + answers `off-seen`. Collapsing that into a bare `off` would make the switch + a leap. + + It earned its keep on the sibling repo's first run: the apex Worker + answered `off` with the rule already live, because a Worker subrequest to a + host in the same zone skips that zone's Transform Rules. Arming the gate + then would have taken the whole admin route down. + + It reports the verdict, never the value, and tells a caller on `run.app` + only what it already knows about its own request. + """ + presented = request.headers.get(ORIGIN_SECRET_HEADER) + if not gate_is_armed(): + return "off-seen" if presented else "off" + if not presented: + return "missing" + return "ok" if secret_matches(presented, settings.origin_secret) else "mismatch" + + +class OriginSecretMiddleware: + """Require the edge's shared header on every request that is not exempt. + + A raw ASGI middleware rather than a `BaseHTTPMiddleware`: a refused request + must cost as little as possible, and this way it never allocates a request + or response object beyond the refusal itself. + + Placed (`api/main.py`) INSIDE `CORSMiddleware`, so a 403 still carries the + CORS headers a browser needs to read it as a 403 rather than as an opaque + network error, and OUTSIDE the analytics middleware, so a refused request + can never fire an outbound Plausible event. That second one is not + hypothetical: `track_asset_fetch` fires per request for anything with a + crawler user agent, and a caller on the direct URL could otherwise turn + each of its own refusals into one. + """ + + def __init__(self, app): + self.app = app + + async def __call__(self, scope, receive, send): + if scope["type"] != "http" or not gate_is_armed() or is_exempt(scope["path"], scope["method"]): + await self.app(scope, receive, send) + return + presented = Request(scope).headers.get(ORIGIN_SECRET_HEADER) + if secret_matches(presented, settings.origin_secret): + await self.app(scope, receive, send) + return + response = JSONResponse( + {"detail": "this API is reached through https://api.anyplot.ai"}, + status_code=403, + headers={"Cache-Control": "private, no-store"}, + ) + await response(scope, receive, send) diff --git a/api/routers/debug.py b/api/routers/debug.py index 6fe9d0522b4..23aa37bccfd 100644 --- a/api/routers/debug.py +++ b/api/routers/debug.py @@ -2,7 +2,6 @@ from __future__ import annotations -import secrets import time from collections import Counter from datetime import datetime, timedelta, timezone @@ -17,6 +16,7 @@ from api.cache import clear_cache, get_cache_stats from api.dependencies import require_db from api.exceptions import raise_validation_error +from api.secret_compare import secret_matches from core.config import settings from core.constants import LIBRARY_NAMES, SUPPORTED_LIBRARIES from core.database import FEEDBACK_REACTIONS, FEEDBACK_STATUSES, FeedbackRepository, SpecRepository @@ -76,6 +76,14 @@ def require_admin( local dev, and break-glass access via the Cloud Run direct URL (which bypasses Cloudflare). + **Break-glass changed shape with the origin gate.** While `ORIGIN_SECRET` + is set on the service, a call over the direct `run.app` URL needs BOTH + headers — `X-Admin-Token` and `X-Origin-Secret` — because + `api/origin_gate.py` answers first and asks which door you came in at, not + who you are. Both values are readable from Secret Manager by whoever is + holding the glass. The admin token alone gets a 403, not a 401, and the + difference is the only hint. (`docs/reference/api.md` § Origin gate.) + Without `settings.admin_token` configured the token path is disabled (503), so a misconfigured prod deploy without Cloudflare Access still fails closed. """ @@ -91,7 +99,9 @@ def require_admin( expected = settings.admin_token if not expected: raise HTTPException(status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Debug endpoints not configured") - if not secrets.compare_digest(x_admin_token or "", expected): + # Constant-time, and byte-wise: a non-ASCII header would make + # `secrets.compare_digest` raise on two `str` (api/secret_compare.py). + if not secret_matches(x_admin_token, expected): raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid admin token") @@ -487,8 +497,11 @@ async def invalidate_cache(x_cache_token: str | None = Header(default=None)) -> expected = settings.cache_invalidate_token if not expected: raise HTTPException(status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Cache invalidation not configured") - # Constant-time compare to avoid byte-by-byte token recovery via timing. - if not secrets.compare_digest(x_cache_token or "", expected): + # Constant-time compare to avoid byte-by-byte token recovery via timing — + # byte-wise, because this endpoint is exempt from the origin gate and is + # therefore reachable on the direct `run.app` URL, where a non-ASCII header + # would otherwise turn a cheap 401 into a logged 500 (api/secret_compare.py). + if not secret_matches(x_cache_token, expected): raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid cache token") stats_before = get_cache_stats() diff --git a/api/routers/health.py b/api/routers/health.py index 1866a0f968b..eb4dddfc3a2 100644 --- a/api/routers/health.py +++ b/api/routers/health.py @@ -1,8 +1,9 @@ """Health and info endpoints.""" -from fastapi import APIRouter +from fastapi import APIRouter, Request from fastapi.responses import JSONResponse +from api.origin_gate import header_verdict from api.version import APP_VERSION @@ -16,10 +17,28 @@ async def root(): @router.get("/health") -async def health_check(): - """Health check endpoint for Cloud Run.""" +async def health_check(request: Request): + """Health check endpoint for Cloud Run — and the one place the origin gate + can be observed. + + `origin_gate` reports what `api/origin_gate.py` makes of THIS request — + `off` · `off-seen` · `ok` · `missing` · `mismatch`. `/health` is exempt from + the gate, so the answer comes back on every route into the service: the + `api.` host, the apex `/api/*` Worker, the site's nginx, the raw `run.app`. + That is what makes the rollout measurable instead of a leap — with the + Transform Rule already stamping but the gate still off, every path that must + keep working has to answer `off-seen` before the switch is thrown. It + reports the verdict, never the value, and tells a caller nothing about its + own request it did not already know. + """ return JSONResponse( - content={"status": "healthy", "service": "anyplot-api", "version": APP_VERSION}, status_code=200 + content={ + "status": "healthy", + "service": "anyplot-api", + "version": APP_VERSION, + "origin_gate": header_verdict(request), + }, + status_code=200, ) diff --git a/api/secret_compare.py b/api/secret_compare.py new file mode 100644 index 00000000000..26344bee6bc --- /dev/null +++ b/api/secret_compare.py @@ -0,0 +1,35 @@ +"""One constant-time comparison for every shared secret that arrives in a header. + +`secrets.compare_digest` accepts two `str` only while both are ASCII; give it a +character above U+007F and it raises `TypeError`. That matters here because a +header value reaches the application latin-1-decoded straight off the wire, so a +caller can put one byte >= 0x80 into `X-Origin-Secret`, `X-Admin-Token` or +`X-Cache-Token` and turn a cheap 401 or 403 into an unhandled 500 — a way to +make a rejection expensive, available to anyone, and logged as an error every +time (Copilot review). + +Encoding both sides first removes the restriction and the whole class of +problem, including the mirror case of a secret that legitimately contains +non-ASCII characters. It lives in one module because the same header pattern +appears at three call sites, and a comparator that is correct in two of them is +the sort of thing nobody notices. + +`presented` is deliberately allowed to be `None` or empty: an absent header is +not a value to compare, it is simply no credential, and it must not be reported +as a coincidental match against an unset secret. +""" + +from __future__ import annotations + +import secrets + + +def secret_matches(presented: str | None, expected: str | None) -> bool: + """Whether the caller presented exactly `expected`, in constant time. + + False whenever either side is missing, so an unconfigured secret can never + be satisfied by an absent header. + """ + if not presented or not expected: + return False + return secrets.compare_digest(presented.encode("utf-8"), expected.encode("utf-8")) diff --git a/core/config.py b/core/config.py index d70f0d97d0b..3e9d5c50f4d 100644 --- a/core/config.py +++ b/core/config.py @@ -167,6 +167,51 @@ class Settings(BaseSettings): """Cloudflare Access Application AUD tag (UUID from the Zero Trust dashboard). Validated as the JWT `aud` claim.""" + origin_secret: str | None = None + """Shared secret a Cloudflare Transform Rule stamps as `X-Origin-Secret` on + every request it proxies for `api.anyplot.ai`. `api/origin_gate.py` refuses + anything without it, which closes the direct `*.run.app` door that bypasses + the bot challenge, the WAF and the edge cache. + + UNSET MEANS OFF, and that is the rollback: remove the variable from the + Cloud Run service, promote the resulting revision, and the gate is gone. + Local dev and the test suite never set it. Exactly two paths stay exempt + when it is set — `/health` and `/debug/cache/invalidate`; the prerendered + `/seo-proxy/…` pages are NOT among them, since the site's nginx fetches + them through the edge and so carries the header. See `api/origin_gate.py` + for why each of the two has to be, and why the third is not.""" + + @field_validator( + "database_url", + "cache_invalidate_token", + "admin_token", + "cf_access_team_domain", + "cf_access_aud", + "origin_secret", + mode="after", + ) + @classmethod + def _strip_secret(cls, value: str | None) -> str | None: + """Strip whitespace from the Secret-Manager-backed values. + + Secret Manager stores whatever bytes the version was created with, and + a value piped in via `echo` carries a trailing newline. Cloud Run + injects those bytes verbatim (`--set-secrets`), so the setting would + keep the newline while an HTTP header physically cannot transport one — + the `X-Admin-Token` and `X-Cache-Token` paths would then reject every + request and no value of the header could ever fix it. `origin_secret` + is compared against a header for exactly the same reason, and it fails + closed for the whole API rather than for one endpoint. + + Strip at the source rather than at each use site; whitespace is + meaningless in all of these values. An all-whitespace value becomes + None, which is the same as unset — for `origin_secret` that means the + gate stays off rather than locking everyone out with a secret nobody + can present.""" + if value is None: + return None + return value.strip() or None + admin_allowed_emails: Annotated[list[str], NoDecode] = [] """Email addresses allowed to authenticate via Cloudflare Access for /debug/* endpoints. Defaults to an empty list — must be set explicitly diff --git a/docs/development.md b/docs/development.md index 70825e32545..680e686159a 100644 --- a/docs/development.md +++ b/docs/development.md @@ -149,6 +149,7 @@ Copy `.env.example` and configure: | `GCS_BUCKET` | No | GCS bucket for images (default: anyplot-images) | | `GOOGLE_APPLICATION_CREDENTIALS` | No | Path to service account JSON | | `ENVIRONMENT` | No | `development` or `production` | +| `ORIGIN_SECRET` | No | Production only — leave unset locally and in tests. Arms the [origin gate](reference/api.md#origin-gate), which refuses every request that did not come through Cloudflare | --- diff --git a/docs/reference/api.md b/docs/reference/api.md index e101f74e9f8..fee43c52e65 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -250,7 +250,7 @@ The anyplot API is a **FastAPI-based REST API** serving plot data to the fronten ```json { "message": "Welcome to anyplot API", - "version": "0.2.0", + "version": "3.2.0", "docs": "/docs", "health": "/health" } @@ -260,17 +260,26 @@ The anyplot API is a **FastAPI-based REST API** serving plot data to the fronten #### GET `/health` -**Purpose**: Health check for Cloud Run +**Purpose**: Health check for Cloud Run, and the one place the +[origin gate](#origin-gate) can be observed **Response**: ```json { "status": "healthy", "service": "anyplot-api", - "version": "0.2.0" + "version": "3.2.0", + "origin_gate": "off" } ``` +`version` is the installed package's version (`api/version.py`), so it moves +with each release rather than staying at the value written here. + +`origin_gate` reports what the gate makes of *this* request — never the secret +itself. `/health` is exempt from the gate, so every route into the service can +be asked. See [Origin gate](#origin-gate) for the five values. + --- ## Insights endpoints @@ -449,6 +458,192 @@ Applied to: --- +## Origin gate + +The API runs on Cloud Run with `ingress=all`, so it answers on two addresses: +`https://api.anyplot.ai`, which Cloudflare proxies, and the raw `*.run.app` +URL, which it does not. Everything the edge enforces — the bot challenge, the +WAF, the cache that makes the `max-age=300` reads free — is one URL away from +being bypassed. + +A Cloudflare Transform Rule stamps `X-Origin-Secret` onto every request it +proxies for `api.anyplot.ai`, and `api/origin_gate.py` refuses anything without +it with `403`. It is not authentication: it says "you came through the front +door", nothing about who you are — `require_admin` still decides what a caller +may do on `/debug/*`. + +**Unset means off.** The gate is dormant unless `ORIGIN_SECRET` is set on the +service, which is what makes local development, the test suite and the rollback +work: remove the variable from the service, promote the resulting revision, and +the gate is gone. + +**Arming and rolling back** are the same procedure with one flag changed. Several +things make it more than two commands, and each of them has bitten a comparable +rollout somewhere. The whole block runs in a fail-fast subshell: half of these +commands feed the next one, so continuing after a failed lookup would mutate the +service from an empty variable and still print a plausible-looking health line +at the end. + +```bash +( +set -euo pipefail +SERVICE=anyplot-api +LOC="--project=anyplot --region=europe-west4" + +# 0. Do not race the deploy pipeline. A build that ALREADY deployed its +# candidate promotes it at the end — and that revision was cloned from the +# pre-arm template, so the promote silently undoes the arm (its own smoke +# accepts `off`, by design). A build that starts AFTER this block inherits +# the binding, because the deploy is additive (`--update-secrets`). So the +# dangerous window is exactly "a build already in flight". +gcloud builds list --project=anyplot --region=europe-west4 --ongoing --format="value(id)" | grep -q . && { + echo "a Cloud Build is in flight; wait for it to finish (or fail) before arming." + exit 1 +} + +# 1. Build the new revision from the image that is SERVING, not from whatever +# is latest. `services update` clones the service's latest template, and the +# deploy pipeline deliberately leaves each build's candidate there — smoked +# and unpromoted on a good build, and still there on a bad one, because a +# failed smoke skips the promote and nothing cleans it up. Pinning the image +# means the arm/disarm revision serves what is serving now, whatever state +# the pipeline is in; naming the revision alone would not have. +read -r SERVING LATEST <<<"$(gcloud run services describe "$SERVICE" $LOC --format=json \ + | python3 -c "import json,sys; d=json.load(sys.stdin); \ + t=[x for x in d['status']['traffic'] if x.get('percent')==100]; \ + print(t[0]['revisionName'], d['status']['latestReadyRevisionName'])")" +IMAGE=$(gcloud run revisions describe "$SERVING" $LOC --format="value(spec.containers[0].image)") +test -n "$SERVING" && test -n "$IMAGE" || { echo "could not resolve the serving revision or its image"; exit 1; } + +# A mismatch is a WARNING, never a stop: it is normal right after a deploy, and +# it is permanent after a failed smoke — a hard refusal here would make the +# emergency rollback unavailable exactly when it is needed. With the image +# pinned, the remaining risk is only that the latest template carries a config +# change nobody promoted; check the revision afterwards if this fires. +test "$SERVING" = "$LATEST" || echo "note: latest ($LATEST) is not serving ($SERVING) — image pinned to the serving one" + +# 2. Pin the secret to a NUMBER, never `:latest`. Cloud Run resolves a +# secret-backed variable when each instance starts, so with `:latest` a new +# secret version reaches new instances while older ones keep the old value — +# and since the edge stamps exactly one value, the difference shows up as +# intermittent 403s inside a single revision. +VERSION=$(gcloud secrets versions list ORIGIN_SECRET --project=anyplot \ + --filter="state=ENABLED" --sort-by=~createTime --limit=1 --format="value(name)") +test -n "$VERSION" || { echo "no ENABLED version of ORIGIN_SECRET"; exit 1; } + +# 3. Update, then promote BY NAME. The service pins traffic to a named +# revision, so the update alone serves nothing; and `--to-latest` here would +# hand traffic to whatever the pipeline last built. +SUFFIX="arm-$(date -u +%Y%m%d%H%M)" +gcloud run services update "$SERVICE" $LOC --image="$IMAGE" \ + --update-secrets="ORIGIN_SECRET=ORIGIN_SECRET:$VERSION" --revision-suffix="$SUFFIX" +gcloud run services update-traffic "$SERVICE" $LOC --to-revisions="$SERVICE-$SUFFIX=100" + +# 4. Confirm the result. Not optional: step 0 only narrows the race, and this +# is what catches a build that promoted over the arm anyway — the verdict +# would read `off` on a path that carries the header. If it does, that +# promote reverted the arm; re-run the whole block. +curl -s "https://api.anyplot.ai/health" +gcloud run services describe "$SERVICE" $LOC --format="value(status.traffic)" +) +``` + +**Rolling back** is its own block, not the one above with a flag swapped. It has +to run in the worst state the service can be in — which includes the secret +having been disabled or deleted during the incident, so it must not look the +secret up at all. Nothing here depends on anything but the currently serving +revision: + +```bash +( +set -euo pipefail +SERVICE=anyplot-api +LOC="--project=anyplot --region=europe-west4" + +# No in-flight check and no secret lookup: when the gate is the outage, waiting +# for a build is the wrong trade, and step 2 above would abort here on a +# disabled version — leaving the gate armed at the moment it must come off. +SERVING=$(gcloud run services describe "$SERVICE" $LOC --format=json \ + | python3 -c "import json,sys; d=json.load(sys.stdin); \ + print(next(x['revisionName'] for x in d['status']['traffic'] if x.get('percent')==100))") +IMAGE=$(gcloud run revisions describe "$SERVING" $LOC --format="value(spec.containers[0].image)") +test -n "$IMAGE" || { echo "could not resolve the serving image"; exit 1; } + +SUFFIX="disarm-$(date -u +%Y%m%d%H%M)" +gcloud run services update "$SERVICE" $LOC --image="$IMAGE" \ + --remove-secrets=ORIGIN_SECRET --revision-suffix="$SUFFIX" +gcloud run services update-traffic "$SERVICE" $LOC --to-revisions="$SERVICE-$SUFFIX=100" + +curl -s "https://api.anyplot.ai/health" # expect "off" or "off-seen" +) +``` + +Removing the Worker's binding is **not** a rollback — while the service is armed +that takes the apex route down rather than freeing it. Roll back here first. + +**Rotating the secret** means changing two sides that must agree, and the gate +accepts exactly one value — so there is no overlap window. Roll back first, +rotate the Secret Manager version, the Transform Rule and the Worker binding, +then arm again on the new version number. The gate is off in between, which is +the documented safe state; `/health` shows `off-seen` throughout, and `ok` when +the new value is live on both sides. + +**Exempt paths** — exact matches, no prefixes, and only these two: + +| Path | Why | +|---|---| +| `/health` | the deploy smoke probes the candidate revision on its `run.app` tag URL, which never passes the edge | +| `/debug/cache/invalidate` | `sync-postgres.yml` posts here from a GitHub runner over the direct URL, because Cloudflare's bot challenge answers an unauthenticated curl POST with a 403 HTML page. The endpoint has its own token (`CACHE_INVALIDATE_TOKEN`, constant-time compared, 503 when unconfigured) | + +`OPTIONS` is exempt too — a browser cannot attach a custom header to a CORS +preflight, so a gate that refused one would break every cross-origin call +instead of protecting anything. + +`/seo-proxy/…` is **not** exempt. The site's nginx fetches the prerendered pages +over `https://api.anyplot.ai`, so that path carries the header — while an +exemption would leave the API's most expensive reads open on the direct URL: a +cache miss or an unknown id queries the repositories, and a crawler user agent +schedules an outbound Plausible event per request. Validate the crawler path end +to end while the gate is still off (below), not with an exemption: + +```bash +curl -s -A 'Mozilla/5.0 (compatible; Googlebot/2.1)' https://anyplot.ai/scatter-basic | head -5 +``` + +**What the gate does not close.** It protects the API service's door. The app +service (`anyplot-app`) also stands with `ingress=all`, and its nginx relays a +crawler user agent through `@seo_proxy` to `https://api.anyplot.ai`, where the +edge stamps the header legitimately — so a caller who sends a crawler user +agent to the app's raw `*.run.app` URL still reaches the prerendered render and +its DB queries. That is a second door on a second service rather than a hole in +this one: the request the API sees really did come through the edge. Closing it +means gating the app service or refusing to proxy for `run.app` hosts in +`app/nginx.conf`, and `bot-serving-check.yml` probes that exact flow on the app +origin every night — so it is a separate change with its own blast radius. + +**Observing it.** `GET /health` reports `origin_gate` for the request it was +asked with, never the value: + +| Value | Meaning | +|---|---| +| `off` | not armed, no header arrived | +| `off-seen` | not armed, the header arrived — the state to be in before arming | +| `ok` | armed, header matches | +| `missing` | armed, no header — this path would now be dead | +| `mismatch` | armed, wrong value (a half-applied rotation) | + +That is what makes the rollout measurable: put the Transform Rule live while +the gate is still off, ask every route into the service (`api.anyplot.ai`, the +apex `anyplot.ai/api/*` Worker, the site's nginx, the raw `run.app`), and only +arm it once every path that must keep working answers `off-seen`. + +The apex Worker stamps the header itself rather than getting it from the rule, +because a Worker subrequest to a host in the same zone bypasses that zone's +Transform Rules. Its source and the measuring procedure live in +[`infra/cloudflare/`](../../infra/cloudflare/README.md). + +--- + ## CORS configuration **Allowed Origins**: @@ -457,6 +652,10 @@ Applied to: **Allowed Methods**: All +The origin gate sits directly inside `CORSMiddleware`, so a `403` from it still +carries the CORS headers a browser needs to read it as a 403 rather than as an +opaque network error. + --- ## GZip compression diff --git a/docs/reference/repository.md b/docs/reference/repository.md index b8527983c94..99ec3025464 100644 --- a/docs/reference/repository.md +++ b/docs/reference/repository.md @@ -133,6 +133,9 @@ anyplot/ ├── alembic/ # Database migrations │ └── versions/ │ +├── infra/ # Infrastructure that lives outside the code +│ └── cloudflare/ # Source for the apex Worker + its rollout notes +│ ├── scripts/ # One-time and manual scripts │ ├── evaluate-plot.py # Manual plot evaluation │ ├── regenerate-thumbnails.py # Image processing diff --git a/infra/cloudflare/README.md b/infra/cloudflare/README.md new file mode 100644 index 00000000000..d5db974690f --- /dev/null +++ b/infra/cloudflare/README.md @@ -0,0 +1,188 @@ +# Cloudflare: the apex Worker in front of the API + +This directory is the source for Cloudflare configuration that would otherwise +exist only in the dashboard. It was created with the origin gate +(`api/origin_gate.py`), because until then the Worker went unmentioned in the +repository and nobody without dashboard access could see what it does. + +## What is here + +| File | Role | +|---|---| +| `anyplot-api-proxy.js` | the Worker behind the route `anyplot.ai/api/*` | + +**The `.js` is the source, not a draft.** Change it here and deploy it; change +it in the dashboard and pull it back here. + +> **After this lands, the Worker needs a redeploy.** The +> `X-Origin-Secret` lines are newer than the running script, so the repository +> and the deployment are out of step until it is pushed. Until then the Worker +> stamps nothing, `anyplot.ai/api/health` answers `off`, and arming the gate +> would take that route down — which is exactly what the rollout order in the +> pull-request description prevents. + +## The Worker + +**Purpose.** `anyplot.ai/api/*` forwards to `https://api.anyplot.ai` with the +`/api` prefix stripped, so the apex can serve API paths on its own origin. The +one path it does not forward is `/api/event`: that is the Plausible analytics +endpoint (the same one `app/nginx.conf` proxies for the app service), and it +goes to Plausible untouched. + +**Why it stamps the origin secret itself.** This is the finding the directory +exists for: + +> A Worker subrequest to a host in the **same zone** bypasses that zone's +> Transform Rules. + +The Transform Rule stamps `X-Origin-Secret` on everything Cloudflare forwards +for `api.anyplot.ai` — but not on a `fetch()` issued from inside a Worker. +Without those lines in the Worker, arming the gate takes `anyplot.ai/api/*` +down. The sibling repository kurrentschrift measured this on `/api/health` +during its own rollout: first `off`, then `off-seen` once the Worker had its +secret binding, then `ok` after arming. + +**Why it strips the header first.** `headers` is cloned from the incoming +request, so without `headers.delete('X-Origin-Secret')` a caller could supply +that header itself and have it forwarded whenever the binding is unset. Two +consequences, one of them subtle: the documented "unset binding stamps nothing" +would be false, and an unarmed `/health` probe would report a spurious +`off-seen` — corrupting the one measurement the rollout hangs on. + +**Why the binding is guarded by `if (env.ORIGIN_SECRET)`.** A missing binding +stamps nothing rather than sending an empty header, which makes the Worker +harmless while the gate is not yet armed. It is **not a rollback**: as long as +the Cloud Run service is armed, every apex API request without the header gets +a 403, so removing the binding takes that route down rather than freeing it. +Rolling back always happens on the API side — remove `ORIGIN_SECRET` from the +service **and promote the resulting revision by name** (the two commands are in +[`docs/reference/api.md`](../../docs/reference/api.md#origin-gate); never +`--to-latest`, which can promote a concurrent build's revision instead). The +ordering holds in both directions: the Worker starts stamping before the gate is +armed, and stops only after it is disarmed. + +## Settings (dashboard) + +| | | +|---|---| +| Script name | `anyplot-api-proxy` | +| Route | `anyplot.ai/api/*` (zone `anyplot.ai`) | +| Binding | `secret_text` **`ORIGIN_SECRET`** — value = Secret Manager `ORIGIN_SECRET`, the same one the Cloud Run service gets | +| `compatibility_date` | `2026-04-29` | +| Module type | ES module (`export default { fetch }`) | + +## Deploying + +**Dashboard** (the usual way): Workers & Pages → `anyplot-api-proxy` → Edit +code → paste the contents of `anyplot-api-proxy.js` → Deploy. The secret +binding is created once under Settings → Variables as a *Secret*; it survives +later deploys. + +**API** (when it has to be scriptable) — a multipart request of metadata plus +module. Neither secret is ever typed. Both are read at execution time and each +reaches its command through a channel other processes cannot read: the origin +secret on `python3`'s **stdin**, the Cloudflare token on a `--config` file +descriptor. Neither is exported, so neither appears in a shell history, a file, +an argument vector, or a `/proc//environ`. + +It runs in a fail-fast subshell for a reason: a script `PUT` **replaces** the +bindings, so a failed or empty secret read would deploy an empty binding, the +Worker would stop stamping, and the armed apex route would go down — the very +outage this gate exists to avoid. Every step is checked before the next one +runs. + +```bash +( + set -euo pipefail + + # Command substitution strips the trailing newline gcloud may print — the + # same newline that would otherwise make the value unusable in a header. + ORIGIN_SECRET=$(gcloud secrets versions access latest \ + --secret=ORIGIN_SECRET --project=anyplot) + [ -n "$ORIGIN_SECRET" ] || { echo "empty ORIGIN_SECRET — refusing to deploy"; exit 1; } + + # The secret goes in on STDIN, not in the child's environment: a + # `VAR=value python3 …` prefix puts it in `/proc//environ`, which any + # same-UID process can read for the life of the call (Copilot review). The + # shell variable itself is never exported. + printf '%s' "$ORIGIN_SECRET" | python3 -c ' +import json, sys +json.dump({ + "main_module": "worker.js", + "compatibility_date": "2026-04-29", + "bindings": [ + {"type": "secret_text", "name": "ORIGIN_SECRET", "text": sys.stdin.read()}, + ], +}, sys.stdout)' | curl --fail-with-body -sS -X PUT \ + "https://api.cloudflare.com/client/v4/accounts/{account}/workers/scripts/anyplot-api-proxy" \ + --config <(printf 'header = "Authorization: Bearer %s"\n' "$CF_API_TOKEN") \ + -F 'metadata=<-;type=application/json' \ + -F 'worker.js=@infra/cloudflare/anyplot-api-proxy.js;type=application/javascript+module' +) +``` + +`pipefail` so a failing `python3` cannot be masked by a succeeding `curl`; +`--fail-with-body` so an HTTP error is a non-zero exit and still prints +Cloudflare's JSON reason, which plain `-f` swallows. + +**The API token goes in through a file descriptor, not `-H`.** `curl`'s argument +vector is world-readable in `/proc` for as long as the request runs, so +`-H "Authorization: Bearer $CF_API_TOKEN"` would publish the credential that +authorises replacing this Worker to every process on the machine — the same +exposure the `ORIGIN_SECRET` handling above avoids, and the more dangerous of +the two. `--config <(…)` passes it on `/dev/fd/N` instead, and `printf` is a +shell builtin, so the token never reaches any argv at all (Copilot review). It +needs `bash` or `zsh`; under a shell without process substitution, write the +config line to a `600` temporary file and pass that path. + +Then measure before trusting it — and read the answer against where the rollout +currently stands, because the same command means two different things: + +```bash +curl -s https://anyplot.ai/api/health +# "off-seen" gate not yet armed, and the Worker IS stamping — the correct +# result right after this deploy, and the state to reach before +# arming. Not a failed deployment. +# "off" gate not armed and NOTHING was stamped — the binding is +# missing. Arming now would take this route down. +# "ok" gate armed and this Worker's value matches. Only reachable +# after the API side is armed. +``` + +Because a script `PUT` replaces the bindings wholesale, omitting one removes it +— the same trap that made the Cloud Run side use `--update-secrets` rather than +`--set-secrets` (`api/cloudbuild.yaml`). So either send every binding or deploy +from the dashboard. + +## Measuring: did the header take this path? + +`/health` is exempt from the origin gate and reports its verdict for the request +it was asked with — **never the value**: + +| `origin_gate` | meaning | +|---|---| +| `off` | gate not armed, **no** header arrived | +| `off-seen` | gate not armed, the header arrived — the state to be in before arming | +| `ok` | gate armed, header matches | +| `missing` | gate armed, no header — this path would now be dead | +| `mismatch` | gate armed, wrong value (half-applied rotation) | + +Every route into the service, in the order the rollout asks them: + +```bash +curl -s https://api.anyplot.ai/health # the public path (Transform Rule) +curl -s https://anyplot.ai/api/health # THIS Worker +curl -s https:///health # must stay "off"/"missing": the closed door +``` + +The site's nginx is the fourth path and cannot be asked directly — it reaches +`https://api.anyplot.ai` for `@seo_proxy`, `/llms-full.txt` and `/sitemap.xml`, +so it rides on the first line's verdict. Probe it end to end instead: + +```bash +curl -s -A 'Mozilla/5.0 (compatible; Googlebot/2.1)' https://anyplot.ai/scatter-basic | head -5 +curl -sI https://anyplot.ai/llms-full.txt +``` + +After any change to the Worker, the Transform Rule or the secret: measure +first, arm second. diff --git a/infra/cloudflare/anyplot-api-proxy.js b/infra/cloudflare/anyplot-api-proxy.js new file mode 100644 index 00000000000..2c299e5699d --- /dev/null +++ b/infra/cloudflare/anyplot-api-proxy.js @@ -0,0 +1,32 @@ +export default { + async fetch(request, env) { + const url = new URL(request.url); + // The Plausible proxy shares this route. `/api/event` is the analytics + // endpoint the site's nginx also proxies (app/nginx.conf); it is not an + // anyplot API path and must reach Plausible untouched. + if (url.pathname === '/api/event') { + return fetch(request); + } + const targetPath = url.pathname.replace(/^\/api/, ''); + const targetUrl = `https://api.anyplot.ai${targetPath}${url.search}`; + const headers = new Headers(request.headers); + headers.delete('host'); + // The API's origin gate (api/origin_gate.py) admits only requests that + // carry the secret Cloudflare stamps at the edge. A Worker subrequest to + // the same zone skips the zone's Transform Rules, so the Worker stamps the + // header itself from its secret binding (unset binding = nothing stamped). + // Delete first: the headers are cloned from the incoming request, so + // without this a caller could supply its own X-Origin-Secret and have it + // forwarded whenever the binding is unset — which would also make an + // unarmed /health probe report a false `off-seen` and corrupt the one + // measurement the rollout depends on. + headers.delete('X-Origin-Secret'); + if (env.ORIGIN_SECRET) headers.set('X-Origin-Secret', env.ORIGIN_SECRET); + return fetch(targetUrl, { + method: request.method, + headers, + body: request.method !== 'GET' && request.method !== 'HEAD' ? request.body : null, + redirect: 'manual', + }); + }, +}; diff --git a/tests/unit/api/test_origin_gate.py b/tests/unit/api/test_origin_gate.py new file mode 100644 index 00000000000..80e10544d05 --- /dev/null +++ b/tests/unit/api/test_origin_gate.py @@ -0,0 +1,347 @@ +"""The shared-secret origin gate (`api/origin_gate.py`). + +Both Cloud Run services stand with `ingress=all`, so the raw `*.run.app` +address answers without ever touching Cloudflare — and every edge measure (the +bot challenge, the WAF, the cache that makes the `max-age=300` reads free) is +one URL away from being bypassed. A Cloudflare Transform Rule stamps +`X-Origin-Secret` onto everything it proxies for `api.anyplot.ai`; this suite +pins that the API requires it, that it is DORMANT until the secret is +configured (which is the rollback), and that the paths which must never be +locked out are not. +""" + +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +import api.main as api_main +from api.main import app +from api.origin_gate import ORIGIN_SECRET_HEADER, is_exempt +from api.secret_compare import secret_matches +from core.config import settings + + +SECRET = "s3cret-from-the-edge" +EDGE = {ORIGIN_SECRET_HEADER: SECRET} + +# A path the gate has an opinion about and that needs neither the database nor +# a seeded row to answer: /hello is a plain echo route, so a non-403 here means +# the request got through rather than that a fixture happened to exist. +OPEN_PATH = "/hello/gate" + + +@pytest.fixture +def client() -> TestClient: + return TestClient(app) + + +@pytest.fixture +def armed(monkeypatch): + """Turn the gate on for one test, the way Cloud Run's env does.""" + monkeypatch.setattr(settings, "origin_secret", SECRET) + + +class TestTheGateIsOffUntilASecretIsConfigured: + """The default everywhere: local dev, the test suite, and production until + step (c) of the rollout. Unsetting the variable is the rollback, and it + must need no deploy — so `origin_secret` is read per request.""" + + def test_no_secret_configured_by_default(self): + assert settings.origin_secret is None + + def test_requests_pass_without_the_header(self, client: TestClient): + assert client.get(OPEN_PATH).status_code == 200 + + def test_requests_pass_even_with_a_wrong_header(self, client: TestClient): + """What the window between the Transform Rule going live and the secret + being set looks like from the service's side.""" + assert client.get(OPEN_PATH, headers={ORIGIN_SECRET_HEADER: "anything"}).status_code == 200 + + def test_health_says_off(self, client: TestClient): + assert client.get("/health").json()["origin_gate"] == "off" + + def test_health_says_off_seen_when_a_header_arrived(self, client: TestClient): + """The verdict step (b) of the rollout is measured with: the rule goes + live first, and every path that must keep working has to answer + `off-seen` before the switch is thrown. A bare `off` would make arming + a leap — above all for `anyplot.ai/api/*`, which reaches this service + through a Cloudflare Worker whose subrequest may or may not carry the + stamp.""" + res = client.get("/health", headers={ORIGIN_SECRET_HEADER: "anything"}) + assert res.json()["origin_gate"] == "off-seen" + + +class TestTheArmedGate: + def test_the_edge_header_passes(self, client: TestClient, armed): + assert client.get(OPEN_PATH, headers=EDGE).status_code == 200 + + @pytest.mark.parametrize( + "headers", + [ + {}, + {ORIGIN_SECRET_HEADER: ""}, + {ORIGIN_SECRET_HEADER: "wrong"}, + {ORIGIN_SECRET_HEADER: SECRET[:-1]}, + {ORIGIN_SECRET_HEADER: SECRET + "x"}, + ], + ids=["absent", "empty", "wrong", "truncated", "extended"], + ) + def test_anything_else_is_refused(self, client: TestClient, armed, headers): + res = client.get(OPEN_PATH, headers=headers) + assert res.status_code == 403 + assert res.headers["cache-control"] == "private, no-store" + + def test_the_refusal_names_the_front_door_and_nothing_else(self, client: TestClient, armed): + """Never the secret, never its length, never whether the caller was + close.""" + detail = client.get(OPEN_PATH).json()["detail"] + assert "api.anyplot.ai" in detail + assert SECRET not in detail + + @pytest.mark.parametrize("method", ["get", "post", "put", "patch", "delete", "head"]) + def test_every_method_is_covered(self, client: TestClient, armed, method): + """A gate that only saw GET would be one verb away from useless.""" + assert getattr(client, method)("/debug/status").status_code == 403 + + def test_an_unknown_route_is_refused_before_it_404s(self, client: TestClient, armed): + assert client.get("/no-such-route").status_code == 403 + + def test_it_answers_before_the_admin_credential_is_looked_at(self, client: TestClient, armed): + """The gate asks which door you came in at, not who you are — and it + answers first. Break-glass over the direct URL needs BOTH headers.""" + assert client.get("/debug/status", headers={"X-Admin-Token": "whatever"}).status_code == 403 + + +class TestTheExemptPaths: + """Two paths, no prefixes. `/health` is how the deploy smoke reaches the + candidate revision on its `run.app` tag URL, which by definition never + passes the edge, so gating it would make every deploy fail closed. + `/debug/cache/invalidate` is the one legitimate caller with no front door — + `sync-postgres.yml` posts to the direct URL because Cloudflare's bot + challenge answers an unauthenticated curl POST with a 403 HTML page; that + endpoint carries its own token.""" + + def test_the_gate_is_really_armed_for_this_test(self, client: TestClient, armed): + assert client.get(OPEN_PATH).status_code == 403 + + def test_health_is_never_gated(self, client: TestClient, armed): + assert client.get("/health").status_code == 200 + + def test_the_cache_flush_is_never_gated(self, client: TestClient, armed): + """503 is the answer with no CACHE_INVALIDATE_TOKEN configured — its + own fail-closed gate, reached rather than pre-empted.""" + assert client.post("/debug/cache/invalidate").status_code == 503 + + def test_the_prerendered_pages_ARE_gated(self, client: TestClient, armed): + """Deliberately not exempt (Copilot review): the site's nginx fetches + them over `https://api.anyplot.ai`, so they carry the header, while an + exemption would leave the API's most expensive reads open on the direct + URL — a cache miss or an unknown id queries the repositories, and a + crawler user agent schedules an outbound Plausible event per request.""" + assert client.get("/seo-proxy/legal").status_code == 403 + + @pytest.mark.parametrize( + ("path", "exempt"), + [ + ("/health", True), + ("/debug/cache/invalidate", True), + ("/seo-proxy", False), + ("/seo-proxy/", False), + ("/seo-proxy/specs", False), + ("/healthz", False), + ("/debug/cache", False), + ("/debug/status", False), + ("/specs", False), + ], + ) + def test_the_exemption_list_is_exactly_these_two_paths(self, path, exempt): + assert is_exempt(path, "GET") is exempt + + +class TestCORS: + def test_a_preflight_is_never_refused(self, client: TestClient, armed): + """A browser cannot attach a custom header to a preflight, so a gate + that refused one would break every cross-origin call on the site + instead of protecting anything. `CORSMiddleware` sits outside the gate + and answers the preflight before it gets there, so this holds twice + over — the explicit exemption is what keeps it true if the stack order + ever moves.""" + assert is_exempt(OPEN_PATH, "OPTIONS") + + preflight = client.options( + OPEN_PATH, + headers={ + "Origin": "https://anyplot.ai", + "Access-Control-Request-Method": "GET", + "Access-Control-Request-Headers": "x-admin-token", + }, + ) + assert preflight.status_code == 200 + assert preflight.headers["access-control-allow-origin"] == "https://anyplot.ai" + + def test_a_refused_request_still_carries_the_cors_headers(self, client: TestClient, armed): + """So the browser reports a 403 rather than an opaque network error — + which is why the middleware sits INSIDE CORSMiddleware.""" + refused = client.get(OPEN_PATH, headers={"Origin": "https://anyplot.ai"}) + assert refused.status_code == 403 + assert refused.headers["access-control-allow-origin"] == "https://anyplot.ai" + + +class TestARefusalCostsNothing: + """The gate has to sit OUTSIDE the analytics middleware. + + An asset path plus a crawler user agent makes the counter fire an outbound + Plausible request per request. If the gate were inside it, a caller on the + direct `run.app` URL could turn each of its OWN refusals into one — + unthrottled, and to a third-party endpoint. A refused request must cost + nothing at all. + """ + + CRAWLER = {"User-Agent": "Mozilla/5.0 (compatible; ClaudeBot/1.0)"} + ASSET = "/specs/scatter-basic/matplotlib/code" + + def test_a_refused_asset_read_reports_nothing(self, client: TestClient, armed): + with patch.object(api_main, "track_asset_fetch") as assets: + assert client.get(self.ASSET, headers=self.CRAWLER).status_code == 403 + assert assets.call_count == 0 + + def test_a_refused_crawler_page_reports_nothing_either(self, client: TestClient, armed): + """The prerendered pages are the ones an unthrottled caller would pick: + every request with a crawler user agent schedules an outbound event.""" + with patch.object(api_main, "track_bot_fetch") as pages: + assert client.get("/seo-proxy/legal", headers=self.CRAWLER).status_code == 403 + assert pages.call_count == 0 + + def test_with_the_header_the_crawler_page_counts(self, client: TestClient, armed): + with patch.object(api_main, "track_bot_fetch") as pages: + client.get("/seo-proxy/legal", headers=self.CRAWLER | EDGE) + assert pages.call_count == 1 + + def test_with_the_header_the_read_counts_again(self, client: TestClient, armed): + """The gate suppresses a refusal, not the measurement.""" + with patch.object(api_main, "track_asset_fetch") as assets: + client.get(self.ASSET, headers=self.CRAWLER | EDGE) + assert assets.call_count == 1 + + +class TestHealthReportsTheVerdictForTheRequestItWasAskedWith: + """What makes the rollout measurable: every route into the service can be + asked whether the header arrives, BEFORE the gate is armed.""" + + @pytest.mark.parametrize( + ("headers", "verdict"), [(EDGE, "ok"), ({}, "missing"), ({ORIGIN_SECRET_HEADER: "wrong"}, "mismatch")] + ) + def test_verdict(self, client: TestClient, armed, headers, verdict): + res = client.get("/health", headers=headers) + assert res.status_code == 200 + assert res.json()["origin_gate"] == verdict + + +class TestANonAsciiHeaderIsRefused: + """`secrets.compare_digest` raises TypeError when either `str` holds a + non-ASCII character, and a header value reaches the middleware + latin-1-decoded straight from the wire. Comparing as `str` would therefore + hand any unauthenticated caller a one-byte way to turn every refusal into + an unhandled 500 — the gate made expensive instead of cheap (Copilot + review). The compare encodes both sides first. + """ + + # Passed as raw bytes: an HTTP client refuses to encode a non-ASCII str + # header, so bytes are the only way one reaches the middleware at all. + RAW = b"s3cret-from-the-edg\xe9" + + def test_a_non_ascii_header_is_a_403_not_a_500(self, client: TestClient, armed): + res = client.get(OPEN_PATH, headers={ORIGIN_SECRET_HEADER: self.RAW}) + assert res.status_code == 403 + + def test_health_calls_it_a_mismatch_not_a_500(self, client: TestClient, armed): + res = client.get("/health", headers={ORIGIN_SECRET_HEADER: self.RAW}) + assert res.status_code == 200 + assert res.json()["origin_gate"] == "mismatch" + + def test_the_str_comparison_this_replaced_would_have_raised(self): + """The proof that the two tests above measure something. Asserted on + the primitive rather than on a response, because the exact byte the + client puts on the wire is the transport's business — what matters is + that a non-ASCII `str` is fatal to `compare_digest` and harmless to + the encoded compare.""" + import secrets as stdlib_secrets + + seen = self.RAW.decode("latin-1") + with pytest.raises(TypeError): + stdlib_secrets.compare_digest(seen, SECRET) + + assert secret_matches(seen, seen) is True + assert secret_matches(seen, SECRET) is False + + @pytest.mark.parametrize( + ("presented", "expected"), + [(None, SECRET), ("", SECRET), (SECRET, None), (SECRET, ""), (None, None)], + ids=["no-header", "empty-header", "no-secret", "empty-secret", "neither"], + ) + def test_a_missing_side_never_matches(self, presented, expected): + """An unconfigured secret must not be satisfiable by an absent header, + which is what a bare `compare_digest("", "")` would do.""" + assert secret_matches(presented, expected) is False + + +class TestTheOtherHeaderSecretsUseTheSameComparator: + """The gate exempts `/debug/cache/invalidate` on the grounds that it has its + own lock — so that lock has to be as cheap to fail as the gate is. It used + the raw `str` comparison, and it is the one endpoint reachable on the direct + `run.app` URL, so a non-ASCII `X-Cache-Token` turned a 401 into a logged 500 + (Copilot review). `X-Admin-Token` had the same comparison.""" + + RAW = b"tok\xe9n" + + def test_a_non_ascii_cache_token_is_a_401_or_503_not_a_500(self, client: TestClient, monkeypatch): + monkeypatch.setattr(settings, "cache_invalidate_token", "the-real-token") + res = client.post("/debug/cache/invalidate", headers={"X-Cache-Token": self.RAW}) + assert res.status_code == 401 + + def test_a_non_ascii_admin_token_is_a_401_not_a_500(self, client: TestClient, monkeypatch): + monkeypatch.setattr(settings, "admin_token", "the-real-token") + monkeypatch.setattr(settings, "cf_access_team_domain", None) + res = client.get("/debug/status", headers={"X-Admin-Token": self.RAW}) + assert res.status_code == 401 + + def test_the_right_cache_token_still_works(self, client: TestClient, monkeypatch): + monkeypatch.setattr(settings, "cache_invalidate_token", "the-real-token") + res = client.post("/debug/cache/invalidate", headers={"X-Cache-Token": "the-real-token"}) + assert res.status_code == 200 + + +class TestATrailingNewlineCannotLockEveryoneOut: + """A Secret Manager version created with `echo` carries a trailing newline, + Cloud Run injects the bytes verbatim, and an HTTP header physically cannot + transport one — so every request would 403 and no value of the header could + ever fix it. The setting strips.""" + + def test_the_env_value_is_stripped(self, monkeypatch): + from core.config import Settings + + monkeypatch.setenv("ORIGIN_SECRET", f"{SECRET}\n") + assert Settings().origin_secret == SECRET + + def test_a_stripped_secret_matches_the_header(self, client: TestClient, monkeypatch): + from core.config import Settings + + monkeypatch.setattr(settings, "origin_secret", Settings(origin_secret=f" {SECRET} ").origin_secret) + assert client.get(OPEN_PATH, headers=EDGE).status_code == 200 + + def test_an_all_whitespace_secret_leaves_the_gate_off(self, client: TestClient, monkeypatch): + """Rather than arming it with a value nobody can present.""" + from core.config import Settings + + monkeypatch.setattr(settings, "origin_secret", Settings(origin_secret=" ").origin_secret) + assert client.get(OPEN_PATH).status_code == 200 + + def test_the_other_secret_manager_values_strip_too(self, monkeypatch): + """Same failure mode, same fix, one PR earlier than the incident.""" + from core.config import Settings + + loaded = Settings(admin_token="tok\n", cache_invalidate_token="cache\n", cf_access_aud=" aud ") + assert loaded.admin_token == "tok" + assert loaded.cache_invalidate_token == "cache" + assert loaded.cf_access_aud == "aud"