From 1ada8f9d94ebc258fd37ff42d81a6c07798f68f7 Mon Sep 17 00:00:00 2001 From: valoq Date: Wed, 20 May 2026 14:12:26 +0200 Subject: [PATCH 01/16] add landlock sandbox --- Cargo.lock | 32 ++++ Cargo.toml | 3 + src/cli/args.rs | 5 + src/commands/decompress.rs | 143 ++++++++++-------- src/commands/mod.rs | 116 +++++++++++--- src/main.rs | 1 + src/sandbox/landlock.rs | 86 +++++++++++ src/sandbox/mod.rs | 123 +++++++++++++++ .../ui__ui_test_usage_help_flag-2.snap | 2 + .../ui__ui_test_usage_help_flag.snap | 4 + tests/utils.rs | 6 +- 11 files changed, 436 insertions(+), 85 deletions(-) create mode 100644 src/sandbox/landlock.rs create mode 100644 src/sandbox/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 641a14c96..535be9711 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -605,6 +605,26 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" +[[package]] +name = "enumflags2" +version = "0.7.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1027f7680c853e056ebcec683615fb6fbbc07dbaa13b4d5d9442b146ded4ecef" +dependencies = [ + "enumflags2_derive", +] + +[[package]] +name = "enumflags2_derive" +version = "0.7.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67c78a4d8fdf9953a5c9d458f9efe940fd97a0cab0941c075a813ac594733827" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -995,6 +1015,17 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "landlock" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "49fefd6652c57d68aaa32544a4c0e642929725bdc1fd929367cdeb673ab81088" +dependencies = [ + "enumflags2", + "libc", + "thiserror", +] + [[package]] name = "leb128fmt" version = "0.1.0" @@ -1178,6 +1209,7 @@ dependencies = [ "insta", "is_executable", "itertools 0.14.0", + "landlock", "libc", "lz4_flex", "lzma-rust2", diff --git a/Cargo.toml b/Cargo.toml index de4fde4a3..d0a6389eb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,9 @@ libc = "0.2.186" lz4_flex = "0.13.0" lzma-rust2 = "0.16.2" num_cpus = "1.17.0" + +[target.'cfg(target_os = "linux")'.dependencies] +landlock = "0.4.4" rayon = "1.12.0" same-file = "1.0.6" sevenz-rust2 = { version = "0.21.0", features = ["compress", "aes256"] } diff --git a/src/cli/args.rs b/src/cli/args.rs index 0da80e4f8..7162786ad 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -50,6 +50,10 @@ pub struct CliArgs { #[arg(short = 'c', long, global = true)] pub threads: Option, + /// Disable the process sandbox. + #[arg(long, global = true)] + pub no_sandbox: bool, + // Ouch and claps subcommands #[command(subcommand)] pub cmd: Subcommand, @@ -156,6 +160,7 @@ mod tests { // This is usually replaced in assertion tests password: None, threads: None, + no_sandbox: false, cmd: Subcommand::Decompress { // Put a crazy value here so no test can assert it unintentionally files: vec!["\x00\x11\x22".into()], diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index a2ba2eba2..fe62f2441 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -39,6 +39,63 @@ pub struct DecompressOptions<'a> { pub question_policy: QuestionPolicy, pub password: Option<&'a [u8]>, pub remove: bool, + /// Resolved target prepared upfront (before sandbox is applied). + pub prepared: PreparedTarget, +} + +/// Per-input decision made before the sandbox is applied. +pub enum PreparedTarget { + Archive { dir: PathBuf }, + NonArchive { file: fs::File, path: PathBuf }, + Cancelled, +} + +/// Resolve conflicts and create the archive's wrapper dir (or open the non-archive +/// output FD) on disk. Must run before the sandbox is applied. +pub fn prepare_decompress_target( + formats: &[Extension], + output_dir: &Path, + output_file_path: &Path, + output_dir_was_explicit: bool, + here: bool, + question_policy: QuestionPolicy, +) -> Result { + let (first_extension, _) = split_first_compression_format(formats); + let is_archive = matches!(first_extension, Tar | Zip | Rar | SevenZip); + + if !is_archive { + return match utils::create_file_or_prompt_on_conflict( + output_file_path, + question_policy, + QuestionAction::Decompression, + )? { + Some((file, path)) => Ok(PreparedTarget::NonArchive { file, path }), + None => Ok(PreparedTarget::Cancelled), + }; + } + + // --dir / --here use output_dir as-is; default mode wraps in basename-derived subdir. + let target = if output_dir_was_explicit || here { + output_dir.to_path_buf() + } else { + output_file_path.to_path_buf() + }; + let is_cwd = target == *INITIAL_CURRENT_DIR; + let is_valid = + is_cwd || !target.fs_err_try_exists()? || (target.is_dir() && target.read_dir()?.next().is_none()); + + let resolved = if is_valid { + target + } else if let Some(p) = resolve_path_conflict(&target, question_policy, QuestionAction::Decompression)? { + p + } else { + return Ok(PreparedTarget::Cancelled); + }; + + if !resolved.fs_err_try_exists()? { + fs::create_dir(&resolved)?; + } + Ok(PreparedTarget::Archive { dir: resolved }) } enum DecompressionSummary { @@ -93,30 +150,13 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { Ok(reader) }; - // Decide where archives extract: - // --dir -> extract into , no wrapper, no flatten - // --here -> extract into CWD (output_dir), no wrapper - // default -> extract into a basename-derived subdirectory; flatten the - // duplicate when the wrapper would contain exactly one entry - // whose name equals the basename - let archive_output_dir: &Path = if options.output_dir_was_explicit || options.here { - options.output_dir - } else { - &options.output_file_path - }; - let control_flow = match first_extension { Gzip | Bzip | Bzip3 | Lz4 | Lzma | Xz | Lzip | Snappy | Zstd | Brotli => { let reader = create_decoder_up_to_first_extension()?; let mut reader = chain_reader_decoder(&first_extension, reader)?; - let (mut writer, final_output_path) = match utils::create_file_or_prompt_on_conflict( - &options.output_file_path, - options.question_policy, - QuestionAction::Decompression, - )? { - Some(file) => file, - None => return Ok(()), + let PreparedTarget::NonArchive { file: mut writer, path: final_output_path } = options.prepared else { + return Ok(()); }; io::copy(&mut reader, &mut writer)?; @@ -124,11 +164,15 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { output_path: final_output_path, }) } - Tar => unpack_archive( - |output_dir| crate::archive::tar::unpack_archive(create_decoder_up_to_first_extension()?, output_dir), - archive_output_dir, - options.question_policy, - )?, + Tar => { + let PreparedTarget::Archive { dir } = options.prepared else { + return Ok(()); + }; + unpack_archive( + |output_dir| crate::archive::tar::unpack_archive(create_decoder_up_to_first_extension()?, output_dir), + dir, + )? + } Zip | SevenZip => { let unpack_fn = match first_extension { Zip => crate::archive::zip::unpack_archive, @@ -169,16 +213,21 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { )) }; - unpack_archive( - |output_dir| unpack_fn(reader, output_dir, options.password), - archive_output_dir, - options.question_policy, - )? + let PreparedTarget::Archive { dir } = options.prepared else { + return Ok(()); + }; + unpack_archive(|output_dir| unpack_fn(reader, output_dir, options.password), dir)? } #[cfg(feature = "unrar")] Rar => { + let PreparedTarget::Archive { dir } = options.prepared else { + return Ok(()); + }; let unpack_fn: Box Result> = if options.formats.len() > 1 || input_is_stdin { - let mut temp_file = tempfile::NamedTempFile::new()?; + // unrar's C API needs a path; spill the decoded stream into the wrapper dir. + let mut temp_file = tempfile::Builder::new() + .prefix(".ouch-rar-") + .tempfile_in(&dir)?; io::copy(&mut create_decoder_up_to_first_extension()?, &mut temp_file)?; Box::new(move |output_dir| { crate::archive::rar::unpack_archive(temp_file.path(), output_dir, options.password) @@ -189,7 +238,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { }) }; - unpack_archive(unpack_fn, archive_output_dir, options.question_policy)? + unpack_archive(unpack_fn, dir)? } #[cfg(not(feature = "unrar"))] Rar => { @@ -239,39 +288,15 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { Ok(()) } -/// Unpacks an archive creating the output directory, this function will create the output_dir -/// directory or replace it if it already exists. The `output_dir` needs to be empty -/// - If `output_dir` does not exist OR is a empty directory, it will unpack there -/// - If `output_dir` exist OR is a directory not empty, the user will be asked what to do -/// - If `output_dir` is the current working directory, files are extracted directly without prompting +/// Run the format-specific unpack closure against an already-prepared target dir. fn unpack_archive( unpack_fn: impl FnOnce(&Path) -> Result, - output_dir: &Path, - question_policy: QuestionPolicy, + output_dir: PathBuf, ) -> Result> { - // Extracting into the CWD is a merge into the user's workspace and should not prompt, - // matching the behaviour of `tar xf` and `unzip` when no destination is given. - let is_cwd = output_dir == *INITIAL_CURRENT_DIR; - let is_valid_output_dir = - is_cwd || !output_dir.fs_err_try_exists()? || (output_dir.is_dir() && output_dir.read_dir()?.next().is_none()); - - let output_dir_cleaned = if is_valid_output_dir { - output_dir.to_owned() - } else if let Some(path) = resolve_path_conflict(output_dir, question_policy, QuestionAction::Decompression)? { - path - } else { - return Ok(ControlFlow::Break(())); - }; - - if !output_dir_cleaned.fs_err_try_exists()? { - fs::create_dir(&output_dir_cleaned)?; - } - - let files_unpacked = unpack_fn(&output_dir_cleaned)?; - + let files_unpacked = unpack_fn(&output_dir)?; Ok(ControlFlow::Continue(DecompressionSummary::Archive { files_unpacked, - output_path: output_dir_cleaned, + output_path: output_dir, })) } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index f712adea1..15d1cdfed 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -5,7 +5,7 @@ mod decompress; mod list; use bstr::ByteSlice; -use decompress::DecompressOptions; +use decompress::{DecompressOptions, PreparedTarget, prepare_decompress_target}; use rayon::prelude::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator}; use utils::colors; @@ -18,6 +18,7 @@ use crate::{ extension::{self, parse_format_flag}, info_accessible, list::ListOptions, + sandbox::{self, SandboxPolicy}, utils::{ self, BytesFmt, FileVisibilityPolicy, NoQuotePathFmt, PathFmt, QuestionAction, canonicalize, colors::*, file_size, is_path_stdin, @@ -97,6 +98,15 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic None => return Ok(()), }; + { + // Read on inputs; the output FD is held already, no write-path grant needed. + let mut policy = SandboxPolicy::new(); + for f in &files { + policy.allow_read(sandbox::canonicalize_for_sandbox(f)); + } + policy.set_disabled(args.no_sandbox).apply(); + } + let level = if fast { Some(1) // Lowest level of compression } else if slow { @@ -119,22 +129,14 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic if let Ok(true) = compress_result { info_accessible!("Output file size: {}", BytesFmt(file_size(&output_path)?)); info_accessible!("Successfully compressed to {}", PathFmt(&output_path)); - } else { - // If Ok(false) or Err() occurred, delete incomplete file at `output_path` - // - // if deleting fails, print an extra alert message pointing - // out that we left a possibly CORRUPTED file at `output_path` - if utils::remove_file_or_dir(&output_path).is_err() { - eprintln!("{red}FATAL ERROR:\n", red = *colors::RED); - eprintln!(" Ouch failed to delete the file {}", PathFmt(&output_path)); - eprintln!(" Please delete it manually."); - eprintln!(" This file is corrupted if compression didn't finished."); - - if compress_result.is_err() { - eprintln!(" Compression failed for reasons below."); - } - } + } else if compress_result.is_err() { + // Sandbox blocks unlinking the partial file; tell the user to delete it. + eprintln!("{red}FATAL ERROR:\n", red = *colors::RED); + eprintln!(" Compression did not finish; the partial file at {} may be corrupted.", PathFmt(&output_path)); + eprintln!(" Please delete it manually."); + eprintln!(" Compression failed for reasons below."); } + // Ok(false) means the user cancelled before any data was written; nothing to report. compress_result.map(|_| ()) } @@ -192,18 +194,71 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic INITIAL_CURRENT_DIR.clone() }; - files - .par_iter() - .zip(files_extensions) - .zip(files_output_paths) - .try_for_each(|((input_path, formats), file_name)| { - // Path used by single file format archives - let output_file_path = if is_path_stdin(&file_name) { + // Per-input output paths for single-file outputs and archive wrapper dirs. + let output_file_paths: Vec = files_output_paths + .iter() + .map(|file_name| { + if is_path_stdin(file_name) { output_dir.join("ouch-output") } else { output_dir.join(file_name) - }; + } + }) + .collect(); + + // TODO: warn when the sandbox grant would include $HOME or an ancestor (default mode w/ CWD=$HOME, --dir/--here pointing at $HOME). + + // Resolve conflicts and create wrappers / open output FDs upfront, before the sandbox applies. + let mut prepared: Vec = Vec::with_capacity(files.len()); + for (formats, output_file_path) in files_extensions.iter().zip(&output_file_paths) { + prepared.push(prepare_decompress_target( + formats, + &output_dir, + output_file_path, + output_dir_was_explicit, + here, + question_policy, + )?); + } + + // Sandbox: read on inputs, RW on prepared wrappers. RAR temp file lands inside its wrapper. + { + let mut policy = SandboxPolicy::new(); + for f in &files { + if !is_path_stdin(f) { + policy.allow_read(sandbox::canonicalize_for_sandbox(f)); + } + } + for p in &prepared { + match p { + PreparedTarget::Archive { dir } => { + policy.allow_read_write(sandbox::canonicalize_for_sandbox(dir)); + } + // Non-archive: held FD covers writes, no path grant needed. + PreparedTarget::NonArchive { .. } | PreparedTarget::Cancelled => {} + } + } + // TODO: warn when that parent is $HOME or an ancestor. + if remove { + for f in &files { + if is_path_stdin(f) { + continue; + } + let canon = sandbox::canonicalize_for_sandbox(f); + if let Some(parent) = canon.parent() { + policy.allow_read_write(parent.to_path_buf()); + } + } + } + policy.set_disabled(args.no_sandbox).apply(); + } + files + .par_iter() + .zip(files_extensions) + .zip(output_file_paths) + .zip(prepared) + .try_for_each(|(((input_path, formats), output_file_path), prepared)| { decompress_file(DecompressOptions { input_file_path: input_path, formats, @@ -216,6 +271,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic <[u8] as ByteSlice>::from_os_str(str).expect("convert password to bytes failed") }), remove, + prepared, }) .map_err(|err| match err { Error::IoError { reason } => Error::Custom { @@ -256,6 +312,18 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // Ensure we were not told to list the content of a non-archive compressed file check::check_for_non_archive_formats(&files, &formats)?; + { + let mut policy = SandboxPolicy::new(); + for f in &files { + let canon = sandbox::canonicalize_for_sandbox(f); + policy.allow_read(canon.clone()); + if let Some(parent) = canon.parent() { + policy.allow_read(parent.to_path_buf()); + } + } + policy.set_disabled(args.no_sandbox).apply(); + } + let list_options = ListOptions { tree, quiet: args.quiet, diff --git a/src/main.rs b/src/main.rs index a82e077ab..cb0b72124 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,6 +7,7 @@ pub mod error; pub mod extension; pub mod list; pub mod non_archive; +pub mod sandbox; pub mod utils; use std::{env, path::PathBuf, sync::LazyLock}; diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs new file mode 100644 index 000000000..767a02166 --- /dev/null +++ b/src/sandbox/landlock.rs @@ -0,0 +1,86 @@ +//! Landlock LSM backend. Targets ABI v6 (Linux 6.12+). + +use std::path::Path; + +use landlock::{ + ABI, Access, AccessFs, BitFlags, CompatLevel, Compatible, PathBeneath, PathFd, Ruleset, RulesetAttr, + RulesetCreated, RulesetCreatedAttr, RulesetError, RulesetStatus, +}; + +use super::SandboxPolicy; +use crate::warning; + +const TARGET_ABI: ABI = ABI::V6; + +pub fn apply(policy: &SandboxPolicy) { + match install(policy) { + Ok(RulesetStatus::FullyEnforced) => {} + Ok(RulesetStatus::PartiallyEnforced) => { + warning!( + "Landlock sandbox is only partially enforced; this kernel does not support all of Landlock ABI v{}", + TARGET_ABI as u8 + ); + } + Ok(RulesetStatus::NotEnforced) => { + warning!("Landlock LSM is not available on this system; running without filesystem sandbox"); + } + Err(err) => { + warning!("Failed to enable Landlock sandbox: {err}; running unrestricted"); + } + } +} + +fn install(policy: &SandboxPolicy) -> Result { + let read_access = AccessFs::from_read(TARGET_ABI); + let all_access = AccessFs::from_all(TARGET_ABI); + + let ruleset = Ruleset::default() + .set_compatibility(CompatLevel::BestEffort) + .handle_access(all_access)? + .create()?; + + let ruleset = add_rules(ruleset, policy.read_paths(), read_access)?; + let ruleset = add_rules(ruleset, policy.read_write_paths(), all_access)?; + + Ok(ruleset.restrict_self()?.ruleset) +} + +fn add_rules(ruleset: RulesetCreated, paths: I, access: BitFlags) -> Result +where + I: IntoIterator, + I::Item: AsRef, +{ + let mut current = ruleset; + for path in paths { + let path = path.as_ref(); + match PathFd::new(path) { + Ok(fd) => { + let rule = PathBeneath::new(fd, access).set_compatibility(CompatLevel::BestEffort); + current = current.add_rule(rule)?; + } + Err(err) => { + warning!("Sandbox: cannot open {}: {err}", path.display()); + } + } + } + Ok(current) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn target_abi_is_v6() { + assert_eq!(TARGET_ABI as u8, ABI::V6 as u8); + } + + #[test] + fn install_with_empty_policy_does_not_error() { + // Even on kernels without Landlock support, BestEffort means install() + // returns Ok with NotEnforced rather than Err. + let policy = SandboxPolicy::new(); + let result = install(&policy); + assert!(result.is_ok(), "install returned {result:?}"); + } +} diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs new file mode 100644 index 000000000..1a3d7c166 --- /dev/null +++ b/src/sandbox/mod.rs @@ -0,0 +1,123 @@ +//! Per-process filesystem sandbox. + +use std::path::{Path, PathBuf}; + +#[cfg(target_os = "linux")] +mod landlock; + +#[derive(Debug, Default, Clone)] +pub struct SandboxPolicy { + read: Vec, + read_write: Vec, + disabled: bool, +} + +impl SandboxPolicy { + pub fn new() -> Self { + Self::default() + } + + pub fn allow_read>(&mut self, path: P) -> &mut Self { + self.read.push(path.into()); + self + } + + pub fn allow_read_write>(&mut self, path: P) -> &mut Self { + self.read_write.push(path.into()); + self + } + + pub fn set_disabled(&mut self, disabled: bool) -> &mut Self { + self.disabled = disabled; + self + } + + pub fn read_paths(&self) -> &[PathBuf] { + &self.read + } + + pub fn read_write_paths(&self) -> &[PathBuf] { + &self.read_write + } + + /// Apply the policy. Failures emit a warning and let the process run + /// unrestricted. + pub fn apply(&self) { + if self.disabled || std::env::var_os("OUCH_NO_SANDBOX").is_some() { + return; + } + #[cfg(target_os = "linux")] + landlock::apply(self); + } +} + +/// Resolve `path` to an absolute path, falling back to its parent if the +/// target itself doesn't yet exist. +pub fn canonicalize_for_sandbox(path: &Path) -> PathBuf { + if let Ok(c) = std::fs::canonicalize(path) { + return c; + } + if let Some(parent) = path.parent() + && let Ok(c) = std::fs::canonicalize(parent) + { + return c.join(path.file_name().unwrap_or_default()); + } + path.to_path_buf() +} + +/// Returns true if `target` is $HOME itself or an ancestor of it. +#[allow(dead_code)] +pub fn is_home_or_ancestor(target: &Path) -> bool { + let Some(home) = std::env::var_os("HOME").map(PathBuf::from) else { + return false; + }; + let Ok(home) = std::fs::canonicalize(&home) else { + return false; + }; + let target = canonicalize_for_sandbox(target); + home == target || home.starts_with(&target) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn policy_collects_paths() { + let mut policy = SandboxPolicy::new(); + policy.allow_read("/tmp/in").allow_read_write("/tmp/out"); + + assert_eq!(policy.read_paths(), &[PathBuf::from("/tmp/in")]); + assert_eq!(policy.read_write_paths(), &[PathBuf::from("/tmp/out")]); + } + + #[test] + fn policy_disabled_default_false() { + let policy = SandboxPolicy::new(); + assert!(!policy.disabled); + } + + #[test] + fn policy_set_disabled() { + let mut policy = SandboxPolicy::new(); + policy.set_disabled(true); + assert!(policy.disabled); + } + + #[test] + fn canonicalize_existing_path_resolves() { + let dir = tempfile::tempdir().unwrap(); + let canon = canonicalize_for_sandbox(dir.path()); + assert!(canon.is_absolute()); + assert!(canon.exists()); + } + + #[test] + fn canonicalize_missing_path_falls_back_to_parent() { + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("does-not-exist-yet.txt"); + let canon = canonicalize_for_sandbox(&missing); + assert!(canon.is_absolute()); + assert_eq!(canon.file_name().unwrap(), "does-not-exist-yet.txt"); + } +} diff --git a/tests/snapshots/ui__ui_test_usage_help_flag-2.snap b/tests/snapshots/ui__ui_test_usage_help_flag-2.snap index af5c50c0b..8ddf9a4e0 100644 --- a/tests/snapshots/ui__ui_test_usage_help_flag-2.snap +++ b/tests/snapshots/ui__ui_test_usage_help_flag-2.snap @@ -1,5 +1,6 @@ --- source: tests/ui.rs +assertion_line: 194 expression: "output_to_string(ouch!(\"-h\"))" --- A command-line utility for easily compressing and decompressing files and directories. @@ -22,5 +23,6 @@ Options: -f, --format Specify the format of the archive -p, --password Decompress or list with password -c, --threads Concurrent working threads + --no-sandbox Disable the process sandbox -h, --help Print help (see more with '--help') -V, --version Print version diff --git a/tests/snapshots/ui__ui_test_usage_help_flag.snap b/tests/snapshots/ui__ui_test_usage_help_flag.snap index 7b0572a13..185866fd6 100644 --- a/tests/snapshots/ui__ui_test_usage_help_flag.snap +++ b/tests/snapshots/ui__ui_test_usage_help_flag.snap @@ -1,5 +1,6 @@ --- source: tests/ui.rs +assertion_line: 193 expression: "output_to_string(ouch!(\"--help\"))" --- A command-line utility for easily compressing and decompressing files and directories. @@ -46,6 +47,9 @@ Options: -c, --threads Concurrent working threads + --no-sandbox + Disable the process sandbox + -h, --help Print help (see a summary with '-h') diff --git a/tests/utils.rs b/tests/utils.rs index 443dd38db..3e16181e5 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -23,7 +23,7 @@ macro_rules! ouch { } pub fn cargo_bin() -> Command { - env::vars() + let mut cmd = env::vars() .find_map(|(k, v)| { (k.starts_with("CARGO_TARGET_") && k.ends_with("_RUNNER")).then(|| { let mut runner = v.split_whitespace(); @@ -32,7 +32,9 @@ pub fn cargo_bin() -> Command { cmd }) }) - .unwrap_or_else(|| Command::cargo_bin("ouch").expect("Failed to find ouch executable")) + .unwrap_or_else(|| Command::cargo_bin("ouch").expect("Failed to find ouch executable")); + cmd.env("OUCH_NO_SANDBOX", "1"); + cmd } pub fn testdir() -> io::Result<(tempfile::TempDir, &'static Path)> { From 79316c576ed4f3304be0637d5675b6f4486b6364 Mon Sep 17 00:00:00 2001 From: valoq Date: Sat, 6 Jun 2026 21:37:31 +0200 Subject: [PATCH 02/16] complete sandbox --- src/commands/decompress.rs | 15 +++++----- src/commands/list.rs | 5 +++- src/commands/mod.rs | 58 +++++++++++++++++++++++++++++--------- src/sandbox/landlock.rs | 3 ++ src/sandbox/mod.rs | 13 ++++++++- 5 files changed, 70 insertions(+), 24 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index fe62f2441..24acf1481 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -28,8 +28,6 @@ pub struct DecompressOptions<'a> { /// Example: [Gz, Tar] (notice it's ordered in decompression order) pub formats: Vec, pub output_dir: &'a Path, - /// Used when extracting single file formats and not archive formats - pub output_file_path: PathBuf, /// Whether the user passed `--dir` explicitly. When false (and `here` is false), /// archives are extracted into a basename-derived subdirectory of the CWD. pub output_dir_was_explicit: bool, @@ -81,8 +79,7 @@ pub fn prepare_decompress_target( output_file_path.to_path_buf() }; let is_cwd = target == *INITIAL_CURRENT_DIR; - let is_valid = - is_cwd || !target.fs_err_try_exists()? || (target.is_dir() && target.read_dir()?.next().is_none()); + let is_valid = is_cwd || !target.fs_err_try_exists()? || (target.is_dir() && target.read_dir()?.next().is_none()); let resolved = if is_valid { target @@ -155,7 +152,11 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { let reader = create_decoder_up_to_first_extension()?; let mut reader = chain_reader_decoder(&first_extension, reader)?; - let PreparedTarget::NonArchive { file: mut writer, path: final_output_path } = options.prepared else { + let PreparedTarget::NonArchive { + file: mut writer, + path: final_output_path, + } = options.prepared + else { return Ok(()); }; @@ -225,9 +226,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { }; let unpack_fn: Box Result> = if options.formats.len() > 1 || input_is_stdin { // unrar's C API needs a path; spill the decoded stream into the wrapper dir. - let mut temp_file = tempfile::Builder::new() - .prefix(".ouch-rar-") - .tempfile_in(&dir)?; + let mut temp_file = tempfile::Builder::new().prefix(".ouch-rar-").tempfile_in(&dir)?; io::copy(&mut create_decoder_up_to_first_extension()?, &mut temp_file)?; Box::new(move |output_dir| { crate::archive::rar::unpack_archive(temp_file.path(), output_dir, options.password) diff --git a/src/commands/list.rs b/src/commands/list.rs index 78c9a79fe..32bfd5097 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -22,6 +22,8 @@ pub fn list_archive_contents( list_options: ListOptions, question_policy: QuestionPolicy, password: Option<&[u8]>, + // Pre-opened tempfile FD for multi-format RAR list under the sandbox. + rar_spill_tempfile: Option, ) -> Result<()> { let reader = fs::File::open(archive_path)?; @@ -100,7 +102,8 @@ pub fn list_archive_contents( #[cfg(feature = "unrar")] Rar => { if formats.len() > 1 { - let mut temp_file = tempfile::NamedTempFile::new()?; + // Reuse the tempfile pre-opened by the caller if there is one. + let mut temp_file = rar_spill_tempfile.unwrap_or(tempfile::NamedTempFile::new()?); io::copy(&mut reader, &mut temp_file)?; Box::new(crate::archive::rar::list_archive(temp_file.path(), password)?) } else { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 15d1cdfed..0bb2c567e 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -4,6 +4,8 @@ mod compress; mod decompress; mod list; +use std::path::PathBuf; + use bstr::ByteSlice; use decompress::{DecompressOptions, PreparedTarget, prepare_decompress_target}; use rayon::prelude::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator}; @@ -23,6 +25,7 @@ use crate::{ self, BytesFmt, FileVisibilityPolicy, NoQuotePathFmt, PathFmt, QuestionAction, canonicalize, colors::*, file_size, is_path_stdin, }, + warning, }; /// Warn the user that (de)compressing this .zip archive might freeze their system. @@ -132,7 +135,10 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic } else if compress_result.is_err() { // Sandbox blocks unlinking the partial file; tell the user to delete it. eprintln!("{red}FATAL ERROR:\n", red = *colors::RED); - eprintln!(" Compression did not finish; the partial file at {} may be corrupted.", PathFmt(&output_path)); + eprintln!( + " Compression did not finish; the partial file at {} may be corrupted.", + PathFmt(&output_path) + ); eprintln!(" Please delete it manually."); eprintln!(" Compression failed for reasons below."); } @@ -232,13 +238,20 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic for p in &prepared { match p { PreparedTarget::Archive { dir } => { - policy.allow_read_write(sandbox::canonicalize_for_sandbox(dir)); + let canon = sandbox::canonicalize_for_sandbox(dir); + // The sandbox cannot confine writes when the target is inside $HOME. + if (output_dir_was_explicit || here) && sandbox::is_home_or_ancestor(&canon) { + warning!( + "Sandbox: extraction target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine writes", + PathFmt(&canon) + ); + } + policy.allow_read_write(canon); } // Non-archive: held FD covers writes, no path grant needed. PreparedTarget::NonArchive { .. } | PreparedTarget::Cancelled => {} } } - // TODO: warn when that parent is $HOME or an ancestor. if remove { for f in &files { if is_path_stdin(f) { @@ -246,7 +259,8 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic } let canon = sandbox::canonicalize_for_sandbox(f); if let Some(parent) = canon.parent() { - policy.allow_read_write(parent.to_path_buf()); + // The decompressor can delete the input archive but not write nearby. + policy.allow_remove_in(parent.to_path_buf()); } } } @@ -256,14 +270,12 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic files .par_iter() .zip(files_extensions) - .zip(output_file_paths) .zip(prepared) - .try_for_each(|(((input_path, formats), output_file_path), prepared)| { + .try_for_each(|((input_path, formats), prepared)| { decompress_file(DecompressOptions { input_file_path: input_path, formats, output_dir: &output_dir, - output_file_path, output_dir_was_explicit, here, question_policy, @@ -312,14 +324,27 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // Ensure we were not told to list the content of a non-archive compressed file check::check_for_non_archive_formats(&files, &formats)?; + // Flatten the per-file formats up front so the next step can inspect them. + let flat_formats: Vec> = formats + .iter() + .map(|f| extension::flatten_compression_formats(f)) + .collect(); + // Open RAR spill tempfiles up front so unrar can read from them under the sandbox. + let mut rar_spill_tempfiles: Vec> = flat_formats + .iter() + .map(|fs| { + if fs.len() > 1 && matches!(fs.last(), Some(extension::CompressionFormat::Rar)) { + tempfile::NamedTempFile::new().ok() + } else { + None + } + }) + .collect(); + { let mut policy = SandboxPolicy::new(); for f in &files { - let canon = sandbox::canonicalize_for_sandbox(f); - policy.allow_read(canon.clone()); - if let Some(parent) = canon.parent() { - policy.allow_read(parent.to_path_buf()); - } + policy.allow_read(sandbox::canonicalize_for_sandbox(f)); } policy.set_disabled(args.no_sandbox).apply(); } @@ -329,11 +354,15 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic quiet: args.quiet, }; - for (i, (archive_path, formats)) in files.iter().zip(formats).enumerate() { + for (i, ((archive_path, formats), spill)) in files + .iter() + .zip(flat_formats) + .zip(rar_spill_tempfiles.iter_mut()) + .enumerate() + { if i > 0 && !args.quiet { println!(); } - let formats = extension::flatten_compression_formats(&formats); list_archive_contents( archive_path, formats, @@ -342,6 +371,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic args.password .as_deref() .map(|str| <[u8] as ByteSlice>::from_os_str(str).expect("convert password to bytes failed")), + spill.take(), )?; } diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index 767a02166..7ef81f27d 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -33,6 +33,8 @@ pub fn apply(policy: &SandboxPolicy) { fn install(policy: &SandboxPolicy) -> Result { let read_access = AccessFs::from_read(TARGET_ABI); let all_access = AccessFs::from_all(TARGET_ABI); + // REMOVE_FILE + REFER lets `--remove` delete the input archive but nothing else. + let remove_access: BitFlags = AccessFs::RemoveFile | AccessFs::Refer; let ruleset = Ruleset::default() .set_compatibility(CompatLevel::BestEffort) @@ -41,6 +43,7 @@ fn install(policy: &SandboxPolicy) -> Result { let ruleset = add_rules(ruleset, policy.read_paths(), read_access)?; let ruleset = add_rules(ruleset, policy.read_write_paths(), all_access)?; + let ruleset = add_rules(ruleset, policy.remove_in_paths(), remove_access)?; Ok(ruleset.restrict_self()?.ruleset) } diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 1a3d7c166..4221f573a 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -9,6 +9,8 @@ mod landlock; pub struct SandboxPolicy { read: Vec, read_write: Vec, + /// Directories where the decompressor can delete the input archive but not write nearby. + remove_in: Vec, disabled: bool, } @@ -27,6 +29,12 @@ impl SandboxPolicy { self } + /// Allow deleting files inside `path` without granting any other write rights. + pub fn allow_remove_in>(&mut self, path: P) -> &mut Self { + self.remove_in.push(path.into()); + self + } + pub fn set_disabled(&mut self, disabled: bool) -> &mut Self { self.disabled = disabled; self @@ -40,6 +48,10 @@ impl SandboxPolicy { &self.read_write } + pub fn remove_in_paths(&self) -> &[PathBuf] { + &self.remove_in + } + /// Apply the policy. Failures emit a warning and let the process run /// unrestricted. pub fn apply(&self) { @@ -66,7 +78,6 @@ pub fn canonicalize_for_sandbox(path: &Path) -> PathBuf { } /// Returns true if `target` is $HOME itself or an ancestor of it. -#[allow(dead_code)] pub fn is_home_or_ancestor(target: &Path) -> bool { let Some(home) = std::env::var_os("HOME").map(PathBuf::from) else { return false; From b931c842c8b4397bee22f73f0fc219e649f7b591 Mon Sep 17 00:00:00 2001 From: valoq Date: Sat, 6 Jun 2026 21:50:43 +0200 Subject: [PATCH 03/16] fix cargo --- Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 497ce6374..45402ddb0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,9 +34,6 @@ libc = "0.2.186" lz4_flex = "0.13.0" lzma-rust2 = "0.16.2" num_cpus = "1.17.0" - -[target.'cfg(target_os = "linux")'.dependencies] -landlock = "0.4.4" rayon = "1.12.0" same-file = "1.0.6" sevenz-rust2 = { version = "0.21.0", features = ["compress", "aes256"] } @@ -52,6 +49,9 @@ zip = { version = "8.6.0", default-features = false, features = [ ] } zstd = { version = "0.13.3", default-features = false, features = ["zstdmt"] } +[target.'cfg(target_os = "linux")'.dependencies] +landlock = "0.4.4" + [dev-dependencies] anyhow = "1.0.102" assert_cmd = "2.2.1" From 2867efd2a45f022c3783ec4df589a71ce2486956 Mon Sep 17 00:00:00 2001 From: valoq Date: Sun, 7 Jun 2026 22:50:45 +0200 Subject: [PATCH 04/16] improve patch --- src/archive/tar.rs | 15 +- src/commands/decompress.rs | 158 ++++++++++-------- src/commands/mod.rs | 33 ++-- src/sandbox/landlock.rs | 47 ++++-- .../snapshots/ui__ui_test_ok_decompress.snap | 2 +- 5 files changed, 145 insertions(+), 110 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 843cd3cf7..5d6edb811 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -55,7 +55,14 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result { let full_link_path = output_folder.join(&link_path); let full_target_path = output_folder.join(&target); - fs::hard_link(&full_target_path, &full_link_path)?; + // Cross-directory hard links need REFER which old kernels deny so copy instead. + if let Err(err) = fs::hard_link(&full_target_path, &full_link_path) { + info!( + "Could not hard link {} ({err}); copying instead", + PathFmt(&full_link_path) + ); + fs::copy(&full_target_path, &full_link_path)?; + } } tar::EntryType::Regular | tar::EntryType::GNUSparse => { entry.unpack_in(output_folder)?; @@ -70,11 +77,11 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result { if cfg!(unix) && is_writeable.not() { // We unpacked a read-only directory, make it writeable so that we can // create the files inside of it, by the end, restore the original mode - let original_path = entry.path()?.to_path_buf(); - let unpacked = output_folder.join(&original_path); + let unpacked = output_folder.join(entry.path()?); set_permission_mode(&unpacked, original_mode | 0o200)?; - read_only_dirs_and_modes.push((original_path, original_mode)); + // Store the absolute path because the restore loop runs without changing directory. + read_only_dirs_and_modes.push((unpacked, original_mode)); } } _ => continue, diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 24acf1481..c6c985eee 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -1,4 +1,5 @@ use std::{ + ffi::OsString, io::{self, BufReader, Read}, ops::ControlFlow, path::{Path, PathBuf}, @@ -16,7 +17,7 @@ use crate::{ info, info_accessible, non_archive::lz4::MultiFrameLz4Decoder, utils::{ - self, BytesFmt, PathFmt, file_size, + BytesFmt, PathFmt, file_size, io::{ReadSeek, lock_and_flush_output_stdio}, is_path_stdin, resolve_path_conflict, user_wants_to_continue, }, @@ -42,14 +43,13 @@ pub struct DecompressOptions<'a> { } /// Per-input decision made before the sandbox is applied. +/// file_name is the single output file for non-archives and None for archives. pub enum PreparedTarget { - Archive { dir: PathBuf }, - NonArchive { file: fs::File, path: PathBuf }, + Target { dir: PathBuf, file_name: Option }, Cancelled, } -/// Resolve conflicts and create the archive's wrapper dir (or open the non-archive -/// output FD) on disk. Must run before the sandbox is applied. +/// Resolve conflicts and create the output directory before the sandbox starts. pub fn prepare_decompress_target( formats: &[Extension], output_dir: &Path, @@ -61,18 +61,8 @@ pub fn prepare_decompress_target( let (first_extension, _) = split_first_compression_format(formats); let is_archive = matches!(first_extension, Tar | Zip | Rar | SevenZip); - if !is_archive { - return match utils::create_file_or_prompt_on_conflict( - output_file_path, - question_policy, - QuestionAction::Decompression, - )? { - Some((file, path)) => Ok(PreparedTarget::NonArchive { file, path }), - None => Ok(PreparedTarget::Cancelled), - }; - } - - // --dir / --here use output_dir as-is; default mode wraps in basename-derived subdir. + // --dir and --here use output_dir directly + // default mode wraps it in a basename subdirectory let target = if output_dir_was_explicit || here { output_dir.to_path_buf() } else { @@ -92,7 +82,9 @@ pub fn prepare_decompress_target( if !resolved.fs_err_try_exists()? { fs::create_dir(&resolved)?; } - Ok(PreparedTarget::Archive { dir: resolved }) + + let file_name = (!is_archive).then(|| output_file_path.file_name().unwrap_or_default().to_os_string()); + Ok(PreparedTarget::Target { dir: resolved, file_name }) } enum DecompressionSummary { @@ -152,21 +144,23 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { let reader = create_decoder_up_to_first_extension()?; let mut reader = chain_reader_decoder(&first_extension, reader)?; - let PreparedTarget::NonArchive { - file: mut writer, - path: final_output_path, + let PreparedTarget::Target { + dir, + file_name: Some(file_name), } = options.prepared else { return Ok(()); }; + let final_output_path = dir.join(file_name); + let mut writer = fs::File::create(&final_output_path)?; io::copy(&mut reader, &mut writer)?; ControlFlow::Continue(DecompressionSummary::NonArchive { output_path: final_output_path, }) } Tar => { - let PreparedTarget::Archive { dir } = options.prepared else { + let PreparedTarget::Target { dir, .. } = options.prepared else { return Ok(()); }; unpack_archive( @@ -214,14 +208,14 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { )) }; - let PreparedTarget::Archive { dir } = options.prepared else { + let PreparedTarget::Target { dir, .. } = options.prepared else { return Ok(()); }; unpack_archive(|output_dir| unpack_fn(reader, output_dir, options.password), dir)? } #[cfg(feature = "unrar")] Rar => { - let PreparedTarget::Archive { dir } = options.prepared else { + let PreparedTarget::Target { dir, .. } = options.prepared else { return Ok(()); }; let unpack_fn: Box Result> = if options.formats.len() > 1 || input_is_stdin { @@ -258,8 +252,9 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { // ended up containing exactly one entry whose name matches the wrapper itself // (e.g. `archive.zip` contained a single `archive/` root), flatten that // duplicate so the user sees `./archive/...` not `./archive/archive/...`. + // Best effort because the cross-directory rename needs REFER which old kernels lack. if !options.output_dir_was_explicit && !options.here { - deduplicate_basename_wrapper(&output_path)?; + let _ = deduplicate_basename_wrapper(&output_path); } info_accessible!("Successfully decompressed archive to {}", PathFmt(&output_path)); info_accessible!("Files unpacked: {files_unpacked}"); @@ -299,11 +294,8 @@ fn unpack_archive( })) } -/// Expects `wrapper` to be a just-decompressed archive output directory, if -/// `wrapper` contains exactly one entry with the same name (e.g. extracting -/// `archive.zip` produced `archive/archive/...`), then flatten it to `archive/...`. -/// -/// Returns the resulting path the user should see. If reads fail, +/// Flattens a wrapper that holds a single entry with the wrapper name +/// For example a zip that gave archive/archive fn deduplicate_basename_wrapper(wrapper: &Path) -> Result<()> { let Some(wrapper_name) = wrapper.file_name() else { return Ok(()); @@ -315,45 +307,46 @@ fn deduplicate_basename_wrapper(wrapper: &Path) -> Result<()> { let Some(first_file) = entries.next().transpose()? else { return Ok(()); }; - // More than one entry, don't deduplicate + // More than one entry so do not deduplicate. if entries.next().transpose()?.is_some() { return Ok(()); } first_file }; - // name doesn't match, nothing to deduplicate + // Name does not match so nothing to deduplicate. if only_file_in_dir.file_name() != wrapper_name { return Ok(()); } - // The wrapper duplicates the inner entry's name. Promote the inner entry by: - // 1. moving it into a sibling temporary directory - // 2. removing the now-empty wrapper - // 3. moving it back to the wrapper's path - // Each step leaves the filesystem in a consistent state, and a failure midway - // leaves the user with valid extracted content (just nested one level). - let inner_path = only_file_in_dir.path(); - let Some(parent) = wrapper.parent() else { + if !only_file_in_dir.file_type()?.is_dir() { return Ok(()); - }; - - // Create the sibling - let sibling_path = parent.join("ouch-temporary"); - fs::create_dir(&sibling_path)?; - - // Move child inside the sibling - let path_inside_sibling = sibling_path.join(wrapper_name); - fs::rename(&inner_path, &path_inside_sibling)?; - - // Delete old parent - fs::remove_dir(wrapper)?; + } - // Move child to its dead parent place - fs::rename(&path_inside_sibling, wrapper)?; + let inner_dir = only_file_in_dir.path(); + + let mut staged: Option = None; + for entry in fs::read_dir(&inner_dir)? { + let entry = entry?; + let name = entry.file_name(); + if name == wrapper_name { + let tmp = tempfile::Builder::new() + .prefix(".ouch-") + .tempfile_in(wrapper)? + .into_temp_path(); + fs::remove_file(&tmp)?; + fs::rename(entry.path(), &tmp)?; + staged = Some(tmp); + } else { + fs::rename(entry.path(), wrapper.join(name))?; + } + } - // Delete the temporary sibling - fs::remove_dir(sibling_path)?; + fs::remove_dir(&inner_dir)?; + if let Some(tmp) = staged { + fs::rename(&tmp, wrapper.join(wrapper_name))?; + let _ = tmp.keep(); + } Ok(()) } @@ -366,7 +359,7 @@ mod tests { use super::*; - /// Helper: collect the relative paths of every entry under `root`, sorted. + /// Collect sorted relative paths of every entry under root. fn list_tree(root: &Path) -> Vec { fn walk(p: &Path, base: &Path, out: &mut Vec) { if let Ok(entries) = std_fs::read_dir(p) { @@ -388,8 +381,7 @@ mod tests { out } - /// The main case: wrapper contains exactly one entry whose name equals the wrapper's - /// name. The inner entry should be promoted up one level. + /// Inner directory matching the wrapper name is promoted up. #[test] fn deduplicate_flattens_when_inner_dir_matches_wrapper_name() { let dir = tempdir().unwrap(); @@ -401,10 +393,15 @@ mod tests { deduplicate_basename_wrapper(&wrapper).unwrap(); assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); + // Parent must be untouched with only the wrapper visible. + let parent_entries: Vec<_> = std_fs::read_dir(dir.path()) + .unwrap() + .map(|e| e.unwrap().file_name()) + .collect(); + assert_eq!(parent_entries, vec![std::ffi::OsString::from("archive")]); } - /// Wrapper contains a single entry, but its name differs from the wrapper's name. - /// No flatten should happen — the wrapper survives and the inner entry stays nested. + /// A single inner entry with a different name stays nested. #[test] fn deduplicate_keeps_wrapper_when_inner_name_differs() { let dir = tempdir().unwrap(); @@ -417,7 +414,7 @@ mod tests { assert_eq!(list_tree(&wrapper), vec!["mytool/", "mytool/file.txt"]); } - /// Wrapper contains two or more entries — no flatten regardless of names. + /// More than one entry means no flatten. #[test] fn deduplicate_keeps_wrapper_when_multiple_entries() { let dir = tempdir().unwrap(); @@ -430,7 +427,7 @@ mod tests { assert_eq!(list_tree(&wrapper), vec!["a.txt", "b.txt"]); } - /// Empty wrapper — nothing to flatten, no-op. + /// An empty wrapper is left alone. #[test] fn deduplicate_is_noop_on_empty_wrapper() { let dir = tempdir().unwrap(); @@ -442,24 +439,38 @@ mod tests { assert_eq!(list_tree(&wrapper), Vec::::new()); } - /// Edge case: the single inner entry is a *file* (not a directory) whose name - /// equals the wrapper's name. The wrapper directory should be replaced with that file. + /// A single inner entry that is a file stays nested. #[test] - fn deduplicate_promotes_single_inner_file_with_matching_name() { + fn deduplicate_keeps_wrapper_when_inner_is_file() { let dir = tempdir().unwrap(); let wrapper = dir.path().join("archive"); std_fs::create_dir(&wrapper).unwrap(); std_fs::write(wrapper.join("archive"), "data").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - assert!(wrapper.is_file(), "wrapper should now be a file, not a directory"); - assert_eq!(std_fs::read(&wrapper).unwrap(), b"data"); + assert!(wrapper.is_dir(), "wrapper must remain a directory"); + assert_eq!(list_tree(&wrapper), vec!["archive".to_string()]); + assert_eq!(std_fs::read(wrapper.join("archive")).unwrap(), b"data"); + } + + /// Inner child named like the wrapper is moved aside under a temp name. + #[test] + fn deduplicate_handles_inner_entry_named_like_wrapper() { + let dir = tempdir().unwrap(); + let wrapper = dir.path().join("archive"); + let inner = wrapper.join("archive"); + let collision = inner.join("archive"); + std_fs::create_dir_all(&collision).unwrap(); + std_fs::write(collision.join("inside"), "x").unwrap(); + std_fs::write(inner.join("other.txt"), "o").unwrap(); + + deduplicate_basename_wrapper(&wrapper).unwrap(); + assert_eq!(list_tree(&wrapper), vec!["archive/", "archive/inside", "other.txt"]); + assert_eq!(std_fs::read(wrapper.join("archive").join("inside")).unwrap(), b"x"); + assert_eq!(std_fs::read(wrapper.join("other.txt")).unwrap(), b"o"); } - /// The flatten only collapses *one* level: nested same-name directories produced - /// by the archive itself stay intact. For example, an archive whose layout is - /// `testing/testing/file` extracted into `./testing/` should leave the user with - /// `./testing/testing/file`, not `./testing/file`. + /// Only the outer wrapper collapses and nested same-name folders stay. #[test] fn deduplicate_only_flattens_outer_wrapper_not_inner_duplicates() { let dir = tempdir().unwrap(); @@ -470,8 +481,7 @@ mod tests { std_fs::write(nested.join("file"), "deep").unwrap(); deduplicate_basename_wrapper(&wrapper).unwrap(); - // After one flatten, `testing/testing/file` should remain — the algorithm only - // collapses the outer wrapper exactly once. + // After one flatten testing/testing/file remains since only the outer wrapper collapses. assert_eq!(list_tree(&wrapper), vec!["testing/", "testing/file"]); } } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 0bb2c567e..e52fc7e5d 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -212,9 +212,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic }) .collect(); - // TODO: warn when the sandbox grant would include $HOME or an ancestor (default mode w/ CWD=$HOME, --dir/--here pointing at $HOME). - - // Resolve conflicts and create wrappers / open output FDs upfront, before the sandbox applies. + // Resolve conflicts and create output dirs before the sandbox applies. let mut prepared: Vec = Vec::with_capacity(files.len()); for (formats, output_file_path) in files_extensions.iter().zip(&output_file_paths) { prepared.push(prepare_decompress_target( @@ -227,7 +225,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic )?); } - // Sandbox: read on inputs, RW on prepared wrappers. RAR temp file lands inside its wrapper. + // Read on inputs and read-write on each output directory. { let mut policy = SandboxPolicy::new(); for f in &files { @@ -236,20 +234,17 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic } } for p in &prepared { - match p { - PreparedTarget::Archive { dir } => { - let canon = sandbox::canonicalize_for_sandbox(dir); - // The sandbox cannot confine writes when the target is inside $HOME. - if (output_dir_was_explicit || here) && sandbox::is_home_or_ancestor(&canon) { - warning!( - "Sandbox: extraction target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine writes", - PathFmt(&canon) - ); - } - policy.allow_read_write(canon); + if let PreparedTarget::Target { dir, .. } = p { + let canon = sandbox::canonicalize_for_sandbox(dir); + // Only --dir or --here can point the grant at $HOME + // default mode always makes a fresh subdirectory + if (output_dir_was_explicit || here) && sandbox::is_home_or_ancestor(&canon) { + warning!( + "Sandbox: extraction target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine writes", + PathFmt(&canon) + ); } - // Non-archive: held FD covers writes, no path grant needed. - PreparedTarget::NonArchive { .. } | PreparedTarget::Cancelled => {} + policy.allow_read_write(canon); } } if remove { @@ -346,6 +341,10 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic for f in &files { policy.allow_read(sandbox::canonicalize_for_sandbox(f)); } + // Grant read on each spill file so unrar can reopen it by path. + for spill in rar_spill_tempfiles.iter().flatten() { + policy.allow_read(sandbox::canonicalize_for_sandbox(spill.path())); + } policy.set_disabled(args.no_sandbox).apply(); } diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index 7ef81f27d..5f6fd5a6d 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -1,4 +1,4 @@ -//! Landlock LSM backend. Targets ABI v6 (Linux 6.12+). +//! Landlock LSM backend targeting ABI v2 use std::path::Path; @@ -10,17 +10,13 @@ use landlock::{ use super::SandboxPolicy; use crate::warning; -const TARGET_ABI: ABI = ABI::V6; +// v2 adds REFER for cross-directory hard links and flatten renames. +const TARGET_ABI: ABI = ABI::V2; pub fn apply(policy: &SandboxPolicy) { match install(policy) { - Ok(RulesetStatus::FullyEnforced) => {} - Ok(RulesetStatus::PartiallyEnforced) => { - warning!( - "Landlock sandbox is only partially enforced; this kernel does not support all of Landlock ABI v{}", - TARGET_ABI as u8 - ); - } + // Both still enforce what we need so only a missing sandbox warns. + Ok(RulesetStatus::FullyEnforced | RulesetStatus::PartiallyEnforced) => {} Ok(RulesetStatus::NotEnforced) => { warning!("Landlock LSM is not available on this system; running without filesystem sandbox"); } @@ -33,8 +29,8 @@ pub fn apply(policy: &SandboxPolicy) { fn install(policy: &SandboxPolicy) -> Result { let read_access = AccessFs::from_read(TARGET_ABI); let all_access = AccessFs::from_all(TARGET_ABI); - // REMOVE_FILE + REFER lets `--remove` delete the input archive but nothing else. - let remove_access: BitFlags = AccessFs::RemoveFile | AccessFs::Refer; + // --remove only needs REMOVE_FILE on the parent directory. + let remove_access: BitFlags = AccessFs::RemoveFile.into(); let ruleset = Ruleset::default() .set_compatibility(CompatLevel::BestEffort) @@ -48,6 +44,15 @@ fn install(policy: &SandboxPolicy) -> Result { Ok(ruleset.restrict_self()?.ruleset) } +// Drop directory-only rights on plain files or the ruleset is only partial. +fn rights_for(access: BitFlags, is_dir: bool) -> BitFlags { + if is_dir { + access + } else { + access & AccessFs::from_file(TARGET_ABI) + } +} + fn add_rules(ruleset: RulesetCreated, paths: I, access: BitFlags) -> Result where I: IntoIterator, @@ -58,7 +63,11 @@ where let path = path.as_ref(); match PathFd::new(path) { Ok(fd) => { - let rule = PathBeneath::new(fd, access).set_compatibility(CompatLevel::BestEffort); + let granted = rights_for(access, path.is_dir()); + if granted.is_empty() { + continue; + } + let rule = PathBeneath::new(fd, granted).set_compatibility(CompatLevel::BestEffort); current = current.add_rule(rule)?; } Err(err) => { @@ -74,8 +83,18 @@ mod tests { use super::*; #[test] - fn target_abi_is_v6() { - assert_eq!(TARGET_ABI as u8, ABI::V6 as u8); + fn target_abi_is_v2() { + assert_eq!(TARGET_ABI as u8, ABI::V2 as u8); + } + + #[test] + fn file_rights_drop_directory_only_access() { + let on_file = rights_for(AccessFs::from_read(TARGET_ABI), false); + assert!(on_file.contains(AccessFs::ReadFile)); + assert!(!on_file.contains(AccessFs::ReadDir)); + + let on_dir = rights_for(AccessFs::from_read(TARGET_ABI), true); + assert!(on_dir.contains(AccessFs::ReadDir)); } #[test] diff --git a/tests/snapshots/ui__ui_test_ok_decompress.snap b/tests/snapshots/ui__ui_test_ok_decompress.snap index cb36ed973..2afbe2390 100644 --- a/tests/snapshots/ui__ui_test_ok_decompress.snap +++ b/tests/snapshots/ui__ui_test_ok_decompress.snap @@ -2,6 +2,6 @@ source: tests/ui.rs expression: "run_ouch(\"ouch decompress output.zst\", dir)" --- -[INFO] File "output.zst" decompressed to "output" +[INFO] File "output.zst" decompressed to "output/output" [INFO] Input file size: [SIZE] [INFO] Output file size: [SIZE] From 10d5fe4f2618ce4d091e996e273f547b79dcd520 Mon Sep 17 00:00:00 2001 From: valoq Date: Mon, 8 Jun 2026 12:52:14 +0200 Subject: [PATCH 05/16] cleanup --- src/archive/tar.rs | 2 +- src/commands/decompress.rs | 2 +- src/sandbox/landlock.rs | 11 +++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 5d6edb811..3e132dc28 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -55,7 +55,7 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result { let full_link_path = output_folder.join(&link_path); let full_target_path = output_folder.join(&target); - // Cross-directory hard links need REFER which old kernels deny so copy instead. + // Fall back to copy when the hard link cannot be created. if let Err(err) = fs::hard_link(&full_target_path, &full_link_path) { info!( "Could not hard link {} ({err}); copying instead", diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index c6c985eee..88997dabb 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -252,7 +252,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { // ended up containing exactly one entry whose name matches the wrapper itself // (e.g. `archive.zip` contained a single `archive/` root), flatten that // duplicate so the user sees `./archive/...` not `./archive/archive/...`. - // Best effort because the cross-directory rename needs REFER which old kernels lack. + // Leave the nested layout if the rename fails. if !options.output_dir_was_explicit && !options.here { let _ = deduplicate_basename_wrapper(&output_path); } diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index 5f6fd5a6d..f81539441 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -15,11 +15,10 @@ const TARGET_ABI: ABI = ABI::V2; pub fn apply(policy: &SandboxPolicy) { match install(policy) { - // Both still enforce what we need so only a missing sandbox warns. - Ok(RulesetStatus::FullyEnforced | RulesetStatus::PartiallyEnforced) => {} Ok(RulesetStatus::NotEnforced) => { warning!("Landlock LSM is not available on this system; running without filesystem sandbox"); } + Ok(_) => {} Err(err) => { warning!("Failed to enable Landlock sandbox: {err}; running unrestricted"); } @@ -33,7 +32,7 @@ fn install(policy: &SandboxPolicy) -> Result { let remove_access: BitFlags = AccessFs::RemoveFile.into(); let ruleset = Ruleset::default() - .set_compatibility(CompatLevel::BestEffort) + .set_compatibility(CompatLevel::SoftRequirement) .handle_access(all_access)? .create()?; @@ -44,7 +43,7 @@ fn install(policy: &SandboxPolicy) -> Result { Ok(ruleset.restrict_self()?.ruleset) } -// Drop directory-only rights on plain files or the ruleset is only partial. +// Drop directory-only rights on plain files so the file rule is fully enforced. fn rights_for(access: BitFlags, is_dir: bool) -> BitFlags { if is_dir { access @@ -67,7 +66,7 @@ where if granted.is_empty() { continue; } - let rule = PathBeneath::new(fd, granted).set_compatibility(CompatLevel::BestEffort); + let rule = PathBeneath::new(fd, granted).set_compatibility(CompatLevel::SoftRequirement); current = current.add_rule(rule)?; } Err(err) => { @@ -99,7 +98,7 @@ mod tests { #[test] fn install_with_empty_policy_does_not_error() { - // Even on kernels without Landlock support, BestEffort means install() + // Even on kernels without Landlock support, SoftRequirement means install() // returns Ok with NotEnforced rather than Err. let policy = SandboxPolicy::new(); let result = install(&policy); From 9ee7992483827701161684015b0777002f5b5be1 Mon Sep 17 00:00:00 2001 From: valoq Date: Mon, 8 Jun 2026 20:55:50 +0200 Subject: [PATCH 06/16] improve patch --- CHANGELOG.md | 1 + src/commands/decompress.rs | 8 +++-- src/commands/mod.rs | 74 ++++++++++++++++++++------------------ src/sandbox/mod.rs | 5 +++ tests/integration.rs | 74 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d48c872..8ece5f7a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Categories Used: - Add `--here` flag to unpack into current directory (https://github.com/ouch-org/ouch/pull/962). - List: show symlink targets (for tar and zip) (https://github.com/ouch-org/ouch/pull/934) - Add aliases for ebooks (`.epub`) (https://github.com/ouch-org/ouch/pull/981) +- Add a Landlock filesystem sandbox on Linux for `compress`/`decompress`/`list` (disable with `--no-sandbox`) (https://github.com/ouch-org/ouch/pull/995) ### Improvements diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 88997dabb..522ef2059 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -61,12 +61,14 @@ pub fn prepare_decompress_target( let (first_extension, _) = split_first_compression_format(formats); let is_archive = matches!(first_extension, Tar | Zip | Rar | SevenZip); - // --dir and --here use output_dir directly - // default mode wraps it in a basename subdirectory + // --dir and --here unpack into output_dir as is + // default mode makes a new directory named after the archive so report.txt.gz becomes report/report.txt let target = if output_dir_was_explicit || here { output_dir.to_path_buf() - } else { + } else if is_archive { output_file_path.to_path_buf() + } else { + output_file_path.with_extension("") }; let is_cwd = target == *INITIAL_CURRENT_DIR; let is_valid = is_cwd || !target.fs_err_try_exists()? || (target.is_dir() && target.read_dir()?.next().is_none()); diff --git a/src/commands/mod.rs b/src/commands/mod.rs index e52fc7e5d..c24b78cf1 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -53,13 +53,8 @@ fn warn_user_about_loading_sevenz_in_memory() { /// /// There are a lot of custom errors to give enough error description and explanation. pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_policy: FileVisibilityPolicy) -> Result<()> { - // Skip --threads 0 ("auto") to match cli::mod.rs, and propagate pool-init errors - if let Some(threads) = args.threads.filter(|&t| t != 0) { - rayon::ThreadPoolBuilder::new() - .num_threads(threads) - .build_global() - .map_err(|e| FinalError::with_title("Failed to initialize thread pool").detail(e.to_string()))?; - } + // No global pool here because Landlock only confines threads created after it is applied + // Decompression builds its pool after the sandbox so the workers are confined match args.cmd { Subcommand::Compress { @@ -262,35 +257,44 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic policy.set_disabled(args.no_sandbox).apply(); } - files - .par_iter() - .zip(files_extensions) - .zip(prepared) - .try_for_each(|((input_path, formats), prepared)| { - decompress_file(DecompressOptions { - input_file_path: input_path, - formats, - output_dir: &output_dir, - output_dir_was_explicit, - here, - question_policy, - password: args.password.as_deref().map(|str| { - <[u8] as ByteSlice>::from_os_str(str).expect("convert password to bytes failed") - }), - remove, - prepared, - }) - .map_err(|err| match err { - Error::IoError { reason } => Error::Custom { - reason: FinalError::with_title(format!( - "Failed to decompress {}", - NoQuotePathFmt(input_path) - )) - .detail(reason), - }, - other => other, + // Build the pool after the sandbox so its worker threads are confined by it + // --threads 0 means auto and lets rayon pick one thread per core + let pool = rayon::ThreadPoolBuilder::new() + .num_threads(args.threads.filter(|&t| t != 0).unwrap_or(0)) + .build() + .map_err(|e| FinalError::with_title("Failed to initialize thread pool").detail(e.to_string()))?; + + pool.install(move || { + files + .par_iter() + .zip(files_extensions) + .zip(prepared) + .try_for_each(|((input_path, formats), prepared)| { + decompress_file(DecompressOptions { + input_file_path: input_path, + formats, + output_dir: &output_dir, + output_dir_was_explicit, + here, + question_policy, + password: args.password.as_deref().map(|str| { + <[u8] as ByteSlice>::from_os_str(str).expect("convert password to bytes failed") + }), + remove, + prepared, + }) + .map_err(|err| match err { + Error::IoError { reason } => Error::Custom { + reason: FinalError::with_title(format!( + "Failed to decompress {}", + NoQuotePathFmt(input_path) + )) + .detail(reason), + }, + other => other, + }) }) - }) + }) } Subcommand::List { archives: files, tree } => { let mut formats = vec![]; diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 4221f573a..0236d4f46 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -58,8 +58,13 @@ impl SandboxPolicy { if self.disabled || std::env::var_os("OUCH_NO_SANDBOX").is_some() { return; } + // On Linux the backend warns instead of failing when Landlock is missing and keeps going #[cfg(target_os = "linux")] landlock::apply(self); + + // The sandbox is Linux only so elsewhere there is nothing to enforce and we just log it + #[cfg(not(target_os = "linux"))] + crate::info!("Filesystem sandbox is only available on Linux; continuing without it"); } } diff --git a/tests/integration.rs b/tests/integration.rs index 4c30533b7..3e0a9a954 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1569,3 +1569,77 @@ fn decompress_concatenated_lz4_frames() { encoder.finish().unwrap() }); } + +// Default mode decompression without --dir or --here is not tested elsewhere +// These lock in the layout where output goes into a new directory named after the archive + +#[test] +fn decompress_single_file_default_mode_creates_named_directory() { + let (_tempdir, dir) = testdir().unwrap(); + + fs::write(dir.join("report.txt"), b"hello").unwrap(); + crate::utils::cargo_bin() + .args(["compress", "report.txt", "report.txt.gz", "--yes"]) + .current_dir(dir) + .assert() + .success(); + fs::remove_file(dir.join("report.txt")).unwrap(); + + crate::utils::cargo_bin() + .args(["decompress", "report.txt.gz", "--yes"]) + .current_dir(dir) + .assert() + .success(); + + // report.txt.gz becomes report/report.txt so the directory has no extension and the file keeps it + assert!(dir.join("report").is_dir()); + assert_eq!("hello", fs::read_to_string(dir.join("report").join("report.txt")).unwrap()); + assert!(!dir.join("report.txt").is_dir()); +} + +#[test] +fn decompress_archive_default_mode_creates_named_directory() { + let (_tempdir, dir) = testdir().unwrap(); + + fs::create_dir(dir.join("payload")).unwrap(); + fs::write(dir.join("payload").join("a.txt"), b"a").unwrap(); + crate::utils::cargo_bin() + .args(["compress", "payload", "bundle.tar", "--yes"]) + .current_dir(dir) + .assert() + .success(); + + crate::utils::cargo_bin() + .args(["decompress", "bundle.tar", "--yes"]) + .current_dir(dir) + .assert() + .success(); + + assert!(dir.join("bundle").is_dir()); + assert_eq!("a", fs::read_to_string(dir.join("bundle").join("payload").join("a.txt")).unwrap()); +} + +// An archive whose only entry matches the wrapper name must not nest +// test.zip with a test/ folder must unpack to test/ and not test/test/ +#[test] +fn decompress_archive_default_mode_does_not_duplicate_directory() { + let (_tempdir, dir) = testdir().unwrap(); + + fs::create_dir(dir.join("test")).unwrap(); + fs::write(dir.join("test").join("inner.txt"), b"y").unwrap(); + crate::utils::cargo_bin() + .args(["compress", "test", "test.zip", "--yes"]) + .current_dir(dir) + .assert() + .success(); + fs::remove_dir_all(dir.join("test")).unwrap(); + + crate::utils::cargo_bin() + .args(["decompress", "test.zip", "--yes"]) + .current_dir(dir) + .assert() + .success(); + + assert!(dir.join("test").join("inner.txt").is_file()); + assert!(!dir.join("test").join("test").exists()); +} From 4ea2db7a6b14895bbe557b023beda7d08990f389 Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 9 Jun 2026 01:38:45 +0200 Subject: [PATCH 07/16] improve patch --- src/commands/mod.rs | 7 ++++++ src/sandbox/landlock.rs | 55 ++++++++++++++++------------------------- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index c24b78cf1..68092b43a 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -249,6 +249,13 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic } let canon = sandbox::canonicalize_for_sandbox(f); if let Some(parent) = canon.parent() { + // Warn when the deletable directory is $HOME or an ancestor of it. + if sandbox::is_home_or_ancestor(parent) { + warning!( + "Sandbox: removal target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine removals", + PathFmt(parent) + ); + } // The decompressor can delete the input archive but not write nearby. policy.allow_remove_in(parent.to_path_buf()); } diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index f81539441..61ec9fb7c 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -1,38 +1,34 @@ -//! Landlock LSM backend targeting ABI v2 +//! Landlock filesystem sandbox backend. use std::path::Path; use landlock::{ - ABI, Access, AccessFs, BitFlags, CompatLevel, Compatible, PathBeneath, PathFd, Ruleset, RulesetAttr, - RulesetCreated, RulesetCreatedAttr, RulesetError, RulesetStatus, + ABI, Access, AccessFs, BitFlags, CompatLevel, Compatible, PathBeneath, PathFd, Ruleset, + RulesetAttr, RulesetCreated, RulesetCreatedAttr, RulesetError, }; use super::SandboxPolicy; use crate::warning; -// v2 adds REFER for cross-directory hard links and flatten renames. -const TARGET_ABI: ABI = ABI::V2; +// Landlock ABI v5 is the floor (Linux 6.10). +// It covers every filesystem right the sandbox needs. +const ABI_FLOOR: ABI = ABI::V5; pub fn apply(policy: &SandboxPolicy) { - match install(policy) { - Ok(RulesetStatus::NotEnforced) => { - warning!("Landlock LSM is not available on this system; running without filesystem sandbox"); - } - Ok(_) => {} - Err(err) => { - warning!("Failed to enable Landlock sandbox: {err}; running unrestricted"); - } + if install(policy).is_err() { + warning!("Sandbox: Landlock ABI v5 (Linux 6.10 or newer) is required; running without a filesystem sandbox"); } } -fn install(policy: &SandboxPolicy) -> Result { - let read_access = AccessFs::from_read(TARGET_ABI); - let all_access = AccessFs::from_all(TARGET_ABI); +fn install(policy: &SandboxPolicy) -> Result<(), RulesetError> { + let read_access = AccessFs::from_read(ABI_FLOOR); + let all_access = AccessFs::from_all(ABI_FLOOR); // --remove only needs REMOVE_FILE on the parent directory. let remove_access: BitFlags = AccessFs::RemoveFile.into(); + // HardRequirement rejects the sandbox unless the kernel supports the floor. let ruleset = Ruleset::default() - .set_compatibility(CompatLevel::SoftRequirement) + .set_compatibility(CompatLevel::HardRequirement) .handle_access(all_access)? .create()?; @@ -40,7 +36,8 @@ fn install(policy: &SandboxPolicy) -> Result { let ruleset = add_rules(ruleset, policy.read_write_paths(), all_access)?; let ruleset = add_rules(ruleset, policy.remove_in_paths(), remove_access)?; - Ok(ruleset.restrict_self()?.ruleset) + let _ = ruleset.restrict_self()?; + Ok(()) } // Drop directory-only rights on plain files so the file rule is fully enforced. @@ -48,7 +45,7 @@ fn rights_for(access: BitFlags, is_dir: bool) -> BitFlags { if is_dir { access } else { - access & AccessFs::from_file(TARGET_ABI) + access & AccessFs::from_file(ABI_FLOOR) } } @@ -66,8 +63,7 @@ where if granted.is_empty() { continue; } - let rule = PathBeneath::new(fd, granted).set_compatibility(CompatLevel::SoftRequirement); - current = current.add_rule(rule)?; + current = current.add_rule(PathBeneath::new(fd, granted))?; } Err(err) => { warning!("Sandbox: cannot open {}: {err}", path.display()); @@ -82,26 +78,17 @@ mod tests { use super::*; #[test] - fn target_abi_is_v2() { - assert_eq!(TARGET_ABI as u8, ABI::V2 as u8); + fn enforced_access_includes_truncate() { + assert!(AccessFs::from_all(ABI_FLOOR).contains(AccessFs::Truncate)); } #[test] fn file_rights_drop_directory_only_access() { - let on_file = rights_for(AccessFs::from_read(TARGET_ABI), false); + let on_file = rights_for(AccessFs::from_read(ABI_FLOOR), false); assert!(on_file.contains(AccessFs::ReadFile)); assert!(!on_file.contains(AccessFs::ReadDir)); - let on_dir = rights_for(AccessFs::from_read(TARGET_ABI), true); + let on_dir = rights_for(AccessFs::from_read(ABI_FLOOR), true); assert!(on_dir.contains(AccessFs::ReadDir)); } - - #[test] - fn install_with_empty_policy_does_not_error() { - // Even on kernels without Landlock support, SoftRequirement means install() - // returns Ok with NotEnforced rather than Err. - let policy = SandboxPolicy::new(); - let result = install(&policy); - assert!(result.is_ok(), "install returned {result:?}"); - } } From 10d9a16ca079bc040b6e99b36eef7ff3a287d7ed Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 9 Jun 2026 15:38:44 +0200 Subject: [PATCH 08/16] improve patch --- CHANGELOG.md | 2 +- src/commands/list.rs | 11 +++- src/commands/mod.rs | 42 +++++++++------ src/sandbox/landlock.rs | 112 ++++++++++++++++++++++++++++++---------- src/sandbox/mod.rs | 21 ++++++++ 5 files changed, 141 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ece5f7a6..21406767e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ Categories Used: - Add `--here` flag to unpack into current directory (https://github.com/ouch-org/ouch/pull/962). - List: show symlink targets (for tar and zip) (https://github.com/ouch-org/ouch/pull/934) - Add aliases for ebooks (`.epub`) (https://github.com/ouch-org/ouch/pull/981) -- Add a Landlock filesystem sandbox on Linux for `compress`/`decompress`/`list` (disable with `--no-sandbox`) (https://github.com/ouch-org/ouch/pull/995) +- Add a Landlock sandbox on Linux for `compress`/`decompress`/`list` that restricts filesystem, network, and IPC access (disable with `--no-sandbox`) (https://github.com/ouch-org/ouch/pull/995) ### Improvements diff --git a/src/commands/list.rs b/src/commands/list.rs index 32bfd5097..e14451c31 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -102,8 +102,15 @@ pub fn list_archive_contents( #[cfg(feature = "unrar")] Rar => { if formats.len() > 1 { - // Reuse the tempfile pre-opened by the caller if there is one. - let mut temp_file = rar_spill_tempfile.unwrap_or(tempfile::NamedTempFile::new()?); + // The caller pre-opens this tempfile before the sandbox starts. + let mut temp_file = match rar_spill_tempfile { + Some(temp_file) => temp_file, + None => { + return Err(crate::error::FinalError::with_title("Failed to list RAR archive") + .detail("the sandbox spill tempfile was not prepared before lockdown") + .into()); + } + }; io::copy(&mut reader, &mut temp_file)?; Box::new(crate::archive::rar::list_archive(temp_file.path(), password)?) } else { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 68092b43a..7005869a7 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -222,6 +222,8 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // Read on inputs and read-write on each output directory. { + let sandbox_enforced = sandbox::sandbox_enforced(args.no_sandbox); + let mut policy = SandboxPolicy::new(); for f in &files { if !is_path_stdin(f) { @@ -233,7 +235,8 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic let canon = sandbox::canonicalize_for_sandbox(dir); // Only --dir or --here can point the grant at $HOME // default mode always makes a fresh subdirectory - if (output_dir_was_explicit || here) && sandbox::is_home_or_ancestor(&canon) { + if sandbox_enforced && (output_dir_was_explicit || here) && sandbox::is_home_or_ancestor(&canon) + { warning!( "Sandbox: extraction target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine writes", PathFmt(&canon) @@ -243,23 +246,31 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic } } if remove { + // Collect unique parents so each directory grants and warns once. + let mut parents: Vec = Vec::new(); for f in &files { if is_path_stdin(f) { continue; } let canon = sandbox::canonicalize_for_sandbox(f); if let Some(parent) = canon.parent() { - // Warn when the deletable directory is $HOME or an ancestor of it. - if sandbox::is_home_or_ancestor(parent) { - warning!( - "Sandbox: removal target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine removals", - PathFmt(parent) - ); + let parent = parent.to_path_buf(); + if !parents.contains(&parent) { + parents.push(parent); } - // The decompressor can delete the input archive but not write nearby. - policy.allow_remove_in(parent.to_path_buf()); } } + for parent in parents { + // The grant covers the whole directory so warn whenever the sandbox runs. + if sandbox_enforced { + warning!( + "Sandbox: the extractor can delete any file in {}, not just the archive", + PathFmt(&parent) + ); + } + // The decompressor can delete the input archive but not write nearby. + policy.allow_remove_in(parent); + } } policy.set_disabled(args.no_sandbox).apply(); } @@ -272,11 +283,8 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic .map_err(|e| FinalError::with_title("Failed to initialize thread pool").detail(e.to_string()))?; pool.install(move || { - files - .par_iter() - .zip(files_extensions) - .zip(prepared) - .try_for_each(|((input_path, formats), prepared)| { + files.par_iter().zip(files_extensions).zip(prepared).try_for_each( + |((input_path, formats), prepared)| { decompress_file(DecompressOptions { input_file_path: input_path, formats, @@ -300,7 +308,8 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic }, other => other, }) - }) + }, + ) }) } Subcommand::List { archives: files, tree } => { @@ -336,10 +345,11 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic .map(|f| extension::flatten_compression_formats(f)) .collect(); // Open RAR spill tempfiles up front so unrar can read from them under the sandbox. + // The archive format sits at the front so match there not at the end. let mut rar_spill_tempfiles: Vec> = flat_formats .iter() .map(|fs| { - if fs.len() > 1 && matches!(fs.last(), Some(extension::CompressionFormat::Rar)) { + if fs.len() > 1 && matches!(fs.first(), Some(extension::CompressionFormat::Rar)) { tempfile::NamedTempFile::new().ok() } else { None diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index 61ec9fb7c..8e90c28e2 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -3,49 +3,68 @@ use std::path::Path; use landlock::{ - ABI, Access, AccessFs, BitFlags, CompatLevel, Compatible, PathBeneath, PathFd, Ruleset, - RulesetAttr, RulesetCreated, RulesetCreatedAttr, RulesetError, + ABI, Access, AccessFs, AccessNet, BitFlags, CompatLevel, Compatible, PathBeneath, PathFd, Ruleset, RulesetAttr, + RulesetCreated, RulesetCreatedAttr, RulesetError, Scope, }; use super::SandboxPolicy; use crate::warning; -// Landlock ABI v5 is the floor (Linux 6.10). -// It covers every filesystem right the sandbox needs. -const ABI_FLOOR: ABI = ABI::V5; +// Landlock ABI v6 is required (Linux 6.12). +// It covers filesystem and network rights and IPC scoping. +const REQUIRED_ABI: ABI = ABI::V6; pub fn apply(policy: &SandboxPolicy) { if install(policy).is_err() { - warning!("Sandbox: Landlock ABI v5 (Linux 6.10 or newer) is required; running without a filesystem sandbox"); + warning!("Sandbox: Landlock ABI v6 (Linux 6.12 or newer) is required; running without a sandbox"); } } -fn install(policy: &SandboxPolicy) -> Result<(), RulesetError> { - let read_access = AccessFs::from_read(ABI_FLOOR); - let all_access = AccessFs::from_all(ABI_FLOOR); - // --remove only needs REMOVE_FILE on the parent directory. - let remove_access: BitFlags = AccessFs::RemoveFile.into(); +// True when the running kernel supports the required Landlock ABI. +pub fn available() -> bool { + base_ruleset().is_ok() +} - // HardRequirement rejects the sandbox unless the kernel supports the floor. - let ruleset = Ruleset::default() +// Handle filesystem and network rights and deny every IPC scope. +// HardRequirement rejects the sandbox unless the kernel supports all of it. +fn base_ruleset() -> Result { + Ruleset::default() .set_compatibility(CompatLevel::HardRequirement) - .handle_access(all_access)? - .create()?; + .handle_access(AccessFs::from_all(REQUIRED_ABI))? + .handle_access(AccessNet::from_all(REQUIRED_ABI))? + .scope(Scope::from_all(REQUIRED_ABI))? + .create() +} + +// Rights an extractor needs inside an output directory. +// It never grants execution or device nodes or device ioctls. +fn write_grant() -> BitFlags { + let denied = AccessFs::Execute | AccessFs::IoctlDev | AccessFs::MakeChar | AccessFs::MakeBlock; + AccessFs::from_all(REQUIRED_ABI) & !denied +} - let ruleset = add_rules(ruleset, policy.read_paths(), read_access)?; - let ruleset = add_rules(ruleset, policy.read_write_paths(), all_access)?; - let ruleset = add_rules(ruleset, policy.remove_in_paths(), remove_access)?; +// Read only rights for input archives. Execution is never granted. +fn read_grant() -> BitFlags { + AccessFs::from_read(REQUIRED_ABI) & !AccessFs::Execute +} +fn install(policy: &SandboxPolicy) -> Result<(), RulesetError> { + let ruleset = base_ruleset()?; + let ruleset = add_rules(ruleset, policy.read_paths(), read_grant())?; + let ruleset = add_rules(ruleset, policy.read_write_paths(), write_grant())?; + let ruleset = add_rules(ruleset, policy.remove_in_paths(), AccessFs::RemoveFile.into())?; + // No network rules and no scope exceptions are added. + // So nothing can open TCP or reach abstract sockets or signal outside processes. let _ = ruleset.restrict_self()?; Ok(()) } -// Drop directory-only rights on plain files so the file rule is fully enforced. +// Drop directory only rights on plain files so the file rule is fully enforced. fn rights_for(access: BitFlags, is_dir: bool) -> BitFlags { if is_dir { access } else { - access & AccessFs::from_file(ABI_FLOOR) + access & AccessFs::from_file(REQUIRED_ABI) } } @@ -78,17 +97,54 @@ mod tests { use super::*; #[test] - fn enforced_access_includes_truncate() { - assert!(AccessFs::from_all(ABI_FLOOR).contains(AccessFs::Truncate)); + fn grants_never_allow_execution() { + assert!(!read_grant().contains(AccessFs::Execute)); + assert!(!write_grant().contains(AccessFs::Execute)); } #[test] - fn file_rights_drop_directory_only_access() { - let on_file = rights_for(AccessFs::from_read(ABI_FLOOR), false); - assert!(on_file.contains(AccessFs::ReadFile)); - assert!(!on_file.contains(AccessFs::ReadDir)); + fn write_grant_denies_devices_and_ioctl() { + let g = write_grant(); + assert!(!g.contains(AccessFs::IoctlDev)); + assert!(!g.contains(AccessFs::MakeChar)); + assert!(!g.contains(AccessFs::MakeBlock)); + } - let on_dir = rights_for(AccessFs::from_read(ABI_FLOOR), true); - assert!(on_dir.contains(AccessFs::ReadDir)); + #[test] + fn write_grant_allows_normal_extraction() { + let g = write_grant(); + for right in [ + AccessFs::WriteFile, + AccessFs::MakeReg, + AccessFs::MakeDir, + AccessFs::MakeSym, + AccessFs::Refer, + AccessFs::Truncate, + ] { + assert!(g.contains(right)); + } + } + + #[test] + fn read_grant_is_read_only() { + let g = read_grant(); + assert!(g.contains(AccessFs::ReadFile)); + assert!(g.contains(AccessFs::ReadDir)); + assert!(!g.contains(AccessFs::WriteFile)); + assert!(!g.contains(AccessFs::RemoveFile)); + } + + #[test] + fn network_access_is_handled() { + let net = AccessNet::from_all(REQUIRED_ABI); + assert!(net.contains(AccessNet::BindTcp)); + assert!(net.contains(AccessNet::ConnectTcp)); + } + + #[test] + fn ipc_scopes_are_handled() { + let scopes = Scope::from_all(REQUIRED_ABI); + assert!(scopes.contains(Scope::AbstractUnixSocket)); + assert!(scopes.contains(Scope::Signal)); } } diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 0236d4f46..2990b502d 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -94,6 +94,22 @@ pub fn is_home_or_ancestor(target: &Path) -> bool { home == target || home.starts_with(&target) } +/// True when the sandbox will actually be enforced for this run. +/// False when disabled or off Linux or the kernel lacks Landlock. +pub fn sandbox_enforced(disabled: bool) -> bool { + if disabled || std::env::var_os("OUCH_NO_SANDBOX").is_some() { + return false; + } + #[cfg(target_os = "linux")] + { + landlock::available() + } + #[cfg(not(target_os = "linux"))] + { + false + } +} + #[cfg(test)] mod tests { use super::*; @@ -120,6 +136,11 @@ mod tests { assert!(policy.disabled); } + #[test] + fn sandbox_not_enforced_when_disabled() { + assert!(!sandbox_enforced(true)); + } + #[test] fn canonicalize_existing_path_resolves() { let dir = tempfile::tempdir().unwrap(); From 85489deee46a09142860228c760684b803b1ca1e Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 9 Jun 2026 17:29:12 +0200 Subject: [PATCH 09/16] improve patch --- src/commands/mod.rs | 90 +++++++++++++++++++++++++---------------- src/sandbox/landlock.rs | 55 +++++++++++++++++++++---- src/sandbox/mod.rs | 40 +++++++----------- 3 files changed, 118 insertions(+), 67 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 7005869a7..7cecbdba7 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -70,6 +70,17 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic return Err(FinalError::with_title("No files to compress").into()); } + // Following symlinks can read targets outside the sandboxed input set. + // Abort early with a clear message instead of failing midway under Landlock. + if follow_symlinks && !args.no_sandbox && cfg!(target_os = "linux") { + return Err( + FinalError::with_title("Cannot follow symlinks while the sandbox is active") + .detail("a symlink target may point outside the files being compressed") + .hint("re-run with --no-sandbox to follow symlinks") + .into(), + ); + } + // Formats from path extension, like "file.tar.gz.xz" -> vec![Tar, Gzip, Lzma] let (formats_from_flag, formats) = match args.format { Some(formats) => { @@ -96,14 +107,14 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic None => return Ok(()), }; - { - // Read on inputs; the output FD is held already, no write-path grant needed. + // Read on inputs. The output FD is held already so no write-path grant is needed. + let sandbox_active = { let mut policy = SandboxPolicy::new(); for f in &files { policy.allow_read(sandbox::canonicalize_for_sandbox(f)); } - policy.set_disabled(args.no_sandbox).apply(); - } + policy.set_disabled(args.no_sandbox).apply() + }; let level = if fast { Some(1) // Lowest level of compression @@ -128,14 +139,17 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic info_accessible!("Output file size: {}", BytesFmt(file_size(&output_path)?)); info_accessible!("Successfully compressed to {}", PathFmt(&output_path)); } else if compress_result.is_err() { - // Sandbox blocks unlinking the partial file; tell the user to delete it. - eprintln!("{red}FATAL ERROR:\n", red = *colors::RED); - eprintln!( - " Compression did not finish; the partial file at {} may be corrupted.", - PathFmt(&output_path) - ); - eprintln!(" Please delete it manually."); - eprintln!(" Compression failed for reasons below."); + // The sandbox blocks unlinking the partial file so only auto-delete when it is off. + let deleted = !sandbox_active && utils::remove_file_or_dir(&output_path).is_ok(); + if !deleted { + eprintln!("{red}FATAL ERROR:\n", red = *colors::RED); + eprintln!( + " Compression did not finish; the partial file at {} may be corrupted.", + PathFmt(&output_path) + ); + eprintln!(" Please delete it manually."); + eprintln!(" Compression failed for reasons below."); + } } // Ok(false) means the user cancelled before any data was written; nothing to report. @@ -222,32 +236,31 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // Read on inputs and read-write on each output directory. { - let sandbox_enforced = sandbox::sandbox_enforced(args.no_sandbox); - let mut policy = SandboxPolicy::new(); for f in &files { if !is_path_stdin(f) { policy.allow_read(sandbox::canonicalize_for_sandbox(f)); } } + + // Collect the directories the warnings refer to while building the policy. + // The warnings are printed only when the sandbox is actually enforced. + let mut home_targets: Vec = Vec::new(); for p in &prepared { if let PreparedTarget::Target { dir, .. } = p { let canon = sandbox::canonicalize_for_sandbox(dir); // Only --dir or --here can point the grant at $HOME // default mode always makes a fresh subdirectory - if sandbox_enforced && (output_dir_was_explicit || here) && sandbox::is_home_or_ancestor(&canon) - { - warning!( - "Sandbox: extraction target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine writes", - PathFmt(&canon) - ); + if (output_dir_was_explicit || here) && sandbox::is_home_or_ancestor(&canon) { + home_targets.push(canon.clone()); } policy.allow_read_write(canon); } } + + let mut remove_parents: Vec = Vec::new(); if remove { // Collect unique parents so each directory grants and warns once. - let mut parents: Vec = Vec::new(); for f in &files { if is_path_stdin(f) { continue; @@ -255,24 +268,33 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic let canon = sandbox::canonicalize_for_sandbox(f); if let Some(parent) = canon.parent() { let parent = parent.to_path_buf(); - if !parents.contains(&parent) { - parents.push(parent); + if !remove_parents.contains(&parent) { + remove_parents.push(parent); } } } - for parent in parents { - // The grant covers the whole directory so warn whenever the sandbox runs. - if sandbox_enforced { - warning!( - "Sandbox: the extractor can delete any file in {}, not just the archive", - PathFmt(&parent) - ); - } - // The decompressor can delete the input archive but not write nearby. - policy.allow_remove_in(parent); + // The decompressor can delete the input archive but not write nearby. + for parent in &remove_parents { + policy.allow_remove_in(parent.clone()); + } + } + + // Apply once and warn only about what a real sandbox cannot confine. + let enforced = policy.set_disabled(args.no_sandbox).apply(); + if enforced { + for target in home_targets { + warning!( + "Sandbox: extraction target {} is $HOME or an ancestor of it; the sandbox cannot meaningfully confine writes", + PathFmt(&target) + ); + } + for parent in remove_parents { + warning!( + "Sandbox: the extractor can delete any file in {}, not just the archive", + PathFmt(&parent) + ); } } - policy.set_disabled(args.no_sandbox).apply(); } // Build the pool after the sandbox so its worker threads are confined by it diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index 8e90c28e2..b21962a5e 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -14,17 +14,16 @@ use crate::warning; // It covers filesystem and network rights and IPC scoping. const REQUIRED_ABI: ABI = ABI::V6; -pub fn apply(policy: &SandboxPolicy) { - if install(policy).is_err() { - warning!("Sandbox: Landlock ABI v6 (Linux 6.12 or newer) is required; running without a sandbox"); +pub fn apply(policy: &SandboxPolicy) -> bool { + match install(policy) { + Ok(()) => true, + Err(_) => { + warning!("Sandbox: Landlock ABI v6 (Linux 6.12 or newer) is required; running without a sandbox"); + false + } } } -// True when the running kernel supports the required Landlock ABI. -pub fn available() -> bool { - base_ruleset().is_ok() -} - // Handle filesystem and network rights and deny every IPC scope. // HardRequirement rejects the sandbox unless the kernel supports all of it. fn base_ruleset() -> Result { @@ -147,4 +146,44 @@ mod tests { assert!(scopes.contains(Scope::AbstractUnixSocket)); assert!(scopes.contains(Scope::Signal)); } + + // Real enforcement check. Runs only where the kernel provides the required ABI. + // The ruleset and paths are built before the fork so the child does almost no work. + #[test] + fn landlock_denies_writes_outside_granted_dir() { + use std::{ffi::CString, os::unix::ffi::OsStrExt}; + + // Skip when the kernel cannot create the required ruleset. + if base_ruleset().is_err() { + return; + } + + let granted = tempfile::tempdir().unwrap(); + let outside_dir = tempfile::tempdir().unwrap(); + let inside_c = CString::new(granted.path().join("inside.txt").as_os_str().as_bytes()).unwrap(); + let outside_c = CString::new(outside_dir.path().join("escape.txt").as_os_str().as_bytes()).unwrap(); + + // Grant read-write on the one directory and build the ruleset in the parent. + let ruleset = add_rules(base_ruleset().unwrap(), [granted.path()], write_grant()).unwrap(); + + // The child only restricts itself and opens two paths then exits. + let pid = unsafe { libc::fork() }; + if pid == 0 { + let enforced = ruleset.restrict_self().is_ok(); + let inside_fd = unsafe { libc::open(inside_c.as_ptr(), libc::O_CREAT | libc::O_WRONLY, 0o600) }; + let outside_fd = unsafe { libc::open(outside_c.as_ptr(), libc::O_CREAT | libc::O_WRONLY, 0o600) }; + // Inside the grant must succeed and outside must be denied. + let ok = enforced && inside_fd >= 0 && outside_fd < 0; + unsafe { libc::_exit(if ok { 0 } else { 1 }) }; + } + assert!(pid > 0, "fork failed"); + let mut status = 0; + unsafe { libc::waitpid(pid, &mut status, 0) }; + assert!(libc::WIFEXITED(status), "the sandboxed child did not exit normally"); + assert_eq!( + libc::WEXITSTATUS(status), + 0, + "a write outside the granted directory was not denied by Landlock" + ); + } } diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 2990b502d..3a0553b93 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -52,19 +52,23 @@ impl SandboxPolicy { &self.remove_in } - /// Apply the policy. Failures emit a warning and let the process run - /// unrestricted. - pub fn apply(&self) { + /// Apply the policy and return whether the sandbox is now enforced. + /// Failures emit a warning and let the process run unrestricted. + pub fn apply(&self) -> bool { if self.disabled || std::env::var_os("OUCH_NO_SANDBOX").is_some() { - return; + return false; } // On Linux the backend warns instead of failing when Landlock is missing and keeps going #[cfg(target_os = "linux")] - landlock::apply(self); - + { + landlock::apply(self) + } // The sandbox is Linux only so elsewhere there is nothing to enforce and we just log it #[cfg(not(target_os = "linux"))] - crate::info!("Filesystem sandbox is only available on Linux; continuing without it"); + { + crate::info!("Filesystem sandbox is only available on Linux; continuing without it"); + false + } } } @@ -94,22 +98,6 @@ pub fn is_home_or_ancestor(target: &Path) -> bool { home == target || home.starts_with(&target) } -/// True when the sandbox will actually be enforced for this run. -/// False when disabled or off Linux or the kernel lacks Landlock. -pub fn sandbox_enforced(disabled: bool) -> bool { - if disabled || std::env::var_os("OUCH_NO_SANDBOX").is_some() { - return false; - } - #[cfg(target_os = "linux")] - { - landlock::available() - } - #[cfg(not(target_os = "linux"))] - { - false - } -} - #[cfg(test)] mod tests { use super::*; @@ -137,8 +125,10 @@ mod tests { } #[test] - fn sandbox_not_enforced_when_disabled() { - assert!(!sandbox_enforced(true)); + fn apply_returns_false_when_disabled() { + let mut policy = SandboxPolicy::new(); + policy.set_disabled(true); + assert!(!policy.apply()); } #[test] From 543ed2fcf8632d2df051f0bef471140227d80e90 Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 9 Jun 2026 17:49:41 +0200 Subject: [PATCH 10/16] fix unit tests --- src/commands/mod.rs | 2 +- src/sandbox/mod.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 7cecbdba7..702d780c0 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -72,7 +72,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // Following symlinks can read targets outside the sandboxed input set. // Abort early with a clear message instead of failing midway under Landlock. - if follow_symlinks && !args.no_sandbox && cfg!(target_os = "linux") { + if follow_symlinks && cfg!(target_os = "linux") && !sandbox::disabled_by_request(args.no_sandbox) { return Err( FinalError::with_title("Cannot follow symlinks while the sandbox is active") .detail("a symlink target may point outside the files being compressed") diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 3a0553b93..ac32f5cba 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -55,7 +55,7 @@ impl SandboxPolicy { /// Apply the policy and return whether the sandbox is now enforced. /// Failures emit a warning and let the process run unrestricted. pub fn apply(&self) -> bool { - if self.disabled || std::env::var_os("OUCH_NO_SANDBOX").is_some() { + if disabled_by_request(self.disabled) { return false; } // On Linux the backend warns instead of failing when Landlock is missing and keeps going @@ -98,6 +98,11 @@ pub fn is_home_or_ancestor(target: &Path) -> bool { home == target || home.starts_with(&target) } +/// True when the user turned the sandbox off by flag or by OUCH_NO_SANDBOX. +pub fn disabled_by_request(no_sandbox: bool) -> bool { + no_sandbox || std::env::var_os("OUCH_NO_SANDBOX").is_some() +} + #[cfg(test)] mod tests { use super::*; From b41dec65bf593eaa842fd5fdc6dbdf866142e941 Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 9 Jun 2026 18:59:06 +0200 Subject: [PATCH 11/16] improve patch --- src/commands/mod.rs | 6 ++++++ src/sandbox/landlock.rs | 2 +- src/sandbox/mod.rs | 11 +++++++++++ tests/integration.rs | 44 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 702d780c0..d93e1090e 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -113,6 +113,12 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic for f in &files { policy.allow_read(sandbox::canonicalize_for_sandbox(f)); } + // Allow reading the global gitignore so --gitignore can apply its rules. + if args.gitignore { + if let Some(path) = sandbox::global_gitignore_path() { + policy.allow_read(sandbox::canonicalize_for_sandbox(&path)); + } + } policy.set_disabled(args.no_sandbox).apply() }; diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index b21962a5e..2761423a5 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -25,7 +25,7 @@ pub fn apply(policy: &SandboxPolicy) -> bool { } // Handle filesystem and network rights and deny every IPC scope. -// HardRequirement rejects the sandbox unless the kernel supports all of it. +// Require ABI v6 from Linux 6.12 for landlock sandbox. fn base_ruleset() -> Result { Ruleset::default() .set_compatibility(CompatLevel::HardRequirement) diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index ac32f5cba..c64aa5c85 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -103,6 +103,17 @@ pub fn disabled_by_request(no_sandbox: bool) -> bool { no_sandbox || std::env::var_os("OUCH_NO_SANDBOX").is_some() } +/// Path to the user global gitignore if it exists. +/// Granting read on it lets --gitignore apply global rules under the sandbox. +pub fn global_gitignore_path() -> Option { + let base = match std::env::var_os("XDG_CONFIG_HOME") { + Some(value) if !value.is_empty() => PathBuf::from(value), + _ => PathBuf::from(std::env::var_os("HOME")?).join(".config"), + }; + let path = base.join("git").join("ignore"); + path.exists().then_some(path) +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/integration.rs b/tests/integration.rs index 3e0a9a954..1475530e4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1643,3 +1643,47 @@ fn decompress_archive_default_mode_does_not_duplicate_directory() { assert!(dir.join("test").join("inner.txt").is_file()); assert!(!dir.join("test").join("test").exists()); } + +// Compress and decompress once with the sandbox active and once with it disabled. +// Both runs must succeed so this exercises the sandbox path and the --no-sandbox path. +#[test] +fn compress_decompress_roundtrip_with_and_without_sandbox() { + for no_sandbox in [false, true] { + let (_tempdir, dir) = testdir().unwrap(); + let input = dir.join("data.txt"); + fs::write(&input, "sandbox round trip").unwrap(); + let archive = dir.join("out.tar.gz"); + let output = dir.join("out"); + + // Build an ouch command with the sandbox forced on or off for this run. + let ouch = || { + let mut cmd = crate::utils::cargo_bin(); + cmd.env_remove("OUCH_NO_SANDBOX"); + if no_sandbox { + cmd.arg("--no-sandbox"); + } + cmd + }; + + ouch() + .arg("compress") + .arg(&input) + .arg(&archive) + .arg("--yes") + .assert() + .success(); + ouch() + .arg("decompress") + .arg(&archive) + .arg("--dir") + .arg(&output) + .arg("--yes") + .assert() + .success(); + + assert_eq!( + "sandbox round trip", + fs::read_to_string(output.join("data.txt")).unwrap() + ); + } +} From bc36468bde8c37457b2f1ca6fa269d318de1a076 Mon Sep 17 00:00:00 2001 From: valoq Date: Tue, 9 Jun 2026 22:23:46 +0200 Subject: [PATCH 12/16] improve unit tests --- tests/integration.rs | 82 +++++++++++++++++++++++--------------------- tests/utils.rs | 4 ++- 2 files changed, 46 insertions(+), 40 deletions(-) diff --git a/tests/integration.rs b/tests/integration.rs index 1475530e4..356cd5312 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -495,7 +495,9 @@ fn symlink_pack_and_unpack() -> Result<()> { fs::remove_file(archive)?; fs::remove_dir_all(&dest_files_path)?; + // follow-symlinks is rejected under the sandbox so keep it off here. crate::utils::cargo_bin() + .env("OUCH_NO_SANDBOX", "1") .arg("compress") .arg("--follow-symlinks") .args(files_path) @@ -587,7 +589,9 @@ fn broken_symlink_error_when_compressing_with_follow_symlinks() { let archive = dir.join(format!("archive.{ext}")); + // follow-symlinks is rejected under the sandbox so keep it off here. crate::utils::cargo_bin() + .env("OUCH_NO_SANDBOX", "1") .arg("compress") .arg("--follow-symlinks") .arg(&input) @@ -628,6 +632,8 @@ fn symlink_treatment_inside_nested_dirs_with_follow_symlinks_flag() { let archive = dir.join(format!("archive.{ext}")); let mut cmd = crate::utils::cargo_bin(); + // follow-symlinks is rejected under the sandbox so keep it off here. + cmd.env("OUCH_NO_SANDBOX", "1"); cmd.arg("compress"); if follow_symlinks_flag { cmd.arg("--follow-symlinks"); @@ -1644,46 +1650,44 @@ fn decompress_archive_default_mode_does_not_duplicate_directory() { assert!(!dir.join("test").join("test").exists()); } -// Compress and decompress once with the sandbox active and once with it disabled. -// Both runs must succeed so this exercises the sandbox path and the --no-sandbox path. +// One run with the sandbox disabled. Checks basic functions and that the sandbox is really off. +#[cfg(target_os = "linux")] #[test] -fn compress_decompress_roundtrip_with_and_without_sandbox() { - for no_sandbox in [false, true] { - let (_tempdir, dir) = testdir().unwrap(); - let input = dir.join("data.txt"); - fs::write(&input, "sandbox round trip").unwrap(); - let archive = dir.join("out.tar.gz"); - let output = dir.join("out"); - - // Build an ouch command with the sandbox forced on or off for this run. - let ouch = || { - let mut cmd = crate::utils::cargo_bin(); - cmd.env_remove("OUCH_NO_SANDBOX"); - if no_sandbox { - cmd.arg("--no-sandbox"); - } - cmd - }; +fn without_sandbox_runs_and_confirms_sandbox_is_off() { + let (_tempdir, dir) = testdir().unwrap(); + let input = dir.join("data.txt"); + fs::write(&input, "no sandbox").unwrap(); - ouch() - .arg("compress") - .arg(&input) - .arg(&archive) - .arg("--yes") - .assert() - .success(); - ouch() - .arg("decompress") - .arg(&archive) - .arg("--dir") - .arg(&output) - .arg("--yes") - .assert() - .success(); + // Basic functions still work with the sandbox disabled. + let archive = dir.join("out.tar.gz"); + crate::utils::cargo_bin() + .arg("--no-sandbox") + .arg("compress") + .arg(&input) + .arg(&archive) + .arg("--yes") + .assert() + .success(); + let output = dir.join("out"); + crate::utils::cargo_bin() + .arg("--no-sandbox") + .arg("decompress") + .arg(&archive) + .arg("--dir") + .arg(&output) + .arg("--yes") + .assert() + .success(); + assert_eq!("no sandbox", fs::read_to_string(output.join("data.txt")).unwrap()); - assert_eq!( - "sandbox round trip", - fs::read_to_string(output.join("data.txt")).unwrap() - ); - } + // Sandbox is really off. With it active --follow-symlinks aborts so success proves it is disabled. + crate::utils::cargo_bin() + .arg("--no-sandbox") + .arg("compress") + .arg("--follow-symlinks") + .arg(&input) + .arg(dir.join("follow.tar")) + .arg("--yes") + .assert() + .success(); } diff --git a/tests/utils.rs b/tests/utils.rs index 3e16181e5..03f9d20a4 100644 --- a/tests/utils.rs +++ b/tests/utils.rs @@ -33,7 +33,9 @@ pub fn cargo_bin() -> Command { }) }) .unwrap_or_else(|| Command::cargo_bin("ouch").expect("Failed to find ouch executable")); - cmd.env("OUCH_NO_SANDBOX", "1"); + // Run the whole suite with the sandbox enabled. + // Clear any inherited OUCH_NO_SANDBOX so a stray value cannot disable it. + cmd.env_remove("OUCH_NO_SANDBOX"); cmd } From 6256dd5ab799b7e4b007b3d3244f21106478092b Mon Sep 17 00:00:00 2001 From: valoq Date: Wed, 10 Jun 2026 12:19:11 +0200 Subject: [PATCH 13/16] improve patch --- src/commands/decompress.rs | 9 ++++- src/commands/mod.rs | 66 ++++++++++++++++++++++------------ src/sandbox/landlock.rs | 1 + src/sandbox/mod.rs | 31 +++++++++------- tests/integration.rs | 73 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 145 insertions(+), 35 deletions(-) diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 522ef2059..b57e707ad 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -1,4 +1,5 @@ use std::{ + collections::HashSet, ffi::OsString, io::{self, BufReader, Read}, ops::ControlFlow, @@ -50,6 +51,7 @@ pub enum PreparedTarget { } /// Resolve conflicts and create the output directory before the sandbox starts. +/// claimed_targets holds directories already taken by earlier inputs of this run. pub fn prepare_decompress_target( formats: &[Extension], output_dir: &Path, @@ -57,6 +59,7 @@ pub fn prepare_decompress_target( output_dir_was_explicit: bool, here: bool, question_policy: QuestionPolicy, + claimed_targets: &mut HashSet, ) -> Result { let (first_extension, _) = split_first_compression_format(formats); let is_archive = matches!(first_extension, Tar | Zip | Rar | SevenZip); @@ -71,7 +74,10 @@ pub fn prepare_decompress_target( output_file_path.with_extension("") }; let is_cwd = target == *INITIAL_CURRENT_DIR; - let is_valid = is_cwd || !target.fs_err_try_exists()? || (target.is_dir() && target.read_dir()?.next().is_none()); + // merging several inputs into the CWD is intentional so the claimed check skips it + let claimed_by_earlier_input = !is_cwd && claimed_targets.contains(&target); + let is_valid = !claimed_by_earlier_input + && (is_cwd || !target.fs_err_try_exists()? || (target.is_dir() && target.read_dir()?.next().is_none())); let resolved = if is_valid { target @@ -84,6 +90,7 @@ pub fn prepare_decompress_target( if !resolved.fs_err_try_exists()? { fs::create_dir(&resolved)?; } + claimed_targets.insert(resolved.clone()); let file_name = (!is_archive).then(|| output_file_path.file_name().unwrap_or_default().to_os_string()); Ok(PreparedTarget::Target { dir: resolved, file_name }) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index d93e1090e..cc9264038 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -18,7 +18,7 @@ use crate::{ commands::{compress::compress_files, decompress::decompress_file, list::list_archive_contents}, error::{Error, FinalError}, extension::{self, parse_format_flag}, - info_accessible, + info, info_accessible, list::ListOptions, sandbox::{self, SandboxPolicy}, utils::{ @@ -70,9 +70,12 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic return Err(FinalError::with_title("No files to compress").into()); } + // gitignore reads git config from places the sandbox cannot know upfront so it runs unsandboxed + let sandbox_disabled = sandbox::disabled_by_request(args.no_sandbox) || args.gitignore; + // Following symlinks can read targets outside the sandboxed input set. // Abort early with a clear message instead of failing midway under Landlock. - if follow_symlinks && cfg!(target_os = "linux") && !sandbox::disabled_by_request(args.no_sandbox) { + if follow_symlinks && cfg!(target_os = "linux") && !sandbox_disabled { return Err( FinalError::with_title("Cannot follow symlinks while the sandbox is active") .detail("a symlink target may point outside the files being compressed") @@ -109,17 +112,14 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // Read on inputs. The output FD is held already so no write-path grant is needed. let sandbox_active = { + if args.gitignore && cfg!(target_os = "linux") && !sandbox::disabled_by_request(args.no_sandbox) { + info!("Sandbox: disabled because --gitignore reads git configuration outside the input files"); + } let mut policy = SandboxPolicy::new(); for f in &files { policy.allow_read(sandbox::canonicalize_for_sandbox(f)); } - // Allow reading the global gitignore so --gitignore can apply its rules. - if args.gitignore { - if let Some(path) = sandbox::global_gitignore_path() { - policy.allow_read(sandbox::canonicalize_for_sandbox(&path)); - } - } - policy.set_disabled(args.no_sandbox).apply() + policy.set_disabled(sandbox_disabled).apply() }; let level = if fast { @@ -228,6 +228,8 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic .collect(); // Resolve conflicts and create output dirs before the sandbox applies. + // claimed targets make duplicate basenames conflict instead of silently merging + let mut claimed_targets = std::collections::HashSet::new(); let mut prepared: Vec = Vec::with_capacity(files.len()); for (formats, output_file_path) in files_extensions.iter().zip(&output_file_paths) { prepared.push(prepare_decompress_target( @@ -237,6 +239,7 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic output_dir_was_explicit, here, question_policy, + &mut claimed_targets, )?); } @@ -374,25 +377,44 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic .collect(); // Open RAR spill tempfiles up front so unrar can read from them under the sandbox. // The archive format sits at the front so match there not at the end. - let mut rar_spill_tempfiles: Vec> = flat_formats - .iter() - .map(|fs| { - if fs.len() > 1 && matches!(fs.first(), Some(extension::CompressionFormat::Rar)) { - tempfile::NamedTempFile::new().ok() - } else { - None - } - }) - .collect(); + let needs_spill = |formats: &[extension::CompressionFormat]| { + formats.len() > 1 && matches!(formats.first(), Some(extension::CompressionFormat::Rar)) + }; + + // all spills share one private temp dir so the sandbox can allow their cleanup + let rar_spill_dir = if flat_formats.iter().any(|fs| needs_spill(fs)) { + Some(tempfile::tempdir()?) + } else { + None + }; + let mut rar_spill_tempfiles: Vec> = Vec::with_capacity(flat_formats.len()); + for fs in &flat_formats { + let spill = if needs_spill(fs) { + let dir = rar_spill_dir + .as_ref() + .expect("spill dir is created when any input needs a spill"); + Some(tempfile::Builder::new().prefix(".ouch-rar-").tempfile_in(dir.path())?) + } else { + None + }; + rar_spill_tempfiles.push(spill); + } { let mut policy = SandboxPolicy::new(); for f in &files { policy.allow_read(sandbox::canonicalize_for_sandbox(f)); } - // Grant read on each spill file so unrar can reopen it by path. - for spill in rar_spill_tempfiles.iter().flatten() { - policy.allow_read(sandbox::canonicalize_for_sandbox(spill.path())); + if let Some(spill_dir) = &rar_spill_dir { + let canon = sandbox::canonicalize_for_sandbox(spill_dir.path()); + // unrar reopens the spill files by path + policy.allow_read(canon.clone()); + // each NamedTempFile unlinks its spill file on drop + policy.allow_remove_in(canon.clone()); + // RemoveDir on the parent lets the TempDir remove itself on drop + if let Some(parent) = canon.parent() { + policy.allow_remove_dir_in(parent.to_path_buf()); + } } policy.set_disabled(args.no_sandbox).apply(); } diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index 2761423a5..b96b8f935 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -52,6 +52,7 @@ fn install(policy: &SandboxPolicy) -> Result<(), RulesetError> { let ruleset = add_rules(ruleset, policy.read_paths(), read_grant())?; let ruleset = add_rules(ruleset, policy.read_write_paths(), write_grant())?; let ruleset = add_rules(ruleset, policy.remove_in_paths(), AccessFs::RemoveFile.into())?; + let ruleset = add_rules(ruleset, policy.remove_dir_in_paths(), AccessFs::RemoveDir.into())?; // No network rules and no scope exceptions are added. // So nothing can open TCP or reach abstract sockets or signal outside processes. let _ = ruleset.restrict_self()?; diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index c64aa5c85..86a5392a9 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -11,6 +11,8 @@ pub struct SandboxPolicy { read_write: Vec, /// Directories where the decompressor can delete the input archive but not write nearby. remove_in: Vec, + /// Directories where only subdirectories may be removed. + remove_dir_in: Vec, disabled: bool, } @@ -35,6 +37,12 @@ impl SandboxPolicy { self } + /// Allow removing subdirectories of `path` without granting any other write rights. + pub fn allow_remove_dir_in>(&mut self, path: P) -> &mut Self { + self.remove_dir_in.push(path.into()); + self + } + pub fn set_disabled(&mut self, disabled: bool) -> &mut Self { self.disabled = disabled; self @@ -52,6 +60,10 @@ impl SandboxPolicy { &self.remove_in } + pub fn remove_dir_in_paths(&self) -> &[PathBuf] { + &self.remove_dir_in + } + /// Apply the policy and return whether the sandbox is now enforced. /// Failures emit a warning and let the process run unrestricted. pub fn apply(&self) -> bool { @@ -103,17 +115,6 @@ pub fn disabled_by_request(no_sandbox: bool) -> bool { no_sandbox || std::env::var_os("OUCH_NO_SANDBOX").is_some() } -/// Path to the user global gitignore if it exists. -/// Granting read on it lets --gitignore apply global rules under the sandbox. -pub fn global_gitignore_path() -> Option { - let base = match std::env::var_os("XDG_CONFIG_HOME") { - Some(value) if !value.is_empty() => PathBuf::from(value), - _ => PathBuf::from(std::env::var_os("HOME")?).join(".config"), - }; - let path = base.join("git").join("ignore"); - path.exists().then_some(path) -} - #[cfg(test)] mod tests { use super::*; @@ -121,10 +122,16 @@ mod tests { #[test] fn policy_collects_paths() { let mut policy = SandboxPolicy::new(); - policy.allow_read("/tmp/in").allow_read_write("/tmp/out"); + policy + .allow_read("/tmp/in") + .allow_read_write("/tmp/out") + .allow_remove_in("/tmp/spill") + .allow_remove_dir_in("/tmp"); assert_eq!(policy.read_paths(), &[PathBuf::from("/tmp/in")]); assert_eq!(policy.read_write_paths(), &[PathBuf::from("/tmp/out")]); + assert_eq!(policy.remove_in_paths(), &[PathBuf::from("/tmp/spill")]); + assert_eq!(policy.remove_dir_in_paths(), &[PathBuf::from("/tmp")]); } #[test] diff --git a/tests/integration.rs b/tests/integration.rs index 356cd5312..108e6e751 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1691,3 +1691,76 @@ fn without_sandbox_runs_and_confirms_sandbox_is_off() { .assert() .success(); } + +// two inputs with the same basename must not silently merge into one target + +#[test] +fn decompress_duplicate_basenames_skips_second_with_no() { + let (_tempdir, dir) = testdir().unwrap(); + + for (sub, content) in [("a", "AAA"), ("b", "BBB")] { + fs::create_dir(dir.join(sub)).unwrap(); + fs::write(dir.join(sub).join("x.txt"), content).unwrap(); + crate::utils::cargo_bin() + .args(["compress", "x.txt", "x.tar.gz", "--yes"]) + .current_dir(dir.join(sub)) + .assert() + .success(); + } + + // the no flag answers the second conflict with skip + crate::utils::cargo_bin() + .args(["decompress", "a/x.tar.gz", "b/x.tar.gz", "--no"]) + .current_dir(dir) + .assert() + .success(); + + // only the first input extracted and its content is intact + assert_eq!("AAA", fs::read_to_string(dir.join("x").join("x.txt")).unwrap()); + assert!(!dir.join("x_1").exists()); +} + +#[test] +fn decompress_duplicate_basenames_prompts_and_renames() { + let (_tempdir, dir) = testdir().unwrap(); + + for (sub, content) in [("a", "AAA"), ("b", "BBB")] { + fs::create_dir(dir.join(sub)).unwrap(); + fs::write(dir.join(sub).join("x.txt"), content).unwrap(); + crate::utils::cargo_bin() + .args(["compress", "x.txt", "x.tar.gz", "--yes"]) + .current_dir(dir.join(sub)) + .assert() + .success(); + } + + // the second input must hit the conflict question and get renamed + crate::utils::cargo_bin() + .args(["decompress", "a/x.tar.gz", "b/x.tar.gz"]) + .current_dir(dir) + .write_stdin("r") + .assert() + .success(); + + assert_eq!("AAA", fs::read_to_string(dir.join("x").join("x.txt")).unwrap()); + assert_eq!("BBB", fs::read_to_string(dir.join("x_1").join("x.txt")).unwrap()); +} + +// follow symlinks only passes when the sandbox is off so success proves gitignore disabled it +#[cfg(target_os = "linux")] +#[test] +fn gitignore_disables_sandbox_and_unlocks_follow_symlinks() { + let (_tempdir, dir) = testdir().unwrap(); + let input = dir.join("data.txt"); + fs::write(&input, "tracked").unwrap(); + + crate::utils::cargo_bin() + .arg("compress") + .arg("--gitignore") + .arg("--follow-symlinks") + .arg(&input) + .arg(dir.join("out.tar")) + .arg("--yes") + .assert() + .success(); +} From af75156fefd8304232f6e078364437ffd083e55a Mon Sep 17 00:00:00 2001 From: valoq Date: Sun, 14 Jun 2026 21:29:17 +0200 Subject: [PATCH 14/16] cleanup --- src/archive/tar.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 3e132dc28..a14b4ba67 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -55,14 +55,7 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result { let full_link_path = output_folder.join(&link_path); let full_target_path = output_folder.join(&target); - // Fall back to copy when the hard link cannot be created. - if let Err(err) = fs::hard_link(&full_target_path, &full_link_path) { - info!( - "Could not hard link {} ({err}); copying instead", - PathFmt(&full_link_path) - ); - fs::copy(&full_target_path, &full_link_path)?; - } + fs::hard_link(&full_target_path, &full_link_path)?; } tar::EntryType::Regular | tar::EntryType::GNUSparse => { entry.unpack_in(output_folder)?; From b6802e3935305206f9430c6b9592c8abb083e9e3 Mon Sep 17 00:00:00 2001 From: valoq Date: Sun, 14 Jun 2026 21:53:23 +0200 Subject: [PATCH 15/16] improve patch --- src/commands/mod.rs | 10 +++++++--- src/sandbox/landlock.rs | 10 +++++++++- src/sandbox/mod.rs | 12 ++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index cc9264038..07a3f2fcd 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -75,9 +75,9 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic // Following symlinks can read targets outside the sandboxed input set. // Abort early with a clear message instead of failing midway under Landlock. - if follow_symlinks && cfg!(target_os = "linux") && !sandbox_disabled { + if follow_symlinks && !sandbox_disabled && sandbox::available() { return Err( - FinalError::with_title("Cannot follow symlinks while the sandbox is active") + FinalError::with_title("Cannot follow symlinks while the sandbox is enabled") .detail("a symlink target may point outside the files being compressed") .hint("re-run with --no-sandbox to follow symlinks") .into(), @@ -144,6 +144,11 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic if let Ok(true) = compress_result { info_accessible!("Output file size: {}", BytesFmt(file_size(&output_path)?)); info_accessible!("Successfully compressed to {}", PathFmt(&output_path)); + } else if let Ok(false) = compress_result { + // cancel leaves a partial output file so remove it when the sandbox is off + if !sandbox_active { + let _ = utils::remove_file_or_dir(&output_path); + } } else if compress_result.is_err() { // The sandbox blocks unlinking the partial file so only auto-delete when it is off. let deleted = !sandbox_active && utils::remove_file_or_dir(&output_path).is_ok(); @@ -157,7 +162,6 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic eprintln!(" Compression failed for reasons below."); } } - // Ok(false) means the user cancelled before any data was written; nothing to report. compress_result.map(|_| ()) } diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index b96b8f935..b2e3fef23 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -18,7 +18,10 @@ pub fn apply(policy: &SandboxPolicy) -> bool { match install(policy) { Ok(()) => true, Err(_) => { - warning!("Sandbox: Landlock ABI v6 (Linux 6.12 or newer) is required; running without a sandbox"); + warning!( + "Sandbox: could not enable Landlock; running without a sandbox. \ + This requires the Landlock LSM to be enabled and a kernel with ABI v6 (Linux 6.12 or newer)" + ); false } } @@ -35,6 +38,11 @@ fn base_ruleset() -> Result { .create() } +// whether landlock can actually be enforced on this kernel +pub fn is_available() -> bool { + base_ruleset().is_ok() +} + // Rights an extractor needs inside an output directory. // It never grants execution or device nodes or device ioctls. fn write_grant() -> BitFlags { diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 86a5392a9..76978b871 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -115,6 +115,18 @@ pub fn disabled_by_request(no_sandbox: bool) -> bool { no_sandbox || std::env::var_os("OUCH_NO_SANDBOX").is_some() } +// whether a sandbox can actually be enforced on this platform +pub fn available() -> bool { + #[cfg(target_os = "linux")] + { + landlock::is_available() + } + #[cfg(not(target_os = "linux"))] + { + false + } +} + #[cfg(test)] mod tests { use super::*; From 63241e7a06f22ff263dbafffb457a504dca76e14 Mon Sep 17 00:00:00 2001 From: valoq Date: Mon, 15 Jun 2026 22:44:28 +0200 Subject: [PATCH 16/16] improve patch --- CHANGELOG.md | 2 +- src/commands/mod.rs | 77 +++++++++++++++++--------------- src/sandbox/landlock.rs | 97 +++++++++++++++++++++++++++++++++-------- src/sandbox/mod.rs | 3 +- 4 files changed, 124 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21406767e..c7d9a31bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ Categories Used: - Add `--here` flag to unpack into current directory (https://github.com/ouch-org/ouch/pull/962). - List: show symlink targets (for tar and zip) (https://github.com/ouch-org/ouch/pull/934) - Add aliases for ebooks (`.epub`) (https://github.com/ouch-org/ouch/pull/981) -- Add a Landlock sandbox on Linux for `compress`/`decompress`/`list` that restricts filesystem, network, and IPC access (disable with `--no-sandbox`) (https://github.com/ouch-org/ouch/pull/995) +- Add a Landlock sandbox on Linux for `compress`/`decompress`/`list` that restricts filesystem access and, where the kernel supports it, blocks new TCP connections, abstract-socket use, and signals to processes outside the sandbox (disable with `--no-sandbox` or the `OUCH_NO_SANDBOX` environment variable) (https://github.com/ouch-org/ouch/pull/995) ### Improvements diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 07a3f2fcd..c81c6d416 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -70,18 +70,16 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic return Err(FinalError::with_title("No files to compress").into()); } - // gitignore reads git config from places the sandbox cannot know upfront so it runs unsandboxed - let sandbox_disabled = sandbox::disabled_by_request(args.no_sandbox) || args.gitignore; - - // Following symlinks can read targets outside the sandboxed input set. - // Abort early with a clear message instead of failing midway under Landlock. - if follow_symlinks && !sandbox_disabled && sandbox::available() { - return Err( - FinalError::with_title("Cannot follow symlinks while the sandbox is enabled") - .detail("a symlink target may point outside the files being compressed") - .hint("re-run with --no-sandbox to follow symlinks") - .into(), - ); + // gitignore and follow_symlinks both read paths outside the declared input set so the + // sandbox cannot confine them; run unsandboxed and say why + let sandbox_disabled = sandbox::disabled_by_request(args.no_sandbox) || args.gitignore || follow_symlinks; + if cfg!(target_os = "linux") && !sandbox::disabled_by_request(args.no_sandbox) { + if args.gitignore { + info!("Sandbox: disabled because --gitignore reads git configuration outside the input files"); + } + if follow_symlinks { + info!("Sandbox: disabled because --follow-symlinks may read files outside the input set"); + } } // Formats from path extension, like "file.tar.gz.xz" -> vec![Tar, Gzip, Lzma] @@ -110,11 +108,10 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic None => return Ok(()), }; - // Read on inputs. The output FD is held already so no write-path grant is needed. + // Read on inputs. The output FD is held already so no write-path grant is needed, and + // the sandbox is deliberately not widened to let compression delete files in the + // output directory. let sandbox_active = { - if args.gitignore && cfg!(target_os = "linux") && !sandbox::disabled_by_request(args.no_sandbox) { - info!("Sandbox: disabled because --gitignore reads git configuration outside the input files"); - } let mut policy = SandboxPolicy::new(); for f in &files { policy.allow_read(sandbox::canonicalize_for_sandbox(f)); @@ -145,21 +142,29 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic info_accessible!("Output file size: {}", BytesFmt(file_size(&output_path)?)); info_accessible!("Successfully compressed to {}", PathFmt(&output_path)); } else if let Ok(false) = compress_result { - // cancel leaves a partial output file so remove it when the sandbox is off + // user cancelled; remove the partial output where the sandbox permits it if !sandbox_active { let _ = utils::remove_file_or_dir(&output_path); } } else if compress_result.is_err() { - // The sandbox blocks unlinking the partial file so only auto-delete when it is off. let deleted = !sandbox_active && utils::remove_file_or_dir(&output_path).is_ok(); if !deleted { - eprintln!("{red}FATAL ERROR:\n", red = *colors::RED); - eprintln!( - " Compression did not finish; the partial file at {} may be corrupted.", - PathFmt(&output_path) - ); - eprintln!(" Please delete it manually."); - eprintln!(" Compression failed for reasons below."); + if sandbox_active { + // Not a cleanup failure: the sandbox intentionally does not grant removal + // in the output directory, so the partial file is left in place. + warning!( + "Compression did not finish; the partial file at {} was left because the sandbox does not permit removing it. Delete it manually.", + PathFmt(&output_path) + ); + } else { + eprintln!("{red}FATAL ERROR:\n", red = *colors::RED); + eprintln!( + " Compression did not finish; the partial file at {} may be corrupted.", + PathFmt(&output_path) + ); + eprintln!(" Please delete it manually."); + eprintln!(" Compression failed for reasons below."); + } } } @@ -278,12 +283,15 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic if is_path_stdin(f) { continue; } - let canon = sandbox::canonicalize_for_sandbox(f); - if let Some(parent) = canon.parent() { - let parent = parent.to_path_buf(); - if !remove_parents.contains(&parent) { - remove_parents.push(parent); - } + // Grant in the directory that actually holds the input. For a symlinked + // input that is the symlink's own directory, not the target's, because + // --remove unlinks the symlink itself. + let input_dir = match f.parent() { + Some(p) if !p.as_os_str().is_empty() => sandbox::canonicalize_for_sandbox(p), + _ => sandbox::canonicalize_for_sandbox(std::path::Path::new(".")), + }; + if !remove_parents.contains(&input_dir) { + remove_parents.push(input_dir); } } // The decompressor can delete the input archive but not write nearby. @@ -415,10 +423,9 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic policy.allow_read(canon.clone()); // each NamedTempFile unlinks its spill file on drop policy.allow_remove_in(canon.clone()); - // RemoveDir on the parent lets the TempDir remove itself on drop - if let Some(parent) = canon.parent() { - policy.allow_remove_dir_in(parent.to_path_buf()); - } + // No RemoveDir grant on the parent: granting it would cover the whole temp + // directory. The empty spill directory may be left behind if a failure stops + // the TempDir from removing itself under the sandbox, which is the safer trade. } policy.set_disabled(args.no_sandbox).apply(); } diff --git a/src/sandbox/landlock.rs b/src/sandbox/landlock.rs index b2e3fef23..cf09c93d5 100644 --- a/src/sandbox/landlock.rs +++ b/src/sandbox/landlock.rs @@ -86,7 +86,16 @@ where let path = path.as_ref(); match PathFd::new(path) { Ok(fd) => { - let granted = rights_for(access, path.is_dir()); + // fstat the opened descriptor so the directory check matches the exact file the + // rule is built from, instead of a separate stat that could be swapped underneath. + let is_dir = { + use std::os::fd::{AsFd, AsRawFd}; + let mut st: libc::stat = unsafe { std::mem::zeroed() }; + // SAFETY: fd is open for this call and st is a valid out-parameter. + let ok = unsafe { libc::fstat(fd.as_fd().as_raw_fd(), &mut st) } == 0; + ok && (st.st_mode & libc::S_IFMT as libc::mode_t) == libc::S_IFDIR as libc::mode_t + }; + let granted = rights_for(access, is_dir); if granted.is_empty() { continue; } @@ -156,37 +165,91 @@ mod tests { assert!(scopes.contains(Scope::Signal)); } - // Real enforcement check. Runs only where the kernel provides the required ABI. - // The ruleset and paths are built before the fork so the child does almost no work. + // Real enforcement check, run only where the kernel provides Landlock. The ruleset is built + // with raw syscalls and the forked child runs only async-signal-safe syscalls and never + // allocates, so forking from the multithreaded test harness cannot deadlock on a held lock. #[test] fn landlock_denies_writes_outside_granted_dir() { use std::{ffi::CString, os::unix::ffi::OsStrExt}; - // Skip when the kernel cannot create the required ruleset. - if base_ruleset().is_err() { - return; + // linux/landlock.h + const FS_WRITE_FILE: u64 = 1 << 1; + const FS_MAKE_REG: u64 = 1 << 8; + const RULE_PATH_BENEATH: libc::c_uint = 1; + #[repr(C)] + struct RulesetAttr { + handled_access_fs: u64, + handled_access_net: u64, + scoped: u64, + } + #[repr(C, packed)] + struct PathBeneathAttr { + allowed_access: u64, + parent_fd: i32, } let granted = tempfile::tempdir().unwrap(); - let outside_dir = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); let inside_c = CString::new(granted.path().join("inside.txt").as_os_str().as_bytes()).unwrap(); - let outside_c = CString::new(outside_dir.path().join("escape.txt").as_os_str().as_bytes()).unwrap(); + let outside_c = CString::new(outside.path().join("escape.txt").as_os_str().as_bytes()).unwrap(); + let dir_c = CString::new(granted.path().as_os_str().as_bytes()).unwrap(); + + let access = FS_WRITE_FILE | FS_MAKE_REG; + let attr = RulesetAttr { + handled_access_fs: access, + handled_access_net: 0, + scoped: 0, + }; + // SAFETY: attr is a valid, fully-initialized ruleset descriptor. + let ruleset_fd = unsafe { + libc::syscall( + libc::SYS_landlock_create_ruleset, + &attr as *const RulesetAttr, + std::mem::size_of::(), + 0u32, + ) + }; + if ruleset_fd < 0 { + // Landlock is unavailable on this kernel; there is nothing to enforce. + return; + } + let ruleset_fd = ruleset_fd as libc::c_int; - // Grant read-write on the one directory and build the ruleset in the parent. - let ruleset = add_rules(base_ruleset().unwrap(), [granted.path()], write_grant()).unwrap(); + // SAFETY: dir_c is a valid path; O_PATH yields a descriptor usable as a rule parent. + let dir_fd = unsafe { libc::open(dir_c.as_ptr(), libc::O_PATH | libc::O_CLOEXEC) }; + assert!(dir_fd >= 0, "failed to open the granted directory"); + let beneath = PathBeneathAttr { + allowed_access: access, + parent_fd: dir_fd, + }; + // SAFETY: beneath references the open dir_fd and a valid access mask. + let added = unsafe { + libc::syscall( + libc::SYS_landlock_add_rule, + ruleset_fd, + RULE_PATH_BENEATH, + &beneath as *const PathBeneathAttr, + 0u32, + ) + }; + assert_eq!(added, 0, "failed to add the path rule"); - // The child only restricts itself and opens two paths then exits. + // SAFETY: the child runs only async-signal-safe syscalls and never allocates before _exit, + // so forking from a possibly-multithreaded harness is safe here. let pid = unsafe { libc::fork() }; if pid == 0 { - let enforced = ruleset.restrict_self().is_ok(); - let inside_fd = unsafe { libc::open(inside_c.as_ptr(), libc::O_CREAT | libc::O_WRONLY, 0o600) }; - let outside_fd = unsafe { libc::open(outside_c.as_ptr(), libc::O_CREAT | libc::O_WRONLY, 0o600) }; - // Inside the grant must succeed and outside must be denied. - let ok = enforced && inside_fd >= 0 && outside_fd < 0; - unsafe { libc::_exit(if ok { 0 } else { 1 }) }; + unsafe { + libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); + let restricted = libc::syscall(libc::SYS_landlock_restrict_self, ruleset_fd, 0u32); + let inside = libc::open(inside_c.as_ptr(), libc::O_CREAT | libc::O_WRONLY, 0o600); + let outside = libc::open(outside_c.as_ptr(), libc::O_CREAT | libc::O_WRONLY, 0o600); + let ok = restricted == 0 && inside >= 0 && outside < 0; + libc::_exit(if ok { 0 } else { 1 }); + } } assert!(pid > 0, "fork failed"); let mut status = 0; + // SAFETY: status is a valid out-parameter for the child just forked. unsafe { libc::waitpid(pid, &mut status, 0) }; assert!(libc::WIFEXITED(status), "the sandboxed child did not exit normally"); assert_eq!( diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 76978b871..84606d09e 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -75,10 +75,9 @@ impl SandboxPolicy { { landlock::apply(self) } - // The sandbox is Linux only so elsewhere there is nothing to enforce and we just log it + // The sandbox is Linux only so elsewhere there is nothing to enforce #[cfg(not(target_os = "linux"))] { - crate::info!("Filesystem sandbox is only available on Linux; continuing without it"); false } }