From 92d38499ed3aab84d768538024b0233feee2cfe6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 19:54:45 +0900 Subject: [PATCH 01/15] test: require private Brew cleanup audit authority --- .../tests/brew_cleanup_audit_authority.rs | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 src-tauri/tests/brew_cleanup_audit_authority.rs diff --git a/src-tauri/tests/brew_cleanup_audit_authority.rs b/src-tauri/tests/brew_cleanup_audit_authority.rs new file mode 100644 index 000000000..4272b42e8 --- /dev/null +++ b/src-tauri/tests/brew_cleanup_audit_authority.rs @@ -0,0 +1,115 @@ +#![cfg(unix)] + +use disksage_lib::brew_cleanup::{ + write_audit_record, BrewCleanupAuditRecord, SCHEMA_VERSION, +}; +use serde_json::json; +use std::os::unix::fs::PermissionsExt; + +fn valid_record() -> BrewCleanupAuditRecord { + let plan_fingerprint = "a".repeat(64); + serde_json::from_value(json!({ + "schema_version": SCHEMA_VERSION, + "plan": { + "schema_version": SCHEMA_VERSION, + "platform": "macos", + "brew_path": "/opt/homebrew/bin/brew", + "brew_identity": "1:2", + "brew_version": "Homebrew 6.0.12", + "dry_run_output": "Would remove old downloads", + "dry_run_output_truncated": false, + "observed_at_ms": 10, + "plan_fingerprint": plan_fingerprint, + "exact_approval_phrase": format!( + "DiskSage Homebrew cleanup 승인 {}", + "a".repeat(64) + ) + }, + "judgment_id": "b".repeat(64), + "verdict": "safe", + "reason": "fixed maintenance command", + "model_name": "test-model", + "judged_at_ms": 20, + "executed_at_ms": 30, + "approved_by": "human:local:test", + "command": ["brew", "cleanup", "--prune-prefix"], + "status_code": 0, + "stdout": "", + "stderr": "", + "output_truncated": false, + "rationale": "approved after bounded dry-run review" + })) + .expect("test audit record must satisfy the production schema") +} + +#[test] +fn shared_writable_app_data_parent_fails_closed_without_creating_audit_storage() { + for unsafe_write_bit in [0o020, 0o002] { + let app_data = tempfile::tempdir().expect("temporary app-data directory"); + std::fs::set_permissions( + app_data.path(), + std::fs::Permissions::from_mode(0o700 | unsafe_write_bit), + ) + .expect("make app-data directory shared-writable for regression"); + + let error = write_audit_record(app_data.path(), &valid_record()) + .expect_err("shared-writable audit parent must fail closed"); + + assert_eq!(error, "brew-cleanup-audit-parent-writable-by-others"); + assert!( + !app_data.path().join("brew-cleanup-records").exists(), + "refusing an unsafe parent must not create durable authority storage" + ); + } +} + +#[test] +fn shared_writable_audit_directory_fails_closed_without_creating_a_record() { + for unsafe_write_bit in [0o020, 0o002] { + let app_data = tempfile::tempdir().expect("temporary app-data directory"); + let audit_directory = app_data.path().join("brew-cleanup-records"); + std::fs::create_dir(&audit_directory).expect("create audit directory fixture"); + std::fs::set_permissions( + &audit_directory, + std::fs::Permissions::from_mode(0o700 | unsafe_write_bit), + ) + .expect("make audit directory shared-writable for regression"); + + let error = write_audit_record(app_data.path(), &valid_record()) + .expect_err("shared-writable audit directory must fail closed"); + + assert_eq!(error, "brew-cleanup-audit-directory-writable-by-others"); + assert_eq!( + std::fs::read_dir(&audit_directory) + .expect("refused audit directory remains readable") + .count(), + 0, + "refusing unsafe durable storage must not create an authority record" + ); + } +} + +#[test] +fn audit_storage_is_private_at_creation_and_object_bound_for_hardening() { + let source = std::fs::read_to_string( + std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/brew_cleanup.rs"), + ) + .expect("brew cleanup source must be readable"); + + assert!( + source.contains("builder.mode(0o700);"), + "the dedicated audit directory must be private from its creation boundary" + ); + assert!( + source.contains("options.mode(0o400);"), + "audit records must be owner-read-only at create_new so a crash cannot leave broader authority" + ); + assert!( + source.contains("file.set_permissions(permissions)"), + "post-write hardening must remain bound to the opened audit record" + ); + assert!( + !source.contains("std::fs::set_permissions(&path, permissions)"), + "audit hardening must not re-resolve a replaceable pathname after create_new" + ); +} From d5ae26e27f9fc0ac4ba702eb600aacbd383b4d98 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 20:15:31 +0900 Subject: [PATCH 02/15] test: move Brew audit authority regression inside crate --- .../src/brew_cleanup_audit_authority_tests.rs | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 src-tauri/src/brew_cleanup_audit_authority_tests.rs diff --git a/src-tauri/src/brew_cleanup_audit_authority_tests.rs b/src-tauri/src/brew_cleanup_audit_authority_tests.rs new file mode 100644 index 000000000..56017398c --- /dev/null +++ b/src-tauri/src/brew_cleanup_audit_authority_tests.rs @@ -0,0 +1,113 @@ +#![cfg(unix)] + +use crate::brew_cleanup::{write_audit_record, BrewCleanupAuditRecord, SCHEMA_VERSION}; +use serde_json::json; +use std::os::unix::fs::PermissionsExt; + +fn valid_record() -> BrewCleanupAuditRecord { + let plan_fingerprint = "a".repeat(64); + serde_json::from_value(json!({ + "schema_version": SCHEMA_VERSION, + "plan": { + "schema_version": SCHEMA_VERSION, + "platform": "macos", + "brew_path": "/opt/homebrew/bin/brew", + "brew_identity": "1:2", + "brew_version": "Homebrew 6.0.12", + "dry_run_output": "Would remove old downloads", + "dry_run_output_truncated": false, + "observed_at_ms": 10, + "plan_fingerprint": plan_fingerprint, + "exact_approval_phrase": format!( + "DiskSage Homebrew cleanup 승인 {}", + "a".repeat(64) + ) + }, + "judgment_id": "b".repeat(64), + "verdict": "safe", + "reason": "fixed maintenance command", + "model_name": "test-model", + "judged_at_ms": 20, + "executed_at_ms": 30, + "approved_by": "human:local:test", + "command": ["brew", "cleanup", "--prune-prefix"], + "status_code": 0, + "stdout": "", + "stderr": "", + "output_truncated": false, + "rationale": "approved after bounded dry-run review" + })) + .expect("test audit record must satisfy the production schema") +} + +#[test] +fn shared_writable_app_data_parent_fails_closed_without_creating_audit_storage() { + for unsafe_write_bit in [0o020, 0o002] { + let app_data = tempfile::tempdir().expect("temporary app-data directory"); + std::fs::set_permissions( + app_data.path(), + std::fs::Permissions::from_mode(0o700 | unsafe_write_bit), + ) + .expect("make app-data directory shared-writable for regression"); + + let error = write_audit_record(app_data.path(), &valid_record()) + .expect_err("shared-writable audit parent must fail closed"); + + assert_eq!(error, "brew-cleanup-audit-parent-writable-by-others"); + assert!( + !app_data.path().join("brew-cleanup-records").exists(), + "refusing an unsafe parent must not create durable authority storage" + ); + } +} + +#[test] +fn shared_writable_audit_directory_fails_closed_without_creating_a_record() { + for unsafe_write_bit in [0o020, 0o002] { + let app_data = tempfile::tempdir().expect("temporary app-data directory"); + let audit_directory = app_data.path().join("brew-cleanup-records"); + std::fs::create_dir(&audit_directory).expect("create audit directory fixture"); + std::fs::set_permissions( + &audit_directory, + std::fs::Permissions::from_mode(0o700 | unsafe_write_bit), + ) + .expect("make audit directory shared-writable for regression"); + + let error = write_audit_record(app_data.path(), &valid_record()) + .expect_err("shared-writable audit directory must fail closed"); + + assert_eq!(error, "brew-cleanup-audit-directory-writable-by-others"); + assert_eq!( + std::fs::read_dir(&audit_directory) + .expect("refused audit directory remains readable") + .count(), + 0, + "refusing unsafe durable storage must not create an authority record" + ); + } +} + +#[test] +fn audit_storage_is_private_at_creation_and_object_bound_for_hardening() { + let source = std::fs::read_to_string( + std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/brew_cleanup.rs"), + ) + .expect("brew cleanup source must be readable"); + + assert!( + source.contains("builder.mode(0o700);"), + "the dedicated audit directory must be private from its creation boundary" + ); + assert!( + source.contains("options.mode(0o400);"), + "audit records must be owner-read-only at create_new so a crash cannot leave broader authority" + ); + assert!( + source.contains("file.set_permissions(permissions)"), + "post-write hardening must remain bound to the opened audit record" + ); + assert!( + !source.contains("std::fs::set_permissions(&path, permissions)"), + "audit hardening must not re-resolve a replaceable pathname after create_new" + ); +} From c46cad6ffa1cd60aab2a7928b0477f8bf1a62fda Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 20:16:12 +0900 Subject: [PATCH 03/15] test: register crate-internal Brew audit authority regressions --- src-tauri/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 8e359a58c..438e6227b 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -38,6 +38,8 @@ mod reasoning; mod dataset_metadata; #[cfg_attr(coverage, allow(dead_code))] mod brew_cleanup; +#[cfg(test)] +mod brew_cleanup_audit_authority_tests; pub mod archive_git_tree; #[cfg_attr(coverage, allow(dead_code))] pub mod cloud; From fc16c5b19873e1a3c979bcea0ba40546ba55faf8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 20:16:52 +0900 Subject: [PATCH 04/15] test: remove external Brew audit regression harness --- .../tests/brew_cleanup_audit_authority.rs | 115 ------------------ 1 file changed, 115 deletions(-) delete mode 100644 src-tauri/tests/brew_cleanup_audit_authority.rs diff --git a/src-tauri/tests/brew_cleanup_audit_authority.rs b/src-tauri/tests/brew_cleanup_audit_authority.rs deleted file mode 100644 index 4272b42e8..000000000 --- a/src-tauri/tests/brew_cleanup_audit_authority.rs +++ /dev/null @@ -1,115 +0,0 @@ -#![cfg(unix)] - -use disksage_lib::brew_cleanup::{ - write_audit_record, BrewCleanupAuditRecord, SCHEMA_VERSION, -}; -use serde_json::json; -use std::os::unix::fs::PermissionsExt; - -fn valid_record() -> BrewCleanupAuditRecord { - let plan_fingerprint = "a".repeat(64); - serde_json::from_value(json!({ - "schema_version": SCHEMA_VERSION, - "plan": { - "schema_version": SCHEMA_VERSION, - "platform": "macos", - "brew_path": "/opt/homebrew/bin/brew", - "brew_identity": "1:2", - "brew_version": "Homebrew 6.0.12", - "dry_run_output": "Would remove old downloads", - "dry_run_output_truncated": false, - "observed_at_ms": 10, - "plan_fingerprint": plan_fingerprint, - "exact_approval_phrase": format!( - "DiskSage Homebrew cleanup 승인 {}", - "a".repeat(64) - ) - }, - "judgment_id": "b".repeat(64), - "verdict": "safe", - "reason": "fixed maintenance command", - "model_name": "test-model", - "judged_at_ms": 20, - "executed_at_ms": 30, - "approved_by": "human:local:test", - "command": ["brew", "cleanup", "--prune-prefix"], - "status_code": 0, - "stdout": "", - "stderr": "", - "output_truncated": false, - "rationale": "approved after bounded dry-run review" - })) - .expect("test audit record must satisfy the production schema") -} - -#[test] -fn shared_writable_app_data_parent_fails_closed_without_creating_audit_storage() { - for unsafe_write_bit in [0o020, 0o002] { - let app_data = tempfile::tempdir().expect("temporary app-data directory"); - std::fs::set_permissions( - app_data.path(), - std::fs::Permissions::from_mode(0o700 | unsafe_write_bit), - ) - .expect("make app-data directory shared-writable for regression"); - - let error = write_audit_record(app_data.path(), &valid_record()) - .expect_err("shared-writable audit parent must fail closed"); - - assert_eq!(error, "brew-cleanup-audit-parent-writable-by-others"); - assert!( - !app_data.path().join("brew-cleanup-records").exists(), - "refusing an unsafe parent must not create durable authority storage" - ); - } -} - -#[test] -fn shared_writable_audit_directory_fails_closed_without_creating_a_record() { - for unsafe_write_bit in [0o020, 0o002] { - let app_data = tempfile::tempdir().expect("temporary app-data directory"); - let audit_directory = app_data.path().join("brew-cleanup-records"); - std::fs::create_dir(&audit_directory).expect("create audit directory fixture"); - std::fs::set_permissions( - &audit_directory, - std::fs::Permissions::from_mode(0o700 | unsafe_write_bit), - ) - .expect("make audit directory shared-writable for regression"); - - let error = write_audit_record(app_data.path(), &valid_record()) - .expect_err("shared-writable audit directory must fail closed"); - - assert_eq!(error, "brew-cleanup-audit-directory-writable-by-others"); - assert_eq!( - std::fs::read_dir(&audit_directory) - .expect("refused audit directory remains readable") - .count(), - 0, - "refusing unsafe durable storage must not create an authority record" - ); - } -} - -#[test] -fn audit_storage_is_private_at_creation_and_object_bound_for_hardening() { - let source = std::fs::read_to_string( - std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/brew_cleanup.rs"), - ) - .expect("brew cleanup source must be readable"); - - assert!( - source.contains("builder.mode(0o700);"), - "the dedicated audit directory must be private from its creation boundary" - ); - assert!( - source.contains("options.mode(0o400);"), - "audit records must be owner-read-only at create_new so a crash cannot leave broader authority" - ); - assert!( - source.contains("file.set_permissions(permissions)"), - "post-write hardening must remain bound to the opened audit record" - ); - assert!( - !source.contains("std::fs::set_permissions(&path, permissions)"), - "audit hardening must not re-resolve a replaceable pathname after create_new" - ); -} From 8e676ea5e89dd473a7b0c6ca763ddb7f71599ea5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 20:30:16 +0900 Subject: [PATCH 05/15] security: bind Brew cleanup audit storage privately --- src-tauri/src/brew_cleanup.rs | 36 +++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/brew_cleanup.rs b/src-tauri/src/brew_cleanup.rs index b5ce62025..816164c2a 100644 --- a/src-tauri/src/brew_cleanup.rs +++ b/src-tauri/src/brew_cleanup.rs @@ -445,9 +445,26 @@ fn audit_directory(app_data_dir: &Path) -> Result { if parent.file_type().is_symlink() || !parent.is_dir() { return Err("brew-cleanup-audit-parent-unsafe".into()); } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if parent.permissions().mode() & 0o022 != 0 { + return Err("brew-cleanup-audit-parent-writable-by-others".into()); + } + } + let directory = app_data_dir.join("brew-cleanup-records"); - std::fs::create_dir_all(&directory) - .map_err(|_| "brew-cleanup-audit-directory-create-failed".to_string())?; + let mut builder = std::fs::DirBuilder::new(); + #[cfg(unix)] + { + use std::os::unix::fs::DirBuilderExt; + builder.mode(0o700); + } + match builder.create(&directory) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(_) => return Err("brew-cleanup-audit-directory-create-failed".into()), + } let metadata = std::fs::symlink_metadata(&directory) .map_err(|_| "brew-cleanup-audit-directory-unavailable".to_string())?; if metadata.file_type().is_symlink() || !metadata.is_dir() { @@ -456,8 +473,9 @@ fn audit_directory(app_data_dir: &Path) -> Result { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&directory, std::fs::Permissions::from_mode(0o700)) - .map_err(|_| "brew-cleanup-audit-directory-permissions-failed".to_string())?; + if metadata.permissions().mode() & 0o022 != 0 { + return Err("brew-cleanup-audit-directory-writable-by-others".into()); + } } Ok(directory) } @@ -482,7 +500,7 @@ pub fn write_audit_record( #[cfg(unix)] { use std::os::unix::fs::OpenOptionsExt; - options.mode(0o600); + options.mode(0o400); } let mut file = options .open(&path) @@ -496,8 +514,14 @@ pub fn write_audit_record( .metadata() .map_err(|_| "brew-cleanup-audit-metadata-failed".to_string())? .permissions(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + permissions.set_mode(0o400); + } + #[cfg(not(unix))] permissions.set_readonly(true); - std::fs::set_permissions(&path, permissions) + file.set_permissions(permissions) .map_err(|_| "brew-cleanup-audit-permissions-failed".to_string())?; std::fs::File::open(&directory) .and_then(|directory| directory.sync_all()) From ac175c4b922d12ddb8297cb1a7e0fa660ad6038d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 21:04:36 +0900 Subject: [PATCH 06/15] test: require directory-bound Brew audit publication --- .../src/brew_cleanup_audit_authority_tests.rs | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/src-tauri/src/brew_cleanup_audit_authority_tests.rs b/src-tauri/src/brew_cleanup_audit_authority_tests.rs index 56017398c..38507b317 100644 --- a/src-tauri/src/brew_cleanup_audit_authority_tests.rs +++ b/src-tauri/src/brew_cleanup_audit_authority_tests.rs @@ -40,6 +40,13 @@ fn valid_record() -> BrewCleanupAuditRecord { .expect("test audit record must satisfy the production schema") } +fn brew_cleanup_source() -> String { + std::fs::read_to_string( + std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/brew_cleanup.rs"), + ) + .expect("brew cleanup source must be readable") +} + #[test] fn shared_writable_app_data_parent_fails_closed_without_creating_audit_storage() { for unsafe_write_bit in [0o020, 0o002] { @@ -89,10 +96,7 @@ fn shared_writable_audit_directory_fails_closed_without_creating_a_record() { #[test] fn audit_storage_is_private_at_creation_and_object_bound_for_hardening() { - let source = std::fs::read_to_string( - std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/brew_cleanup.rs"), - ) - .expect("brew cleanup source must be readable"); + let source = brew_cleanup_source(); assert!( source.contains("builder.mode(0o700);"), @@ -111,3 +115,29 @@ fn audit_storage_is_private_at_creation_and_object_bound_for_hardening() { "audit hardening must not re-resolve a replaceable pathname after create_new" ); } + +#[test] +fn audit_publication_must_be_bound_to_an_opened_directory_identity() { + let source = brew_cleanup_source(); + let writer = source + .split_once("pub fn write_audit_record(") + .map(|(_, writer)| writer) + .expect("audit writer must remain present"); + + assert!( + writer.contains("openat(") || writer.contains("openat2("), + "record creation must be relative to an already-opened audit directory identity" + ); + assert!( + writer.contains("O_NOFOLLOW"), + "directory-relative record publication must reject symbolic-link substitution" + ); + assert!( + !writer.contains(".open(&path)"), + "record publication must not re-resolve a replaceable directory pathname" + ); + assert!( + !writer.contains("std::fs::remove_file(&path)"), + "failure cleanup must remain relative to the same opened audit directory identity" + ); +} From 5463267344a3397c66335a487ab9c80ae187190b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 22:17:08 +0900 Subject: [PATCH 07/15] build: expose libc for Unix audit authority --- src-tauri/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 445985b92..6f152e884 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -150,13 +150,13 @@ memchr = "2.8.3" infer = "0.22.0" png = "0.17.16" mail-parser = "0.11.5" +libc = "0.2" [target.'cfg(windows)'.dependencies] winapi-util = "0.1.11" [target.'cfg(target_os = "macos")'.dependencies] embed_plist = "1.2.2" -libc = "0.2" objc2 = "0.6.4" objc2-foundation = { version = "0.3.2", default-features = false, features = ["std", "NSError", "NSFileManager", "NSObject", "NSString", "NSURL", "NSValue"] } plist = "1" From 9d7c06a2ea5dc09d6855406a0b013e8dd7337a9a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 22:20:34 +0900 Subject: [PATCH 08/15] security: bind Brew audit publication to directory identity --- src-tauri/src/brew_cleanup.rs | 181 ++++++++++++++++++++++++++++------ 1 file changed, 149 insertions(+), 32 deletions(-) diff --git a/src-tauri/src/brew_cleanup.rs b/src-tauri/src/brew_cleanup.rs index 816164c2a..1f67b6220 100644 --- a/src-tauri/src/brew_cleanup.rs +++ b/src-tauri/src/brew_cleanup.rs @@ -480,33 +480,122 @@ fn audit_directory(app_data_dir: &Path) -> Result { Ok(directory) } -pub fn write_audit_record( +fn prepare_audit_record( app_data_dir: &Path, record: &BrewCleanupAuditRecord, -) -> Result { +) -> Result<(PathBuf, String, PathBuf, Vec), String> { let directory = audit_directory(app_data_dir)?; let filename = format!( "{:020}-{}-{}.json", record.executed_at_ms, record.plan.plan_fingerprint, record.judgment_id ); - let path = directory.join(filename); + if filename.as_bytes().contains(&b'/') || filename == "." || filename == ".." { + return Err("brew-cleanup-audit-filename-invalid".into()); + } + let path = directory.join(&filename); let encoded = serde_json::to_vec_pretty(record) .map_err(|_| "brew-cleanup-audit-serialization-failed".to_string())?; if encoded.len() > MAX_AUDIT_BYTES { return Err("brew-cleanup-audit-too-large".into()); } - let mut options = std::fs::OpenOptions::new(); - options.write(true).create_new(true); + Ok((directory, filename, path, encoded)) +} + +pub fn write_audit_record( + app_data_dir: &Path, + record: &BrewCleanupAuditRecord, +) -> Result { + let (directory, filename, path, encoded) = prepare_audit_record(app_data_dir, record)?; #[cfg(unix)] { - use std::os::unix::fs::OpenOptionsExt; - options.mode(0o400); + return write_audit_record_unix_with_hook( + &directory, + &filename, + &path, + &encoded, + || {}, + ); + } + #[cfg(not(unix))] + { + let _ = (directory, filename, path, encoded); + Err("brew-cleanup-audit-platform-unsupported".into()) } - let mut file = options - .open(&path) - .map_err(|_| "brew-cleanup-audit-create-failed".to_string())?; +} + +#[cfg(unix)] +fn write_audit_record_unix_with_hook( + directory: &Path, + filename: &str, + path: &Path, + encoded: &[u8], + before_create: F, +) -> Result +where + F: FnOnce(), +{ + use std::ffi::CString; + use std::os::fd::{AsRawFd, FromRawFd}; + use std::os::unix::ffi::OsStrExt; + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + let directory_name = CString::new(directory.as_os_str().as_bytes()) + .map_err(|_| "brew-cleanup-audit-directory-invalid".to_string())?; + let directory_fd = unsafe { + libc::open( + directory_name.as_ptr(), + libc::O_RDONLY | libc::O_CLOEXEC | libc::O_DIRECTORY | libc::O_NOFOLLOW, + ) + }; + if directory_fd < 0 { + return Err("brew-cleanup-audit-directory-open-failed".into()); + } + let directory_file = unsafe { std::fs::File::from_raw_fd(directory_fd) }; + let opened_directory = directory_file + .metadata() + .map_err(|_| "brew-cleanup-audit-directory-metadata-failed".to_string())?; + let current_directory = std::fs::symlink_metadata(directory) + .map_err(|_| "brew-cleanup-audit-directory-identity-drift".to_string())?; + if !opened_directory.is_dir() + || current_directory.file_type().is_symlink() + || !current_directory.is_dir() + || opened_directory.permissions().mode() & 0o022 != 0 + || current_directory.permissions().mode() & 0o022 != 0 + || opened_directory.dev() != current_directory.dev() + || opened_directory.ino() != current_directory.ino() + { + return Err("brew-cleanup-audit-directory-identity-drift".into()); + } + + let record_name = CString::new(filename) + .map_err(|_| "brew-cleanup-audit-filename-invalid".to_string())?; + before_create(); + let record_fd = unsafe { + libc::openat( + directory_file.as_raw_fd(), + record_name.as_ptr(), + libc::O_WRONLY + | libc::O_CREAT + | libc::O_EXCL + | libc::O_CLOEXEC + | libc::O_NOFOLLOW, + 0o400, + ) + }; + if record_fd < 0 { + return Err("brew-cleanup-audit-create-failed".into()); + } + let mut file = unsafe { std::fs::File::from_raw_fd(record_fd) }; + + let cleanup = || { + unsafe { + libc::unlinkat(directory_file.as_raw_fd(), record_name.as_ptr(), 0); + } + let _ = directory_file.sync_all(); + }; + let result = (|| -> Result<(), String> { - file.write_all(&encoded) + file.write_all(encoded) .and_then(|_| file.write_all(b"\n")) .and_then(|_| file.sync_all()) .map_err(|_| "brew-cleanup-audit-write-failed".to_string())?; @@ -514,25 +603,55 @@ pub fn write_audit_record( .metadata() .map_err(|_| "brew-cleanup-audit-metadata-failed".to_string())? .permissions(); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - permissions.set_mode(0o400); - } - #[cfg(not(unix))] - permissions.set_readonly(true); + permissions.set_mode(0o400); file.set_permissions(permissions) .map_err(|_| "brew-cleanup-audit-permissions-failed".to_string())?; - std::fs::File::open(&directory) - .and_then(|directory| directory.sync_all()) - .map_err(|_| "brew-cleanup-audit-directory-sync-failed".to_string()) + directory_file + .sync_all() + .map_err(|_| "brew-cleanup-audit-directory-sync-failed".to_string())?; + + let opened_directory = directory_file + .metadata() + .map_err(|_| "brew-cleanup-audit-directory-metadata-failed".to_string())?; + let current_directory = std::fs::symlink_metadata(directory) + .map_err(|_| "brew-cleanup-audit-directory-identity-drift".to_string())?; + let opened_record = file + .metadata() + .map_err(|_| "brew-cleanup-audit-metadata-failed".to_string())?; + let current_record = std::fs::symlink_metadata(path) + .map_err(|_| "brew-cleanup-audit-directory-identity-drift".to_string())?; + if current_directory.file_type().is_symlink() + || !current_directory.is_dir() + || current_record.file_type().is_symlink() + || !current_record.is_file() + || opened_directory.dev() != current_directory.dev() + || opened_directory.ino() != current_directory.ino() + || opened_record.dev() != current_record.dev() + || opened_record.ino() != current_record.ino() + { + return Err("brew-cleanup-audit-directory-identity-drift".into()); + } + Ok(()) })(); + if let Err(error) = result { - drop(file); - let _ = std::fs::remove_file(&path); + cleanup(); return Err(error); } - Ok(path) + Ok(path.to_path_buf()) +} + +#[cfg(all(test, unix))] +pub(crate) fn write_audit_record_with_before_create_hook( + app_data_dir: &Path, + record: &BrewCleanupAuditRecord, + before_create: F, +) -> Result +where + F: FnOnce(), +{ + let (directory, filename, path, encoded) = prepare_audit_record(app_data_dir, record)?; + write_audit_record_unix_with_hook(&directory, &filename, &path, &encoded, before_create) } #[cfg(test)] @@ -621,6 +740,7 @@ mod tests { assert_eq!(output.stdout, "object-bound\n"); } + #[cfg(unix)] #[test] fn audit_records_are_create_new_and_private() { let temp = tempfile::tempdir().unwrap(); @@ -650,13 +770,10 @@ mod tests { let path = write_audit_record(temp.path(), &record).unwrap(); assert!(path.exists()); assert!(write_audit_record(temp.path(), &record).is_err()); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - assert_eq!( - std::fs::metadata(path).unwrap().permissions().mode() & 0o777, - 0o400 - ); - } + use std::os::unix::fs::PermissionsExt; + assert_eq!( + std::fs::metadata(path).unwrap().permissions().mode() & 0o777, + 0o400 + ); } } From e266a603cb95811ce3777905a95718ef6035451c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 12 Aug 2026 22:21:46 +0900 Subject: [PATCH 09/15] test: prove Brew audit directory replacement fails closed --- .../src/brew_cleanup_audit_authority_tests.rs | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/src-tauri/src/brew_cleanup_audit_authority_tests.rs b/src-tauri/src/brew_cleanup_audit_authority_tests.rs index 38507b317..a2b6f1ffc 100644 --- a/src-tauri/src/brew_cleanup_audit_authority_tests.rs +++ b/src-tauri/src/brew_cleanup_audit_authority_tests.rs @@ -1,6 +1,9 @@ #![cfg(unix)] -use crate::brew_cleanup::{write_audit_record, BrewCleanupAuditRecord, SCHEMA_VERSION}; +use crate::brew_cleanup::{ + write_audit_record, write_audit_record_with_before_create_hook, BrewCleanupAuditRecord, + SCHEMA_VERSION, +}; use serde_json::json; use std::os::unix::fs::PermissionsExt; @@ -97,14 +100,18 @@ fn shared_writable_audit_directory_fails_closed_without_creating_a_record() { #[test] fn audit_storage_is_private_at_creation_and_object_bound_for_hardening() { let source = brew_cleanup_source(); + let writer = source + .split_once("pub fn write_audit_record(") + .map(|(_, writer)| writer) + .expect("audit writer must remain present"); assert!( source.contains("builder.mode(0o700);"), "the dedicated audit directory must be private from its creation boundary" ); assert!( - source.contains("options.mode(0o400);"), - "audit records must be owner-read-only at create_new so a crash cannot leave broader authority" + writer.contains("libc::openat(") && writer.contains("0o400,"), + "audit records must be owner-read-only at descriptor-relative create_new" ); assert!( source.contains("file.set_permissions(permissions)"), @@ -141,3 +148,42 @@ fn audit_publication_must_be_bound_to_an_opened_directory_identity() { "failure cleanup must remain relative to the same opened audit directory identity" ); } + +#[test] +fn directory_replacement_after_authorization_cannot_redirect_publication() { + let app_data = tempfile::tempdir().expect("temporary app-data directory"); + let audit_directory = app_data.path().join("brew-cleanup-records"); + let moved_directory = app_data.path().join("authorized-audit-directory-moved"); + + let error = write_audit_record_with_before_create_hook( + app_data.path(), + &valid_record(), + || { + std::fs::rename(&audit_directory, &moved_directory) + .expect("move the already-authorized directory"); + std::fs::create_dir(&audit_directory).expect("install replacement directory"); + std::fs::set_permissions( + &audit_directory, + std::fs::Permissions::from_mode(0o700), + ) + .expect("keep the replacement privately writable so identity is the only defect"); + }, + ) + .expect_err("directory identity drift must fail closed"); + + assert_eq!(error, "brew-cleanup-audit-directory-identity-drift"); + assert_eq!( + std::fs::read_dir(&audit_directory) + .expect("replacement directory remains readable") + .count(), + 0, + "the replacement pathname must receive no authority record" + ); + assert_eq!( + std::fs::read_dir(&moved_directory) + .expect("original authorized directory remains readable") + .count(), + 0, + "failed publication must clean the descriptor-relative partial record" + ); +} From aaba16f80c38872e3d7789130a4f14a61b61d9a5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 13 Aug 2026 11:14:29 +0900 Subject: [PATCH 10/15] test: require standard Intel Homebrew executable target --- .../src/brew_cleanup_audit_authority_tests.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src-tauri/src/brew_cleanup_audit_authority_tests.rs b/src-tauri/src/brew_cleanup_audit_authority_tests.rs index a2b6f1ffc..20757fb05 100644 --- a/src-tauri/src/brew_cleanup_audit_authority_tests.rs +++ b/src-tauri/src/brew_cleanup_audit_authority_tests.rs @@ -187,3 +187,17 @@ fn directory_replacement_after_authorization_cannot_redirect_publication() { "failed publication must clean the descriptor-relative partial record" ); } + +#[test] +fn standard_intel_homebrew_repository_executable_is_admitted_without_following_alias_symlinks() { + let source = brew_cleanup_source(); + + assert!( + source.contains("Path::new(\"/usr/local/Homebrew/bin/brew\")"), + "fixed executable candidates must include the standard Intel Homebrew repository target" + ); + assert!( + source.contains("!metadata.file_type().is_symlink()"), + "supporting Intel Homebrew must not weaken symbolic-link rejection" + ); +} From b633e3efc90fcb053db3b6ca4f3695304206cf2c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 13 Aug 2026 11:18:33 +0900 Subject: [PATCH 11/15] revert: keep Intel Homebrew compatibility in a dedicated PR --- .../src/brew_cleanup_audit_authority_tests.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src-tauri/src/brew_cleanup_audit_authority_tests.rs b/src-tauri/src/brew_cleanup_audit_authority_tests.rs index 20757fb05..a2b6f1ffc 100644 --- a/src-tauri/src/brew_cleanup_audit_authority_tests.rs +++ b/src-tauri/src/brew_cleanup_audit_authority_tests.rs @@ -187,17 +187,3 @@ fn directory_replacement_after_authorization_cannot_redirect_publication() { "failed publication must clean the descriptor-relative partial record" ); } - -#[test] -fn standard_intel_homebrew_repository_executable_is_admitted_without_following_alias_symlinks() { - let source = brew_cleanup_source(); - - assert!( - source.contains("Path::new(\"/usr/local/Homebrew/bin/brew\")"), - "fixed executable candidates must include the standard Intel Homebrew repository target" - ); - assert!( - source.contains("!metadata.file_type().is_symlink()"), - "supporting Intel Homebrew must not weaken symbolic-link rejection" - ); -} From 16c1060adf062caff254c185589a4367fb0073be Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 21:41:59 -0700 Subject: [PATCH 12/15] test(security): reject symlinked Brew audit ancestors --- .../src/brew_cleanup_audit_authority_tests.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src-tauri/src/brew_cleanup_audit_authority_tests.rs b/src-tauri/src/brew_cleanup_audit_authority_tests.rs index a2b6f1ffc..e617b74b7 100644 --- a/src-tauri/src/brew_cleanup_audit_authority_tests.rs +++ b/src-tauri/src/brew_cleanup_audit_authority_tests.rs @@ -71,6 +71,27 @@ fn shared_writable_app_data_parent_fails_closed_without_creating_audit_storage() } } +#[test] +fn symlinked_app_data_ancestor_fails_closed_before_creating_external_storage() { + use std::os::unix::fs::symlink; + + let temp = tempfile::tempdir().expect("temporary authority root"); + let outside = temp.path().join("outside"); + std::fs::create_dir(&outside).expect("create external target directory"); + let alias = temp.path().join("app-data-alias"); + symlink(&outside, &alias).expect("create symlinked app-data ancestor"); + let app_data = alias.join("nested-app-data"); + + let error = write_audit_record(&app_data, &valid_record()) + .expect_err("symlinked app-data ancestor must fail closed"); + + assert_eq!(error, "brew-cleanup-audit-parent-unsafe"); + assert!( + !outside.join("nested-app-data").exists(), + "authority admission must reject the symlink before creating storage outside app-data" + ); +} + #[test] fn shared_writable_audit_directory_fails_closed_without_creating_a_record() { for unsafe_write_bit in [0o020, 0o002] { From c3e85196d483cd5a3fc1d4be23f52aa560ff8fd4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 21:43:10 -0700 Subject: [PATCH 13/15] fix(security): reject Brew audit ancestor symlinks --- src-tauri/src/brew_cleanup.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src-tauri/src/brew_cleanup.rs b/src-tauri/src/brew_cleanup.rs index 1f67b6220..e7a52a912 100644 --- a/src-tauri/src/brew_cleanup.rs +++ b/src-tauri/src/brew_cleanup.rs @@ -430,6 +430,23 @@ pub fn execute( const MAX_AUDIT_BYTES: usize = 128 * 1024; +fn validate_audit_parent_ancestors(app_data_dir: &Path, allow_missing: bool) -> Result<(), String> { + for ancestor in app_data_dir + .ancestors() + .filter(|ancestor| !ancestor.as_os_str().is_empty()) + { + match std::fs::symlink_metadata(ancestor) { + Ok(metadata) if metadata.file_type().is_symlink() || !metadata.is_dir() => { + return Err("brew-cleanup-audit-parent-unsafe".into()); + } + Ok(_) => {} + Err(error) if allow_missing && error.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return Err("brew-cleanup-audit-parent-unavailable".into()), + } + } + Ok(()) +} + fn audit_directory(app_data_dir: &Path) -> Result { if !app_data_dir.is_absolute() || app_data_dir @@ -438,8 +455,10 @@ fn audit_directory(app_data_dir: &Path) -> Result { { return Err("brew-cleanup-audit-directory-invalid".into()); } + validate_audit_parent_ancestors(app_data_dir, true)?; std::fs::create_dir_all(app_data_dir) .map_err(|_| "brew-cleanup-audit-parent-create-failed".to_string())?; + validate_audit_parent_ancestors(app_data_dir, false)?; let parent = std::fs::symlink_metadata(app_data_dir) .map_err(|_| "brew-cleanup-audit-parent-unavailable".to_string())?; if parent.file_type().is_symlink() || !parent.is_dir() { From eedee5e25b0461bd27dd51982e8c010f0440435c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 23 Aug 2026 22:32:37 -0700 Subject: [PATCH 14/15] security: bind private evidence publication to directory identity (#228) * test: prove private evidence parent replacement race * fix: bind private evidence writes to opened parent * test: prove private evidence parent permission drift * fix: revalidate private evidence parent permissions * test: preserve replaced private evidence on failed cleanup * security: preserve identity-mismatched evidence replacements * security: invalidate failed evidence through opened descriptor * test: prove private evidence mode survives restrictive umask * fix: normalize private evidence mode after open * test: isolate restrictive umask to record creation * test: cover post-create parent replacement * refactor: centralize private parent validation * fix(stack): preserve current audit root in private evidence owner * test: expose pre-open private evidence parent race * fix: bind private evidence parent before canonicalization * refactor: expose object-bound evidence publication primitive * test: expose shared object-bound publication seam * fix: use C-compatible private evidence mode argument --- src-tauri/src/private_evidence.rs | 558 ++++++++++++++++++++-- src-tauri/tests/private_evidence_umask.rs | 63 +++ 2 files changed, 577 insertions(+), 44 deletions(-) create mode 100644 src-tauri/tests/private_evidence_umask.rs diff --git a/src-tauri/src/private_evidence.rs b/src-tauri/src/private_evidence.rs index b35e6e8a8..b0e3d769f 100644 --- a/src-tauri/src/private_evidence.rs +++ b/src-tauri/src/private_evidence.rs @@ -17,77 +17,331 @@ pub struct PrivateEvidenceReceipt { pub is_approval: bool, } -/// Persist exact local evidence outside the audited source tree. +#[cfg(unix)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ObjectBoundPublicationError { + ParentMissing, + ParentUnavailable, + ParentUnsafe, + ParentWritableByOthers, + ParentIdentityDrift, + ForbiddenRootUnavailable, + InsideForbiddenRoot, + NameInvalid, + CreateFailed, + ModeInvalid, + WriteFailed, + MetadataFailed, + ParentSyncFailed, + RecordIdentityDrift, + InvalidationFailed, +} + +#[cfg(unix)] +fn revalidate_private_parent( + directory: &std::fs::File, + canonical_parent: &Path, + expected_dev: u64, + expected_ino: u64, +) -> Result<(), ObjectBoundPublicationError> { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + let opened = directory + .metadata() + .map_err(|_| ObjectBoundPublicationError::ParentUnavailable)?; + if !opened.is_dir() || opened.file_type().is_symlink() { + return Err(ObjectBoundPublicationError::ParentUnsafe); + } + if opened.permissions().mode() & 0o022 != 0 { + return Err(ObjectBoundPublicationError::ParentWritableByOthers); + } + + let named = std::fs::symlink_metadata(canonical_parent) + .map_err(|_| ObjectBoundPublicationError::ParentIdentityDrift)?; + if named.file_type().is_symlink() + || !named.is_dir() + || named.dev() != expected_dev + || named.ino() != expected_ino + { + return Err(ObjectBoundPublicationError::ParentIdentityDrift); + } + if named.permissions().mode() & 0o022 != 0 { + return Err(ObjectBoundPublicationError::ParentWritableByOthers); + } + Ok(()) +} + +#[cfg(unix)] +fn publication_error_string(error: ObjectBoundPublicationError) -> String { + match error { + ObjectBoundPublicationError::ParentMissing => "private-evidence-parent-missing", + ObjectBoundPublicationError::ParentUnavailable => "private-evidence-parent-unavailable", + ObjectBoundPublicationError::ParentUnsafe => "private-evidence-parent-unsafe", + ObjectBoundPublicationError::ParentWritableByOthers => { + "private-evidence-parent-writable-by-others" + } + ObjectBoundPublicationError::ParentIdentityDrift => { + "private-evidence-parent-identity-drift" + } + ObjectBoundPublicationError::ForbiddenRootUnavailable => { + "private-evidence-source-root-unavailable" + } + ObjectBoundPublicationError::InsideForbiddenRoot => "private-evidence-inside-source-root", + ObjectBoundPublicationError::NameInvalid => "private-evidence-name-invalid", + ObjectBoundPublicationError::CreateFailed => "private-evidence-create-failed", + ObjectBoundPublicationError::ModeInvalid => "private-evidence-mode-invalid", + ObjectBoundPublicationError::WriteFailed => "private-evidence-write-failed", + ObjectBoundPublicationError::MetadataFailed => "private-evidence-metadata-failed", + ObjectBoundPublicationError::ParentSyncFailed => "private-evidence-parent-sync-failed", + ObjectBoundPublicationError::RecordIdentityDrift => { + "private-evidence-record-identity-drift" + } + ObjectBoundPublicationError::InvalidationFailed => { + "private-evidence-invalidation-failed" + } + } + .to_string() +} + +/// Create and durably publish one immutable byte record relative to the exact private parent +/// directory object admitted by the caller-supplied pathname. /// -/// The destination parent must already exist, must not be a symlink, and must not be writable by -/// group or other principals. The file is created once with mode 0600, synced, and never -/// overwritten. A failed write is removed before returning. +/// The parent must already exist and must not be writable by group or other principals. The +/// pathname is opened with `O_NOFOLLOW` before canonicalization and the opened directory is bound to +/// the device/inode observed during initial admission. Record creation is descriptor-relative and +/// create-new. `forbidden_root`, when present, is checked only after the opened parent has been bound +/// and revalidated, so a pathname replacement cannot redirect publication into that root. #[cfg(unix)] -pub fn write_private_json_create_new( - source_root: &Path, +pub(crate) fn write_object_bound_bytes_create_new( path: &Path, - value: &impl Serialize, -) -> Result { - use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + encoded: &[u8], + unix_mode: u32, + forbidden_root: Option<&Path>, +) -> Result<(), ObjectBoundPublicationError> { + write_object_bound_bytes_create_new_with_hooks( + path, + encoded, + unix_mode, + forbidden_root, + || {}, + || {}, + || {}, + ) +} + +/// Internal deterministic seam for dependent authority writers to prove directory-replacement +/// handling while exercising the same descriptor-bound publication implementation used in production. +#[cfg(unix)] +pub(crate) fn write_object_bound_bytes_create_new_with_hooks( + path: &Path, + encoded: &[u8], + unix_mode: u32, + forbidden_root: Option<&Path>, + before_parent_open: F, + before_create: G, + before_finalize: H, +) -> Result<(), ObjectBoundPublicationError> +where + F: FnOnce(), + G: FnOnce(), + H: FnOnce(), +{ + use std::ffi::CString; + use std::os::fd::{AsRawFd, FromRawFd}; + use std::os::unix::ffi::OsStrExt; + use std::os::unix::fs::{MetadataExt, PermissionsExt}; let parent = path .parent() .filter(|parent| !parent.as_os_str().is_empty()) - .ok_or_else(|| "private-evidence-parent-missing".to_string())?; + .ok_or(ObjectBoundPublicationError::ParentMissing)?; let parent_metadata = std::fs::symlink_metadata(parent) - .map_err(|_| "private-evidence-parent-unavailable".to_string())?; + .map_err(|_| ObjectBoundPublicationError::ParentUnavailable)?; if !parent_metadata.is_dir() || parent_metadata.file_type().is_symlink() { - return Err("private-evidence-parent-unsafe".into()); + return Err(ObjectBoundPublicationError::ParentUnsafe); } if parent_metadata.permissions().mode() & 0o022 != 0 { - return Err("private-evidence-parent-writable-by-others".into()); + return Err(ObjectBoundPublicationError::ParentWritableByOthers); + } + let expected_parent_dev = parent_metadata.dev(); + let expected_parent_ino = parent_metadata.ino(); + + before_parent_open(); + + let parent_c = CString::new(parent.as_os_str().as_bytes()) + .map_err(|_| ObjectBoundPublicationError::ParentUnavailable)?; + let directory_fd = unsafe { + libc::open( + parent_c.as_ptr(), + libc::O_RDONLY | libc::O_CLOEXEC | libc::O_DIRECTORY | libc::O_NOFOLLOW, + ) + }; + if directory_fd < 0 { + return Err(ObjectBoundPublicationError::ParentIdentityDrift); + } + let directory = unsafe { std::fs::File::from_raw_fd(directory_fd) }; + let opened_parent_metadata = directory + .metadata() + .map_err(|_| ObjectBoundPublicationError::ParentUnavailable)?; + if opened_parent_metadata.dev() != expected_parent_dev + || opened_parent_metadata.ino() != expected_parent_ino + { + return Err(ObjectBoundPublicationError::ParentIdentityDrift); } + let canonical_parent = std::fs::canonicalize(parent) - .map_err(|_| "private-evidence-parent-unavailable".to_string())?; - let canonical_source = std::fs::canonicalize(source_root) - .map_err(|_| "private-evidence-source-root-unavailable".to_string())?; - if canonical_parent.starts_with(&canonical_source) { - return Err("private-evidence-inside-source-root".into()); + .map_err(|_| ObjectBoundPublicationError::ParentIdentityDrift)?; + revalidate_private_parent( + &directory, + &canonical_parent, + expected_parent_dev, + expected_parent_ino, + )?; + + if let Some(forbidden_root) = forbidden_root { + let canonical_forbidden = std::fs::canonicalize(forbidden_root) + .map_err(|_| ObjectBoundPublicationError::ForbiddenRootUnavailable)?; + if canonical_parent.starts_with(canonical_forbidden) { + return Err(ObjectBoundPublicationError::InsideForbiddenRoot); + } } + let file_name = path .file_name() .filter(|name| !name.is_empty()) - .ok_or_else(|| "private-evidence-name-invalid".to_string())?; + .ok_or(ObjectBoundPublicationError::NameInvalid)?; let final_path = canonical_parent.join(file_name); - let encoded = serde_json::to_vec_pretty(value) - .map_err(|_| "private-evidence-json-invalid".to_string())?; - if encoded.len() > MAX_PRIVATE_EVIDENCE_BYTES { - return Err("private-evidence-too-large".into()); + let file_name_c = CString::new(file_name.as_bytes()) + .map_err(|_| ObjectBoundPublicationError::NameInvalid)?; + + before_create(); + revalidate_private_parent( + &directory, + &canonical_parent, + expected_parent_dev, + expected_parent_ino, + )?; + + let file_fd = unsafe { + libc::openat( + directory.as_raw_fd(), + file_name_c.as_ptr(), + libc::O_WRONLY + | libc::O_CREAT + | libc::O_EXCL + | libc::O_CLOEXEC + | libc::O_NOFOLLOW, + unix_mode as libc::c_uint, + ) + }; + if file_fd < 0 { + return Err(ObjectBoundPublicationError::CreateFailed); } + let mut file = unsafe { std::fs::File::from_raw_fd(file_fd) }; - let mut file = std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .mode(0o600) - .open(&final_path) - .map_err(|_| "private-evidence-create-failed".to_string())?; - let result = (|| -> Result<(), String> { - file.write_all(&encoded) + let publication = (|| -> Result<(), ObjectBoundPublicationError> { + file.set_permissions(std::fs::Permissions::from_mode(unix_mode)) + .map_err(|_| ObjectBoundPublicationError::ModeInvalid)?; + file.write_all(encoded) .and_then(|_| file.sync_all()) - .map_err(|_| "private-evidence-write-failed".to_string())?; - let metadata = file + .map_err(|_| ObjectBoundPublicationError::WriteFailed)?; + let opened_file_metadata = file .metadata() - .map_err(|_| "private-evidence-metadata-failed".to_string())?; - if !metadata.is_file() - || metadata.file_type().is_symlink() - || metadata.permissions().mode() & 0o777 != 0o600 + .map_err(|_| ObjectBoundPublicationError::MetadataFailed)?; + if !opened_file_metadata.is_file() + || opened_file_metadata.file_type().is_symlink() + || opened_file_metadata.permissions().mode() & 0o777 != unix_mode { - return Err("private-evidence-mode-invalid".into()); + return Err(ObjectBoundPublicationError::ModeInvalid); } - std::fs::File::open(&canonical_parent) - .and_then(|directory| directory.sync_all()) - .map_err(|_| "private-evidence-parent-sync-failed".to_string()) + directory + .sync_all() + .map_err(|_| ObjectBoundPublicationError::ParentSyncFailed)?; + + before_finalize(); + + revalidate_private_parent( + &directory, + &canonical_parent, + expected_parent_dev, + expected_parent_ino, + )?; + + let final_file_metadata = std::fs::symlink_metadata(&final_path) + .map_err(|_| ObjectBoundPublicationError::RecordIdentityDrift)?; + if final_file_metadata.file_type().is_symlink() + || !final_file_metadata.is_file() + || final_file_metadata.dev() != opened_file_metadata.dev() + || final_file_metadata.ino() != opened_file_metadata.ino() + { + return Err(ObjectBoundPublicationError::RecordIdentityDrift); + } + Ok(()) })(); - if let Err(error) = result { - drop(file); - let _ = std::fs::remove_file(&final_path); + + if let Err(error) = publication { + let invalidation = file + .set_len(0) + .and_then(|_| file.sync_all()) + .and_then(|_| directory.sync_all()); + if invalidation.is_err() { + return Err(ObjectBoundPublicationError::InvalidationFailed); + } return Err(error); } + Ok(()) +} + +/// Persist exact local evidence outside the audited source tree. +/// +/// The destination parent must already exist, must not be a symlink, and must not be writable by +/// group or other principals. On Unix, publication is bound to the exact caller-supplied parent +/// directory object admitted before canonicalization, so a same-user pathname replacement cannot +/// redirect either canonicalization or the later write. The file is created once with mode 0600, +/// synced, and never overwritten. After a post-create failure, the still-open record is truncated +/// and synced through its descriptor. The pathname is deliberately not unlinked because a same-user +/// process may already have replaced that name; this can leave a zero-length mode-0600 create-new +/// tombstone that requires explicit operator cleanup. +#[cfg(unix)] +pub fn write_private_json_create_new( + source_root: &Path, + path: &Path, + value: &impl Serialize, +) -> Result { + write_private_json_create_new_unix_with_hooks(source_root, path, value, || {}, || {}, || {}) +} + +#[cfg(unix)] +fn write_private_json_create_new_unix_with_hooks( + source_root: &Path, + path: &Path, + value: &impl Serialize, + before_parent_open: F, + before_create: G, + before_finalize: H, +) -> Result +where + F: FnOnce(), + G: FnOnce(), + H: FnOnce(), +{ + let encoded = serde_json::to_vec_pretty(value) + .map_err(|_| "private-evidence-json-invalid".to_string())?; + if encoded.len() > MAX_PRIVATE_EVIDENCE_BYTES { + return Err("private-evidence-too-large".into()); + } + + write_object_bound_bytes_create_new_with_hooks( + path, + &encoded, + 0o600, + Some(source_root), + before_parent_open, + before_create, + before_finalize, + ) + .map_err(publication_error_string)?; + let sha256 = Sha256::digest(&encoded) .iter() .map(|byte| format!("{byte:02x}")) @@ -164,4 +418,220 @@ mod tests { assert_eq!(error, "private-evidence-parent-writable-by-others"); assert!(!path.exists()); } + + #[cfg(unix)] + #[test] + fn fails_closed_if_parent_is_replaced_before_open() { + use std::os::unix::fs::PermissionsExt; + + let source = tempfile::tempdir().unwrap(); + let fixture = tempfile::tempdir().unwrap(); + let parent = fixture.path().join("records"); + let moved_parent = fixture.path().join("authorized-records-moved"); + std::fs::create_dir(&parent).unwrap(); + std::fs::set_permissions(&parent, std::fs::Permissions::from_mode(0o700)).unwrap(); + let path = parent.join("audit.json"); + let parent_for_hook = parent.clone(); + let moved_for_hook = moved_parent.clone(); + + let error = write_private_json_create_new_unix_with_hooks( + source.path(), + &path, + &serde_json::json!({"private": true}), + move || { + std::fs::rename(&parent_for_hook, &moved_for_hook).unwrap(); + std::fs::create_dir(&parent_for_hook).unwrap(); + std::fs::set_permissions( + &parent_for_hook, + std::fs::Permissions::from_mode(0o700), + ) + .unwrap(); + }, + || {}, + || {}, + ) + .unwrap_err(); + + assert_eq!(error, "private-evidence-parent-identity-drift"); + assert!(!parent.join("audit.json").exists()); + assert!(!moved_parent.join("audit.json").exists()); + } + + #[cfg(unix)] + #[test] + fn fails_closed_if_parent_becomes_shared_writable_after_authorization() { + use std::os::unix::fs::PermissionsExt; + + let source = tempfile::tempdir().unwrap(); + let private = tempfile::tempdir().unwrap(); + std::fs::set_permissions(private.path(), std::fs::Permissions::from_mode(0o700)).unwrap(); + let path = private.path().join("audit.json"); + let parent_for_hook = private.path().to_path_buf(); + + let error = write_private_json_create_new_unix_with_hooks( + source.path(), + &path, + &serde_json::json!({"private": true}), + || {}, + move || { + std::fs::set_permissions( + &parent_for_hook, + std::fs::Permissions::from_mode(0o770), + ) + .unwrap(); + }, + || {}, + ) + .unwrap_err(); + + assert_eq!(error, "private-evidence-parent-writable-by-others"); + assert!(!path.exists()); + } + + #[cfg(unix)] + #[test] + fn failed_post_create_validation_leaves_private_zero_length_tombstone() { + use std::os::unix::fs::PermissionsExt; + + let source = tempfile::tempdir().unwrap(); + let private = tempfile::tempdir().unwrap(); + std::fs::set_permissions(private.path(), std::fs::Permissions::from_mode(0o700)).unwrap(); + let path = private.path().join("audit.json"); + let parent_for_hook = private.path().to_path_buf(); + + let error = write_private_json_create_new_unix_with_hooks( + source.path(), + &path, + &serde_json::json!({"private": true}), + || {}, + || {}, + move || { + std::fs::set_permissions( + &parent_for_hook, + std::fs::Permissions::from_mode(0o770), + ) + .unwrap(); + }, + ) + .unwrap_err(); + + assert_eq!(error, "private-evidence-parent-writable-by-others"); + let metadata = std::fs::metadata(&path).unwrap(); + assert_eq!(metadata.len(), 0); + assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + } + + #[cfg(unix)] + #[test] + fn fails_closed_if_private_parent_is_replaced_after_authorization() { + use std::os::unix::fs::PermissionsExt; + + let source = tempfile::tempdir().unwrap(); + let fixture = tempfile::tempdir().unwrap(); + let parent = fixture.path().join("records"); + let moved_parent = fixture.path().join("authorized-records-moved"); + std::fs::create_dir(&parent).unwrap(); + std::fs::set_permissions(&parent, std::fs::Permissions::from_mode(0o700)).unwrap(); + let path = parent.join("audit.json"); + let replacement_parent = parent.clone(); + let parent_for_hook = parent.clone(); + let moved_for_hook = moved_parent.clone(); + + let error = write_private_json_create_new_unix_with_hooks( + source.path(), + &path, + &serde_json::json!({"private": true}), + || {}, + move || { + std::fs::rename(&parent_for_hook, &moved_for_hook).unwrap(); + std::fs::create_dir(&parent_for_hook).unwrap(); + std::fs::set_permissions( + &parent_for_hook, + std::fs::Permissions::from_mode(0o700), + ) + .unwrap(); + }, + || {}, + ) + .unwrap_err(); + + assert_eq!(error, "private-evidence-parent-identity-drift"); + assert!(!replacement_parent.join("audit.json").exists()); + assert!(!moved_parent.join("audit.json").exists()); + } + + #[cfg(unix)] + #[test] + fn post_create_parent_replacement_invalidates_only_authorized_record() { + use std::os::unix::fs::PermissionsExt; + + let source = tempfile::tempdir().unwrap(); + let fixture = tempfile::tempdir().unwrap(); + let parent = fixture.path().join("records"); + let moved_parent = fixture.path().join("authorized-records-moved"); + std::fs::create_dir(&parent).unwrap(); + std::fs::set_permissions(&parent, std::fs::Permissions::from_mode(0o700)).unwrap(); + let path = parent.join("audit.json"); + let replacement_parent = parent.clone(); + let parent_for_hook = parent.clone(); + let moved_for_hook = moved_parent.clone(); + + let error = write_private_json_create_new_unix_with_hooks( + source.path(), + &path, + &serde_json::json!({"private": true}), + || {}, + || {}, + move || { + std::fs::rename(&parent_for_hook, &moved_for_hook).unwrap(); + std::fs::create_dir(&parent_for_hook).unwrap(); + std::fs::set_permissions( + &parent_for_hook, + std::fs::Permissions::from_mode(0o700), + ) + .unwrap(); + }, + ) + .unwrap_err(); + + assert_eq!(error, "private-evidence-parent-identity-drift"); + assert!( + !replacement_parent.join("audit.json").exists(), + "replacement directory must never receive the authorized record" + ); + let tombstone = moved_parent.join("audit.json"); + let metadata = std::fs::metadata(&tombstone).unwrap(); + assert_eq!(metadata.len(), 0, "authorized record must be invalidated"); + assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + } + + #[cfg(unix)] + #[test] + fn failed_final_identity_check_preserves_unrelated_replacement_record() { + use std::os::unix::fs::PermissionsExt; + + let source = tempfile::tempdir().unwrap(); + let private = tempfile::tempdir().unwrap(); + std::fs::set_permissions(private.path(), std::fs::Permissions::from_mode(0o700)).unwrap(); + let path = private.path().join("audit.json"); + let path_for_hook = path.clone(); + let replacement = b"attacker-replacement".to_vec(); + let replacement_for_hook = replacement.clone(); + + let error = write_private_json_create_new_unix_with_hooks( + source.path(), + &path, + &serde_json::json!({"private": true}), + || {}, + || {}, + move || { + std::fs::remove_file(&path_for_hook).unwrap(); + std::fs::write(&path_for_hook, &replacement_for_hook).unwrap(); + }, + ) + .unwrap_err(); + + assert_eq!(error, "private-evidence-record-identity-drift"); + assert_eq!(std::fs::read(&path).unwrap(), replacement); + } } diff --git a/src-tauri/tests/private_evidence_umask.rs b/src-tauri/tests/private_evidence_umask.rs new file mode 100644 index 000000000..b8b871181 --- /dev/null +++ b/src-tauri/tests/private_evidence_umask.rs @@ -0,0 +1,63 @@ +//! Regression coverage for private-evidence creation under a restrictive process umask. +//! +//! The Unix `openat(O_CREAT, 0o600)` mode argument is filtered by the process umask. DiskSage +//! promises a durable private evidence object with exact mode `0600`, so publication must explicitly +//! harden the already-opened descriptor rather than relying on the creation request alone. + +#![cfg(unix)] + +use disksage_lib::private_evidence::write_private_json_create_new; +use std::os::unix::fs::PermissionsExt; + +const CHILD_ENV: &str = "DISKSAGE_PRIVATE_EVIDENCE_RESTRICTIVE_UMASK_CHILD"; + +#[test] +fn restrictive_umask_still_publishes_mode_0600() { + if std::env::var_os(CHILD_ENV).is_none() { + let status = std::process::Command::new(std::env::current_exe().expect("test executable")) + .arg("--exact") + .arg("restrictive_umask_still_publishes_mode_0600") + .arg("--nocapture") + .env(CHILD_ENV, "1") + .status() + .expect("spawn isolated restrictive-umask test process"); + assert!( + status.success(), + "private evidence publication must succeed under a restrictive umask" + ); + return; + } + + // Build writable fixtures before changing the process-global umask. Applying 0o200 while + // tempfile creates its private directories can remove owner-write permission from the parent + // itself, which tests directory writability rather than the record-creation boundary. + let source = tempfile::tempdir().expect("source tempdir"); + let private = tempfile::tempdir().expect("private tempdir"); + let path = private.path().join("audit.json"); + + // Isolate the process-global umask in this child test process so concurrently executing tests + // cannot observe the temporary mask. Removing owner-write makes a raw openat(..., 0o600) + // create mode 0400 and reproduces the production file-mode boundary. + let previous_umask = unsafe { libc::umask(0o200) }; + let publication = write_private_json_create_new( + source.path(), + &path, + &serde_json::json!({"private": true}), + ); + unsafe { + libc::umask(previous_umask); + } + let receipt = publication.expect("publication must normalize the opened file to mode 0600"); + + assert!(receipt.written); + assert_eq!(receipt.unix_mode, "0600"); + assert_eq!( + std::fs::metadata(&path) + .expect("published evidence metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + assert!(std::fs::metadata(&path).expect("published evidence").len() > 0); +} From fb274c2d1de12459c24da5a093a21301e7d5830e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 30 Aug 2026 00:46:27 +0900 Subject: [PATCH 15/15] style: format private audit authority --- src-tauri/src/brew_cleanup.rs | 18 ++----- .../src/brew_cleanup_audit_authority_tests.rs | 18 +++---- src-tauri/src/private_evidence.rs | 49 ++++++------------- src-tauri/tests/private_evidence_umask.rs | 7 +-- 4 files changed, 26 insertions(+), 66 deletions(-) diff --git a/src-tauri/src/brew_cleanup.rs b/src-tauri/src/brew_cleanup.rs index e720ee173..f04144571 100644 --- a/src-tauri/src/brew_cleanup.rs +++ b/src-tauri/src/brew_cleanup.rs @@ -567,13 +567,7 @@ pub fn write_audit_record( let (directory, filename, path, encoded) = prepare_audit_record(app_data_dir, record)?; #[cfg(unix)] { - return write_audit_record_unix_with_hook( - &directory, - &filename, - &path, - &encoded, - || {}, - ); + return write_audit_record_unix_with_hook(&directory, &filename, &path, &encoded, || {}); } #[cfg(not(unix))] { @@ -626,18 +620,14 @@ where return Err("brew-cleanup-audit-directory-identity-drift".into()); } - let record_name = CString::new(filename) - .map_err(|_| "brew-cleanup-audit-filename-invalid".to_string())?; + let record_name = + CString::new(filename).map_err(|_| "brew-cleanup-audit-filename-invalid".to_string())?; before_create(); let record_fd = unsafe { libc::openat( directory_file.as_raw_fd(), record_name.as_ptr(), - libc::O_WRONLY - | libc::O_CREAT - | libc::O_EXCL - | libc::O_CLOEXEC - | libc::O_NOFOLLOW, + libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL | libc::O_CLOEXEC | libc::O_NOFOLLOW, 0o400, ) }; diff --git a/src-tauri/src/brew_cleanup_audit_authority_tests.rs b/src-tauri/src/brew_cleanup_audit_authority_tests.rs index e617b74b7..74a6a63b1 100644 --- a/src-tauri/src/brew_cleanup_audit_authority_tests.rs +++ b/src-tauri/src/brew_cleanup_audit_authority_tests.rs @@ -176,21 +176,15 @@ fn directory_replacement_after_authorization_cannot_redirect_publication() { let audit_directory = app_data.path().join("brew-cleanup-records"); let moved_directory = app_data.path().join("authorized-audit-directory-moved"); - let error = write_audit_record_with_before_create_hook( - app_data.path(), - &valid_record(), - || { + let error = + write_audit_record_with_before_create_hook(app_data.path(), &valid_record(), || { std::fs::rename(&audit_directory, &moved_directory) .expect("move the already-authorized directory"); std::fs::create_dir(&audit_directory).expect("install replacement directory"); - std::fs::set_permissions( - &audit_directory, - std::fs::Permissions::from_mode(0o700), - ) - .expect("keep the replacement privately writable so identity is the only defect"); - }, - ) - .expect_err("directory identity drift must fail closed"); + std::fs::set_permissions(&audit_directory, std::fs::Permissions::from_mode(0o700)) + .expect("keep the replacement privately writable so identity is the only defect"); + }) + .expect_err("directory identity drift must fail closed"); assert_eq!(error, "brew-cleanup-audit-directory-identity-drift"); assert_eq!( diff --git a/src-tauri/src/private_evidence.rs b/src-tauri/src/private_evidence.rs index b0e3d769f..ea8b4f3cd 100644 --- a/src-tauri/src/private_evidence.rs +++ b/src-tauri/src/private_evidence.rs @@ -96,9 +96,7 @@ fn publication_error_string(error: ObjectBoundPublicationError) -> String { ObjectBoundPublicationError::RecordIdentityDrift => { "private-evidence-record-identity-drift" } - ObjectBoundPublicationError::InvalidationFailed => { - "private-evidence-invalidation-failed" - } + ObjectBoundPublicationError::InvalidationFailed => "private-evidence-invalidation-failed", } .to_string() } @@ -211,8 +209,8 @@ where .filter(|name| !name.is_empty()) .ok_or(ObjectBoundPublicationError::NameInvalid)?; let final_path = canonical_parent.join(file_name); - let file_name_c = CString::new(file_name.as_bytes()) - .map_err(|_| ObjectBoundPublicationError::NameInvalid)?; + let file_name_c = + CString::new(file_name.as_bytes()).map_err(|_| ObjectBoundPublicationError::NameInvalid)?; before_create(); revalidate_private_parent( @@ -226,11 +224,7 @@ where libc::openat( directory.as_raw_fd(), file_name_c.as_ptr(), - libc::O_WRONLY - | libc::O_CREAT - | libc::O_EXCL - | libc::O_CLOEXEC - | libc::O_NOFOLLOW, + libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL | libc::O_CLOEXEC | libc::O_NOFOLLOW, unix_mode as libc::c_uint, ) }; @@ -441,11 +435,8 @@ mod tests { move || { std::fs::rename(&parent_for_hook, &moved_for_hook).unwrap(); std::fs::create_dir(&parent_for_hook).unwrap(); - std::fs::set_permissions( - &parent_for_hook, - std::fs::Permissions::from_mode(0o700), - ) - .unwrap(); + std::fs::set_permissions(&parent_for_hook, std::fs::Permissions::from_mode(0o700)) + .unwrap(); }, || {}, || {}, @@ -474,11 +465,8 @@ mod tests { &serde_json::json!({"private": true}), || {}, move || { - std::fs::set_permissions( - &parent_for_hook, - std::fs::Permissions::from_mode(0o770), - ) - .unwrap(); + std::fs::set_permissions(&parent_for_hook, std::fs::Permissions::from_mode(0o770)) + .unwrap(); }, || {}, ) @@ -506,11 +494,8 @@ mod tests { || {}, || {}, move || { - std::fs::set_permissions( - &parent_for_hook, - std::fs::Permissions::from_mode(0o770), - ) - .unwrap(); + std::fs::set_permissions(&parent_for_hook, std::fs::Permissions::from_mode(0o770)) + .unwrap(); }, ) .unwrap_err(); @@ -545,11 +530,8 @@ mod tests { move || { std::fs::rename(&parent_for_hook, &moved_for_hook).unwrap(); std::fs::create_dir(&parent_for_hook).unwrap(); - std::fs::set_permissions( - &parent_for_hook, - std::fs::Permissions::from_mode(0o700), - ) - .unwrap(); + std::fs::set_permissions(&parent_for_hook, std::fs::Permissions::from_mode(0o700)) + .unwrap(); }, || {}, ) @@ -585,11 +567,8 @@ mod tests { move || { std::fs::rename(&parent_for_hook, &moved_for_hook).unwrap(); std::fs::create_dir(&parent_for_hook).unwrap(); - std::fs::set_permissions( - &parent_for_hook, - std::fs::Permissions::from_mode(0o700), - ) - .unwrap(); + std::fs::set_permissions(&parent_for_hook, std::fs::Permissions::from_mode(0o700)) + .unwrap(); }, ) .unwrap_err(); diff --git a/src-tauri/tests/private_evidence_umask.rs b/src-tauri/tests/private_evidence_umask.rs index b8b871181..1e3f8afb7 100644 --- a/src-tauri/tests/private_evidence_umask.rs +++ b/src-tauri/tests/private_evidence_umask.rs @@ -39,11 +39,8 @@ fn restrictive_umask_still_publishes_mode_0600() { // cannot observe the temporary mask. Removing owner-write makes a raw openat(..., 0o600) // create mode 0400 and reproduces the production file-mode boundary. let previous_umask = unsafe { libc::umask(0o200) }; - let publication = write_private_json_create_new( - source.path(), - &path, - &serde_json::json!({"private": true}), - ); + let publication = + write_private_json_create_new(source.path(), &path, &serde_json::json!({"private": true})); unsafe { libc::umask(previous_umask); }