diff --git a/crates/biorouter-sandbox/src/lib.rs b/crates/biorouter-sandbox/src/lib.rs index 51aa26e19..c26610aff 100644 --- a/crates/biorouter-sandbox/src/lib.rs +++ b/crates/biorouter-sandbox/src/lib.rs @@ -44,6 +44,13 @@ pub fn protect_owner_private_handle(file: &std::fs::File) -> std::io::Result<()> owner_private::protect(file, file.metadata()?.is_dir()) } +/// Restrict a regular SQLite file without releasing this process's SQLite locks. +/// The containing directory must already be private to the current account. +#[cfg(unix)] +pub fn protect_owner_private_sqlite_file(path: &std::path::Path) -> std::io::Result<()> { + owner_private::protect_sqlite_file(path) +} + /// Refuse Windows ACL migration while another unprivileged account can mutate a directory. #[cfg(windows)] pub fn validate_owner_private_directory_acl_handle(file: &std::fs::File) -> std::io::Result<()> { diff --git a/crates/biorouter-sandbox/src/owner_private.rs b/crates/biorouter-sandbox/src/owner_private.rs index 6f068a663..7785a5a21 100644 --- a/crates/biorouter-sandbox/src/owner_private.rs +++ b/crates/biorouter-sandbox/src/owner_private.rs @@ -2,9 +2,8 @@ use std::fs::File; use std::io; #[cfg(unix)] -pub(crate) fn protect(file: &File, directory: bool) -> io::Result<()> { - use std::os::unix::fs::{MetadataExt, PermissionsExt}; - let metadata = file.metadata()?; +fn validate_owner(metadata: &std::fs::Metadata, directory: bool) -> io::Result<()> { + use std::os::unix::fs::MetadataExt; if !directory && metadata.nlink() != 1 { return Err(io::Error::new( io::ErrorKind::PermissionDenied, @@ -17,6 +16,13 @@ pub(crate) fn protect(file: &File, directory: bool) -> io::Result<()> { "Private storage must be owned by the current account", )); } + Ok(()) +} + +#[cfg(unix)] +pub(crate) fn protect(file: &File, directory: bool) -> io::Result<()> { + use std::os::unix::fs::PermissionsExt; + validate_owner(&file.metadata()?, directory)?; let mode = if directory { 0o700 } else { 0o600 }; file.set_permissions(std::fs::Permissions::from_mode(mode))?; #[cfg(target_os = "macos")] @@ -69,6 +75,136 @@ pub(crate) fn protect(file: &File, directory: bool) -> io::Result<()> { Ok(()) } +#[cfg(unix)] +fn validate_sqlite_metadata(metadata: &std::fs::Metadata) -> io::Result<()> { + use std::os::unix::fs::MetadataExt; + + if metadata.nlink() == 0 { + return Err(io::Error::new( + io::ErrorKind::NotFound, + "SQLite removed the storage file during its permissions check", + )); + } + if !metadata.is_file() { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "session database and sidecars must be regular files", + )); + } + validate_owner(metadata, false) +} + +#[cfg(target_os = "linux")] +pub(crate) fn protect_sqlite_file(path: &std::path::Path) -> io::Result<()> { + use std::ffi::CString; + use std::os::fd::AsRawFd; + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + + // Ordinary descriptor closes release this process's SQLite fcntl locks. + // O_PATH pins the inode without releasing those locks on close. + let file = std::fs::OpenOptions::new() + .read(true) + .custom_flags(libc::O_PATH | libc::O_NOFOLLOW) + .open(path)?; + validate_sqlite_metadata(&file.metadata()?)?; + let pinned = CString::new(format!("/proc/self/fd/{}", file.as_raw_fd())).unwrap(); + let mutation_error = || { + let error = io::Error::last_os_error(); + if error.kind() == io::ErrorKind::NotFound { + io::Error::new( + io::ErrorKind::Unsupported, + "Private SQLite storage requires accessible /proc/self/fd inode paths", + ) + } else { + error + } + }; + if unsafe { libc::chmod(pinned.as_ptr(), 0o600) } != 0 { + return Err(mutation_error()); + } + if unsafe { libc::removexattr(pinned.as_ptr(), c"system.posix_acl_access".as_ptr()) } != 0 { + let error = io::Error::last_os_error(); + if !matches!(error.raw_os_error(), Some(libc::ENODATA | libc::ENOTSUP)) { + return Err(mutation_error()); + } + } + let metadata = file.metadata()?; + validate_sqlite_metadata(&metadata)?; + if metadata.permissions().mode() & 0o777 != 0o600 { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "The filesystem does not enforce owner-only permissions", + )); + } + Ok(()) +} + +#[cfg(all(unix, not(target_os = "linux")))] +pub(crate) fn protect_sqlite_file(path: &std::path::Path) -> io::Result<()> { + use std::ffi::CString; + use std::os::unix::ffi::OsStrExt; + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + let before = std::fs::symlink_metadata(path)?; + validate_sqlite_metadata(&before)?; + let name = CString::new(path.as_os_str().as_bytes()).map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidInput, + "storage path contains a null byte", + ) + })?; + if unsafe { + libc::fchmodat( + libc::AT_FDCWD, + name.as_ptr(), + 0o600, + libc::AT_SYMLINK_NOFOLLOW, + ) + } != 0 + { + return Err(io::Error::last_os_error()); + } + #[cfg(target_os = "macos")] + { + const ACL_TYPE_EXTENDED: libc::c_int = 0x100; + + unsafe extern "C" { + fn acl_init(count: libc::c_int) -> *mut libc::c_void; + fn acl_set_link_np( + path: *const libc::c_char, + kind: libc::c_int, + acl: *mut libc::c_void, + ) -> libc::c_int; + fn acl_free(acl: *mut libc::c_void) -> libc::c_int; + } + let acl = unsafe { acl_init(0) }; + if acl.is_null() { + return Err(io::Error::last_os_error()); + } + let result = unsafe { acl_set_link_np(name.as_ptr(), ACL_TYPE_EXTENDED, acl) }; + let error = (result != 0).then(io::Error::last_os_error); + unsafe { acl_free(acl) }; + if let Some(error) = error { + return Err(error); + } + } + let after = std::fs::symlink_metadata(path)?; + validate_sqlite_metadata(&after)?; + if (after.dev(), after.ino()) != (before.dev(), before.ino()) { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "Private storage changed during its permissions check", + )); + } + if after.permissions().mode() & 0o777 != 0o600 { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "The filesystem does not enforce owner-only permissions", + )); + } + Ok(()) +} + #[cfg(windows)] fn current_token_information( information: windows_sys::Win32::Security::TOKEN_INFORMATION_CLASS, diff --git a/crates/biorouter/src/session/session_manager.rs b/crates/biorouter/src/session/session_manager.rs index b33e72f7c..8cbc7294d 100644 --- a/crates/biorouter/src/session/session_manager.rs +++ b/crates/biorouter/src/session/session_manager.rs @@ -4382,22 +4382,11 @@ impl SessionStorage { .open(path)?; biorouter_sandbox::protect_owner_private_handle(&directory)?; for name in [DB_NAME, "sessions.db-wal", "sessions.db-shm"] { - let file = match fs::OpenOptions::new() - .read(true) - .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK) - .open(path.join(name)) - { - Ok(file) => file, + match biorouter_sandbox::protect_owner_private_sqlite_file(&path.join(name)) { + Ok(()) => {} Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, Err(error) => return Err(error), - }; - if !file.metadata()?.is_file() { - return Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - "session database and sidecars must be regular files", - )); } - biorouter_sandbox::protect_owner_private_handle(&file)?; } Ok(()) } diff --git a/crates/biorouter/tests/session_storage_security.rs b/crates/biorouter/tests/session_storage_security.rs index ba855dec7..4857e3cb2 100644 --- a/crates/biorouter/tests/session_storage_security.rs +++ b/crates/biorouter/tests/session_storage_security.rs @@ -1,6 +1,108 @@ use biorouter::session::{SessionManager, SessionType}; use std::fs; +#[cfg(unix)] +#[tokio::test] +async fn independent_sqlite_reader_process() { + use sqlx::{Connection, SqliteConnection}; + + let Some(path) = std::env::var_os("BIOROUTER_TEST_SQLITE_READER_PATH") else { + return; + }; + let options = sqlx::sqlite::SqliteConnectOptions::new() + .filename(path) + .create_if_missing(false); + let mut connection = SqliteConnection::connect_with(&options).await.unwrap(); + let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM sessions") + .fetch_one(&mut connection) + .await + .unwrap(); + assert!(count > 0); + connection.close().await.unwrap(); +} + +#[cfg(unix)] +#[tokio::test] +async fn live_storage_validation_preserves_cross_process_wal_locks_and_later_writes() { + use std::os::unix::fs::MetadataExt; + + let fixture = tempfile::tempdir().unwrap(); + let data = fixture.path().to_path_buf(); + let manager = SessionManager::new(data.clone()); + let first = manager + .create_session(data.clone(), "BEFORE_READER".into(), SessionType::User) + .await + .unwrap(); + let wal = data.join("sessions/sessions.db-wal"); + let original = fs::metadata(&wal).unwrap(); + + manager.validate_storage().unwrap(); + let second_manager = SessionManager::new(data.clone()); + assert_eq!( + second_manager + .get_session(&first.id, false) + .await + .unwrap() + .name, + "BEFORE_READER" + ); + + // A normal read-write SQLite connection cleans up WAL sidecars if a live + // owner's permission checks accidentally released its POSIX locks. + let reader = || { + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "independent_sqlite_reader_process", + "--nocapture", + ]) + .env( + "BIOROUTER_TEST_SQLITE_READER_PATH", + data.join("sessions/sessions.db"), + ) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let current = fs::metadata(&wal).expect("reader removed a live pool's WAL"); + assert_eq!( + (current.dev(), current.ino()), + (original.dev(), original.ino()) + ); + assert_eq!(current.nlink(), 1); + }; + reader(); + + manager + .update(&first.id) + .user_provided_name("AFTER_READER") + .apply() + .await + .unwrap(); + let later = manager + .create_session(data.clone(), "LATER_SESSION".into(), SessionType::User) + .await + .unwrap(); + reader(); + second_manager.close().await; + manager.close().await; + + let reopened = SessionManager::new(data); + assert_eq!( + reopened.get_session(&first.id, false).await.unwrap().name, + "AFTER_READER" + ); + assert_eq!( + reopened.get_session(&later.id, false).await.unwrap().name, + "LATER_SESSION" + ); + reopened.close().await; +} + #[tokio::test] async fn explicit_real_directory_relocation_preserves_history_and_reopening() { let fixture = tempfile::tempdir().unwrap(); diff --git a/docs/security/session-storage.md b/docs/security/session-storage.md index 963726f4b..6bad68bbd 100644 --- a/docs/security/session-storage.md +++ b/docs/security/session-storage.md @@ -2,6 +2,8 @@ BioRouter keeps session history in the app data directory's `sessions` folder. Before connecting to SQLite, it requires a real directory owned by the current account. Existing `sessions.db`, `sessions.db-wal`, and `sessions.db-shm` entries must be regular files owned by that account, with no symbolic-link, junction, reparse-point, or hard-link aliases. BioRouter refuses unsuitable entries before opening SQLite, without changing the linked outside file's contents or permissions. It restricts Unix directory access to that account and the known database files to mode `0600`, clears extended ACLs, and applies a protected current-user DACL to Windows session storage, including existing history files and sidecars. New database files are checked again after initialization. Filesystems that cannot enforce these permissions produce a recovery message; BioRouter does not silently continue with public history. +Unix file permission checks preserve SQLite's process locks even when another session manager or an external SQLite client opens the same store. macOS uses nofollow permission and ACL operations without opening the database files. Linux pins each inode with an `O_PATH` descriptor and changes permissions through `/proc/self/fd`; closing that descriptor preserves POSIX locks. Linux storage preparation requires that procfs descriptor path to be available and fails closed if it is unavailable. + Unsuitable storage now returns an actionable error rather than panicking during construction. The daemon checks storage before serving requests. Other hosts check it before the first database connection, and retry preparation after a failed attempt so a corrected folder can be used without replacing the session manager. For deliberate relocation, use the existing `BIOROUTER_PATH_ROOT` setting with a real, absolute app-root directory. BioRouter uses `/config`, `/data`, and `/state`; history is `/data/sessions`. This setting relocates the whole app root, so use the same value for the desktop daemon and CLI.