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

backend/internal/handlers/readerconfig/software.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/trakrf/platform/backend/internal/middleware"
 	modelerrors "github.com/trakrf/platform/backend/internal/models/errors"
+	"github.com/trakrf/platform/backend/internal/readercontrol"
 	"github.com/trakrf/platform/backend/internal/readerrelease"
 	"github.com/trakrf/platform/backend/internal/readerrpc"
 	"github.com/trakrf/platform/backend/internal/util/httputil"
@@ -156,14 +157,17 @@ func (h *Handler) UpdateSoftware(w http.ResponseWriter, r *http.Request) {
 	res, err := h.daemon.UpdateDaemon(r.Context(), base, manifest)
 	if err != nil {
 		var re *readerrpc.UpdateRefusedError
-		if errors.As(err, &re) {
-			httputil.WriteJSON(w, http.StatusUnprocessableEntity, map[string]any{
-				"error":  "update_refused",
-				"reason": re.Reason,
-			})
-			return
+		switch {
+		case errors.As(err, &re):
+			// The reader checked the request and changed nothing; detail is its reason.
+			httputil.WriteJSONError(w, r, http.StatusUnprocessableEntity, modelerrors.ErrValidation, re.Reason, reqID)
+		case errors.Is(err, readercontrol.ErrNoReply):
+			// Sent, but the reply was lost: the update may be running. 504, not
+			// 502, so the app watches the reader instead of saying "not started".
+			httputil.WriteJSONError(w, r, http.StatusGatewayTimeout, modelerrors.ErrInternal, err.Error(), reqID)
+		default:
+			httputil.WriteJSONError(w, r, http.StatusBadGateway, modelerrors.ErrInternal, err.Error(), reqID)
 		}
-		httputil.WriteJSONError(w, r, http.StatusBadGateway, modelerrors.ErrInternal, err.Error(), reqID)
 		return
 	}
 	httputil.WriteJSON(w, http.StatusAccepted, map[string]any{"data": res})

backend/internal/handlers/readerconfig/software_integration_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/trakrf/platform/backend/internal/readercontrol"
 	"github.com/trakrf/platform/backend/internal/readerrelease"
 	"github.com/trakrf/platform/backend/internal/readerrpc"
 	"github.com/trakrf/platform/backend/internal/testutil"
@@ -90,8 +91,35 @@ func TestUpdateSoftware_RefusedIs422WithTheReason(t *testing.T) {
 	h, org, dev := softwareFixture(t, d, &fakeReleases{m: benchManifest})
 	rr := do(t, h, org, http.MethodPost, fmt.Sprintf("/api/v1/scan-devices/%d/reader-software/update", dev), `{"version":"v2"}`)
 	require.Equal(t, http.StatusUnprocessableEntity, rr.Code, rr.Body.String())
-	assert.Contains(t, rr.Body.String(), "update_refused")
-	assert.Contains(t, rr.Body.String(), "an update is already in progress")
+	// The standard envelope, like every other error this API returns.
+	var body struct {
+		Error struct {
+			Type   string `json:"type"`
+			Status int    `json:"status"`
+			Detail string `json:"detail"`
+		} `json:"error"`
+	}
+	require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &body))
+	assert.Equal(t, "validation_error", body.Error.Type)
+	assert.Equal(t, http.StatusUnprocessableEntity, body.Error.Status)
+	assert.Equal(t, "an update is already in progress", body.Error.Detail)
+}
+
+// The request reached the reader but its reply was lost: the update may well be
+// running. 504 says "outcome unknown", distinct from 502 "never sent".
+func TestUpdateSoftware_LostReplyIs504(t *testing.T) {
+	d := &fakeDaemon{updErr: fmt.Errorf("readercontrol: Daemon.Update on trakrf.id/x: %w", readercontrol.ErrNoReply)}
+	h, org, dev := softwareFixture(t, d, &fakeReleases{m: benchManifest})
+	rr := do(t, h, org, http.MethodPost, fmt.Sprintf("/api/v1/scan-devices/%d/reader-software/update", dev), `{"version":"v2"}`)
+	assert.Equal(t, http.StatusGatewayTimeout, rr.Code, rr.Body.String())
+	assert.Contains(t, rr.Body.String(), `"type":"internal_error"`)
+}
+
+func TestUpdateSoftware_UnsentRequestIs502(t *testing.T) {
+	d := &fakeDaemon{updErr: errors.New("readercontrol: publish request: not connected")}
+	h, org, dev := softwareFixture(t, d, &fakeReleases{m: benchManifest})
+	rr := do(t, h, org, http.MethodPost, fmt.Sprintf("/api/v1/scan-devices/%d/reader-software/update", dev), `{"version":"v2"}`)
+	assert.Equal(t, http.StatusBadGateway, rr.Code, rr.Body.String())
 }
 
 func TestUpdateSoftware_UnknownReleaseIs404AndNeverReachesTheReader(t *testing.T) {

backend/internal/readercontrol/client.go

@@ -11,6 +11,7 @@ package readercontrol
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"sync"
@@ -156,6 +157,11 @@ func (c *Client) next() int {
 	return c.nextID
 }
 
+// ErrNoReply: the request was published but no reply arrived in time. Unlike a
+// publish failure, the reader may have received it and acted — a caller that
+// started something must treat the outcome as unknown, not as "not started".
+var ErrNoReply = errors.New("reader did not respond (offline?)")
+
 // call publishes a request to the reader's <base>/rpc topic and blocks until the
 // matching reply arrives, the context is canceled, or the configured timeout
 // elapses. On timeout the pending entry is reclaimed and an offline-style error
@@ -201,12 +207,12 @@ func (c *Client) call(ctx context.Context, base, method string, params any) (rea
 		c.mu.Lock()
 		delete(c.pending, id)
 		c.mu.Unlock()
-		return readerrpc.Response{}, fmt.Errorf("readercontrol: %s on %s: reader did not respond (offline?): %w", method, base, ctx.Err())
+		return readerrpc.Response{}, fmt.Errorf("readercontrol: %s on %s: %w: %w", method, base, ErrNoReply, ctx.Err())
 	case <-timer.C:
 		c.mu.Lock()
 		delete(c.pending, id)
 		c.mu.Unlock()
-		return readerrpc.Response{}, fmt.Errorf("readercontrol: %s on %s: reader did not respond (offline?)", method, base)
+		return readerrpc.Response{}, fmt.Errorf("readercontrol: %s on %s: %w", method, base, ErrNoReply)
 	}
 }
 

