diff --git a/.github/workflows/sync-postgres.yml b/.github/workflows/sync-postgres.yml index 916a3abb01..941d8a49f5 100644 --- a/.github/workflows/sync-postgres.yml +++ b/.github/workflows/sync-postgres.yml @@ -79,16 +79,35 @@ jobs: # is unprotected by CF and accepts the bearer token directly. API_URL: ${{ vars.API_DIRECT_URL || 'https://anyplot-api-r3tvmejsmq-ez.a.run.app' }} CACHE_INVALIDATE_TOKEN: ${{ secrets.CACHE_INVALIDATE_TOKEN }} + # The header the Cloudflare Transform Rule stamps on everything it + # proxies for api.anyplot.ai. This call skips the edge on purpose (see + # above), so it has to stamp its own — which is what let + # api/origin_gate.py drop the /debug/cache/invalidate exemption. An + # exempt path is one anybody may POST to from anywhere; a caller that + # carries the header needs no hole in the gate. The value is never + # echoed: it reaches curl through the environment, and the step runs + # without `set -x`. + ORIGIN_SECRET: ${{ secrets.ORIGIN_SECRET }} run: | if [ -z "${CACHE_INVALIDATE_TOKEN}" ]; then echo "CACHE_INVALIDATE_TOKEN not set — skipping cache invalidation (cache will fall back to TTL expiry)" exit 0 fi + # Absent secret => no header => the gate answers 403, which the branch + # below names. Sending an empty header instead would look like a + # mismatch and read as a rotation problem rather than a missing secret. + HDR=() + if [ -n "${ORIGIN_SECRET}" ]; then HDR=(-H "X-Origin-Secret: ${ORIGIN_SECRET}"); fi status=$(curl -sS -o /tmp/invalidate.json -w '%{http_code}' \ -X POST "${API_URL}/debug/cache/invalidate" \ + "${HDR[@]}" \ -H "X-Cache-Token: ${CACHE_INVALIDATE_TOKEN}") echo "HTTP ${status}" cat /tmp/invalidate.json || true + if [ "${status}" = "403" ]; then + echo "::error::Cache invalidation was refused by the origin gate (HTTP 403). Set the ORIGIN_SECRET repository secret to the same value as the ORIGIN_SECRET on the anyplot-api Cloud Run service." + exit 1 + fi if [ "${status}" = "503" ] || [ "${status}" = "401" ]; then echo "::error::Cache invalidation returned HTTP ${status} — token misconfigured server-side (503) or mismatched (401)" exit 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0475c20e96..a08647385d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -324,6 +324,30 @@ aggregate instead: an italic *Catalog* line at the end of the version section an ### Security +- **The origin gate loses its last real exemption: the cache flush now carries the edge's + header instead of being waved through** — `/debug/cache/invalidate` was exempt because + `sync-postgres.yml` has no front door to come through: it posts from a GitHub runner to + the direct `*.run.app` URL on purpose, since Cloudflare's bot challenge answers an + unauthenticated curl POST against `api.anyplot.ai` with a 403 HTML page. The workflow now + sends `X-Origin-Secret` itself, out of an `ORIGIN_SECRET` repository secret, so the + exemption is gone and `EXEMPT_PATHS` holds exactly one entry — `/health`, which the deploy + smoke needs on the candidate's tag URL. The trade is worth naming: an exempt path is one + anybody may POST to from anywhere with only `CACHE_INVALIDATE_TOKEN` behind it, whereas a + caller that carries the header needs no hole at all. The two locks now sit in series, and + the test suite pins both ends — 403 without the header, 503 (the endpoint's own + fail-closed answer) with it. A missing repository secret fails that step with a message + naming it, rather than leaving the cache to go quietly stale. The second door the gate + still does not close — a crawler user agent reaching the prerendered pages through the + APP service's raw `run.app` URL — is now measured rather than suspected (a Googlebot UA + gets HTTP 200 and the correct canonical), and `api/origin_gate.py` records the two facts + that decide how it can be closed: Cloud Run answers a foreign `Host` header with its own + 404, so a host rule in `app/nginx.conf` would be a real boundary rather than theatre — + and `bot-serving-check.yml` probes exactly that origin and cannot spoof the host either, + so the exception it needs has to be the shared secret, which means templating the app's + nginx, attaching the secret to `anyplot-app`, and a Cloudflare Transform Rule for the + `anyplot.ai` host that does not exist yet. Four coordinated changes, two in the dashboard, + one able to lock out every visitor if it lands out of order. (#11214) + - **`click` 8.3.1 → 8.3.3 closes PYSEC-2026-2132** — the only advisory `pip-audit` reports against the resolved runtime dependency set (`uv export --no-dev`), which now comes back clean. A transitive dependency, so the fix is a lock-file bump with no diff --git a/api/origin_gate.py b/api/origin_gate.py index 423b584726..4ce6c416e1 100644 --- a/api/origin_gate.py +++ b/api/origin_gate.py @@ -27,21 +27,22 @@ gate, and the rollout can put the code in production long before the rule and the secret exist. -Exempt by path, and only these two: +Exempt by path, and only this one: * `/health` — the deploy's pre-traffic smoke probes the candidate revision on its `run.app` tag URL, which by definition never passes the edge. Gating it would make every deploy fail closed. -* `/debug/cache/invalidate` — the one legitimate caller that has no front door. - `sync-postgres.yml` flushes the cache from a GitHub runner at the end of each - sync, and it posts to the direct `*.run.app` URL *on purpose*: Cloudflare's - bot challenge answers an unauthenticated curl POST against `api.anyplot.ai` - with a 403 HTML page. The endpoint carries its own shared secret - (`CACHE_INVALIDATE_TOKEN`, constant-time compared) and returns 503 when none - is configured, so it is gated — just by a different lock. Sending the origin - secret from CI instead would let this exemption go; that needs a repository - secret and a workflow change, and is named as the follow-up in the PR that - introduced this file. + +`/debug/cache/invalidate` used to be the second, and is not any more. +`sync-postgres.yml` flushes the cache from a GitHub runner at the end of each +sync and 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 — so it had no front door to come through. It has one now: the +workflow sends the header itself, out of the `ORIGIN_SECRET` repository secret. +That is strictly better than the exemption it replaces. An exempt path is one +anybody may POST to from anywhere, with only the endpoint's own +`CACHE_INVALIDATE_TOKEN` behind it; a workflow that carries the header needs no +hole in the gate at all. `/seo-proxy/…` is deliberately NOT exempt, though the sibling repo exempts it belt-and-braces. The site's nginx fetches the prerendered pages over @@ -61,18 +62,42 @@ 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. +**What this gate does not close, measured 2026-09-03.** 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). Confirmed live: a Googlebot UA +against `…run.app/scatter-basic` answers 200 with the correct +``. 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. + +Two things about closing it are now known, and neither was before: + +* **A `Host` rule would be a real boundary, not theatre.** The obvious worry is + that anyone could send `Host: anyplot.ai` to the `run.app` URL and walk + straight through a host check. They cannot: Google's frontend routes by Host + and answers a foreign one with its own 404 before the container is reached + (measured — `curl -H "Host: anyplot.ai" https://anyplot-app-….run.app/…` + returns Google's 404 page). On that origin `$host` is therefore always the + `run.app` name, so `app/nginx.conf` could refuse `@seo_proxy` for it. +* **That alone would break `bot-serving-check.yml`**, which probes exactly this + origin with crawler UAs and cannot spoof the Host either, because Cloudflare + 403s GitHub-runner IPs even for a UA-spoofed Googlebot. An exception keyed on + a header or a UA the workflow sends is worthless — this repository is public, + so the value is public with it. The exception has to be the shared secret, + which means the app's nginx must LEARN the secret: template the config + (`nginx-unprivileged` ships the `envsubst` entrypoint), attach `ORIGIN_SECRET` + to `anyplot-app` in `app/cloudbuild.yaml`, add a Cloudflare Transform Rule for + the `anyplot.ai` host (today's rule covers `api.anyplot.ai` only — without it + the enforcing config locks out every human visitor), and hand the workflow the + same secret. + +Four coordinated changes, two of them in the dashboard and one of them able to +take the whole site down if it lands out of order. That is the reason this is +still described here rather than done. """ from __future__ import annotations @@ -87,9 +112,9 @@ ORIGIN_SECRET_HEADER = "x-origin-secret" # Exact paths only, no prefixes: a prefix exemption is how a gate quietly grows -# a hole. See the module docstring for what each of these two buys, and why -# `/seo-proxy/…` is not among them. -EXEMPT_PATHS = frozenset({"/health", "/debug/cache/invalidate"}) +# a hole. See the module docstring for what this one buys, why +# `/debug/cache/invalidate` no longer needs it, and why `/seo-proxy/…` never did. +EXEMPT_PATHS = frozenset({"/health"}) def gate_is_armed() -> bool: diff --git a/api/routers/debug.py b/api/routers/debug.py index 23aa37bccf..f9417c5fb3 100644 --- a/api/routers/debug.py +++ b/api/routers/debug.py @@ -498,9 +498,11 @@ async def invalidate_cache(x_cache_token: str | None = Header(default=None)) -> if not expected: raise HTTPException(status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Cache invalidation not configured") # Constant-time compare to avoid byte-by-byte token recovery via timing — - # byte-wise, because this endpoint is exempt from the origin gate and is - # therefore reachable on the direct `run.app` URL, where a non-ASCII header - # would otherwise turn a cheap 401 into a logged 500 (api/secret_compare.py). + # byte-wise, because a non-ASCII header would otherwise turn a cheap 401 + # into a logged 500 (api/secret_compare.py). This endpoint was exempt from + # the origin gate while `sync-postgres.yml` had no way to send the edge's + # header on the direct `run.app` URL; it now sends it, the exemption is gone, + # and this token is the second lock behind the gate rather than the only one. if not secret_matches(x_cache_token, expected): raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid cache token") diff --git a/core/config.py b/core/config.py index 3e9d5c50f4..03f3f7caf4 100644 --- a/core/config.py +++ b/core/config.py @@ -175,11 +175,13 @@ 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. 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 + Local dev and the test suite never set it. Exactly ONE path stays exempt + when it is set — `/health`, which the deploy smoke reaches on the + candidate's `run.app` tag URL. `/debug/cache/invalidate` was the second + until `sync-postgres.yml` learned to send the header itself, and the + prerendered `/seo-proxy/…` pages never were, 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.""" + for why the one has to be, and why the other two are not.""" @field_validator( "database_url", diff --git a/docs/reference/api.md b/docs/reference/api.md index fee43c52e6..eace38ed1d 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -581,19 +581,36 @@ 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 +**Rotating the secret** means changing every side 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. +rotate the Secret Manager version, the Transform Rule, the Worker binding **and +the `ORIGIN_SECRET` repository secret** in GitHub Actions settings, 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: +The repository secret is the copy that is easiest to forget, because nothing +about it lives in the Google Cloud console: `sync-postgres.yml` sends it as +`X-Origin-Secret` on the cache flush, which goes to the direct `run.app` URL and +therefore never passes the edge. Skip it in a rotation and the sync's last step +starts failing with `Cache invalidation was refused by the origin gate (HTTP +403)` — loudly, by design, but a day after the rotation rather than during it. + +**Exempt paths** — exact matches, no prefixes, and only this one: | Path | Why | |---|---| | `/health` | the deploy smoke probes the candidate revision on its `run.app` tag URL, which never passes the edge | -| `/debug/cache/invalidate` | `sync-postgres.yml` posts here from a GitHub runner over the direct URL, because Cloudflare's bot challenge answers an unauthenticated curl POST with a 403 HTML page. The endpoint has its own token (`CACHE_INVALIDATE_TOKEN`, constant-time compared, 503 when unconfigured) | + +`/debug/cache/invalidate` was the second until `sync-postgres.yml` learned to +send `X-Origin-Secret` itself, out of the `ORIGIN_SECRET` repository secret. It +still posts to the direct `run.app` URL — Cloudflare's bot challenge answers an +unauthenticated curl POST against `api.anyplot.ai` with a 403 HTML page — but it +now arrives carrying the header the edge would have stamped, so it needs no hole +in the gate. Its own `CACHE_INVALIDATE_TOKEN` (constant-time compared, 503 when +unconfigured) is the second lock behind the first. If the repository secret goes +missing, that step fails with a message naming it rather than letting the flush +go quietly stale. `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 diff --git a/tests/unit/api/test_origin_gate.py b/tests/unit/api/test_origin_gate.py index 80e10544d0..787b67a1b9 100644 --- a/tests/unit/api/test_origin_gate.py +++ b/tests/unit/api/test_origin_gate.py @@ -114,13 +114,9 @@ def test_it_answers_before_the_admin_credential_is_looked_at(self, client: TestC class TestTheExemptPaths: - """Two paths, no prefixes. `/health` is how the deploy smoke reaches the + """One path, 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.""" + passes the edge, so gating it would make every deploy fail closed.""" def test_the_gate_is_really_armed_for_this_test(self, client: TestClient, armed): assert client.get(OPEN_PATH).status_code == 403 @@ -128,10 +124,18 @@ 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_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_cache_flush_is_gated_like_everything_else(self, client: TestClient, armed): + """It was the second exemption until `sync-postgres.yml` learned to send + the header itself. An exempt path is one anybody may POST to from + anywhere with only `CACHE_INVALIDATE_TOKEN` behind it; a caller that + carries the origin secret needs no hole in the gate.""" + assert client.post("/debug/cache/invalidate").status_code == 403 + + def test_the_cache_flush_still_answers_the_caller_that_carries_the_header(self, client: TestClient, armed): + """403 above is the gate; 503 here is the endpoint's OWN fail-closed + lock with no CACHE_INVALIDATE_TOKEN configured — reached, not pre-empted. + The pair is what proves the flush is gated twice rather than moved.""" + assert client.post("/debug/cache/invalidate", headers=EDGE).status_code == 503 def test_the_prerendered_pages_ARE_gated(self, client: TestClient, armed): """Deliberately not exempt (Copilot review): the site's nginx fetches @@ -145,7 +149,7 @@ def test_the_prerendered_pages_ARE_gated(self, client: TestClient, armed): ("path", "exempt"), [ ("/health", True), - ("/debug/cache/invalidate", True), + ("/debug/cache/invalidate", False), ("/seo-proxy", False), ("/seo-proxy/", False), ("/seo-proxy/specs", False), @@ -155,7 +159,7 @@ def test_the_prerendered_pages_ARE_gated(self, client: TestClient, armed): ("/specs", False), ], ) - def test_the_exemption_list_is_exactly_these_two_paths(self, path, exempt): + def test_the_exemption_list_is_exactly_this_one_path(self, path, exempt): assert is_exempt(path, "GET") is exempt @@ -287,11 +291,12 @@ def test_a_missing_side_never_matches(self, presented, expected): class TestTheOtherHeaderSecretsUseTheSameComparator: - """The gate exempts `/debug/cache/invalidate` on the grounds that it has its - own lock — so that lock has to be as cheap to fail as the gate is. It used - the raw `str` comparison, and it is the one endpoint reachable on the direct - `run.app` URL, so a non-ASCII `X-Cache-Token` turned a 401 into a logged 500 - (Copilot review). `X-Admin-Token` had the same comparison.""" + """`/debug/cache/invalidate` has its own lock behind the gate — so that lock + has to be as cheap to fail as the gate is. It used the raw `str` comparison, + and it was for a while the one endpoint reachable on the direct `run.app` + URL, so a non-ASCII `X-Cache-Token` turned a 401 into a logged 500 (Copilot + review). `X-Admin-Token` had the same comparison. These run with the gate + unarmed, which is the state of local development and of the suite at large.""" RAW = b"tok\xe9n"