From db22e54731c07dfd3874f9c537599d20d38d95cc Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:16:27 +0200 Subject: [PATCH 01/12] Close the direct run.app door with a shared-secret origin gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API stands 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 onto every request it proxies for the API host; api/origin_gate.py requires that header and refuses anything else with 403 before the request costs anything. It is not authentication — it says "you came through the front door", nothing about who you are. UNSET MEANS OFF, which is what makes this safe to ship first: the code can go to production long before the rule and the secret exist, local dev and the test suite never see the gate, and taking the variable off the service is the rollback. /health reports the verdict for the request it was asked with — off, off-seen, ok, missing, mismatch — never the value, so every route into the service can be measured before the switch is thrown. Exempt: /health (the deploy smoke reaches the candidate on its run.app tag URL, which never passes the edge), /seo-proxy/… (belt and braces), OPTIONS (a browser cannot attach a custom header to a preflight), and /debug/cache/invalidate — sync-postgres.yml posts there over 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. The apex Worker behind anyplot.ai/api/* gets its source into the repository at infra/cloudflare/, because a Worker subrequest to a host in the same zone bypasses that zone's Transform Rules — so it stamps the header itself, deleting any inbound one first so a caller cannot supply it and corrupt an unarmed /health probe. Its Plausible passthrough for /api/event is preserved. Two changes beyond the gate. The deploy step attaches secrets with --update-secrets instead of --set-secrets: the latter replaces the whole binding set and would have stripped ORIGIN_SECRET, silently disarming the gate on the next deploy. And the analytics middleware moves inside CORSMiddleware, because the gate has to be inside CORS (so its 403 stays readable to a browser) and outside the bot counter (so a refused request cannot fire an outbound Plausible event), which in this stack was only possible with the counter inside CORS. api/main.py now documents the whole order. Transferred from the sibling repo kurrentschrift, where this shipped and was measured live. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- .env.example | 7 + CHANGELOG.md | 41 ++++ api/cloudbuild.yaml | 57 +++++- api/main.py | 58 ++++-- api/origin_gate.py | 157 +++++++++++++++ api/routers/health.py | 27 ++- core/config.py | 43 +++++ docs/development.md | 1 + docs/reference/api.md | 68 ++++++- docs/reference/repository.md | 3 + infra/cloudflare/README.md | 161 ++++++++++++++++ infra/cloudflare/anyplot-api-proxy.js | 32 ++++ tests/unit/api/test_origin_gate.py | 262 ++++++++++++++++++++++++++ 13 files changed, 891 insertions(+), 26 deletions(-) create mode 100644 api/origin_gate.py create mode 100644 infra/cloudflare/README.md create mode 100644 infra/cloudflare/anyplot-api-proxy.js create mode 100644 tests/unit/api/test_origin_gate.py 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 7d2675e6f22..f575cce045a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,31 @@ 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: `/health` (the deploy + smoke reaches the candidate on its `run.app` tag URL, which never passes the edge), + `/seo-proxy/…` (belt and braces — the cost of being wrong there is every crawler seeing a + 403), `/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) and `OPTIONS`, which a browser + cannot attach a custom header to. 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. (#11207) - **IndexNow: changed pages are pushed to Bing, Yandex, Seznam, Naver and Yep instead of waiting for a crawl** — Bing Webmaster Tools' first recommendation for the site. A public key file (`app/public/.txt`, served by an explicit nginx `location` so crawler UAs @@ -174,6 +199,22 @@ 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 attaches secrets with `--update-secrets` instead of `--set-secrets`** — + the latter replaces the whole binding set, so any secret attached to the service out of + band would be 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 the old flag would have silently disarmed the gate on the next deploy. It + cannot 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. (#11207) +- **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. (#11207) ## [3.2.0] — 2026-08-29 — Findable by assistants diff --git a/api/cloudbuild.yaml b/api/cloudbuild.yaml index 34ec5fdb51e..0cff206da10 100644 --- a/api/cloudbuild.yaml +++ b/api/cloudbuild.yaml @@ -68,7 +68,19 @@ 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}" @@ -112,18 +124,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..3ac42046043 --- /dev/null +++ b/api/origin_gate.py @@ -0,0 +1,157 @@ +"""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 three: + +* `/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. +* `/seo-proxy/…` — 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 they DO pass the edge and DO carry the header; the + exemption exists because the cost of being wrong there is every crawler + seeing a 403, and the path is a read with no admin surface behind it. +* `/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. + +`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. +""" + +from __future__ import annotations + +import secrets + +from fastapi import Request +from fastapi.responses import JSONResponse + +from core.config import settings + + +ORIGIN_SECRET_HEADER = "x-origin-secret" + +# The `/seo-proxy` prefix keeps its slash so it cannot also swallow a future +# `/seo-proxy-admin`; the bare form is listed separately for the redirect to +# `/seo-proxy/`. See the module docstring for what each exemption buys. +EXEMPT_PATHS = frozenset({"/health", "/seo-proxy", "/debug/cache/invalidate"}) +EXEMPT_PREFIXES = ("/seo-proxy/",) + + +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 or path.startswith(EXEMPT_PREFIXES) + + +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 secrets.compare_digest(presented, settings.origin_secret or "") 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) + # compare_digest needs two str: an absent header is not a mismatch to + # measure, it is simply the wrong door. + if presented and secrets.compare_digest(presented, settings.origin_secret or ""): + 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/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/core/config.py b/core/config.py index d70f0d97d0b..1d4eca4073b 100644 --- a/core/config.py +++ b/core/config.py @@ -167,6 +167,49 @@ 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. `/health`, `/seo-proxy/…` and + `/debug/cache/invalidate` stay exempt even when it is set — see + `api/origin_gate.py` for why each one has to be.""" + + @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 2927b64c93b..4139521c560 100644 --- a/docs/development.md +++ b/docs/development.md @@ -147,6 +147,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..b4c6c0f8c4e 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -260,17 +260,23 @@ 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": "0.2.0", + "origin_gate": "off" } ``` +`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 +455,60 @@ 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. + +**Exempt paths**, and only these: + +| Path | Why | +|---|---| +| `/health` | the deploy smoke probes the candidate revision on its `run.app` tag URL, which never passes the edge | +| `/seo-proxy/…` | belt and braces — the prerendered pages do come through the edge, but the cost of being wrong is every crawler seeing a 403 | +| `/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. + +**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 +517,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..5d2cfe316bc --- /dev/null +++ b/infra/cloudflare/README.md @@ -0,0 +1,161 @@ +# 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**. 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. The secret is never typed: it is read from Secret Manager at execution +time, stays in a shell variable, and reaches `curl` through stdin, so it lands +in no shell history, no file and no process list. + +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; } + + ORIGIN_SECRET="$ORIGIN_SECRET" python3 -c ' +import json, os, sys +json.dump({ + "main_module": "worker.js", + "compatibility_date": "2026-04-29", + "bindings": [ + {"type": "secret_text", "name": "ORIGIN_SECRET", "text": os.environ["ORIGIN_SECRET"]}, + ], +}, sys.stdout)' | curl --fail-with-body -sS -X PUT \ + "https://api.cloudflare.com/client/v4/accounts/{account}/workers/scripts/anyplot-api-proxy" \ + -H "Authorization: Bearer $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. Then measure before +trusting it: + +```bash +curl -s https://anyplot.ai/api/health # expect "origin_gate":"ok" +``` + +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..939bd70a304 --- /dev/null +++ b/tests/unit/api/test_origin_gate.py @@ -0,0 +1,262 @@ +"""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 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: + """`/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. `/seo-proxy` is belt and braces: the + site's nginx DOES come through the edge, but the cost of being wrong there + is every crawler seeing a 403. `/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_prerendered_pages_are_never_gated(self, client: TestClient, armed): + assert client.get("/seo-proxy/legal").status_code != 403 + + 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 + + @pytest.mark.parametrize( + ("path", "exempt"), + [ + ("/health", True), + ("/seo-proxy", True), # the redirect to /seo-proxy/ + ("/seo-proxy/", True), + ("/seo-proxy/specs", True), + ("/debug/cache/invalidate", True), + ("/seo-proxy-admin", False), + ("/healthz", False), + ("/debug/cache", False), + ("/debug/status", False), + ("/specs", False), + ], + ) + def test_the_exemption_list_is_exactly_these_prefixes(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_the_exempt_crawler_path_is_still_counted(self, client: TestClient, armed): + with patch.object(api_main, "track_bot_fetch") as pages: + client.get("/seo-proxy/legal", headers=self.CRAWLER) + 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 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" From eac2d8d26aa3ec45acea0820a1d7929a8d2557e4 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:17:25 +0200 Subject: [PATCH 02/12] Point the changelog entries at this PR's number Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f575cce045a..ea7f1ac4c8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ aggregate instead: an italic *Catalog* line at the end of the version section an 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. (#11207) + the gate is armed and after a rollback. (#11208) - **IndexNow: changed pages are pushed to Bing, Yandex, Seznam, Naver and Yep instead of waiting for a crawl** — Bing Webmaster Tools' first recommendation for the site. A public key file (`app/public/.txt`, served by an explicit nginx `location` so crawler UAs @@ -205,7 +205,7 @@ aggregate instead: an italic *Catalog* line at the end of the version section an exactly such a binding — attached by hand to arm the origin gate, removed by hand to roll back — and the old flag would have silently disarmed the gate on the next deploy. It cannot 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. (#11207) + not exist, which would break every build until the rollout creates it. (#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 @@ -214,7 +214,7 @@ aggregate instead: an italic *Catalog* line at the end of the version section an 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. (#11207) + now carries the stack order and the reason for each position. (#11208) ## [3.2.0] — 2026-08-29 — Findable by assistants From 4be71da48a0808e7708bf3fd10de5c1be833dbdb Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:31:46 +0200 Subject: [PATCH 03/12] Close the seo-proxy bypass and compare the header as bytes Copilot review, four findings, all applied. 1. The /seo-proxy exemption was a real bypass, not belt and braces. Those handlers are not uniformly cheap: a cache miss or an unknown id queries SpecRepository/ImplRepository, and any request with a recognized crawler user agent schedules an outbound Plausible event. Exempting them left the API's most expensive reads open on the direct run.app URL - exactly the cost the gate exists to refuse. The site's nginx already fetches those pages over api.anyplot.ai, so the path carries the header; the rollout validates it end to end with a crawler user agent before arming, and bot-serving-check.yml runs daily, so being wrong is loud. The exemption list is now exact paths only, no prefixes. 2. 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 - so one byte >= 0x80 in X-Origin-Secret turned every refusal into an unhandled 500, an unauthenticated way to make the gate expensive. Both comparisons now encode first, which also covers a non-ASCII secret at the other end. 3. The /health example in docs/reference/api.md still showed version 0.2.0, as did the root endpoint two blocks above it; both now show the real version, with a note that the field tracks the installed package. 4. agentic/docs/project-guide.md's directory structure did not list the new infra/ tree. It does now, next to the same entry in docs/reference/repository.md. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- CHANGELOG.md | 21 +++++--- agentic/docs/project-guide.md | 2 + api/origin_gate.py | 51 ++++++++++++------ docs/reference/api.md | 21 ++++++-- tests/unit/api/test_origin_gate.py | 86 +++++++++++++++++++++++------- 5 files changed, 135 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea7f1ac4c8b..c20cab49623 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,13 +41,20 @@ aggregate instead: an italic *Catalog* line at the end of the version section an 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: `/health` (the deploy - smoke reaches the candidate on its `run.app` tag URL, which never passes the edge), - `/seo-proxy/…` (belt and braces — the cost of being wrong there is every crawler seeing a - 403), `/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) and `OPTIONS`, which a browser - cannot attach a custom header to. The Cloudflare Worker behind `anyplot.ai/api/*` now has + 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. + The header is compared as bytes rather than as `str`, because `secrets.compare_digest` + raises `TypeError` on a non-ASCII `str` and a header arrives latin-1-decoded from the wire: + comparing strings would have handed any caller a one-byte way to turn every refusal into an + unhandled 500. 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 diff --git a/agentic/docs/project-guide.md b/agentic/docs/project-guide.md index d2e7fbf7f98..5df6be00028 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/origin_gate.py b/api/origin_gate.py index 3ac42046043..666b8424e52 100644 --- a/api/origin_gate.py +++ b/api/origin_gate.py @@ -27,16 +27,11 @@ gate, and the rollout can put the code in production long before the rule and the secret exist. -Exempt by path, and only these three: +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. -* `/seo-proxy/…` — 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 they DO pass the edge and DO carry the header; the - exemption exists because the cost of being wrong there is every crawler - seeing a 403, and the path is a read with no admin surface behind it. * `/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 @@ -48,6 +43,17 @@ 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 @@ -68,11 +74,24 @@ ORIGIN_SECRET_HEADER = "x-origin-secret" -# The `/seo-proxy` prefix keeps its slash so it cannot also swallow a future -# `/seo-proxy-admin`; the bare form is listed separately for the redirect to -# `/seo-proxy/`. See the module docstring for what each exemption buys. -EXEMPT_PATHS = frozenset({"/health", "/seo-proxy", "/debug/cache/invalidate"}) -EXEMPT_PREFIXES = ("/seo-proxy/",) +# 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 _matches(presented: str, expected: str) -> bool: + """Constant-time compare of the presented header against the secret. + + Compared as BYTES, not as `str`: `secrets.compare_digest` raises TypeError + when either `str` holds a non-ASCII character, and a header value reaches + here latin-1-decoded straight from the wire. So a caller could put one byte + ≥ 0x80 in `X-Origin-Secret` and turn every refusal into an unhandled 500 — + an unauthenticated way to make the gate expensive instead of cheap (Copilot + review). Encoding first removes the restriction and the whole class of + problem, and covers a non-ASCII secret at the other end too. + """ + return secrets.compare_digest(presented.encode("utf-8"), expected.encode("utf-8")) def gate_is_armed() -> bool: @@ -88,7 +107,7 @@ 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 or path.startswith(EXEMPT_PREFIXES) + return path in EXEMPT_PATHS def header_verdict(request: Request) -> str: @@ -117,7 +136,7 @@ def header_verdict(request: Request) -> str: return "off-seen" if presented else "off" if not presented: return "missing" - return "ok" if secrets.compare_digest(presented, settings.origin_secret or "") else "mismatch" + return "ok" if _matches(presented, settings.origin_secret or "") else "mismatch" class OriginSecretMiddleware: @@ -144,9 +163,9 @@ async def __call__(self, scope, receive, send): await self.app(scope, receive, send) return presented = Request(scope).headers.get(ORIGIN_SECRET_HEADER) - # compare_digest needs two str: an absent header is not a mismatch to - # measure, it is simply the wrong door. - if presented and secrets.compare_digest(presented, settings.origin_secret or ""): + # An absent header is not a mismatch to measure, it is simply the wrong + # door — so it short-circuits before the compare. + if presented and _matches(presented, settings.origin_secret or ""): await self.app(scope, receive, send) return response = JSONResponse( diff --git a/docs/reference/api.md b/docs/reference/api.md index b4c6c0f8c4e..67b4d94c891 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" } @@ -268,11 +268,14 @@ The anyplot API is a **FastAPI-based REST API** serving plot data to the fronten { "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. @@ -474,18 +477,28 @@ 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. -**Exempt paths**, and only these: +**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 | -| `/seo-proxy/…` | belt and braces — the prerendered pages do come through the edge, but the cost of being wrong is every crawler seeing a 403 | | `/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 +``` + **Observing it.** `GET /health` reports `origin_gate` for the request it was asked with, never the value: diff --git a/tests/unit/api/test_origin_gate.py b/tests/unit/api/test_origin_gate.py index 939bd70a304..c0cc2fd165e 100644 --- a/tests/unit/api/test_origin_gate.py +++ b/tests/unit/api/test_origin_gate.py @@ -17,7 +17,7 @@ import api.main as api_main from api.main import app -from api.origin_gate import ORIGIN_SECRET_HEADER, is_exempt +from api.origin_gate import ORIGIN_SECRET_HEADER, _matches, is_exempt from core.config import settings @@ -113,14 +113,13 @@ def test_it_answers_before_the_admin_credential_is_looked_at(self, client: TestC class TestTheExemptPaths: - """`/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. `/seo-proxy` is belt and braces: the - site's nginx DOES come through the edge, but the cost of being wrong there - is every crawler seeing a 403. `/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.""" + """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 @@ -128,30 +127,34 @@ def test_the_gate_is_really_armed_for_this_test(self, client: TestClient, armed) def test_health_is_never_gated(self, client: TestClient, armed): assert client.get("/health").status_code == 200 - def test_the_prerendered_pages_are_never_gated(self, client: TestClient, armed): - assert client.get("/seo-proxy/legal").status_code != 403 - 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), - ("/seo-proxy", True), # the redirect to /seo-proxy/ - ("/seo-proxy/", True), - ("/seo-proxy/specs", True), ("/debug/cache/invalidate", True), - ("/seo-proxy-admin", False), + ("/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_prefixes(self, path, exempt): + def test_the_exemption_list_is_exactly_these_two_paths(self, path, exempt): assert is_exempt(path, "GET") is exempt @@ -202,9 +205,16 @@ def test_a_refused_asset_read_reports_nothing(self, client: TestClient, armed): assert client.get(self.ASSET, headers=self.CRAWLER).status_code == 403 assert assets.call_count == 0 - def test_the_exempt_crawler_path_is_still_counted(self, client: TestClient, armed): + 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) + 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): @@ -227,6 +237,44 @@ def test_verdict(self, client: TestClient, armed, headers, verdict): 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 _matches(seen, seen) is True + assert _matches(seen, SECRET) is False + + 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 From a78d224692fa2b7fba700fbc2e732d8d15c20c25 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:33:56 +0200 Subject: [PATCH 04/12] Keep the Cloudflare API token out of curl's argument vector Copilot review: the Worker-deploy recipe passed the credential as -H "Authorization: Bearer $CF_API_TOKEN", which publishes it in /proc//cmdline for the life of the request - to every process on the machine, and for the token that authorises replacing this Worker. The ORIGIN_SECRET handling in the same block already avoided exactly that. It now goes in through curl --config on a process-substitution file descriptor, with printf as a shell builtin, so the token reaches no argv at all. Noted that this needs bash or zsh, with the 600-mode temporary file as the fallback. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- infra/cloudflare/README.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/infra/cloudflare/README.md b/infra/cloudflare/README.md index 5d2cfe316bc..2eb40fae7e2 100644 --- a/infra/cloudflare/README.md +++ b/infra/cloudflare/README.md @@ -107,7 +107,7 @@ json.dump({ ], }, sys.stdout)' | curl --fail-with-body -sS -X PUT \ "https://api.cloudflare.com/client/v4/accounts/{account}/workers/scripts/anyplot-api-proxy" \ - -H "Authorization: Bearer $CF_API_TOKEN" \ + --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' ) @@ -115,8 +115,19 @@ json.dump({ `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. Then measure before -trusting it: +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: ```bash curl -s https://anyplot.ai/api/health # expect "origin_gate":"ok" From 3f75335c05142e73ff325ad805da499191f4de81 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:42:15 +0200 Subject: [PATCH 05/12] Correct the settings docstring that still claimed seo-proxy is exempt Copilot review: core/config.py's origin_secret docstring listed /seo-proxy/... among the exempt paths, which is what it was before the exemption was removed earlier in this PR. An operator reading only the settings file could arm the gate expecting crawler traffic to be waved through. It now names the two real exemptions and says explicitly that the prerendered pages are not among them. The PR description raised in the same review was already corrected before this push; the review read the pre-edit body. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- core/config.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/config.py b/core/config.py index 1d4eca4073b..3e9d5c50f4d 100644 --- a/core/config.py +++ b/core/config.py @@ -175,9 +175,11 @@ class Settings(BaseSettings): 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. `/health`, `/seo-proxy/…` and - `/debug/cache/invalidate` stay exempt even when it is set — see - `api/origin_gate.py` for why each one has to be.""" + 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", From b922c6150eb54d2cfb1f5bc9d15e9bc6a12cbe07 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:09:12 +0200 Subject: [PATCH 06/12] Promote by revision name when arming or rolling back, never --to-latest Copilot review: the rollout used 'update-traffic --to-latest' for both arming and rollback - the exact flag api/cloudbuild.yaml refuses, and for the same reason. The deploy pipeline leaves each build's smoked-but-unpromoted candidate as the latest revision, so --to-latest can promote a concurrent build's image while the operator believes they are only turning the gate on or off. Both procedures now stamp a deterministic --revision-suffix on the service update and promote exactly that revision with --to-revisions. The commands move into docs/reference/api.md so the procedure outlives the PR description, and the Worker README points at them instead of paraphrasing. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- docs/reference/api.md | 19 +++++++++++++++++++ infra/cloudflare/README.md | 8 +++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/docs/reference/api.md b/docs/reference/api.md index 67b4d94c891..81c381104af 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -477,6 +477,25 @@ 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 two commands with a different first +flag. The service pins traffic to a named revision, so a `services update` +alone creates a revision that serves nothing — the promote is what takes +effect, and it must name the revision the update just made: + +```bash +# Arm. (Roll back with --remove-secrets=ORIGIN_SECRET instead.) +SUFFIX="arm-$(date -u +%Y%m%d%H%M)" +gcloud run services update anyplot-api --project=anyplot --region=europe-west4 \ + --update-secrets=ORIGIN_SECRET=ORIGIN_SECRET:latest --revision-suffix="$SUFFIX" +gcloud run services update-traffic anyplot-api --project=anyplot --region=europe-west4 \ + --to-revisions="anyplot-api-$SUFFIX=100" +``` + +**Never `--to-latest` here**, for the reason `api/cloudbuild.yaml` avoids it: +the deploy pipeline leaves each build's smoked-but-unpromoted candidate as the +latest revision, so `--to-latest` can promote a concurrent build's untested +image while you think you are only turning the gate on or off. + **Exempt paths** — exact matches, no prefixes, and only these two: | Path | Why | diff --git a/infra/cloudflare/README.md b/infra/cloudflare/README.md index 2eb40fae7e2..6720bc67a3c 100644 --- a/infra/cloudflare/README.md +++ b/infra/cloudflare/README.md @@ -55,9 +55,11 @@ 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**. The ordering holds in both -directions: the Worker starts stamping before the gate is armed, and stops only -after it is disarmed. +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) From ce03fbc5641525d4ba541b3c5149eec2ce7ed1e6 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:16:44 +0200 Subject: [PATCH 07/12] Guard the arm procedure against an in-flight candidate and a floating secret Copilot review, two more on the same procedure, both correct. 1. 'services update' clones the service's LATEST template, not the serving one, and the deploy pipeline deliberately leaves each build's smoked-but-unpromoted candidate as latest. Arming during a deploy would therefore promote that build's image along with the gate - naming the new revision precisely does not change which image it inherits. The block now asserts that latestReadyRevisionName equals the revision serving 100% and refuses otherwise, the same shape of assertion the cloudbuild smoke makes about its own candidate tag. 2. ORIGIN_SECRET:latest floats. Cloud Run resolves a secret-backed variable when each instance starts, so a new version reaches new instances while older ones keep the old value - and because the edge stamps exactly one value, that surfaces as intermittent 403s inside a single revision. The binding is pinned to a resolved version number. Rotation gets its own paragraph, because the gate accepts exactly one value and there is no overlap window: roll back, rotate both sides, arm again on the new number. The gate is off in between, which is the documented safe state. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- docs/reference/api.md | 59 +++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/docs/reference/api.md b/docs/reference/api.md index 81c381104af..82e23eef342 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -477,24 +477,57 @@ 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 two commands with a different first -flag. The service pins traffic to a named revision, so a `services update` -alone creates a revision that serves nothing — the promote is what takes -effect, and it must name the revision the update just made: +**Arming and rolling back** are the same procedure with one flag changed. Three +things make it more than two commands, and each of them has bitten a comparable +rollout somewhere: ```bash -# Arm. (Roll back with --remove-secrets=ORIGIN_SECRET instead.) +SERVICE=anyplot-api +LOC="--project=anyplot --region=europe-west4" + +# 1. Refuse to act while a candidate revision is in flight. `services update` +# clones the service's LATEST template, not the serving one, and the deploy +# pipeline deliberately leaves each build's smoked-but-unpromoted candidate +# as latest — so arming during a deploy would promote that build's image +# along with the gate, and naming the new revision precisely does not change +# which image it inherits. +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'])")" +test "$SERVING" = "$LATEST" || { + echo "latest revision $LATEST is not the serving one ($SERVING): a candidate is in flight." + echo "Wait for the pipeline to promote or roll it back, then start again." + exit 1 +} + +# 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)") + +# 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 +# reintroduce exactly the problem step 1 guards against. SUFFIX="arm-$(date -u +%Y%m%d%H%M)" -gcloud run services update anyplot-api --project=anyplot --region=europe-west4 \ - --update-secrets=ORIGIN_SECRET=ORIGIN_SECRET:latest --revision-suffix="$SUFFIX" -gcloud run services update-traffic anyplot-api --project=anyplot --region=europe-west4 \ - --to-revisions="anyplot-api-$SUFFIX=100" +gcloud run services update "$SERVICE" $LOC \ + --update-secrets="ORIGIN_SECRET=ORIGIN_SECRET:$VERSION" --revision-suffix="$SUFFIX" +gcloud run services update-traffic "$SERVICE" $LOC --to-revisions="$SERVICE-$SUFFIX=100" ``` -**Never `--to-latest` here**, for the reason `api/cloudbuild.yaml` avoids it: -the deploy pipeline leaves each build's smoked-but-unpromoted candidate as the -latest revision, so `--to-latest` can promote a concurrent build's untested -image while you think you are only turning the gate on or off. +**Rolling back** is the same block with `--remove-secrets=ORIGIN_SECRET` in +place of `--update-secrets` (step 2 then has nothing to resolve) and a +`disarm-` suffix. + +**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: From eb944f779932538251960a0e13e23fbe892cfe03 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:20:56 +0200 Subject: [PATCH 08/12] Label the post-deploy verdict as off-seen, not ok Copilot review: the Worker README told the operator to expect 'origin_gate':'ok' straight after deploying the Worker, which contradicts the rollout order three sections above it - at that point the API gate is deliberately still off, so a correctly stamped request answers 'off-seen'. Read literally, the required pre-arm measurement looked like a failed deployment, and the obvious reaction to it would have been to arm the gate early. The command now lists all three reachable verdicts with what each one means at that moment, including that 'off' is the one that says the binding is missing and arming would take the route down. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- infra/cloudflare/README.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/infra/cloudflare/README.md b/infra/cloudflare/README.md index 6720bc67a3c..44bd74a56b1 100644 --- a/infra/cloudflare/README.md +++ b/infra/cloudflare/README.md @@ -129,10 +129,18 @@ 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: +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 # expect "origin_gate":"ok" +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 From 56467da7796a51d0eaccac063d90576d9ae6b223 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:27:29 +0200 Subject: [PATCH 09/12] Name the residual the gate does not close: the app origin relays Copilot review, and it is correct as an observation: the app service 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 can still reach the prerendered render and its DB queries by asking the APP's raw run.app URL instead of the API's. That is a second door on a second service, not a hole in this one - the request this process sees really did come through the edge, so there is nothing for this middleware to refuse. 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 carries its own blast radius and belongs in its own PR. Named in api/origin_gate.py and docs/reference/api.md rather than left for someone to rediscover, because a gate whose limits are undocumented gets trusted past them. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- api/origin_gate.py | 13 +++++++++++++ docs/reference/api.md | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/api/origin_gate.py b/api/origin_gate.py index eb426f50561..423b5847267 100644 --- a/api/origin_gate.py +++ b/api/origin_gate.py @@ -60,6 +60,19 @@ 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 diff --git a/docs/reference/api.md b/docs/reference/api.md index 82e23eef342..2f9b79c24ad 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -551,6 +551,17 @@ to end while the gate is still off (below), not with an exemption: 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: From 622f751e145e5c31d343f1a372b071673bca53d1 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:33:59 +0200 Subject: [PATCH 10/12] Keep the rollback executable, and state that break-glass now needs both headers Copilot review, two findings. 1. The in-flight guard could have made the emergency rollback unavailable. A failed smoke skips the promote and nothing cleans the candidate up, so the failed revision stays latestReadyRevisionName indefinitely and SERVING == LATEST would never become true by waiting - a hard refusal there would deadlock disarming at exactly the moment it is needed, and the check was racy besides. The block now pins --image to the SERVING revision's image instead, which makes the operation correct whatever state the pipeline is in, and demotes the equality test to a warning that says what a mismatch still means (an unpromoted config change). A step 4 was added to confirm the result, since a warning nobody reads is not a safeguard. 2. require_admin's docstring still promised that X-Admin-Token alone gets an operator in over the direct URL. With the gate armed that call needs BOTH headers, because the origin gate answers first - and it answers 403, not 401, which is the only hint an operator would get. Documented where they will read it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- api/routers/debug.py | 8 ++++++++ docs/reference/api.md | 38 ++++++++++++++++++++++++-------------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/api/routers/debug.py b/api/routers/debug.py index d001f7f85c1..23aa37bccfd 100644 --- a/api/routers/debug.py +++ b/api/routers/debug.py @@ -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. """ diff --git a/docs/reference/api.md b/docs/reference/api.md index 2f9b79c24ad..f67a320853a 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -485,21 +485,25 @@ rollout somewhere: SERVICE=anyplot-api LOC="--project=anyplot --region=europe-west4" -# 1. Refuse to act while a candidate revision is in flight. `services update` -# clones the service's LATEST template, not the serving one, and the deploy -# pipeline deliberately leaves each build's smoked-but-unpromoted candidate -# as latest — so arming during a deploy would promote that build's image -# along with the gate, and naming the new revision precisely does not change -# which image it inherits. +# 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'])")" -test "$SERVING" = "$LATEST" || { - echo "latest revision $LATEST is not the serving one ($SERVING): a candidate is in flight." - echo "Wait for the pipeline to promote or roll it back, then start again." - exit 1 -} +IMAGE=$(gcloud run revisions describe "$SERVING" $LOC --format="value(spec.containers[0].image)") + +# 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 @@ -511,16 +515,22 @@ VERSION=$(gcloud secrets versions list ORIGIN_SECRET --project=anyplot \ # 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 -# reintroduce exactly the problem step 1 guards against. +# hand traffic to whatever the pipeline last built. SUFFIX="arm-$(date -u +%Y%m%d%H%M)" -gcloud run services update "$SERVICE" $LOC \ +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 what is now serving — the gate's verdict AND the image, because +# step 1's warning is only worth having if somebody looks. +curl -s "https://api.anyplot.ai/health" +gcloud run services describe "$SERVICE" $LOC --format="value(status.traffic)" ``` **Rolling back** is the same block with `--remove-secrets=ORIGIN_SECRET` in place of `--update-secrets` (step 2 then has nothing to resolve) and a -`disarm-` suffix. +`disarm-` suffix. It must stay executable in the worst state the service can be +in, which is why step 1 warns rather than refuses. **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, From 34f1604296c855dfde2fdca3dad3d46579d35c7f Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:41:44 +0200 Subject: [PATCH 11/12] Make the operational blocks fail-fast, race-aware and leak-free Copilot review, three findings on the procedures. 1. The block was not fail-fast: a failed lookup left an empty variable and the shell walked on into the mutation and the verification, so an operator could leave traffic unchanged while the closing health line still looked plausible. It now runs in a set -euo pipefail subshell with explicit non-empty assertions on SERVING, IMAGE and VERSION. 2. Arming raced the deploy pipeline. A build that had ALREADY deployed its candidate promotes it at the end, and that revision was cloned from the pre-arm template - so the promote silently undid the arm, and the build's own smoke accepts 'off' by design. A build that STARTS after the block inherits the binding, because the deploy is additive, so the dangerous window is exactly 'a build already in flight'. Step 0 refuses to start in that window; step 4 is what catches it if it happens anyway, and says to re-run. The rollback paragraph says to skip step 0 when the gate itself is the outage. 3. The Worker recipe claimed the secret reaches no process list while passing it as a VAR=value prefix, which puts it in /proc//environ for any same-UID process. It goes in on stdin now, and the claim above it is rewritten to describe both secrets accurately: stdin for one, a --config file descriptor for the other, neither exported. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- docs/reference/api.md | 33 ++++++++++++++++++++++++++++----- infra/cloudflare/README.md | 18 ++++++++++++------ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/docs/reference/api.md b/docs/reference/api.md index f67a320853a..cfb7e7fe743 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -477,14 +477,30 @@ 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. Three +**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: +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 @@ -497,6 +513,7 @@ read -r SERVING LATEST <<<"$(gcloud run services describe "$SERVICE" $LOC --form 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 @@ -512,6 +529,7 @@ test "$SERVING" = "$LATEST" || echo "note: latest ($LATEST) is not serving ($SER # 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 @@ -521,16 +539,21 @@ 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 what is now serving — the gate's verdict AND the image, because -# step 1's warning is only worth having if somebody looks. +# 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 the same block with `--remove-secrets=ORIGIN_SECRET` in place of `--update-secrets` (step 2 then has nothing to resolve) and a `disarm-` suffix. It must stay executable in the worst state the service can be -in, which is why step 1 warns rather than refuses. +in, which is why step 1 warns rather than refuses — and when the gate is causing +an outage, step 0's wait is the wrong trade: skip it, disarm, and re-check +afterwards. **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, diff --git a/infra/cloudflare/README.md b/infra/cloudflare/README.md index 44bd74a56b1..d5db974690f 100644 --- a/infra/cloudflare/README.md +++ b/infra/cloudflare/README.md @@ -79,9 +79,11 @@ 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. The secret is never typed: it is read from Secret Manager at execution -time, stays in a shell variable, and reaches `curl` through stdin, so it lands -in no shell history, no file and no process list. +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 @@ -99,13 +101,17 @@ runs. --secret=ORIGIN_SECRET --project=anyplot) [ -n "$ORIGIN_SECRET" ] || { echo "empty ORIGIN_SECRET — refusing to deploy"; exit 1; } - ORIGIN_SECRET="$ORIGIN_SECRET" python3 -c ' -import json, os, sys + # 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": os.environ["ORIGIN_SECRET"]}, + {"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" \ From b56be24c406988979cf6e4640c9a0f4e02f6fdc0 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:46:48 +0200 Subject: [PATCH 12/12] Write the rollback out as its own block, with no secret lookup Copilot review: 'the same block with one flag swapped' still ran step 2, which resolves an ENABLED version of ORIGIN_SECRET and exits when there is none. If the secret was disabled or deleted during the incident - a plausible thing to have happened, and a plausible reason to be rolling back - set -e would abort before --remove-secrets ever ran, leaving the gate armed at exactly the moment it has to come off. The rollback is now its own block that depends on nothing but the currently serving revision: no in-flight check (waiting for a build is the wrong trade when the gate is the outage) and no secret lookup at all. It ends by reading the verdict back, and the note that removing the Worker binding is not a rollback moved next to it. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke --- docs/reference/api.md | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/docs/reference/api.md b/docs/reference/api.md index cfb7e7fe743..fee43c52e65 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -548,12 +548,38 @@ gcloud run services describe "$SERVICE" $LOC --format="value(status.traffic)" ) ``` -**Rolling back** is the same block with `--remove-secrets=ORIGIN_SECRET` in -place of `--update-secrets` (step 2 then has nothing to resolve) and a -`disarm-` suffix. It must stay executable in the worst state the service can be -in, which is why step 1 warns rather than refuses — and when the gate is causing -an outage, step 0's wait is the wrong trade: skip it, disarm, and re-check -afterwards. +**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,