Skip to content

fix: mqtt-rpcd updater hardening deferred from the remote-update review (TRA-1319) - #779

Open
mikestankavich wants to merge 8 commits into
mainfrom
fix/tra-1319-mqtt-rpcd-updater-hardening-deferred-fro
Open

mikestankavich wants to merge 8 commits into
mainfrom
fix/tra-1319-mqtt-rpcd-updater-hardening-deferred-fro

Conversation

@mikestankavich

Copy link
Copy Markdown
Contributor

The nine findings the remote-update review (#765) deferred, each fixed test-first. Daemon and guard changes reach readers as a remote update. The guard and install.sh changes reach a reader only through install.sh.

What changed

Daemon (mqtt-rpc/internal/update, readerd)

  • Item 1. Preflight requires the guard timer to be enabled (timers.target.wants/mqtt-rpcd-guard.timer), not just installed. It makes no systemctl call.
  • Item 2. versions/ is fsynced after the download is renamed in, before pending names the new binary.
  • Item 5. CleanStale removes .mqtt-rpcd-*.download and .upload files left behind by a killed transfer. It runs at startup and before each download, and only touches files older than any live transfer could be (15 min).
  • Item 7. A reply that loses the commit race to this process's own earlier commit gets ErrAlreadyCommitted, and answered() no longer logs that as a failure.
  • Item 6. The download client trusts the system pool plus the Mozilla roots from golang.org/x/crypto/x509roots/fallback/bundle, so every update ships fresh roots. It adds to the system roots rather than replacing them: Ed25519 and sha256 decide what gets installed, and TLS only decides whether the download can happen. The broker's TLS is unchanged. That module requires Go 1.26, so mqtt-rpc's go line moves from 1.25.1 to 1.26.0. The backend is already on 1.26, and the release workflow reads go-version-file.

Guard (deploy/edge/mqtt-rpc/mqtt-rpcd-guard)

  • Item 3. When the recorded from version is gone, the guard falls back to the newest other installed version, using legacy only as a last resort, and records rolled_back with reason <why>_fallback. If nothing else is installed, it records a new unguarded state instead of failed ("nothing replaced" was false: the target is live).

  • Item 4. A new place <version> mode is install.sh's binary-and-link step, so the existing harness tests it against the real script:

    • it stops the guard timer first;
    • it removes rollback.claim, pending.commit and pending before the binary moves in;
    • it hardlinks a plain pre-versioning binary to versions/mqtt-rpcd-legacy and renames the symlink over it, so /opt/trakrf/mqtt-rpcd never goes missing;
    • it converges when re-run.

    install.sh calls it and restarts the timer on any exit, including an error or a dropped session. It is named place, not install, because TRA-1321 plans install <version|latest> for the full one-command install.

Backend and app

  • Item 8. A refused update is now a 422 in the standard error envelope (validation_error, detail = the reader's reason). A request that was published but never answered is now 504 (readercontrol.ErrNoReply) instead of 502. A failed publish stays 502. On a 504 the panel watches the requested version instead of showing "Could not start".
  • Item 9. "Timeout" now needs a check of the reader after the deadline. A successful fetch or an error both count. A background tab whose refetches paused no longer settles "timeout" without asking the reader.
  • The panel shows unguarded as running but unconfirmed, never as "failed, nothing replaced".

Other commits: a pre-existing gofmt drift in servlet_test.go, and README updates for the guard fallback, the new state and the embedded roots.

Not done here: bench verification (acceptance item 10)

This was dispatched with no hardware, because an overnight soak was running. The ticket asks for items 4–6 on cs463-212 with a remote update from a CI release, so this PR refs the ticket rather than closing it.

Worth testing on hardware

  • Go 1.26 daemon build. It cross-builds for linux/armv7 (10.6 MB, well under the 64 MB cap). It has not run on a CS463.
  • Item 6. A remote update from a CI release succeeds on the reader with the new transport (same-origin GitHub download, redirect to objects.githubusercontent.com).
  • Item 4.
    • install.sh onto a reader that is mid-probation for the same version: the binary survives and the timer comes back active.
    • An interrupted install.sh (kill the SSH session during the remote step), then a re-run.
    • A pre-versioning plain-file reader if one is still around: legacy should be the same inode.
  • Item 5. Kill the daemon mid-download, then restart. The .download file is removed once it is older than 15 minutes.
  • Item 1. systemctl disable mqtt-rpcd-guard.timer, then the update is refused with "not enabled".

Spun out

recover still trusts only the firmware CA bundle, so a stale bundle would also break on-site recovery. That fix needs the reader's curl/OpenSSL 1.0.2 to validate, so it is a comment on TRA-1321, which owns the recover/install redesign.

Validation

  • just validate: exit 0. Frontend 2,665 passed / 0 failed; Go, mqtt-rpc (-race) and shellcheck green; lint rewrote nothing.
  • csw-gates --worktree main: no gates fire (no migrations).
  • readerconfig software integration tests (-tags=integration) pass against a throwaway TimescaleDB.

Refs TRA-1319

🤖 Generated with Claude Code

Mike Stankavich and others added 8 commits November 8, 2026 22:06
…race

Preflight now requires the guard timer to be enabled (its
timers.target.wants link), not merely installed: a disabled timer never
ticks, so a build that never reaches the broker would never roll back.

versions/ is fsynced after the download is renamed into place, before
pending names the new binary.

Hidden .mqtt-rpcd-*.download/.upload files left by a killed transfer are
removed at startup and before each download once they are older than any
live transfer can be.

A reply that loses the commit race to this process's own earlier commit
gets ErrAlreadyCommitted, and answered() no longer logs it as a failure.

Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gone

An update whose recorded 'from' version is no longer installed used to
leave the failing build live with no guard, recorded as 'failed', which
the app shows as 'nothing replaced'. The guard now falls back to the
newest other installed version (legacy only as a last resort) and records
a rolled_back with reason <why>_fallback. With nothing else installed it
records the new 'unguarded' state instead of 'failed'.

Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The legacy migration moved the plain /opt/trakrf/mqtt-rpcd away before
the symlink existed, so an SSH drop in between left no binary at all.
The binary-and-link step is now the guard's 'install' mode, which the
existing guard harness tests against the real script:

- the plain binary is hardlinked to versions/mqtt-rpcd-legacy and the
  symlink is renamed over it, so the link path never goes missing;
- the guard timer is stopped first, and install.sh starts it again on
  any exit (error or dropped session);
- pending, rollback.claim and pending.commit are removed before the new
  binary moves in, so a guard tick cannot roll back and delete a
  just-installed binary of the same version.

Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Downloads trusted only the reader firmware's CA bundle, which is never
refreshed. Once it stops validating GitHub, remote updates end for good
and only a site visit recovers the reader. The download client now
trusts the system pool plus the Mozilla roots from
golang.org/x/crypto/x509roots/fallback/bundle, so every update ships
fresh roots. It is a union, not a replacement: the Ed25519 signature and
sha256 are the integrity check, TLS only availability. The broker
connection's trust is unchanged.

The bundle module requires Go 1.26, so mqtt-rpc's go directive moves
1.25.1 -> 1.26.0 (the backend is already on 1.26; the release workflow
reads go-version-file).

Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st reply

Backend: a refused Daemon.Update now answers 422 in the standard error
envelope (validation_error, detail = the reader's reason) instead of an
ad-hoc {error, reason} body. A request that was published but never
answered is now 504 (readercontrol.ErrNoReply) rather than 502: the
reader may be running the update, so the outcome is unknown, not 'not
started'. A publish failure stays 502.

App: on a 504 the panel watches the requested version instead of
showing 'Could not start'. Timeout now needs a check of the reader after
the deadline (success or error), so a background tab whose refetches
paused does not settle 'timeout' unobserved. The guard's new
'unguarded' state is shown as running-but-unconfirmed, never as
'failed, nothing replaced'.

Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TRA-1321 plans 'mqtt-rpcd-guard install <version|latest>' as the full
one-command install from a release. This step only puts an uploaded
binary in place and swaps the link, so it gets a name of its own rather
than squatting on that one with a narrower meaning.

Refs: TRA-1319

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment Update

✅ This PR has been successfully merged into the preview branch.

The preview environment will update shortly at: https://app.preview.trakrf.id

trakrf-preview-sync Bot pushed a commit that referenced this pull request Nov 9, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant