fix(core): remove the added cameras' staging files with the last profile

A write of the added cameras' store, or of its commit journal, first
puts the new bytes in a staging file beside the target and then renames
it over the target. If the process crashes, or the write or sync fails,
before the rename, the staging file stays behind. On a host without a
TPM it holds plaintext embeddings. Deleting the last profile removed
the store and the journal but not these files.

- storage::delete also removes the staging files that the store's and
  the journal's writers leave (.<user>.json.tmp-<pid>,
  .<user>.json.commit-tmp-<pid>, .<user>.json.intent.commit-tmp-<pid>).
  It removes them after the journal and the store and before the
  enrollment, syncing the directory after each removal, so a failure
  still leaves the request repeatable. Deleted.camera_store counts them.
- The writers take these names from multi_camera::staging_path, and the
  sweep matches them with is_staging_file_of. A match needs the
  account's exact file name followed by one staging tag and a decimal
  pid, so another account's files never match.

Tests: deleting_an_account_removes_the_staging_files_its_camera_store_writers_left,
which fails without the sweep, and
staging_files_of_the_store_and_its_journal_match_and_other_names_do_not.

Signed-off-by: Wisbendji Fimerlus <archledger236@gmail.com>

Author: archledger

crates/irlume-core/src/multi_camera.rs

@@ -116,6 +116,55 @@ pub fn secondary_store_path(user: &str) -> PathBuf {
         .join(format!("{user}.json"))
 }
 
+/// The staging tag of [`save_secondary`]'s writes.
+const SAVE_STAGING_TAG: &str = "tmp";
+
+/// The staging tag of the commit protocol's writes, of the store and of its
+/// journal ([`commit::publish_with_intent`]).
+const COMMIT_STAGING_TAG: &str = "commit-tmp";
+
+/// The file a writer stages `path`'s new bytes in before renaming it over
+/// `path`: `.<file name>.<tag>-<pid>` in the same directory. A crash, or a
+/// failed write or sync, before the rename leaves it behind with those
+/// bytes, which are plaintext embeddings on a host without a TPM.
+fn staging_path(path: &Path, tag: &str) -> PathBuf {
+    let name = path
+        .file_name()
+        .map_or_else(|| "secondary".into(), |n| n.to_string_lossy().into_owned());
+    path.parent()
+        .unwrap_or_else(|| Path::new("."))
+        .join(format!(".{name}.{tag}-{}", std::process::id()))
+}
+
+/// Whether `name`, an entry of the directory of the store at `store`, is a
+/// staging file a writer of that store or of its commit journal left behind
+/// ([`staging_path`], any process id). Another account's files never match:
+/// the file name must be followed by exactly one staging tag and a decimal
+/// process id.
+pub(crate) fn is_staging_file_of(store: &Path, name: &std::ffi::OsStr) -> bool {
+    let Some(name) = name.to_str() else {
+        return false;
+    };
+    let journal = commit::intent_path_for(store);
+    let staged_by = |file: &str, tag: &str| {
+        name.strip_prefix('.')
+            .and_then(|rest| rest.strip_prefix(file))
+            .and_then(|rest| rest.strip_prefix('.'))
+            .and_then(|rest| rest.strip_prefix(tag))
+            .and_then(|rest| rest.strip_prefix('-'))
+            .is_some_and(|pid| !pid.is_empty() && pid.bytes().all(|b| b.is_ascii_digit()))
+    };
+    let (Some(store), Some(journal)) = (
+        store.file_name().and_then(std::ffi::OsStr::to_str),
+        journal.file_name().and_then(std::ffi::OsStr::to_str),
+    ) else {
+        return false;
+    };
+    staged_by(store, SAVE_STAGING_TAG)
+        || staged_by(store, COMMIT_STAGING_TAG)
+        || staged_by(journal, COMMIT_STAGING_TAG)
+}
+
 /// The primary enrollment's on-disk path for `user` - the exact file whose
 /// bytes the secondary store's activation digest is taken over. Delegates
 /// to the loader's own resolution so the two can never drift apart.
