security(gate): the cache flush carries the header instead of being waved through - #11214
Merged
Conversation
…being waved through /debug/cache/invalidate was exempt from the origin gate because sync-postgres.yml has no front door: it posts from a GitHub runner to the direct *.run.app URL on purpose, since Cloudflare's bot challenge answers an unauthenticated curl POST against api.anyplot.ai with a 403 HTML page. The workflow now sends X-Origin-Secret itself, out of an ORIGIN_SECRET repository secret, so EXEMPT_PATHS holds exactly one entry — /health, which the deploy smoke needs on the candidate's tag URL. An exempt path is one anybody may POST to from anywhere with only CACHE_INVALIDATE_TOKEN behind it; a caller that carries the header needs no hole at all. The suite pins both locks: 403 without the header, 503 (the endpoint's own fail-closed answer) with it. A missing repository secret fails that step with a message naming it. The second door the gate still does not close — a crawler UA reaching the prerendered pages through the APP service's raw run.app URL — is now measured rather than suspected, and origin_gate.py records the two facts that decide how it can be closed: Cloud Run answers a foreign Host header with its own 404, so a host rule in app/nginx.conf would be a real boundary rather than theatre; and bot-serving-check.yml probes exactly that origin and cannot spoof the host either, so its exception has to be the shared secret — which means templating the app's nginx, attaching the secret to anyplot-app, and a Cloudflare Transform Rule for the anyplot.ai host that does not exist yet. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Update the stale security-boundary comments and repository-secret rotation procedure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Moves cache invalidation behind the API origin gate by supplying the shared secret from GitHub Actions.
Changes:
- Removes the cache endpoint exemption and adds regression tests.
- Sends
X-Origin-Secretduring database synchronization. - Updates API documentation and the changelog.
File summaries
| File | Description |
|---|---|
tests/unit/api/test_origin_gate.py |
Tests gated and authorized cache invalidation. |
docs/reference/api.md |
Documents revised gate behavior; rotation steps need updating. |
CHANGELOG.md |
Records the security change. |
api/origin_gate.py |
Removes the cache endpoint exemption; related comments remain stale. |
.github/workflows/sync-postgres.yml |
Adds the origin-secret header and explicit 403 handling. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- 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! |
…s along Copilot: core/config.py still said exactly two paths stay exempt including this endpoint, api/routers/debug.py still said the cache flush is exempt and therefore directly reachable, and the rotation procedure named the Secret Manager version, the Transform Rule and the Worker binding but not the new repository copy — after which the workflow keeps sending the old value. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two loose ends from #11208.
1. The cache flush stops being an exemption
/debug/cache/invalidatewas exempt from the origin gate becausesync-postgres.ymlhad no front door: it posts from a GitHub runner to the direct*.run.appURL on purpose, since Cloudflare's bot challenge answers an unauthenticated curl POST againstapi.anyplot.aiwith a 403 HTML page.The workflow now stamps
X-Origin-Secretitself.EXEMPT_PATHSis 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_TOKENstanding 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/invalidatewith the gate armed and no header → 403 (the gate)CACHE_INVALIDATE_TOKENconfigured) — reached, not pre-emptedA 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 andapi/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_proxytoapi.anyplot.ai, where the edge stamps the header legitimately. #11208 described this; it is now measured: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
Hostrule would be a real boundary, not theatre. The obvious objection is that anyone could sendHost: anyplot.aito therun.appURL 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:So on that origin
$hostis always therun.appname, andapp/nginx.confcould refuse@seo_proxyfor 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-unprivilegedships theenvsubstentrypoint), attachORIGIN_SECRETtoanyplot-appinapp/cloudbuild.yaml, add a Cloudflare Transform Rule for theanyplot.aihost (today's coversapi.anyplot.aionly — 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 coreclean. The workflow's YAML parses and its cache-flush step's script passesbash -nafter extraction. The two curl probes above were run against the live origins.🤖 Generated with Claude Code
https://claude.ai/code/session_01UEScQMZFvxxNNyNJYryfa3