diff --git a/crates/ui-egui/src/recovery.rs b/crates/ui-egui/src/recovery.rs index 4f1f1d8e8..3d5f78ad1 100644 --- a/crates/ui-egui/src/recovery.rs +++ b/crates/ui-egui/src/recovery.rs @@ -9,6 +9,9 @@ //! Encrypted documents are stored encrypted (the working file is), so recovery never writes //! plaintext of a protected document to disk. The web build has no recovery store yet. //! +//! On Unix the folder is owner-only (`0o700`), so other accounts on the machine can't read the +//! snapshots of documents that aren't encrypted. +//! //! A running instance holds an exclusively locked `instance--.lock` in the folder and //! names it in the snapshots it writes, so a second instance can tell live autosaves from a //! crash's leftovers and neither offers nor discards them (#2166). The OS releases the lock @@ -75,12 +78,31 @@ impl RecoveryStore { Self { dir: dir.into(), lock: Arc::default() } } + /// Create the folder if it is missing. On Unix only the owner may open it: snapshots are + /// unsaved copies of open documents, and with the default mode other accounts on the + /// machine could read them. A folder an earlier version made is tightened too. A volume + /// that can't change modes (FAT, some network shares) works as before, with one warning in + /// the log. + fn create_dir(&self) -> std::io::Result<()> { + std::fs::create_dir_all(&self.dir)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let open_to_others = std::fs::metadata(&self.dir).is_ok_and(|m| m.permissions().mode() & 0o077 != 0); + if open_to_others && let Err(e) = std::fs::set_permissions(&self.dir, std::fs::Permissions::from_mode(0o700)) { + static WARNED: std::sync::Once = std::sync::Once::new(); + WARNED.call_once(|| log::warn!("recovery folder {} can't be made owner-only: {e}", self.dir.display())); + } + } + Ok(()) + } + /// Mark the folder as in use by a running instance (see the module doc). Without a lock /// (the folder can't be written, or the platform has no file locks) snapshots are written /// and offered as before. pub fn claim(&self) { self.lock.get_or_init(|| { - if let Err(e) = std::fs::create_dir_all(&self.dir) { + if let Err(e) = self.create_dir() { log::warn!("recovery folder {} can't be made: {e}", self.dir.display()); return None; } @@ -166,7 +188,7 @@ impl RecoveryStore { /// Write (or replace) an entry. The PDF is written before its metadata, both atomically, so /// a listed entry always has complete bytes. pub fn write(&self, meta: &RecoveryMeta, bytes: &[u8]) -> std::io::Result<()> { - std::fs::create_dir_all(&self.dir)?; + self.create_dir()?; crate::editing::write_atomically(&self.dir.join(format!("{}.pdf", meta.key)).to_string_lossy(), bytes)?; let json = serde_json::to_vec_pretty(meta).map_err(std::io::Error::other)?; crate::editing::write_atomically(&self.dir.join(format!("{}.json", meta.key)).to_string_lossy(), &json) diff --git a/crates/ui-egui/tests/recovery.rs b/crates/ui-egui/tests/recovery.rs index 807261c8c..a259ca3ad 100644 --- a/crates/ui-egui/tests/recovery.rs +++ b/crates/ui-egui/tests/recovery.rs @@ -411,3 +411,23 @@ fn a_running_instances_autosaves_are_not_offered_to_a_second_instance() { let fourth = harness(instance()); assert_eq!(fourth.state().recoverable.len(), 1, "a name outside the folder is not looked at, and the snapshot is offered"); } + +/// Snapshots are unsaved copies of open documents, so on Unix only the owner may open the +/// folder: a new one is made that way, and one an earlier version left with the default mode is +/// tightened when an instance starts. +#[cfg(unix)] +#[test] +fn the_recovery_folder_is_owner_only() { + use std::os::unix::fs::PermissionsExt; + let mode = |s: &RecoveryStore| std::fs::metadata(s.dir()).unwrap().permissions().mode() & 0o777; + let fresh = store("owner-only"); + crashed_session(&fresh, fixture(2), None, None); + assert_eq!(files_in(&fresh), 2); + assert_eq!(mode(&fresh), 0o700); + + let earlier = store("owner-only-earlier"); + std::fs::create_dir_all(earlier.dir()).unwrap(); + std::fs::set_permissions(earlier.dir(), std::fs::Permissions::from_mode(0o755)).unwrap(); + earlier.claim(); + assert_eq!(mode(&earlier), 0o700); +}