diff --git a/core/archipelago/src/update.rs b/core/archipelago/src/update.rs index 956860d7..ad99aef6 100644 --- a/core/archipelago/src/update.rs +++ b/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 { + 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 /// 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();