Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions crates/tinybox-jail/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 22 additions & 4 deletions crates/tinybox-jail/src/jail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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<PathBuf>,
/// 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<PathBuf>,
/// Allow outbound network. Most agent tools need this; some risky tools
/// (untrusted code execution) should disable it.
pub allow_net: bool,
Expand All @@ -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(),
Expand All @@ -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<Path>) -> 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 {
Expand All @@ -67,15 +85,15 @@ 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
///
/// 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;
}
Expand Down
35 changes: 35 additions & 0 deletions crates/tinybox-jail/src/jail_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<PathBuf>::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::<PathBuf>::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")]);
}
7 changes: 7 additions & 0 deletions crates/tinybox-jail/src/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))?;
Expand Down
30 changes: 30 additions & 0 deletions crates/tinybox-jail/src/linux_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
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(())
}
21 changes: 13 additions & 8 deletions crates/tinybox-jail/src/macos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions crates/tinybox-jail/src/macos_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
37 changes: 29 additions & 8 deletions crates/tinybox-jail/src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
//!
Expand Down Expand Up @@ -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;

Expand All @@ -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.
Expand Down Expand Up @@ -185,10 +187,10 @@ unsafe fn spawn_in_container(jail: &Jail, cmd: Command) -> io::Result<Child> {
}
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)?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

// 3. Build SECURITY_CAPABILITIES. AppContainers start with no network
Expand Down Expand Up @@ -315,6 +317,25 @@ unsafe fn spawn_in_container(jail: &Jail, cmd: Command) -> io::Result<Child> {
))
}

/// 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());

Expand Down Expand Up @@ -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,
Expand Down
35 changes: 35 additions & 0 deletions crates/tinybox-jail/src/windows_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
]
);
}
Loading