diff --git a/crates/tinybox-jail/README.md b/crates/tinybox-jail/README.md index de14f71..d67d2a3 100644 --- a/crates/tinybox-jail/README.md +++ b/crates/tinybox-jail/README.md @@ -14,11 +14,15 @@ spawns, never the core process itself. ## Responsibilities - Describe a jail declaratively via a builder (`Jail::new(root, label)` plus - `.add_read_only(...)`, `.deny_net()`, `.deny_subprocess()`). + `.add_read_only(...)`, `.add_read_write(...)`, `.deny_net()`, + `.deny_subprocess()`). `add_read_write` grants an extra path outside the + root the same access as the root (Landlock rule, Seatbelt `file-write*` + subpath, `AppContainer` ACL), for host-owned scratch such as a per-call + output-capture directory that must not land inside the root. - Cache the default backend; currently this is an unsupported backend on every platform while OS implementations are being brought into compliance. - Spawn a `std::process::Command` inside the jail, canonicalizing `root` - (and read-only paths) first so backends never see `..` or symlink + (and the read-only and read/write paths) first so backends never see `..` or symlink trickery. - Provide a persistent registry to manage many jailed workspaces side by side, each with a stable id, label, directory, and metadata, indexed in a diff --git a/crates/tinybox-jail/src/jail.rs b/crates/tinybox-jail/src/jail.rs index 0289e49..d0a02c7 100644 --- a/crates/tinybox-jail/src/jail.rs +++ b/crates/tinybox-jail/src/jail.rs @@ -10,8 +10,8 @@ use std::process::{Child, Command}; /// Declarative description of a directory jail. /// -/// One `root` (read/write), zero or more `read_only` paths, an optional -/// allow-list of extra paths the child *may* read, and a network toggle. +/// One `root` (read/write), zero or more extra `read_write` paths, zero or +/// more `read_only` paths, and a network toggle. /// Backends translate this into Landlock rules, a Seatbelt profile, or an /// `AppContainer` ACL. #[derive(Debug, Clone)] @@ -22,6 +22,11 @@ pub struct Jail { /// Extra paths the child may read (e.g. `/usr/lib`, the runtime-node /// install). Writes are still denied. pub read_only: Vec, + /// Extra paths outside the root the child may read **and write**, with + /// the same access the root gets. Meant for host-owned scratch the child + /// must write to without it landing inside the root (e.g. a per-call + /// output-capture directory). Grant the narrowest directory that works. + pub read_write: Vec, /// Allow outbound network. Most agent tools need this; some risky tools /// (untrusted code execution) should disable it. pub allow_net: bool, @@ -40,6 +45,7 @@ impl Jail { Self { root: root.as_ref().to_path_buf(), read_only: Vec::new(), + read_write: Vec::new(), allow_net: true, allow_subprocess: true, label: label.into(), @@ -53,6 +59,18 @@ impl Jail { self } + /// Grants read and write access to an extra path outside the root. + /// + /// Backends grant it exactly what they grant the root: a Landlock rule on + /// Linux, a `file-write*` subpath on macOS, an `AppContainer` ACL on + /// Windows. The path should exist before spawning; backends that need an + /// open handle (Landlock) fail the spawn when it does not. + #[must_use] + pub fn add_read_write(mut self, path: impl AsRef) -> Self { + self.read_write.push(path.as_ref().to_path_buf()); + self + } + /// Asks the backend to block network access where it can. #[must_use] pub fn deny_net(mut self) -> Self { @@ -67,7 +85,7 @@ impl Jail { self } - /// Canonicalize `root` and `read_only` so backends never see `..` or + /// Canonicalize `root`, `read_only` and `read_write` so backends never see `..` or /// symlink trickery. Returns an error if `root` does not exist. /// /// # Errors @@ -75,7 +93,7 @@ impl Jail { /// Returns the filesystem error encountered while canonicalizing `root`. pub fn canonicalize(&mut self) -> std::io::Result<()> { self.root = self.root.canonicalize()?; - for p in &mut self.read_only { + for p in self.read_only.iter_mut().chain(self.read_write.iter_mut()) { if let Ok(c) = p.canonicalize() { *p = c; } diff --git a/crates/tinybox-jail/src/jail_tests.rs b/crates/tinybox-jail/src/jail_tests.rs index f84537a..0cf4ffc 100644 --- a/crates/tinybox-jail/src/jail_tests.rs +++ b/crates/tinybox-jail/src/jail_tests.rs @@ -58,3 +58,38 @@ fn canonicalize_errors_on_missing_root() { let err = j.canonicalize().unwrap_err(); assert_eq!(err.kind(), std::io::ErrorKind::NotFound); } + +#[test] +fn defaults_have_no_extra_read_write_paths() { + let j = Jail::new("/tmp", "x"); + assert_eq!(j.read_write, Vec::::new()); +} + +#[test] +fn add_read_write_appends_in_order_and_leaves_read_only_alone() { + let j = Jail::new("/tmp", "x") + .add_read_write("/a") + .add_read_write("/b"); + assert_eq!(j.read_write, vec![PathBuf::from("/a"), PathBuf::from("/b")]); + assert_eq!(j.read_only, Vec::::new()); + assert_eq!(j.root, PathBuf::from("/tmp")); +} + +#[test] +fn canonicalize_resolves_read_write_paths() { + let root = tempfile::tempdir().unwrap(); + let extra = tempfile::tempdir().unwrap(); + let dotted = extra.path().join("sub").join(".."); + std::fs::create_dir_all(extra.path().join("sub")).unwrap(); + let mut j = Jail::new(root.path(), "x").add_read_write(&dotted); + j.canonicalize().unwrap(); + assert_eq!(j.read_write, vec![extra.path().canonicalize().unwrap()]); +} + +#[test] +fn canonicalize_keeps_missing_read_write_as_is() { + let root = tempfile::tempdir().unwrap(); + let mut j = Jail::new(root.path(), "x").add_read_write("/this/never/existed"); + j.canonicalize().unwrap(); + assert_eq!(j.read_write, vec![PathBuf::from("/this/never/existed")]); +} diff --git a/crates/tinybox-jail/src/linux.rs b/crates/tinybox-jail/src/linux.rs index e6685d4..592cbf9 100644 --- a/crates/tinybox-jail/src/linux.rs +++ b/crates/tinybox-jail/src/linux.rs @@ -80,6 +80,13 @@ impl JailBackend for LandlockBackend { ruleset = ruleset .add_rule(PathBeneath::new(root_fd, writes | reads)) .map_err(|error| std::io::Error::other(error.to_string()))?; + for path in &jail.read_write { + let fd = + PathFd::new(path).map_err(|error| std::io::Error::other(error.to_string()))?; + ruleset = ruleset + .add_rule(PathBeneath::new(fd, writes | reads)) + .map_err(|error| std::io::Error::other(error.to_string()))?; + } for path in &jail.read_only { let fd = PathFd::new(path).map_err(|error| std::io::Error::other(error.to_string()))?; diff --git a/crates/tinybox-jail/src/linux_tests.rs b/crates/tinybox-jail/src/linux_tests.rs index bfcfd01..6b6c90b 100644 --- a/crates/tinybox-jail/src/linux_tests.rs +++ b/crates/tinybox-jail/src/linux_tests.rs @@ -27,3 +27,33 @@ fn landlock_spawns_with_configured_system_read_paths() -> std::io::Result<()> { assert!(child.wait()?.success()); Ok(()) } + +#[test] +fn landlock_grants_writes_to_read_write_paths_outside_the_root() -> std::io::Result<()> { + let backend = LandlockBackend::new(); + if !backend.is_available() { + return Ok(()); + } + let root = tempfile::tempdir()?; + let scratch = tempfile::tempdir()?; + let denied = tempfile::tempdir()?; + let mut jail = Jail::new(root.path(), "landlock-rw").add_read_write(scratch.path()); + for path in ["/usr", "/bin", "/lib", "/lib64"] { + if Path::new(path).exists() { + jail = jail.add_read_only(path); + } + } + + let write = |dir: &Path| -> std::io::Result { + let mut cmd = Command::new("/bin/sh"); + cmd.arg("-c") + .arg(format!("echo ok > '{}'", dir.join("out").display())); + Ok(backend.spawn(&jail, cmd)?.wait()?.success()) + }; + + assert!(write(scratch.path())?, "read_write path must be writable"); + assert_eq!(std::fs::read_to_string(scratch.path().join("out"))?, "ok\n"); + assert!(!write(denied.path())?, "paths not granted must stay unwritable"); + assert!(!denied.path().join("out").exists()); + Ok(()) +} diff --git a/crates/tinybox-jail/src/macos.rs b/crates/tinybox-jail/src/macos.rs index 4e614d3..86c8d29 100644 --- a/crates/tinybox-jail/src/macos.rs +++ b/crates/tinybox-jail/src/macos.rs @@ -106,15 +106,20 @@ fn render_profile(jail: &Jail) -> String { } // The actual directory jail: deny writes everywhere, then re-allow - // them under root + /private/tmp (the macOS scratchpad most tools - // assume exists and is writable). Shell redirections to /dev/null are - // ordinary output disposal, not a write to another workspace; permit - // that exact device without opening the rest of /dev. + // them under root, every `read_write` path, and /private/tmp (the macOS + // scratchpad most tools assume exists and is writable). Shell + // redirections to /dev/null are ordinary output disposal, not a write to + // another workspace; permit that exact device without opening the rest + // of /dev. out.push_str("(deny file-write*)\n"); - out.push_str(&format!( - "(allow file-write*\n (subpath \"{}\")\n (subpath \"/private/tmp\")\n (literal \"/dev/null\")\n)\n", - escape(&jail.root.to_string_lossy()) - )); + out.push_str("(allow file-write*\n"); + for path in std::iter::once(&jail.root).chain(&jail.read_write) { + out.push_str(&format!( + " (subpath \"{}\")\n", + escape(&path.to_string_lossy()) + )); + } + out.push_str(" (subpath \"/private/tmp\")\n (literal \"/dev/null\")\n)\n"); // `read_only` is informational on macOS — reads are already allowed // by `(allow default)`. We keep the field on `Jail` because Landlock diff --git a/crates/tinybox-jail/src/macos_tests.rs b/crates/tinybox-jail/src/macos_tests.rs index 74e19a2..3011f6c 100644 --- a/crates/tinybox-jail/src/macos_tests.rs +++ b/crates/tinybox-jail/src/macos_tests.rs @@ -202,3 +202,24 @@ fn seatbelt_blocks_write_outside_root() { ); let _ = fs::remove_dir_all(&root); } + +#[test] +fn profile_allows_writes_under_read_write_paths() { + let jail = Jail::new("/work/root", "x") + .add_read_write("/state/capture/1") + .add_read_write("/state/with \"quote\""); + let p = render_profile(&jail); + let allow = p + .split("(allow file-write*") + .nth(1) + .expect("profile has a file-write allow block"); + assert!(allow.contains("(subpath \"/work/root\")")); + assert!(allow.contains("(subpath \"/state/capture/1\")")); + assert!(allow.contains("(subpath \"/state/with \\\"quote\\\"\")")); +} + +#[test] +fn profile_without_read_write_paths_only_allows_root_and_tmp() { + let p = render_profile(&Jail::new("/work/root", "x")); + assert_eq!(p.matches("(subpath ").count(), 2); +} diff --git a/crates/tinybox-jail/src/windows.rs b/crates/tinybox-jail/src/windows.rs index f171fb9..49c1646 100644 --- a/crates/tinybox-jail/src/windows.rs +++ b/crates/tinybox-jail/src/windows.rs @@ -7,7 +7,8 @@ //! //! 1. `CreateAppContainerProfile` → derive a per-jail SID. //! 2. Grant the SID `GENERIC_READ | GENERIC_WRITE | DELETE` on `jail.root` -//! via `SetNamedSecurityInfoW` (additive ACE on the existing DACL). +//! and every `jail.read_write` path via `SetNamedSecurityInfoW` (additive +//! ACE on the existing DACL). //! 3. Build `STARTUPINFOEXW` with `PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES`. //! 4. `CreateProcessW` with `EXTENDED_STARTUPINFO_PRESENT`. //! @@ -37,7 +38,7 @@ use std::ffi::OsStr; use std::io; use std::os::windows::ffi::OsStrExt; use std::os::windows::io::{FromRawHandle, OwnedHandle}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Child, Command}; use std::ptr; @@ -63,7 +64,8 @@ use windows_sys::core::PWSTR; const GENERIC_READ: u32 = 0x8000_0000; const GENERIC_WRITE: u32 = 0x4000_0000; const DELETE: u32 = 0x0001_0000; -const NO_INHERITANCE: u32 = 0; +const OBJECT_INHERIT_ACE: u32 = 0x01; +const CONTAINER_INHERIT_ACE: u32 = 0x02; /// SE_GROUP_ENABLED — marks a SID in a `SID_AND_ATTRIBUTES` entry as active. /// Required for capability SIDs passed to AppContainer SECURITY_CAPABILITIES. /// Source: WinNT.h. @@ -185,10 +187,10 @@ unsafe fn spawn_in_container(jail: &Jail, cmd: Command) -> io::Result { } let _sid_guard = SidGuard(sid); - // 2. Grant the container SID access to the root + read-only paths. - grant_sid_access(&jail.root, sid, GENERIC_READ | GENERIC_WRITE | DELETE)?; - for ro in &jail.read_only { - grant_sid_access(ro, sid, GENERIC_READ)?; + // 2. Grant the container SID access to the root, the extra read/write + // paths, and the read-only paths. + for (path, access) in path_grants(jail) { + grant_sid_access(path, sid, access)?; } // 3. Build SECURITY_CAPABILITIES. AppContainers start with no network @@ -315,6 +317,25 @@ unsafe fn spawn_in_container(jail: &Jail, cmd: Command) -> io::Result { )) } +/// The DACL grants a jail needs: the root and every `read_write` path get +/// read, write and delete; every `read_only` path gets read. +fn path_grants(jail: &Jail) -> Vec<(&Path, u32)> { + let read_write = GENERIC_READ | GENERIC_WRITE | DELETE; + std::iter::once(jail.root.as_path()) + .chain(jail.read_write.iter().map(PathBuf::as_path)) + .map(|path| (path, read_write)) + .chain( + jail.read_only + .iter() + .filter(|path| { + path.as_path() != jail.root.as_path() + && !jail.read_write.iter().any(|rw| rw == *path) + }) + .map(|path| (path.as_path(), GENERIC_READ)), + ) + .collect() +} + unsafe fn grant_sid_access(path: &Path, sid: PSID, access: u32) -> io::Result<()> { let path_w = to_wide(&path.to_string_lossy()); @@ -345,7 +366,7 @@ unsafe fn grant_sid_access(path: &Path, sid: PSID, access: u32) -> io::Result<() let mut ea: EXPLICIT_ACCESS_W = std::mem::zeroed(); ea.grfAccessPermissions = access; ea.grfAccessMode = SET_ACCESS; - ea.grfInheritance = NO_INHERITANCE; + ea.grfInheritance = OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE; ea.Trustee = TRUSTEE_W { pMultipleTrustee: ptr::null_mut(), MultipleTrusteeOperation: 0, diff --git a/crates/tinybox-jail/src/windows_tests.rs b/crates/tinybox-jail/src/windows_tests.rs index b588fc1..97bde31 100644 --- a/crates/tinybox-jail/src/windows_tests.rs +++ b/crates/tinybox-jail/src/windows_tests.rs @@ -69,3 +69,38 @@ fn appcontainer_backend_reports_unavailable_until_child_bridge_lands() { PR #4723 for the orphan-spawn hazard" ); } + +#[test] +fn path_grants_give_read_write_paths_the_same_access_as_the_root() { + let jail = Jail::new(r"C:\work\root", "x") + .add_read_write(r"C:\state\capture\1") + .add_read_only(r"C:\tools"); + let grants = path_grants(&jail); + let rw = GENERIC_READ | GENERIC_WRITE | DELETE; + assert_eq!( + grants, + vec![ + (Path::new(r"C:\work\root"), rw), + (Path::new(r"C:\state\capture\1"), rw), + (Path::new(r"C:\tools"), GENERIC_READ), + ] + ); +} + +#[test] +fn path_grants_keep_write_access_on_paths_also_listed_read_only() { + let jail = Jail::new(r"C:\work\root", "x") + .add_read_write(r"C:\state\capture") + .add_read_only(r"C:\work\root") + .add_read_only(r"C:\state\capture") + .add_read_only(r"C:\tools"); + let rw = GENERIC_READ | GENERIC_WRITE | DELETE; + assert_eq!( + path_grants(&jail), + vec![ + (Path::new(r"C:\work\root"), rw), + (Path::new(r"C:\state\capture"), rw), + (Path::new(r"C:\tools"), GENERIC_READ), + ] + ); +}