From dd2e198996cbeab958ece4b363f865a7c7ffc112 Mon Sep 17 00:00:00 2001 From: Airead Date: Wed, 29 Jul 2026 16:15:29 +0800 Subject: [PATCH 1/3] =?UTF-8?q?inject:=20per-target=20exposure=20lock=20?= =?UTF-8?q?=E2=80=94=20refuse=20overlapping=20-r=20runs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two rapid `vt inject -r FILE` runs left the plaintext on disk forever: the second run read the first run's exposed plaintext and snapshotted it as its "ciphertext" backup, and its supervisor — firing last — renamed that plaintext back over the target, with no backup or sidecar left for --recover to find. The ciphertext backup's name is now deterministic per target (`.{name}.vt-backup`), so the existing O_CREAT|O_EXCL create doubles as a per-target exposure lock: it exists exactly while an exposure window is open (created before any plaintext hits disk, consumed by rename() on every restore path). A second inject of the same file fails EEXIST and is refused with guidance picked from the sidecar: window still live (retry shortly), supervisor dead (run `vt inject --recover`), or untracked (manual mv). A pre-decrypt check refuses before spending a Touch ID / phone approval, and a `-r` file with zero vt:// records is refused outright — nothing to decrypt means the wrong file, or plaintext left by a broken exposure. --- CLAUDE.md | 8 +- README.md | 8 ++ src/client.rs | 317 ++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 325 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f6cc255..6ac1069 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,7 +44,13 @@ pin the transport. See [`config.example.toml`](config.example.toml) and behavior when adding a new command. - `vt inject -r` restores ciphertext through its self-exec'd restore supervisor. `vt inject --recover` must remain unauthenticated and only move - the ciphertext backup back over the target. + the ciphertext backup back over the target. The ciphertext backup's + deterministic per-target name (`.{name}.vt-backup`) IS the exposure lock: + it is created O_EXCL before any plaintext hits disk, and every restore path + must consume it via rename() — never copy+delete — so an overlapping + `inject -r` of the same file fails EEXIST instead of snapshotting exposed + plaintext as its "ciphertext" backup. Do not reintroduce a randomized + backup name, and keep refusing `-r` files with zero `vt://` records. - DEK caching is opt-in, requires `CACHE_SECKEY`, and uses the Worker-selected TTLs. A cache hit is not a phone approval; preserve its audit row and IP binding. Every write mints an immutable `cache_group_id` (the handle the admin diff --git a/README.md b/README.md index 95b6fd1..508e290 100644 --- a/README.md +++ b/README.md @@ -167,6 +167,14 @@ Options: or rebooted restore supervisor left decrypted. Needs no auth (it only moves the ciphertext backup back). Safe to run from a login/boot hook. +Overlap protection: only one exposure window per file may be open at a time. +A second `inject -r` of the same file is refused while the first window is +open — retry after it closes (the error says how long). If a previous run's +restore supervisor died (crash/reboot), the refusal points at +`vt inject --recover`. A file containing no `vt://` records is also refused: +there is nothing to decrypt, which usually means the wrong file — or plaintext +left behind by a broken exposure. + ### SSH Agent VT can act as an SSH agent, storing private keys in VT's encrypted diff --git a/src/client.rs b/src/client.rs index 50a6935..30969e5 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1316,6 +1316,33 @@ pub async fn inject( } None => String::new(), }; + // Exposure-overlap pre-checks, before the decrypt round trip spends a + // Touch ID / phone approval. The O_EXCL backup create below (step 2) is + // the authoritative, race-free gate; these two only fail earlier and + // louder: + // - an existing backup means the target is mid-exposure (or a dead + // supervisor left it plaintext) — arming again would snapshot that + // plaintext as "ciphertext" and restore it permanently; + // - zero vt:// records means there is nothing to decrypt: either the + // wrong file, or plaintext left behind by a broken exposure. + if let Some(file) = replace_file.as_ref() { + let backup = exposure_backup_path(file)?; + if backup.symlink_metadata().is_ok() { + return Err(exposure_conflict_refusal( + file, + &backup, + inject_state_dir().as_deref(), + )); + } + if iter_vt_urls(&replace_file_content).next().is_none() { + return Err(anyhow::anyhow!( + "{} contains no vt:// records — nothing to decrypt. Was it \ + left decrypted by another process, or is this the wrong file?", + file + )); + } + } + // Keep the original (ciphertext) bytes for the backup; the same bytes we // just read and are about to decrypt. Cheap clone (empty when no -r file). let orig_file_bytes = replace_file_content.clone().into_bytes(); @@ -1351,8 +1378,13 @@ pub async fn inject( // Plaintext exposure protocol when -r is set: // 1. Open target with O_NOFOLLOW, stat to capture mode + reject non-regular. - // 2. Write a backup file alongside the target (ciphertext copy, randomized - // hidden name, O_CREAT|O_EXCL|O_NOFOLLOW, mode = orig). + // 2. Write a backup file alongside the target (ciphertext copy, + // O_CREAT|O_EXCL|O_NOFOLLOW, mode = orig) at the DETERMINISTIC + // per-target path `.{name}.vt-backup`. The backup doubles as the + // exposure lock (see `exposure_backup_path`): a second inject of the + // same target fails EEXIST here instead of snapshotting the exposed + // plaintext as its "ciphertext" backup — which its supervisor would + // later restore permanently. // 3. Spawn the detached supervisor before any plaintext can hit disk. // Failure here deletes the backup and aborts — target stays ciphertext. // 4. Write the tmp file (plaintext, same hidden-name rules as backup). @@ -1388,7 +1420,10 @@ pub async fn inject( rand::thread_rng().fill_bytes(&mut rnd); } let suffix: String = rnd.iter().map(|b| format!("{:02x}", b)).collect(); - let backup_path = dir.join(format!(".{}.vt-backup-{}", file_name, suffix)); + // Backup: deterministic per-target name (the exposure lock). Tmp and + // sidecar keep the random suffix — the tmp is consumed by rename() + // within this function and a leftover must not block the next inject. + let backup_path = exposure_backup_path(replace_file_path)?; let tmp_path = dir.join(format!(".{}.vt-tmp-{}", file_name, suffix)); // Crash-recovery sidecar path in the state dir. Computed up front so // the supervisor gets it at spawn and can delete it after a normal @@ -1405,15 +1440,30 @@ pub async fn inject( // the single O_NOFOLLOW open above) — NOT a fresh re-read of the target, // which would be a TOCTOU window for a directory-writable attacker. { - let mut backup_file = std::fs::OpenOptions::new() + let mut backup_file = match std::fs::OpenOptions::new() .write(true) .create_new(true) .custom_flags(libc::O_NOFOLLOW) .mode(orig_mode) .open(&backup_path) - .with_context(|| { - format!("Failed to create backup file: {}", backup_path.display()) - })?; + { + Ok(f) => f, + // EEXIST: another exposure of this target is open (or a dead + // supervisor left one). The pre-decrypt check already refused + // the common case; this is the authoritative race-free gate. + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + return Err(exposure_conflict_refusal( + replace_file_path, + &backup_path, + inject_state_dir().as_deref(), + )); + } + Err(e) => { + return Err(e).with_context(|| { + format!("Failed to create backup file: {}", backup_path.display()) + }); + } + }; backup_file.write_all(&orig_file_bytes).with_context(|| { format!( "Failed to copy content to backup: {}", @@ -1642,6 +1692,127 @@ fn write_inject_sidecar(path: &std::path::Path, sc: &InjectSidecar) -> Result<() Ok(()) } +// ── Per-target exposure lock ──────────────────────────────────────────────── +// +// Overlapping `inject -r` runs on the same target are unsafe by construction: +// the second run would read the first run's exposed plaintext, snapshot it as +// its "ciphertext" backup, and its supervisor — firing last — would rename +// that plaintext back over the target permanently, with no sidecar or backup +// left for `--recover` to find. The lock that prevents this is the backup +// file itself: its name is deterministic per target, it is created O_EXCL +// before any plaintext hits disk, and it is consumed by rename() on every +// restore path (supervisor, immediate_restore, --recover). Its existence +// therefore means exactly "an exposure window is open, or a dead supervisor +// left the target plaintext" — and a new inject must refuse. + +/// Deterministic ciphertext-backup path for `target`: `dir/.{name}.vt-backup`. +/// Deliberately NOT randomized — see the module comment above. Restore paths +/// must keep consuming it via rename(), never copy+delete (a copy would leave +/// the lock behind or release it before the target is ciphertext again). +fn exposure_backup_path(target: &str) -> Result { + let orig_path = std::path::Path::new(target); + let dir = orig_path + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| std::path::Path::new(".")); + let file_name = orig_path + .file_name() + .ok_or_else(|| anyhow::anyhow!("Invalid replace file path: {}", target))? + .to_string_lossy() + .into_owned(); + Ok(dir.join(format!(".{}.vt-backup", file_name))) +} + +/// Why an inject over an existing backup was refused — picks the operator +/// guidance in the refusal message. +#[derive(Debug, PartialEq, Eq)] +enum ExposureConflict { + /// A supervisor is presumed alive; the window ends in ~`remaining_s`. + Live { remaining_s: u64 }, + /// Deadline + grace elapsed with the backup still present: the supervisor + /// died and the target may still be plaintext. + Orphaned, + /// No sidecar records this backup (its write failed, or it was cleaned); + /// manual restore is the only guidance left. + Untracked, +} + +/// Pure classification of an exposure conflict from the sidecar (if one still +/// records the colliding backup) and the clock. Mirrors [`plan_recovery`]'s +/// liveness rule: within deadline + grace the supervisor is presumed alive. +fn classify_exposure_conflict(sidecar: Option<&InjectSidecar>, now_ms: u64) -> ExposureConflict { + match sidecar { + Some(sc) if now_ms < sc.deadline_ms.saturating_add(RECOVER_GRACE_MS) => { + ExposureConflict::Live { + remaining_s: sc.deadline_ms.saturating_sub(now_ms).div_ceil(1000), + } + } + Some(_) => ExposureConflict::Orphaned, + None => ExposureConflict::Untracked, + } +} + +/// Best-effort scan of `state_dir` for the sidecar recording `backup` as its +/// ciphertext backup. Unreadable entries are skipped: this only upgrades the +/// quality of a refusal message, never gates the refusal itself. +fn find_sidecar_for_backup( + state_dir: &std::path::Path, + backup: &std::path::Path, +) -> Option { + let want = absolutize(backup); + for entry in std::fs::read_dir(state_dir).ok()?.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("json") { + continue; + } + let sc: Option = std::fs::read(&path) + .ok() + .and_then(|b| serde_json::from_slice(&b).ok()); + if let Some(sc) = sc { + if std::path::Path::new(&sc.backup) == want.as_path() { + return Some(sc); + } + } + } + None +} + +/// Build the refusal error for a colliding backup at `backup`. Consults the +/// sidecar in `state_dir` — falling back to the target's own directory, where +/// the no-home-dir fallback sidecar lives — purely to pick the guidance. +fn exposure_conflict_refusal( + target: &str, + backup: &std::path::Path, + state_dir: Option<&std::path::Path>, +) -> anyhow::Error { + let sidecar = [state_dir, backup.parent()] + .into_iter() + .flatten() + .find_map(|d| find_sidecar_for_backup(d, backup)); + match classify_exposure_conflict(sidecar.as_ref(), now_ms()) { + ExposureConflict::Live { remaining_s } => anyhow::anyhow!( + "{} is mid-exposure by another vt inject (auto-restore in ~{}s); \ + retry after the window closes", + target, + remaining_s + ), + ExposureConflict::Orphaned => anyhow::anyhow!( + "a previous exposure of {} was never restored (its supervisor is \ + gone) and the file may still be plaintext; run `vt inject \ + --recover` first", + target + ), + ExposureConflict::Untracked => anyhow::anyhow!( + "found leftover backup {} for {}; verify the file's state, then \ + restore the ciphertext manually: mv '{}' '{}'", + backup.display(), + target, + backup.display(), + target + ), + } +} + /// Sweep the sidecar dir and restore any orphaned plaintext exposure. Invoked /// by `vt inject --recover` (run at login/boot). Never errors on an individual /// bad entry — it logs and moves on so one corrupt sidecar can't wedge the sweep. @@ -2502,4 +2673,136 @@ mod tests { // A relative path becomes absolute (prefixed by some cwd). assert!(absolutize(std::path::Path::new("rel/x")).is_absolute()); } + + #[test] + fn exposure_backup_path_is_deterministic_per_target() { + // Same target → same path: this determinism IS the exposure lock. + let a = exposure_backup_path("/etc/app/config.yaml").unwrap(); + let b = exposure_backup_path("/etc/app/config.yaml").unwrap(); + assert_eq!(a, b); + assert_eq!( + a, + std::path::PathBuf::from("/etc/app/.config.yaml.vt-backup") + ); + // A bare file name resolves beside it in the cwd. + assert_eq!( + exposure_backup_path("config.yaml").unwrap(), + std::path::Path::new(".").join(".config.yaml.vt-backup") + ); + // Same basename in different dirs must not collide. + assert_ne!( + exposure_backup_path("/a/config.yaml").unwrap(), + exposure_backup_path("/b/config.yaml").unwrap() + ); + // A path with no file name is refused. + assert!(exposure_backup_path("/").is_err()); + } + + #[test] + fn classify_exposure_conflict_branches() { + let sc = InjectSidecar { + target: "/t/f".into(), + backup: "/t/.f.vt-backup".into(), + tmp: "/t/.f.vt-tmp-ab".into(), + deadline_ms: 100_000, + }; + // Before the deadline: live, remaining seconds round up. + assert_eq!( + classify_exposure_conflict(Some(&sc), 98_500), + ExposureConflict::Live { remaining_s: 2 } + ); + // Inside the grace window: still presumed live (about to fire). + assert_eq!( + classify_exposure_conflict(Some(&sc), 100_000 + RECOVER_GRACE_MS - 1), + ExposureConflict::Live { remaining_s: 0 } + ); + // Past deadline + grace with the backup still present: dead supervisor. + assert_eq!( + classify_exposure_conflict(Some(&sc), 100_000 + RECOVER_GRACE_MS), + ExposureConflict::Orphaned + ); + assert_eq!( + classify_exposure_conflict(None, 0), + ExposureConflict::Untracked + ); + } + + #[test] + fn find_sidecar_for_backup_matches_and_skips_garbage() { + let dir = std::env::temp_dir().join(format!("vt-inject-lock-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let backup = "/abs/.secret.env.vt-backup"; + let sc = InjectSidecar { + target: "/abs/secret.env".into(), + backup: backup.into(), + tmp: "/abs/.secret.env.vt-tmp-ab".into(), + deadline_ms: 42, + }; + std::fs::write(dir.join("aa.json"), serde_json::to_vec(&sc).unwrap()).unwrap(); + std::fs::write(dir.join("bb.json"), b"not json").unwrap(); // skipped + std::fs::write(dir.join("cc.txt"), b"wrong extension").unwrap(); // ignored + + assert_eq!( + find_sidecar_for_backup(&dir, std::path::Path::new(backup)), + Some(sc) + ); + assert_eq!( + find_sidecar_for_backup(&dir, std::path::Path::new("/abs/.other.vt-backup")), + None + ); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn exposure_conflict_refusal_picks_the_right_guidance() { + let dir = std::env::temp_dir().join(format!("vt-inject-refusal-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let target = dir.join("secret.env"); + let target_str = target.to_str().unwrap(); + let backup = exposure_backup_path(target_str).unwrap(); + std::fs::write(&backup, b"ciphertext").unwrap(); + + // A sidecar whose deadline is long past → Orphaned → names --recover. + let state = dir.join("state"); + std::fs::create_dir_all(&state).unwrap(); + let sc = InjectSidecar { + target: target_str.to_string(), + backup: backup.to_string_lossy().into_owned(), + tmp: "unused".into(), + deadline_ms: 0, + }; + std::fs::write(state.join("aa.json"), serde_json::to_vec(&sc).unwrap()).unwrap(); + let err = exposure_conflict_refusal(target_str, &backup, Some(&state)); + assert!(err.to_string().contains("--recover"), "got: {err}"); + + // A live sidecar (far-future deadline) → retry guidance. + std::fs::write( + state.join("aa.json"), + serde_json::to_vec(&InjectSidecar { + deadline_ms: u64::MAX / 2, + ..sc.clone() + }) + .unwrap(), + ) + .unwrap(); + let err = exposure_conflict_refusal(target_str, &backup, Some(&state)); + assert!(err.to_string().contains("mid-exposure"), "got: {err}"); + + // No sidecar anywhere → manual-restore guidance naming the backup. + std::fs::remove_file(state.join("aa.json")).unwrap(); + let err = exposure_conflict_refusal(target_str, &backup, Some(&state)); + assert!( + err.to_string().contains(&backup.display().to_string()), + "got: {err}" + ); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn no_vt_records_means_nothing_to_decrypt() { + // The -r guard: zero vt:// records refuses the exposure protocol + // (either the wrong file, or plaintext left by a broken exposure). + assert!(iter_vt_urls("plain: text\nno records here").next().is_none()); + assert!(iter_vt_urls("key: vt://0abc").next().is_some()); + } } From 475cbb1cd80034c4e00f47309c4c4d465aca93ce Mon Sep 17 00:00:00 2001 From: Airead Date: Wed, 29 Jul 2026 20:25:22 +0800 Subject: [PATCH 2/3] inject: bind every restore path to its backup's generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deterministic backup path made the sidecar ambiguous in the other direction: a stale record (crash between a restore's rename and the sidecar's removal) could pair with a LATER exposure's backup at the same path, and a delayed supervisor (suspend pauses its monotonic sleep while the wall-clock deadline lapses) could blindly rename a successor's backup — either way consuming the only ciphertext copy and, in the worst interleaving, stranding plaintext with no recovery record. Four guards, from review: - every new sidecar records its backup's (dev, ino); arming aborts before any plaintext if the id cannot be captured - recovery re-checks the id plus an mtime ordering bound (a legitimate backup always predates its own deadline) — the bound is all an id-less legacy record has - arming retires stale sidecars naming the path while the O_EXCL lock is held - the supervisor receives dev:ino in its argv and renames only the generation it armed for, keeping the sidecar when the restore fails or the backup's state is unknowable Also hardened: sidecar reads are bounded (O_NOFOLLOW|O_NONBLOCK, regular-file check, 128 KiB cap shared with the write side, so an over-cap record fails loudly at arm time instead of being skipped at recovery); only "not there" counts as backup-consumed — other stat errors keep the recovery record; orphan guidance for a no-home-dir fallback sidecar says manual restore instead of naming --recover, which never scans there; sidecar write failures warn on stderr. --- CLAUDE.md | 13 +- src/client.rs | 758 ++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 717 insertions(+), 54 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6ac1069..0b7b2ea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,8 +49,17 @@ pin the transport. See [`config.example.toml`](config.example.toml) and it is created O_EXCL before any plaintext hits disk, and every restore path must consume it via rename() — never copy+delete — so an overlapping `inject -r` of the same file fails EEXIST instead of snapshotting exposed - plaintext as its "ciphertext" backup. Do not reintroduce a randomized - backup name, and keep refusing `-r` files with zero `vt://` records. + plaintext as its "ciphertext" backup. Because that path is reused across + exposures, every new sidecar MUST carry its backup's `(dev, ino)` generation + id, recovery refuses a backup whose generation or mtime-vs-deadline ordering + its sidecar did not record (the ordering bound is all an id-less legacy + record has), arming retires stale sidecars naming the path, and the restore + supervisor only renames the `(dev, ino)` generation it armed for (a suspend + can delay its monotonic sleep past the wall-clock deadline) and keeps its + sidecar when a restore fails or the backup's state is unknowable — only + "not there" counts as consumed; other stat errors must never clean a + recovery record. Do not reintroduce a randomized backup name, and keep + refusing `-r` files with zero `vt://` records. - DEK caching is opt-in, requires `CACHE_SECKEY`, and uses the Worker-selected TTLs. A cache hit is not a phone approval; preserve its audit row and IP binding. Every write mints an immutable `cache_group_id` (the handle the admin diff --git a/src/client.rs b/src/client.rs index 30969e5..11d9ba2 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1401,7 +1401,7 @@ pub async fn inject( if let Some(replace_file_path) = &replace_file { use std::io::Write; - use std::os::unix::fs::OpenOptionsExt; + use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; let orig_path = std::path::Path::new(replace_file_path); let dir = orig_path @@ -1439,7 +1439,10 @@ pub async fn inject( // Step 2: write backup from the in-memory ORIGINAL bytes (captured at // the single O_NOFOLLOW open above) — NOT a fresh re-read of the target, // which would be a TOCTOU window for a directory-writable attacker. - { + // The open fd also yields the backup's (dev, ino) generation id for + // the sidecar, binding the record to THIS backup — the deterministic + // path alone is ambiguous across exposures of the same target. + let backup_id: (u64, u64) = { let mut backup_file = match std::fs::OpenOptions::new() .write(true) .create_new(true) @@ -1471,9 +1474,30 @@ pub async fn inject( ) })?; backup_file.sync_all().ok(); - } + // The id is REQUIRED on new records: silently degrading to None + // would put recovery back on existence-only matching and reopen + // the stale-sidecar race. No plaintext exists yet, so aborting + // (after releasing the lock) is safe. + match backup_file.metadata() { + Ok(m) => (m.dev(), m.ino()), + Err(e) => { + let _ = std::fs::remove_file(&backup_path); + return Err(e).with_context(|| { + format!( + "Failed to stat backup for its generation id: {}", + backup_path.display() + ) + }); + } + } + }; debug!("Created backup at: {}", backup_path.display()); + // Holding the exposure lock proves any older sidecar naming this + // backup path is stale; retire it before a concurrent `--recover` + // can pair its expired deadline with the backup just created. + retire_stale_sidecars_for(&backup_path, inject_state_dir().as_deref()); + // Step 3: arm supervisor before any plaintext touches disk. match spawn_restore_supervisor( timeout, @@ -1481,6 +1505,7 @@ pub async fn inject( &backup_path, replace_file_path, &sidecar_path, + backup_id, ) { Ok(()) => debug!("Restore supervisor armed (timeout={}s)", timeout), Err(e) => { @@ -1499,9 +1524,15 @@ pub async fn inject( backup: absolutize(&backup_path).to_string_lossy().into_owned(), tmp: absolutize(&tmp_path).to_string_lossy().into_owned(), deadline_ms: now_ms().saturating_add((timeout as u64).saturating_mul(1000)), + backup_id: Some(backup_id), }; if let Err(e) = write_inject_sidecar(&sidecar_path, &sidecar) { - debug!("inject sidecar not written ({e:#}); crash-recovery disabled for this run"); + // Loud on stderr, not debug!: the user is about to expose + // plaintext with no reboot recovery for it. + eprintln!( + "vt inject: warning: crash-recovery sidecar not written ({e:#}); \ + a crash or reboot during this window will NOT be auto-restored" + ); } // Step 4: write tmp (plaintext). @@ -1609,6 +1640,12 @@ struct InjectSidecar { /// Epoch ms after which the supervisor should already have restored; past /// this (plus a grace) a surviving backup means the supervisor died. deadline_ms: u64, + /// `(st_dev, st_ino)` of the backup at creation — the generation id that + /// distinguishes THIS exposure's backup from a successor at the same + /// deterministic path. `None` on records written by builds without the + /// id; those keep existence-only matching. + #[serde(default)] + backup_id: Option<(u64, u64)>, } /// Wait past the deadline by this much before `--recover` acts, so a @@ -1626,10 +1663,15 @@ enum RecoverAction { /// Pure recovery decision. `None` = leave it alone (an injection whose window /// has not yet elapsed — its supervisor is presumed still running). -fn plan_recovery(deadline_ms: u64, backup_exists: bool, now_ms: u64) -> Option { - if !backup_exists { +/// `backup_survives` must be the GENERATION-CHECKED answer from +/// [`probe_sidecar_backup`], not bare existence: the deterministic backup +/// path is reused by every exposure of a target. +fn plan_recovery(deadline_ms: u64, backup_survives: bool, now_ms: u64) -> Option { + if !backup_survives { // The supervisor (or immediate_restore) already renamed the backup - // over the target: the injection completed. Only the sidecar lingers. + // over the target — or the path now holds a NEWER exposure's backup + // (generation mismatch). Either way this sidecar's injection is + // over; only the sidecar lingers. return Some(RecoverAction::CleanStale); } if now_ms >= deadline_ms.saturating_add(RECOVER_GRACE_MS) { @@ -1680,6 +1722,15 @@ fn write_inject_sidecar(path: &std::path::Path, sc: &InjectSidecar) -> Result<() .with_context(|| format!("creating inject state dir {}", dir.display()))?; } let json = serde_json::to_vec(sc).context("serialize inject sidecar")?; + // Same bound as read_sidecar_bounded: writing a record recovery would + // later skip as unreadable is worse than writing none — the operator + // would believe crash recovery exists when it doesn't. + ensure!( + json.len() as u64 <= SIDECAR_MAX_BYTES, + "inject sidecar would be {} bytes, over the {} byte cap recovery reads", + json.len(), + SIDECAR_MAX_BYTES + ); let mut f = std::fs::OpenOptions::new() .write(true) .create_new(true) @@ -1704,6 +1755,20 @@ fn write_inject_sidecar(path: &std::path::Path, sc: &InjectSidecar) -> Result<() // restore path (supervisor, immediate_restore, --recover). Its existence // therefore means exactly "an exposure window is open, or a dead supervisor // left the target plaintext" — and a new inject must refuse. +// +// The sidecar shares the path's ambiguity in the other direction: it records +// the deterministic backup path, so a stale record (crash between a restore's +// rename() and the sidecar's removal) must never be matched against a LATER +// exposure's backup — and no restore path may consume one. Four complementary +// guards enforce that: arming retires matching stale sidecars while the lock +// is held; every new sidecar MUST carry its backup's (dev, ino) generation +// id, verified before recovery consumes anything; recovery refuses any +// backup modified past the record's deadline — the ordering bound that +// covers id-less legacy records and a successor reusing the old inode; +// and the supervisor re-checks the (dev, ino) it armed for before its +// timeout restore (see `supervisor_restore` — a suspend can delay its +// monotonic sleep past the wall-clock deadline, letting --recover and a new +// exposure run first). /// Deterministic ciphertext-backup path for `target`: `dir/.{name}.vt-backup`. /// Deliberately NOT randomized — see the module comment above. Restore paths @@ -1752,24 +1817,121 @@ fn classify_exposure_conflict(sidecar: Option<&InjectSidecar>, now_ms: u64) -> E } } -/// Best-effort scan of `state_dir` for the sidecar recording `backup` as its -/// ciphertext backup. Unreadable entries are skipped: this only upgrades the -/// quality of a refusal message, never gates the refusal itself. +/// What the sidecar's recorded backup path currently holds. +#[derive(Debug, PartialEq, Eq)] +enum BackupProbe { + /// The exact generation the sidecar recorded — safe to restore. + Ours, + /// Consumed, or replaced by a successor generation: this exposure is + /// settled and only the sidecar lingers. + Gone, + /// The stat failed for a reason other than "not there" (EACCES, EIO, a + /// sick mount): the truth is unknowable right now. Callers must KEEP the + /// sidecar and retry later — deleting it would silently turn a transient + /// error into permanently unrecoverable exposed plaintext. + Unknown, +} + +/// Probe whether the sidecar's recorded backup still exists AS THE GENERATION +/// THE SIDECAR RECORDED. The backup path is deterministic per target, so bare +/// existence is ambiguous: a stale sidecar (crash between a restore's +/// rename() and its own removal) sees the path recreated by the target's +/// NEXT exposure, and acting on the old deadline would consume that newer +/// exposure's backup — early-restoring ciphertext mid-window at best, +/// stranding plaintext with no backup at worst. The `(dev, ino)` id +/// disambiguates, backed by an mtime ordering bound: a legitimate backup is +/// written BEFORE its sidecar's deadline is even computed (deadline = arm +/// time + timeout), so a backup modified after the deadline is a successor. +/// The bound is the only generation signal an id-less legacy record has — +/// it cannot catch a successor created before the deadline; arm-time +/// retirement covers those — and the guard against a successor reusing the +/// old inode. +fn probe_sidecar_backup(sc: &InjectSidecar) -> BackupProbe { + use std::os::unix::fs::MetadataExt; + let md = match std::fs::symlink_metadata(&sc.backup) { + Ok(md) => md, + Err(e) + if matches!( + e.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::NotADirectory + ) => + { + return BackupProbe::Gone; + } + Err(_) => return BackupProbe::Unknown, + }; + // Generation id (always present on new records) must match. + if let Some((dev, ino)) = sc.backup_id { + if md.dev() != dev || md.ino() != ino { + return BackupProbe::Gone; + } + } + match md + .modified() + .ok() + .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) + { + Some(mtime) if (mtime.as_millis() as u64) > sc.deadline_ms => BackupProbe::Gone, + // At/before the deadline — or no mtime available, where we skip the + // ordering bound rather than strand a genuine record (the id check + // above still applies to new records). + _ => BackupProbe::Ours, + } +} + +/// Upper bound for one sidecar read AND write. Sized so no legitimate record +/// is ever refused — three PATH_MAX (4096 on Linux) paths under worst-case +/// `\uXXXX` JSON escaping is ~72 KiB — while still bounding what a planted +/// file can make the refusal-path scan read. [`write_inject_sidecar`] +/// enforces the same limit, so a record recovery would skip as unreadable is +/// refused loudly at arm time (before plaintext exposure) instead. +const SIDECAR_MAX_BYTES: u64 = 128 * 1024; + +/// Read and parse one sidecar file defensively: O_NOFOLLOW + O_NONBLOCK so a +/// planted symlink or FIFO can't redirect or hang the scan, a regular-file +/// check before reading, and a hard size cap. Needed because the refusal-path +/// scan can run over the TARGET's own directory (the no-home-dir fallback +/// sidecar lives there), which may be shared and attacker-writable — unlike +/// the 0700 state dir. `None` for anything unreadable or not sidecar-shaped. +fn read_sidecar_bounded(path: &std::path::Path) -> Option { + use std::io::Read; + use std::os::unix::fs::OpenOptionsExt; + let f = std::fs::OpenOptions::new() + .read(true) + .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK) + .open(path) + .ok()?; + let md = f.metadata().ok()?; + if !md.is_file() || md.len() > SIDECAR_MAX_BYTES { + return None; + } + let mut buf = Vec::new(); + // Cap the read too — fstat can't bind a file that grows after the check. + f.take(SIDECAR_MAX_BYTES).read_to_end(&mut buf).ok()?; + serde_json::from_slice(&buf).ok() +} + +/// Best-effort scan of `dir` for the sidecar recording `backup` — the CURRENT +/// generation of it, per [`probe_sidecar_backup`], so a stale record from +/// an earlier exposure of the same target can't hijack the classification. +/// Unreadable entries are skipped: this only upgrades the quality of a +/// refusal message, never gates the refusal itself. fn find_sidecar_for_backup( - state_dir: &std::path::Path, + dir: &std::path::Path, backup: &std::path::Path, ) -> Option { let want = absolutize(backup); - for entry in std::fs::read_dir(state_dir).ok()?.flatten() { + for entry in std::fs::read_dir(dir).ok()?.flatten() { let path = entry.path(); if path.extension().and_then(|e| e.to_str()) != Some("json") { continue; } - let sc: Option = std::fs::read(&path) - .ok() - .and_then(|b| serde_json::from_slice(&b).ok()); - if let Some(sc) = sc { - if std::path::Path::new(&sc.backup) == want.as_path() { + if let Some(sc) = read_sidecar_bounded(&path) { + // Unknown reads as no-match: this scan only picks a refusal + // message, and the conservative fallback is manual guidance. + if std::path::Path::new(&sc.backup) == want.as_path() + && probe_sidecar_backup(&sc) == BackupProbe::Ours + { return Some(sc); } } @@ -1777,18 +1939,52 @@ fn find_sidecar_for_backup( None } +/// Retire state-dir sidecars that record `backup` as their ciphertext backup. +/// Called right after the O_EXCL backup create — holding the exposure lock +/// proves the target has no open exposure, so any sidecar still naming this +/// deterministic path is stale (a crash landed between a restore's rename() +/// and the sidecar's removal). Left in place, its long-expired deadline would +/// tell a concurrent `--recover` to consume the backup just created. Best +/// effort, and complementary to the generation id: the sweep also covers the +/// (filesystem-dependent) case of the new backup reusing the old inode, while +/// the id covers a `--recover` that read the stale record before this sweep. +fn retire_stale_sidecars_for(backup: &std::path::Path, state_dir: Option<&std::path::Path>) { + let Some(dir) = state_dir else { return }; + let want = absolutize(backup); + let Ok(entries) = std::fs::read_dir(dir) else { return }; + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("json") { + continue; + } + if let Some(sc) = read_sidecar_bounded(&path) { + if std::path::Path::new(&sc.backup) == want.as_path() { + let _ = std::fs::remove_file(&path); + } + } + } +} + /// Build the refusal error for a colliding backup at `backup`. Consults the /// sidecar in `state_dir` — falling back to the target's own directory, where /// the no-home-dir fallback sidecar lives — purely to pick the guidance. +/// `--recover` sweeps only the state dir, so orphan guidance may name it only +/// for a sidecar found there; a fallback-dir orphan gets the manual restore. fn exposure_conflict_refusal( target: &str, backup: &std::path::Path, state_dir: Option<&std::path::Path>, ) -> anyhow::Error { - let sidecar = [state_dir, backup.parent()] - .into_iter() - .flatten() - .find_map(|d| find_sidecar_for_backup(d, backup)); + let (sidecar, in_recover_reach) = + match state_dir.and_then(|d| find_sidecar_for_backup(d, backup)) { + Some(sc) => (Some(sc), true), + None => ( + backup + .parent() + .and_then(|d| find_sidecar_for_backup(d, backup)), + false, + ), + }; match classify_exposure_conflict(sidecar.as_ref(), now_ms()) { ExposureConflict::Live { remaining_s } => anyhow::anyhow!( "{} is mid-exposure by another vt inject (auto-restore in ~{}s); \ @@ -1796,12 +1992,24 @@ fn exposure_conflict_refusal( target, remaining_s ), - ExposureConflict::Orphaned => anyhow::anyhow!( + ExposureConflict::Orphaned if in_recover_reach => anyhow::anyhow!( "a previous exposure of {} was never restored (its supervisor is \ gone) and the file may still be plaintext; run `vt inject \ --recover` first", target ), + // Orphaned, but the record lives beside the target (written when no + // home dir was resolvable) where `--recover` never looks: naming + // --recover here would end in "nothing to recover" while the + // plaintext stays. Manual restore is the only honest guidance. + ExposureConflict::Orphaned => anyhow::anyhow!( + "a previous exposure of {} was never restored (its supervisor is \ + gone) and the file may still be plaintext; verify the file's \ + state, then restore the ciphertext manually: mv '{}' '{}'", + target, + backup.display(), + target + ), ExposureConflict::Untracked => anyhow::anyhow!( "found leftover backup {} for {}; verify the file's state, then \ restore the ciphertext manually: mv '{}' '{}'", @@ -1834,18 +2042,30 @@ pub fn inject_recover() -> Result<()> { if path.extension().and_then(|e| e.to_str()) != Some("json") { continue; } - let sc: InjectSidecar = match std::fs::read(&path) - .ok() - .and_then(|b| serde_json::from_slice(&b).ok()) - { + let sc: InjectSidecar = match read_sidecar_bounded(&path) { Some(sc) => sc, None => { eprintln!("vt inject --recover: skipping unreadable sidecar {}", path.display()); continue; } }; - let backup_exists = std::path::Path::new(&sc.backup).exists(); - match plan_recovery(sc.deadline_ms, backup_exists, now) { + let backup_survives = match probe_sidecar_backup(&sc) { + BackupProbe::Ours => true, + BackupProbe::Gone => false, + // Transient stat failure: leave the record for a later sweep — + // cleaning it now could orphan exposed plaintext whose backup + // still exists behind the error. + BackupProbe::Unknown => { + eprintln!( + "vt inject --recover: cannot stat backup {}; leaving {} for a later sweep", + sc.backup, + path.display() + ); + active += 1; + continue; + } + }; + match plan_recovery(sc.deadline_ms, backup_survives, now) { Some(RecoverAction::Restore) => { let _ = std::fs::remove_file(&sc.tmp); if let Err(e) = std::fs::rename(&sc.backup, &sc.target) { @@ -1883,6 +2103,7 @@ fn spawn_restore_supervisor( backup_path: &std::path::Path, target_path: &str, sidecar_path: &std::path::Path, + backup_id: (u64, u64), ) -> Result<()> { use std::os::unix::process::CommandExt; use std::process::{Command, Stdio}; @@ -1896,6 +2117,7 @@ fn spawn_restore_supervisor( .arg(backup_path) .arg(target_path) .arg(sidecar_path) + .arg(format!("{}:{}", backup_id.0, backup_id.1)) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()); @@ -2286,18 +2508,93 @@ pub async fn doctor(auth_token: &str, file_populated_keys: &[String]) -> Result< Ok(()) } +/// Parse the supervisor's `dev:ino` argv token — the generation id of the +/// backup it armed for. +fn parse_dev_ino(s: &str) -> Option<(u64, u64)> { + let (dev, ino) = s.split_once(':')?; + Some((dev.parse().ok()?, ino.parse().ok()?)) +} + +/// Post-sleep body of the restore supervisor, extracted so tests can drive +/// it without fork/sleep. `armed_id` is the `(dev, ino)` of the backup this +/// supervisor was spawned for. The backup path is deterministic per target, +/// so a DELAYED supervisor (suspend pauses the monotonic sleep while the +/// wall-clock deadline lapses) can wake to find `--recover` already consumed +/// its backup and a NEWER exposure owning the path; renaming blindly would +/// consume the successor's backup — early-restoring its ciphertext +/// mid-window, or stranding its plaintext if it hasn't renamed its tmp yet. +/// So: restore only our own generation, and keep the sidecar — `--recover`'s +/// retry record — on a real rename failure or when the backup's state is +/// unknowable (stat error other than "not there"). +/// +/// Known, accepted residuals: (a) a successor reusing our exact inode is +/// indistinguishable here — unlike `--recover` we know no deadline to bound +/// mtime by; (b) the stat→rename pair is not atomic, so a window of +/// microseconds remains in which `--recover` consuming the verified backup +/// AND a new inject recreating the path would misdirect the rename. Both +/// events landing inside that window is negligible; absolute closure would +/// need every consumer to serialize on a parent-directory flock. +fn supervisor_restore( + tmp: &std::path::Path, + backup: &std::path::Path, + target: &std::path::Path, + sidecar: &std::path::Path, + armed_id: (u64, u64), +) { + use std::os::unix::fs::MetadataExt; + // Wipe any orphaned plaintext tmp first. If the parent crashed between + // write_plaintext_tmp and rename(tmp,target), this is the only path that + // removes it. Random-suffixed per run, so never another run's tmp. + let _ = std::fs::remove_file(tmp); + let ours = match std::fs::symlink_metadata(backup) { + Ok(md) => md.dev() == armed_id.0 && md.ino() == armed_id.1, + // Gone: the parent (or --recover) already restored this exposure. + Err(e) + if matches!( + e.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::NotADirectory + ) => + { + false + } + // Transient stat failure (EACCES, EIO, a sick mount): the truth is + // unknowable — keep the backup AND the sidecar so `vt inject + // --recover` retries once the path is reachable again. + Err(_) => return, + }; + if !ours { + // Nothing of ours left at the path — the exposure is settled. The + // sidecar (per-run random name, always ours) is stale; drop it. + let _ = std::fs::remove_file(sidecar); + return; + } + match std::fs::rename(backup, target) { + Ok(()) => { + let _ = std::fs::remove_file(sidecar); + } + // Consumed between the check and the rename: --recover won the race + // and the exposure is settled — same as the gone case above. + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + let _ = std::fs::remove_file(sidecar); + } + // Real failure (EACCES, EIO, ...): keep the sidecar so a later + // `vt inject --recover` retries the restore we could not perform. + Err(_) => {} + } +} + /// Pre-tokio, pre-clap entry point for the detached restore supervisor. /// Dispatched from `main()` so this process never builds a tokio runtime, /// parses clap definitions, or loads tracing — its RSS is just vt's text /// segment (shared with the parent via page cache) + a few KB of heap. /// /// Args (after the SUPERVISOR_SUBCOMMAND marker): -/// ` `. +/// ` :`. pub fn supervisor_main(args: &[std::ffi::OsString]) -> i32 { // Stdio is /dev/null — any failure here is invisible to the user. Parent // distinguishes the intermediate's success (exit 0 after double-fork) // from any failure (non-zero) via Child::wait(). - if args.len() != 5 { + if args.len() != 6 { return 2; } let secs: u64 = match args[0].to_str().and_then(|s| s.parse().ok()) { @@ -2308,6 +2605,10 @@ pub fn supervisor_main(args: &[std::ffi::OsString]) -> i32 { let backup = std::path::PathBuf::from(&args[2]); let target = std::path::PathBuf::from(&args[3]); let sidecar = std::path::PathBuf::from(&args[4]); + let armed_id = match args[5].to_str().and_then(parse_dev_ino) { + Some(id) => id, + None => return 2, + }; // Install SIG_IGN for the signals that would otherwise sweep us up when // the user closes the terminal, hits Ctrl+C, or runs `pkill vt`. These @@ -2341,8 +2642,8 @@ pub fn supervisor_main(args: &[std::ffi::OsString]) -> i32 { // interference with `waitpid(-1)` from the user-cmd or its parent shell). // // SAFETY: fork has well-defined POSIX behavior; the grandchild runs - // only async-signal-safe code below (thread::sleep, remove_file, rename - // — all of which are async-signal-safe at the syscall level). The + // only async-signal-safe code below (thread::sleep, stat, remove_file, + // rename — all of which are async-signal-safe at the syscall level). The // intermediate uses _exit(0) which is async-signal-safe and skips any // Rust destructors that would touch shared global state. unsafe { @@ -2355,16 +2656,7 @@ pub fn supervisor_main(args: &[std::ffi::OsString]) -> i32 { std::thread::sleep(std::time::Duration::from_secs(secs)); - // Wipe any orphaned plaintext tmp first. If the parent crashed between - // write_plaintext_tmp and rename(tmp,target), this is the only path - // that removes it — the supervisor doesn't otherwise know. - let _ = std::fs::remove_file(&tmp); - // Restore ciphertext over the target. Either succeeds (normal case) or - // returns ENOENT (parent already restored on exec failure / rename failure). - let _ = std::fs::rename(&backup, &target); - // The exposure is over; drop the crash-recovery sidecar so `--recover` - // doesn't later see a stale entry. - let _ = std::fs::remove_file(&sidecar); + supervisor_restore(&tmp, &backup, &target, &sidecar, armed_id); 0 } @@ -2657,13 +2949,22 @@ mod tests { fn inject_sidecar_json_roundtrips() { let sc = InjectSidecar { target: "/abs/secret.env".into(), - backup: "/abs/.secret.env.vt-backup-ab".into(), + backup: "/abs/.secret.env.vt-backup".into(), tmp: "/abs/.secret.env.vt-tmp-ab".into(), deadline_ms: 1_723_000_000_000, + backup_id: Some((16_777_232, 42)), }; let bytes = serde_json::to_vec(&sc).unwrap(); let back: InjectSidecar = serde_json::from_slice(&bytes).unwrap(); assert_eq!(sc, back); + + // Records written by builds that predate the generation id must still + // parse; they fall back to existence-only backup matching. + let legacy: InjectSidecar = serde_json::from_slice( + br#"{"target":"/t/f","backup":"/t/.f.vt-backup","tmp":"/t/.f.vt-tmp-ab","deadline_ms":7}"#, + ) + .unwrap(); + assert_eq!(legacy.backup_id, None); } #[test] @@ -2705,6 +3006,7 @@ mod tests { backup: "/t/.f.vt-backup".into(), tmp: "/t/.f.vt-tmp-ab".into(), deadline_ms: 100_000, + backup_id: None, }; // Before the deadline: live, remaining seconds round up. assert_eq!( @@ -2727,29 +3029,341 @@ mod tests { ); } + /// The real `(dev, ino)` of `backup` — a matching generation id for tests. + fn current_backup_id(backup: &std::path::Path) -> (u64, u64) { + use std::os::unix::fs::MetadataExt; + let md = std::fs::metadata(backup).unwrap(); + (md.dev(), md.ino()) + } + + /// The real `(dev, ino)` of `backup` with the inode perturbed — a + /// guaranteed generation mismatch for tests. + fn mismatched_backup_id(backup: &std::path::Path) -> (u64, u64) { + let (dev, ino) = current_backup_id(backup); + (dev, ino.wrapping_add(1)) + } + #[test] fn find_sidecar_for_backup_matches_and_skips_garbage() { let dir = std::env::temp_dir().join(format!("vt-inject-lock-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); - let backup = "/abs/.secret.env.vt-backup"; + let backup = dir.join(".secret.env.vt-backup"); + std::fs::write(&backup, b"ciphertext").unwrap(); let sc = InjectSidecar { - target: "/abs/secret.env".into(), - backup: backup.into(), + target: dir.join("secret.env").to_string_lossy().into_owned(), + backup: backup.to_string_lossy().into_owned(), tmp: "/abs/.secret.env.vt-tmp-ab".into(), - deadline_ms: 42, + // In-window deadline: the ordering bound must not filter a record + // whose exposure is simply still open. + deadline_ms: now_ms() + 60_000, + backup_id: Some(current_backup_id(&backup)), }; std::fs::write(dir.join("aa.json"), serde_json::to_vec(&sc).unwrap()).unwrap(); std::fs::write(dir.join("bb.json"), b"not json").unwrap(); // skipped std::fs::write(dir.join("cc.txt"), b"wrong extension").unwrap(); // ignored - assert_eq!( - find_sidecar_for_backup(&dir, std::path::Path::new(backup)), - Some(sc) - ); + assert_eq!(find_sidecar_for_backup(&dir, &backup), Some(sc.clone())); assert_eq!( find_sidecar_for_backup(&dir, std::path::Path::new("/abs/.other.vt-backup")), None ); + + // A stale record whose generation id names a different inode is not a + // match: it describes an earlier, already-consumed backup that + // happened to live at the same deterministic path. + let stale = InjectSidecar { + backup_id: Some(mismatched_backup_id(&backup)), + ..sc + }; + std::fs::write(dir.join("aa.json"), serde_json::to_vec(&stale).unwrap()).unwrap(); + assert_eq!(find_sidecar_for_backup(&dir, &backup), None); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn probe_sidecar_backup_checks_generation_and_ordering() { + let dir = std::env::temp_dir().join(format!("vt-inject-gen-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let backup = dir.join(".f.vt-backup"); + std::fs::write(&backup, b"ciphertext").unwrap(); + let sc = |backup_id, deadline_ms| InjectSidecar { + target: dir.join("f").to_string_lossy().into_owned(), + backup: backup.to_string_lossy().into_owned(), + tmp: "unused".into(), + deadline_ms, + backup_id, + }; + let future = now_ms() + 60_000; + // Legacy in-window record: existence is enough. + assert_eq!(probe_sidecar_backup(&sc(None, future)), BackupProbe::Ours); + // Matching generation survives; a different inode means the recorded + // backup was consumed and the path re-created by a later exposure. + assert_eq!( + probe_sidecar_backup(&sc(Some(current_backup_id(&backup)), future)), + BackupProbe::Ours + ); + assert_eq!( + probe_sidecar_backup(&sc(Some(mismatched_backup_id(&backup)), future)), + BackupProbe::Gone + ); + // Ordering bound: a backup modified after the record's deadline is a + // successor, whatever the record's id says — a legitimate backup + // always predates its own deadline. This is what protects id-less + // legacy records (and inode reuse) from the stale-sidecar race; the + // fresh file here postdates the long-expired deadline. + assert_eq!(probe_sidecar_backup(&sc(None, 0)), BackupProbe::Gone); + assert_eq!( + probe_sidecar_backup(&sc(Some(current_backup_id(&backup)), 0)), + BackupProbe::Gone + ); + // A genuinely old backup (mtime at or before the deadline) still + // reads as ours for an id-less record: legacy crash recovery keeps + // working, including the exact-deadline boundary a zero-timeout arm + // produces. One millisecond past the deadline is already a successor + // — a supervisor restoring at the deadline and a new inject arming + // right after must not pair with this record. + let f = std::fs::File::options().write(true).open(&backup).unwrap(); + f.set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_millis(1_000)) + .unwrap(); + drop(f); + assert_eq!(probe_sidecar_backup(&sc(None, 2_000)), BackupProbe::Ours); + assert_eq!(probe_sidecar_backup(&sc(None, 1_000)), BackupProbe::Ours); + assert_eq!(probe_sidecar_backup(&sc(None, 999)), BackupProbe::Gone); + // Backup gone: consumed; settled regardless of id. + std::fs::remove_file(&backup).unwrap(); + assert_eq!(probe_sidecar_backup(&sc(None, future)), BackupProbe::Gone); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn probe_sidecar_backup_reports_unknown_on_stat_errors() { + // Directory modes don't bind root; the EACCES this test relies on + // never happens there. + if unsafe { libc::geteuid() } == 0 { + return; + } + use std::os::unix::fs::PermissionsExt; + let dir = std::env::temp_dir().join(format!("vt-inject-probe-{}", std::process::id())); + let sealed = dir.join("sealed"); + std::fs::create_dir_all(&sealed).unwrap(); + let backup = sealed.join(".f.vt-backup"); + std::fs::write(&backup, b"ciphertext").unwrap(); + let sc = InjectSidecar { + target: sealed.join("f").to_string_lossy().into_owned(), + backup: backup.to_string_lossy().into_owned(), + tmp: "unused".into(), + deadline_ms: 0, + backup_id: None, + }; + std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o000)).unwrap(); + let probe = probe_sidecar_backup(&sc); + std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o700)).unwrap(); + // EACCES is not "consumed": recovery must keep the record, not clean + // it as stale. + assert_eq!(probe, BackupProbe::Unknown); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn supervisor_restore_keeps_sidecar_when_backup_state_unknown() { + if unsafe { libc::geteuid() } == 0 { + return; + } + use std::os::unix::fs::PermissionsExt; + let dir = std::env::temp_dir().join(format!("vt-inject-supstat-{}", std::process::id())); + let sealed = dir.join("sealed"); + std::fs::create_dir_all(&sealed).unwrap(); + let target = sealed.join("f"); + let backup = sealed.join(".f.vt-backup"); + std::fs::write(&target, b"plaintext").unwrap(); + std::fs::write(&backup, b"ciphertext").unwrap(); + let armed = current_backup_id(&backup); + let sidecar = dir.join("sc.json"); + std::fs::write(&sidecar, b"{}").unwrap(); + std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o000)).unwrap(); + + supervisor_restore(&dir.join("no-tmp"), &backup, &target, &sidecar, armed); + + std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o700)).unwrap(); + assert!( + sidecar.exists(), + "an unknowable backup state must keep the sidecar for --recover" + ); + assert_eq!(std::fs::read(&backup).unwrap(), b"ciphertext"); + assert_eq!(std::fs::read(&target).unwrap(), b"plaintext"); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn write_inject_sidecar_refuses_records_over_the_read_cap() { + let dir = std::env::temp_dir().join(format!("vt-inject-cap-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("aa.json"); + let sc = InjectSidecar { + target: "t".repeat(2 * SIDECAR_MAX_BYTES as usize), + backup: "/t/.f.vt-backup".into(), + tmp: "unused".into(), + deadline_ms: 0, + backup_id: Some((1, 2)), + }; + let err = write_inject_sidecar(&path, &sc).unwrap_err(); + assert!(err.to_string().contains("cap"), "got: {err}"); + assert!(!path.exists(), "an over-cap record must not be written"); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn retire_stale_sidecars_removes_only_records_of_this_backup() { + let dir = std::env::temp_dir().join(format!("vt-inject-retire-{}", std::process::id())); + let state = dir.join("state"); + std::fs::create_dir_all(&state).unwrap(); + let backup = dir.join(".f.vt-backup"); + let other = dir.join(".other.vt-backup"); + let mk = |backup: &std::path::Path| InjectSidecar { + target: dir.join("f").to_string_lossy().into_owned(), + backup: backup.to_string_lossy().into_owned(), + tmp: "unused".into(), + deadline_ms: 0, + backup_id: None, + }; + std::fs::write(state.join("stale.json"), serde_json::to_vec(&mk(&backup)).unwrap()) + .unwrap(); + std::fs::write(state.join("other.json"), serde_json::to_vec(&mk(&other)).unwrap()) + .unwrap(); + + retire_stale_sidecars_for(&backup, Some(&state)); + assert!(!state.join("stale.json").exists(), "stale record must be retired"); + assert!(state.join("other.json").exists(), "unrelated record must survive"); + // No resolvable state dir → quietly a no-op. + retire_stale_sidecars_for(&backup, None); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn parse_dev_ino_parses_and_rejects() { + assert_eq!(parse_dev_ino("123:456"), Some((123, 456))); + assert_eq!(parse_dev_ino("0:0"), Some((0, 0))); + assert_eq!(parse_dev_ino("123"), None); + assert_eq!(parse_dev_ino("a:1"), None); + assert_eq!(parse_dev_ino("1:b"), None); + assert_eq!(parse_dev_ino("-1:2"), None); + assert_eq!(parse_dev_ino(""), None); + } + + #[test] + fn supervisor_restore_restores_only_its_own_generation() { + let dir = std::env::temp_dir().join(format!("vt-inject-sup-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let target = dir.join("f"); + let backup = dir.join(".f.vt-backup"); + let tmp = dir.join(".f.vt-tmp-ab"); + let sidecar = dir.join("sc.json"); + + // Our generation: tmp wiped, backup renamed home, sidecar dropped. + std::fs::write(&target, b"plaintext").unwrap(); + std::fs::write(&backup, b"ciphertext").unwrap(); + std::fs::write(&tmp, b"orphan").unwrap(); + std::fs::write(&sidecar, b"{}").unwrap(); + supervisor_restore(&tmp, &backup, &target, &sidecar, current_backup_id(&backup)); + assert_eq!(std::fs::read(&target).unwrap(), b"ciphertext"); + assert!(!backup.exists() && !tmp.exists() && !sidecar.exists()); + + // A successor's generation at the path (delayed supervisor waking + // into a newer exposure): its backup and the target must be left + // untouched; only our stale sidecar is dropped. + std::fs::write(&target, b"plaintext-B").unwrap(); + std::fs::write(&backup, b"ciphertext-B").unwrap(); + std::fs::write(&sidecar, b"{}").unwrap(); + supervisor_restore(&tmp, &backup, &target, &sidecar, mismatched_backup_id(&backup)); + assert_eq!( + std::fs::read(&target).unwrap(), + b"plaintext-B", + "successor's live exposure must not be restored early" + ); + assert_eq!( + std::fs::read(&backup).unwrap(), + b"ciphertext-B", + "successor's backup must not be consumed" + ); + assert!(!sidecar.exists()); + + // Backup gone (already restored): just drop the sidecar. + std::fs::remove_file(&backup).unwrap(); + std::fs::write(&sidecar, b"{}").unwrap(); + supervisor_restore(&tmp, &backup, &target, &sidecar, (1, 2)); + assert_eq!(std::fs::read(&target).unwrap(), b"plaintext-B"); + assert!(!sidecar.exists()); + + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn supervisor_restore_keeps_sidecar_when_rename_fails() { + // Directory modes don't bind root; the EACCES this test relies on + // never happens there. + if unsafe { libc::geteuid() } == 0 { + return; + } + use std::os::unix::fs::PermissionsExt; + let dir = std::env::temp_dir().join(format!("vt-inject-supfail-{}", std::process::id())); + let ro = dir.join("ro"); + std::fs::create_dir_all(&ro).unwrap(); + let target = ro.join("f"); + let backup = ro.join(".f.vt-backup"); + let sidecar = dir.join("sc.json"); + std::fs::write(&target, b"plaintext").unwrap(); + std::fs::write(&backup, b"ciphertext").unwrap(); + std::fs::write(&sidecar, b"{}").unwrap(); + let armed = current_backup_id(&backup); + std::fs::set_permissions(&ro, std::fs::Permissions::from_mode(0o500)).unwrap(); + + supervisor_restore(&dir.join("no-tmp"), &backup, &target, &sidecar, armed); + + std::fs::set_permissions(&ro, std::fs::Permissions::from_mode(0o700)).unwrap(); + assert!( + sidecar.exists(), + "sidecar must survive a failed restore so --recover can retry" + ); + assert_eq!(std::fs::read(&backup).unwrap(), b"ciphertext"); + assert_eq!(std::fs::read(&target).unwrap(), b"plaintext"); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn read_sidecar_bounded_refuses_symlinks_fifos_and_oversize() { + let dir = std::env::temp_dir().join(format!("vt-inject-bounded-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let sc = InjectSidecar { + target: "/t/f".into(), + backup: "/t/.f.vt-backup".into(), + tmp: "unused".into(), + deadline_ms: 0, + backup_id: None, + }; + let real = dir.join("real.json"); + std::fs::write(&real, serde_json::to_vec(&sc).unwrap()).unwrap(); + assert_eq!(read_sidecar_bounded(&real), Some(sc)); + + // Symlink final component: refused even when it points at a valid + // sidecar — the fallback scan may run in an attacker-writable dir. + let link = dir.join("link.json"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + assert_eq!(read_sidecar_bounded(&link), None); + + // Oversized file: never read past the cap. + let big = dir.join("big.json"); + std::fs::write(&big, vec![b'x'; (SIDECAR_MAX_BYTES + 1) as usize]).unwrap(); + assert_eq!(read_sidecar_bounded(&big), None); + + // FIFO: O_NONBLOCK + the regular-file check make this return, not hang. + let fifo = dir.join("fifo.json"); + let cpath = { + use std::os::unix::ffi::OsStrExt; + std::ffi::CString::new(fifo.as_os_str().as_bytes()).unwrap() + }; + assert_eq!(unsafe { libc::mkfifo(cpath.as_ptr(), 0o600) }, 0); + assert_eq!(read_sidecar_bounded(&fifo), None); + std::fs::remove_dir_all(&dir).unwrap(); } @@ -2761,20 +3375,60 @@ mod tests { let target_str = target.to_str().unwrap(); let backup = exposure_backup_path(target_str).unwrap(); std::fs::write(&backup, b"ciphertext").unwrap(); + // Backdate the backup so records with long-past deadlines read as + // genuine orphans (a real orphan's backup predates its deadline; a + // fresh mtime would trip the ordering bound and classify Untracked). + let f = std::fs::File::options().write(true).open(&backup).unwrap(); + f.set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_millis(1_000)) + .unwrap(); + drop(f); // A sidecar whose deadline is long past → Orphaned → names --recover. + // The deadline sits at/after the backdated mtime, as it does for any + // genuine orphan — the ordering bound rejects mtimes past it. let state = dir.join("state"); std::fs::create_dir_all(&state).unwrap(); let sc = InjectSidecar { target: target_str.to_string(), backup: backup.to_string_lossy().into_owned(), tmp: "unused".into(), - deadline_ms: 0, + deadline_ms: 2_000, + backup_id: None, }; std::fs::write(state.join("aa.json"), serde_json::to_vec(&sc).unwrap()).unwrap(); let err = exposure_conflict_refusal(target_str, &backup, Some(&state)); assert!(err.to_string().contains("--recover"), "got: {err}"); + // The same orphan record found only beside the target (the no-home-dir + // fallback location) must NOT name --recover — it sweeps only the + // state dir and would report nothing to recover. Manual restore only. + std::fs::remove_file(state.join("aa.json")).unwrap(); + let fallback = dir.join(".secret.env.vt-recover-ab.json"); + std::fs::write(&fallback, serde_json::to_vec(&sc).unwrap()).unwrap(); + let err = exposure_conflict_refusal(target_str, &backup, Some(&state)); + assert!( + err.to_string().contains("never restored") && err.to_string().contains("mv"), + "got: {err}" + ); + assert!(!err.to_string().contains("--recover"), "got: {err}"); + std::fs::remove_file(&fallback).unwrap(); + + // A state-dir record whose generation id names a different inode + // describes an EARLIER exposure's consumed backup, not this one → + // classified Untracked (manual guidance), not Orphaned. + std::fs::write( + state.join("aa.json"), + serde_json::to_vec(&InjectSidecar { + backup_id: Some(mismatched_backup_id(&backup)), + ..sc.clone() + }) + .unwrap(), + ) + .unwrap(); + let err = exposure_conflict_refusal(target_str, &backup, Some(&state)); + assert!(err.to_string().contains("found leftover backup"), "got: {err}"); + assert!(!err.to_string().contains("--recover"), "got: {err}"); + // A live sidecar (far-future deadline) → retry guidance. std::fs::write( state.join("aa.json"), From ce6f35ed7a7233594c17b73f8ae52a1a0324736c Mon Sep 17 00:00:00 2001 From: Airead Date: Thu, 30 Jul 2026 09:55:11 +0800 Subject: [PATCH 3/3] inject: generation-check parent restores, clean partial backups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review follow-ups on the exposure-lock series: - The parent's step-4/5/exec failure paths called immediate_restore(), a blind rename of the deterministic backup path. A parent suspended past its own window could resume into a NEWER exposure owning that path and consume its backup — stranding that run's plaintext with no restore source. Those paths now share the supervisor's restore body (renamed restore_exposure): rename only the (dev, ino) generation this run armed, and keep the sidecar for --recover when a restore fails or the backup's state is unknowable. - A backup write failing part-way (ENOSPC/EIO) left a truncated .vt-backup with no sidecar or supervisor; the next inject would read it as an untracked exposure lock and suggest moving the partial copy over the intact ciphertext. create_exposure_backup() now removes the just-created file on any post-create failure, while propagating EEXIST untouched — that file is another exposure's live lock. --- CLAUDE.md | 30 +++--- src/client.rs | 283 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 187 insertions(+), 126 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0b7b2ea..39d9525 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,20 +46,22 @@ pin the transport. See [`config.example.toml`](config.example.toml) and supervisor. `vt inject --recover` must remain unauthenticated and only move the ciphertext backup back over the target. The ciphertext backup's deterministic per-target name (`.{name}.vt-backup`) IS the exposure lock: - it is created O_EXCL before any plaintext hits disk, and every restore path - must consume it via rename() — never copy+delete — so an overlapping - `inject -r` of the same file fails EEXIST instead of snapshotting exposed - plaintext as its "ciphertext" backup. Because that path is reused across - exposures, every new sidecar MUST carry its backup's `(dev, ino)` generation - id, recovery refuses a backup whose generation or mtime-vs-deadline ordering - its sidecar did not record (the ordering bound is all an id-less legacy - record has), arming retires stale sidecars naming the path, and the restore - supervisor only renames the `(dev, ino)` generation it armed for (a suspend - can delay its monotonic sleep past the wall-clock deadline) and keeps its - sidecar when a restore fails or the backup's state is unknowable — only - "not there" counts as consumed; other stat errors must never clean a - recovery record. Do not reintroduce a randomized backup name, and keep - refusing `-r` files with zero `vt://` records. + it is created O_EXCL before any plaintext hits disk (and removed if + filling it fails — a partial copy must not read as a lock), and every + restore path must consume it via rename() — never copy+delete — so an + overlapping `inject -r` of the same file fails EEXIST instead of + snapshotting exposed plaintext as its "ciphertext" backup. Because that + path is reused across exposures, every new sidecar MUST carry its backup's + `(dev, ino)` generation id, recovery refuses a backup whose generation or + mtime-vs-deadline ordering its sidecar did not record (the ordering bound + is all an id-less legacy record has), arming retires stale sidecars naming + the path, and every restorer — the supervisor and the parent's failure + paths alike — only renames the `(dev, ino)` generation it armed for (a + suspend can delay either past the wall-clock deadline, into a successor's + window) and keeps its sidecar when a restore fails or the backup's state + is unknowable — only "not there" counts as consumed; other stat errors + must never clean a recovery record. Do not reintroduce a randomized + backup name, and keep refusing `-r` files with zero `vt://` records. - DEK caching is opt-in, requires `CACHE_SECKEY`, and uses the Worker-selected TTLs. A cache hit is not a phone approval; preserve its audit row and IP binding. Every write mints an immutable `cache_group_id` (the handle the admin diff --git a/src/client.rs b/src/client.rs index 11d9ba2..e4da966 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1391,18 +1391,17 @@ pub async fn inject( // 5. rename(tmp, target) — atomic plaintext exposure. // 6. Parent execs the user command; supervisor restores after timeout. // - // Steps 4, 5, and 6 each call `immediate_restore` on failure so the - // observable post-failure state collapses to "target = ciphertext, no - // sidecars" without waiting for the supervisor. The supervisor is the - // durable backstop: even if a parent crash makes immediate_restore - // unreachable, the supervisor's `unlink(tmp) + rename(backup, target)` - // after timeout still brings everything home (modulo SIGKILL / reboot). - let armed: Option<(String, std::path::PathBuf, std::path::PathBuf)> = - if let Some(replace_file_path) = &replace_file - { - use std::io::Write; - use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; - + // Steps 4, 5, and 6 each call `restore_exposure` on failure so the + // observable post-failure state normally collapses to "target = + // ciphertext, no sidecars" without waiting for the supervisor — under + // the same generation check the supervisor applies: a parent suspended + // past its own window can resume to find the deterministic backup path + // owned by a NEWER exposure, whose backup a blind rename would consume. + // The supervisor is the durable backstop: even if a parent crash makes + // the fast path unreachable, the supervisor's `unlink(tmp) + + // rename(backup, target)` after timeout still brings everything home + // (modulo SIGKILL / reboot). + let armed: Option = if let Some(replace_file_path) = &replace_file { let orig_path = std::path::Path::new(replace_file_path); let dir = orig_path .parent() @@ -1439,18 +1438,12 @@ pub async fn inject( // Step 2: write backup from the in-memory ORIGINAL bytes (captured at // the single O_NOFOLLOW open above) — NOT a fresh re-read of the target, // which would be a TOCTOU window for a directory-writable attacker. - // The open fd also yields the backup's (dev, ino) generation id for - // the sidecar, binding the record to THIS backup — the deterministic + // The backup fd also yields the (dev, ino) generation id for the + // sidecar, binding the record to THIS backup — the deterministic // path alone is ambiguous across exposures of the same target. - let backup_id: (u64, u64) = { - let mut backup_file = match std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .custom_flags(libc::O_NOFOLLOW) - .mode(orig_mode) - .open(&backup_path) - { - Ok(f) => f, + let backup_id: (u64, u64) = + match create_exposure_backup(&backup_path, &orig_file_bytes, orig_mode) { + Ok(id) => id, // EEXIST: another exposure of this target is open (or a dead // supervisor left one). The pre-decrypt check already refused // the common case; this is the authoritative race-free gate. @@ -1463,34 +1456,10 @@ pub async fn inject( } Err(e) => { return Err(e).with_context(|| { - format!("Failed to create backup file: {}", backup_path.display()) + format!("Failed to write backup file: {}", backup_path.display()) }); } }; - backup_file.write_all(&orig_file_bytes).with_context(|| { - format!( - "Failed to copy content to backup: {}", - backup_path.display() - ) - })?; - backup_file.sync_all().ok(); - // The id is REQUIRED on new records: silently degrading to None - // would put recovery back on existence-only matching and reopen - // the stale-sidecar race. No plaintext exists yet, so aborting - // (after releasing the lock) is safe. - match backup_file.metadata() { - Ok(m) => (m.dev(), m.ino()), - Err(e) => { - let _ = std::fs::remove_file(&backup_path); - return Err(e).with_context(|| { - format!( - "Failed to stat backup for its generation id: {}", - backup_path.display() - ) - }); - } - } - }; debug!("Created backup at: {}", backup_path.display()); // Holding the exposure lock proves any older sidecar naming this @@ -1535,29 +1504,43 @@ pub async fn inject( ); } - // Step 4: write tmp (plaintext). + // Step 4: write tmp (plaintext). Parent-side cleanup here (and in + // steps 5/6) normally leaves no visible sidecar state for the + // timeout window; the supervisor later observes ENOENT on the + // backup and exits silently. if let Err(e) = write_plaintext_tmp(&tmp_path, orig_mode, &decrypted_file_content) { - // Parent does immediate cleanup so the failure leaves no visible - // sidecar state for the timeout window. Supervisor will later - // observe ENOENT on backup and exit silently. - let _ = std::fs::remove_file(&tmp_path); - let _ = std::fs::remove_file(&sidecar_path); - immediate_restore(replace_file_path, &backup_path); + restore_exposure( + &tmp_path, + &backup_path, + std::path::Path::new(replace_file_path), + &sidecar_path, + backup_id, + ); return Err(e); } // Step 5: atomically expose plaintext at target. if let Err(e) = std::fs::rename(&tmp_path, replace_file_path) { - let _ = std::fs::remove_file(&tmp_path); - let _ = std::fs::remove_file(&sidecar_path); - immediate_restore(replace_file_path, &backup_path); + restore_exposure( + &tmp_path, + &backup_path, + std::path::Path::new(replace_file_path), + &sidecar_path, + backup_id, + ); return Err(e).with_context(|| { format!("Failed to atomically replace file: {}", replace_file_path) }); } debug!("Content written to replace file: {}", replace_file_path); - Some((replace_file_path.clone(), backup_path, sidecar_path)) + Some(ArmedExposure { + target: replace_file_path.clone(), + backup: backup_path, + tmp: tmp_path, + sidecar: sidecar_path, + backup_id, + }) } else { None }; @@ -1576,29 +1559,29 @@ pub async fn inject( // exec() never returns on success; reaching here means it failed. let err = exec::Command::new(command).args(args).exec(); - // Restore immediately on exec failure so the user doesn't wait out the + // Restore on exec failure so the user doesn't wait out the // supervisor's timeout. The supervisor will later observe ENOENT on the // backup and exit silently. - if let Some((target, backup, sidecar)) = &armed { - immediate_restore(target, backup); - let _ = std::fs::remove_file(sidecar); + if let Some(a) = &armed { + restore_exposure( + &a.tmp, + &a.backup, + std::path::Path::new(&a.target), + &a.sidecar, + a.backup_id, + ); } Err(anyhow::anyhow!("Failed to execute command: {}", err)) } -/// Best-effort parent-side restore. Called on any step-4/5/6 failure to -/// collapse the post-failure state to "target = ciphertext, no sidecars" as -/// fast as possible. The detached supervisor will later see ENOENT on backup -/// and exit silently — there are deliberately two restorers, with the parent -/// as the fast path and the supervisor as the durable fallback. -fn immediate_restore(target: &str, backup: &std::path::Path) { - if let Err(e) = std::fs::rename(backup, target) { - eprintln!( - "vt inject: restore-on-fail failed: {}; backup remains at {}", - e, - backup.display() - ); - } +/// One armed exposure's unwind state, kept by the parent for the exec-failure +/// path — the same tuple the supervisor got via argv at spawn. +struct ArmedExposure { + target: String, + backup: std::path::PathBuf, + tmp: std::path::PathBuf, + sidecar: std::path::PathBuf, + backup_id: (u64, u64), } fn write_plaintext_tmp(tmp: &std::path::Path, mode: u32, content: &str) -> Result<()> { @@ -1668,10 +1651,10 @@ enum RecoverAction { /// path is reused by every exposure of a target. fn plan_recovery(deadline_ms: u64, backup_survives: bool, now_ms: u64) -> Option { if !backup_survives { - // The supervisor (or immediate_restore) already renamed the backup - // over the target — or the path now holds a NEWER exposure's backup - // (generation mismatch). Either way this sidecar's injection is - // over; only the sidecar lingers. + // The supervisor (or the parent's fast path) already renamed the + // backup over the target — or the path now holds a NEWER exposure's + // backup (generation mismatch). Either way this sidecar's injection + // is over; only the sidecar lingers. return Some(RecoverAction::CleanStale); } if now_ms >= deadline_ms.saturating_add(RECOVER_GRACE_MS) { @@ -1751,10 +1734,11 @@ fn write_inject_sidecar(path: &std::path::Path, sc: &InjectSidecar) -> Result<() // that plaintext back over the target permanently, with no sidecar or backup // left for `--recover` to find. The lock that prevents this is the backup // file itself: its name is deterministic per target, it is created O_EXCL -// before any plaintext hits disk, and it is consumed by rename() on every -// restore path (supervisor, immediate_restore, --recover). Its existence -// therefore means exactly "an exposure window is open, or a dead supervisor -// left the target plaintext" — and a new inject must refuse. +// before any plaintext hits disk (and removed if filling it fails — a +// partial copy must not read as a lock), and it is consumed by rename() on +// every restore path (supervisor, parent failure paths, --recover). Its +// existence therefore means exactly "an exposure window is open, or a dead +// supervisor left the target plaintext" — and a new inject must refuse. // // The sidecar shares the path's ambiguity in the other direction: it records // the deterministic backup path, so a stale record (crash between a restore's @@ -1765,10 +1749,10 @@ fn write_inject_sidecar(path: &std::path::Path, sc: &InjectSidecar) -> Result<() // id, verified before recovery consumes anything; recovery refuses any // backup modified past the record's deadline — the ordering bound that // covers id-less legacy records and a successor reusing the old inode; -// and the supervisor re-checks the (dev, ino) it armed for before its -// timeout restore (see `supervisor_restore` — a suspend can delay its -// monotonic sleep past the wall-clock deadline, letting --recover and a new -// exposure run first). +// and every restorer re-checks the (dev, ino) it armed for before renaming +// (see `restore_exposure` — a suspend can delay the supervisor's monotonic +// sleep, or stop the parent short of its failure path, past the wall-clock +// deadline, letting --recover and a new exposure run first). /// Deterministic ciphertext-backup path for `target`: `dir/.{name}.vt-backup`. /// Deliberately NOT randomized — see the module comment above. Restore paths @@ -1788,6 +1772,42 @@ fn exposure_backup_path(target: &str) -> Result { Ok(dir.join(format!(".{}.vt-backup", file_name))) } +/// Create the exposure-lock backup (O_CREAT|O_EXCL|O_NOFOLLOW, `mode`), fill +/// it with the ciphertext `bytes`, and return its `(dev, ino)` generation id. +/// On any failure AFTER the exclusive create — a partial write (ENOSPC/EIO) +/// or the generation-id stat — the just-created file is removed before +/// returning: a partial backup with no sidecar and no supervisor would read +/// as an untracked exposure lock to the next inject, whose refusal guidance +/// would then move truncated bytes over the intact ciphertext target. No +/// plaintext exists yet at that point, so aborting (releasing the lock) is +/// safe. The EEXIST failure propagates untouched and must NEVER remove: that +/// file is another exposure's live lock (or its orphaned backup). +/// +/// The generation id is REQUIRED on new records: silently degrading to None +/// would put recovery back on existence-only matching and reopen the +/// stale-sidecar race. +fn create_exposure_backup( + backup_path: &std::path::Path, + bytes: &[u8], + mode: u32, +) -> std::io::Result<(u64, u64)> { + use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; + let mut f = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .custom_flags(libc::O_NOFOLLOW) + .mode(mode) + .open(backup_path)?; + let filled = f.write_all(bytes).and_then(|()| { + f.sync_all().ok(); + f.metadata().map(|m| (m.dev(), m.ino())) + }); + if filled.is_err() { + let _ = std::fs::remove_file(backup_path); + } + filled +} + /// Why an inject over an existing backup was refused — picks the operator /// guidance in the refusal message. #[derive(Debug, PartialEq, Eq)] @@ -2515,17 +2535,22 @@ fn parse_dev_ino(s: &str) -> Option<(u64, u64)> { Some((dev.parse().ok()?, ino.parse().ok()?)) } -/// Post-sleep body of the restore supervisor, extracted so tests can drive -/// it without fork/sleep. `armed_id` is the `(dev, ino)` of the backup this -/// supervisor was spawned for. The backup path is deterministic per target, -/// so a DELAYED supervisor (suspend pauses the monotonic sleep while the -/// wall-clock deadline lapses) can wake to find `--recover` already consumed -/// its backup and a NEWER exposure owning the path; renaming blindly would -/// consume the successor's backup — early-restoring its ciphertext -/// mid-window, or stranding its plaintext if it hasn't renamed its tmp yet. -/// So: restore only our own generation, and keep the sidecar — `--recover`'s -/// retry record — on a real rename failure or when the backup's state is -/// unknowable (stat error other than "not there"). +/// The generation-checked restore shared by the supervisor (its post-sleep +/// body, extracted so tests can drive it without fork/sleep) and the parent's +/// step-4/5/6 failure paths. `armed_id` is the `(dev, ino)` of the backup +/// this caller armed. The backup path is deterministic per target, so a +/// DELAYED caller — a supervisor whose monotonic sleep a suspend paused while +/// the wall-clock deadline lapsed, or a parent stopped between arming and its +/// failure path — can find `--recover` already consumed its backup and a +/// NEWER exposure owning the path; renaming blindly would consume the +/// successor's backup — early-restoring its ciphertext mid-window, or +/// stranding its plaintext with no restore source if it hasn't renamed its +/// tmp yet. So: restore only our own generation, and keep the sidecar — +/// `--recover`'s retry record — on a real rename failure or when the backup's +/// state is unknowable (stat error other than "not there"). +/// +/// Failure warnings go to stderr: user-visible on the parent paths, /dev/null +/// in the supervisor. /// /// Known, accepted residuals: (a) a successor reusing our exact inode is /// indistinguishable here — unlike `--recover` we know no deadline to bound @@ -2534,7 +2559,7 @@ fn parse_dev_ino(s: &str) -> Option<(u64, u64)> { /// AND a new inject recreating the path would misdirect the rename. Both /// events landing inside that window is negligible; absolute closure would /// need every consumer to serialize on a parent-directory flock. -fn supervisor_restore( +fn restore_exposure( tmp: &std::path::Path, backup: &std::path::Path, target: &std::path::Path, @@ -2560,7 +2585,14 @@ fn supervisor_restore( // Transient stat failure (EACCES, EIO, a sick mount): the truth is // unknowable — keep the backup AND the sidecar so `vt inject // --recover` retries once the path is reachable again. - Err(_) => return, + Err(e) => { + eprintln!( + "vt inject: cannot stat backup {}: {}; leaving it for `vt inject --recover`", + backup.display(), + e + ); + return; + } }; if !ours { // Nothing of ours left at the path — the exposure is settled. The @@ -2579,7 +2611,13 @@ fn supervisor_restore( } // Real failure (EACCES, EIO, ...): keep the sidecar so a later // `vt inject --recover` retries the restore we could not perform. - Err(_) => {} + Err(e) => { + eprintln!( + "vt inject: restore failed: {}; backup remains at {}", + e, + backup.display() + ); + } } } @@ -2656,7 +2694,7 @@ pub fn supervisor_main(args: &[std::ffi::OsString]) -> i32 { std::thread::sleep(std::time::Duration::from_secs(secs)); - supervisor_restore(&tmp, &backup, &target, &sidecar, armed_id); + restore_exposure(&tmp, &backup, &target, &sidecar, armed_id); 0 } @@ -2916,7 +2954,7 @@ mod tests { #[test] fn plan_recovery_cleans_stale_when_backup_gone() { - // Backup already consumed (supervisor/immediate_restore renamed it) → + // Backup already consumed (a restorer renamed it home) → // the injection completed; only the sidecar lingers. Deadline is // irrelevant on this arm. assert_eq!( @@ -2999,6 +3037,27 @@ mod tests { assert!(exposure_backup_path("/").is_err()); } + #[test] + fn create_exposure_backup_returns_live_id_and_keeps_foreign_locks() { + let dir = std::env::temp_dir().join(format!("vt-inject-mkbak-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let backup = dir.join(".f.vt-backup"); + + // Success: bytes land, and the returned generation id is the live one. + let id = create_exposure_backup(&backup, b"ciphertext", 0o600).unwrap(); + assert_eq!(std::fs::read(&backup).unwrap(), b"ciphertext"); + assert_eq!(id, current_backup_id(&backup)); + + // EEXIST: propagated for the conflict-refusal path, and the existing + // lock must NOT be removed — it is another exposure's live backup, + // not something this call created and may clean up. + let err = create_exposure_backup(&backup, b"other", 0o600).unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::AlreadyExists); + assert_eq!(std::fs::read(&backup).unwrap(), b"ciphertext"); + + std::fs::remove_dir_all(&dir).unwrap(); + } + #[test] fn classify_exposure_conflict_branches() { let sc = InjectSidecar { @@ -3165,7 +3224,7 @@ mod tests { } #[test] - fn supervisor_restore_keeps_sidecar_when_backup_state_unknown() { + fn restore_exposure_keeps_sidecar_when_backup_state_unknown() { if unsafe { libc::geteuid() } == 0 { return; } @@ -3182,7 +3241,7 @@ mod tests { std::fs::write(&sidecar, b"{}").unwrap(); std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o000)).unwrap(); - supervisor_restore(&dir.join("no-tmp"), &backup, &target, &sidecar, armed); + restore_exposure(&dir.join("no-tmp"), &backup, &target, &sidecar, armed); std::fs::set_permissions(&sealed, std::fs::Permissions::from_mode(0o700)).unwrap(); assert!( @@ -3251,7 +3310,7 @@ mod tests { } #[test] - fn supervisor_restore_restores_only_its_own_generation() { + fn restore_exposure_restores_only_its_own_generation() { let dir = std::env::temp_dir().join(format!("vt-inject-sup-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let target = dir.join("f"); @@ -3264,7 +3323,7 @@ mod tests { std::fs::write(&backup, b"ciphertext").unwrap(); std::fs::write(&tmp, b"orphan").unwrap(); std::fs::write(&sidecar, b"{}").unwrap(); - supervisor_restore(&tmp, &backup, &target, &sidecar, current_backup_id(&backup)); + restore_exposure(&tmp, &backup, &target, &sidecar, current_backup_id(&backup)); assert_eq!(std::fs::read(&target).unwrap(), b"ciphertext"); assert!(!backup.exists() && !tmp.exists() && !sidecar.exists()); @@ -3274,7 +3333,7 @@ mod tests { std::fs::write(&target, b"plaintext-B").unwrap(); std::fs::write(&backup, b"ciphertext-B").unwrap(); std::fs::write(&sidecar, b"{}").unwrap(); - supervisor_restore(&tmp, &backup, &target, &sidecar, mismatched_backup_id(&backup)); + restore_exposure(&tmp, &backup, &target, &sidecar, mismatched_backup_id(&backup)); assert_eq!( std::fs::read(&target).unwrap(), b"plaintext-B", @@ -3290,7 +3349,7 @@ mod tests { // Backup gone (already restored): just drop the sidecar. std::fs::remove_file(&backup).unwrap(); std::fs::write(&sidecar, b"{}").unwrap(); - supervisor_restore(&tmp, &backup, &target, &sidecar, (1, 2)); + restore_exposure(&tmp, &backup, &target, &sidecar, (1, 2)); assert_eq!(std::fs::read(&target).unwrap(), b"plaintext-B"); assert!(!sidecar.exists()); @@ -3298,7 +3357,7 @@ mod tests { } #[test] - fn supervisor_restore_keeps_sidecar_when_rename_fails() { + fn restore_exposure_keeps_sidecar_when_rename_fails() { // Directory modes don't bind root; the EACCES this test relies on // never happens there. if unsafe { libc::geteuid() } == 0 { @@ -3317,7 +3376,7 @@ mod tests { let armed = current_backup_id(&backup); std::fs::set_permissions(&ro, std::fs::Permissions::from_mode(0o500)).unwrap(); - supervisor_restore(&dir.join("no-tmp"), &backup, &target, &sidecar, armed); + restore_exposure(&dir.join("no-tmp"), &backup, &target, &sidecar, armed); std::fs::set_permissions(&ro, std::fs::Permissions::from_mode(0o700)).unwrap(); assert!(