From 5b6a3c1eaae4419a20e2cf247f7ffb3e555fdcec Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:17:55 +0200 Subject: [PATCH 1/3] security(gate): the cache flush carries the edge's header instead of being waved through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /debug/cache/invalidate was exempt from the origin gate because sync-postgres.yml has no front door: 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 EXEMPT_PATHS holds exactly one entry — /health, which the deploy smoke needs on the candidate's tag URL. An exempt path is one anybody may POST to from anywhere with only CACHE_INVALIDATE_TOKEN behind it; a caller that carries the header needs no hole at all. The suite pins both locks: 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. The second door the gate still does not close — a crawler UA reaching the prerendered pages through the APP service's raw run.app URL — is now measured rather than suspected, and 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 its exception 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. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3 --- .github/workflows/sync-postgres.yml | 19 +++++++ CHANGELOG.md | 24 +++++++++ api/origin_gate.py | 77 +++++++++++++++++++---------- docs/reference/api.md | 13 ++++- tests/unit/api/test_origin_gate.py | 39 ++++++++------- 5 files changed, 127 insertions(+), 45 deletions(-) diff --git a/.github/workflows/sync-postgres.yml b/.github/workflows/sync-postgres.yml index 916a3abb016..941d8a49f5d 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 0475c20e969..a2920d403c5 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. (#PRNUM) + - **`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 423b5847267..4ce6c416e18 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/docs/reference/api.md b/docs/reference/api.md index fee43c52e65..c2c1b73bf47 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -588,12 +588,21 @@ 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: +**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 80e10544d05..787b67a1b97 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" From 44de783dbbf408057ac45aee89d6132ac690f1d4 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:18:38 +0200 Subject: [PATCH 2/3] docs(changelog): PR reference Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2920d403c5..a08647385d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -346,7 +346,7 @@ aggregate instead: an italic *Catalog* line at the end of the version section an 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. (#PRNUM) + 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 From 97dab5bc93b066fc9d65d4bd9b0c74447b34402a Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:39:59 +0200 Subject: [PATCH 3/3] docs(gate): bring the two stale exemption notes and the rotation steps along MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot: core/config.py still said exactly two paths stay exempt including this endpoint, api/routers/debug.py still said the cache flush is exempt and therefore directly reachable, and the rotation procedure named the Secret Manager version, the Transform Rule and the Worker binding but not the new repository copy — after which the workflow keeps sending the old value. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3 --- api/routers/debug.py | 8 +++++--- core/config.py | 10 ++++++---- docs/reference/api.md | 18 +++++++++++++----- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/api/routers/debug.py b/api/routers/debug.py index 23aa37bccfd..f9417c5fb3f 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 3e9d5c50f4d..03f3f7caf4b 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 c2c1b73bf47..eace38ed1dc 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -581,12 +581,20 @@ 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. + +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: