Cache the firmware repository's released images on disk #55

Merged
Claude merged 4 commits from feat/firmware-cache into feat/firmware-updates 2026-09-19 08:11:30 +00:00
Collaborator

The updater now asks the firmware repository for its newest release every few
hours and downloads any image it does not already hold. It is built to find
nothing: no network, no release, a refused download and a short one are all
ordinary outcomes, logged at info and never raised.

An image is only offered once its bytes are on disk and hashed. The cache tree
is <environment>/<version>/<environment>-<version>-firmware.bin beside an
image.json recording environment, node type, version, size and SHA-256. The
metadata is written last, through a rename, so a reader never sees half an
entry, and the ticket that serves these to nodes over HTTP can map a URL path
straight onto the tree. Nothing upstream publishes a hash, so this end computes
it after the download; the release API's size is what catches a truncated one.

Node to image mapping comes from protocol/identifiers.py, not a second table:
the firmware's PlatformIO environments are node_ plus the registry name. That
needed the protocol package inside the updater container, so compose mounts it
read-only, the same way it already mounts the integration into Home Assistant.

FIRMWARE_REGISTRY_TOKEN travels in an Authorization header only, never in a
URL or a log line.

Tested with a substituted release source, so nothing in the suite touches the
network: no network at all, no release published, an asset missing for a node,
a truncated download, a refused one, the hash recorded, and a second check that
finds nothing new and downloads nothing.

CCS-UHA-3.

The updater now asks the firmware repository for its newest release every few hours and downloads any image it does not already hold. It is built to find nothing: no network, no release, a refused download and a short one are all ordinary outcomes, logged at info and never raised. An image is only offered once its bytes are on disk and hashed. The cache tree is `<environment>/<version>/<environment>-<version>-firmware.bin` beside an `image.json` recording environment, node type, version, size and SHA-256. The metadata is written last, through a rename, so a reader never sees half an entry, and the ticket that serves these to nodes over HTTP can map a URL path straight onto the tree. Nothing upstream publishes a hash, so this end computes it after the download; the release API's `size` is what catches a truncated one. Node to image mapping comes from `protocol/identifiers.py`, not a second table: the firmware's PlatformIO environments are `node_` plus the registry name. That needed the protocol package inside the updater container, so compose mounts it read-only, the same way it already mounts the integration into Home Assistant. `FIRMWARE_REGISTRY_TOKEN` travels in an `Authorization` header only, never in a URL or a log line. Tested with a substituted release source, so nothing in the suite touches the network: no network at all, no release published, an asset missing for a node, a truncated download, a refused one, the hash recorded, and a second check that finds nothing new and downloads nothing. CCS-UHA-3.
Cache the firmware repository's released images on disk
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 18s
Lint, type check and test / quality (pull_request) Successful in 1m41s
Lint, type check and test / release (pull_request) Has been skipped
4e5f1d3172
Claude left a comment

One real defect (the poll loop can die silently), one token-leak risk, and three nits.

One real defect (the poll loop can die silently), one token-leak risk, and three nits.
@ -0,0 +115,4 @@
return None
def environment_for_node(node_type: int) -> str | None:
Author
Collaborator

environment_for_node has no caller outside its own test. Same for Release.version — refresh takes the version from the asset filename, so the tag_name parse is never read. Drop both until something needs them.

`environment_for_node` has no caller outside its own test. Same for `Release.version` — `refresh` takes the version from the asset filename, so the `tag_name` parse is never read. Drop both until something needs them.
@ -0,0 +144,4 @@
return (self.root / environment / version / METADATA_NAME).is_file()
def images(self) -> tuple[CachedImage, ...]:
"""Every image that is installable right now, newest version first."""
Author
Collaborator

Docstring says "newest version first"; the sort is ascending, so it returns oldest first (['2026.09.9', '2026.10.1']). latest_for_node compensates with reversed, so nothing is broken today, but the tree is described as an interface for the serving ticket and this will mislead it.

Docstring says "newest version first"; the sort is ascending, so it returns oldest first (`['2026.09.9', '2026.10.1']`). `latest_for_node` compensates with `reversed`, so nothing is broken today, but the tree is described as an interface for the serving ticket and this will mislead it.
@ -0,0 +193,4 @@
raise _ShortDownloadError
digest = _sha256_of(partial)
partial.replace(directory / asset.name)
except OSError, urllib.error.URLError, _ShortDownloadError:
Author
Collaborator

urllib.error.URLError subclasses OSError, so it is redundant here and in refresh's tuple at line 221.

`urllib.error.URLError` subclasses `OSError`, so it is redundant here and in `refresh`'s tuple at line 221.
@ -0,0 +262,4 @@
await asyncio.wait_for(stop.wait(), delay)
if stop.is_set():
return
await asyncio.to_thread(refresh, cache, releases)
Author
Collaborator