@@ -835,13 +884,7 @@ pub(crate) fn save_secondary_with_key(
     // Writers create the store's directory before publication (the fixed
     // location sits in a `cameras/` subdirectory legacy code never made).
     std::fs::create_dir_all(dir).map_err(|error| SecondaryStoreError::Io(error.to_string()))?;
-    let temp = dir.join(format!(
-        ".{}.tmp-{}",
-        path.file_name()
-            .map(|n| n.to_string_lossy().into_owned())
-            .unwrap_or_else(|| "secondary".into()),
-        std::process::id()
-    ));
+    let temp = staging_path(path, SAVE_STAGING_TAG);
     use std::io::Write;
     let write_all = |temp: &Path| -> std::io::Result<()> {
         // Owner-only regardless of umask (ADR-0024 s1.2 permission clause).
@@ -1230,6 +1273,46 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    #[test]
+    fn staging_files_of_the_store_and_its_journal_match_and_other_names_do_not() {
+        let store = Path::new("/state/cameras/alice.json");
+        let journal = commit::intent_path_for(store);
+        // Exactly the names the writers stage in, so the deletion sweep and
+        // the writers cannot drift apart.
+        for staged in [
+            staging_path(store, SAVE_STAGING_TAG),
+            staging_path(store, COMMIT_STAGING_TAG),
+            staging_path(&journal, COMMIT_STAGING_TAG),
+        ] {
+            assert_eq!(staged.parent(), store.parent());
+            let name = staged.file_name().expect("a staging file name");
+            assert!(is_staging_file_of(store, name), "{}", staged.display());
+            assert!(
+                !is_staging_file_of(Path::new("/state/cameras/bob.json"), name),
+                "another account's deletion took {}",
+                staged.display()
+            );
+        }
+        for name in [
+            "alice.json",
+            "alice.json.intent",
+            ".alice.json.tmp-",
+            ".alice.json.tmp-12a",
+            ".alice.json.tmp-1.swp",
+            ".alice.json.intent.tmp-x",
+            "alice.json.tmp-1",
+            ".alice.json.bak",
+            ".alice.json.json.tmp-1",
+            ".alicee.json.tmp-1",
+            ".bob.json.commit-tmp-1",
+        ] {
+            assert!(
+                !is_staging_file_of(store, std::ffi::OsStr::new(name)),
+                "{name} is not a staging file of alice's store"
+            );
+        }
+    }
+
     #[test]
     fn profile_group_calibrations_round_trip_and_phase1_fixtures_still_load() {
         let calib = crate::calib::IrCalibration {

crates/irlume-core/src/multi_camera/commit.rs

@@ -118,13 +118,7 @@ fn fsync_dir(dir: &Path) -> Result<(), CommitError> {
 fn durable_write(path: &Path, bytes: &[u8]) -> Result<(), CommitError> {
     use std::io::Write;
     let dir = path.parent().unwrap_or_else(|| Path::new("."));
-    let temp = dir.join(format!(
-        ".{}.commit-tmp-{}",
-        path.file_name()
-            .map(|n| n.to_string_lossy().into_owned())
-            .unwrap_or_else(|| "store".into()),
-        std::process::id()
-    ));
+    let temp = super::staging_path(path, super::COMMIT_STAGING_TAG);
     let mut file = std::fs::OpenOptions::new()
         .write(true)
         .create_new(true)

crates/irlume-core/src/storage.rs

@@ -939,13 +939,14 @@ pub fn store_is_encrypted(user: &str) -> irlume_common::Result<Option<bool>> {
 pub struct Deleted {
     /// The primary enrollment file existed and was removed.
     pub enrollment: bool,
-    /// The added cameras' store (`cameras/<user>.json`) or its commit
-    /// journal existed and was removed.
+    /// The added cameras' store (`cameras/<user>.json`), its commit journal
+    /// or a staging file one of their writers left existed and was removed.
     pub camera_store: bool,
 }
 
 /// Deletes all of `user`'s face data under the user state lock: the added
-/// cameras' store and its commit journal (removing the account's face data
+/// cameras' store, its commit journal and any staging file an interrupted
+/// write of either left beside them (removing the account's face data
 /// covers every camera group, ADR-0024 §4.2), then the primary enrollment,
 /// then the now-orphaned template key and recovery envelope (a fresh
 /// enrollment mints a new key).
@@ -975,15 +976,21 @@ pub fn delete(user: &str) -> irlume_common::Result<Deleted> {
     })
 }
 
-/// Removes `user`'s added-camera commit journal, then the store, syncing the
+/// Removes `user`'s added-camera commit journal, then the store, then the
+/// staging files their writers left ([`camera_staging_files`]), syncing the
 /// directory after each removal so the journal cannot outlive the store
-/// after a crash. The caller holds the user state lock. `Ok(true)` when
-/// either file existed.
+/// after a crash. The caller holds the user state lock. `Ok(true)` when any
+/// of these files existed.
 fn delete_camera_store_unlocked(user: &str) -> irlume_common::Result<bool> {
     let store = crate::multi_camera::secondary_store_path(user);
-    let journal = crate::multi_camera::commit::intent_path_for(&store);
+    let dir = store.parent().unwrap_or_else(|| Path::new("."));
+    let mut paths = vec![
+        crate::multi_camera::commit::intent_path_for(&store),
+        store.clone(),
+    ];
+    paths.extend(camera_staging_files(&store, dir)?);
     let mut removed = false;
-    for path in [&journal, &store] {
+    for path in &paths {
         match fs::remove_file(path) {
             Ok(()) => removed = true,
             Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue,
@@ -994,14 +1001,37 @@ fn delete_camera_store_unlocked(user: &str) -> irlume_common::Result<bool> {
                 )))
             }
         }
-        let dir = path.parent().unwrap_or_else(|| Path::new("."));
         fs::File::open(dir)
             .and_then(|dir| dir.sync_all())
             .map_err(|e| irlume_common::Error::Io(format!("sync {}: {e}", dir.display())))?;
     }
     Ok(removed)
 }
 
+/// The staging files in `dir` that a writer of the added cameras' store at
+/// `store`, or of its commit journal, left before its rename: an interrupted
+/// or failed write keeps the new store bytes there
+/// ([`crate::multi_camera::is_staging_file_of`]). A missing directory has
+/// none.
+fn camera_staging_files(store: &Path, dir: &Path) -> irlume_common::Result<Vec<PathBuf>> {
+    let list_error =
+        |e: std::io::Error| irlume_common::Error::Io(format!("list {}: {e}", dir.display()));
+    let entries = match fs::read_dir(dir) {
+        Ok(entries) => entries,
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()),
+        Err(e) => return Err(list_error(e)),
+    };
+    let mut found = Vec::new();
+    for entry in entries {
+        let entry = entry.map_err(list_error)?;
+        if crate::multi_camera::is_staging_file_of(store, &entry.file_name()) {
+            found.push(entry.path());
+        }
+    }
+    found.sort();
+    Ok(found)
+}
+
 /// Has the startup IR compatibility sweep already run for this space?
 /// The historical name and marker format are retained for compatibility;
 /// the daemon no longer retags enrollment data.
@@ -1211,6 +1241,56 @@ mod tests {
         let _ = fs::remove_dir_all(&dir);
     }
 
+    #[test]
+    fn deleting_an_account_removes_the_staging_files_its_camera_store_writers_left() {
+        let _env = crate::testenv::ENV_LOCK
+            .lock()
+            .unwrap_or_else(|e| e.into_inner());
+        let dir = PathBuf::from(crate::test_tmp_dir("delete-camera-staging"));
+        let (primary, store, journal, other) = plant_account_state(&dir);
+        fs::remove_file(&store).unwrap();
+        fs::remove_file(&journal).unwrap();
+        let cameras = store.parent().unwrap();
+        // What an interrupted save_secondary, and an interrupted commit of
+        // the store or of its journal, leave: the store's bytes under a
+        // staging name.
+        let left = [
+            ".u.json.tmp-4242",
+            ".u.json.commit-tmp-17",
+            ".u.json.intent.commit-tmp-17",
+        ];
+        // Another account's staging files ("v", and "u.json" whose store is
+        // u.json.json), and names that only resemble u's.
+        let kept = [
+            ".v.json.tmp-4242",
+            ".u.json.json.tmp-17",
+            ".u.json.tmp-",
+            ".u.json.tmp-17.swp",
+            "u.json.tmp-17",
+            ".u.json.bak",
+        ];
+        for name in left.iter().chain(&kept) {
+            fs::write(cameras.join(name), b"{}").unwrap();
+        }
+
+        assert_eq!(
+            delete("u").unwrap(),
+            Deleted {
+                enrollment: true,
+                camera_store: true
+            }
+        );
+        for name in left {
+            assert!(!cameras.join(name).exists(), "{name} outlived the deletion");
+        }
+        for name in kept {
+            assert!(cameras.join(name).exists(), "{name} is not u's and stays");
+        }
+        assert!(!primary.exists() && other.exists());
+        std::env::remove_var("IRLUME_STATE_DIR");
+        let _ = fs::remove_dir_all(&dir);
+    }
+
     #[test]
     fn read_only_plaintext_load_preserves_enrollment_and_missing_state() {
         let _env = crate::testenv::ENV_LOCK