max / goingson
7 files changed,
+129 insertions,
-3 deletions
| @@ -239,6 +239,9 @@ pub async fn open_attachment( | |||
| 239 | 239 | let temp_dir = std::env::temp_dir().join("goingson-attachments").join(hash_prefix); | |
| 240 | 240 | std::fs::create_dir_all(&temp_dir) | |
| 241 | 241 | .map_api_err("Failed to create temp dir", ApiError::internal)?; | |
| 242 | + | // Owner-only before the copy: the decrypted blob must not be readable by | |
| 243 | + | // other local users via the world-readable system temp dir. | |
| 244 | + | super::harden_temp_dir(&temp_dir); | |
| 242 | 245 | ||
| 243 | 246 | // Sanitize filename: strip path separators, .., and control characters to prevent path traversal | |
| 244 | 247 | let safe_name: String = attachment.filename | |
| @@ -252,6 +255,7 @@ pub async fn open_attachment( | |||
| 252 | 255 | // Copy blob to temp with original filename (overwrite if exists). On the blocking | |
| 253 | 256 | // pool — a large attachment copy must not stall the reactor (Perf S1). | |
| 254 | 257 | copy_blocking(blob_path, temp_path.clone()).await?; | |
| 258 | + | super::harden_temp_file(&temp_path); | |
| 255 | 259 | ||
| 256 | 260 | open::that(&temp_path) | |
| 257 | 261 | .map_api_err("Failed to open file", ApiError::internal)?; | |
| @@ -390,6 +394,7 @@ pub async fn open_email_blob( | |||
| 390 | 394 | let temp_dir = std::env::temp_dir().join("goingson-attachments").join(hash_prefix); | |
| 391 | 395 | std::fs::create_dir_all(&temp_dir) | |
| 392 | 396 | .map_api_err("Failed to create temp dir", ApiError::internal)?; | |
| 397 | + | super::harden_temp_dir(&temp_dir); | |
| 393 | 398 | ||
| 394 | 399 | let safe_name: String = filename | |
| 395 | 400 | .replace(['/', '\\'], "_") | |
| @@ -401,6 +406,7 @@ pub async fn open_email_blob( | |||
| 401 | 406 | let temp_path = temp_dir.join(&safe_name); | |
| 402 | 407 | ||
| 403 | 408 | copy_blocking(blob_path, temp_path.clone()).await?; | |
| 409 | + | super::harden_temp_file(&temp_path); | |
| 404 | 410 | ||
| 405 | 411 | open::that(&temp_path) | |
| 406 | 412 | .map_api_err("Failed to open file", ApiError::internal)?; | |
| @@ -431,6 +437,16 @@ pub async fn save_email_blob( | |||
| 431 | 437 | ||
| 432 | 438 | // ============ Helpers ============ | |
| 433 | 439 | ||
| 440 | + | /// Remove the attachment temp spool (`<tmp>/goingson-attachments`) left by | |
| 441 | + | /// previous sessions. Opened attachments are copied there for an external app to | |
| 442 | + | /// render and are not reaped per-file (a slow app may still be holding one open), | |
| 443 | + | /// so sweeping on startup bounds their lifetime to a single session. Owner-only | |
| 444 | + | /// perms (see `harden_temp_dir`/`harden_temp_file`) keep them private meanwhile. | |
| 445 | + | pub async fn cleanup_stale_attachment_temp() { | |
| 446 | + | let dir = std::env::temp_dir().join("goingson-attachments"); | |
| 447 | + | let _ = tokio::fs::remove_dir_all(&dir).await; | |
| 448 | + | } | |
| 449 | + | ||
| 434 | 450 | /// True only for a well-formed content-addressed blob hash (64-char lowercase | |
| 435 | 451 | /// SHA-256 hex). Guards every `blobs/<hash>` path join: a `blob_hash` can arrive | |
| 436 | 452 | /// from the frontend (`open_email_blob`/`save_email_blob`) or from a synced |
| @@ -540,7 +540,12 @@ pub async fn open_email_in_browser(state: State<'_, Arc<AppState>>, id: EmailId) | |||
| 540 | 540 | let file_name = format!("goingson_email_{}_{}.html", id, uuid::Uuid::new_v4().simple()); | |
| 541 | 541 | let file_path = temp_dir.join(file_name); | |
| 542 | 542 | ||
| 543 | - | tokio::fs::write(&file_path, html_content).await | |
| 543 | + | // Written owner-only (0600): the email body/subject/sender must not be | |
| 544 | + | // readable by other local users via the world-readable system temp dir. | |
| 545 | + | let write_path = file_path.clone(); | |
| 546 | + | tokio::task::spawn_blocking(move || super::write_private_temp(&write_path, html_content.as_bytes())) | |
| 547 | + | .await | |
| 548 | + | .map_api_err("Task join error", ApiError::internal)? | |
| 544 | 549 | .map_api_err("Failed to write temp file", ApiError::internal)?; | |
| 545 | 550 | ||
| 546 | 551 | let path = file_path.clone(); |
| @@ -53,6 +53,63 @@ mod tests; | |||
| 53 | 53 | pub use error::ApiError; | |
| 54 | 54 | pub use error::{OptionNotFound, OptionApiError, ResultApiError}; | |
| 55 | 55 | ||
| 56 | + | // ============ Private temp-file helpers ============ | |
| 57 | + | // | |
| 58 | + | // Email previews and opened attachments are copied into the system temp dir so | |
| 59 | + | // an external app can render them. On Linux that dir is world-readable/traversable | |
| 60 | + | // (`/tmp`, mode 1777) and a plain copy inherits mode 0644, so any other local user | |
| 61 | + | // could read a decrypted attachment or email body. These helpers confine such | |
| 62 | + | // files to the owner. No-ops on non-unix (Windows temp is per-user; macOS confines | |
| 63 | + | // $TMPDIR per-user, and 0600/0700 there is still correct). | |
| 64 | + | ||
| 65 | + | /// Restrict a temp directory to owner-only (0700). Call this immediately after | |
| 66 | + | /// creating the directory and *before* writing any file into it: an | |
| 67 | + | /// un-traversable parent makes every file inside unreachable to other users | |
| 68 | + | /// regardless of the file's own mode, closing the copy-then-chmod race. | |
| 69 | + | pub(crate) fn harden_temp_dir(path: &std::path::Path) { | |
| 70 | + | #[cfg(unix)] | |
| 71 | + | { | |
| 72 | + | use std::os::unix::fs::PermissionsExt; | |
| 73 | + | let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)); | |
| 74 | + | } | |
| 75 | + | #[cfg(not(unix))] | |
| 76 | + | let _ = path; | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | /// Restrict a temp file to owner-only (0600). Defense-in-depth alongside | |
| 80 | + | /// [`harden_temp_dir`]; also the sole protection for files written directly into | |
| 81 | + | /// the (shared) temp root. | |
| 82 | + | pub(crate) fn harden_temp_file(path: &std::path::Path) { | |
| 83 | + | #[cfg(unix)] | |
| 84 | + | { | |
| 85 | + | use std::os::unix::fs::PermissionsExt; | |
| 86 | + | let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)); | |
| 87 | + | } | |
| 88 | + | #[cfg(not(unix))] | |
| 89 | + | let _ = path; | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | /// Write bytes to a fresh temp file created owner-only from the start. On unix | |
| 93 | + | /// the file is opened with mode 0600 atomically (no world-readable window | |
| 94 | + | /// between create and chmod) -- the right primitive for a file written directly | |
| 95 | + | /// into the shared temp root, which [`harden_temp_dir`] can't protect. | |
| 96 | + | pub(crate) fn write_private_temp(path: &std::path::Path, bytes: &[u8]) -> std::io::Result<()> { | |
| 97 | + | use std::io::Write; | |
| 98 | + | #[cfg(unix)] | |
| 99 | + | let mut file = { | |
| 100 | + | use std::os::unix::fs::OpenOptionsExt; | |
| 101 | + | std::fs::OpenOptions::new() | |
| 102 | + | .write(true) | |
| 103 | + | .create(true) | |
| 104 | + | .truncate(true) | |
| 105 | + | .mode(0o600) | |
| 106 | + | .open(path)? | |
| 107 | + | }; | |
| 108 | + | #[cfg(not(unix))] | |
| 109 | + | let mut file = std::fs::File::create(path)?; | |
| 110 | + | file.write_all(bytes) | |
| 111 | + | } | |
| 112 | + | ||
| 56 | 113 | // Re-export all commands for registration in main.rs | |
| 57 | 114 | pub use app_info::*; | |
| 58 | 115 | pub use attachment::*; |
| @@ -18,4 +18,5 @@ mod search_tests; | |||
| 18 | 18 | mod snooze_tests; | |
| 19 | 19 | mod stats_tests; | |
| 20 | 20 | mod task_tests; | |
| 21 | + | mod temp_security_tests; | |
| 21 | 22 | mod time_tracking_tests; |
| @@ -0,0 +1,45 @@ | |||
| 1 | + | //! Tests for the private temp-file helpers that keep email previews and opened | |
| 2 | + | //! attachments unreadable to other local users on a shared (world-readable) | |
| 3 | + | //! system temp dir. See `commands::{harden_temp_dir, harden_temp_file, | |
| 4 | + | //! write_private_temp}`. | |
| 5 | + | ||
| 6 | + | #[cfg(unix)] | |
| 7 | + | #[test] | |
| 8 | + | fn write_private_temp_creates_owner_only_file() { | |
| 9 | + | use std::os::unix::fs::PermissionsExt; | |
| 10 | + | ||
| 11 | + | let dir = std::env::temp_dir().join(format!("go-tst-{}", uuid::Uuid::new_v4().simple())); | |
| 12 | + | std::fs::create_dir_all(&dir).unwrap(); | |
| 13 | + | let path = dir.join("secret.txt"); | |
| 14 | + | ||
| 15 | + | crate::commands::write_private_temp(&path, b"sensitive body").unwrap(); | |
| 16 | + | ||
| 17 | + | let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; | |
| 18 | + | assert_eq!(mode, 0o600, "temp file must be owner read/write only"); | |
| 19 | + | assert_eq!(std::fs::read(&path).unwrap(), b"sensitive body"); | |
| 20 | + | ||
| 21 | + | std::fs::remove_dir_all(&dir).ok(); | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | #[cfg(unix)] | |
| 25 | + | #[test] | |
| 26 | + | fn harden_helpers_restrict_dir_and_file() { | |
| 27 | + | use std::os::unix::fs::PermissionsExt; | |
| 28 | + | ||
| 29 | + | let dir = std::env::temp_dir().join(format!("go-tst-{}", uuid::Uuid::new_v4().simple())); | |
| 30 | + | std::fs::create_dir_all(&dir).unwrap(); | |
| 31 | + | // Simulate a plain copy landing world-readable (default umask on /tmp). | |
| 32 | + | let path = dir.join("blob.bin"); | |
| 33 | + | std::fs::write(&path, b"decrypted attachment").unwrap(); | |
| 34 | + | std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap(); | |
| 35 | + | ||
| 36 | + | crate::commands::harden_temp_dir(&dir); | |
| 37 | + | crate::commands::harden_temp_file(&path); | |
| 38 | + | ||
| 39 | + | let dir_mode = std::fs::metadata(&dir).unwrap().permissions().mode() & 0o777; | |
| 40 | + | let file_mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; | |
| 41 | + | assert_eq!(dir_mode, 0o700, "temp dir must be owner-only (not traversable by others)"); | |
| 42 | + | assert_eq!(file_mode, 0o600, "temp file must be owner-only"); | |
| 43 | + | ||
| 44 | + | std::fs::remove_dir_all(&dir).ok(); | |
| 45 | + | } |
| @@ -344,9 +344,10 @@ pub fn build_mobile_app() -> tauri::Builder<tauri::Wry> { | |||
| 344 | 344 | sync_scheduler::start_sync_scheduler(cloud_sync_handle, cloud_cancel).await; | |
| 345 | 345 | }); | |
| 346 | 346 | ||
| 347 | - | // Clean up stale email preview temp files from previous sessions | |
| 347 | + | // Clean up stale email preview + attachment temp files from previous sessions | |
| 348 | 348 | tauri::async_runtime::spawn(async { | |
| 349 | 349 | commands::email::cleanup_stale_temp_files().await; | |
| 350 | + | commands::attachment::cleanup_stale_attachment_temp().await; | |
| 350 | 351 | }); | |
| 351 | 352 | ||
| 352 | 353 | Ok(()) |
| @@ -356,9 +356,10 @@ fn main() { | |||
| 356 | 356 | sync_scheduler::start_sync_scheduler(cloud_sync_handle, cloud_cancel).await; | |
| 357 | 357 | }); | |
| 358 | 358 | ||
| 359 | - | // Clean up stale email preview temp files from previous sessions | |
| 359 | + | // Clean up stale email preview + attachment temp files from previous sessions | |
| 360 | 360 | tauri::async_runtime::spawn(async { | |
| 361 | 361 | commands::cleanup_stale_temp_files().await; | |
| 362 | + | commands::cleanup_stale_attachment_temp().await; | |
| 362 | 363 | }); | |
| 363 | 364 | ||
| 364 | 365 | // Check for OTA updates after a short delay (desktop only). |