backend/internal/readercontrol/client_test.go

@@ -117,6 +117,24 @@ func TestCall_Timeout(t *testing.T) {
 	}
 }
 
+// A request that was published but never answered may still have been acted
+// on: callers must be able to tell it from one that was never sent.
+func TestCall_NoReplyIsErrNoReply(t *testing.T) {
+	c, _ := newTestClient()
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+	_, err := c.call(ctx, "trakrf.id/cs463-212", readerrpc.MethodGetStatus, nil)
+	if !errors.Is(err, ErrNoReply) {
+		t.Fatalf("context expiry: err = %v, want ErrNoReply", err)
+	}
+
+	c.timeout = 20 * time.Millisecond
+	_, err = c.call(context.Background(), "trakrf.id/cs463-212", readerrpc.MethodGetStatus, nil)
+	if !errors.Is(err, ErrNoReply) {
+		t.Fatalf("client timeout: err = %v, want ErrNoReply", err)
+	}
+}
+
 func TestSetOperProfile_RPCErrorMapsToGoError(t *testing.T) {
 	c, cp := newTestClient()
 

backend/internal/readerrpc/daemon.go

@@ -41,6 +41,10 @@ const (
 	UpdateStateCommitted  = "committed"
 	UpdateStateRolledBack = "rolled_back"
 	UpdateStateFailed     = "failed"
+	// UpdateStateUnguarded: the target never committed, and the guard found no
+	// other installed version to roll back to, so the target stays live with no
+	// rollback armed. Unlike failed, the target DID replace the old build.
+	UpdateStateUnguarded = "unguarded"
 )
 
 // UpdateInfo describes the most recent update attempt.

deploy/edge/mqtt-rpc/README.md

@@ -31,6 +31,10 @@ binaries keeps its old binary as version `legacy` until it ages out of retention
 (newest 3 versions are kept); `legacy` has no updater, so rolling back to it ends
 remote updates for that reader.
 
+`install.sh` is safe to interrupt and re-run: an SSH drop at any point leaves a
+runnable `/opt/trakrf/mqtt-rpcd`, and the rollback guard is stopped and disarmed
+before the new binary moves in (`mqtt-rpcd-guard place`).
+
 ## The golden CloudServer
 The one entity the daemon cannot create is `TrakRF mqtt-rpc MQTT Server`. It
 holds the broker credential and a trust bundle, so it is set up by hand when the

deploy/edge/mqtt-rpc/install.sh

@@ -33,16 +33,19 @@ scp "$HERE/mqtt-rpcd.env.example" "$HOST:/tmp/mqtt-rpcd.env.example"
 ssh "$HOST" "VERSION='$VERSION' sh -s" <<'REMOTE'
 set -eu
 cd /opt/trakrf
-chmod 755 "versions/.mqtt-rpcd-$VERSION.upload" .mqtt-rpcd-guard.upload
-mv -f "versions/.mqtt-rpcd-$VERSION.upload" "versions/mqtt-rpcd-$VERSION"
+chmod 755 .mqtt-rpcd-guard.upload
 mv -f .mqtt-rpcd-guard.upload mqtt-rpcd-guard
-# A pre-versioning install has a plain file here: keep it as a manual rollback
-# target named "legacy" (it has no updater, so rolling back to it ends OTA).
-if [ -f mqtt-rpcd ] && [ ! -L mqtt-rpcd ]; then mv -f mqtt-rpcd versions/mqtt-rpcd-legacy; fi
-# An SSH install supersedes any remote update on probation.
-rm -f update/pending update/starts update/ticks
-rm -f .mqtt-rpcd.link
-ln -s "versions/mqtt-rpcd-$VERSION" .mqtt-rpcd.link && mv -f .mqtt-rpcd.link mqtt-rpcd
+# The guard's install step stops its timer. Start it again however this script
+# ends — an error under set -e, or the SSH session dropping — so a failed
+# install never leaves a reader with no no-commit rollback until its next boot.
+restart_guard_timer() { systemctl start mqtt-rpcd-guard.timer >/dev/null 2>&1 || true; }
+trap restart_guard_timer EXIT
+trap 'exit 1' HUP INT TERM
+# Stop the timer, disarm any remote update on probation, move the binary in,
+# and swap the link — keeping a pre-versioning plain binary as "legacy" (a
+# manual rollback target with no updater; rolling back to it ends OTA). Each
+# step is safe to interrupt and re-run; see mqtt-rpcd-guard "place".
+./mqtt-rpcd-guard place "$VERSION"
 test -f mqtt-rpcd.env || mv /tmp/mqtt-rpcd.env.example mqtt-rpcd.env
 chmod 600 mqtt-rpcd.env
 

deploy/edge/mqtt-rpc/mqtt-rpcd-guard

@@ -18,6 +18,11 @@
 #   list               Print the installed versions; `*` marks the active one.
 #   rollback <version> Operator recovery over SSH: point the link at <version>,
 #                      drop any pending update, restart.
+#   place <version>    install.sh's step on the reader: stop the guard timer,
+#                      disarm any pending update, move the uploaded binary in
+#                      and point the link at it (a plain pre-versioning binary
+#                      is hardlinked to "legacy" first, so the link path never
+#                      goes missing). install.sh restarts the timer after.
 #   recover            The on-site operator's one command (the `recover` SSH
 #                      login runs it and nothing else): fetch the newest daemon
 #                      release from GitHub, check it against its manifest's
@@ -113,6 +118,23 @@ resume_claim() {
 	fi
 }
 
+# fallback_version <exclude>: the newest installed version (by mtime) other
+# than <exclude>. legacy has no updater, so it is chosen only when nothing else
+# is installed. Prints nothing when there is no candidate at all.
+fallback_version() {
+	# ls -t is the only mtime sort the reader's busybox has; every name is
+	# checked by valid_version before use, so odd filenames cannot leak through.
+	# shellcheck disable=SC2012
+	c=$(ls -t "$DIR/versions" 2>/dev/null | sed -n 's/^mqtt-rpcd-//p' | while read -r cand; do
+		if valid_version "$cand" && [ "$cand" != "$1" ] && [ -f "$DIR/versions/mqtt-rpcd-$cand" ]; then
+			echo "$cand"
+		fi
+	done)
+	pick=$(printf '%s\n' "$c" | grep -vx legacy | head -n 1)
+	[ -n "$pick" ] || pick=$(printf '%s\n' "$c" | grep -x legacy | head -n 1)
+	echo "$pick"
+}
+
 # auto_rollback <reason>: claim the pending update and point back at `from`.
 # Returns 0 only when the link was moved (the caller restarts). Every failure
 # leaves the target in place and pending armed, so the next run retries:
@@ -121,21 +143,32 @@ auto_rollback() {
 	mv "$PENDING" "$CLAIM" 2>/dev/null || return 1 # the daemon committed first
 	target=$(field target "$CLAIM")
 	from=$(field from "$CLAIM")
+	reason="$1"
 	if ! valid_version "$from" || [ ! -f "$DIR/versions/mqtt-rpcd-$from" ]; then
-		write_last failed "$target" "${1}_no_from"
-		log "cannot roll back $target: version '$from' is not installed ($1)"
-		rm -f "$CLAIM"
-		clear_counters
-		"$SYNC"
-		return 1
+		# The recorded version is gone (removed by hand, or never recorded).
+		# Any other build beats leaving the failing one live with no guard.
+		gone="$from"
+		from=$(fallback_version "$target")
+		if [ -z "$from" ]; then
+			# Nothing else to run. The target stays live, unguarded; say so
+			# rather than "failed", which reads as "nothing replaced".
+			write_last unguarded "$target" "${1}_no_from"
+			log "cannot roll back $target: '$gone' is not installed and no other version is ($1)"
+			rm -f "$CLAIM"
+			clear_counters
+			"$SYNC"
+			return 1
+		fi
+		reason="${1}_fallback"
+		log "version '$gone' is not installed; falling back to $from ($1)"
 	fi
 	if ! point_at "$from"; then
 		mv "$CLAIM" "$PENDING" 2>/dev/null
 		log "could not repoint the link at $from; will retry ($1)"
 		return 1
 	fi
 	"$SYNC" # the link swap is durable before anything is deleted
-	write_last rolled_back "$target" "$1"
+	write_last rolled_back "$target" "$reason"
 	# A build that failed probation is never a rollback target and must not push
 	# a good version out of retention.
 	if valid_version "$target" && [ "$target" != "$from" ]; then
@@ -144,7 +177,7 @@ auto_rollback() {
 	rm -f "$CLAIM"
 	clear_counters
 	"$SYNC"
-	log "rolled back $target -> $from ($1); removed $target"
+	log "rolled back $target -> $from ($reason); removed $target"
 	return 0
 }
 
@@ -281,6 +314,46 @@ rollback)
 	"$SYSTEMCTL" restart mqtt-rpcd
 	exit 0
 	;;
+place)
+	# install.sh's binary-and-link step, run on the reader after it uploads
+	# versions/.mqtt-rpcd-<version>.upload. Every step is safe to interrupt
+	# (an SSH drop) and to re-run.
+	v="${2:-}"
+	up="$DIR/versions/.mqtt-rpcd-$v.upload"
+	if ! valid_version "$v" || [ ! -f "$up" ]; then
+		log "place: no upload for version '$v' at $up"
+		exit 1
+	fi
+	# No guard tick may act while the install changes what it acts on.
+	# install.sh starts the timer again when it finishes, however it finishes.
+	"$SYSTEMCTL" stop mqtt-rpcd-guard.timer 2>/dev/null || true
+	# An SSH install supersedes any remote update on probation. Disarm BEFORE
+	# the new binary moves in: a rollback of a same-version update would
+	# otherwise delete the binary just installed. The claim goes first, since
+	# resume_claim turns a lone claim back into pending.
+	rm -f "$CLAIM" "$PENDING.commit" "$PENDING" "$STATE/starts" "$STATE/ticks"
+	"$SYNC"
+	if ! { chmod 755 "$up" && mv -f "$up" "$DIR/versions/mqtt-rpcd-$v"; }; then
+		log "place: could not put $v in place"
+		exit 1
+	fi
+	# A pre-versioning install has a plain binary at the link path. Keep it as
+	# the "legacy" rollback target by hardlink, so it never leaves the link
+	# path: the symlink rename below replaces it in one step.
+	if [ -f "$LINK" ] && [ ! -L "$LINK" ]; then
+		ln -f "$LINK" "$DIR/versions/mqtt-rpcd-legacy" || {
+			log "place: could not keep the old binary as legacy; nothing switched"
+			exit 1
+		}
+	fi
+	if ! point_at "$v"; then
+		log "place: could not point the link at $v"
+		exit 1
+	fi
+	"$SYNC"
+	log "installed $v"
+	exit 0
+	;;
 recover)
 	mkdir -p "$STATE"
 	: >"$RECOVER_LOG" # the last run only
