Close the direct run.app door with a shared-secret origin gate - #11208
Conversation
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
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke
There was a problem hiding this comment.
🔵 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.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🟡 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-proxyexemption keeps a substantial direct-origin bypass open. A caller can hit arbitrary/seo-proxy/{spec_id}URLs onrun.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 inagentic/docs/project-guide.md:186-206stale. The repository documentation contract requires structural project changes to update that agent guide too, so addinfra/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
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 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
There was a problem hiding this comment.
🔵 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-77is incomplete:X-Admin-Tokenalone 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 requireX-Origin-Secretas 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
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
There was a problem hiding this comment.
🔵 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-139andinfra/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 reportmismatch. 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, contradictingEXEMPT_PATHSand 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
There was a problem hiding this comment.
🟡 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_secretaccepts arbitrary Unicode, but the WorkersHeaders.setAPI converts values toByteStringand throws for code points above U+00FF. A CJK or emoji secret that Cloud Run accepts will therefore make everyanyplot.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
_matchesavoids for the new gate.invalidate_cachestill compares Starlette's Latin-1-decodedX-Cache-Tokenasstr(api/routers/debug.py:487-492), so a raw byte ≥ 0x80 raisesTypeError; 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
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
There was a problem hiding this comment.
🔵 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 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
There was a problem hiding this comment.
🟡 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 describeor the JSON parser fails. The here-string still givesreadan empty line, so both variables become empty andtest "" = ""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 updatewill 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 fromSERVING, 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
ASGIAppand-> None, and the call hook with Starlette'sScope,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
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
There was a problem hiding this comment.
🟡 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
OPTIONSexemption admits more than CORS preflights.CORSMiddlewareonly consumes anOPTIONSrequest when it includesOriginandAccess-Control-Request-Method; a bare direct-originOPTIONS /seo-proxy/legalreaches this branch, passes the gate, and thenrecord_bot_fetchemits 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
…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
There was a problem hiding this comment.
🟡 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
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
There was a problem hiding this comment.
🟡 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 becauseoffis 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
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
There was a problem hiding this comment.
🔵 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 != LATESTand 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_secretstill accepts any nonempty string. That leaves two concrete failure modes: a short value is brute-forceable through the exempt/healthok/mismatchoracle, and a value containing characters above U+00FF makes the Worker'sHeaders.set()throw because HTTP header values are ByteStrings. Encoding inside the API only fixescompare_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
There was a problem hiding this comment.
🟡 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
| # 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" |
| --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" |
| 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. |
| **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 |
…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>
…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>
…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>
Summary
api/origin_gate.pyrequires the header the Cloudflare edge stamps, and refuses anything else with403before the request costs anything. The API stands on Cloud Run withingress=all, so it answers on two addresses —api.anyplot.aibehind Cloudflare, and the raw*.run.appURL in front of nothing. Every edge measure (bot challenge, WAF, the cache that makes themax-age=300reads free) was one URL away from being bypassed;api/request_context.pyalready documented callers doing it.ORIGIN_SECRETis 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./healthreportsorigin_gate—off·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.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.ingress=all, and its nginx relays a crawler user agent through@seo_proxytoapi.anyplot.ai, where the edge stamps the header legitimately — so the prerendered render stays reachable via the app's rawrun.appURL. That is a second door on a second service (the request the API sees really did pass the edge), and closing it means gatinganyplot-appor refusing to proxy forrun.apphosts, whichbot-serving-check.ymlprobes nightly. Named inapi/origin_gate.pyanddocs/reference/api.md; own PR.What is exempt, and why each one has to be
Exact paths, no prefixes:
/healthrun.apptag URL, which by definition never passes the edge — gating it makes every deploy fail closed/debug/cache/invalidatesync-postgres.ymlposts here from a GitHub runner over the direct*.run.appURL on purpose — Cloudflare's bot challenge answers an unauthenticated curl POST againstapi.anyplot.aiwith 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 lockOPTIONS/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 querySpecRepository/ImplRepositoryon 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 overhttps://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, andbot-serving-check.ymlruns 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/eventPlausible passthrough is preserved untouched.anyplot-api-…run.app— the Cloud Build smoke (now sends the header) andsync-postgres.yml(exempt path, above).anyplot-app-…run.app—bot-serving-check.ymlhits the app origin, whose nginx then goes out throughapi.anyplot.ai. Unaffected.Every header secret goes through one byte-wise comparator
secrets.compare_digestraisesTypeErrorwhen eitherstrholds 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-TokenandX-Cache-Token, which matters more:/debug/cache/invalidateis exempt from the gate on the grounds that it has its own lock, and it is the one endpoint reachable on the directrun.appURL. So the fix is one comparator inapi/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
strcomparison 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-secretsand--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_SECRETis 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 inapi/cloudbuild.yamlis 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_fetchfires 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 itssetdefaultfor the/og/cards depends on being.api/main.pynow 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_agentclassifies 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_SECRETdoes 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-seenon EVERY path. Cloudflare dashboard: Rules → Transform Rules → Modify Request Header → set staticX-Origin-Secretforhttp.host eq "api.anyplot.ai". Then deployinfra/cloudflare/anyplot-api-proxy.jswith theORIGIN_SECRETsecret binding (procedure ininfra/cloudflare/README.md). The gate is still off, so nothing can break; what this step buys is the evidence: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.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:services updateclones 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 assertslatestReadyRevisionName == the revision serving 100%and stops otherwise.:latest. Cloud Run resolves a secret-backed variable when each instance starts, so with:latesta 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.--to-latest— the same hazard as (1), and the reasonapi/cloudbuild.yamlrefuses 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.
Then walk the site once (gallery, a spec page, the stats page), fetch an OG card cross-origin, and let one
sync-postgresrun finish — its cache flush must still return 200.(e) Rollback — one variable, no code change:
The same block as arming, with
--remove-secrets=ORIGIN_SECRETin place of--update-secretsand adisarm-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/invalidateis exempt becausesync-postgres.ymlhas no front door. The cleaner end state is for that workflow to sendX-Origin-Secretfrom 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/healthverdicts, the non-ASCII header on all three secrets, and the trailing-newline strip onORIGIN_SECRETand 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.yamlparses; step ids unchanged (build-image,push-image,push-latest,deploy,smoke,promote,get-url)./librariesonce it is on — and it acceptsoff/off-seen, so it cannot take the deploy pipeline down during the rollout or after a rollback.Checklist
CHANGELOG.mdupdated under[Unreleased]— one### Addedentry for the gate, two### Changedentries for the deploy flag and the middleware order.docs/reference/api.md(new "Origin gate" section,/healthresponse),docs/development.md(env table),docs/reference/repository.mdandagentic/docs/project-guide.md(both repository maps getinfra/),.env.example, andinfra/cloudflare/README.mdfor the Worker and the measuring procedure.🤖 Generated with Claude Code
https://claude.ai/code/session_01PBQdMbboxo59sSThGSbfke