From a7ca4f5ad80bd7d09c4a5cb5f02c60fccaa25b7c Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Wed, 11 Nov 2026 15:39:14 +0530 Subject: [PATCH 1/2] Run Windows jails under AppContainer through the owned-process backend On Windows every jail request was refused. AppContainer can run work only through the owned-process API (it cannot return a std::process::Child), so default_backend() is "unsupported" there by design. But the module used default_backend() for every jailed path, and native_jail::prepare refused any non-Unix host outright. - The native shell, StartJailedExec and stdio paths take default_owned_backend(). Off Windows it picks the same backend as before (only AppContainer overrides is_owned_available), so nothing changes there. - prepare admits a backend that is owned-available instead of requiring Unix plus is_available. - StdioSession spawns through spawn_owned_captured_with and holds an OwnedChild. Its pipes are converted back into std child pipes, so the async plumbing is unchanged. ChildStatus now carries the signal, so a signalled stdio child still reports StdioExit::Signal on Unix. - JailStatus reports the owned backend and its owned availability. What AppContainer cannot enforce stays refused, now with the reason in the message (same error codes): a jail with allow_subprocess false ("cannot enforce subprocess"), require_process_isolation (it reports no isolation), and read denials overlapping the root, a grant or a writable grant (it cannot carve denials). Closes #22 --- crates/tinybox-jail/src/detect_tests.rs | 19 +++ crates/tinybox-jail/src/jail.rs | 28 +++- crates/tinybox-jail/src/jail_tests.rs | 41 ++++++ crates/tinybox-jail/src/lib.rs | 4 + .../src/tinybus_module/README.md | 9 +- .../tinybox-module/src/tinybus_module/mod.rs | 7 +- .../src/tinybus_module/mod_tests.rs | 56 ++++++++ .../src/tinybus_module/native_jail.rs | 102 +++++++++++--- .../src/tinybus_module/native_jail_tests.rs | 129 ++++++++++++++++++ .../src/tinybus_module/native_shell/mod.rs | 7 +- .../src/tinybus_module/resources_stdio.rs | 2 +- .../src/tinybus_module/resources_stream.rs | 2 +- .../src/tinybus_module/stdio.rs | 69 ++++++---- 13 files changed, 416 insertions(+), 59 deletions(-) diff --git a/crates/tinybox-jail/src/detect_tests.rs b/crates/tinybox-jail/src/detect_tests.rs index b1988d95..8c519a36 100644 --- a/crates/tinybox-jail/src/detect_tests.rs +++ b/crates/tinybox-jail/src/detect_tests.rs @@ -148,3 +148,22 @@ fn owned_selection_fails_closed_when_no_candidate_can_return_owned_child() { assert_eq!(picked.name(), UNSUPPORTED_BACKEND_NAME); assert!(!picked.is_owned_available()); } + +/// Every jailed path asks for the owned-process backend. Off Windows that must +/// be the same backend the plain selection picks, so the switch to it (#22) +/// changes nothing there. +#[cfg(not(windows))] +#[test] +fn off_windows_the_owned_selection_is_the_plain_selection() { + assert_eq!(pick_owned_backend().name(), pick_backend().name()); +} + +/// On Windows the owned selection is `AppContainer`, the backend the plain +/// selection can never return. +#[cfg(windows)] +#[test] +fn on_windows_the_owned_selection_is_appcontainer() { + let backend = pick_owned_backend(); + assert_eq!(backend.name(), "appcontainer"); + assert!(backend.is_owned_available()); +} diff --git a/crates/tinybox-jail/src/jail.rs b/crates/tinybox-jail/src/jail.rs index 3b9aa4dc..3011b72a 100644 --- a/crates/tinybox-jail/src/jail.rs +++ b/crates/tinybox-jail/src/jail.rs @@ -543,6 +543,7 @@ pub trait JailBackend: Send + Sync { #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct ChildStatus { code: Option, + signal: Option, } impl ChildStatus { @@ -550,7 +551,15 @@ impl ChildStatus { /// is represented by `None` on platforms that expose it that way. #[must_use] pub const fn from_code(code: Option) -> Self { - Self { code } + Self { code, signal: None } + } + + /// The same status, recording the signal that ended the process on + /// platforms that report one. + #[must_use] + pub const fn with_signal(mut self, signal: Option) -> Self { + self.signal = signal; + self } /// Exit code when the process has one (signal termination has none). @@ -559,6 +568,12 @@ impl ChildStatus { self.code } + /// The signal that ended the process, when it was signalled (Unix). + #[must_use] + pub const fn signal(self) -> Option { + self.signal + } + /// Whether the process returned exit code zero. #[must_use] pub const fn success(self) -> bool { @@ -717,8 +732,15 @@ impl OwnedProcess for StdOwnedProcess { fn try_wait(&mut self) -> std::io::Result> { self.0.try_wait().map(|status| { - status.map(|status| ChildStatus { - code: status.code(), + status.map(|status| { + #[cfg(unix)] + let signal = { + use std::os::unix::process::ExitStatusExt; + status.signal() + }; + #[cfg(not(unix))] + let signal = None; + ChildStatus::from_code(status.code()).with_signal(signal) }) }) } diff --git a/crates/tinybox-jail/src/jail_tests.rs b/crates/tinybox-jail/src/jail_tests.rs index f46d08f4..6b139602 100644 --- a/crates/tinybox-jail/src/jail_tests.rs +++ b/crates/tinybox-jail/src/jail_tests.rs @@ -320,3 +320,44 @@ fn nested_denial_refuses_only_paths_deeper_than_a_direct_child() { assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput, "{nested}"); } } + +#[test] +fn a_status_from_an_exit_code_carries_no_signal() { + let status = ChildStatus::from_code(Some(3)); + assert_eq!(status.code(), Some(3)); + assert_eq!(status.signal(), None); + assert_eq!( + ChildStatus::from_code(None).with_signal(Some(9)).signal(), + Some(9) + ); +} + +#[cfg(unix)] +#[test] +fn an_owned_child_ended_by_a_signal_reports_the_signal() -> std::io::Result<()> { + use std::time::{Duration, Instant}; + + let root = tempfile::tempdir()?; + let jail = Jail::new(root.path(), "owned-signal"); + let mut command = std::process::Command::new("/bin/sh"); + command.args(["-c", "kill -9 $$"]); + + let mut child = crate::spawn_owned_captured_with(&crate::NoopBackend, &jail, command)?; + let deadline = Instant::now() + Duration::from_secs(5); + let status = loop { + if let Some(status) = child.try_wait()? { + break status; + } + if Instant::now() >= deadline { + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "child did not exit", + )); + } + std::thread::sleep(Duration::from_millis(10)); + }; + + assert_eq!(status.signal(), Some(9)); + assert_eq!(status.code(), None); + Ok(()) +} diff --git a/crates/tinybox-jail/src/lib.rs b/crates/tinybox-jail/src/lib.rs index eae82bed..00af81b2 100644 --- a/crates/tinybox-jail/src/lib.rs +++ b/crates/tinybox-jail/src/lib.rs @@ -95,6 +95,10 @@ pub fn default_backend() -> Arc { } /// Cached default backend for the process-owned child API. +/// +/// Every jailed path in the module uses this one. Off Windows it is the same +/// backend [`default_backend`] picks; on Windows it is `AppContainer`, which +/// `default_backend` can never return. pub fn default_owned_backend() -> Arc { DEFAULT_OWNED_BACKEND .get_or_init(detect::pick_owned_backend) diff --git a/crates/tinybox-module/src/tinybus_module/README.md b/crates/tinybox-module/src/tinybus_module/README.md index aa2def9e..ee48c768 100644 --- a/crates/tinybox-module/src/tinybus_module/README.md +++ b/crates/tinybox-module/src/tinybus_module/README.md @@ -283,8 +283,13 @@ grant remain default-denied on allow-list backends. The jail owns a separate native collector for each execution. Output replay, Cancel, ReleaseOutput, Close and Shutdown use the same retained cleanup barriers -as StartExec. Windows rejects this operation until its native jail has a -supervised child bridge; explicit ordinary local execution remains available. +as StartExec. Every jailed path (native shell, `StartJailedExec`, stdio) uses the +owned-process backend, which on Windows is `AppContainer`: it runs each child in +its own Job Object. `AppContainer` enforces filesystem, network, CPU, memory and +process-count limits, but not `Subprocess`, and reports no isolation level, so a +jail with `allow_subprocess: false`, `require_process_isolation`, or a read +denial overlapping its grants is refused there, with the reason in the error +message. ## Observed SSH execution diff --git a/crates/tinybox-module/src/tinybus_module/mod.rs b/crates/tinybox-module/src/tinybus_module/mod.rs index a467d0a8..6bd23e36 100644 --- a/crates/tinybox-module/src/tinybus_module/mod.rs +++ b/crates/tinybox-module/src/tinybus_module/mod.rs @@ -170,11 +170,14 @@ impl BoxService { /// network namespace) reports its weaker level here. Admission decisions /// use the per-jail form in `native_jail::prepare`. async fn jail_status(&self) -> TinyBusResult { - let backend = tinybox_jail::default_backend(); + // The owned-process backend is the one every jailed path uses. On + // Windows it is `AppContainer`, which can run work only through the + // owned API, so its plain `is_available` is always false. + let backend = tinybox_jail::default_owned_backend(); let support = backend.constraint_support(); std::future::ready(Ok(tinybox_bus::JailStatus { backend: backend.name().into(), - available: backend.is_available(), + available: backend.is_owned_available(), isolation: backend.isolation().to_string(), suitable_for_untrusted_code: backend.is_suitable_for_untrusted_code(), filesystem: support diff --git a/crates/tinybox-module/src/tinybus_module/mod_tests.rs b/crates/tinybox-module/src/tinybus_module/mod_tests.rs index d4cae5b0..74c0a682 100644 --- a/crates/tinybox-module/src/tinybus_module/mod_tests.rs +++ b/crates/tinybox-module/src/tinybus_module/mod_tests.rs @@ -1520,6 +1520,62 @@ async fn run_native_shell_returns_finished_output_after_module_cleanup() Ok(()) } +/// On Windows a jailed native shell runs under `AppContainer`, through the +/// owned-process backend (#22). Before, every Windows jail was refused. +#[cfg(windows)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn run_native_shell_runs_a_jailed_command_under_appcontainer_on_windows() +-> Result<(), Box> { + let state = tempfile::tempdir()?; + let workspace = tempfile::tempdir()?; + let bus = MemoryBus::new(); + Broker::new().spawn(bus.clone()); + let _storage_host = serve_test_storage(&bus).await?; + let service = Connection::connect(bus.connect().await?).await?; + setup(service.clone()).await?; + let client = Connection::connect(bus.connect().await?).await?; + let proxy = client.proxy(INTERFACE, OBJECT_PATH, INTERFACE)?; + let mut request = run_native_shell_request("echo jailed", state.path(), None); + request.create.workspace = + tinybox_bus::Workspace::Directory(workspace.path().to_string_lossy().into_owned()); + request.shell.jail = Some(tinybox_bus::NativeJailConfig { + read_only: Vec::new(), + read_write: Vec::new(), + allow_net: true, + allow_subprocess: true, + enforcement: tinybox_bus::JailEnforcement::Enforced, + require_process_isolation: false, + deny_read: Vec::new(), + resources: None, + }); + + let result: tinybox_bus::RunNativeShellResult = + proxy.call("RunNativeShell", (request,)).await?; + let tinybox_bus::RunNativeShellResult::Finished { + exit_code, + stdout, + stderr, + timed_out, + .. + } = result + else { + return Err(format!("a jailed shell should finish: {result:?}").into()); + }; + assert_eq!( + exit_code, + 0, + "stdout={:?} stderr={:?}", + String::from_utf8_lossy(&stdout), + String::from_utf8_lossy(&stderr) + ); + assert!(!timed_out); + assert_eq!( + String::from_utf8_lossy(&stdout).replace("\r\n", "\n"), + "jailed\n" + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn run_native_shell_cancels_a_shell_that_outlives_its_timeout() -> Result<(), Box> { diff --git a/crates/tinybox-module/src/tinybus_module/native_jail.rs b/crates/tinybox-module/src/tinybus_module/native_jail.rs index 2d7c0799..22638c7d 100644 --- a/crates/tinybox-module/src/tinybus_module/native_jail.rs +++ b/crates/tinybox-module/src/tinybus_module/native_jail.rs @@ -16,28 +16,18 @@ pub(super) fn prepare( command: &tinybox_core::ExecRequest, backend: &dyn JailBackend, ) -> Result { - if !cfg!(unix) || !backend.is_available() || backend.name() == tinybox_jail::NOOP_BACKEND_NAME { - return Err(unavailable()); + // Owned availability, not `is_available`: Windows `AppContainer` runs work + // only through the owned-process API every jailed path uses. Off Windows + // the two are the same. + if !backend.is_owned_available() || backend.name() == tinybox_jail::NOOP_BACKEND_NAME { + return Err(refused("no native jail backend is available on this host")); } if config.read_only.len() + config.read_write.len() + config.deny_read.len() > tinybox_bus::MAX_NATIVE_JAIL_GRANTS { return Err(invalid()); } - // Before anything is canonicalized: a link the jailed program planted - // (`/vendor -> `) is only visible in the raw spellings. - { - let writable: Vec<&Path> = std::iter::once(root) - .chain(config.read_write.iter().map(Path::new)) - .collect(); - let nested: Vec<&Path> = config - .read_only - .iter() - .chain(&config.read_write) - .map(Path::new) - .collect(); - tinybox_jail::reject_symlinked_grants(&writable, &nested).map_err(|_| invalid())?; - } + reject_planted_links(root, config)?; let root = root.canonicalize().map_err(|_| invalid())?; if !root.is_dir() { return Err(invalid()); @@ -67,7 +57,11 @@ pub(super) fn prepare( || (!backend.carves_denials() && denied.starts_with(granted)) }) }) { - return Err(invalid()); + return Err(invalid_because(&format!( + "a read denial covers the jail root or a writable grant, or sits inside a writable \ + grant that backend `{}` cannot carve a denial out of", + backend.name() + ))); } // A carving backend may still be unable to hold a deeply nested denial. backend @@ -87,19 +81,29 @@ pub(super) fn prepare( .chain(&implicit_grants) .any(|granted| overlaps(denied, granted)) }) { - return Err(unavailable()); + return Err(refused(&format!( + "a read denial overlaps the jail root, a read-only grant or a default grant, and \ + backend `{}` cannot carve denials", + backend.name() + ))); } } // Per-jail, never per-backend: a backend's isolation can depend on the // configuration (a shared network namespace under `allow_net`). if config.require_process_isolation && !backend.is_suitable_for_untrusted_jail(&jail) { - return Err(unavailable()); + return Err(refused(&format!( + "the jail requires process isolation, and backend `{}` reports {} isolation for it", + backend.name(), + backend.isolation_for(&jail) + ))); } backend .require_resources(&jail) .map_err(|_| unavailable())?; match config.enforcement { - JailEnforcement::Enforced => backend.require(&jail).map_err(|_| unavailable())?, + JailEnforcement::Enforced => backend + .require(&jail) + .map_err(|_| refused(&cannot_enforce(backend, &jail, true)))?, JailEnforcement::Declared => { if backend .plan_check(&jail) @@ -107,7 +111,7 @@ pub(super) fn prepare( .iter() .any(|(_, support)| *support == tinybox_core::Enforcement::Unsupported) { - return Err(unavailable()); + return Err(refused(&cannot_enforce(backend, &jail, false))); } } } @@ -150,6 +154,21 @@ fn required_resource_constraints( } } +/// Before anything is canonicalized: a link the jailed program planted +/// (`/vendor -> `) is only visible in the raw spellings. +fn reject_planted_links(root: &Path, config: &NativeJailConfig) -> Result<()> { + let writable: Vec<&Path> = std::iter::once(root) + .chain(config.read_write.iter().map(Path::new)) + .collect(); + let nested: Vec<&Path> = config + .read_only + .iter() + .chain(&config.read_write) + .map(Path::new) + .collect(); + tinybox_jail::reject_symlinked_grants(&writable, &nested).map_err(|_| invalid()) +} + /// Validate and canonicalize each existing absolute grant path. fn grants(paths: &[String]) -> Result> { paths @@ -256,6 +275,47 @@ fn unavailable() -> Error { "requested native jail enforcement is unavailable", ) } +/// [`invalid`], saying which rule the request broke. +fn invalid_because(reason: &str) -> Error { + super::resources::failure( + tinybox_bus::INVALID_FILE_PATH, + &format!("invalid native jail grant or working directory: {reason}"), + ) +} +/// [`unavailable`], saying what the backend cannot provide. The code is the +/// same, so a host that branches on it is unaffected. +fn refused(reason: &str) -> Error { + super::resources::failure( + tinybox_bus::UNSUPPORTED_OPERATION, + &format!("requested native jail enforcement is unavailable: {reason}"), + ) +} +/// The constraints `backend` cannot enforce for `jail`: anything short of +/// `Enforced` when `strict`, otherwise only what it does not support at all. +fn cannot_enforce(backend: &dyn JailBackend, jail: &Jail, strict: bool) -> String { + let missing: Vec = backend + .plan_check(jail) + .constraints + .iter() + .filter(|(_, support)| { + if strict { + *support != tinybox_core::Enforcement::Enforced + } else { + *support == tinybox_core::Enforcement::Unsupported + } + }) + .map(|(constraint, _)| format!("{constraint:?}").to_lowercase()) + .collect(); + format!( + "backend `{}` cannot enforce {}", + backend.name(), + if missing.is_empty() { + "a hard requirement of this jail".to_string() + } else { + missing.join(", ") + } + ) +} /// A private backend whose sole accepted command is the validated launcher /// constructed by `prepare_with_resources`. It is created per execution and 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 ebbab801..b3e9be9f 100644 --- a/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs +++ b/crates/tinybox-module/src/tinybus_module/native_jail_tests.rs @@ -1563,3 +1563,132 @@ fn a_symlink_planted_in_the_writable_root_cannot_redirect_a_grant() assert!(prepare(root.path(), &outside, &command, &backend).is_ok()); Ok(()) } + +/// Shaped like Windows `AppContainer`: usable only through the owned-process +/// API, enforcing filesystem, network and resource limits but not +/// `Subprocess`, reporting no isolation level, and unable to carve denials. +struct OwnedOnlyBackend; +impl JailBackend for OwnedOnlyBackend { + fn name(&self) -> &'static str { + "owned-only" + } + fn is_available(&self) -> bool { + false + } + fn is_owned_available(&self) -> bool { + true + } + fn constraint_support(&self) -> tinybox_core::ConstraintSupport { + use tinybox_core::{Constraint, ConstraintSupport, Enforcement}; + ConstraintSupport::NONE + .with(Constraint::Filesystem, Enforcement::Enforced) + .with(Constraint::Network, Enforcement::Enforced) + .with(Constraint::Cpu, Enforcement::Enforced) + .with(Constraint::Memory, Enforcement::Enforced) + .with(Constraint::Pids, Enforcement::Enforced) + } + fn spawn(&self, _: &Jail, _: std::process::Command) -> std::io::Result { + Err(std::io::Error::other("preflight fixture never spawns")) + } +} + +/// The request a host sends for an MCP stdio server: network and children +/// allowed, no isolation floor, no denials. +fn owned_only_request(root: &std::path::Path) -> (NativeJailConfig, tinybox_core::ExecRequest) { + let mut config = config(); + config.allow_net = true; + config.allow_subprocess = true; + let mut command = tinybox_core::ExecRequest::new(["irrelevant"]); + command.cwd = Some(root.into()); + (config, command) +} + +/// The refusal message, which must say why. +fn refusal(result: Result) -> std::result::Result> { + match result { + Err(Error::MethodFailed { message, .. }) => Ok(message), + other => Err(format!("expected a refusal, got {other:?}").into()), + } +} + +#[test] +fn a_backend_usable_only_through_the_owned_api_is_admitted() +-> std::result::Result<(), Box> { + let root = tempfile::tempdir()?; + let (config, command) = owned_only_request(root.path()); + + let jail = prepare(root.path(), &config, &command, &OwnedOnlyBackend)?; + + assert_eq!(jail.root, root.path().canonicalize()?); + Ok(()) +} + +#[test] +fn a_jail_forbidding_children_is_refused_naming_subprocess() +-> std::result::Result<(), Box> { + let root = tempfile::tempdir()?; + let (mut config, command) = owned_only_request(root.path()); + config.allow_subprocess = false; + + for enforcement in [JailEnforcement::Enforced, JailEnforcement::Declared] { + config.enforcement = enforcement; + let message = refusal(prepare(root.path(), &config, &command, &OwnedOnlyBackend))?; + assert!( + message.contains("backend `owned-only` cannot enforce subprocess"), + "{message}" + ); + } + Ok(()) +} + +#[test] +fn a_jail_requiring_process_isolation_is_refused_saying_so() +-> std::result::Result<(), Box> { + let root = tempfile::tempdir()?; + let (mut config, command) = owned_only_request(root.path()); + config.require_process_isolation = true; + + let message = refusal(prepare(root.path(), &config, &command, &OwnedOnlyBackend))?; + + assert!(message.contains("requires process isolation"), "{message}"); + assert!(message.contains("`owned-only`"), "{message}"); + Ok(()) +} + +#[test] +fn a_denial_overlapping_a_read_only_grant_is_refused_for_a_backend_that_cannot_carve() +-> std::result::Result<(), Box> { + let root = tempfile::tempdir()?; + let shared = tempfile::tempdir()?; + let secret = shared.path().join("secret"); + std::fs::create_dir(&secret)?; + let (mut config, command) = owned_only_request(root.path()); + config + .read_only + .push(shared.path().to_string_lossy().into_owned()); + config.deny_read.push(secret.to_string_lossy().into_owned()); + + let message = refusal(prepare(root.path(), &config, &command, &OwnedOnlyBackend))?; + + assert!(message.contains("cannot carve denials"), "{message}"); + Ok(()) +} + +#[test] +fn a_denial_inside_a_writable_grant_is_refused_for_a_backend_that_cannot_carve() +-> std::result::Result<(), Box> { + let root = tempfile::tempdir()?; + let writable = tempfile::tempdir()?; + let secret = writable.path().join("secret"); + std::fs::create_dir(&secret)?; + let (mut config, command) = owned_only_request(root.path()); + config + .read_write + .push(writable.path().to_string_lossy().into_owned()); + config.deny_read.push(secret.to_string_lossy().into_owned()); + + let message = refusal(prepare(root.path(), &config, &command, &OwnedOnlyBackend))?; + + assert!(message.contains("cannot carve a denial"), "{message}"); + Ok(()) +} diff --git a/crates/tinybox-module/src/tinybus_module/native_shell/mod.rs b/crates/tinybox-module/src/tinybus_module/native_shell/mod.rs index f18cbb1f..f7ff234a 100644 --- a/crates/tinybox-module/src/tinybus_module/native_shell/mod.rs +++ b/crates/tinybox-module/src/tinybus_module/native_shell/mod.rs @@ -33,7 +33,12 @@ impl ShellContext { root: &Path, resolved: &tinybox_core::ExecRequest, ) -> Result> { - Self::prepare_with_backend(request, root, resolved, tinybox_jail::default_backend()) + Self::prepare_with_backend( + request, + root, + resolved, + tinybox_jail::default_owned_backend(), + ) } fn prepare_with_backend( request: ShellExecRequest, diff --git a/crates/tinybox-module/src/tinybus_module/resources_stdio.rs b/crates/tinybox-module/src/tinybus_module/resources_stdio.rs index 28e67b10..42274050 100644 --- a/crates/tinybox-module/src/tinybus_module/resources_stdio.rs +++ b/crates/tinybox-module/src/tinybus_module/resources_stdio.rs @@ -105,7 +105,7 @@ impl Resources { &self, request: StdioStartRequest, ) -> Result { - self.start_stdio_with(request, tinybox_jail::default_backend()) + self.start_stdio_with(request, tinybox_jail::default_owned_backend()) .await } diff --git a/crates/tinybox-module/src/tinybus_module/resources_stream.rs b/crates/tinybox-module/src/tinybus_module/resources_stream.rs index 3a2b5add..08f49ddb 100644 --- a/crates/tinybox-module/src/tinybus_module/resources_stream.rs +++ b/crates/tinybox-module/src/tinybus_module/resources_stream.rs @@ -156,7 +156,7 @@ fn select_collector( root, config, resolved, - tinybox_jail::default_backend(), + tinybox_jail::default_owned_backend(), )?, NativeJailRequest::Resources(config) => { super::super::native_jail::collector_with_resources(root, config, resolved)? diff --git a/crates/tinybox-module/src/tinybus_module/stdio.rs b/crates/tinybox-module/src/tinybus_module/stdio.rs index ea7c5537..261a92a5 100644 --- a/crates/tinybox-module/src/tinybus_module/stdio.rs +++ b/crates/tinybox-module/src/tinybus_module/stdio.rs @@ -1,18 +1,19 @@ //! One interactive jailed child with piped stdio, owned by the module. //! -//! The child is spawned through the jail backend (never unconfined), leads its -//! own process group, and is reaped only by its waiter task. Output is pumped +//! The child is spawned through the jail backend's owned-process API (never +//! unconfined), so a backend that cannot hand back a `std::process::Child` +//! (Windows `AppContainer`) works too. It leads its own process group where the +//! backend owns one, and is reaped only by its waiter task. Output is pumped //! into bounded per-stream buffers; a full buffer stops reading the pipe, which //! backpressures the child instead of dropping bytes. use super::resources::failure; use std::collections::VecDeque; -use std::process::Child; use std::sync::{Arc, Mutex as StdMutex}; use tinybox_bus::{ MAX_STDIO_BUFFER_BYTES, MAX_STDIO_READ_BYTES, MAX_STDIO_WAIT_MS, STDIO_CLOSED, StdioChunk, StdioExit, StdioStartRequest, StdioStatus, }; -use tinybox_jail::{Jail, JailBackend}; +use tinybox_jail::{ChildStatus, Jail, JailBackend, OwnedChild}; use tinybus::Result; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWriteExt}; use tokio::sync::Notify; @@ -65,7 +66,7 @@ pub(super) struct StdioSession { request: StdioStartRequest, shared: Arc, stdin: tokio::sync::Mutex>, - child: Arc>>, + child: Arc>>, group: i32, pumps: Vec, } @@ -77,15 +78,12 @@ impl StdioSession { request: StdioStartRequest, backend: Arc, jail: Jail, - mut command: std::process::Command, + command: std::process::Command, ) -> Result> { - #[cfg(unix)] - { - use std::os::unix::process::CommandExt; - command.process_group(0); - } + // The owned adapter puts the child in a new process group on Unix and + // reports it; a backend with its own ownership (a Job Object) does not. let mut child = tokio::task::spawn_blocking(move || { - tinybox_jail::spawn_captured_with(backend.as_ref(), &jail, command) + tinybox_jail::spawn_owned_captured_with(backend.as_ref(), &jail, command) }) .await .map_err(|_| backend_failure("stdio start worker failed"))? @@ -96,12 +94,15 @@ impl StdioSession { ); backend_failure("jailed stdio start failed") })?; - let group = i32::try_from(child.id()).unwrap_or(0); + let group = child + .local_process_group_id() + .and_then(|group| i32::try_from(group).ok()) + .unwrap_or(0); let pipes = (|| { Some(( - tokio::process::ChildStdin::from_std(child.stdin.take()?).ok()?, - tokio::process::ChildStdout::from_std(child.stdout.take()?).ok()?, - tokio::process::ChildStderr::from_std(child.stderr.take()?).ok()?, + tokio::process::ChildStdin::from_std(child_pipe(child.take_stdin()?)).ok()?, + tokio::process::ChildStdout::from_std(child_pipe(child.take_stdout()?)).ok()?, + tokio::process::ChildStderr::from_std(child_pipe(child.take_stderr()?)).ok()?, )) })(); let Some((stdin, stdout, stderr)) = pipes else { @@ -241,7 +242,21 @@ fn drain(buffers: &mut Buffers, max: usize) -> StdioChunk { } } -fn terminate(group: i32, child: &mut Child) { +/// An owned pipe back as a standard child pipe, so the async plumbing is the +/// same whichever backend spawned the child. +#[cfg(unix)] +fn child_pipe>(file: std::fs::File) -> T { + T::from(std::os::fd::OwnedFd::from(file)) +} + +/// An owned pipe back as a standard child pipe, so the async plumbing is the +/// same whichever backend spawned the child. +#[cfg(windows)] +fn child_pipe>(file: std::fs::File) -> T { + T::from(std::os::windows::io::OwnedHandle::from(file)) +} + +fn terminate(group: i32, child: &mut OwnedChild) { #[cfg(unix)] if group > 0 { use nix::{ @@ -252,13 +267,15 @@ fn terminate(group: i32, child: &mut Child) { } #[cfg(not(unix))] let _ = group; - let _ = child.kill(); + let _ = child.terminate(); } /// A killed child still needs a wait to leave no zombie behind. -fn reap_detached(mut child: Child) { +fn reap_detached(mut child: OwnedChild) { std::thread::spawn(move || { - let _ = child.wait(); + while matches!(child.try_wait(), Ok(None)) { + std::thread::sleep(REAP_POLL); + } }); } @@ -300,7 +317,7 @@ async fn pump(mut pipe: impl AsyncRead + Unpin, shared: Arc, which: Pipe /// Sole reaper: record the exit, then kill what the child left behind so the /// pipes reach end of file. -async fn reap(child: Arc>>, shared: Arc, group: i32) { +async fn reap(child: Arc>>, shared: Arc, group: i32) { loop { let waited = { let mut guard = child @@ -341,13 +358,9 @@ async fn reap(child: Arc>>, shared: Arc, group: i } } -fn exit_of(status: std::process::ExitStatus) -> StdioExit { - #[cfg(unix)] - { - use std::os::unix::process::ExitStatusExt; - if let Some(signal) = status.signal() { - return StdioExit::Signal(signal); - } +fn exit_of(status: ChildStatus) -> StdioExit { + if let Some(signal) = status.signal() { + return StdioExit::Signal(signal); } status.code().map_or(StdioExit::Unknown, StdioExit::Code) } From fafa86cc1b81e53100011c3a4c526a1490813648 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Thu, 12 Nov 2026 12:28:23 +0530 Subject: [PATCH 2/2] Pass a cmd script through AppContainer verbatim, and pin the owned jail status Under AppContainer the native shell's cmd invocation failed: the spawner re-quoted every argument by CommandLineToArgvW rules, so the script cmd was handed as an already-quoted raw argument reached it as \"script\" and ran as a program name. cmd takes everything after /C verbatim, so for cmd.exe those arguments now pass through unchanged. jail_status_reports_the_detected_backend_without_starting_a_process still compared against the legacy selection, which is "unsupported" on Windows; it now checks the owned backend jail_status reports. --- crates/tinybox-jail/src/windows.rs | 18 ++++++++++++- crates/tinybox-jail/src/windows_tests.rs | 25 +++++++++++++++++++ .../src/tinybus_module/mod_tests.rs | 6 +++-- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/crates/tinybox-jail/src/windows.rs b/crates/tinybox-jail/src/windows.rs index d11e5574..2188d7d2 100644 --- a/crates/tinybox-jail/src/windows.rs +++ b/crates/tinybox-jail/src/windows.rs @@ -775,12 +775,28 @@ unsafe fn grant_sid_access(path: &Path, sid: PSID, access: u32) -> io::Result<() fn build_command_line(cmd: &Command) -> String { // Minimal CommandLineToArgvW-compatible quoting. std's internal // `make_command_line` is private; this mirrors the same rules. + // + // cmd.exe does not parse its command line that way: after `/C` (or `/K`) + // it takes the rest verbatim, and the native shell hands it an already + // quoted `raw_arg` for exactly that. Re-quoting it would escape those + // quotes and cmd would run `\"script\"` as a program name, so for cmd + // everything after `/C` is passed through as given. let mut out = String::new(); let prog = cmd.get_program().to_string_lossy().into_owned(); push_arg(&mut out, &prog); + let is_cmd = std::path::Path::new(&prog).file_name().is_some_and(|name| { + name.eq_ignore_ascii_case("cmd.exe") || name.eq_ignore_ascii_case("cmd") + }); + let mut verbatim = false; for a in cmd.get_args() { out.push(' '); - push_arg(&mut out, &a.to_string_lossy()); + let a = a.to_string_lossy(); + if verbatim { + out.push_str(&a); + } else { + push_arg(&mut out, &a); + verbatim = is_cmd && (a.eq_ignore_ascii_case("/c") || a.eq_ignore_ascii_case("/k")); + } } out } diff --git a/crates/tinybox-jail/src/windows_tests.rs b/crates/tinybox-jail/src/windows_tests.rs index f322fccc..aba4ab88 100644 --- a/crates/tinybox-jail/src/windows_tests.rs +++ b/crates/tinybox-jail/src/windows_tests.rs @@ -480,3 +480,28 @@ fn wait_for_child(child: &mut crate::OwnedChild) -> std::io::Result tinybus::Result tinybus::Result<()> { let status = BoxService::default().jail_status().await?; - let backend = tinybox_jail::default_backend(); + // The owned-process backend every jailed path uses: AppContainer on + // Windows, where the legacy selection is "unsupported". + let backend = tinybox_jail::default_owned_backend(); assert_eq!(status.backend, backend.name()); - assert_eq!(status.available, backend.is_available()); + assert_eq!(status.available, backend.is_owned_available()); assert_eq!(status.isolation, backend.isolation().to_string()); assert_eq!( status.suitable_for_untrusted_code,