Repository navigation
Radiation page: RadiaCode readings (JSON bridge, radwatch, or Home Assistant) - #76
Conversation
/radiation shows dose rate, count rate, accumulated dose, battery, temperature, reading age and radwatch's watch alarm; /api/radiation serves the same JSON. Served like /nerd (HTML read from disk per request) behind the same auth. carwatch/radiation.py reads one of three sources chosen by radiation.source in config.json: - json: a bridge's plain-GET JSON object (recommended, no token) - radwatch: `radwatch.py status` on this box, read-only; the alarm verdict is radwatch's own, never recomputed here - ha: HA REST /api/states/<entity>, token from config or env, only sent to a private host (mercedesme._is_private_ha) Five states that never look alike: unconfigured, error (with reason), empty, stale (older than stale_seconds, default 60, or age unknown), live. Fields a source has not sent yet read "not yet reported". status() never raises; an exception becomes an error state with its text. tests/test_radiation.py: 47 tests (HA 401/404/unreachable, bridge bad JSON/missing fields/stale ts, empty radwatch DB, an older radwatch without status) plus a negative control that collapses the stale and error distinctions and checks that the scenario table catches it. Co-Authored-By: Claude Opus 5.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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478a9c91d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for key in [k for k, _, _ in FIELDS] + ["last_reading"]: | ||
| eid = prefix + key | ||
| try: | ||
| states[key] = json.loads(_get_json(f"{url}/api/states/{eid}", hdr).decode()) |
There was a problem hiding this comment.
Revalidate redirects before forwarding the HA token
When the private Home Assistant endpoint or its reverse proxy returns a 30x redirect to a public URL, urllib.request.urlopen follows it and copies the Authorization header to the redirected request. Consequently, read_ha can send the long-lived bearer token to a host that never passed _is_private_ha, violating the documented private-host guarantee. Disable redirects or validate every redirect destination and only re-add the header after it passes the private-host check.
Useful? React with 👍 / 👎.
HA token requests never follow a redirect (P1 from the #76 review)
Codex's review of ThinkOffApp#76 (P1): urllib's default redirect handler carries every header except the body ones to the next request, Authorization included. So if the private Home Assistant, or the proxy in front of it, answered 30x to another host, CarWatch sent the long-lived HA token to a host that never passed _is_private_ha. Measured: Python 3.11, 3.12 (CI) and 3.13 forward it; 3.14 already drops it on a cross-origin redirect. - carwatch/tokenhttp.py: an opener whose redirect handler refuses (HTTPError with the 30x code and the target) instead of following. - radiation.read_ha uses it for every token request, and reports a redirect as such ("set radiation.ha.url to the final address"). - The same bug existed in mercedesme._get and _post, which send the same token: both use tokenhttp now. _post also lacked the private-host check that _get has; it has it now. - Tests: test_tokenhttp.py with two local servers (HOME redirects, OTHER logs every Authorization it sees). A negative control shows plain urllib carrying the token to OTHER; tokenhttp, radiation and both Mercedes calls must refuse, and OTHER must see nothing. Reverting either module's switch makes its tests fail. Full suite on 3.12: 235 tests OK. Not changed: room.py sends the GroupMind key to https://groupmind.one, which does not redirect; lower risk, separate decision. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
A new
/radiationpage plus/api/radiationJSON, for a RadiaCode 10x. It shows dose rate (µSv/h), count rate (CPS), accumulated dose, battery, temperature, the age of the last reading, and radwatch'swatchalarm verdict when that source is used. It follows the/nerdpattern:carwatch/radiation.htmlis read from disk on each request, behind the same_authorised()gate, and its fetches carry?t=like/cloudcar. The unified page links to it from its links row.Sources (
radiation.sourcein config.json)jsonis the recommended source. A bridge on another machine serves one JSON object over a plain GET with no auth, about every 10 s:dose_rate,count_rate,ts(unix seconds or ISO 8601), plus optionalaccumulated_dose,battery,temperature,hardness,serial,firmware. It needs no token and no radwatch on the car. Staleness is computed fromts. The optional fields are absent for about the first minute after a bridge restart; the page shows them as "not yet reported", not as an error.radwatchrunsradwatch.py status --db ...on this box. That command opens the database read-only and comes from the companion PR status: read-only JSON for dashboards (CarWatch Radiation page) radwatch#1. CarWatch shows radwatch's numbers and its alarm verdict as radwatch reports them. It does not recompute any statistics.hareads Home Assistant REST (GET /api/states/sensor.radiacode_*, prefix configurable). The URL and the long-lived token come fromradiation.ha.*or$CARWATCH_RADIATION_HA_URL/$CARWATCH_RADIATION_HA_TOKEN. Nothing is hardcoded. The token is sent only to a private,.localor Tailscale host, using the same check mercedesme uses (Security review: CarWatch dashboard (:8088) surface #14). No HA token exists on the fleet today, so this source stays off until one is minted.States
The page has five states and none of them looks like another: no source configured, source error (with the reason, e.g. "Home Assistant rejected the token (HTTP 401)" or "bridge returned invalid JSON"), connected, no reading yet, stale (older than
radiation.stale_seconds, default 60, or age unknown, or a future timestamp from clock skew), and live. If CarWatch itself can't be reached, the page shows that too.radiation.status()never raises: an unexpected exception becomes an error state that carries the exception text.Files
carwatch/radiation.py(new): the three sources and the classifiercarwatch/radiation.html(new): the pagecarwatch/webchat.py: the/api/radiationand/radiationroutes, plus a link on the unified pagetests/test_radiation.py(new)config.example.json,README.md,FEATURES.md: the config block and docsTests
The tests use local HTTP servers and a stand-in radwatch script. Nothing touches the network. The bridge fixture follows the shape of a real bridge reading, with serial and address removed.
Negative control:
TestNegativeControlruns the unconfigured / error / stale / live table against two broken versions, one whose classifier drops the age check and one whosestatus()turns errors into "empty". The table must catch both. I also mutated the source by hand: removing the age check makes 5 tests fail, and swallowing source errors into "empty" makes 16 fail.Verified vs not
🤖 Generated with Claude Code