From 2762a169045535646b352f46fb54fd52ca965a13 Mon Sep 17 00:00:00 2001 From: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Date: Fri, 14 Aug 2026 17:07:21 -0700 Subject: [PATCH 1/5] fix(system): confine batch session exports to the selected directory save_exported_session_files wrote folder_path.join(renderer filename) after resolve_export_filename returned unsafe names unchanged, so absolute paths and .. components escaped the user-selected folder. Validate the renderer-controlled filename at the IPC boundary: accept only a single Component::Normal leaf, rejecting empty/whitespace names, /, backslash and : separators, embedded NUL, absolute/root/prefix/parent components, dot-only names, trailing dot/space, and Windows reserved device names. Write with OpenOptions::create_new(true) so a pre-existing file, directory, or symlink at the target is skipped rather than followed or overwritten, closing the symlink/TOCTOU escape while preserving deterministic -N collision suffixing. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Signed-off-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> --- src-tauri/src/commands/system.rs | 298 +++++++++++++++++++++++++++++-- 1 file changed, 279 insertions(+), 19 deletions(-) diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 93726bb3b..eedb81616 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -482,11 +482,12 @@ pub async fn save_exported_session_files( let mut written: Vec = Vec::with_capacity(items.len()); for item in items { - let resolved = resolve_export_filename(&folder_path, &item.filename, &used); - let path = folder_path.join(&resolved); - std::fs::write(&path, &item.contents) - .map_err(|e| format!("Failed to write file '{}': {}", path.display(), e))?; - used.insert(resolved.clone()); + let resolved = write_export_file( + &folder_path, + &item.filename, + item.contents.as_bytes(), + &mut used, + )?; written.push(resolved); } @@ -496,24 +497,122 @@ pub async fn save_exported_session_files( })) } -fn resolve_export_filename(folder: &Path, filename: &str, used: &HashSet) -> String { - if !folder.join(filename).exists() && !used.contains(filename) { - return filename.to_string(); +/// Windows reserved device names. A file name is unsafe if its stem (the part +/// before the first `.`) matches one of these case-insensitively, with or +/// without an extension (e.g. `CON`, `con.json`). +const WINDOWS_RESERVED_FILE_NAMES: &[&str] = &[ + "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", + "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", +]; + +/// Validate a renderer-supplied export filename and return a safe leaf name. +/// +/// `SessionExportItem.filename` is renderer-controlled, so this is the IPC +/// security boundary — frontend cleanup does not count. Only a bare leaf file +/// name may be written, so the result always stays directly inside the +/// user-selected directory. Rejected inputs, on every supported OS: +/// +/// - empty or whitespace-only names, +/// - the path separators `/`, `\`, and the Windows drive / alternate-data- +/// stream separator `:`, plus embedded NUL, +/// - anything that does not parse to exactly one `Component::Normal` (absolute +/// paths, drive/UNC prefixes, root, `.` and `..`), +/// - dot-only names (`.`, `..`, `...`), +/// - trailing `.` or space (silently stripped by Windows, which would change +/// the target), +/// - Windows reserved device names. +fn sanitize_export_leaf_filename(filename: &str) -> Result { + let reject = || format!("Unsafe export filename: {filename:?}"); + + if filename.trim().is_empty() { + return Err(reject()); + } + if filename.contains('\0') + || filename.contains('/') + || filename.contains('\\') + || filename.contains(':') + { + return Err(reject()); + } + + // Must be exactly one normal path component. This rejects absolute paths, + // drive/UNC prefixes, root, "." and "..". + let mut components = Path::new(filename).components(); + match (components.next(), components.next()) { + (Some(Component::Normal(part)), None) if part == std::ffi::OsStr::new(filename) => {} + _ => return Err(reject()), + } + + // Dot-only names (e.g. "...") are not useful and are treated specially by + // some filesystems. + if filename.chars().all(|c| c == '.') { + return Err(reject()); } - let (stem, ext) = match filename.rsplit_once('.') { - Some((s, e)) => (s.to_string(), format!(".{}", e)), - None => (filename.to_string(), String::new()), + // Windows strips trailing dots and spaces, which can change the target. + if filename.ends_with('.') || filename.ends_with(' ') { + return Err(reject()); + } + + let stem = filename.split('.').next().unwrap_or(filename); + if WINDOWS_RESERVED_FILE_NAMES + .iter() + .any(|reserved| stem.eq_ignore_ascii_case(reserved)) + { + return Err(reject()); + } + + Ok(filename.to_string()) +} + +/// Write one exported session file into `folder` under a validated leaf name, +/// never following symlinks and never overwriting an existing entry. +/// +/// Collisions get a deterministic `-N` suffix. `create_new(true)` guarantees +/// each write lands on a freshly created file directly inside `folder`: a +/// pre-existing file, directory, or symlink (including a dangling one) at the +/// target fails with `AlreadyExists` and is skipped rather than followed or +/// overwritten. Combined with the leaf-only filename, this confines every +/// write beneath the selected directory and closes the symlink/TOCTOU escape. +fn write_export_file( + folder: &Path, + filename: &str, + contents: &[u8], + used: &mut HashSet, +) -> Result { + let leaf = sanitize_export_leaf_filename(filename)?; + + let (stem, ext) = match leaf.rsplit_once('.') { + Some((s, e)) => (s.to_string(), format!(".{e}")), + None => (leaf.clone(), String::new()), }; - for n in 2..=9999 { - let candidate = format!("{}-{}{}", stem, n, ext); - if !folder.join(&candidate).exists() && !used.contains(&candidate) { - return candidate; + for attempt in 1..=9999 { + let candidate = if attempt == 1 { + leaf.clone() + } else { + format!("{stem}-{attempt}{ext}") + }; + if used.contains(&candidate) { + continue; + } + match OpenOptions::new() + .create_new(true) + .write(true) + .open(folder.join(&candidate)) + { + Ok(mut file) => { + file.write_all(contents) + .map_err(|e| format!("Failed to write file '{candidate}': {e}"))?; + used.insert(candidate.clone()); + return Ok(candidate); + } + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => continue, + Err(e) => return Err(format!("Failed to write file '{candidate}': {e}")), } } - format!("{}-{}{}", stem, 9999, ext) + Err(format!("Too many name collisions for export file '{leaf}'")) } #[tauri::command] @@ -1887,9 +1986,9 @@ mod tests { get_or_build_file_mention_index_from_cache, inspect_attachment_path, inspect_attachment_paths, normalize_attachment_paths, normalize_roots, read_directory_entries, read_image_attachment, read_text_file, - search_file_mentions_blocking, validate_external_url, write_agent_image_atomically, - write_sibling_then_replace, FileMentionIndexCache, MAX_IMAGE_ATTACHMENT_BYTES, - MAX_TEXT_FILE_BYTES, + sanitize_export_leaf_filename, search_file_mentions_blocking, validate_external_url, + write_agent_image_atomically, write_export_file, write_sibling_then_replace, + FileMentionIndexCache, MAX_IMAGE_ATTACHMENT_BYTES, MAX_TEXT_FILE_BYTES, }; use base64::Engine; use std::fs; @@ -2881,4 +2980,165 @@ mod tests { assert!(validate_external_url("file:///etc/passwd").is_err()); assert!(validate_external_url("not a url").is_err()); } + + #[test] + fn export_filename_accepts_plain_leaf_names() { + assert_eq!( + sanitize_export_leaf_filename("session.json").unwrap(), + "session.json" + ); + assert_eq!( + sanitize_export_leaf_filename("My Chat - 2026.json").unwrap(), + "My Chat - 2026.json" + ); + // No extension is fine. + assert_eq!(sanitize_export_leaf_filename("notes").unwrap(), "notes"); + // A leading dot (dotfile) is allowed as long as it is not dot-only. + assert_eq!( + sanitize_export_leaf_filename(".hidden.json").unwrap(), + ".hidden.json" + ); + } + + #[test] + fn export_filename_rejects_empty_and_whitespace() { + assert!(sanitize_export_leaf_filename("").is_err()); + assert!(sanitize_export_leaf_filename(" ").is_err()); + assert!(sanitize_export_leaf_filename("\t\n").is_err()); + } + + #[test] + fn export_filename_rejects_parent_and_current_dir() { + assert!(sanitize_export_leaf_filename(".").is_err()); + assert!(sanitize_export_leaf_filename("..").is_err()); + assert!(sanitize_export_leaf_filename("...").is_err()); + assert!(sanitize_export_leaf_filename("../secret.json").is_err()); + assert!(sanitize_export_leaf_filename("../../etc/passwd").is_err()); + assert!(sanitize_export_leaf_filename("foo/../bar.json").is_err()); + } + + #[test] + fn export_filename_rejects_separators_on_every_os() { + // Unix separator. + assert!(sanitize_export_leaf_filename("sub/dir.json").is_err()); + assert!(sanitize_export_leaf_filename("/etc/passwd").is_err()); + // Windows separator, rejected on all platforms. + assert!(sanitize_export_leaf_filename("sub\\dir.json").is_err()); + assert!(sanitize_export_leaf_filename("\\\\server\\share\\x").is_err()); + // Mixed separators. + assert!(sanitize_export_leaf_filename("a/b\\c.json").is_err()); + // Windows drive / alternate-data-stream separator. + assert!(sanitize_export_leaf_filename("C:\\Windows\\x.json").is_err()); + assert!(sanitize_export_leaf_filename("file.json:stream").is_err()); + // Embedded NUL. + assert!(sanitize_export_leaf_filename("a\0b.json").is_err()); + } + + #[test] + fn export_filename_rejects_windows_unsafe_names() { + // Reserved device names, with and without extension, any case. + assert!(sanitize_export_leaf_filename("CON").is_err()); + assert!(sanitize_export_leaf_filename("con.json").is_err()); + assert!(sanitize_export_leaf_filename("NUL.txt").is_err()); + assert!(sanitize_export_leaf_filename("Com1").is_err()); + assert!(sanitize_export_leaf_filename("lpt9.json").is_err()); + // Trailing dot or space are stripped by Windows. + assert!(sanitize_export_leaf_filename("session.json.").is_err()); + assert!(sanitize_export_leaf_filename("session ").is_err()); + // Not reserved: names that merely start with a reserved token. + assert_eq!( + sanitize_export_leaf_filename("console.json").unwrap(), + "console.json" + ); + } + + #[test] + fn write_export_file_writes_valid_export_into_folder() { + let dir = tempdir().expect("tempdir"); + let mut used = std::collections::HashSet::new(); + + let name = write_export_file(dir.path(), "session.json", b"hello", &mut used) + .expect("valid export"); + assert_eq!(name, "session.json"); + assert_eq!(fs::read(dir.path().join("session.json")).unwrap(), b"hello"); + } + + #[test] + fn write_export_file_rejects_traversal_and_absolute_paths() { + let dir = tempdir().expect("tempdir"); + let mut used = std::collections::HashSet::new(); + + assert!(write_export_file(dir.path(), "../escape.json", b"x", &mut used).is_err()); + assert!(write_export_file(dir.path(), "/etc/passwd", b"x", &mut used).is_err()); + assert!(write_export_file(dir.path(), "sub\\win.json", b"x", &mut used).is_err()); + // Nothing was created inside the folder, and no escape file exists. + assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 0); + assert!(!dir.path().parent().unwrap().join("escape.json").exists()); + } + + #[test] + fn write_export_file_suffixes_disk_collisions_deterministically() { + let dir = tempdir().expect("tempdir"); + fs::write(dir.path().join("chat.json"), b"pre-existing").unwrap(); + let mut used = std::collections::HashSet::new(); + + let name = write_export_file(dir.path(), "chat.json", b"new", &mut used) + .expect("collision suffix"); + assert_eq!(name, "chat-2.json"); + // The pre-existing file is untouched. + assert_eq!( + fs::read(dir.path().join("chat.json")).unwrap(), + b"pre-existing" + ); + assert_eq!(fs::read(dir.path().join("chat-2.json")).unwrap(), b"new"); + } + + #[test] + fn write_export_file_suffixes_duplicate_names_within_one_batch() { + let dir = tempdir().expect("tempdir"); + let mut used = std::collections::HashSet::new(); + + let first = + write_export_file(dir.path(), "chat.json", b"a", &mut used).expect("first duplicate"); + let second = + write_export_file(dir.path(), "chat.json", b"b", &mut used).expect("second duplicate"); + let third = + write_export_file(dir.path(), "chat.json", b"c", &mut used).expect("third duplicate"); + + assert_eq!(first, "chat.json"); + assert_eq!(second, "chat-2.json"); + assert_eq!(third, "chat-3.json"); + assert_eq!(fs::read(dir.path().join("chat.json")).unwrap(), b"a"); + assert_eq!(fs::read(dir.path().join("chat-2.json")).unwrap(), b"b"); + assert_eq!(fs::read(dir.path().join("chat-3.json")).unwrap(), b"c"); + } + + #[cfg(unix)] + #[test] + fn write_export_file_does_not_follow_symlink_escape() { + use std::os::unix::fs::symlink; + + let root = tempdir().expect("tempdir"); + let export_dir = root.path().join("export"); + let outside_dir = root.path().join("outside"); + fs::create_dir(&export_dir).unwrap(); + fs::create_dir(&outside_dir).unwrap(); + + // Attacker pre-plants a symlink in the export dir that points outside. + let target = outside_dir.join("victim.json"); + symlink(&target, export_dir.join("chat.json")).unwrap(); + + let mut used = std::collections::HashSet::new(); + let name = write_export_file(&export_dir, "chat.json", b"payload", &mut used) + .expect("symlink collision is skipped"); + + // The symlinked target outside the folder must never be written. + assert!(!target.exists()); + // The write lands on a fresh, suffixed name inside the folder. + assert_eq!(name, "chat-2.json"); + assert_eq!( + fs::read(export_dir.join("chat-2.json")).unwrap(), + b"payload" + ); + } } From 5a277f6ba0d1c99b7781cd16c68c6c65eaf2287e Mon Sep 17 00:00:00 2001 From: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Date: Fri, 14 Aug 2026 17:17:14 -0700 Subject: [PATCH 2/5] fix(system): pin export directory handle to close parent-swap TOCTOU The prior leaf-only + create_new fix still re-resolved folder.join(candidate) through the ambient filesystem namespace on every write. An attacker who renamed the selected directory and dropped a directory symlink/reparse point in its place after the picker returned could redirect the parent and land exports outside the selected directory (P1, reproduced on macOS). Open a cap_std::fs::Dir handle to the selected directory once, immediately after the picker returns, and perform all create_new writes relative to that handle inside cap-std's sandbox. Writes stay attached to the originally selected inode and can never resolve through a swapped parent. Add a unix regression that renames the selected directory and replaces its path with a symlink after the handle is acquired, asserting the write lands on the original inode and never in the redirect target. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Signed-off-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> --- src-tauri/Cargo.lock | 96 ++++++++++++++++++++++++ src-tauri/Cargo.toml | 1 + src-tauri/src/commands/system.rs | 125 +++++++++++++++++++++++-------- 3 files changed, 192 insertions(+), 30 deletions(-) diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 4d2ee9298..07564e6f5 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -14,6 +14,7 @@ dependencies = [ "buzz-voice", "bytes", "bzip2 0.6.1", + "cap-std", "chrono", "dirs", "doctor", @@ -146,6 +147,12 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "ambient-authority" +version = "0.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" + [[package]] name = "android_log-sys" version = "0.3.2" @@ -815,6 +822,36 @@ dependencies = [ "serde_core", ] +[[package]] +name = "cap-primitives" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdadbd7c002d3a484b35243669abdae85a0ebaded5a61117169dc3400f9a7ff0" +dependencies = [ + "ambient-authority", + "fs-set-times", + "io-extras", + "io-lifetimes 3.0.1", + "ipnet", + "maybe-owned", + "rustix", + "rustix-linux-procfs", + "windows-sys 0.61.2", + "winx", +] + +[[package]] +name = "cap-std" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7281235d6e96d3544ca18bba9049be92f4190f8d923e3caef1b5f66cfa752608" +dependencies = [ + "cap-primitives", + "io-extras", + "io-lifetimes 3.0.1", + "rustix", +] + [[package]] name = "cargo-platform" version = "0.1.9" @@ -2106,6 +2143,17 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs-set-times" +version = "0.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94e7099f6313ecacbe1256e8ff9d617b75d1bcb16a6fddef94866d225a01a14a" +dependencies = [ + "io-lifetimes 2.0.4", + "rustix", + "windows-sys 0.59.0", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -2991,6 +3039,28 @@ dependencies = [ "cfb", ] +[[package]] +name = "io-extras" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20fd6de4ccfcc187e38bc21cfa543cb5a302cb86a8b114eb7f0bf0dc9f8ac00f" +dependencies = [ + "io-lifetimes 3.0.1", + "windows-sys 0.60.2", +] + +[[package]] +name = "io-lifetimes" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06432fb54d3be7964ecd3649233cddf80db2832f47fec34c01f65b3d9d774983" + +[[package]] +name = "io-lifetimes" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f0fb0570afe1fed943c5c3d4102d5358592d8625fda6a0007fdbe65a92fba96" + [[package]] name = "ipnet" version = "2.12.1" @@ -3487,6 +3557,12 @@ dependencies = [ "rawpointer", ] +[[package]] +name = "maybe-owned" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4facc753ae494aeb6e3c22f839b158aebd4f9270f55cd3c79906c45476c47ab4" + [[package]] name = "md-5" version = "0.10.6" @@ -5306,6 +5382,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustix-linux-procfs" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fc84bf7e9aa16c4f2c758f27412dc9841341e16aa682d9c7ac308fe3ee12056" +dependencies = [ + "once_cell", + "rustix", +] + [[package]] name = "rustls" version = "0.23.43" @@ -8749,6 +8835,16 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "winx" +version = "0.36.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f3fd376f71958b862e7afb20cfe5a22830e1963462f3a17f49d82a6c1d1f42d" +dependencies = [ + "bitflags 2.13.1", + "windows-sys 0.59.0", +] + [[package]] name = "wit-bindgen" version = "0.57.1" diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index ac854cd54..92d543e04 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -78,6 +78,7 @@ tokio = { version = "1.50.0", features = ["full"] } url = "2" uuid = { version = "1", features = ["v4", "serde"] } zip = { version = "2", default-features = false, features = ["deflate"] } +cap-std = "4.0.2" [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.59", features = [ diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index eedb81616..8e59518b8 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -478,16 +478,28 @@ pub async fn save_exported_session_files( .into_path() .map_err(|_| "Selected folder path is not available".to_string())?; + // Pin the selected directory by an open handle immediately after the + // picker returns. Every subsequent write is performed relative to this + // handle (see `write_export_file`), so an attacker who renames the + // selected directory and drops a symlink/reparse point in its place + // afterwards cannot redirect writes: the handle still refers to the + // originally selected inode. Re-resolving `folder_path` per write would + // reopen the ambient path and reintroduce that TOCTOU escape. + let dir = cap_std::fs::Dir::open_ambient_dir(&folder_path, cap_std::ambient_authority()) + .map_err(|e| { + format!( + "Failed to open selected folder '{}': {}", + folder_path.display(), + e + ) + })?; + let mut used: HashSet = HashSet::new(); let mut written: Vec = Vec::with_capacity(items.len()); for item in items { - let resolved = write_export_file( - &folder_path, - &item.filename, - item.contents.as_bytes(), - &mut used, - )?; + let resolved = + write_export_file(&dir, &item.filename, item.contents.as_bytes(), &mut used)?; written.push(resolved); } @@ -565,17 +577,23 @@ fn sanitize_export_leaf_filename(filename: &str) -> Result { Ok(filename.to_string()) } -/// Write one exported session file into `folder` under a validated leaf name, -/// never following symlinks and never overwriting an existing entry. +/// Write one exported session file beneath the pinned directory handle `dir` +/// under a validated leaf name, never following symlinks and never overwriting +/// an existing entry. +/// +/// `dir` is a `cap_std::fs::Dir` opened once when the picker returned. All +/// operations here are performed relative to that handle inside cap-std's +/// sandbox, so a candidate can never resolve outside the originally selected +/// directory even if its path is swapped for a symlink/reparse point after +/// selection — this closes the parent-directory TOCTOU escape that a plain +/// `folder.join(candidate)` re-resolution would reopen. /// /// Collisions get a deterministic `-N` suffix. `create_new(true)` guarantees -/// each write lands on a freshly created file directly inside `folder`: a -/// pre-existing file, directory, or symlink (including a dangling one) at the -/// target fails with `AlreadyExists` and is skipped rather than followed or -/// overwritten. Combined with the leaf-only filename, this confines every -/// write beneath the selected directory and closes the symlink/TOCTOU escape. +/// each write lands on a freshly created file: a pre-existing file, directory, +/// or symlink (including a dangling one) at the target fails with +/// `AlreadyExists` and is skipped rather than followed or overwritten. fn write_export_file( - folder: &Path, + dir: &cap_std::fs::Dir, filename: &str, contents: &[u8], used: &mut HashSet, @@ -596,11 +614,10 @@ fn write_export_file( if used.contains(&candidate) { continue; } - match OpenOptions::new() - .create_new(true) - .write(true) - .open(folder.join(&candidate)) - { + match dir.open_with( + &candidate, + cap_std::fs::OpenOptions::new().create_new(true).write(true), + ) { Ok(mut file) => { file.write_all(contents) .map_err(|e| format!("Failed to write file '{candidate}': {e}"))?; @@ -3055,10 +3072,12 @@ mod tests { #[test] fn write_export_file_writes_valid_export_into_folder() { let dir = tempdir().expect("tempdir"); + let handle = + cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - let name = write_export_file(dir.path(), "session.json", b"hello", &mut used) - .expect("valid export"); + let name = + write_export_file(&handle, "session.json", b"hello", &mut used).expect("valid export"); assert_eq!(name, "session.json"); assert_eq!(fs::read(dir.path().join("session.json")).unwrap(), b"hello"); } @@ -3066,11 +3085,13 @@ mod tests { #[test] fn write_export_file_rejects_traversal_and_absolute_paths() { let dir = tempdir().expect("tempdir"); + let handle = + cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - assert!(write_export_file(dir.path(), "../escape.json", b"x", &mut used).is_err()); - assert!(write_export_file(dir.path(), "/etc/passwd", b"x", &mut used).is_err()); - assert!(write_export_file(dir.path(), "sub\\win.json", b"x", &mut used).is_err()); + assert!(write_export_file(&handle, "../escape.json", b"x", &mut used).is_err()); + assert!(write_export_file(&handle, "/etc/passwd", b"x", &mut used).is_err()); + assert!(write_export_file(&handle, "sub\\win.json", b"x", &mut used).is_err()); // Nothing was created inside the folder, and no escape file exists. assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 0); assert!(!dir.path().parent().unwrap().join("escape.json").exists()); @@ -3080,10 +3101,12 @@ mod tests { fn write_export_file_suffixes_disk_collisions_deterministically() { let dir = tempdir().expect("tempdir"); fs::write(dir.path().join("chat.json"), b"pre-existing").unwrap(); + let handle = + cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - let name = write_export_file(dir.path(), "chat.json", b"new", &mut used) - .expect("collision suffix"); + let name = + write_export_file(&handle, "chat.json", b"new", &mut used).expect("collision suffix"); assert_eq!(name, "chat-2.json"); // The pre-existing file is untouched. assert_eq!( @@ -3096,14 +3119,16 @@ mod tests { #[test] fn write_export_file_suffixes_duplicate_names_within_one_batch() { let dir = tempdir().expect("tempdir"); + let handle = + cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); let first = - write_export_file(dir.path(), "chat.json", b"a", &mut used).expect("first duplicate"); + write_export_file(&handle, "chat.json", b"a", &mut used).expect("first duplicate"); let second = - write_export_file(dir.path(), "chat.json", b"b", &mut used).expect("second duplicate"); + write_export_file(&handle, "chat.json", b"b", &mut used).expect("second duplicate"); let third = - write_export_file(dir.path(), "chat.json", b"c", &mut used).expect("third duplicate"); + write_export_file(&handle, "chat.json", b"c", &mut used).expect("third duplicate"); assert_eq!(first, "chat.json"); assert_eq!(second, "chat-2.json"); @@ -3128,8 +3153,10 @@ mod tests { let target = outside_dir.join("victim.json"); symlink(&target, export_dir.join("chat.json")).unwrap(); + let handle = + cap_std::fs::Dir::open_ambient_dir(&export_dir, cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - let name = write_export_file(&export_dir, "chat.json", b"payload", &mut used) + let name = write_export_file(&handle, "chat.json", b"payload", &mut used) .expect("symlink collision is skipped"); // The symlinked target outside the folder must never be written. @@ -3141,4 +3168,42 @@ mod tests { b"payload" ); } + + /// Regression for the parent-directory TOCTOU: after the picker returns and + /// the directory handle is opened, an attacker renames the selected + /// directory and replaces its path with a symlink pointing outside. Writes + /// must stay attached to the originally selected inode (via the pinned + /// handle) and must never land in the attacker-controlled location. + #[cfg(unix)] + #[test] + fn write_export_file_stays_attached_to_pinned_dir_after_parent_swap() { + use std::os::unix::fs::symlink; + + let root = tempdir().expect("tempdir"); + let selected = root.path().join("selected"); + let outside = root.path().join("outside"); + fs::create_dir(&selected).unwrap(); + fs::create_dir(&outside).unwrap(); + + // Handle is pinned to the originally selected directory. + let handle = + cap_std::fs::Dir::open_ambient_dir(&selected, cap_std::ambient_authority()).unwrap(); + + // Attacker swaps the selected path for a symlink to `outside`. + fs::rename(&selected, root.path().join("selected-old")).unwrap(); + symlink(&outside, &selected).unwrap(); + + let mut used = std::collections::HashSet::new(); + let name = write_export_file(&handle, "chat.json", b"payload", &mut used) + .expect("write via pinned handle"); + assert_eq!(name, "chat.json"); + + // The write landed on the original inode (now `selected-old`), not the + // attacker's redirect target. + assert_eq!( + fs::read(root.path().join("selected-old/chat.json")).unwrap(), + b"payload" + ); + assert!(!outside.join("chat.json").exists()); + } } From 12b1ee65081813ea705f14e4bc00a7e23f3a9222 Mon Sep 17 00:00:00 2001 From: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Date: Fri, 14 Aug 2026 17:30:30 -0700 Subject: [PATCH 3/5] fix(system): fail closed on symlinked selection and reject full windows unsafe name set Two review blockers on the export path confinement: P1 (picker->open race): Tauri's folder picker returns only a path (FilePath -> PathBuf), never an OS handle or security-scoped object, so open_ambient_dir re-resolved that path and could attach the handle to a symlink/reparse point swapped in during the picker->open window. Open the selection no-follow instead (O_NOFOLLOW|O_DIRECTORY on unix; FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OPEN_REPARSE_POINT on windows), then wrap the handle in cap-std. A swap of the selected directory itself now fails closed. A swap of an ancestor above the selection cannot be closed from a path-only picker API; this is documented, not claimed closed. P2 (incomplete filename validation): the validator accepted names Windows rejects, so a batch could partially succeed then abort mid-write. Reject the Windows-forbidden punctuation < > " | ? * and all control characters on every OS, add the superscript-digit device forms (COM/LPT superscript 1-3) and COM0/LPT0, and right-trim trailing spaces on the base name before the device-name check so 'CON .json' is caught. Windows rules are enforced on all platforms so validation is deterministic and CI-independent. Adds unix regressions for no-follow acquisition (symlinked selection fails closed; real directory accepted) and platform-independent validator cases for the forbidden-char/control and reserved-name additions. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Signed-off-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> --- src-tauri/src/commands/system.rs | 212 ++++++++++++++++++++++++++----- 1 file changed, 182 insertions(+), 30 deletions(-) diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 8e59518b8..29f085e7d 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -479,20 +479,20 @@ pub async fn save_exported_session_files( .map_err(|_| "Selected folder path is not available".to_string())?; // Pin the selected directory by an open handle immediately after the - // picker returns. Every subsequent write is performed relative to this - // handle (see `write_export_file`), so an attacker who renames the - // selected directory and drops a symlink/reparse point in its place - // afterwards cannot redirect writes: the handle still refers to the - // originally selected inode. Re-resolving `folder_path` per write would - // reopen the ambient path and reintroduce that TOCTOU escape. - let dir = cap_std::fs::Dir::open_ambient_dir(&folder_path, cap_std::ambient_authority()) - .map_err(|e| { - format!( - "Failed to open selected folder '{}': {}", - folder_path.display(), - e - ) - })?; + // picker returns, and open it *without following symlinks* on the final + // component. Tauri's folder picker returns only a path (`FilePath` -> + // `PathBuf`), not an OS handle or security-scoped object, so a returned + // path alone cannot prove the identity of the picked directory. The + // narrow residual is the picker->open window: an attacker who renames the + // selected directory and drops a symlink/reparse point in its place before + // this open runs. Opening no-follow makes that swap *fail closed* (the open + // errors) rather than silently attaching the handle to a redirect target. + // Every subsequent write is then handle-relative (see `write_export_file`), + // so once acquired the handle cannot be redirected. This does not defend + // against a swap of an ancestor *above* the selected directory during the + // same window — that cannot be closed from a path-only picker API and would + // require an identity-bearing handle from the selection boundary itself. + let dir = open_selected_export_dir(&folder_path)?; let mut used: HashSet = HashSet::new(); let mut written: Vec = Vec::with_capacity(items.len()); @@ -509,14 +509,74 @@ pub async fn save_exported_session_files( })) } -/// Windows reserved device names. A file name is unsafe if its stem (the part -/// before the first `.`) matches one of these case-insensitively, with or -/// without an extension (e.g. `CON`, `con.json`). -const WINDOWS_RESERVED_FILE_NAMES: &[&str] = &[ - "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", - "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", +/// Open the user-selected export directory into a cap-std `Dir` handle without +/// following a symlink/reparse point on the final path component. +/// +/// This closes the picker->open swap of the selected directory itself: if the +/// selection path has been replaced by a symlink/reparse point, the open fails +/// instead of attaching to the redirect target. Once the handle is held, all +/// writes are performed relative to it inside cap-std's sandbox. +fn open_selected_export_dir(folder_path: &Path) -> Result { + let map_err = |e: io::Error| { + format!( + "Failed to open selected folder '{}': {}", + folder_path.display(), + e + ) + }; + + #[cfg(unix)] + let std_dir = { + use std::os::unix::fs::OpenOptionsExt; + fs::OpenOptions::new() + .read(true) + // O_NOFOLLOW: fail if the final component is a symlink. + // O_DIRECTORY: fail if it is not a directory. + .custom_flags(libc::O_NOFOLLOW | libc::O_DIRECTORY) + .open(folder_path) + .map_err(map_err)? + }; + + #[cfg(windows)] + let std_dir = { + use std::os::windows::fs::OpenOptionsExt; + // FILE_FLAG_BACKUP_SEMANTICS is required to obtain a handle to a + // directory; FILE_FLAG_OPEN_REPARSE_POINT opens the reparse point + // itself rather than following it, so a swapped symlink/junction fails + // the subsequent directory-handle use instead of redirecting. + const FILE_FLAG_BACKUP_SEMANTICS: u32 = 0x0200_0000; + const FILE_FLAG_OPEN_REPARSE_POINT: u32 = 0x0020_0000; + fs::OpenOptions::new() + .read(true) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT) + .open(folder_path) + .map_err(map_err)? + }; + + #[cfg(not(any(unix, windows)))] + let std_dir = fs::File::open(folder_path).map_err(map_err)?; + + Ok(cap_std::fs::Dir::from_std_file(std_dir)) +} + +/// Windows reserved device names. A file name is unsafe if its base name (the +/// part before the first `.`, with trailing spaces stripped as Windows does) +/// matches one of these case-insensitively, with or without an extension +/// (e.g. `CON`, `con.json`, `CON .json`). Includes the superscript-digit forms +/// Windows also reserves. See +/// . +const WINDOWS_RESERVED_DEVICE_NAMES: &[&str] = &[ + "CON", "PRN", "AUX", "NUL", "COM0", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", + "COM8", "COM9", "COM¹", "COM²", "COM³", "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", + "LPT7", "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³", ]; +/// Characters Windows forbids in a file name, rejected on every OS so a batch +/// validates identically everywhere and cannot partially succeed then abort on +/// a name only Windows refuses. The path separators `/`, `\`, and the drive / +/// alternate-data-stream separator `:` are handled separately. +const WINDOWS_FORBIDDEN_CHARS: &[char] = &['<', '>', '"', '|', '?', '*']; + /// Validate a renderer-supplied export filename and return a safe leaf name. /// /// `SessionExportItem.filename` is renderer-controlled, so this is the IPC @@ -527,12 +587,18 @@ const WINDOWS_RESERVED_FILE_NAMES: &[&str] = &[ /// - empty or whitespace-only names, /// - the path separators `/`, `\`, and the Windows drive / alternate-data- /// stream separator `:`, plus embedded NUL, +/// - ASCII/Unicode control characters and the Windows-reserved punctuation +/// `< > " | ? *`, /// - anything that does not parse to exactly one `Component::Normal` (absolute /// paths, drive/UNC prefixes, root, `.` and `..`), /// - dot-only names (`.`, `..`, `...`), /// - trailing `.` or space (silently stripped by Windows, which would change /// the target), -/// - Windows reserved device names. +/// - Windows reserved device names (incl. superscript-digit forms), matched on +/// the base name before the first `.` with trailing spaces stripped. +/// +/// Windows rules are enforced on all platforms so validation does not depend on +/// the host filesystem and CI is deterministic. fn sanitize_export_leaf_filename(filename: &str) -> Result { let reject = || format!("Unsafe export filename: {filename:?}"); @@ -547,6 +613,15 @@ fn sanitize_export_leaf_filename(filename: &str) -> Result { return Err(reject()); } + // Control characters and the Windows-reserved punctuation set are illegal + // in file names on Windows; reject everywhere for identical validation. + if filename + .chars() + .any(|c| c.is_control() || WINDOWS_FORBIDDEN_CHARS.contains(&c)) + { + return Err(reject()); + } + // Must be exactly one normal path component. This rejects absolute paths, // drive/UNC prefixes, root, "." and "..". let mut components = Path::new(filename).components(); @@ -566,10 +641,13 @@ fn sanitize_export_leaf_filename(filename: &str) -> Result { return Err(reject()); } - let stem = filename.split('.').next().unwrap_or(filename); - if WINDOWS_RESERVED_FILE_NAMES + // Reserved device names apply to the base name before the first '.', after + // trailing spaces are stripped (Windows ignores them): "CON", "con.json", + // and "CON .json" all resolve to the CON device. + let base = filename.split('.').next().unwrap_or(filename).trim_end(); + if WINDOWS_RESERVED_DEVICE_NAMES .iter() - .any(|reserved| stem.eq_ignore_ascii_case(reserved)) + .any(|reserved| base.eq_ignore_ascii_case(reserved)) { return Err(reject()); } @@ -2002,7 +2080,7 @@ mod tests { build_file_mention_index, build_file_tree_entry, ensure_directory_path, get_or_build_file_mention_index_from_cache, inspect_attachment_path, inspect_attachment_paths, normalize_attachment_paths, normalize_roots, - read_directory_entries, read_image_attachment, read_text_file, + open_selected_export_dir, read_directory_entries, read_image_attachment, read_text_file, sanitize_export_leaf_filename, search_file_mentions_blocking, validate_external_url, write_agent_image_atomically, write_export_file, write_sibling_then_replace, FileMentionIndexCache, MAX_IMAGE_ATTACHMENT_BYTES, MAX_TEXT_FILE_BYTES, @@ -3059,6 +3137,14 @@ mod tests { assert!(sanitize_export_leaf_filename("NUL.txt").is_err()); assert!(sanitize_export_leaf_filename("Com1").is_err()); assert!(sanitize_export_leaf_filename("lpt9.json").is_err()); + assert!(sanitize_export_leaf_filename("COM0").is_err()); + assert!(sanitize_export_leaf_filename("LPT0.txt").is_err()); + // Superscript-digit device forms Windows also reserves. + assert!(sanitize_export_leaf_filename("COM¹").is_err()); + assert!(sanitize_export_leaf_filename("COM².json").is_err()); + assert!(sanitize_export_leaf_filename("lpt³.txt").is_err()); + // Trailing spaces before the extension still resolve to the device. + assert!(sanitize_export_leaf_filename("CON .json").is_err()); // Trailing dot or space are stripped by Windows. assert!(sanitize_export_leaf_filename("session.json.").is_err()); assert!(sanitize_export_leaf_filename("session ").is_err()); @@ -3069,6 +3155,34 @@ mod tests { ); } + #[test] + fn export_filename_rejects_windows_forbidden_chars_and_controls() { + // Windows-reserved punctuation, rejected on every OS. + for name in [ + "bad?.json", + "bad*.json", + "bad|name.json", + "badname.json", + "bad\"name.json", + ] { + assert!( + sanitize_export_leaf_filename(name).is_err(), + "expected reject: {name:?}" + ); + } + // ASCII control characters (1..=31), e.g. tab, newline, and a raw + // control byte. + assert!(sanitize_export_leaf_filename("bad\tname.json").is_err()); + assert!(sanitize_export_leaf_filename("bad\nname.json").is_err()); + assert!(sanitize_export_leaf_filename("bad\u{001f}name.json").is_err()); + // A comparable safe name with none of the above is accepted. + assert_eq!( + sanitize_export_leaf_filename("badname.json").unwrap(), + "badname.json" + ); + } + #[test] fn write_export_file_writes_valid_export_into_folder() { let dir = tempdir().expect("tempdir"); @@ -3169,8 +3283,8 @@ mod tests { ); } - /// Regression for the parent-directory TOCTOU: after the picker returns and - /// the directory handle is opened, an attacker renames the selected + /// Regression for the parent-directory TOCTOU: after the handle to the + /// selected directory is acquired, an attacker renames the selected /// directory and replaces its path with a symlink pointing outside. Writes /// must stay attached to the originally selected inode (via the pinned /// handle) and must never land in the attacker-controlled location. @@ -3185,9 +3299,9 @@ mod tests { fs::create_dir(&selected).unwrap(); fs::create_dir(&outside).unwrap(); - // Handle is pinned to the originally selected directory. - let handle = - cap_std::fs::Dir::open_ambient_dir(&selected, cap_std::ambient_authority()).unwrap(); + // Handle is acquired via the real acquisition path, pinned to the + // originally selected directory. + let handle = open_selected_export_dir(&selected).expect("acquire handle"); // Attacker swaps the selected path for a symlink to `outside`. fs::rename(&selected, root.path().join("selected-old")).unwrap(); @@ -3206,4 +3320,42 @@ mod tests { ); assert!(!outside.join("chat.json").exists()); } + + /// Acquisition fails closed: if the selected path is (or is swapped to) a + /// symlink before the handle is opened, `open_selected_export_dir` errors + /// rather than attaching the handle to the symlink's redirect target. This + /// bounds the picker->open race for a swap of the selected directory + /// itself. + #[cfg(unix)] + #[test] + fn open_selected_export_dir_rejects_symlinked_selection() { + use std::os::unix::fs::symlink; + + let root = tempdir().expect("tempdir"); + let outside = root.path().join("outside"); + fs::create_dir(&outside).unwrap(); + let selected = root.path().join("selected"); + // The selection path is a symlink to another directory. + symlink(&outside, &selected).unwrap(); + + let err = + open_selected_export_dir(&selected).expect_err("symlinked selection must fail closed"); + assert!(err.contains("Failed to open selected folder")); + // Nothing was written through the redirect. + assert_eq!(fs::read_dir(&outside).unwrap().count(), 0); + } + + /// A real (non-symlink) selected directory is accepted by the acquisition + /// path and writes land inside it. + #[test] + fn open_selected_export_dir_accepts_real_directory() { + let dir = tempdir().expect("tempdir"); + let handle = open_selected_export_dir(dir.path()).expect("acquire handle"); + let mut used = std::collections::HashSet::new(); + + let name = + write_export_file(&handle, "session.json", b"hi", &mut used).expect("valid export"); + assert_eq!(name, "session.json"); + assert_eq!(fs::read(dir.path().join("session.json")).unwrap(), b"hi"); + } } From 014833cc9422e7db0bfa4a3d76f41b6590f896c2 Mon Sep 17 00:00:00 2001 From: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Date: Fri, 14 Aug 2026 17:52:11 -0700 Subject: [PATCH 4/5] fix(system): set windows dir share mode and validate export batch before writing Two review blockers on the export path confinement. P1 (windows cap-std precondition): open_selected_export_dir's windows branch relied on Rust std's default share mode, which includes FILE_SHARE_DELETE. cap-std 4.0.2's Dir::from_std_file requires the handle be opened without FILE_SHARE_DELETE (src/fs/dir.rs) so the directory root cannot be renamed/deleted underneath the capability. Set .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE) explicitly on the windows branch, excluding DELETE. Source-verified; windows runtime still unverified locally (mingw cc build-script gap in a dependency, not in this code). P2 (partial export on malformed batch item): validation ran inside write_export_file during the mutation loop, so a batch like [session.json, ../escape.json] created the first file before rejecting the second, leaving an avoidable partial export. Introduce write_export_batch: phase 1 validates every filename via a ValidExportLeaf newtype before any file is created; phase 2 writes from the already-validated leaves. write_export_file now accepts only a ValidExportLeaf, so the validate-before-mutate ordering cannot be bypassed. Adds regressions: valid-first/malformed-later batch leaves the directory empty; traversal/absolute names are rejected at the batch boundary. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Signed-off-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> --- src-tauri/src/commands/system.rs | 193 +++++++++++++++++++++++++------ 1 file changed, 156 insertions(+), 37 deletions(-) diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 29f085e7d..9e19adf3a 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -494,14 +494,7 @@ pub async fn save_exported_session_files( // require an identity-bearing handle from the selection boundary itself. let dir = open_selected_export_dir(&folder_path)?; - let mut used: HashSet = HashSet::new(); - let mut written: Vec = Vec::with_capacity(items.len()); - - for item in items { - let resolved = - write_export_file(&dir, &item.filename, item.contents.as_bytes(), &mut used)?; - written.push(resolved); - } + let written = write_export_batch(&dir, &items)?; Ok(Some(SessionExportBatchResult { folder: folder_path.to_string_lossy().into_owned(), @@ -509,6 +502,38 @@ pub async fn save_exported_session_files( })) } +/// Write a whole renderer-controlled export batch beneath the pinned directory +/// handle in two phases. +/// +/// Phase 1 validates *every* filename before any file is created. A batch such +/// as `[session.json, ../escape.json]` is rejected in full — the malformed +/// second item aborts the command before the valid first item is written, so a +/// rejected batch never leaves a partial export on disk. Phase 2 then creates +/// the files from the already-validated leaf names. The `ValidExportLeaf` +/// newtype makes this ordering unbypassable: `write_export_file` only accepts a +/// name that has already cleared `sanitize_export_leaf_filename`. +fn write_export_batch( + dir: &cap_std::fs::Dir, + items: &[SessionExportItem], +) -> Result, String> { + let leaves = items + .iter() + .map(|item| ValidExportLeaf::parse(&item.filename)) + .collect::, _>>()?; + + let mut used: HashSet = HashSet::new(); + let mut written: Vec = Vec::with_capacity(items.len()); + for (leaf, item) in leaves.iter().zip(items) { + written.push(write_export_file( + dir, + leaf, + item.contents.as_bytes(), + &mut used, + )?); + } + Ok(written) +} + /// Open the user-selected export directory into a cap-std `Dir` handle without /// following a symlink/reparse point on the final path component. /// @@ -546,8 +571,16 @@ fn open_selected_export_dir(folder_path: &Path) -> Result', '"', '|', '?', '*']; +/// A renderer-supplied export filename that has been validated as a safe bare +/// leaf name. Constructing one is the only way to obtain a name +/// `write_export_file` will write, so the batch cannot mutate the filesystem +/// with an unvalidated name. +struct ValidExportLeaf(String); + +impl ValidExportLeaf { + fn parse(filename: &str) -> Result { + Ok(Self(sanitize_export_leaf_filename(filename)?)) + } + + fn as_str(&self) -> &str { + &self.0 + } +} + /// Validate a renderer-supplied export filename and return a safe leaf name. /// /// `SessionExportItem.filename` is renderer-controlled, so this is the IPC @@ -672,20 +721,20 @@ fn sanitize_export_leaf_filename(filename: &str) -> Result { /// `AlreadyExists` and is skipped rather than followed or overwritten. fn write_export_file( dir: &cap_std::fs::Dir, - filename: &str, + leaf: &ValidExportLeaf, contents: &[u8], used: &mut HashSet, ) -> Result { - let leaf = sanitize_export_leaf_filename(filename)?; + let leaf = leaf.as_str(); let (stem, ext) = match leaf.rsplit_once('.') { Some((s, e)) => (s.to_string(), format!(".{e}")), - None => (leaf.clone(), String::new()), + None => (leaf.to_string(), String::new()), }; for attempt in 1..=9999 { let candidate = if attempt == 1 { - leaf.clone() + leaf.to_string() } else { format!("{stem}-{attempt}{ext}") }; @@ -2082,8 +2131,9 @@ mod tests { inspect_attachment_paths, normalize_attachment_paths, normalize_roots, open_selected_export_dir, read_directory_entries, read_image_attachment, read_text_file, sanitize_export_leaf_filename, search_file_mentions_blocking, validate_external_url, - write_agent_image_atomically, write_export_file, write_sibling_then_replace, - FileMentionIndexCache, MAX_IMAGE_ATTACHMENT_BYTES, MAX_TEXT_FILE_BYTES, + write_agent_image_atomically, write_export_batch, write_export_file, + write_sibling_then_replace, FileMentionIndexCache, SessionExportItem, ValidExportLeaf, + MAX_IMAGE_ATTACHMENT_BYTES, MAX_TEXT_FILE_BYTES, }; use base64::Engine; use std::fs; @@ -3190,22 +3240,56 @@ mod tests { cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - let name = - write_export_file(&handle, "session.json", b"hello", &mut used).expect("valid export"); + let name = write_export_file( + &handle, + &ValidExportLeaf::parse("session.json").unwrap(), + b"hello", + &mut used, + ) + .expect("valid export"); assert_eq!(name, "session.json"); assert_eq!(fs::read(dir.path().join("session.json")).unwrap(), b"hello"); } #[test] - fn write_export_file_rejects_traversal_and_absolute_paths() { + fn write_export_batch_validates_all_names_before_writing_any() { let dir = tempdir().expect("tempdir"); - let handle = - cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); - let mut used = std::collections::HashSet::new(); + let handle = open_selected_export_dir(dir.path()).expect("acquire handle"); + + // A valid first item followed by a malformed later item must abort the + // whole batch before any file is created. + let items = vec![ + SessionExportItem { + filename: "session.json".to_string(), + contents: "hello".to_string(), + }, + SessionExportItem { + filename: "../escape.json".to_string(), + contents: "x".to_string(), + }, + ]; + + assert!(write_export_batch(&handle, &items).is_err()); + // No partial export: the valid first item was not written. + assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 0); + assert!(!dir.path().parent().unwrap().join("escape.json").exists()); + } + + #[test] + fn write_export_batch_rejects_traversal_and_absolute_names() { + let dir = tempdir().expect("tempdir"); + let handle = open_selected_export_dir(dir.path()).expect("acquire handle"); - assert!(write_export_file(&handle, "../escape.json", b"x", &mut used).is_err()); - assert!(write_export_file(&handle, "/etc/passwd", b"x", &mut used).is_err()); - assert!(write_export_file(&handle, "sub\\win.json", b"x", &mut used).is_err()); + for bad in ["../escape.json", "/etc/passwd", "sub\\win.json"] { + let items = vec![SessionExportItem { + filename: bad.to_string(), + contents: "x".to_string(), + }]; + assert!( + write_export_batch(&handle, &items).is_err(), + "expected reject: {bad:?}" + ); + } // Nothing was created inside the folder, and no escape file exists. assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 0); assert!(!dir.path().parent().unwrap().join("escape.json").exists()); @@ -3219,8 +3303,13 @@ mod tests { cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - let name = - write_export_file(&handle, "chat.json", b"new", &mut used).expect("collision suffix"); + let name = write_export_file( + &handle, + &ValidExportLeaf::parse("chat.json").unwrap(), + b"new", + &mut used, + ) + .expect("collision suffix"); assert_eq!(name, "chat-2.json"); // The pre-existing file is untouched. assert_eq!( @@ -3237,12 +3326,27 @@ mod tests { cap_std::fs::Dir::open_ambient_dir(dir.path(), cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - let first = - write_export_file(&handle, "chat.json", b"a", &mut used).expect("first duplicate"); - let second = - write_export_file(&handle, "chat.json", b"b", &mut used).expect("second duplicate"); - let third = - write_export_file(&handle, "chat.json", b"c", &mut used).expect("third duplicate"); + let first = write_export_file( + &handle, + &ValidExportLeaf::parse("chat.json").unwrap(), + b"a", + &mut used, + ) + .expect("first duplicate"); + let second = write_export_file( + &handle, + &ValidExportLeaf::parse("chat.json").unwrap(), + b"b", + &mut used, + ) + .expect("second duplicate"); + let third = write_export_file( + &handle, + &ValidExportLeaf::parse("chat.json").unwrap(), + b"c", + &mut used, + ) + .expect("third duplicate"); assert_eq!(first, "chat.json"); assert_eq!(second, "chat-2.json"); @@ -3270,8 +3374,13 @@ mod tests { let handle = cap_std::fs::Dir::open_ambient_dir(&export_dir, cap_std::ambient_authority()).unwrap(); let mut used = std::collections::HashSet::new(); - let name = write_export_file(&handle, "chat.json", b"payload", &mut used) - .expect("symlink collision is skipped"); + let name = write_export_file( + &handle, + &ValidExportLeaf::parse("chat.json").unwrap(), + b"payload", + &mut used, + ) + .expect("symlink collision is skipped"); // The symlinked target outside the folder must never be written. assert!(!target.exists()); @@ -3308,8 +3417,13 @@ mod tests { symlink(&outside, &selected).unwrap(); let mut used = std::collections::HashSet::new(); - let name = write_export_file(&handle, "chat.json", b"payload", &mut used) - .expect("write via pinned handle"); + let name = write_export_file( + &handle, + &ValidExportLeaf::parse("chat.json").unwrap(), + b"payload", + &mut used, + ) + .expect("write via pinned handle"); assert_eq!(name, "chat.json"); // The write landed on the original inode (now `selected-old`), not the @@ -3353,8 +3467,13 @@ mod tests { let handle = open_selected_export_dir(dir.path()).expect("acquire handle"); let mut used = std::collections::HashSet::new(); - let name = - write_export_file(&handle, "session.json", b"hi", &mut used).expect("valid export"); + let name = write_export_file( + &handle, + &ValidExportLeaf::parse("session.json").unwrap(), + b"hi", + &mut used, + ) + .expect("valid export"); assert_eq!(name, "session.json"); assert_eq!(fs::read(dir.path().join("session.json")).unwrap(), b"hi"); } From b26cedb5e043a0e39490d0e2c11ed05bcddb7fdf Mon Sep 17 00:00:00 2001 From: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Date: Fri, 14 Aug 2026 18:09:05 -0700 Subject: [PATCH 5/5] fix(system): reject reparse-point selection on windows export acquisition FILE_FLAG_OPEN_REPARSE_POINT does not fail the open on a junction/symlink; it returns a handle to the reparse object itself, and cap-std 4.0.2's Dir::from_std_file (src/fs/dir.rs) performs no type or attribute validation on the handle it wraps. The windows branch therefore made a swapped reparse point the capability root, and later handle-relative creates would land on its target instead of failing closed. After opening, query the handle's own attributes via std MetadataExt::file_attributes (a GetFileInformationByHandle by-handle query, not a second path lookup) and reject FILE_ATTRIBUTE_REPARSE_POINT or a non-directory before Dir::from_std_file. This makes acquisition fail closed on a reparse point in the selected final component, matching the unix O_NOFOLLOW behavior. Adds a windows-gated regression (symlink_dir selection must fail closed). Windows compile/runtime remain unverified locally: cross-compiling to x86_64-pc-windows-msvc fails in the aws-lc-sys build script (windows.h not found), a dependency build-script gap, not this code. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Signed-off-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> --- src-tauri/src/commands/system.rs | 56 ++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 9e19adf3a..5e417efb5 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -578,12 +578,38 @@ fn open_selected_export_dir(folder_path: &Path) -> Result