Skip to content

Close the direct run.app door with a shared-secret origin gate - #11208

Merged
MarkusNeusinger merged 15 commits into
mainfrom
feat/origin-secret-gate
Sep 2, 2026
Merged

MarkusNeusinger merged 15 commits into
mainfrom
feat/origin-secret-gate

Conversation

@MarkusNeusinger

@MarkusNeusinger MarkusNeusinger commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • api/origin_gate.py requires the header the Cloudflare edge stamps, and refuses anything else with 403 before the request costs anything. The API stands on Cloud Run with ingress=all, so it answers on two addresses — api.anyplot.ai behind Cloudflare, and the raw *.run.app URL in front of nothing. Every edge measure (bot challenge, WAF, the cache that makes the max-age=300 reads free) was one URL away from being bypassed; api/request_context.py already documented callers doing it.
  • Unset means off. Nothing changes until ORIGIN_SECRET is set on the service, which is also the rollback. Local dev and the test suite never see the gate, so this can merge and deploy long before the Cloudflare rule or the secret exist.
  • /health reports origin_gateoff · off-seen · ok · missing · mismatch — for the request it was asked with, never the value. That is what turns the rollout into a measurement instead of a leap.
  • The apex Worker's source moves into infra/cloudflare/, because a Worker subrequest to a host in the same zone bypasses that zone's Transform Rules. It now stamps the header itself, deleting any inbound one first.
  • Transferred from the sibling repo kurrentschrift (PRs [line-basic] bokeh implementation #493, [box-basic] highcharts implementation #495), where this shipped and was measured live.
  • Known residual, deliberately out of scope: the app service also stands with ingress=all, and its nginx relays a crawler user agent through @seo_proxy to api.anyplot.ai, where the edge stamps the header legitimately — so the prerendered render stays reachable via the app's raw run.app URL. That is a second door on a second service (the request the API sees really did pass the edge), and closing it means gating anyplot-app or refusing to proxy for run.app hosts, which bot-serving-check.yml probes nightly. Named in api/origin_gate.py and docs/reference/api.md; own PR.

What is exempt, and why each one has to be

Exact paths, no prefixes:

Path Why
/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 makes every deploy fail closed
/debug/cache/invalidate anyplot-specific. sync-postgres.yml posts here from a GitHub runner over 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, 503 when unconfigured), so it is gated, just by a different lock
OPTIONS a browser cannot attach a custom header to a preflight

/seo-proxy/… is deliberately NOT exempt, though the sibling repo exempts it belt-and-braces. Copilot was right that it would be a real hole: those handlers query SpecRepository/ImplRepository on a cache miss or an unknown id, and any request with a recognized crawler user agent schedules an outbound Plausible event — so an exemption would leave the API's most expensive reads open on the direct URL, which is the cost this gate exists to refuse. The site's nginx already fetches those pages over https://api.anyplot.ai, so the path carries the header; step (b) of the rollout validates it end to end with a crawler user agent before anything is armed, and bot-serving-check.yml runs daily, so being wrong here is loud rather than silent.

