dash: drop wildcard CORS on /api/status, send Referrer-Policy (#12) - #68
ThinkOffApp wants to merge 1 commit into
Conversation
/api/status returns live car facts, the Pi's process list (ps -eo) and the last agent journal lines. It carried Access-Control-Allow-Origin: *. That header is only inert while every caller needs a token. On home wifi the dashboard authorises by NETWORK instead - _peer_is_owner() trusts any private address once _pi_on_home_wifi() is true, deliberately, so phones at home open the dash without one. Ambient authority plus a wildcard CORS header means any web page open on a phone on that wifi could fetch this endpoint in the background and READ the reply cross-origin. The dashboard is same-origin and never needed the header: /api/status is fetched only from the dash page itself, and it is the sole ACAO in the repo, so nothing external breaks. Also adds Referrer-Policy: no-referrer to the common send path. The token rides in the URL as ?t= so a one-tap link works in the car, which means every outbound link from a dashboard page would otherwise carry the credential in its Referer. That is one of the four leak routes #12 lists, and it is the one that costs nothing to close. This does not address the rest of #12 or #13: the token still travels in URLs and still gates /api/update, which pulls and runs code. Splitting that into tiers changes how petrus repairs the car from the roadside, so it needs his call rather than a quiet commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Now confirmed against the live car, not just the source. Reached Vadelma over its own dial-out cloudflared tunnel (the And the payload is not trivial — it really is the thing worth protecting: So the finding is observed behaviour on the deployed car, not an inference from reading the handler. Two honest limits on that check:
Read-only throughout: GETs only, nothing written, |
Partial fix for #12. Found while assessing #12/#13 rather than reported.
The finding
/api/statusreturns live car facts, the Pi's process list (ps -eo) and the last agent journal lines — and carried:That header is inert only while every caller needs a token. On home wifi the dashboard authorises by network instead:
_peer_is_owner()trusts any private address once_pi_on_home_wifi()is true — deliberately, so phones at home open the dash without typing anything.Ambient authority + wildcard CORS means any web page open on a phone on that wifi could fetch this endpoint in the background and read the reply cross-origin.
Why removing it is safe
/api/statusis fetched only from the dashboard page itself (same-origin), and it is the onlyAccess-Control-Allow-Originin the repo. Nothing external consumes it.Also
Referrer-Policy: no-referreron the common send path. The token rides in the URL as?t=so a one-tap link works in the car — which means every outbound link from a dashboard page would otherwise carry the credential in itsReferer. That is one of the four leak routes #12 lists and the one that costs nothing to close.What this does NOT fix
The token still travels in URLs and still gates
/api/update, which pulls and runs code (#13). Splitting that into privilege tiers changes how petrus repairs the car from the roadside, and getting it wrong strands him. That needs his decision, not a quiet commit.Tests
tests/test_dash_headers.py, 4 cases: no wildcard CORS is ever sent (a comment naming it is fine), the referrer policy is sent, the status payload is undisturbed, and — so the premise cannot rot silently — that ambient auth still exists, which is what makes the CORS removal matter.Suite failure on this branch is
test_dash_loop's missing pytest — pre-existing onmain, fixed separately in #64.Not verified on the car
Header-level change verified in source and tests; the car is unreachable, so it has not been exercised against a live dashboard.
🤖 Generated with Claude Code