diff --git a/crates/tinybox-bus/src/jail.rs b/crates/tinybox-bus/src/jail.rs index f75e27cb..6a5fe287 100644 --- a/crates/tinybox-bus/src/jail.rs +++ b/crates/tinybox-bus/src/jail.rs @@ -27,6 +27,12 @@ pub struct NativeJailConfig { pub enforcement: JailEnforcement, /// Require the provider to meet the process-isolation security floor. pub require_process_isolation: bool, + /// Absolute paths the child may neither read nor write, even where the + /// root or a grant would otherwise reach them (credential stores, for a + /// provider that reads allow-by-default). Paths that do not exist are + /// ignored. Added in contract 1.20; empty by default. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub deny_read: Vec, } #[cfg(test)] diff --git a/crates/tinybox-bus/src/jail_tests.rs b/crates/tinybox-bus/src/jail_tests.rs index f1c52617..b080dc87 100644 --- a/crates/tinybox-bus/src/jail_tests.rs +++ b/crates/tinybox-bus/src/jail_tests.rs @@ -17,3 +17,22 @@ fn native_jail_request_keeps_explicit_enforcement_and_grants() -> Result<(), ser ); Ok(()) } + +#[test] +fn deny_read_defaults_empty_and_round_trips() -> Result<(), serde_json::Error> { + let mut value = serde_json::json!({ + "read_only": [], "read_write": [], "allow_net": true, + "allow_subprocess": true, "enforcement": "declared", + "require_process_isolation": false, + }); + // A 1.19 request still decodes, and an empty list is not written back. + let config: NativeJailConfig = serde_json::from_value(value.clone())?; + assert_eq!(config.deny_read, Vec::::new()); + assert_eq!(serde_json::to_value(&config)?, value); + + value["deny_read"] = serde_json::json!(["/Users/a/.ssh"]); + let config: NativeJailConfig = serde_json::from_value(value.clone())?; + assert_eq!(config.deny_read, ["/Users/a/.ssh"]); + assert_eq!(serde_json::to_value(config)?, value); + Ok(()) +} diff --git a/crates/tinybox-bus/src/lib.rs b/crates/tinybox-bus/src/lib.rs index abc17922..e655c1f6 100644 --- a/crates/tinybox-bus/src/lib.rs +++ b/crates/tinybox-bus/src/lib.rs @@ -66,7 +66,8 @@ pub use files::{ /// suppression for a native shell child; ignored elsewhere). /// 1.19 adds `NetworkPolicy::Allowlist` and the observational /// [`SandboxEvent`] vocabulary (`Started`, `Denied`, `LimitHit`, `Exited`). -pub const CONTRACT_VERSION: (u32, u32) = (1, 19); +/// 1.20 adds `deny_read` to [`NativeJailConfig`]. +pub const CONTRACT_VERSION: (u32, u32) = (1, 20); /// Whether this host vocabulary can bind to a module's advertised version. /// The major must match and the module's minor must include every host member. diff --git a/crates/tinybox-bus/src/lib_tests.rs b/crates/tinybox-bus/src/lib_tests.rs index a521639f..a546272f 100644 --- a/crates/tinybox-bus/src/lib_tests.rs +++ b/crates/tinybox-bus/src/lib_tests.rs @@ -51,7 +51,7 @@ fn docker_run_vocabulary_is_typed_bounded_and_additive() -> Result<(), serde_jso assert_eq!(METHODS[29], "CleanupDocker"); assert_eq!(METHODS[30], "RunDockerWithoutWorkspace"); assert_eq!(METHODS[34], "StartNativeShellExec"); - assert_eq!(CONTRACT_VERSION, (1, 19)); + assert_eq!(CONTRACT_VERSION, (1, 20)); assert_eq!(MAX_DOCKER_RUN_TIMEOUT_MS, 86_400_000); let serialized = serde_json::to_value(&request)?; assert_eq!(serialized["read_only_rootfs"], false); @@ -129,7 +129,7 @@ fn host_gateway_and_docker_facts_are_additive_and_default_safely() -> Result<(), #[test] fn version_and_capability_wire_snapshot_preserve_contract_compatibility() -> Result<(), serde_json::Error> { - assert_eq!(CONTRACT_VERSION, (1, 19)); + assert_eq!(CONTRACT_VERSION, (1, 20)); assert!(is_compatible(CONTRACT_VERSION)); assert!(!is_compatible((1, 8))); assert!(!is_compatible((1, 7))); @@ -147,7 +147,7 @@ fn version_and_capability_wire_snapshot_preserve_contract_compatibility() exec_backends: Vec::new(), spawn_backends: Vec::new(), }; - let snapshot = serde_json::json!({"contract_version":[1,19],"create_backends":["passthrough"],"exec_backends":[],"spawn_backends":[]}); + let snapshot = serde_json::json!({"contract_version":[1,20],"create_backends":["passthrough"],"exec_backends":[],"spawn_backends":[]}); assert_eq!(serde_json::to_value(&capabilities)?, snapshot); assert_eq!( serde_json::from_value::(snapshot)?, @@ -166,7 +166,7 @@ fn no_workspace_docker_operation_is_present_in_the_contract() { .count(), 1 ); - assert_eq!(CONTRACT_VERSION, (1, 19)); + assert_eq!(CONTRACT_VERSION, (1, 20)); } #[test] diff --git a/crates/tinybox-jail/src/jail.rs b/crates/tinybox-jail/src/jail.rs index c34e31b9..968e97ea 100644 --- a/crates/tinybox-jail/src/jail.rs +++ b/crates/tinybox-jail/src/jail.rs @@ -27,6 +27,11 @@ pub struct Jail { /// 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, + /// Paths the child may neither read nor write, even inside the root or a + /// grant. Only a backend that [`carves_denials`](JailBackend::carves_denials) + /// can subtract them from a grant; everywhere else they are unreachable + /// already because nothing grants them. + pub deny_read: Vec, /// Allow outbound network. Most agent tools need this; some risky tools /// (untrusted code execution) should disable it. pub allow_net: bool, @@ -46,6 +51,7 @@ impl Jail { root: root.as_ref().to_path_buf(), read_only: Vec::new(), read_write: Vec::new(), + deny_read: Vec::new(), allow_net: true, allow_subprocess: true, label: label.into(), @@ -107,7 +113,12 @@ 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 self.read_only.iter_mut().chain(self.read_write.iter_mut()) { + for p in self + .read_only + .iter_mut() + .chain(self.read_write.iter_mut()) + .chain(self.deny_read.iter_mut()) + { if let Ok(c) = p.canonicalize() { *p = c; } @@ -135,6 +146,13 @@ impl Jail { /// `AppContainer` requires custom `CreateProcess` flags that `std`'s /// `Command::spawn` does not expose. pub trait JailBackend: Send + Sync { + /// Whether [`Jail::deny_read`] is enforced even inside the root or a grant. + /// Allow-list backends (Landlock, `AppContainer`) cannot subtract from a + /// grant, so they keep the default `false`. + fn carves_denials(&self) -> bool { + false + } + /// Stable identifier, used in logs / audit ("landlock", "seatbelt", /// "appcontainer", "noop"). fn name(&self) -> &'static str; diff --git a/crates/tinybox-jail/src/macos.rs b/crates/tinybox-jail/src/macos.rs index 4686d111..564fee0e 100644 --- a/crates/tinybox-jail/src/macos.rs +++ b/crates/tinybox-jail/src/macos.rs @@ -50,6 +50,11 @@ impl JailBackend for SeatbeltBackend { declared_constraint_support(self.is_available()) } + /// The profile's last rule denies `deny_read` paths over every allow. + fn carves_denials(&self) -> bool { + true + } + /// Launcher presence proves no process-isolation guarantee. fn isolation(&self) -> tinybox_core::IsolationLevel { tinybox_core::IsolationLevel::None @@ -186,6 +191,17 @@ fn render_profile(jail: &Jail) -> String { } out.push_str(" (subpath \"/private/tmp\")\n (literal \"/dev/null\")\n)\n"); + // Reads are allow-default, so a credential store is readable unless it is + // denied here. Last, so it also overrides the write grants above; a + // `subpath` matches a single file as well as a directory tree. + if !jail.deny_read.is_empty() { + out.push_str("(deny file-read* file-write*\n"); + for path in &jail.deny_read { + let _ = writeln!(out, " (subpath \"{}\")", escape(&path.to_string_lossy())); + } + out.push_str(")\n"); + } + // `read_only` is informational on macOS — reads are already allowed // by `(allow default)`. We keep the field on `Jail` because Landlock // and AppContainer use it, and it lets callers express intent diff --git a/crates/tinybox-jail/src/macos_tests.rs b/crates/tinybox-jail/src/macos_tests.rs index 6940ba1b..21bd9c15 100644 --- a/crates/tinybox-jail/src/macos_tests.rs +++ b/crates/tinybox-jail/src/macos_tests.rs @@ -28,6 +28,22 @@ fn profile_allow_subprocess_default_has_no_process_deny() { assert!(!p.contains("(deny process-exec)")); } +#[test] +fn profile_denies_reads_and_writes_of_denied_paths_last() { + let mut jail = Jail::new("/Users/a/project", "x"); + assert!(!render_profile(&jail).contains("file-read*")); + jail.deny_read = vec!["/Users/a/.ssh".into(), "/Users/a/.netrc".into()]; + let p = render_profile(&jail); + let denial = p + .find("(deny file-read* file-write*") + .expect("denied paths are emitted"); + // After the write allows, so it overrides them. + assert!(denial > p.find("(allow file-write*").unwrap()); + assert!(p[denial..].contains("(subpath \"/Users/a/.ssh\")")); + assert!(p[denial..].contains("(subpath \"/Users/a/.netrc\")")); + assert!(SeatbeltBackend::new().carves_denials()); +} + #[test] fn escape_handles_backslash_and_quote() { assert_eq!(escape("a\\b"), "a\\\\b"); diff --git a/crates/tinybox-module/src/tinybus_module/README.md b/crates/tinybox-module/src/tinybus_module/README.md index 517a41d1..7659d026 100644 --- a/crates/tinybox-module/src/tinybus_module/README.md +++ b/crates/tinybox-module/src/tinybus_module/README.md @@ -50,7 +50,7 @@ The interface is `ai.tinyhumans.tinybox.Box` at | StartNativeShellExec | ShellExecRequest | ProcessRef | | ValidateDockerPolicy | DockerPolicyRequest | DockerPolicyReport | -Capabilities advertises contract version 1.19; 1.0 denotes the original +Capabilities advertises contract version 1.20; 1.0 denotes the original discovery-only surface. Hosts require equal majors and a module minor at least as new as their vocabulary, using `tinybox_bus::is_compatible`. Version 1.1 added reserved resource/process ownership and terminal shutdown; 1.2 adds host @@ -245,7 +245,12 @@ enforcement preflight before consuming process admission. `Enforced` refuses partial constraints; `Declared` requires an explicit host decision using `JailStatus`. Neither mode admits the no-op or unavailable backend. A caller requiring the process-isolation security floor sets `require_process_isolation`. -The host retains credential-path exclusions and approval policy. +The host retains approval policy and chooses the credential paths; it sends them +as `deny_read` (1.20), which the module canonicalizes (dropping paths that do not +exist) and refuses when a denial covers the root or a writable grant, or lies +inside the root or a grant on a backend that cannot subtract from grants. +Seatbelt denies them last in its profile; Landlock and `AppContainer` never grant +them. The jail owns a separate native collector for each execution. Output replay, Cancel, ReleaseOutput, Close and Shutdown use the same retained cleanup barriers @@ -379,4 +384,5 @@ operation. State directory authorization and Windows private parent ACLs belong to the caller. See [the native shell contract](../../../docs/specs/native-shell-execution.md). Version 1.19 adds `NetworkPolicy::Allowlist` and the `SandboxEvent` vocabulary -without a new member; see `docs/specs/contract-1-19.md`. +without a new member; see `docs/specs/contract-1-19.md`. Version 1.20 adds +`NativeJailConfig::deny_read`; see `docs/specs/contract-1-20.md`. diff --git a/crates/tinybox-module/src/tinybus_module/mod_tests.rs b/crates/tinybox-module/src/tinybus_module/mod_tests.rs index f6ca1cc1..e95a770b 100644 --- a/crates/tinybox-module/src/tinybus_module/mod_tests.rs +++ b/crates/tinybox-module/src/tinybus_module/mod_tests.rs @@ -246,6 +246,7 @@ async fn new_module_operations_dispatch_and_preserve_validation_errors() -> tiny allow_subprocess: false, enforcement: tinybox_bus::JailEnforcement::Declared, require_process_isolation: false, + deny_read: Vec::new(), }, ) .await; diff --git a/crates/tinybox-module/src/tinybus_module/native_jail.rs b/crates/tinybox-module/src/tinybus_module/native_jail.rs index 86a33d9c..be000af1 100644 --- a/crates/tinybox-module/src/tinybus_module/native_jail.rs +++ b/crates/tinybox-module/src/tinybus_module/native_jail.rs @@ -19,7 +19,9 @@ pub(super) fn prepare( if config.require_process_isolation && !backend.is_suitable_for_untrusted_code() { return Err(unavailable()); } - if config.read_only.len() + config.read_write.len() > tinybox_bus::MAX_NATIVE_JAIL_GRANTS { + if config.read_only.len() + config.read_write.len() + config.deny_read.len() + > tinybox_bus::MAX_NATIVE_JAIL_GRANTS + { return Err(invalid()); } let root = root.canonicalize().map_err(|_| invalid())?; @@ -39,6 +41,28 @@ pub(super) fn prepare( jail.allow_subprocess = config.allow_subprocess; jail.read_only = grants(&config.read_only)?; jail.read_write = grants(&config.read_write)?; + jail.deny_read = denials(&config.deny_read)?; + // A denial over the root or a writable grant would leave the command no + // workspace; that is a host error, not a weaker jail. + if jail.deny_read.iter().any(|denied| { + std::iter::once(&jail.root) + .chain(&jail.read_write) + .any(|granted| granted.starts_with(denied)) + }) { + return Err(invalid()); + } + // Inside the root or a grant, only a backend that subtracts from grants + // can keep the denial; outside them nothing grants the path anyway. + if !backend.carves_denials() + && jail.deny_read.iter().any(|denied| { + std::iter::once(&jail.root) + .chain(&jail.read_only) + .chain(&jail.read_write) + .any(|granted| denied.starts_with(granted)) + }) + { + return Err(unavailable()); + } match config.enforcement { JailEnforcement::Enforced => backend.require(&jail).map_err(|_| unavailable())?, JailEnforcement::Declared => { @@ -72,6 +96,30 @@ fn grants(paths: &[String]) -> Result> { }) .collect() } +/// Denied paths, validated like grants; a path that does not exist cannot be +/// read and is dropped, an existing one is canonicalized so the backend matches +/// the real location behind a symlink. +fn denials(paths: &[String]) -> Result> { + let mut denied = Vec::new(); + for value in paths { + let path = Path::new(value); + if value.len() > tinybox_bus::MAX_FILE_PATH_BYTES + || value.contains('\0') + || !path.is_absolute() + || path + .components() + .any(|part| matches!(part, Component::ParentDir)) + { + return Err(invalid()); + } + match path.canonicalize() { + Ok(path) => denied.push(path), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return Err(invalid()), + } + } + Ok(denied) +} fn invalid() -> Error { super::resources::failure( tinybox_bus::INVALID_FILE_PATH, diff --git a/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs b/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs index 577d1c4a..d6e83b7d 100644 --- a/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs +++ b/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs @@ -20,6 +20,9 @@ impl JailBackend for Backend { fn is_suitable_for_untrusted_code(&self) -> bool { self.suitable } + fn carves_denials(&self) -> bool { + self.name == "seatbelt" + } fn constraint_support(&self) -> tinybox_core::ConstraintSupport { use tinybox_core::{Constraint, ConstraintSupport, Enforcement}; if self.full { @@ -43,6 +46,7 @@ fn config() -> NativeJailConfig { allow_subprocess: false, enforcement: JailEnforcement::Enforced, require_process_isolation: false, + deny_read: Vec::new(), } } #[test] @@ -198,3 +202,65 @@ fn declared_enforcement_rejects_each_unsupported_requested_constraint() ); Ok(()) } + +// `prepare` refuses every native jail off Unix before reading any path. +#[cfg(unix)] +#[test] +fn denials_are_canonicalized_and_refused_where_a_backend_cannot_keep_them() +-> std::result::Result<(), Box> { + let home = tempfile::tempdir()?; + let root = home.path().join("project"); + let secrets = home.path().join(".ssh"); + let token = home.path().join(".netrc"); + std::fs::create_dir(&root)?; + std::fs::create_dir(&secrets)?; + std::fs::write(&token, b"secret")?; + let mut command = tinybox_core::ExecRequest::new(["irrelevant"]); + command.cwd = Some(root.clone()); + let backend = |name| Backend { + available: true, + suitable: false, + name, + full: true, + }; + let path = |p: &std::path::Path| p.to_string_lossy().into_owned(); + let mut config = config(); + config.deny_read = vec![ + path(&secrets), + path(&token), + path(&home.path().join(".aws")), + ]; + + // Outside every grant: any backend keeps it; the absent path is dropped. + for name in ["fixture", "seatbelt"] { + let jail = prepare(&root, &config, &command, &backend(name))?; + assert_eq!( + jail.deny_read, + vec![secrets.canonicalize()?, token.canonicalize()?] + ); + } + + // Inside a read grant: only a backend that subtracts from grants keeps it. + config.read_only = vec![path(home.path())]; + assert!(prepare(&root, &config, &command, &backend("fixture")).is_err()); + let jail = prepare(&root, &config, &command, &backend("seatbelt"))?; + assert_eq!(jail.deny_read.len(), 2); + config.read_only.clear(); + + // A denial over the root or a writable grant is refused everywhere. + for denied in [path(home.path()), path(&root)] { + config.deny_read = vec![denied]; + assert!(prepare(&root, &config, &command, &backend("seatbelt")).is_err()); + } + config.deny_read = vec![path(&secrets)]; + config.read_write = vec![path(&secrets)]; + assert!(prepare(&root, &config, &command, &backend("seatbelt")).is_err()); + config.read_write.clear(); + + // Malformed denials are refused like malformed grants. + for bad in ["relative", "/usr/../etc", "/bad\0path"] { + config.deny_read = vec![bad.into()]; + assert!(prepare(&root, &config, &command, &backend("seatbelt")).is_err()); + } + Ok(()) +} diff --git a/crates/tinybox-module/src/tinybus_module/native_shell/mod_tests.rs b/crates/tinybox-module/src/tinybus_module/native_shell/mod_tests.rs index 038a3370..b8f6823e 100644 --- a/crates/tinybox-module/src/tinybus_module/native_shell/mod_tests.rs +++ b/crates/tinybox-module/src/tinybus_module/native_shell/mod_tests.rs @@ -129,6 +129,7 @@ async fn native_shell_jail_adds_owned_scratch_and_refuses_unsupported_constraint allow_subprocess: false, enforcement: tinybox_bus::JailEnforcement::Enforced, require_process_isolation: false, + deny_read: Vec::new(), }); let shell = ShellContext::prepare_with_backend( request.clone(), diff --git a/crates/tinybox-module/src/tinybus_module/resources_stream_tests.rs b/crates/tinybox-module/src/tinybus_module/resources_stream_tests.rs index 3b11b765..c8e53270 100644 --- a/crates/tinybox-module/src/tinybus_module/resources_stream_tests.rs +++ b/crates/tinybox-module/src/tinybus_module/resources_stream_tests.rs @@ -287,6 +287,7 @@ async fn jailed_start_rejects_missing_grants_without_consuming_reservation() -> allow_subprocess: true, enforcement: tinybox_bus::JailEnforcement::Declared, require_process_isolation: false, + deny_read: Vec::new(), }; assert!( resources diff --git a/crates/tinybox-module/tests/module_e2e.rs b/crates/tinybox-module/tests/module_e2e.rs index ef5d493e..8fb6eaf3 100644 --- a/crates/tinybox-module/tests/module_e2e.rs +++ b/crates/tinybox-module/tests/module_e2e.rs @@ -670,6 +670,7 @@ async fn verify_native_jail( allow_subprocess: true, enforcement: JailEnforcement::Enforced, require_process_isolation: false, + deny_read: Vec::new(), }; let status: tinybox_bus::JailStatus = proxy.call("JailStatus", ()).await?; eprintln!( @@ -806,6 +807,7 @@ async fn verify_docker_rejects_local_jail( allow_subprocess: true, enforcement: JailEnforcement::Declared, require_process_isolation: false, + deny_read: Vec::new(), }, ), ) diff --git a/docs/specs/contract-1-20.md b/docs/specs/contract-1-20.md new file mode 100644 index 00000000..8ac16637 --- /dev/null +++ b/docs/specs/contract-1-20.md @@ -0,0 +1,32 @@ +# Contract 1.20: native jail read denials + +Part of tinyhumansai/openhuman#7819. + +Contract 1.20 is a vocabulary bump. It appends no bus member, so `METHODS` is +unchanged. A host that sends a non-empty `deny_read` requires a module minor of +at least 20, which `tinybox_bus::is_compatible` already enforces. + +## Added + +- `NativeJailConfig::deny_read: Vec`: absolute paths the child may + neither read nor write, even inside the root or a grant. Serde default empty + and omitted when empty, so 1.19 requests decode unchanged. +- `JailBackend::carves_denials()`, default `false`: whether a backend can + subtract a denial from a grant. + +## Semantics + +- Validated like grants (absolute, no `..`, no NUL, bounded length, counted + against `MAX_NATIVE_JAIL_GRANTS`). A path that does not exist is dropped; an + existing one is canonicalized so a symlinked store is denied at its target. +- Refused (`INVALID_FILE_PATH`) when a denial is the root, an ancestor of it, or + covers a writable grant: the command would have no workspace. +- Refused as unavailable when a denial lies inside the root or a grant and the + backend cannot carve it out. Outside every grant, allow-list backends + (Landlock, `AppContainer`) already deny the path. +- Seatbelt reads are allow-default, so the profile ends with + `(deny file-read* file-write* (subpath ...))`, after the write allows it + overrides. `subpath` matches a single file as well as a tree. + +The host chooses the paths (its credential-store floor); the module only +enforces them.