The direct paths I checked (this is the part that does not transfer, and had to be re-derived for this repo):

  • api.anyplot.ai — SPA (VITE_API_URL), MCP clients, OG cards embedded cross-origin, and the site's nginx for @seo_proxy, @seo_proxy_python, /llms-full.txt, /sitemap.xml. All through the edge → the Transform Rule stamps them.
  • anyplot.ai/api/* — the Worker. Same-zone subrequest, so it must stamp for itself. Its /api/event Plausible passthrough is preserved untouched.
  • anyplot-api-…run.app — the Cloud Build smoke (now sends the header) and sync-postgres.yml (exempt path, above).
  • anyplot-app-…run.appbot-serving-check.yml hits the app origin, whose nginx then goes out through api.anyplot.ai. Unaffected.

Every header secret goes through one byte-wise comparator

secrets.compare_digest raises TypeError when either str holds a non-ASCII character, and a header value reaches the application latin-1-decoded straight from the wire. Comparing strings handed any unauthenticated caller a one-byte way to turn a cheap 401 or 403 into an unhandled, logged 500 (Copilot).

That was true of the gate — and of X-Admin-Token and X-Cache-Token, which matters more: /debug/cache/invalidate is exempt from the gate on the grounds that it has its own lock, and it is the one endpoint reachable on the direct run.app URL. So the fix is one comparator in api/secret_compare.py, used by all three call sites, rather than three separate patches: a comparator that is correct in two places out of three is exactly what nobody notices. It also refuses when either side is missing, so an unconfigured secret can never be satisfied by an absent header.

Pinned by tests including one that asserts the str comparison this replaced does raise on the same input, so the others cannot quietly stop measuring anything.

Two changes beyond the gate itself

The deploy step configures the revision additively — --update-secrets and --update-env-vars. Both --set- forms replace their whole set, so anything attached to the service out of band is stripped from every revision the pipeline creates. ORIGIN_SECRET is exactly that kind of binding — attached by hand to arm, removed by hand to roll back — and a secret-backed variable lives in the same revision environment as a literal one, so either flag was a way to silently disarm the gate on the next deploy (the second half found by Copilot, after the first fix). It cannot simply be listed in the flags instead: Cloud Run refuses a deploy naming a secret that does not exist, which would break every build until step (c) below. Two flags in the deploy step; everything else in api/cloudbuild.yaml is confined to the smoke step.

The analytics middleware moves inside CORSMiddleware. The gate has to be inside CORS (so its 403 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, unthrottled, at a third-party endpoint). In this repo the counter sat outside CORS, which makes those two mutually exclusive; moving it in resolves it. 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.

The only behavioural consequence of the move: a CORS preflight no longer reaches the counter. Preflights carry the browser's user agent, and both tracking functions return early unless detect_ai_agent classifies the UA, so nothing that was being counted stops being counted.

Rollout — in this order, and measured at each step

(a) Merge and deploy with the check off. Nothing to configure; ORIGIN_SECRET does not exist yet, gate_is_armed() is false, every path behaves exactly as today. Confirm with:

curl -s https://api.anyplot.ai/health   # expect "origin_gate":"off"

(b) Put the Transform Rule live and give the Worker its binding — then measure off-seen on EVERY path. Cloudflare dashboard: Rules → Transform Rules → Modify Request Header → set static X-Origin-Secret for http.host eq "api.anyplot.ai". Then deploy infra/cloudflare/anyplot-api-proxy.js with the ORIGIN_SECRET secret binding (procedure in infra/cloudflare/README.md). The gate is still off, so nothing can break; what this step buys is the evidence:

curl -s https://api.anyplot.ai/health   # must read "off-seen"
curl -s https://anyplot.ai/api/health   # must read "off-seen"  ← the Worker; this is the one that reads "off" if the binding is missing
curl -s https://<api-run-url>/health    # must stay "off" — that is the door being closed
# nginx rides on the first line's verdict; probe it end to end:
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

Do not proceed while any path that must keep working still reads off.

(c) Create the secret and arm the service. Both the Cloud Run runtime and the Cloud Build trigger use the same identity, 239660669828-compute@developer.gserviceaccount.com, so one grant covers the service and the smoke step's read.

# Never `echo` — it appends a newline. (The config strips whitespace as a
# second net, but the value should be right in the first place.)
gcloud secrets create ORIGIN_SECRET --project=anyplot --replication-policy=automatic
printf %s "<value>" | gcloud secrets versions add ORIGIN_SECRET --project=anyplot --data-file=-

gcloud secrets add-iam-policy-binding ORIGIN_SECRET --project=anyplot \
  --member="serviceAccount:239660669828-compute@developer.gserviceaccount.com" \
  --role="roles/secretmanager.secretAccessor"

# Same value into the Cloudflare Transform Rule and the Worker binding.

# Same value into the Cloudflare Transform Rule and the Worker binding.

Then arm the service. The full block is in docs/reference/api.md § Origin gate — it lives in the repository rather than in this description, because a procedure that exists only in a PR body is one nobody finds at 2 a.m. Three things in it are not obvious, each from a Copilot round:

  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 pipeline deliberately leaves each build's smoked-but-unpromoted candidate as latest — so arming during a deploy would ship that build's image along with the gate, and naming the new revision precisely does not change which image it inherits. The block asserts latestReadyRevisionName == the revision serving 100% and stops otherwise.
  2. Pin the secret to a version 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, that shows up as intermittent 403s inside a single revision.
  3. Promote by name, never --to-latest — the same hazard as (1), and the reason api/cloudbuild.yaml refuses that flag.

Rotation gets its own paragraph there: the gate accepts exactly one value, so there is no overlap window. Roll back, rotate both sides, arm again on the new version number.

(d) Verify.

curl -s https://api.anyplot.ai/health   # "ok"
curl -s https://anyplot.ai/api/health   # "ok"
curl -s https://<api-run-url>/health    # "missing"  ← the door is now shut
curl -s -o /dev/null -w '%{http_code}\n' https://<api-run-url>/libraries   # 403
curl -s -o /dev/null -w '%{http_code}\n' https://api.anyplot.ai/libraries  # 200

Then walk the site once (gallery, a spec page, the stats page), fetch an OG card cross-origin, and let one sync-postgres run finish — its cache flush must still return 200.

(e) Rollback — one variable, no code change:

The same block as arming, with --remove-secrets=ORIGIN_SECRET in place of --update-secrets and a disarm- suffix — including the in-flight-candidate guard, which matters more here than when arming.

Removing the Worker binding is not a rollback: while the service is armed, that takes anyplot.ai/api/* down instead of freeing it. Roll back on the API side, always.

Follow-up this PR deliberately leaves open

/debug/cache/invalidate is exempt because sync-postgres.yml has no front door. The cleaner end state is for that workflow to send X-Origin-Secret from a repository secret, at which point the exemption can go. That needs a GitHub Actions secret plus a change to .github/workflows/sync-postgres.yml, which is out of this PR's scope — noted here so it is not lost.

Test plan

  • tests/unit/api/test_origin_gate.py — 57 tests: dormant by default (including with a wrong header), the armed gate across five header shapes and six methods, the exemption list both as live requests and as assertions on the list itself (including that /seo-proxy/… is refused), preflight and CORS-headers-on-the-403, the analytics middleware never firing on a refusal — for an asset path and a crawler page — all five /health verdicts, the non-ASCII header on all three secrets, and the trailing-newline strip on ORIGIN_SECRET and the other Secret-Manager-backed values.
  • uv run pytest tests/unit — 1818 passed, 1 skipped (pre-existing local skip: MonoLisa italic not cached).
  • uv run ruff check . / ruff format --check . — clean.
  • uv run --extra typecheck mypy api core — no issues in 37 source files.
  • api/cloudbuild.yaml parses; step ids unchanged (build-image, push-image, push-latest, deploy, smoke, promote, get-url).
  • Not verifiable before merge: the Cloud Build smoke's new lines and the Cloudflare side. The smoke is written so that a missing secret or a missing permission yields an empty value and bare probes — correct while the gate is off, loud at /libraries once it is on — and it accepts off/off-seen, so it cannot take the deploy pipeline down during the rollout or after a rollback.

Checklist

  • CHANGELOG.md updated under [Unreleased] — one ### Added entry for the gate, two ### Changed entries for the deploy flag and the middleware order.
  • Docs updated: docs/reference/api.md (new "Origin gate" section, /health response), docs/development.md (env table), docs/reference/repository.md and agentic/docs/project-guide.md (both repository maps get infra/), .env.example, and infra/cloudflare/README.md for the Worker and the measuring procedure.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI balanced review requested due to automatic review settings September 2, 2026 21:16
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The security-sensitive cross-system rollout retains a DB-backed direct-origin bypass and needs configuration hardening and human operational review.

Pull request overview

Adds a Cloudflare-stamped shared-secret gate to prevent direct Cloud Run API access.

Changes:

  • Adds the origin-gate middleware, health diagnostics, configuration, and tests.
  • Updates Cloud Run deployment and Cloudflare Worker behavior.
  • Documents rollout, rollback, and infrastructure changes.
File summaries
File Description
.env.example Documents ORIGIN_SECRET.
CHANGELOG.md Records security and deployment changes.
api/cloudbuild.yaml Preserves and supplies the origin secret.
api/main.py Registers and orders the middleware.
api/origin_gate.py Implements the shared-secret gate.
api/routers/health.py Reports the gate verdict.
core/config.py Adds and normalizes secret configuration.
docs/development.md Documents the environment variable.
docs/reference/api.md Documents gate behavior and exemptions.
docs/reference/repository.md Adds the infrastructure directory.
infra/cloudflare/README.md Documents Worker deployment and rollout.
infra/cloudflare/anyplot-api-proxy.js Stamps the secret on Worker subrequests.
tests/unit/api/test_origin_gate.py Tests gate states, exemptions, and ordering.
Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 4
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread api/origin_gate.py Outdated
Comment thread core/config.py
Comment thread docs/reference/api.md Outdated
Comment thread docs/reference/repository.md
Copilot AI review requested due to automatic review settings September 2, 2026 21:21
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The SEO exemption retains a costly direct-origin bypass, and the documented Worker deployment exposes its API token through process arguments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

api/origin_gate.py:75

  • The /seo-proxy exemption keeps a substantial direct-origin bypass open. A caller can hit arbitrary /seo-proxy/{spec_id} URLs on run.app, causing database lookups (api/routers/seo.py:1288-1296), and a spoofed crawler user agent also creates an outbound Plausible request (api/main.py:213-215, api/analytics.py:400-414) without passing Cloudflare's WAF or cache. This recreates the denial-of-wallet path the gate is intended to remove; require the origin header for this prefix after validating the nginx route during rollout, rather than exempting the entire SEO surface.
EXEMPT_PATHS = frozenset({"/health", "/seo-proxy", "/debug/cache/invalidate"})
EXEMPT_PREFIXES = ("/seo-proxy/",)

docs/reference/repository.md:137

  • Adding a top-level infra/ directory here leaves the corresponding directory structure in agentic/docs/project-guide.md:186-206 stale. The repository documentation contract requires structural project changes to update that agent guide too, so add infra/ and its Cloudflare role there in this PR.
├── infra/                             # Infrastructure that lives outside the code
│   └── cloudflare/                    # Source for the apex Worker + its rollout notes
  • Files reviewed: 13/13 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread infra/cloudflare/README.md Outdated
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 21:31
Copilot review: the Worker-deploy recipe passed the credential as
-H "Authorization: Bearer $CF_API_TOKEN", which publishes it in
/proc/<pid>/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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes production ingress security and manual Cloudflare and Cloud Run rollout behavior that cannot be fully validated from the repository alone.

Review details

Suppressed comments (1)

api/origin_gate.py:163

  • Once this middleware is armed, the existing break-glass contract in api/routers/debug.py:75-77 is incomplete: X-Admin-Token alone on the direct Cloud Run URL now receives this 403, and the new test confirms that both headers are required. Update that operator-facing guidance to require X-Origin-Secret as well, otherwise incident responders are directed to a path guaranteed to fail.
    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)
  • Files reviewed: 14/14 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread api/origin_gate.py
Comment thread core/config.py Outdated
Copilot AI review requested due to automatic review settings September 2, 2026 21:40
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The security-sensitive cross-system rollout has unresolved secret-normalization and exempt-endpoint robustness issues.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

core/config.py:211

  • This validator changes the canonical ORIGIN_SECRET, but the new stampers use the raw Secret Manager value (api/cloudbuild.yaml:137-139 and infra/cloudflare/README.md:96-106). A space-padded value explicitly accepted by the new tests therefore makes the Worker/edge send a different value and makes the smoke report mismatch. Either reject surrounding whitespace here or normalize the secret identically before every header is stamped.

core/config.py:180

  • This docstring still says /seo-proxy/… is exempt, contradicting EXEMPT_PATHS and the updated API documentation. Remove it from the list so future maintainers do not reintroduce the bypass that this revision intentionally closed.
    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
  • Files reviewed: 14/14 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread api/origin_gate.py
Copilot AI review requested due to automatic review settings September 2, 2026 21:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The deployment can still remove ORIGIN_SECRET, and two malformed-secret paths can cause production failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

infra/cloudflare/anyplot-api-proxy.js:24

  • origin_secret accepts arbitrary Unicode, but the Workers Headers.set API converts values to ByteString and throws for code points above U+00FF. A CJK or emoji secret that Cloud Run accepts will therefore make every anyplot.ai/api/* request fail in the Worker before reaching the API. Restrict and document the secret as a strong ASCII token (with a generation command), or apply one consistent transport encoding in the Worker and API.
    infra/cloudflare/README.md:123
  • Use the repository documentation's American spelling: “authorizes.”

api/origin_gate.py:80

  • This exemption leaves the same malformed-header failure that _matches avoids for the new gate. invalidate_cache still compares Starlette's Latin-1-decoded X-Cache-Token as str (api/routers/debug.py:487-492), so a raw byte ≥ 0x80 raises TypeError; because this exact path bypasses the origin gate, an unauthenticated direct-origin caller can force a logged 500 on every request. Compare bytes there too and add the corresponding raw-header regression test before relying on this endpoint as the alternate lock.
EXEMPT_PATHS = frozenset({"/health", "/debug/cache/invalidate"})
  • Files reviewed: 14/14 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread api/cloudbuild.yaml Outdated
Merge resolves the CHANGELOG conflict by keeping both Added entries (#11205
landed on main while this branch was open).

Copilot review, two findings.

1. /debug/cache/invalidate is exempt from the gate on the grounds that it has
   its own lock - so that lock has to be as cheap to fail as the gate is, and
   it was not: it compared the token as str, and it is the one endpoint
   reachable on the direct run.app URL, so a non-ASCII X-Cache-Token turned a
   401 into an unhandled, logged 500. X-Admin-Token had the same comparison.
   Rather than fixing three call sites separately, the byte-wise compare moves
   into api/secret_compare.py and all three use it; a comparator that is
   correct in two places out of three is exactly what nobody notices.

2. --set-env-vars is destructive over the revision environment, where a
   secret-backed variable lives alongside a literal one - so it was a second
   way for a pipeline deploy to drop the hand-attached ORIGIN_SECRET and
   disarm the gate, undoing what --update-secrets had just bought. Both flags
   are additive now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 21:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The security boundary depends on coordinated, live Cloudflare and Cloud Run configuration that cannot be verified from the repository alone.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

CHANGELOG only; both sides add entries under Added and both are kept.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 22:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The security-sensitive rollout can promote an unrelated unsmoked revision through its current --to-latest commands.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread api/origin_gate.py
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The app origin still relays gated requests, and the documented arm procedure can fail open or race a concurrent deployment.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

docs/reference/api.md:498

  • This safety check fails open if gcloud run services describe or the JSON parser fails. The here-string still gives read an empty line, so both variables become empty and test "" = "" succeeds; this block has no fail-fast wrapper. The subsequent update can then clone the candidate that this check was meant to exclude. Capture and validate the command status/output (or run the full procedure in a fail-fast subshell) before comparing nonempty revision names.
    docs/reference/api.md:518
  • The earlier equality check is subject to a deployment race: a Cloud Build can create a new no-traffic candidate after line 498 but before this command, and services update will then clone that new latest template. Naming the arm revision does not prevent it from inheriting and promoting the unsmoked image. Pin the update to the image digest captured from SERVING, introduce mutual exclusion with deployments, or verify the created revision against the captured serving revision before shifting traffic.
    api/origin_gate.py:146
  • Both new middleware methods are untyped, contrary to the repository requirement that all functions include type hints. Annotate the constructor with ASGIApp and -> None, and the call hook with Starlette's Scope, Receive, Send, and -> None; this also lets mypy check the raw ASGI boundary.
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread api/origin_gate.py
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 22:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The blanket OPTIONS exemption permits direct-origin analytics traffic, and the rollback procedure can become unavailable after a failed or concurrent deployment.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

api/origin_gate.py:108

  • The blanket OPTIONS exemption admits more than CORS preflights. CORSMiddleware only consumes an OPTIONS request when it includes Origin and Access-Control-Request-Method; a bare direct-origin OPTIONS /seo-proxy/legal reaches this branch, passes the gate, and then record_bot_fetch emits a Plausible event for a crawler user agent (api/main.py:211-215). This recreates the unthrottled third-party cost the middleware ordering is intended to prevent. Since CORS is already outside the gate, remove the method-wide exemption, or restrict it to an actual preflight by checking the required headers.
  • Files reviewed: 16/16 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread docs/reference/api.md Outdated
Comment thread api/routers/debug.py
…th 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 22:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The rollout can race Cloud Build promotion, lacks fail-fast execution, and exposes the secret through a child-process environment.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 3
  • Review effort level: Balanced

Comment thread docs/reference/api.md
Comment thread infra/cloudflare/README.md Outdated
Comment thread docs/reference/api.md
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/<pid>/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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 22:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The rollout remains vulnerable to a concurrent deployment race, and the rollback can fail when no secret version is enabled.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

docs/reference/api.md:502

  • This preflight is still subject to a deploy race. A build can start after this check, deploy its candidate from the pre-arm template before the arm-* update, pass smoke because off is accepted, and promote that secretless candidate after step 4 has already checked the service. The same race can re-arm a rollback. Serialize the service mutation against the deploy trigger, or make promotion reject a candidate whose gate state differs from the intended service state; the one-time build check and post-check cannot prevent a later promotion.
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
}
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread docs/reference/api.md Outdated
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 22:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Secret strength and header-safe encoding remain unenforced, and the PR description conflicts with the final runbook.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

docs/reference/api.md:523

  • The PR description still says arming stops when SERVING != LATEST and that rollback uses the same in-flight-build guard. This procedure now deliberately warns and pins the serving image instead, while the rollback block explicitly has no build guard or secret lookup. Update the PR body to match these final procedures so operators do not rely on the obsolete stop condition or rollback sequence.

core/config.py:170

  • origin_secret still accepts any nonempty string. That leaves two concrete failure modes: a short value is brute-forceable through the exempt /health ok/mismatch oracle, and a value containing characters above U+00FF makes the Worker's Headers.set() throw because HTTP header values are ByteStrings. Encoding inside the API only fixes compare_digest's exception; it cannot make arbitrary Unicode transport-safe. Generate a high-entropy ASCII value (for example, 32 random bytes encoded as hex/base64), validate its format/minimum length here, and update the rollout and tests accordingly.
    origin_secret: str | None = None
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

CHANGELOG only, resolved as a union: every bullet from both sides kept, main's
own order untouched. Added gains this branch's origin-gate entry above the
agent-instruction pin; Changed keeps the two gate entries alongside the Node
pin; the new Security section is carried over unchanged.

The agent-instruction pin test that arrived with #11210 passes against this
branch, including the infra/ entries this PR adds to both repository maps.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
Copilot AI review requested due to automatic review settings September 2, 2026 22:58
@MarkusNeusinger
MarkusNeusinger merged commit 78669a9 into main Sep 2, 2026
11 checks passed
@MarkusNeusinger
MarkusNeusinger deleted the feat/origin-secret-gate branch September 2, 2026 23:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The runbooks do not enforce their final verification states, and the checked-in setup procedure is incomplete.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment thread docs/reference/api.md
# 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"
Comment thread docs/reference/api.md
--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"
Comment thread api/origin_gate.py
Comment on lines +14 to +16
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.
Comment thread docs/reference/api.md
Comment on lines +480 to +484
**Arming and rolling back** are the same procedure with one flag changed. Several
things make it more than two commands, and each of them has bitten a comparable
rollout somewhere. The whole block runs in a fail-fast subshell: half of these
commands feed the next one, so continuing after a failed lookup would mutate the
service from an empty variable and still print a plausible-looking health line
MarkusNeusinger added a commit that referenced this pull request Sep 3, 2026
…aved through (#11214)

> **Before merging, one thing has to exist:** a repository secret
**`ORIGIN_SECRET`**, set to the same value as the `ORIGIN_SECRET` on the
`anyplot-api` Cloud Run service. Without it the sync's cache-flush step
fails with a message naming exactly that. The gate is already armed in
production (`/health` reports `origin_gate: "ok"`), so this is not a
dormant path.

Two loose ends from #11208.

## 1. The cache flush stops being an exemption

`/debug/cache/invalidate` was exempt from the origin gate because
`sync-postgres.yml` had 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 stamps `X-Origin-Secret` itself. `EXEMPT_PATHS` is down
to one entry — `/health`, which the deploy smoke needs on the
candidate's tag URL, and which is therefore structural rather than a
convenience.

Worth stating plainly why this is the better shape rather than a lateral
move: **an exempt path is one anybody may POST to from anywhere**, with
only `CACHE_INVALIDATE_TOKEN` standing behind it. A caller that carries
the header needs no hole in the gate at all, and the two locks then sit
in series. The suite pins both ends:

- `POST /debug/cache/invalidate` with the gate armed and no header →
**403** (the gate)
- the same with the header → **503** (the endpoint's own fail-closed
answer, with no `CACHE_INVALIDATE_TOKEN` configured) — reached, not
pre-empted

A missing repository secret is a named failure, not a silent one: the
step prints `Cache invalidation was refused by the origin gate (HTTP
403). Set the ORIGIN_SECRET repository secret …` and exits 1, rather
than letting the cache go quietly stale. An absent secret sends no
header at all rather than an empty one, so the log reads "missing
secret" instead of "rotation mismatch".

`docs/reference/api.md`'s exempt-path table and `api/origin_gate.py`'s
docstring both move with it.

## 2. The app origin — measured, and the design that would close it

The gate protects the API service's own door. The APP service also
stands with `ingress=all`, and its nginx relays a crawler user agent
through `@seo_proxy` to `api.anyplot.ai`, where the edge stamps the
header legitimately. #11208 described this; it is now measured:

```
$ curl -A "…Googlebot/2.1…" https://anyplot-app-….run.app/scatter-basic
HTTP 200
<link rel="canonical" href="https://anyplot.ai/scatter-basic" />
```

The prerendered page, its DB queries and its Plausible event, without
the caller having passed the edge.

**This PR does not close it**, and the reason is worth more than the
attempt would have been. Two facts came out of probing it, and both are
now recorded in `api/origin_gate.py`:

- **A `Host` rule would be a real boundary, not theatre.** The obvious
objection is that anyone could send `Host: anyplot.ai` to the `run.app`
URL and walk 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:
  ```
$ curl -H "Host: anyplot.ai" https://anyplot-app-….run.app/scatter-basic
  HTTP 404   <title>Error 404 (Not Found)!!1</title>
  ```
So on that origin `$host` is always the `run.app` name, and
`app/nginx.conf` could refuse `@seo_proxy` for it.

- **That alone breaks `bot-serving-check.yml`**, which probes exactly
this origin with crawler UAs — deliberately, because Cloudflare 403s
GitHub-runner IPs even for a UA-spoofed Googlebot — and cannot spoof the
Host either. An exception keyed on a UA or a header value 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 covers
`api.anyplot.ai` only — without it an enforcing config locks out every
human visitor), and hand the workflow the same secret.

Four coordinated changes, two of them in the dashboard, one able to take
the whole site down if it lands out of order — and no local nginx here
to test any of it against. **Owner call**, filed where the next person
to ask will look.

## Verification

`pytest tests/unit/api` — 761 passed. `ruff check`, `ruff format
--check`, `mypy api core` clean. The workflow's YAML parses and its
cache-flush step's script passes `bash -n` after extraction. The two
curl probes above were run against the live origins.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
MarkusNeusinger added a commit that referenced this pull request Sep 3, 2026
…pped the CSP hardening (#11213)

The brief was "swap `script-src 'unsafe-inline'` for sha256 hashes, then
walk the public pages with the console open". The walk is what changed
the answer, so the measurement comes first.

## The measurement: hash-only `script-src` breaks Cloudflare, not the
SPA

`app/index.html` has three executable inline scripts (theme resolver,
Eruda loader, Plausible stub) and three JSON-LD data blocks that need no
hash. `yarn build` was verified to copy the three through byte-for-byte,
so hashing the source file describes what nginx serves.

The candidate policy was then mounted over the **live production
bundle** — a local proxy that mirrors `https://anyplot.ai` and re-stamps
the response with each header set — and loaded twice in Chrome:

| `script-src` | Cloudflare's injected script | console |
|---|---|---|
| `'self' 'unsafe-inline' cdn.jsdelivr.net` (today) | runs — its hidden
iframe is in the DOM | clean of CSP |
| `'self' <3 × sha256> cdn.jsdelivr.net` | **blocked** — no iframe |
`Executing inline script violates the following Content Security Policy
directive … The action has been blocked.` |

Exactly one script is blocked, and it is not ours. **Cloudflare
JavaScript Detections** injects an inline script into every HTML
response at the edge, after nginx:

```js
window.__CF$cv$params={r:'a357b436fa1aa625',t:'MTc4ODQ2OTQzNQ=='};…
```

The ray id and timestamp change per response, so its hash does too — it
can never be listed. Confirmed the same script and iframe on
`https://anyplot.ai` itself, so this is production behaviour, not a
proxy artefact.

Shipping the hardened policy would have silently degraded bot detection
on a site whose brand-new origin gate (#11208) explicitly leans on the
edge. So it is **not shipped**, and — equally deliberately — the hashes
are not added *next to* `'unsafe-inline'` either: a browser ignores
`'unsafe-inline'` the moment a hash appears, so that combination is the
identical breakage wearing a stricter-looking label. A test now forbids
it.

**The way out is a nonce, not a hash.** Cloudflare
[documents](https://developers.cloudflare.com/bots/additional-configurations/javascript-detections/)
that it parses this response header and stamps its injected script with
the nonce it finds, and recommends that over `'unsafe-inline'`. That
needs nginx to mint one per request and rewrite index.html's `<script>`
tags (`sub_filter` plus `gzip_static off` for the shell) — a delivery
change no test in this repo can prove and no local nginx here can run.
**Owner call**, and the alternative (turning JavaScript Detections off
in the zone) is a security trade rather than a fix. The reasoning, the
numbers and the three hashes sit in `security-headers.conf` at the
directive they explain, so the switch is a one-line edit whenever one of
the two happens.

## What does ship

**The API host stamps its own baseline headers.** `api.anyplot.ai` is a
separate origin with no nginx in front of it, so it inherited none of
`app/security-headers.conf` — only `/proxy/html` set `nosniff` and a
`Referrer-Policy`, by hand, on that one response. An outermost
middleware now `setdefault`s both on every response, including CORS
preflights, the origin gate's 403 and the exception handlers' 500s.
Deliberately **not** `X-Frame-Options`: the SPA embeds `/proxy/html`
cross-origin in an iframe (`frame-src https://api.anyplot.ai`), and
`SAMEORIGIN` would break every interactive plot preview — the test
asserts its absence so nobody adds it as an obvious-looking improvement.

**Both `/_health` locations stop dropping the site's headers.** They set
an `add_header` of their own, and nginx drops every inherited header in
such a location — the rule stated at the top of `security-headers.conf`,
and the one place in `nginx.conf` that had missed it. Found by the new
test, verified against `origin/main`:

```
locations missing the include on origin/main: ['location /_health {', 'location /_health {']
after the fix: []
```

**`tests/unit/api/test_csp_policy.py`** — six checks over files that
nothing else compiles or imports: the reserve hashes still match
`index.html`; `script-src` never mixes `'unsafe-inline'` with a hash;
`object-src 'none'` and `base-uri 'self'` stay closed; the policy never
carries `report-to` *beside* `report-uri` (measured in the sibling repo:
Chromium then reports nothing at all — neither is set here today, so
this exists for whoever adds reporting); every nginx location with its
own header re-includes the snippet; and the API host's two headers are
present while `X-Frame-Options` is not.

One anyplot-specific trap it had to handle: `index.html` documents its
own Eruda loader with the words `Plain <script> (not type="module")`
**inside an HTML comment**, and a script regex that reads that as a tag
hashes the comment prose instead of the script — silently, with a hash
that looks perfectly plausible. Comments are stripped first.

## Items from the brief that turned out to be no-ops here

- **`report-to` beside `report-uri`** — anyplot's CSP has neither.
Encoded as a test instead of a fix.
- **`bluetooth=()` in the Permissions-Policy** — anyplot sends no
`Permissions-Policy` at all. Nothing to remove. Adding one is a
separate, easy hardening pass; not folded in here.

## Verification

`pytest tests/unit tests/integration` — 1913 passed, 1 skipped. `ruff
check`, `ruff format --check` and `mypy api core` clean. The CSP walk
itself is the table above, run against the live bundle in Chrome.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
MarkusNeusinger added a commit that referenced this pull request Sep 4, 2026
…1221)

## Summary

- **The second door has a gate now, and it ships switched off.**
`api/origin_gate.py` closed the API service's door and wrote the app
service's door down as the thing it could not close from its own side:
`anyplot-app` stands with `ingress=all`, serves the whole site from
`https://anyplot-app-r3tvmejsmq-ez.a.run.app` with no bot challenge, no
WAF and no rate limit, and relays any crawler user agent through
`@seo_proxy` to `https://api.anyplot.ai` — where the edge stamps the
API's secret *legitimately*, so the API gate cannot tell.
`app/origin-gate.conf.template` is the nginx half of the same mechanism:
same secret, same five verdicts, `ORIGIN_GATE` unset = off.
- **Nothing is armed by merging this.** The image defaults
`ORIGIN_GATE=off`, the service declares no environment variables at all,
and `/_health` already reports `X-Origin-Gate` — so every route into the
container can be measured before anything is switched on. The runbook is
in `infra/cloudflare/README.md` § "The site's own origin", summarised at
the bottom here.
- **Three callers reach that origin without the edge and each now stamps
its own header**, plus one that turned out to be a live production path
nobody had connected to the gate: the apex Worker's `/api/event`.

## The mechanism, in three sentences

`app/origin-gate.conf.template` renders into
`/etc/nginx/conf.d/00-origin-gate.conf` at container start, via the
entrypoint script `20-envsubst-on-templates.sh` that
`nginxinc/nginx-unprivileged:alpine` already ships — no start script of
ours, and `NGINX_ENVSUBST_FILTER=^ORIGIN_` keeps envsubst away from
`$host` and `$uri`. Seven `map` blocks turn the presented header into
`$origin_gate_status` (`off` · `off-seen` · `ok` · `missing` ·
`mismatch`, the API's own vocabulary) and `$origin_gate_deny`; every
server block in `app/nginx.conf` carries `if ($origin_gate_deny) {
return 421; }` at server level, which runs in the server-rewrite phase
and therefore covers every location the block has and every location
added later. The secret is written in exactly one map key,
`app/nginx.conf` never names `$http_x_origin_secret` at all, and every
`proxy_pass` clears the header so no upstream is ever handed it — three
rules with a test each, plus a CI smoke that greps the refusal page and
the container log for the value.

## The finding the brief asked for: `Host` would have worked, and still
cannot be the mechanism

`api/origin_gate.py` guessed that `$host` at this origin might be the
`run.app` name for *all* traffic, which would have settled the question.
It is not:

| | |
|---|---|
| `gcloud beta run domain-mappings list --region=europe-west4
--project=anyplot` | `anyplot.ai → anyplot-app`, `www.anyplot.ai →
anyplot-app`, `api.anyplot.ai → anyplot-api` |

They are **Cloud Run domain mappings**, so Cloudflare forwards the
original `Host` and `$host` really does distinguish the edge from the
raw URL — and the value cannot be spoofed, because Google's frontend
answers a foreign Host on a `run.app` address with its own 404 before
the container is reached (measured in #11208).

A Host rule still cannot be the gate. `bot-serving-check.yml` probes
this exact origin with crawler user agents *because* Cloudflare 403s
GitHub-runner IPs even for a UA-spoofed Googlebot; it cannot spoof the
Host either, and any exception keyed on something it could present
instead — a header it invents, a user agent — is public with this
repository. The exception has to be the shared secret. Once the workflow
carries the secret, the Host rule buys nothing the header does not, so
it is not built.

## Every hostname this container serves

The Transform Rule has to cover all of them, or arming locks out the
visitors it protects.

| Hostname | Reaches the container via | Transform Rule |
|---|---|---|
| `anyplot.ai` | Cloud Run domain mapping, proxied | **required** |
| `www.anyplot.ai` | Cloud Run domain mapping, proxied — it **serves the
site**, it does not redirect (`curl -sI https://www.anyplot.ai/` → 200,
no `Location`) | **required** |
| `anyplot.ai/api/event` | the apex Worker, `fetch(request)` back to
this origin | none — the **Worker** stamps it (see below) |
| `python.anyplot.ai` | nothing today: it is a `server_name` in
`app/nginx.conf` with **no DNS record and no domain mapping** (`curl` →
`Could not resolve host`, checked 2026-09-04) | add it the day the DNS
record is added; the block is gated already |
| `anyplot-app-r3tvmejsmq-ez.a.run.app` | direct | none — this is the
door being closed |
| `anyplot-app-239660669828.europe-west4.run.app` | direct, the
service's second URL | none, same door |
| `candidate---anyplot-app-r3tvmejsmq-ez.a.run.app` | direct, the
pre-traffic tag URL | none — the smoke sends the header itself |

The rule must be a **Set**, not an Add: a caller supplying its own
`X-Origin-Secret` has to have it replaced.

## Every legitimate direct caller, and what each now sends

| Caller | Now sends | Notes |
|---|---|---|
| `app/cloudbuild.yaml` pre-traffic smoke | `X-Origin-Secret` on every
probe, read from Secret Manager **inside the step** | Not
`availableSecrets`, which resolves at build start and would fail every
build until the secret exists — the same reasoning `api/cloudbuild.yaml`
already carries. It also asks `/_health` for the verdict **before** any
content probe, so a wired-up-wrong secret is reported as itself instead
of as a mystifying 403 on the home page. |
| `.github/workflows/bot-serving-check.yml` | the header from the
`ORIGIN_SECRET` repository secret, on all ~36 probes | Reads `/_health`
first: `missing` and `mismatch` are hard failures with a message naming
the secret; `off` is a warning; an absent header is a warning naming
deploy lag. Without that, an armed gate plus a missing secret would open
an incident saying "every crawler page is broken". |
| the apex Worker, `/api/event` | stamps from its own `ORIGIN_SECRET`
binding | **The one that would have broken production.**
`anyplot.ai/api/event` is the only path under the Worker's route that
goes to the *site's* origin instead of the API host, and a Worker
subrequest to a host in the same zone skips that zone's Transform Rules
— the exact finding `infra/cloudflare/README.md` exists for, biting a
second time. Arming without this answers **every Plausible pageview on
the site** with a 403, quietly. |
| Cloud Run startup probe | nothing, and needs nothing | `gcloud run
services describe` → `startupProbe: tcpSocket: port 8080`,
`failureThreshold 1`. Not an HTTP probe, so no exemption. |
| IndexNow (`indexnow-submit.yml`) + Bing's key verification | nothing,
and needs nothing | Both fetch `https://anyplot.ai/<key>.txt`, i.e.
through the edge. |
| PageSpeed Insights (audit harness) | nothing, and needs nothing |
Audits `https://anyplot.ai/...`, through the edge. |

Also checked and empty: `gcloud monitoring uptime list-configs
--project=anyplot` → `Listed 0 items.` No Lighthouse CI workflow exists.
The `ORIGIN_SECRET` repository secret **already exists** (created
2026-09-03 for the API rollout, used by `sync-postgres.yml`), and the
build service account
`239660669828-compute@developer.gserviceaccount.com` — the account
**both** triggers run as — **already holds**
`roles/secretmanager.secretAccessor` on it. So no grant and no new
secret are needed; the runbook only confirms them.

## Correcting a claim from #11220

That PR's description recorded, as the reason a Cloud Run env-var switch
was not built:

> `/etc/nginx` is read-only in `nginxinc/nginx-unprivileged` (checked:
no `chown`/`chmod g+w` on it, final `USER 101`), so the image's own
`NGINX_ENVSUBST_OUTPUT_DIR` mechanism bails out with "not writable" and
renders nothing.

That reading came from the wrong file in the image's repository.
`nginxinc/nginx-unprivileged:alpine` is `FROM
nginxinc/nginx-unprivileged:1.31.5-alpine-slim`, and
`mainline/alpine/Dockerfile` — the variant that adds the modules —
genuinely contains no such lines, because they are in the base it
inherits from. `mainline/alpine-slim/Dockerfile`:

```dockerfile
# nginx user must own the cache and etc directory to write cache and tweak the nginx config
    && chown -R $UID:0 /var/cache/nginx \
    && chmod -R g+w /var/cache/nginx \
    && chown -R $UID:0 /etc/nginx \
    && chmod -R g+w /etc/nginx
```

`/etc/nginx/conf.d` is **owned by uid 101**, which is the uid the
container runs as. Now confirmed live rather than read: the new CI job's
log carries `20-envsubst-on-templates.sh: Running envsubst on
/etc/nginx/templates/00-origin-gate.conf.template to
/etc/nginx/conf.d/00-origin-gate.conf`. #11220 was right that nothing
could exercise it before it mattered — which is what that job is for,
and it earned its keep on the first run (below).

## Verification

**CI builds the app image and runs the gate against it.**
`app/Dockerfile` was hadolinted but never *built* before Cloud Build,
i.e. after the merge — and what it produces is not a program that fails
to import but an nginx whose config is rendered at container start. The
new `app-image` job in `ci-image.yml` runs the real image three ways:

| | |
|---|---|
| gate off | `/` 200; `/_health` → `X-Origin-Gate: off`; with any header
→ `off-seen` |
| armed | `/` 403 bare, 403 with a wrong secret, 200 with the right one;
`/_health` still 200 (exempt) and reporting `missing` / `mismatch` /
`ok` |
| armed, no secret | 403 with no header **and** with an empty one — the
tagged map keys, proven |
| always | the 403 is the gate's own page; the secret appears in neither
that page nor `docker logs`; `nginx -t` validates the rendered config
(`nginx -T` would print the secret into the CI log, so it is never run)
|

**It failed on its first run, with a defect nothing else here could have
found.** With the gate armed:

```
nginx: [emerg] could not build map_hash, you should increase map_hash_bucket_size: 64
```

nginx cannot hash a `map` key longer than one bucket, and the default
bucket is the processor's cache line — 64 bytes. The tagged key is
`presented:` plus the whole secret, so a 32-byte secret written as hex
is 74 characters. **With the gate off the container starts perfectly**,
because the key is short then; the failure would have appeared at the
exact moment of arming and nowhere earlier. Cloud Run would have kept
the previous revision serving, so it would have been a safe failure
rather than an outage — but it would have been a failure in the middle
of the one procedure this whole PR exists to make undramatic. The
template now sets `map_hash_bucket_size 512`, the smoke uses a
production-length 64-character secret so the ceiling stays exercised,
and a test pins the directive.

**The rendered config, parsed by nginx's own grammar.** No Docker in
this environment, so `crossplane` (nginx's own config parser) was run
over the template rendered three ways and assembled into the same `http
{}` context the container has:

```
gate on,  secret set    → status: ok    undefined variables: []
gate off, secret empty  → status: ok    undefined variables: []   ← the shipping state
gate on,  secret empty  → status: ok    undefined variables: []   ← fail-closed
```

Worth noting for calibration: crossplane parses, it does not build hash
tables, so it passed the `map_hash` defect cleanly. A text check cannot
replace a running container, which is the argument for the CI job.

**Static guards**, `tests/unit/api/test_app_origin_gate.py` (14 cases),
next door to the API gate's own tests: every server block gated and each
with its own `error_page`/`@origin_denied`; the gate before the
trailing-slash rewrite; `app/nginx.conf` never naming
`$http_x_origin_secret`; every `$origin_gate_*` it reads defined by the
template; the secret written once and **tagged**; the hash bucket
raised; no `proxy_pass` without clearing the header; the exemption
exactly `/_health`; both `/_health` blocks reporting the verdict and
re-including the header snippet; the Worker stamping and
deleting-before-stamping on `/api/event`; the smoke and the monitor
carrying the header; the Dockerfile's three lines.

**Local gates:** `uv run pytest tests/unit tests/integration` — 1988
passed. `ruff check .`, `ruff format --check .`, `mypy api core` clean.
`uv run python -m tools.changelog check --base origin/main` — 7
fragments well-formed.

**No `/verify-frontend` run**, and the reason is worth stating rather
than skipping: the changed behaviour is nginx's, there is no SPA change
in the diff (no file under `app/src`, no TypeScript at all), and the
flow cannot be driven from a browser without the container. The
container smoke above is that loop, and the deploy's pre-traffic smoke
is the second one.

## Review round

Four findings, three of them defects, all applied; threads answered and
resolved.

- **The gate header was forwarded to every upstream.** The sharpest one.
nginx passes incoming request headers to a proxied server by default, so
once the Transform Rule stamps this host, `/js/script.js` and
`/api/event` would have handed the shared secret to **plausible.io** —
and with it the API service's key, since both take the same value. Fixed
wider than reported: the rule is now that the header is consumed by this
server and never forwarded, so *every* `proxy_pass` in both blocks
clears it, and a test refuses a location that proxies without doing so.
- **Arming was written as two flags.** This service pins traffic to a
named revision (`app/cloudbuild.yaml` promotes with
`--to-revisions=<name>=100`), so `gcloud run services update` alone
creates an armed revision that serves nothing — and step (e) would have
read `off` on a path that carries the header. The runbook is now the
API's own block, copied rather than paraphrased: refuse while a Cloud
Build is in flight, pin the **serving** image rather than the latest
template, `--revision-suffix`, then `update-traffic
--to-revisions=…=100`. Rolling back is its own block that looks nothing
up.
- **`ORIGIN_SECRET:latest` instead of a pinned version.** Cloud Run
resolves a secret-backed variable when each *instance* starts, so a
rotation reaches new instances while older ones keep the old value —
intermittent 403s inside one revision. The block resolves the newest
ENABLED version number and refuses if there is none.
- **`ORIGIN_GATE=ON` also arms**, because a plain `map` key is matched
without regard to case. Kept, and the documentation corrected instead:
nobody sets that variable to `ON` without meaning to arm, and the other
failure direction — an operator who armed the gate, was told nothing,
and still has an open origin — is the one worth avoiding.

One more, from CI rather than review: hadolint **DL3064** reads the ENV
variable *name* and warns that a secret may be baked into the image. The
value is the empty string, and it is declared for the opposite reason —
so a service supplying no secret renders a config that refuses everyone
rather than one nginx cannot parse. Renaming would silence the rule and
break a four-place contract (Secret Manager, the API service, the Worker
binding, the repository secret), so the exception sits on an `ENV`
instruction of its own with its reason beside it, per the repository's
own hadolint convention, and `ci-image.yml`'s claim that
`app/Dockerfile` needs no exceptions is updated.

## Two decisions worth reviewing

- **`return 421`, not `return 403`.** An `error_page 403` sends the
refusal back through the server-rewrite phase, where it hits the same
`if` again and loses the custom page. A distinct internal code also
keeps this trick apart from the crawler's `418`. nginx generates a 421
of its own only for a coalesced HTTP/2 connection with a mismatched
authority, which cannot happen here — Cloud Run speaks HTTP/1.1 to this
container (`ports: name: http1`).
- **A custom 403 page instead of nginx's stock one.** The stock body
prints the exact nginx version to anyone knocking on the raw origin. The
refusal is a named location, entered without re-running the
server-rewrite phase, and it carries `X-Origin-Gate` so a half-applied
rotation says `mismatch` instead of just failing.
- **The map comparison is case-insensitive and not constant-time**,
unlike `api/secret_compare.py`. nginx lowercases a `map` source before
hashing and offers no constant-time primitive. Both are written down at
the top of the template; against a random 256-bit secret the answer to
both is the entropy, not the comparison.

## Rollout — for the main session

Steps (a) and (b) are safe on their own and can sit for days. **The arm
and the rollback are full blocks, not one-liners** — they live in
`infra/cloudflare/README.md` § "Arming, in full" and § "Rolling back",
and mirror the API's.

```bash
# (a) merge + deploy. Nothing is armed.
curl -sI https://anyplot.ai/_health | grep -i x-origin-gate            # expect: off

# (b) widen the Transform Rule to anyplot.ai AND www.anyplot.ai (dashboard →
#     Rules → Transform Rules → Modify Request Header, "Set"), and redeploy the
#     Worker so its /api/event branch stamps too. Then measure EVERY path —
#     each must read off-seen before anything is armed:
curl -sI https://anyplot.ai/_health                          | grep -i x-origin-gate
curl -sI https://www.anyplot.ai/_health                      | grep -i x-origin-gate
curl -si -X POST -A 'Googlebot' https://anyplot.ai/api/event -d '{}' | grep -i -e '^HTTP' -e x-origin-gate
curl -sI https://anyplot-app-r3tvmejsmq-ez.a.run.app/_health | grep -i x-origin-gate   # stays off

# (c) confirm the two callers (both already exist — nothing to create):
gh secret list --repo MarkusNeusinger/anyplot | grep ORIGIN_SECRET
gcloud secrets get-iam-policy ORIGIN_SECRET --project=anyplot   # the compute SA has secretAccessor
gh workflow run bot-serving-check.yml --repo MarkusNeusinger/anyplot   # expect "origin gate: off-seen"

# (d) arm — infra/cloudflare/README.md § "Arming, in full". In outline:
#       refuse while a Cloud Build is in flight; resolve the SERVING revision's
#       image; resolve the newest ENABLED secret VERSION (never :latest);
#       services update --image=… --update-secrets=ORIGIN_SECRET=ORIGIN_SECRET:$VERSION
#         --update-env-vars=ORIGIN_GATE=on --revision-suffix="arm-<stamp>";
#       services update-traffic --to-revisions="anyplot-app-arm-<stamp>=100".

# (e) verify:
curl -sI https://anyplot.ai/_health | grep -i x-origin-gate                            # ok
curl -s -o /dev/null -w '%{http_code}\n' https://anyplot.ai/                           # 200
curl -s -o /dev/null -w '%{http_code}\n' https://anyplot-app-r3tvmejsmq-ez.a.run.app/  # 403
curl -s -A 'Mozilla/5.0 (compatible; Googlebot/2.1)' https://anyplot.ai/scatter-basic | grep canonical
curl -si -X POST -A 'Googlebot' https://anyplot.ai/api/event -d '{}' | head -1         # 202, not 403
gh workflow run bot-serving-check.yml --repo MarkusNeusinger/anyplot                   # green
gcloud run services describe anyplot-app --project=anyplot --region=europe-west4 --format="value(status.traffic)"

# (f) rollback — infra/cloudflare/README.md § "Rolling back". Same shape, no
#     lookups: pin the serving image, --remove-env-vars=ORIGIN_GATE,
#     --revision-suffix="disarm-<stamp>", promote that revision by name.
```

**Rotation** now touches **five** copies of one value: Secret Manager,
the API service, the app service, the Worker binding and the GitHub
repository secret. Roll back, rotate, arm again; the gate is off in
between, which is the documented safe state. Note also the secret's
length ceiling: the map key is `presented:` plus the secret and the
bucket is 512 bytes, so a secret past roughly 500 characters would keep
the armed revision from ever becoming ready.

## Plan

N/A — audit item A30, decided directly.

## Test plan

- [x] CI `app-image`: the real image passes the gate matrix — off /
armed / armed-with-no-secret, the exempt path, each verdict, the refusal
page, and the secret in neither the page nor the logs
- [x] `crossplane` parses the rendered config in all three states inside
the container's own `http {}`; no undefined variables
- [x] `uv run pytest tests/unit tests/integration` — 1988 passed, 14 of
them new in `tests/unit/api/test_app_origin_gate.py`
- [x] `uv run ruff check .` + `ruff format --check .` + `mypy api core`
— clean
- [x] `uv run python -m tools.changelog check --base origin/main` —
fragment well-formed
- [x] the base image's entrypoint, template directory and `/etc/nginx`
ownership verified against `nginxinc/docker-nginx-unprivileged`
`mainline/alpine-slim/Dockerfile`, then confirmed live in the CI job's
entrypoint log
- [x] `gcloud run services describe anyplot-app` — no env vars today,
`startupProbe` is `tcpSocket` (no HTTP probe to exempt)
- [ ] After merge, before arming: every path in step (b) of the rollout
reads `off-seen`
- [ ] After arming: step (e), including `anyplot.ai/api/event` answering
202 rather than 403

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants