Merge pull request #5379 from storytold/fix/sec-prefs-private

App: keep preference copies, dialog passwords and migrated folders private

apps/vectorcraft/src/prefs.rs

@@ -62,7 +62,12 @@ pub(crate) fn keep_prefs_copy(path: &std::path::Path, bytes: &[u8], stem: &str)
     use std::io::Write as _;
     for n in 1..100u32 {
         let name = if n == 1 { format!("{stem}.json") } else { format!("{stem}-{n}.json") };
-        match std::fs::OpenOptions::new().write(true).create_new(true).open(path.with_file_name(name)) {
+        let mut options = std::fs::OpenOptions::new();
+        options.write(true).create_new(true);
+        // The preferences hold recent document paths: the copy is owner-only, whatever the umask.
+        #[cfg(unix)]
+        std::os::unix::fs::OpenOptionsExt::mode(&mut options, 0o600);
+        match options.open(path.with_file_name(name)) {
             Ok(mut file) => {
                 if file.write_all(bytes).is_ok() {
                     return;
@@ -312,6 +317,8 @@ pub(crate) fn prefs_bytes(app: &VectorcraftApp) -> serde_json::Result<Vec<u8>> {
 /// [`prefs_bytes`] as JSON.
 fn prefs_value(app: &VectorcraftApp) -> serde_json::Result<serde_json::Value> {
     let mut ui = app.ui.clone();
+    // An open dialog is never restored, and its fields can hold passwords (PDF import and save).
+    ui.dialog = None;
     ui.engine_prefs = app.session.prefs.to_json();
     serde_json::to_value(&ui)
 }
@@ -324,6 +331,19 @@ mod tests {
 
     /// Two windows open at once (#4625): the one that saves last keeps the other's changes and
     /// adds its own, instead of putting back the preferences it started from.
+    #[cfg(unix)]
+    #[test]
+    fn a_kept_preferences_copy_is_owner_only() {
+        use std::os::unix::fs::PermissionsExt as _;
+        let dir = std::env::temp_dir().join(format!("vc-prefs-copy-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        super::keep_prefs_copy(&dir.join("ui.json"), b"{}", "ui.unreadable");
+        let mode = std::fs::metadata(dir.join("ui.unreadable.json")).unwrap().permissions().mode() & 0o777;
+        assert_eq!(mode, 0o600);
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     #[test]
     fn a_window_saves_only_what_it_changed_over_another_windows_save() {
         let base = json!({"workspace": "Essentials", "toolbar": true, "engine_prefs": {"uiAccent": "blue", "uiScaling": 1.0}});

apps/vectorcraft/src/profile.rs

@@ -149,6 +149,15 @@ fn copy_dir(from: &Path, to: &Path) -> Result<(), String> {
     }
 }
 
+/// Make folder `dir`, owner-only on Unix.
+fn private_dir(dir: &Path) -> std::io::Result<()> {
+    let mut builder = std::fs::DirBuilder::new();
+    builder.recursive(true);
+    #[cfg(unix)]
+    std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700);
+    builder.create(dir)
+}
+
 fn copy_dir_into(from: &Path, to: &Path, skip: &mut impl FnMut(String)) {
     let entries = match std::fs::read_dir(from) {
         Ok(entries) => entries,
@@ -165,7 +174,12 @@ fn copy_dir_into(from: &Path, to: &Path, skip: &mut impl FnMut(String)) {
         let (src, dst) = (entry.path(), to.join(entry.file_name()));
         // A symbolic link is copied as what it points to, or skipped when that is a folder.
         let copied = match entry.file_type() {
-            Ok(kind) if kind.is_dir() => std::fs::create_dir_all(&dst).map(|()| copy_dir_into(&src, &dst, skip)),
+            // The copy keeps the folder's permissions (a private recovery folder stays private):
+            // owner-only while it fills, then the source's.
+            Ok(kind) if kind.is_dir() => private_dir(&dst).and_then(|()| {
+                copy_dir_into(&src, &dst, skip);
+                std::fs::set_permissions(&dst, std::fs::metadata(&src)?.permissions())
+            }),
             Ok(_) => std::fs::copy(&src, &dst).map(|_| ()),
             Err(e) => Err(e),
         };
@@ -387,4 +401,19 @@ mod tests {
         let _ = std::fs::remove_dir_all(&from);
         let _ = std::fs::remove_dir_all(&to);
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn a_copied_folder_keeps_its_permissions() {
+        use std::os::unix::fs::PermissionsExt as _;
+        let from = temp("copy-mode-from");
+        let to = temp("copy-mode-to");
+        std::fs::create_dir_all(from.join("recovery")).unwrap();
+        std::fs::write(from.join("recovery/copy.vectorcraft"), b"unsaved").unwrap();
+        std::fs::set_permissions(from.join("recovery"), std::fs::Permissions::from_mode(0o700)).unwrap();
+        copy_dir(&from, &to).unwrap();
+        assert_eq!(std::fs::metadata(to.join("recovery")).unwrap().permissions().mode() & 0o777, 0o700);
+        let _ = std::fs::remove_dir_all(&from);
+        let _ = std::fs::remove_dir_all(&to);
+    }
 }