diff --git a/Cargo.toml b/Cargo.toml index e5a41184..7aa3f706 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,13 +35,14 @@ categories = ["development-tools", "virtualization"] [workspace.dependencies] # TinyBus defines the message types, interface macro, and frozen module ABI used # by the bus adapter. Socket and CLI features are unnecessary here. -tinybus = { path = "vendor/tinybus/crates/tinybus", version = "0.1.0", default-features = false, features = [ +tinybus = { version = "=0.1.2", default-features = false, features = [ "macros", "modules", ] } # The module-side SDK owns the isolated runtime and exports the ABI entrypoints # required by TinyBus's dynamic loader. -tinybus-module = { path = "vendor/tinybus/crates/tinybus-module", version = "0.1.0" } +tinybus-module = "=0.1.2" + # Derive macros for the crate-wide error type in `tinybox-core`. thiserror = "2" # `Host` and `Sandbox` are object-safe async traits stored as trait objects in a @@ -99,3 +100,7 @@ private_intra_doc_links = "warn" lto = "thin" codegen-units = 1 strip = "debuginfo" + +[patch.crates-io] +tinybus = { path = "vendor/tinybus/crates/tinybus" } +tinybus-module = { path = "vendor/tinybus/crates/tinybus-module" } diff --git a/crates/tinybox-core/src/shell/mod.rs b/crates/tinybox-core/src/shell/mod.rs index 3688e1c8..2da4cddf 100644 --- a/crates/tinybox-core/src/shell/mod.rs +++ b/crates/tinybox-core/src/shell/mod.rs @@ -93,5 +93,9 @@ pub fn script( parts.join(" ") } +pub mod scan; + +#[cfg(test)] +mod scan_test; #[cfg(test)] mod test; diff --git a/crates/tinybox-core/src/shell/scan.rs b/crates/tinybox-core/src/shell/scan.rs new file mode 100644 index 00000000..fafdcfae --- /dev/null +++ b/crates/tinybox-core/src/shell/scan.rs @@ -0,0 +1,604 @@ +//! Quote-aware scanning of a shell command *string*. +//! +//! These helpers are lexical only: they split a command on unquoted +//! separators, find unquoted control characters, blank quoted-heredoc bodies, +//! and normalise a command word. They decide nothing about whether a command +//! is safe; a host that gates execution builds its policy on top of them. +//! Every function is pure and none of them expands or evaluates anything. + +use std::borrow::Cow; +use std::collections::VecDeque; +use std::iter::Peekable; +use std::str::Chars; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum QuoteState { + None, + Single, + Double, +} + +/// Split a shell command into sub-commands by unquoted separators. +/// +/// Separators: +/// - `;` and newline +/// - `|` +/// - `&&`, `||` +/// +/// Characters inside single or double quotes are treated as literals, so +/// `sqlite3 db "SELECT 1; SELECT 2;"` remains a single segment. +#[must_use] +pub fn split_unquoted_segments(command: &str) -> Vec { + let mut segments = Vec::new(); + let mut current = String::new(); + let mut quote = QuoteState::None; + let mut escaped = false; + let mut chars = command.chars().peekable(); + + let push_segment = |segments: &mut Vec, current: &mut String| { + let trimmed = current.trim(); + if !trimmed.is_empty() { + segments.push(trimmed.to_string()); + } + current.clear(); + }; + + while let Some(ch) = chars.next() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + current.push(ch); + } + QuoteState::Double => { + if escaped { + escaped = false; + current.push(ch); + continue; + } + if ch == '\\' { + escaped = true; + current.push(ch); + continue; + } + if ch == '"' { + quote = QuoteState::None; + } + current.push(ch); + } + QuoteState::None => { + if escaped { + escaped = false; + current.push(ch); + continue; + } + if ch == '\\' { + escaped = true; + current.push(ch); + continue; + } + + match ch { + '\'' => { + quote = QuoteState::Single; + current.push(ch); + } + '"' => { + quote = QuoteState::Double; + current.push(ch); + } + ';' | '\n' => push_segment(&mut segments, &mut current), + '|' => { + if chars.next_if_eq(&'|').is_some() { + // Consume full `||`; both characters are separators. + } + push_segment(&mut segments, &mut current); + } + '&' => { + if chars.next_if_eq(&'&').is_some() { + // `&&` is a separator; single `&` is handled separately. + push_segment(&mut segments, &mut current); + } else { + current.push(ch); + } + } + _ => current.push(ch), + } + } + } + } + + let trimmed = current.trim(); + if !trimmed.is_empty() { + segments.push(trimmed.to_string()); + } + + segments +} + +/// Detect a single unquoted `&` operator (background/chain). `&&` is allowed. +/// +/// We treat any standalone `&` as unsafe in policy validation because it can +/// chain hidden sub-commands and escape foreground timeout expectations. +#[must_use] +pub fn contains_unquoted_single_ampersand(command: &str) -> bool { + let mut quote = QuoteState::None; + let mut escaped = false; + let mut chars = command.chars().peekable(); + + while let Some(ch) = chars.next() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + } + QuoteState::Double => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if ch == '"' { + quote = QuoteState::None; + } + } + QuoteState::None => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + match ch { + '\'' => quote = QuoteState::Single, + '"' => quote = QuoteState::Double, + '&' if chars.next_if_eq(&'&').is_none() => { + return true; + } + _ => {} + } + } + } + } + + false +} + +/// Like [`contains_unquoted_single_ampersand`] but ignores file-descriptor +/// duplication redirects, where the `&` is part of a redirect operator rather +/// than a background/separator: `2>&1`, `>&2` (prev char `>`), and `&>file` +/// (next char `>`). Used by a hidden-execution guard so +/// a benign `… 2>&1` — which a classifier already accounts for as a +/// `Write` redirect — is not mistaken for a backgrounded command and +/// hard-blocked after the human approved it. A standalone `&` (e.g. `cmd &`, +/// `a & b`) still returns true, since it can run a second command +/// a classifier wouldn't see. +#[must_use] +pub fn contains_unquoted_background_ampersand(command: &str) -> bool { + let mut quote = QuoteState::None; + let mut escaped = false; + let mut prev = '\0'; + let mut chars = command.chars().peekable(); + + while let Some(ch) = chars.next() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + } + QuoteState::Double => { + if escaped { + escaped = false; + prev = ch; + continue; + } + if ch == '\\' { + escaped = true; + prev = ch; + continue; + } + if ch == '$' && chars.peek() == Some(&'(') { + chars.next(); + let nested = command_substitution_body(&mut chars); + if contains_unquoted_background_ampersand(&nested) { + return true; + } + continue; + } + if ch == '`' { + let nested = backtick_substitution_body(&mut chars); + if contains_unquoted_background_ampersand(&nested) { + return true; + } + continue; + } + if ch == '"' { + quote = QuoteState::None; + } + } + QuoteState::None => { + if escaped { + escaped = false; + prev = ch; + continue; + } + if ch == '\\' { + escaped = true; + prev = ch; + continue; + } + match ch { + '\'' => quote = QuoteState::Single, + '"' => quote = QuoteState::Double, + '&' => { + if chars.next_if_eq(&'&').is_some() { + // `&&` logical AND — consume both, not background. + } else { + let next = chars.peek().copied().unwrap_or('\0'); + // Skip fd-dup redirects: `2>&1`/`>&2` (prev `>`) and + // `&>file` (next `>`). + if prev != '>' && next != '>' { + return true; + } + } + } + _ => {} + } + } + } + prev = ch; + } + + false +} + +fn command_substitution_body(chars: &mut Peekable>) -> String { + let mut nested = String::new(); + let mut depth = 1usize; + let mut quote = QuoteState::None; + let mut escaped = false; + + for inner in chars.by_ref() { + if escaped { + escaped = false; + nested.push(inner); + continue; + } + match quote { + QuoteState::Single if inner == '\'' => quote = QuoteState::None, + QuoteState::Double => match inner { + '\\' => escaped = true, + '"' => quote = QuoteState::None, + _ => {} + }, + QuoteState::None => match inner { + '\\' => escaped = true, + '\'' => quote = QuoteState::Single, + '"' => quote = QuoteState::Double, + '(' => depth += 1, + ')' => { + depth -= 1; + if depth == 0 { + break; + } + } + _ => {} + }, + QuoteState::Single => {} + } + nested.push(inner); + } + + nested +} + +fn backtick_substitution_body(chars: &mut Peekable>) -> String { + let mut nested = String::new(); + let mut escaped_tick = false; + for inner in chars.by_ref() { + if inner == '`' && !escaped_tick { + break; + } + escaped_tick = inner == '\\' && !escaped_tick; + nested.push(inner); + } + nested +} + +/// Detect an unquoted character in a shell command. +#[must_use] +pub fn contains_unquoted_char(command: &str, target: char) -> bool { + let mut quote = QuoteState::None; + let mut escaped = false; + + for ch in command.chars() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + } + QuoteState::Double => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if ch == '"' { + quote = QuoteState::None; + } + } + QuoteState::None => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + match ch { + '\'' => quote = QuoteState::Single, + '"' => quote = QuoteState::Double, + _ if ch == target => return true, + _ => {} + } + } + } + } + + false +} + +/// Blank out the body of every **quoted-delimiter** heredoc in `command`. +/// +/// A command classifier and structural guards scan the raw command string, so +/// until now a heredoc body was read as live shell text. That is wrong for a +/// quoted delimiter: in `cat > f << 'EOF' … EOF` the shell performs no +/// expansion inside the body, so an `&`, a `` ` `` or a `$(` there is data. +/// Measured consequence : a recipe document was +/// refused as "background (&) is not allowed" because four dinner titles read +/// "Chicken & Spinach". +/// +/// An **unquoted** delimiter (`<< EOF`) is deliberately left alone — expansion +/// *is* live inside that body, so `$(rm -rf ~)` there really would execute and +/// must keep tripping the guard. +/// +/// Body lines are replaced by empty lines rather than removed, so line counts, +/// the surrounding command text and the `>` of the redirect are all preserved: +/// a heredoc write still classifies as a write and still +/// prompts. Returns [`Cow::Borrowed`] when the command has no quoted heredoc, +/// which is the overwhelmingly common case. +#[must_use] +pub fn strip_quoted_heredoc_bodies(command: &str) -> Cow<'_, str> { + if !command.contains("<<") { + return Cow::Borrowed(command); + } + + let mut out = String::with_capacity(command.len()); + let mut active_delimiters = VecDeque::new(); + let mut changed = false; + + for line in command.split_inclusive('\n') { + if let Some((delim, quoted)) = active_delimiters.front() { + if line.trim() == delim { + out.push_str(line); + active_delimiters.pop_front(); + } else if *quoted { + // Blank only bodies the shell does not expand. + if line.ends_with('\n') { + out.push('\n'); + } + changed = true; + } else { + out.push_str(line); + } + continue; + } + + out.push_str(line); + let declarations = heredoc_declarations(line); + if declarations.iter().any(|(_, quoted)| *quoted) { + changed = true; + } + active_delimiters.extend(declarations); + } + + if changed { + Cow::Owned(out) + } else { + Cow::Borrowed(command) + } +} + +/// Finds heredocs declared on one executable shell line. Callers must skip +/// heredoc body lines before invoking this function, since their text is data. +fn heredoc_declarations(line: &str) -> VecDeque<(String, bool)> { + let mut declarations = VecDeque::new(); + let mut quote = QuoteState::None; + let mut escaped = false; + let mut chars = line.char_indices().peekable(); + + while let Some((index, ch)) = chars.next() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + } + QuoteState::Double => { + if escaped { + escaped = false; + } else if ch == '\\' { + escaped = true; + } else if ch == '"' { + quote = QuoteState::None; + } + } + QuoteState::None => { + if escaped { + escaped = false; + continue; + } + match ch { + '\\' => escaped = true, + '\'' => quote = QuoteState::Single, + '"' => quote = QuoteState::Double, + '<' if chars.next_if(|(_, next)| *next == '<').is_some() => { + // `<<<` is a here-string, not a heredoc. + if chars.next_if(|(_, next)| *next == '<').is_some() { + continue; + } + if let Some((delimiter, quoted, consumed)) = + heredoc_delimiter(&line[index + 2..]) + { + declarations.push_back((delimiter, quoted)); + for _ in 0..consumed { + chars.next(); + } + } + } + _ => {} + } + } + } + } + + declarations +} + +/// Parse one complete shell delimiter word after `<<`, preserving whether any +/// part was quoted or escaped and how many characters it occupies. +fn heredoc_delimiter(rest: &str) -> Option<(String, bool, usize)> { + let mut consumed = 0usize; + let mut chars = rest.chars().peekable(); + + if chars.peek() == Some(&'-') { + chars.next(); + consumed += 1; + } + while chars.peek().is_some_and(|c| *c == ' ' || *c == '\t') { + chars.next(); + consumed += 1; + } + + let mut delim = String::new(); + let mut quote = None; + let mut quoted = false; + let mut escaped = false; + for c in chars { + consumed += 1; + if escaped { + delim.push(c); + escaped = false; + quoted = true; + continue; + } + if c == '\\' && quote != Some('\'') { + escaped = true; + quoted = true; + continue; + } + if let Some(active) = quote { + if c == '\n' { + return None; + } + if c == active { + quote = None; + } else { + delim.push(c); + } + } else if c == '\'' || c == '"' { + quote = Some(c); + quoted = true; + } else if c.is_whitespace() || ";&|<>()".contains(c) { + consumed -= 1; + break; + } else { + delim.push(c); + } + } + (quote.is_none() && !escaped && !delim.is_empty()).then_some((delim, quoted, consumed)) +} + +/// The last path component of a command word, splitting on both `/` and `\`. +#[must_use] +pub fn command_basename(command: &str) -> &str { + command.split(['/', '\\']).next_back().unwrap_or(command) +} + +/// The lowercased basename of a command word with a trailing `.exe` removed. +#[must_use] +pub fn normalized_command_name(command: &str) -> String { + let command = command_basename(command).to_ascii_lowercase(); + command + .strip_suffix(".exe") + .unwrap_or(command.as_str()) + .to_string() +} + +/// Returns true if `s` starts with at least one inline env-var +/// assignment of the shape `NAME=...`, where `NAME` begins with an ASCII +/// letter or underscore. Matches any name, so `GIT_SSH=…`, `SSH_ASKPASS=…`, `LD_PRELOAD=…` and +/// `IFS=…` all count; deciding which of them are dangerous is the caller's job. +#[must_use] +pub fn has_leading_env_assignment(s: &str) -> bool { + let Some(word) = s.split_whitespace().next() else { + return false; + }; + let Some((name, _value)) = word.split_once('=') else { + return false; + }; + if name.is_empty() { + return false; + } + // Identifier shape: first char letter or `_`, the rest alphanumeric + // or `_`. Anything else (e.g. `foo[bar]=`) is not a shell assignment. + let mut chars = name.chars(); + let Some(first) = chars.next() else { + return false; + }; + if !(first.is_ascii_alphabetic() || first == '_') { + return false; + } + if !chars.all(|c| c.is_ascii_alphanumeric() || c == '_') { + return false; + } + true +} + +/// Skip leading environment variable assignments (e.g. `FOO=bar cmd args`). +/// Returns the remainder starting at the first non-assignment word. +#[must_use] +pub fn skip_env_assignments(s: &str) -> &str { + let mut rest = s; + loop { + rest = rest.trim_start(); + let Some(word) = rest.split_whitespace().next() else { + return rest; + }; + // Environment assignment: contains '=' and starts with a letter or underscore + if word.contains('=') + && word + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphabetic() || c == '_') + { + // Advance past this word + rest = &rest[word.len()..]; + } else { + return rest; + } + } +} diff --git a/crates/tinybox-core/src/shell/scan_test.rs b/crates/tinybox-core/src/shell/scan_test.rs new file mode 100644 index 00000000..2c12ca8a --- /dev/null +++ b/crates/tinybox-core/src/shell/scan_test.rs @@ -0,0 +1,291 @@ +//! Tests for the quote-aware shell scanners. +//! +//! A host that gates command execution relies on these, so every case here is +//! one such a host was found to depend on: quoting that must hide a separator, +//! an escape that must not, and heredoc bodies that are data only when their +//! delimiter is quoted. + +use std::borrow::Cow; + +use super::scan::{ + command_basename, contains_unquoted_background_ampersand, contains_unquoted_char, + contains_unquoted_single_ampersand, has_leading_env_assignment, normalized_command_name, + skip_env_assignments, split_unquoted_segments, strip_quoted_heredoc_bodies, +}; + +fn segs(command: &str) -> Vec { + split_unquoted_segments(command) +} + +#[test] +fn splits_on_every_unquoted_separator() { + assert_eq!(segs("a; b"), ["a", "b"]); + assert_eq!(segs("a\nb"), ["a", "b"]); + assert_eq!(segs("a | b"), ["a", "b"]); + assert_eq!(segs("a && b"), ["a", "b"]); + assert_eq!(segs("a || b"), ["a", "b"]); + assert_eq!(segs("a && b || c; d | e"), ["a", "b", "c", "d", "e"]); +} + +#[test] +fn quotes_hide_separators() { + assert_eq!( + segs(r#"sqlite3 db "SELECT 1; SELECT 2;""#), + [r#"sqlite3 db "SELECT 1; SELECT 2;""#] + ); + assert_eq!(segs("echo 'a | b && c'"), ["echo 'a | b && c'"]); + assert_eq!(segs("echo 'a'; echo \"b\""), ["echo 'a'", "echo \"b\""]); +} + +#[test] +fn backslash_escapes_a_separator_outside_and_inside_double_quotes() { + assert_eq!(segs(r"echo a\; b"), [r"echo a\; b"]); + assert_eq!(segs(r#"echo "a\"; b""#), [r#"echo "a\"; b""#]); + // Inside single quotes a backslash is literal, so the quote still closes. + assert_eq!(segs(r"echo 'a\'; b"), [r"echo 'a\'", "b"]); +} + +#[test] +fn single_ampersand_stays_inside_its_segment() { + assert_eq!(segs("a & b"), ["a & b"]); + assert_eq!(segs("cmd 2>&1 && next"), ["cmd 2>&1", "next"]); +} + +#[test] +fn empty_and_blank_segments_are_dropped() { + assert!(segs("").is_empty()); + assert!(segs(" ; ;\n").is_empty()); + assert_eq!(segs(";;a;;"), ["a"]); +} + +#[test] +fn single_ampersand_detection() { + assert!(contains_unquoted_single_ampersand("sleep 1 &")); + assert!(contains_unquoted_single_ampersand("a & b")); + assert!(!contains_unquoted_single_ampersand("a && b")); + assert!(!contains_unquoted_single_ampersand("echo 'a & b'")); + assert!(!contains_unquoted_single_ampersand(r#"echo "a & b""#)); + assert!(contains_unquoted_single_ampersand(r"echo \& &")); + assert!(!contains_unquoted_single_ampersand(r"echo \&")); + // The fd-dup redirect counts here; the background variant is the lenient one. + assert!(contains_unquoted_single_ampersand("cmd 2>&1")); +} + +#[test] +fn background_ampersand_ignores_fd_duplication() { + assert!(!contains_unquoted_background_ampersand("cmd 2>&1")); + assert!(!contains_unquoted_background_ampersand("cmd >&2")); + assert!(!contains_unquoted_background_ampersand("cmd &>out.log")); + assert!(!contains_unquoted_background_ampersand("a && b")); + assert!(contains_unquoted_background_ampersand("cmd &")); + assert!(contains_unquoted_background_ampersand("a & b")); + assert!(contains_unquoted_background_ampersand("cmd 2>&1 &")); + assert!(!contains_unquoted_background_ampersand("echo 'a & b'")); + assert!(!contains_unquoted_background_ampersand(r#"echo "a & b""#)); + assert!(!contains_unquoted_background_ampersand(r"echo a\&b")); + assert!(contains_unquoted_background_ampersand( + r#"echo "$(malicious &)""# + )); + assert!(contains_unquoted_background_ampersand( + r#"echo "`malicious &`""# + )); + assert!(contains_unquoted_background_ampersand( + r#"echo "$(echo \) &)""# + )); +} + +#[test] +fn unquoted_char_detection() { + assert!(contains_unquoted_char("echo hi > out", '>')); + assert!(!contains_unquoted_char("echo '>'", '>')); + assert!(!contains_unquoted_char(r#"echo ">""#, '>')); + assert!(!contains_unquoted_char(r"echo \>", '>')); + assert!(contains_unquoted_char(r#"echo "x" > f"#, '>')); + assert!(contains_unquoted_char("a`b", '`')); + assert!(!contains_unquoted_char("", '>')); + // An escaped quote inside double quotes does not close them. + assert!(!contains_unquoted_char(r#"echo "a\" > b""#, '>')); +} + +const MEAL_PLAN: &str = "cat > out/meal_plan.md << 'EOF'\n\ + # Dinners\n\ + - Chicken & Spinach\n\ + - `backtick` and $(subshell)\n\ + EOF"; + +#[test] +fn quoted_heredoc_body_is_blanked_and_shape_preserved() { + let out = strip_quoted_heredoc_bodies(MEAL_PLAN); + assert_eq!(out, "cat > out/meal_plan.md << 'EOF'\n\n\n\nEOF"); + assert_eq!(out.lines().count(), MEAL_PLAN.lines().count()); + assert!(out.contains('>')); +} + +#[test] +fn heredocs_are_consumed_in_input_order_and_delimiters_are_complete_words() { + let mixed = "cat < out/$(whoami).md << 'EOF'\n\nEOF\nrm -rf /tmp/x"); +} + +#[test] +fn unterminated_heredoc_blanks_to_the_end() { + let out = strip_quoted_heredoc_bodies("cat << 'EOF'\nbody & more\nstill body"); + assert_eq!(out, "cat << 'EOF'\n\n"); +} + +#[test] +fn delimiter_line_is_matched_after_trimming() { + let out = strip_quoted_heredoc_bodies("cat << 'EOF'\nEOF is not it\n EOF \nnext"); + assert_eq!(out, "cat << 'EOF'\n\n EOF \nnext"); +} + +#[test] +fn empty_or_multiline_quoted_delimiter_is_not_a_heredoc() { + assert!(matches!( + strip_quoted_heredoc_bodies("cat << ''\nbody\n"), + Cow::Borrowed(_) + )); + assert!(matches!( + strip_quoted_heredoc_bodies("cat << 'EO\nF'\nbody\n"), + Cow::Borrowed(_) + )); +} + +#[test] +fn basename_handles_both_separators() { + assert_eq!(command_basename("/usr/bin/git"), "git"); + assert_eq!(command_basename(r"C:\Windows\System32\cmd.exe"), "cmd.exe"); + assert_eq!(command_basename("git"), "git"); + assert_eq!(command_basename(""), ""); + assert_eq!(command_basename("/bin/"), ""); +} + +#[test] +fn normalized_name_lowercases_and_strips_exe() { + assert_eq!(normalized_command_name("/usr/bin/GIT"), "git"); + assert_eq!( + normalized_command_name(r"C:\x\PowerShell.EXE"), + "powershell" + ); + assert_eq!(normalized_command_name("python3"), "python3"); + assert_eq!(normalized_command_name("a.exe.exe"), "a.exe"); +} + +#[test] +fn leading_env_assignment_shape() { + assert!(has_leading_env_assignment("FOO=bar cmd")); + assert!(has_leading_env_assignment(" _x1=1 cmd")); + assert!(has_leading_env_assignment("LD_PRELOAD=/x ls")); + assert!(has_leading_env_assignment("A= cmd")); + assert!(!has_leading_env_assignment("cmd FOO=bar")); + assert!(!has_leading_env_assignment("=bar cmd")); + assert!(!has_leading_env_assignment("1A=b cmd")); + assert!(!has_leading_env_assignment("foo[bar]=1 cmd")); + assert!(!has_leading_env_assignment("git log")); + assert!(!has_leading_env_assignment("")); +} + +#[test] +fn skipping_env_assignments() { + assert_eq!(skip_env_assignments("A=1 B=2 git log"), "git log"); + assert_eq!(skip_env_assignments(" A=1 B=2 git log"), "git log"); + assert_eq!(skip_env_assignments("git log"), "git log"); + assert_eq!(skip_env_assignments("A=1"), ""); + assert_eq!(skip_env_assignments(""), ""); + // `=` in a non-identifier word is a command, not an assignment. + assert_eq!(skip_env_assignments("1A=b cmd"), "1A=b cmd"); + // The assignment filter is intentionally looser than the shape check. + assert_eq!(skip_env_assignments("foo[bar]=1 cmd"), "cmd"); +} diff --git a/crates/tinybox-jail/Cargo.toml b/crates/tinybox-jail/Cargo.toml index e37fa292..db6a4b85 100644 --- a/crates/tinybox-jail/Cargo.toml +++ b/crates/tinybox-jail/Cargo.toml @@ -43,27 +43,5 @@ windows-sys = { version = "0.61", features = [ [dev-dependencies] tempfile = "3" -# This crate is FFI-heavy by nature (Win32 AppContainer, `pre_exec` for -# Landlock), so the workspace-wide `unsafe_code = "forbid"` cannot apply. Every -# other lint keeps the workspace level. -[lints.rust] -unsafe_code = "allow" -missing_docs = "warn" -missing_debug_implementations = "warn" -unreachable_pub = "warn" -rust_2018_idioms = { level = "warn", priority = -1 } - -# Ported wholesale from the OpenHuman host, which did not run `pedantic`; the -# doc-section lints (`missing_errors_doc`, ...) are left off until the public -# surface is documented to that standard. Tests may unwrap (see `lib.rs`). -[lints.clippy] -all = { level = "warn", priority = -1 } -unwrap_used = "warn" -expect_used = "warn" -panic = "warn" -todo = "warn" -unimplemented = "warn" - -[lints.rustdoc] -broken_intra_doc_links = "warn" -private_intra_doc_links = "warn" +[lints] +workspace = true diff --git a/crates/tinybox-jail/README.md b/crates/tinybox-jail/README.md index 5dcaf291..de14f71f 100644 --- a/crates/tinybox-jail/README.md +++ b/crates/tinybox-jail/README.md @@ -1,9 +1,11 @@ # cwd_jail -Cross-platform directory-jail facade. Given a declarative description of a -workspace (`Jail`), it picks the strongest available OS sandbox backend and -spawns a child process caged into a single read/write root (plus optional -read-only paths), with toggles for outbound network and subprocess creation. +Directory-jail facade. Given a declarative description of a workspace +(`Jail`), it spawns a child through an available sandbox backend. Platform +backends are currently disabled until they can satisfy the workspace safety +policy and enforce the declared jail contract. The default backend therefore +returns `Unsupported`; callers can explicitly select `NoopBackend` when +unrestricted execution is intended. It is a per-process complement to the box-level isolation in `tinybox-linux`: the autonomy gate decides whether a command may run, and `cwd_jail` decides what filesystem the approved child process sees. It jails the child it @@ -13,8 +15,8 @@ spawns, never the core process itself. - Describe a jail declaratively via a builder (`Jail::new(root, label)` plus `.add_read_only(...)`, `.deny_net()`, `.deny_subprocess()`). -- Auto-detect and cache the strongest backend for the current OS (Landlock, - Seatbelt, or AppContainer). +- 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 trickery. @@ -30,11 +32,11 @@ spawns, never the core process itself. | --- | --- | | `crates/tinybox-jail/src/lib.rs` | Module docstring plus the thin facade: `spawn` / `spawn_with` / `default_backend` (cached via `OnceLock`). Re-exports the public surface. | | `crates/tinybox-jail/src/jail.rs` | Core types: the `Jail` description struct (builder plus `canonicalize`/`canonicalize_or_log`) and the `JailBackend` trait (`name`/`is_available`/`spawn`). | -| `crates/tinybox-jail/src/detect.rs` | `pick_backend()`: cfg-gated platform selection; returns the first available backend or `NoopBackend`. | +| `crates/tinybox-jail/src/detect.rs` | `pick_backend()`: returns an unsupported backend until a compliant platform backend is available. | | `crates/tinybox-jail/src/noop.rs` | `NoopBackend`: no enforcement, plain `Command::spawn`. Always available. | -| `crates/tinybox-jail/src/linux.rs` | `LandlockBackend`: kernel 5.13+ Landlock LSM applied in `pre_exec` (child-side, after fork, before exec). Gated on the `landlock` cargo feature. | -| `crates/tinybox-jail/src/macos.rs` | `SeatbeltBackend`: wraps the command in `/usr/bin/sandbox-exec -p ''`. Renders an allow-default-reads / deny-default-writes Seatbelt profile. | -| `crates/tinybox-jail/src/windows.rs` | `AppContainerBackend`: `CreateAppContainerProfile` plus a DACL grant and `STARTUPINFOEX`/`CreateProcessW` via `windows-sys`. | +| `crates/tinybox-jail/src/linux.rs` | Proposed Landlock implementation; currently not compiled or selected. | +| `crates/tinybox-jail/src/macos.rs` | Proposed Seatbelt implementation; currently not compiled or selected. | +| `crates/tinybox-jail/src/windows.rs` | Proposed AppContainer implementation; currently not compiled or selected. | | `crates/tinybox-jail/src/registry.rs` | `JailRegistry` and `JailRecord`: multi-jail manager persisted to `index.json`, with atomic-rename writes and containment checks. | | `crates/tinybox-jail/src/{lib,jail,noop,macos,windows,registry}_tests.rs` | Sibling test suites, each `#[path]`-included from its source file. | diff --git a/crates/tinybox-jail/src/detect.rs b/crates/tinybox-jail/src/detect.rs index 60118abb..a3087202 100644 --- a/crates/tinybox-jail/src/detect.rs +++ b/crates/tinybox-jail/src/detect.rs @@ -1,9 +1,9 @@ //! Platform auto-detection. Picks the strongest available backend. +use std::process::{Child, Command}; use std::sync::Arc; use super::jail::{Jail, JailBackend}; -use std::process::{Child, Command}; #[derive(Debug)] struct UnsupportedBackend; @@ -27,31 +27,8 @@ impl JailBackend for UnsupportedBackend { /// Picks the strongest available backend, returning an unsupported backend /// when no OS sandbox works. +#[must_use] pub fn pick_backend() -> Arc { - #[cfg(target_os = "linux")] - { - let lb = super::linux::LandlockBackend::new(); - if lb.is_available() { - log::info!("[cwd_jail] backend=landlock"); - return Arc::new(lb); - } - } - #[cfg(target_os = "macos")] - { - let sb = super::macos::SeatbeltBackend::new(); - if sb.is_available() { - log::info!("[cwd_jail] backend=seatbelt"); - return Arc::new(sb); - } - } - #[cfg(target_os = "windows")] - { - let ac = super::windows::AppContainerBackend::new(); - if ac.is_available() { - log::info!("[cwd_jail] backend=appcontainer"); - return Arc::new(ac); - } - } log::warn!("[cwd_jail] no OS sandbox available"); Arc::new(UnsupportedBackend) } diff --git a/crates/tinybox-jail/src/detect_tests.rs b/crates/tinybox-jail/src/detect_tests.rs index b76e9da1..3e371e5f 100644 --- a/crates/tinybox-jail/src/detect_tests.rs +++ b/crates/tinybox-jail/src/detect_tests.rs @@ -1,20 +1,21 @@ -//! Tests for backend selection and the unavailable-backend result. +//! Tests for platform backend selection and the unsupported fallback. use super::*; #[test] -fn unavailable_backend_rejects_spawning() { +fn unsupported_backend_reports_and_rejects_spawns() { let backend = UnsupportedBackend; assert_eq!(backend.name(), "unsupported"); assert!(!backend.is_available()); let error = backend - .spawn(&Jail::new(".", "unsupported"), Command::new("true")) - .err() - .map(|error| error.kind()); - assert_eq!(error, Some(std::io::ErrorKind::Unsupported)); + .spawn(&Jail::new("/", "unsupported"), Command::new("true")) + .expect_err("unsupported backend must reject a spawn"); + assert_eq!(error.kind(), std::io::ErrorKind::Unsupported); } #[test] -fn backend_detection_returns_a_backend() { - assert!(!pick_backend().name().is_empty()); +fn backend_selection_returns_the_unsupported_backend() { + let backend = pick_backend(); + assert_eq!(backend.name(), "unsupported"); + assert!(!backend.is_available()); } diff --git a/crates/tinybox-jail/src/jail.rs b/crates/tinybox-jail/src/jail.rs index f007ac52..0289e49d 100644 --- a/crates/tinybox-jail/src/jail.rs +++ b/crates/tinybox-jail/src/jail.rs @@ -13,7 +13,7 @@ use std::process::{Child, Command}; /// 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. /// Backends translate this into Landlock rules, a Seatbelt profile, or an -/// AppContainer ACL. +/// `AppContainer` ACL. #[derive(Debug, Clone)] pub struct Jail { /// Primary read/write root. The child cannot escape this directory for @@ -25,11 +25,11 @@ pub struct Jail { /// Allow outbound network. Most agent tools need this; some risky tools /// (untrusted code execution) should disable it. pub allow_net: bool, - /// Allow the child to spawn further child processes. AppContainer and + /// Allow the child to spawn further child processes. `AppContainer` and /// Seatbelt can deny this; Landlock cannot. pub allow_subprocess: bool, /// Free-form label used by audit logs and (on Windows) as the basis for - /// the AppContainer profile name. Keep it short and ASCII. + /// the `AppContainer` profile name. Keep it short and ASCII. pub label: String, } @@ -47,18 +47,21 @@ impl Jail { } /// Grants read (and execute) access to an extra path outside the root. + #[must_use] pub fn add_read_only(mut self, path: impl AsRef) -> Self { self.read_only.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 { self.allow_net = false; self } /// Asks the backend to forbid spawning further subprocesses where it can. + #[must_use] pub fn deny_subprocess(mut self) -> Self { self.allow_subprocess = false; self @@ -66,9 +69,13 @@ impl Jail { /// Canonicalize `root` and `read_only` 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 self.read_only.iter_mut() { + for p in &mut self.read_only { if let Ok(c) = p.canonicalize() { *p = c; } @@ -93,7 +100,7 @@ impl Jail { /// OS-specific enforcement of a [`Jail`]. /// /// We model spawning rather than `Command` mutation because Windows -/// AppContainer requires custom `CreateProcess` flags that `std`'s +/// `AppContainer` requires custom `CreateProcess` flags that `std`'s /// `Command::spawn` does not expose. pub trait JailBackend: Send + Sync { /// Stable identifier, used in logs / audit ("landlock", "seatbelt", @@ -107,7 +114,11 @@ pub trait JailBackend: Send + Sync { /// Spawn `cmd` under the jail described by `jail`. Backends own how the /// jail is materialized (Landlock ruleset, sandbox-exec wrapper, - /// AppContainer profile + restricted token). + /// `AppContainer` profile + restricted token). + /// + /// # Errors + /// + /// Returns an error if the backend cannot apply the jail or spawn command. fn spawn(&self, jail: &Jail, cmd: Command) -> std::io::Result; } diff --git a/crates/tinybox-jail/src/lib.rs b/crates/tinybox-jail/src/lib.rs index 77f2b14b..943e6ce8 100644 --- a/crates/tinybox-jail/src/lib.rs +++ b/crates/tinybox-jail/src/lib.rs @@ -1,4 +1,4 @@ -//! Directory jail (cwd_jail): jail an agent/tool into a single workspace. +//! Directory jail (`cwd_jail)`: jail an agent/tool into a single workspace. //! //! ## Why this exists //! @@ -50,12 +50,8 @@ pub mod jail; pub mod noop; pub mod registry; -#[cfg(target_os = "linux")] -pub mod linux; -#[cfg(target_os = "macos")] -pub mod macos; -#[cfg(target_os = "windows")] -pub mod windows; +// Platform backends are intentionally not compiled until they can preserve +// the workspace unsafe-code policy and enforce the public jail contract. pub use jail::{Jail, JailBackend}; pub use noop::{NOOP_BACKEND_NAME, NoopBackend}; @@ -78,6 +74,11 @@ pub fn default_backend() -> Arc { /// `..` or symlinks. If the root does not exist, the spawn fails with /// `NotFound` (canonicalize bubbles it up) — callers should create the /// workspace before encapsulating. +/// +/// # Errors +/// +/// Returns an error if the jail root cannot be canonicalized or the backend +/// rejects the command. pub fn spawn(jail: &Jail, cmd: Command) -> std::io::Result { let mut jail = jail.clone(); jail.canonicalize()?; @@ -87,6 +88,11 @@ pub fn spawn(jail: &Jail, cmd: Command) -> std::io::Result { /// Same as [`spawn`] but with a caller-supplied backend. Useful in /// tests and for callers that want to opt into a weaker backend /// explicitly (e.g. forcing [`NoopBackend`] during local dev). +/// +/// # Errors +/// +/// Returns an error if the jail root cannot be canonicalized or the backend +/// rejects the command. pub fn spawn_with(backend: &dyn JailBackend, jail: &Jail, cmd: Command) -> std::io::Result { let mut jail = jail.clone(); jail.canonicalize()?; diff --git a/crates/tinybox-jail/src/linux.rs b/crates/tinybox-jail/src/linux.rs index e6685d4a..44c62504 100644 --- a/crates/tinybox-jail/src/linux.rs +++ b/crates/tinybox-jail/src/linux.rs @@ -91,13 +91,7 @@ impl JailBackend for LandlockBackend { // SAFETY: the child callback only applies this prebuilt ruleset. unsafe { - cmd.pre_exec(move || match ruleset.take() { - Some(ruleset) => match ruleset.restrict_self() { - Ok(_) => Ok(()), - Err(_) => Err(std::io::Error::from_raw_os_error(5)), - }, - None => Err(std::io::Error::from_raw_os_error(22)), - }); + cmd.pre_exec(move || apply_ruleset(ruleset.take(), restrict_self)); } cmd.spawn() @@ -110,6 +104,25 @@ impl JailBackend for LandlockBackend { } } -#[cfg(all(test, feature = "landlock"))] +#[cfg(feature = "landlock")] +fn apply_ruleset( + ruleset: Option, + restrict: impl FnOnce(T) -> std::io::Result<()>, +) -> std::io::Result<()> { + match ruleset { + Some(ruleset) => restrict(ruleset), + None => Err(std::io::Error::from_raw_os_error(22)), + } +} + +#[cfg(feature = "landlock")] +fn restrict_self(ruleset: landlock::Ruleset) -> std::io::Result<()> { + ruleset + .restrict_self() + .map(|_| ()) + .map_err(|_| std::io::Error::from_raw_os_error(5)) +} + +#[cfg(test)] #[path = "linux_tests.rs"] mod tests; diff --git a/crates/tinybox-jail/src/linux_tests.rs b/crates/tinybox-jail/src/linux_tests.rs index bfcfd01a..84c6bac6 100644 --- a/crates/tinybox-jail/src/linux_tests.rs +++ b/crates/tinybox-jail/src/linux_tests.rs @@ -1,29 +1,82 @@ -//! Tests for the Linux Landlock backend. +//! Tests for Landlock setup and child spawning. use super::*; use std::path::Path; +#[cfg(feature = "landlock")] #[test] -fn landlock_spawns_with_configured_system_read_paths() -> std::io::Result<()> { - let root = tempfile::tempdir()?; - let mut jail = Jail::new(root.path(), "landlock-test"); +fn apply_ruleset_handles_success_failure_and_missing_ruleset() { + assert!(apply_ruleset(Some(()), |_| Ok(())).is_ok()); + assert_eq!( + apply_ruleset(Some(()), |_| Err(std::io::Error::other("failed"))) + .err() + .map(|error| error.kind()), + Some(std::io::ErrorKind::Other) + ); + assert_eq!( + apply_ruleset::<()>(None, |_| Ok(())) + .err() + .map(|error| error.raw_os_error()), + Some(Some(22)) + ); +} + +#[test] +fn backend_reports_name_and_availability() { + let backend = LandlockBackend::new(); + assert_eq!(backend.name(), "landlock"); + let _ = backend.is_available(); + let default_backend: LandlockBackend = Default::default(); + assert_eq!(default_backend.name(), "landlock"); +} + +#[test] +#[cfg(feature = "landlock")] +fn spawn_rejects_a_missing_read_only_path() { + let root = tempfile::tempdir().expect("temporary jail root"); + let jail = Jail::new(root.path(), "missing-read-only") + .add_read_only(root.path().join("missing-read-only-path")); + let result = LandlockBackend::new().spawn(&jail, Command::new("true")); + assert_eq!( + result.err().map(|error| error.kind()), + Some(std::io::ErrorKind::Other) + ); +} + +#[test] +fn spawn_applies_rules_before_running_the_command_when_supported() { + let root = tempfile::tempdir().expect("temporary jail root"); + let mut jail = Jail::new(root.path(), "landlock-spawn"); for path in ["/usr", "/bin", "/lib", "/lib64"] { if Path::new(path).exists() { jail = jail.add_read_only(path); } } - let backend = LandlockBackend::new(); - if !backend.is_available() { - let error = backend + #[cfg(not(feature = "landlock"))] + { + let mut child = backend .spawn(&jail, Command::new("/usr/bin/true")) - .err() - .map(|error| error.kind()); - assert_eq!(error, Some(std::io::ErrorKind::PermissionDenied)); - return Ok(()); + .expect("spawn without Landlock support"); + assert!(child.wait().expect("wait for command").success()); + return; } - let mut child = backend.spawn(&jail, Command::new("/usr/bin/true"))?; - assert!(child.wait()?.success()); - Ok(()) + #[cfg(feature = "landlock")] + { + if !backend.is_available() { + let error = backend + .spawn(&jail, Command::new("/usr/bin/true")) + .err() + .map(|error| error.kind()); + assert_eq!(error, Some(std::io::ErrorKind::PermissionDenied)); + return; + } + + let mut child = backend + .spawn(&jail, Command::new("/usr/bin/true")) + .expect("spawn with Landlock restrictions"); + let status = child.wait().expect("wait for jailed command"); + assert!(status.success()); + } } diff --git a/crates/tinybox-jail/src/mod_tests.rs b/crates/tinybox-jail/src/mod_tests.rs index cb3eb22d..d12eb091 100644 --- a/crates/tinybox-jail/src/mod_tests.rs +++ b/crates/tinybox-jail/src/mod_tests.rs @@ -70,8 +70,12 @@ fn spawn_uses_default_backend() { // `spawn` entry rather than `spawn_with`. let result = spawn(&jail, cmd); if default_backend().is_available() { - let mut child = result.expect("spawn through available backend"); - let _ = child.wait().expect("wait"); + match result { + Ok(mut child) => { + let _ = child.wait(); + } + Err(error) => assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied), + } } else { assert_eq!( result.err().map(|error| error.kind()), @@ -80,6 +84,24 @@ fn spawn_uses_default_backend() { } } +#[test] +fn spawn_uses_default_backend_for_an_existing_root() { + let dir = tempfile::tempdir().expect("temporary directory"); + let result = super::spawn( + &Jail::new(dir.path(), "default-spawn"), + Command::new("true"), + ); + match result { + Ok(mut child) => { + let _ = child.wait(); + } + Err(error) => assert!(matches!( + error.kind(), + std::io::ErrorKind::Unsupported | std::io::ErrorKind::PermissionDenied + )), + } +} + #[test] fn canonicalize_or_log_does_not_panic_on_missing() { // The lossy helper is supposed to log + continue rather than diff --git a/crates/tinybox-jail/src/noop_tests.rs b/crates/tinybox-jail/src/noop_tests.rs index dc269103..a02279b3 100644 --- a/crates/tinybox-jail/src/noop_tests.rs +++ b/crates/tinybox-jail/src/noop_tests.rs @@ -3,9 +3,9 @@ use super::*; /// Regression guard for #3235. /// /// On platforms where no OS-level jail backend is available -/// (Landlock / Seatbelt / AppContainer absent or unsupported), +/// (Landlock / Seatbelt / `AppContainer` absent or unsupported), /// `cwd_jail::detect::pick_backend()` returns `NoopBackend`. This -/// test pins the NoopBackend contract — it must always report +/// test pins the `NoopBackend` contract — it must always report /// `is_available() == true` so it's a usable fallback, must /// identify as `"noop"` so operators can see in logs which backend /// is active, and `spawn` must be a passthrough that runs the diff --git a/crates/tinybox-jail/src/registry.rs b/crates/tinybox-jail/src/registry.rs index 38df2861..9df263af 100644 --- a/crates/tinybox-jail/src/registry.rs +++ b/crates/tinybox-jail/src/registry.rs @@ -6,8 +6,8 @@ //! //! - A stable **id** (UUID-ish, used in paths and the index). //! - A user-visible **label** (free text, displayed in UI, used for -//! AppContainer profile derivation on Windows). -//! - A **directory** at `//` that the [`crate::Jail`] +//! `AppContainer` profile derivation on Windows). +//! - A **directory** at `//` that the [`Jail`] //! is rooted in. //! - **Metadata**: created/updated timestamps, backend used at create //! time, optional notes. @@ -62,7 +62,7 @@ pub struct JailRecord { #[derive(Debug, Default, Serialize, Deserialize)] struct Index { - /// id → record. BTreeMap so list() is deterministically ordered. + /// id → record. `BTreeMap` so `list()` is deterministically ordered. records: BTreeMap, #[serde(default)] schema_version: u32, @@ -83,6 +83,11 @@ impl JailRegistry { /// Open (or create) a registry rooted at `base`. The directory is /// created if it does not exist; the index file is loaded if present /// and seeded blank otherwise. + /// + /// # Errors + /// + /// Returns an error if the base directory cannot be created or the index + /// cannot be read or decoded. pub fn open(base: impl AsRef) -> io::Result { Self::open_with_clock(base, Arc::new(SystemClock::new())) } @@ -130,6 +135,14 @@ impl JailRegistry { } /// Create a new jail directory. Returns the persisted record. + /// + /// # Errors + /// + /// Returns an error if directory creation or index persistence fails. + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn create(&self, label: impl Into) -> io::Result { let label = label.into(); // Label is free-form user input — log only its length so we get @@ -143,12 +156,18 @@ impl JailRegistry { let mut idx = self.index.lock().unwrap(); let (id, dir) = loop { let candidate = generate_id(now_unix(self.clock.as_ref())); - if !idx.records.contains_key(&candidate) { + if idx.records.contains_key(&candidate) { + log::trace!("[cwd_jail] id collision, regenerating"); + } else { let dir = self.base.join(&candidate); - fs::create_dir_all(&dir)?; - break (candidate, dir); + match fs::create_dir(&dir) { + Ok(()) => break (candidate, dir), + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => { + log::trace!("[cwd_jail] id directory collision, regenerating"); + } + Err(error) => return Err(error), + } } - log::trace!("[cwd_jail] id collision, regenerating"); }; let now = now_unix(self.clock.as_ref()); @@ -180,11 +199,19 @@ impl JailRegistry { } /// Look up a jail by id. + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn get(&self, id: &str) -> Option { self.index.lock().unwrap().records.get(id).cloned() } /// List every active jail. Deterministic order (by id). + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn list(&self) -> Vec { self.index .lock() @@ -196,6 +223,10 @@ impl JailRegistry { } /// Search by label substring (case-insensitive). Useful for UI. + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn find_by_label(&self, needle: &str) -> Vec { let needle = needle.to_lowercase(); self.index @@ -209,8 +240,16 @@ impl JailRegistry { } /// Rename: changes the *label* only. The directory id stays put so - /// existing path references keep working. AppContainer profile names + /// existing path references keep working. `AppContainer` profile names /// are derived from `id` (stable), not `label`, for the same reason. + /// + /// # Errors + /// + /// Returns an error if the id is unknown or index persistence fails. + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn rename(&self, id: &str, new_label: impl Into) -> io::Result { let new_label = new_label.into(); log::debug!( @@ -241,6 +280,14 @@ impl JailRegistry { } /// Update the free-form notes field. + /// + /// # Errors + /// + /// Returns an error if the id is unknown or index persistence fails. + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn set_notes(&self, id: &str, notes: Option) -> io::Result { log::debug!( "[cwd_jail] registry.set_notes id={id} has_notes={}", @@ -274,6 +321,15 @@ impl JailRegistry { /// so a filesystem error doesn't leave the registry in a state /// where the entry is gone in-memory but the directory survives on /// disk until the next `open()` reload. + /// + /// # Errors + /// + /// Returns an error if the id is unknown, the path is unsafe, or removal + /// and persistence fail. + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn delete(&self, id: &str) -> io::Result<()> { let mut idx = self.index.lock().unwrap(); let record = idx @@ -332,6 +388,14 @@ impl JailRegistry { /// Drop *all* jails. Convenience for tests / "reset everything" /// flows. Returns the number of jails removed. + /// + /// # Errors + /// + /// Returns an error if deleting any jail fails. + /// + /// # Panics + /// + /// Panics if the registry mutex was poisoned by a previous panic. pub fn clear(&self) -> io::Result { let ids: Vec = self.index.lock().unwrap().records.keys().cloned().collect(); let n = ids.len(); @@ -345,6 +409,11 @@ impl JailRegistry { /// Spawn `cmd` inside the named jail, using the default backend. /// Convenience wrapper — the same effect as /// `spawn(&Jail::new(record.dir, record.label), cmd)`. + /// + /// # Errors + /// + /// Returns an error if the id is unknown, the jail path is unsafe, or + /// spawning is unsupported or fails. pub fn spawn_in(&self, id: &str, cmd: Command) -> io::Result { let jail = self.jail_for(id)?; log::debug!("[cwd_jail] registry.spawn_in id={id}"); @@ -352,6 +421,11 @@ impl JailRegistry { } /// Same as [`Self::spawn_in`] but with a caller-supplied backend. + /// + /// # Errors + /// + /// Returns an error if the id is unknown, the jail path is unsafe, or + /// the backend cannot spawn the command. pub fn spawn_in_with( &self, id: &str, @@ -439,8 +513,7 @@ fn now_unix(clock: &dyn Clock) -> u64 { clock .now() .duration_since(UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0) + .map_or(0, |d| d.as_secs()) } /// Short, URL-safe id. Not cryptographically random — we use it as a diff --git a/crates/tinybox-jail/src/registry_tests.rs b/crates/tinybox-jail/src/registry_tests.rs index bdef5d40..d5ce94ce 100644 --- a/crates/tinybox-jail/src/registry_tests.rs +++ b/crates/tinybox-jail/src/registry_tests.rs @@ -54,6 +54,19 @@ fn delete_removes_dir_and_record() { assert!(reg.get(&a.id).is_none()); } +#[test] +fn delete_persists_removal_after_jail_directory_is_already_missing() { + let base = tempdir("delete-missing-directory"); + let reg = JailRegistry::open(&base).unwrap(); + let record = reg.create("missing directory").unwrap(); + fs::remove_dir_all(&record.dir).unwrap(); + + reg.delete(&record.id).unwrap(); + + assert!(reg.get(&record.id).is_none()); + assert!(JailRegistry::open(&base).unwrap().list().is_empty()); +} + #[test] fn delete_missing_errors() { let base = tempdir("missing"); @@ -164,7 +177,6 @@ fn spawn_in_with_missing_id_errors() { } #[test] -#[cfg(not(feature = "landlock"))] fn spawn_in_uses_default_backend() { let base = tempdir("spawn-default"); let reg = JailRegistry::open(&base).unwrap(); @@ -178,8 +190,12 @@ fn spawn_in_uses_default_backend() { }; let result = reg.spawn_in(&a.id, cmd); if super::super::default_backend().is_available() { - let mut child = result.unwrap(); - let _ = child.wait().unwrap(); + match result { + Ok(mut child) => { + let _ = child.wait(); + } + Err(error) => assert_eq!(error.kind(), io::ErrorKind::PermissionDenied), + } } else { assert_eq!( result.err().map(|error| error.kind()), @@ -214,6 +230,18 @@ fn open_creates_base_directory_if_missing() { assert!(reg.list().is_empty()); } +#[test] +fn open_creates_a_missing_nested_base_directory() { + let parent = tempdir("missing-parent"); + let base = parent.path().join("nested").join("registry"); + assert!(!base.exists()); + + let registry = JailRegistry::open(&base).unwrap(); + + assert!(base.is_dir()); + assert!(registry.list().is_empty()); +} + #[test] fn corrupt_index_returns_invalid_data() { let base = tempdir("corrupt"); @@ -307,6 +335,22 @@ fn create_consecutive_ids_are_unique_in_same_second() { assert_eq!(ids.len(), 32); } +#[test] +fn create_skips_an_existing_unindexed_jail_directory() { + let base = tempdir("directory-collision"); + let current = generate_id(0); + let counter = u64::from_str_radix(¤t[2..], 16).unwrap(); + let collision = base.path().join(format!("j0{:x}", counter + 1)); + fs::create_dir(&collision).unwrap(); + + let reg = JailRegistry::open_with_clock(base.path(), Arc::new(FixedClock::at_epoch())).unwrap(); + let created = reg.create("safe").unwrap(); + + assert_ne!(created.dir, collision); + assert!(collision.is_dir()); + assert!(created.dir.is_dir()); +} + #[test] fn registry_uses_the_injected_clock_for_timestamps() { let base = tempdir("clock"); @@ -322,27 +366,67 @@ fn registry_uses_the_injected_clock_for_timestamps() { } #[test] -fn failed_index_write_rolls_back_registry_mutations() { - let base = tempdir("rollback"); +fn create_rolls_back_when_index_persistence_fails() { + let base = tempdir("create-persist-failure"); + fs::create_dir(base.path().join("index.json.tmp")).unwrap(); let reg = JailRegistry::open(base.path()).unwrap(); - let blocked_tmp = base.path().join("index.json.tmp"); - fs::create_dir(&blocked_tmp).unwrap(); - let create_error = reg.create("not-persisted").err(); - assert!(create_error.is_some()); + assert!(reg.create("not-persisted").is_err()); assert!(reg.list().is_empty()); + assert_eq!(fs::read_dir(base.path()).unwrap().count(), 1); +} - fs::remove_dir(&blocked_tmp).unwrap(); - let record = reg.create("original").unwrap(); +#[test] +fn rename_and_notes_roll_back_when_index_persistence_fails() { + let base = tempdir("update-persist-failure"); + let reg = JailRegistry::open(base.path()).unwrap(); + let created = reg.create("original").unwrap(); + fs::create_dir(base.path().join("index.json.tmp")).unwrap(); + + assert!(reg.rename(&created.id, "changed").is_err()); + assert!(reg.set_notes(&created.id, Some("changed".into())).is_err()); + let current = reg.get(&created.id).unwrap(); + assert_eq!(current.label, "original"); + assert!(current.notes.is_none()); +} + +#[test] +fn delete_keeps_memory_aligned_when_index_persistence_fails() { + let base = tempdir("delete-persist-failure"); + let reg = JailRegistry::open(base.path()).unwrap(); + let created = reg.create("remove").unwrap(); + fs::create_dir(base.path().join("index.json.tmp")).unwrap(); + + assert!(reg.delete(&created.id).is_err()); + assert!(reg.get(&created.id).is_none()); + assert!(!created.dir.exists()); +} + +#[test] +fn delete_persists_when_the_jail_directory_is_already_missing() { + let base = tempdir("delete-missing-directory"); + let reg = JailRegistry::open(base.path()).unwrap(); + let created = reg.create("removed-outside").unwrap(); + fs::remove_dir_all(&created.dir).unwrap(); + + reg.delete(&created.id).unwrap(); + + assert!(reg.get(&created.id).is_none()); +} + +#[test] +fn failed_index_write_after_delete_keeps_in_memory_removal() { + let base = tempdir("delete-rollback"); + let reg = JailRegistry::open(base.path()).unwrap(); + let record = reg.create("deleted").unwrap(); + let blocked_tmp = base.path().join("index.json.tmp"); fs::create_dir(&blocked_tmp).unwrap(); - assert!(reg.rename(&record.id, "changed").is_err()); - assert_eq!( - reg.get(&record.id).map(|item| item.label), - Some("original".into()) - ); - assert!(reg.set_notes(&record.id, Some("changed".into())).is_err()); - assert_eq!(reg.get(&record.id).and_then(|item| item.notes), None); + let error = reg.delete(&record.id).unwrap_err(); + + assert_eq!(error.kind(), io::ErrorKind::IsADirectory); + assert!(!record.dir.exists()); + assert!(reg.get(&record.id).is_none()); } #[test] @@ -350,8 +434,7 @@ fn open_rejects_a_file_as_registry_directory() { let base = tempdir("not-directory"); let file = base.path().join("file"); fs::write(&file, b"not a directory").unwrap(); - let error = JailRegistry::open(file).err(); - assert!(error.is_some()); + assert!(JailRegistry::open(file).is_err()); } #[test] diff --git a/crates/tinybox-module/Cargo.toml b/crates/tinybox-module/Cargo.toml index 44d694a9..8bb68c15 100644 --- a/crates/tinybox-module/Cargo.toml +++ b/crates/tinybox-module/Cargo.toml @@ -35,5 +35,10 @@ tokio.workspace = true # The GitHub release verifier passes an explicit empty module configuration. serde_json.workspace = true +[features] +default = [] +linked = ["static-link"] +static-link = [] + [lints] workspace = true diff --git a/crates/tinybox-module/src/lib.rs b/crates/tinybox-module/src/lib.rs index 9af09e1a..c2dcb1ac 100644 --- a/crates/tinybox-module/src/lib.rs +++ b/crates/tinybox-module/src/lib.rs @@ -31,3 +31,7 @@ //! of the public rustdoc surface. mod tinybus_module; + +/// Constructs this module for registration with an in-process `TinyBus` host. +#[cfg(feature = "static-link")] +pub use tinybus_module::linked_module; diff --git a/crates/tinybox-module/src/tinybus_module/mod.rs b/crates/tinybox-module/src/tinybus_module/mod.rs index e6f7d471..c808460d 100644 --- a/crates/tinybox-module/src/tinybus_module/mod.rs +++ b/crates/tinybox-module/src/tinybus_module/mod.rs @@ -106,7 +106,7 @@ async fn setup(connection: Connection) -> TinyBusResult<()> { Ok(()) } -tinybus_module::module_export! { +tinybus_module::module_export_optional_static! { setup = setup, worker_threads = 1, provides = ["ai.tinyhumans.tinybox.Box"],