Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions desktop/src-tauri/src/commands/export_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ pub async fn pick_save_path(
/// user cancelled the dialog.
///
/// NOT for secrets: the write is plain `std::fs::write` (no atomic commit, no
/// 0o600). Secret exports go through `pick_save_path` +
/// `key_backup::write_backup_file`.
/// 0o600). Secret exports go through `pick_save_path` and a dedicated
/// secret-file writer such as `key_backup::write_portable_backup_file`.
pub async fn save_bytes_with_dialog(
app: &AppHandle,
suggested_filename: &str,
Expand Down
9 changes: 5 additions & 4 deletions desktop/src-tauri/src/commands/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,10 @@ pub async fn verify_ncryptsec_backup(
/// Save a portable copy of an `ncryptsec1…` backup to a user-chosen path.
///
/// The input must parse as a structurally valid NIP-49 payload. The dialog is
/// selection-only; the write uses secret-file semantics (atomic + 0o600).
/// Never mutates canonical app state. Returns the chosen path, or `None` when
/// the user cancelled.
/// selection-only; the write uses the exact save-panel-authorized path with
/// owner-only permissions, sync, and reread verification. Existing files are
/// preserved rather than truncated. Never mutates canonical app state. Returns
/// the chosen path, or `None` when the user cancelled.
#[tauri::command]
pub async fn save_ncryptsec_copy(
ncryptsec: String,
Expand All @@ -324,7 +325,7 @@ pub async fn save_ncryptsec_copy(

let dest_for_write = dest.clone();
tokio::task::spawn_blocking(move || {
crate::key_backup::write_backup_file(&dest_for_write, &normalized)
crate::key_backup::write_portable_backup_file(&dest_for_write, &normalized)
Comment thread
tellaho marked this conversation as resolved.
})
.await
.map_err(|e| format!("spawn_blocking failed: {e}"))??;
Expand Down
59 changes: 57 additions & 2 deletions desktop/src-tauri/src/key_backup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,14 @@ pub fn backup_file_path(data_dir: &std::path::Path) -> std::path::PathBuf {
data_dir.join(BACKUP_FILE_NAME)
}

/// Atomically write `ncryptsec` to `path` with owner-only permissions, then
/// reread and byte-compare. Same crash-safety pattern as
/// Atomically write the app-managed `ncryptsec` backup with owner-only
/// permissions, then reread and byte-compare. Same crash-safety pattern as
/// `app_state::save_key_file`.
///
/// Portable exports selected through a native save panel must use
/// [`write_portable_backup_file`] instead: sandboxed macOS grants access to the
/// selected path, but not to the sibling temporary file this writer needs.
#[allow(dead_code)] // Retained for durable app-managed backups; portable exports must not use it.
pub fn write_backup_file(path: &std::path::Path, ncryptsec: &str) -> Result<(), String> {
use atomic_write_file::AtomicWriteFile;
use std::io::Write;
Expand All @@ -155,6 +160,56 @@ pub fn write_backup_file(path: &std::path::Path, ncryptsec: &str) -> Result<(),
file.commit()
.map_err(|e| format!("commit backup file: {e}"))?;

verify_backup_file(path, ncryptsec)
}

/// Write a user-selected portable backup without creating a sibling file.
///
/// Native macOS save panels authorize the exact selected path in protected
/// folders such as Downloads, not an atomic writer's hidden sibling. Opening
/// with `create_new` uses only that authorized path and also guarantees an
/// existing backup is never truncated: users must choose a new filename when
/// the destination already exists. After writing, the file is synced and its
/// persisted bytes are reread before success is reported.
pub fn write_portable_backup_file(path: &std::path::Path, ncryptsec: &str) -> Result<(), String> {
use std::io::Write;

let mut options = std::fs::OpenOptions::new();
options.write(true).create_new(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
options.mode(0o600);
}

let mut file = options.open(path).map_err(|error| {
if error.kind() == std::io::ErrorKind::AlreadyExists {
"backup file already exists; choose a new filename so the existing backup stays safe"
.to_string()
} else {
format!("create portable backup file: {error}")
}
})?;

let write_result = file
.write_all(ncryptsec.as_bytes())
.map_err(|e| format!("write portable backup file: {e}"))
.and_then(|()| {
file.sync_all()
.map_err(|e| format!("sync portable backup file: {e}"))
});
drop(file);

let result = write_result.and_then(|()| verify_backup_file(path, ncryptsec));
if result.is_err() {
// This function created the destination exclusively, so cleanup cannot
// clobber a backup that existed before the save attempt.
let _ = std::fs::remove_file(path);
}
result
}

fn verify_backup_file(path: &std::path::Path, ncryptsec: &str) -> Result<(), String> {
// Reread and byte-compare: only report success for bytes that are
// actually on disk.
let on_disk = std::fs::read_to_string(path).map_err(|e| format!("reread backup file: {e}"))?;
Expand Down
39 changes: 39 additions & 0 deletions desktop/src-tauri/src/key_backup_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,45 @@ fn write_backup_file_overwrites_atomically() {
assert_eq!(entries, vec![std::ffi::OsString::from(BACKUP_FILE_NAME)]);
}

#[test]
fn write_portable_backup_file_persists_0600_without_a_sibling() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("portable.ncryptsec");
write_portable_backup_file(&path, SPEC_NCRYPTSEC).unwrap();

assert_eq!(std::fs::read_to_string(&path).unwrap(), SPEC_NCRYPTSEC);
let entries: Vec<_> = std::fs::read_dir(dir.path())
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
assert_eq!(
entries,
vec![std::ffi::OsString::from("portable.ncryptsec")]
);

#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mode = std::fs::metadata(&path).unwrap().permissions().mode();
assert_eq!(mode & 0o777, 0o600, "portable backup must be owner-only");
}
}

#[test]
fn write_portable_backup_file_preserves_an_existing_backup() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("portable.ncryptsec");
std::fs::write(&path, "ncryptsec1existing").unwrap();

let error = write_portable_backup_file(&path, SPEC_NCRYPTSEC).unwrap_err();

assert!(error.contains("already exists"), "{error}");
assert_eq!(
std::fs::read_to_string(&path).unwrap(),
"ncryptsec1existing"
);
}

#[test]
fn delete_backup_file_is_idempotent() {
let dir = tempfile::tempdir().unwrap();
Expand Down
Loading