@@ -310,7 +383,7 @@ recover)
 	exit "$rc"
 	;;
 *)
-	echo "usage: $0 start|tick|list|rollback <version>|recover" >&2
+	echo "usage: $0 start|tick|list|rollback <version>|place <version>|recover" >&2
 	exit 2
 	;;
 esac

frontend/src/components/scandevices/ReaderSoftwarePanel.test.tsx

@@ -126,16 +126,46 @@ describe('ReaderSoftwarePanel', () => {
       startError: {
         response: {
           status: 422,
+          // The standard error envelope.
           data: {
-            error: 'update_refused',
-            reason: 'an update is already in progress',
+            error: {
+              type: 'validation_error',
+              status: 422,
+              detail: 'an update is already in progress',
+            },
           },
         },
       },
     });
     expect(screen.getByText(/an update is already in progress/)).toBeInTheDocument();
   });
 
+  it('does not say "nothing replaced" for a target left running unguarded', () => {
+    setup({
+      software: {
+        daemon: {
+          version: 'v2',
+          update: { state: 'unguarded', target: 'v2', error: 'no_commit_no_from' },
+        },
+        supports_update: true,
+      },
+    });
+    expect(screen.getByText(/v2 is running but never confirmed/i)).toBeInTheDocument();
+    expect(screen.queryByText(/nothing replaced/i)).not.toBeInTheDocument();
+  });
+
+  it('explains an unguarded outcome while watching it land', () => {
+    setup({
+      software: {
+        daemon: { version: 'v2', update: { state: 'unguarded', target: 'v2' } },
+        supports_update: true,
+      },
+      target: 'v2',
+      outcome: 'unguarded',
+    });
+    expect(screen.getByText(/no earlier version to roll back to/i)).toBeInTheDocument();
+  });
+
   it('tells a non-admin why the update was not started', () => {
     setup({
       software: { daemon: { version: 'v1' }, supports_update: true },