The poll loop dies permanently and silently on any exception refresh does not catch. http.client.HTTPException — IncompleteRead from json.load(response) or copyfileobj on a chunked body, BadStatusLine from getresponse() — is neither OSError nor ValueError, so it escapes refresh, ends this task, and serve only looks at the task at shutdown under return_exceptions=True. Nothing is logged and no further check runs until the container restarts.

Confirmed: a source raising IncompleteRead leaves poll done after one call, exception swallowed.

Wrap the to_thread call in except Exception logged at info, which also makes "a failed check is an ordinary outcome" true for anything future code adds. Worth a test with a source that raises something outside the caught set.

The poll loop dies permanently and silently on any exception `refresh` does not catch. `http.client.HTTPException` — `IncompleteRead` from `json.load(response)` or `copyfileobj` on a chunked body, `BadStatusLine` from `getresponse()` — is neither `OSError` nor `ValueError`, so it escapes `refresh`, ends this task, and `serve` only looks at the task at shutdown under `return_exceptions=True`. Nothing is logged and no further check runs until the container restarts. Confirmed: a source raising `IncompleteRead` leaves `poll` done after one call, exception swallowed. Wrap the `to_thread` call in `except Exception` logged at info, which also makes "a failed check is an ordinary outcome" true for anything future code adds. Worth a test with a source that raises something outside the caught set.
@ -0,0 +282,4 @@
"""
request = urllib.request.Request(url)
if self.token:
request.add_header("Authorization", f"token {self.token}")
Author
Collaborator

urllib's HTTPRedirectHandler.redirect_request copies every header except content-length and content-type onto the new request, so a 302 from browser_download_url to another host carries Authorization with it. Forgejo redirects release attachments when storage is external. Send the header only when the URL's host matches repository's, or install a redirect handler that drops it on a host change.

I have not confirmed this instance redirects; the stdlib forwarding behaviour is confirmed.

`urllib`'s `HTTPRedirectHandler.redirect_request` copies every header except content-length and content-type onto the new request, so a 302 from `browser_download_url` to another host carries `Authorization` with it. Forgejo redirects release attachments when storage is external. Send the header only when the URL's host matches `repository`'s, or install a redirect handler that drops it on a host change. I have not confirmed this instance redirects; the stdlib forwarding behaviour is confirmed.
Take the review: keep the poll alive, and pin the token to one host
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m26s
Lint, type check and test / release (pull_request) Has been skipped
5656cc25d8
Claude left a comment

All five earlier points fixed and verified. One narrow gap left in the new redirect handler.

All five earlier points fixed and verified. One narrow gap left in the new redirect handler.
@ -0,0 +324,4 @@
newurl: str,
) -> urllib.request.Request | None:
"""Follow the redirect, without the token if the host changed."""
redirected = super().redirect_request(req, fp, code, msg, headers, newurl)
Author
Collaborator

The host check ignores the scheme, so an https -> http redirect on the same host keeps the token and sends it in clear. Confirmed: redirecting the latest-release request to http://git.robware.uk/blob still carries token secret.

Compare (req.type, req.host) rather than req.host alone, and extend test_the_token_does_not_follow_a_redirect_off_the_registry with a same-host downgrade.

The host check ignores the scheme, so an `https` -> `http` redirect on the same host keeps the token and sends it in clear. Confirmed: redirecting the latest-release request to `http://git.robware.uk/blob` still carries `token secret`. Compare `(req.type, req.host)` rather than `req.host` alone, and extend `test_the_token_does_not_follow_a_redirect_off_the_registry` with a same-host downgrade.
Keep the token off a plain-HTTP redirect on the same host
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m27s
Lint, type check and test / release (pull_request) Has been skipped
02676cf9cf
Claude left a comment

Scheme fix verified. One leftover import, then this is good to merge.

Scheme fix verified. One leftover import, then this is good to merge.
@ -0,0 +24,4 @@
import logging
import re
import shutil
import urllib.parse
Author
Collaborator

import urllib.parse is now unused — urlsplit was its only caller. Ruff cannot flag it because import urllib.request binds urllib too. Drop the line.

`import urllib.parse` is now unused — `urlsplit` was its only caller. Ruff cannot flag it because `import urllib.request` binds `urllib` too. Drop the line.
Drop an import nothing uses any more
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m29s
Lint, type check and test / release (pull_request) Has been skipped
4a340baeba
Claude merged commit cf140c4546 into feat/firmware-updates 2026-09-19 08:11:30 +00:00
Claude deleted branch feat/firmware-cache 2026-09-19 08:11:30 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanHomeAssistant!55
No description provided.