complete sandbox

valoq · valoq · commit 79316c576ed4 · 2026-06-06T21:37:31.000+02:00
diff --git 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<Extension>,
     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<dyn FnOnce(&Path) -> Result<u64>> = 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
@@ -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<tempfile::NamedTempFile>,
 ) -> 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
@@ -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,21 +238,29 @@ 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) {
                             continue;
                         }
                         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<Vec<extension::CompressionFormat>> = 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<Option<tempfile::NamedTempFile>> = 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
@@ -33,6 +33,8 @@ pub fn apply(policy: &SandboxPolicy) {
 fn install(policy: &SandboxPolicy) -> Result<RulesetStatus, RulesetError> {
     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> = AccessFs::RemoveFile | AccessFs::Refer;
 
     let ruleset = Ruleset::default()
         .set_compatibility(CompatLevel::BestEffort)
@@ -41,6 +43,7 @@ fn install(policy: &SandboxPolicy) -> Result<RulesetStatus, RulesetError> {
 
     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
@@ -9,6 +9,8 @@ mod landlock;
 pub struct SandboxPolicy {
     read: Vec<PathBuf>,
     read_write: Vec<PathBuf>,
+    /// Directories where the decompressor can delete the input archive but not write nearby.
+    remove_in: Vec<PathBuf>,
     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<P: Into<PathBuf>>(&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;
