fix(update): never apply unverified staging — close the OTA truncated-binary race

Root cause of .198 bricking on the 1.7.103 OTA: two concurrent
update.download RPCs shared one staging file, cancel_download wiped
staging mid-flight, a third download began re-filling it, and
apply_update mv'd the 3-second-old 17MB partial of the 49MB binary
into /usr/local/bin -> SEGV boot loop (236 restarts, no rollback).

- Single-flight UPDATE_OP_LOCK across download/apply; concurrent
  callers get an explicit 'already running' error.
- apply_update now requires the .download-complete marker AND
  re-verifies every staged component (size + SHA-256 + BLAKE3)
  against the manifest before touching the system.
- cancel_download only wipes staging when no operation holds the
  lock; otherwise it just flags the in-flight loop to bail.
- Fixed the 'file already complete' path in
  download_component_resumable, which skipped verification and fell
  through to the bogus 'download failed without a captured error'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

core/archipelago/src/update.rs

@@ -24,6 +24,16 @@ pub static DOWNLOAD_CANCEL: AtomicBool = AtomicBool::new(false);
 /// confidence than "looks stuck at 0%".
 pub static DOWNLOAD_PROGRESS_AT: AtomicU64 = AtomicU64::new(0);
 
+/// Serializes the mutating update operations (download, apply, and the
+/// staging wipe in cancel). The .198 v1.7.103 bricking (2026-07-18) was
+/// exactly this race: two concurrent `update.download` RPCs shared one
+/// staging file, a cancel wiped staging mid-flight, a third download began
+/// re-filling it, and `apply_update` mv'd the 3-second-old 17MB partial of
+/// a 49MB binary into /usr/local/bin → SEGV boot loop. Writers take this
+/// via `try_lock` so a concurrent caller gets an explicit "already running"
+/// error instead of silently interleaving.
+static UPDATE_OP_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(());
+
 fn now_ms() -> u64 {
     use std::time::{SystemTime, UNIX_EPOCH};
     SystemTime::now()
@@ -976,6 +986,9 @@ pub async fn dismiss_update(data_dir: &Path) -> Result<()> {
 /// verified over the complete file at the end of each component, so a
 /// partially-corrupt resume still fails cleanly.
 pub async fn download_update(data_dir: &Path) -> Result<DownloadProgress> {
+    let _op = UPDATE_OP_LOCK.try_lock().map_err(|_| {
+        anyhow::anyhow!("another update operation (download or apply) is already running")
+    })?;
     let mut state = load_state(data_dir).await?;
     if state.available_update.is_none() {
         state = check_for_updates(data_dir).await?;
@@ -1133,7 +1146,6 @@ async fn download_component_resumable(
     dest: &Path,
     prior_total: u64,
 ) -> Result<()> {
-    use sha2::{Digest, Sha256};
     use tokio::io::AsyncWriteExt;
     const MAX_ATTEMPTS: u32 = 6;
     const BACKOFFS: [u64; 5] = [5, 15, 30, 60, 120];
@@ -1145,8 +1157,19 @@ async fn download_component_resumable(
             Err(_) => 0,
         };
         if existing_len >= component.size_bytes {
-            // File is already complete — break out and go verify.
-            break;
+            // File is already complete (a resumed run finished it, or a
+            // leftover from an earlier attempt) — verify it instead of
+            // trusting it. The old code `break`d here, which skipped
+            // verification entirely AND landed on the error return below
+            // ("download failed without a captured error").
+            match verify_component_on_disk(component, dest).await {
+                Ok(()) => return Ok(()),
+                Err(e) => {
+                    let _ = tokio::fs::remove_file(dest).await;
+                    last_err = Some(e);
+                    continue;
+                }
+            }
         }
         if attempt > 1 {
             let delay = BACKOFFS[(attempt as usize - 2).min(BACKOFFS.len() - 1)];
@@ -1294,44 +1317,83 @@ async fn download_component_resumable(
             continue;
         }
 
-        // Full file — verify hash.
-        let bytes = tokio::fs::read(dest)
-            .await
-            .context("read staging file for hash check")?;
-        let hash = hex::encode(Sha256::digest(&bytes));
-        if hash == component.sha256 {
-            // DHT Phase 1: if the manifest also pins a BLAKE3 digest, it must
-            // match too. SHA-256 stays the mandatory gate during migration;
-            // BLAKE3 is the hash the iroh swarm will fetch/verify by, so a
-            // present-but-wrong BLAKE3 means the bytes aren't swarm-consistent
-            // — treat it like a SHA mismatch and re-download.
-            if let Some(b3) = component.blake3.as_deref() {
-                let expected = b3.trim().strip_prefix("blake3:").unwrap_or(b3.trim());
-                let actual = crate::content_hash::blake3_hex(&bytes);
-                if !actual.eq_ignore_ascii_case(expected) {
-                    let _ = tokio::fs::remove_file(dest).await;
-                    last_err = Some(anyhow::anyhow!(
-                        "BLAKE3 mismatch for {}: expected {}, got {}",
-                        component.name,
-                        expected,
-                        actual
-                    ));
-                    continue;
-                }
+        // Full file — verify hashes. On mismatch the file on disk is
+        // garbage: nuke it and start over from scratch on the next attempt.
+        match verify_component_on_disk(component, dest).await {
+            Ok(()) => return Ok(()),
+            Err(e) => {
+                let _ = tokio::fs::remove_file(dest).await;
+                last_err = Some(e);
             }
-            return Ok(());
         }
-        // SHA mismatch — the file on disk is garbage. Nuke it and
-        // start over from scratch on the next attempt.
-        let _ = tokio::fs::remove_file(dest).await;
-        last_err = Some(anyhow::anyhow!(
+    }
+    Err(last_err.unwrap_or_else(|| anyhow::anyhow!("download failed without a captured error")))
+}
+
+/// Verify a fully-downloaded component file on disk: SHA-256 is the
+/// mandatory gate; when the manifest also pins a BLAKE3 digest it must
+/// match too (BLAKE3 is the hash the iroh swarm fetches/verifies by, so
+/// a present-but-wrong BLAKE3 means the bytes aren't swarm-consistent —
+/// treated exactly like a SHA mismatch). Err = mismatch; the caller
+/// decides whether to remove the file and retry.
+async fn verify_component_on_disk(component: &ComponentUpdate, dest: &Path) -> Result<()> {
+    use sha2::{Digest, Sha256};
+    let bytes = tokio::fs::read(dest)
+        .await
+        .context("read staging file for hash check")?;
+    let hash = hex::encode(Sha256::digest(&bytes));
+    if hash != component.sha256 {
+        anyhow::bail!(
             "SHA256 mismatch for {}: expected {}, got {}",
             component.name,
             component.sha256,
             hash
-        ));
+        );
     }
-    Err(last_err.unwrap_or_else(|| anyhow::anyhow!("download failed without a captured error")))
+    if let Some(b3) = component.blake3.as_deref() {
+        let expected = b3.trim().strip_prefix("blake3:").unwrap_or(b3.trim());
+        let actual = crate::content_hash::blake3_hex(&bytes);
+        if !actual.eq_ignore_ascii_case(expected) {
+            anyhow::bail!(
+                "BLAKE3 mismatch for {}: expected {}, got {}",
+                component.name,
+                expected,
+                actual
+            );
+        }
+    }
+    Ok(())
+}
+
+/// Re-verify every manifest component against the bytes actually sitting
+/// in staging, immediately before install. The download path verifies as
+/// it goes, but staging can change between download and apply — on .198
+/// (v1.7.103, 2026-07-18) a concurrent download was re-filling a wiped
+/// staging dir when apply ran, and a 17MB partial of the 49MB binary got
+/// installed. This apply-time gate is the one that must never be skipped.
+async fn verify_staged_components(staging_dir: &Path, manifest: &UpdateManifest) -> Result<()> {
+    for component in &manifest.components {
+        let dest = staging_dir.join(&component.name);
+        let len = tokio::fs::metadata(&dest).await.map(|m| m.len()).unwrap_or(0);
+        if len != component.size_bytes {
+            anyhow::bail!(
+                "staged component {} is {} bytes but the manifest says {} — \
+                 refusing to apply (incomplete or concurrently-rewritten download)",
+                component.name,
+                len,
+                component.size_bytes
+            );
+        }
+        verify_component_on_disk(component, &dest)
+            .await
+            .with_context(|| {
+                format!(
+                    "staged component {} failed verification — refusing to apply",
+                    component.name
+                )
+            })?;
+    }
+    Ok(())
 }
 
 /// Cancel an in-flight download. Sets the cancellation flag so the
@@ -1343,11 +1405,21 @@ pub async fn cancel_download(data_dir: &Path) -> Result<()> {
     DOWNLOAD_CANCEL.store(true, Ordering::Relaxed);
     DOWNLOAD_BYTES.store(0, Ordering::Relaxed);
     DOWNLOAD_TOTAL.store(0, Ordering::Relaxed);
+    // Only wipe staging when no download/apply holds the op lock. Wiping
+    // under a live operation is how .198 ended up applying a re-filling
+    // staging dir; with the lock held elsewhere we just set the cancel
+    // flag and let the in-flight loop bail at its next chunk boundary
+    // (partials are size+hash revalidated on the next resume anyway).
     let staging = data_dir.join("update-staging");
-    let wiped = if staging.exists() {
-        tokio::fs::remove_dir_all(&staging).await.is_ok()
-    } else {
-        false
+    let wiped = match UPDATE_OP_LOCK.try_lock() {
+        Ok(_op) => {
+            if staging.exists() {
+                tokio::fs::remove_dir_all(&staging).await.is_ok()
+            } else {
+                false
+            }
+        }
+        Err(_) => false,
     };
     // Clear the "downloaded, ready to apply" marker too — a canceled
     // download is not a staged update.
@@ -1398,11 +1470,34 @@ pub(crate) async fn host_sudo(args: &[&str]) -> Result<std::process::ExitStatus>
 
 /// Apply a downloaded update. Backs up current binaries, replaces with staged versions.
 pub async fn apply_update(data_dir: &Path) -> Result<()> {
+    let _op = UPDATE_OP_LOCK.try_lock().map_err(|_| {
+        anyhow::anyhow!("another update operation (download or apply) is already running")
+    })?;
     let staging_dir = data_dir.join("update-staging");
     if !staging_dir.exists() {
         anyhow::bail!("No staged update found. Download first.");
     }
 
+    // Gate 1: the completion marker is written only after EVERY component
+    // downloaded and hash-verified. A staging dir without it is a partial
+    // or in-flight download — exactly what got installed on .198.
+    if !has_staged_update(data_dir).await {
+        anyhow::bail!(
+            "Staged update is incomplete (no completion marker) — download the update again before applying"
+        );
+    }
+
+    // Gate 2: re-verify the actual staged bytes against the manifest.
+    let manifest = load_state(data_dir)
+        .await?
+        .available_update
+        .ok_or_else(|| {
+            anyhow::anyhow!(
+                "no update manifest in state to verify staged files against — re-download the update"
+            )
+        })?;
+    verify_staged_components(&staging_dir, &manifest).await?;
+
     let backup_dir = data_dir.join("update-backup");
     fs::create_dir_all(&backup_dir)
         .await
@@ -2443,6 +2538,65 @@ mod tests {
         assert!(!persisted.update_in_progress);
     }
 
+    #[tokio::test]
+    async fn test_apply_refuses_unmarked_staging() {
+        // Regression: .198 v1.7.103 bricking — apply ran against a staging
+        // dir that a concurrent download was still filling. Without the
+        // .download-complete marker, apply must refuse before touching
+        // anything.
+        let dir = tempfile::tempdir().unwrap();
+        let staging = dir.path().join("update-staging");
+        tokio::fs::create_dir_all(&staging).await.unwrap();
+        tokio::fs::write(staging.join("archipelago"), b"partial")
+            .await
+            .unwrap();
+        let err = apply_update(dir.path()).await.unwrap_err();
+        assert!(
+            err.to_string().contains("completion marker"),
+            "got: {err:#}"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_apply_refuses_staged_bytes_that_mismatch_manifest() {
+        // Marker present (a complete download once existed) but the staged
+        // bytes no longer match the manifest — apply must re-verify and
+        // refuse rather than install whatever is on disk.
+        let dir = tempfile::tempdir().unwrap();
+        let staging = dir.path().join("update-staging");
+        tokio::fs::create_dir_all(&staging).await.unwrap();
+        tokio::fs::write(staging.join(STAGED_COMPLETE_MARKER), b"1")
+            .await
+            .unwrap();
+        tokio::fs::write(staging.join("archipelago"), b"truncated-garbage")
+            .await
+            .unwrap();
+        let state = UpdateState {
+            available_update: Some(UpdateManifest {
+                version: "999.0.0".to_string(),
+                release_date: "2026-07-18".to_string(),
+                changelog: vec![],
+                components: vec![ComponentUpdate {
+                    name: "archipelago".to_string(),
+                    current_version: "1.0.0".to_string(),
+                    new_version: "999.0.0".to_string(),
+                    download_url: "http://example.invalid/archipelago".to_string(),
+                    sha256: "0".repeat(64),
+                    size_bytes: 49_949_048,
+                    blake3: None,
+                }],
+            }),
+            update_in_progress: true,
+            ..UpdateState::default()
+        };
+        save_state(dir.path(), &state).await.unwrap();
+        let err = apply_update(dir.path()).await.unwrap_err();
+        assert!(
+            err.to_string().contains("refusing to apply"),
+            "got: {err:#}"
+        );
+    }
+
     #[tokio::test]
     async fn test_dismiss_update_clears_available() {
         let dir = tempfile::tempdir().unwrap();