Skip to content

feat: mxc lib crate#524

Open
caarlos0 wants to merge 85 commits into
microsoft:mainfrom
caarlos0:mxc-crate
Open

feat: mxc lib crate#524
caarlos0 wants to merge 85 commits into
microsoft:mainfrom
caarlos0:mxc-crate

Conversation

@caarlos0

@caarlos0 caarlos0 commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown

📖 Description

Adds mxc, an importable Rust library crate for starting sandboxes in-process, without a pty. Callers can stream sandboxed processes without shelling out to the executor binaries or depending on the @microsoft/mxc-sdk TypeScript module.

Supported backends: Bubblewrap (Linux), Seatbelt (macOS), and Windows ProcessContainer (AppContainer + BaseContainer fallback).

To share one execution path between the new library (no-pty, caller-owns-stdio) and the existing executor binaries (run-to-completion), the three library backends were unified on a single SandboxBackend trait. That part is a refactor of the shared backend layer — not purely additive; see "Unified backend execution model" below.

What's included

  • Handle-based streaming API — spawn_sandbox takes a pre-built ExecutionRequest (typically from build_request, see below) and returns a SandboxProcess (in wxc_common::sandbox_process) for persistent bidirectional stdio (take_stdin/take_stdout/take_stderr), try_wait(), id(), kill(), and wait(). No pty is allocated; wait() drains and discards any untaken stdout/stderr (so the child can't block on a full pipe) and returns the exit code as io::Result<i32> (ErrorKind::TimedOut on timeout). Streaming is implemented for Seatbelt, Bubblewrap, and Windows ProcessContainer. The pipe-deadlock contract (drain stdout/stderr concurrently) is documented on the trait.
  • Unified backend execution model — the three library backends (Seatbelt, Bubblewrap, Windows ProcessContainer) now implement one SandboxBackend trait (validate + spawn(request, logger, StdioMode) -> Box<dyn SandboxProcess> + a diagnose_exit hook that preserves AppContainer launch diagnostics). StdioMode::Pipes is what the library uses (the caller takes stdin/stdout/stderr); StdioMode::Inherit is what the executor binaries use (the child inherits the binary's own stdio, preserving the TTY when launched under a pty). A single generic Runner<B> adapter in wxc_common::sandbox_process provides the run-to-completion ScriptRunner the binaries dispatch on, by calling spawn(StdioMode::Inherit) then wait() — so there is no longer any per-backend run-to-completion logic and the old StreamingRunner trait is removed. The wxc-exec / lxc-exec / mxc-exec-mac binaries wrap their backends in Runner and otherwise behave identically (LXC keeps its own native pty path).
  • Process-tree termination — kill() and timeouts take down the whole tree (Unix process-group signal with an unconditional SIGKILL sweep so a racily-forked descendant can't survive; Windows job-object terminate), so descendants can't keep inherited pipe write-ends open and hang wait(). A timeout surfaces uniformly as wait() returning io::ErrorKind::TimedOut across all backends.
  • Clean environment — sandboxed children never inherit the host process's environment (the Seatbelt backend now always env_clear()s, matching Bubblewrap's --clearenv and the AppContainer clean block), so an embedding host app's secrets aren't exposed to untrusted code.
  • Ported SDK config-building helpers — mxc::policy (SandboxPolicy, build_request, plus available_tools_policy / user_profile_policy / temporary_files_policy discovery) and mxc::platform_support, so callers no longer need the TypeScript SDK for policy construction or platform discovery. build_request maps a SandboxPolicy to the ExecutionRequest that spawn_sandbox consumes (set script_code, and any working_directory / env, on the returned request before spawning).
  • Unsupported-host guard — a clear error on platforms with no MXC backend.

Limitations

  • LXC is not supported by the library: the LXC backend has no non-pty capture path (it always streams via a pty and returns empty stdout/stderr), so it can't honour the crate's no-pty contract; it returns unsupported_containment. The lxc-exec binary keeps its native LXC support.
  • dry_run is rejected by the streaming spawn (there is no process to stream).
  • The Windows ProcessContainer integration tests run a real sandbox, so they require an elevated, host-prepped Windows host (see docs/host-prep.md) and are #[ignore]d.

🔭 Follow-ups (separate PRs)

The unified SandboxBackend / Runner model can be extended to the remaining backends incrementally; the cost is set by each backend's I/O model, not by trait plumbing:

  • LXC and IsolationSession are the natural next steps — both drive a real process (IsolationSession already streams live via relay threads + ConPTY), so each is a focused SandboxProcess impl that would also bring the backend into the mxc library (Linux-via-LXC; capable Windows hosts).
  • NanVix (MicroVM) is doable but heavier — it has a host child, but the filesystem copy-back/staging teardown has to move onto the handle's lifecycle.
  • Hyperlight, Windows Sandbox, and WSLC run to completion across an in-process FFI / in-VM daemon / COM-SDK boundary with no local child, so real streaming is gated by the underlying runtime/protocol. These can stay on ScriptRunner until a concrete need arises.
  • platform_support UI / isolation reporting — the SDK getPlatformSupport parity fields (isolation tier, tier-degradation warnings, and the Windows UI-capability probe results) were trimmed from PlatformSupport since no consumer reads them yet. Restore them (re-wiring the AppContainer probe) when a caller needs richer host-capability discovery.

🔗 References

Follow-up to the TypeScript SDK's one-shot spawn surface; this is its in-process Rust counterpart.

🔍 Validation

  • cargo test -p mxc on macOS — 30 tests pass (sandbox 7, sdk_helpers 13, streaming 10); 4 Windows-host integration tests #[ignore]d (they spawn a real sandbox and need an elevated, host-prepped Windows host).
  • cargo fmt --all -- --check and cargo clippy --all-targets -- -D warnings clean (macOS + Linux + Windows cross-check).
  • Cross-compilation: cargo check against x86_64-pc-windows-msvc and aarch64-unknown-linux-gnu, plus the per-backend crates on each target.
  • Runtime-validated on all three host platforms after the backend unification:
    • macOS / Seatbelt — exec with piped stdio and with inherited stdio, exit-code propagation, the guiAccess GUI path, launchMethod: open (Terminal), and timeout tree-kill.
    • Linux / Bubblewrap — output streaming, non-zero exit propagation, inherited TTY, and timeout tree-kill.
    • Windows / ProcessContainer — BaseContainer (schema 0.7.0-alpha) streams stdout and propagates a non-zero exit code (exit /b 7 → EXIT=7); a 2000 ms scriptTimeout fires and tree-kills a process tree blocked on a modal dialog (before streamed, after never ran). The direct-AppContainer/BFS path needs bfscfg.exe, absent on the test host, so it was covered by compile checks only.
  • Test coverage spans output streaming via taken stdout/stderr, host-environment scrubbing, env reaching the sandboxed child, streaming bidirectional stdio, process-tree termination (incl. the racily-forked-descendant case), finite-timeout enforcement (uniform io::ErrorKind::TimedOut from wait()), the guiAccess streaming rejection, dry_run rejection, version-out-of-range / empty-version rejection, and the build_request filesystem/network/clipboard/proxy wire mapping.

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task

caarlos0 and others added 20 commits June 11, 2026 15:51
@caarlos0
feat(mxc): add importable library crate for starting sandboxes
9fee9b0 
Add a new `core/mxc` library crate — the Rust analogue of the SDK's
`spawnSandboxFromConfig` with `usePty: false`. It parses the same JSON
config the executor binaries consume, selects the host containment
backend, runs the sandboxed process without ever allocating a pty, and
returns captured stdout/stderr in a `ScriptResponse`.

Supported backends: Bubblewrap (Linux), Seatbelt (macOS), and
ProcessContainer — AppContainer plus the BaseContainer fallback —
(Windows). Other backends return `MxcError::unsupported_containment`.

Output capture is gated behind the new `ExecutionRequest::capture_output`
flag (default `false`, preserving the binaries' streaming behaviour;
the library forces it `true`):
- Seatbelt gains a non-pty captured exec path (mirrors bubblewrap).
- AppContainer/BaseContainer gain a capture path using CreatePipe +
  reader threads, reusing the existing `process_util` helpers.
- Bubblewrap already captures.

Backend selection is centralised in `mxc::dispatch::select_runner`, and
the `wxc`, `lxc`, and `mxc_darwin` binaries now delegate their shared
backend arms to it so the selection logic has a single home.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
feat(mxc): add handle-based spawn for live stdio and kill
2610992 
Add a streaming, handle-based spawn API alongside the run-to-completion
entrypoint, so importers can drive a sandboxed process while it runs:
persistent bidirectional stdio plus termination. No pty is allocated;
the streams are ordinary pipes.

- `wxc_common::sandbox_process`: new `SandboxProcess` handle trait
  (`take_stdin`/`take_stdout`/`take_stderr` -> boxed Write/Read,
  `try_wait`, `kill`, `wait`) and the `StreamingRunner` spawn trait.
  Interfaces live in `wxc_common`; impls live in the backend crates.
- Seatbelt: `SeatbeltSandboxProcess` + `StreamingRunner` impl — spawns
  the sandboxed shell with piped stdio, `kill()` does graceful SIGTERM
  then SIGKILL after a grace period, and `wait()` drains any stream the
  caller did not take into the `ScriptResponse`.
- `mxc::spawn_sandbox` + `dispatch::spawn_runner` return the handle
  (cfg-split; Seatbelt implemented, other backends report
  `unsupported_containment` for now). Re-exports `SandboxProcess`.

Verified on macOS (bidirectional stdio against `cat`, kill of `sleep`,
and untaken-stream capture via `wait`). Linux/Windows streaming impls
are follow-ups.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(seatbelt): report ProcessExited and stop getcwd stderr leak
1a98ee0 
Two Seatbelt backend bugs:

- Non-zero exits reported `failurePhase: None`. The runner never set
  `failure_phase`, so every result defaulted to `None`. Add an
  `exit_response` helper that tags `ProcessExited` on a non-zero exit
  (and `None` on success), and route all run-completed paths (exec/pty,
  captured, open, and the streaming `wait()`) through it.

- With no `working_directory` set, the child inherited the host cwd;
  under the deny-by-default profile that directory (or its parents)
  can be unreadable, so the child's `getcwd()` walked parent dirs and
  leaked "getcwd: ... Operation not permitted" to stderr. Default the
  cwd to a sandbox-allowed path (first readwrite, else readonly, else
  `/`) and export a matching `PWD` so `getcwd()` uses its fast stat
  path instead of enumerating unreadable parents.

Adds regression tests for both (failure phase on non-zero/zero exit and
no getcwd leak when no working directory is set).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
feat(mxc): Windows AppContainer streaming spawn (handle-based)
0987176 
Implement the handle-based streaming API (live stdio + kill) for the
Windows AppContainer backend, the counterpart to the Seatbelt support.

- Add Win32 `PipeReader`/`PipeWriter` adapters in `process_util` so the
  child's pipe ends can be exposed as `Read`/`Write`.
- Refactor `AppContainerScriptRunner` so the monolithic run path is split
  into a shared `spawn_suspended` (setup + CreateProcess, suspended) and a
  `SpawnedChild` that either runs to completion (existing blocking path,
  behaviour preserved) or is wrapped in a streaming handle. Per-run
  firewall/BFS policy setup/teardown is factored into `prepare`/`teardown`
  so both paths share it.
- Add `AppContainerSandboxProcess` (impl `SandboxProcess`): take_stdin/
  stdout/stderr, try_wait, kill (TerminateProcess), and wait (drains
  untaken streams, runs teardown, adds exit diagnostics). Tears down
  firewall/BFS policy on wait or drop.
- Wire `mxc::dispatch::spawn_runner` for ProcessContainer on Windows
  (AppContainer fast path). The BaseContainer fallback returns
  `unsupported_containment` for now (streaming pending).

Cross-checked for x86_64-pc-windows-msvc; the in-process Windows path is
not runtime-verified here and needs Windows CI. macOS run path and tests
unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
feat(mxc): Windows BaseContainer streaming spawn (handle-based)
a259734 
Complete Windows streaming by adding the handle-based path for the
BaseContainer backend (the OS sandbox API used for experimental /
newer-schema configs), the common case for the library on modern schemas.

- Split BaseContainerRunner::execute() into a shared `spawn_base`
  (proxy/spec/identity setup + CreateProcessInSandbox, returning a
  `BaseChild`) and a `run_to_completion` for the blocking path; behaviour
  of the run path is preserved.
- `BaseChild` owns the process handle, parent-side pipes, and the per-run
  proxy/sandbox-tracking state, and performs `run_sandbox_cleanup` +
  proxy stop + exit diagnostics after the child exits.
- Add `BaseContainerSandboxProcess` (impl `SandboxProcess`): live stdio,
  kill (TerminateProcess), try_wait, and wait (drains untaken streams,
  tears down sandbox/proxy, shapes the response). Drop runs teardown.
- `mxc::dispatch::spawn_runner` now selects AppContainer vs BaseContainer
  for ProcessContainer streaming, mirroring the run-to-completion choice.

Cross-checked for x86_64-pc-windows-msvc; like the AppContainer path the
in-process Windows behaviour (and resource teardown ordering) is not
runtime-verified here and needs Windows CI. Linux/Bubblewrap streaming
remains a follow-up. macOS tests unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
feat(mxc): clear error on unsupported host OS
ef7c507 
Before backend selection, `select_runner` and `spawn_runner` now check the
host OS and return a clear message on platforms with no MXC backend:
"the mxc library has no sandbox backend for this host OS (supported:
Windows, Linux, macOS)".

Previously an unsupported host (e.g. FreeBSD) fell through to the default
ProcessContainer arm and surfaced the misleading "...only available on
Windows" error. Supported hosts are unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
feat(mxc): expose process id and make kill() a process-tree kill
776f103 
Callers cancelling/timing out a sandboxed command need to take its whole
process tree down (pipelines, `&`, servers), not just the root. Add the
pid and make kill() tree-kill on every streaming backend:

- `SandboxProcess::id() -> u32` exposes the child's OS process id for
  external monitoring or a caller-driven tree kill.
- Seatbelt: spawn the streaming child with `setsid()` so it leads its own
  process group; kill() now signals the whole group (negative-pid
  SIGTERM then SIGKILL), which is safe even if setsid failed (a negative
  pid only targets that pid's group, never the host's).
- Windows AppContainer: kill() terminates the job object (new
  `UiJobObject::terminate`) instead of just the root process.
- Windows BaseContainer: best-effort-assign the child to a job object
  after creation so kill() can terminate the tree (falls back to the
  root process if assignment fails).

Adds a macOS test that backgrounds a descendant and asserts kill() reaps
it. Windows paths cross-checked only (need Windows CI).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
feat(mxc): streaming spawn for the Bubblewrap backend
c96648b 
Add the handle-based streaming API (live stdio, kill, id) for the
Bubblewrap (Linux) backend, completing streaming across every backend the
library supports.

- Split BubblewrapScriptRunner::execute() into a shared `spawn_bwrap`
  (proxy + iptables setup + bwrap spawn, returning a `BwrapChild`) and a
  `run_to_completion` for the blocking path; run-path behaviour preserved.
- Streaming spawns bwrap with piped stdin and `process_group(0)` so the
  handle can tree-kill it: since bwrap is PID 1 of the sandbox pid
  namespace (`--unshare-pid`), a `killpg` on its group tears the whole
  sandbox down. `BubblewrapSandboxProcess` (impl `SandboxProcess`) owns the
  network proxy/iptables state and tears it down on wait or drop.
- Wire `mxc::dispatch::spawn_runner` for the Bubblewrap arm on Linux.

Cross-checked + clippy-clean for aarch64-unknown-linux-gnu (via the stable
toolchain, since the pinned toolchain's x86_64-linux std is unavailable
here); runtime still needs Linux CI with bwrap. macOS run path/tests
unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(seatbelt): dedupe spawn-error message
ccea5cd 
The PermissionDenied-vs-generic `Command::spawn` failure message was
copy-pasted across the GUI, captured, and streaming exec paths. Extract a
private `spawn_error(&io::Error) -> String` helper; behaviour unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
feat(mxc): port SDK config-building helpers into the crate
45fd2b7 
Port the 5 `@microsoft/mxc-sdk` config-building helpers to Rust so callers
no longer need the TypeScript module to build a spawnable config.

- `mxc::policy`: `SandboxPolicy` (mirrors the SDK type) and
  `build_request` — the port of `createConfigFromPolicy`, restricted to the
  backends the crate runs (`Containment::Process` resolves to Seatbelt /
  Bubblewrap / ProcessContainer per host; `Containment::Bubblewrap` forces
  Bubblewrap). It mirrors the SDK field mapping and network proxy /
  host-filtering validation, building the same wire config internally and
  running it through the shared parser (with allow_missing_command, since the
  caller fills script_code), so validation and the wire->model mapping match
  production. Output is an `ExecutionRequest`.
- `mxc::policy` discovery helpers `available_tools_policy`,
  `user_profile_policy`, `temporary_files_policy` (ports of policy.ts) —
  PATH + tool/SDK env dirs, user-profile dirs, and the host temp dir.
- `mxc::platform::platform_support` — port of `getPlatformSupport`; on
  Windows it reads the isolation tier / UI capabilities from the in-process
  fallback probe instead of shelling out to `wxc-exec --probe`.
- Add `spawn_streaming_from_request` to round out the request-based API.
- Derive `Clone` on the probe's `UiCapabilitySupport` so it can be carried
  on the cloneable `PlatformSupport`.

Verified on macOS (9 new tests incl. policy -> build_request -> real Seatbelt
run, plus the existing suite; clippy + fmt clean). Cross-checked for
x86_64-pc-windows-msvc and aarch64-unknown-linux-gnu; their runtime behaviour
still needs platform CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): don't close the BaseContainer process handle during job ass…
d905a6b 
…ignment

`spawn_base` wrapped `pi.hProcess` in a temporary `OwnedHandle` just to pass
it to `job.assign_process(...)`. `OwnedHandle::get()` returns a copy (HANDLE is
Copy) and the temporary's `Drop` then closed `pi.hProcess` — so the handle
stored on `BaseChild` pointed at a closed (and possibly reused) handle. That
broke every BaseContainer run/stream: `WaitForSingleObject`/`GetExitCodeProcess`
on a dead handle (garbage/-1 exit codes), wait/terminate potentially acting on
an unrelated reused handle, and a double-close on drop.

`assign_process` only borrows the handle (it calls `AssignProcessToJobObject`
and neither stores nor closes it), so pass the raw `pi.hProcess` and keep sole
ownership on the `BaseChild.process` field.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): tree-kill on AppContainer timeout so scriptTimeout is enforced
7801d72 
Both timeout arms (run-to-completion SpawnedChild::wait_exit and the streaming
AppContainerSandboxProcess::wait) terminated only the direct child. The job
has no KILL_ON_JOB_CLOSE, so descendants survived holding the inherited
stdout/stderr write-ends open — the capture reader threads then blocked in
read_to_string forever, join() never returned, and the timeout was silently
not enforced (the exact runaway case it exists for), leaving orphaned
processes behind.

Terminate the job object (as kill() already does) in both WAIT_TIMEOUT arms so
the whole tree dies and the pipe write-ends are released.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
docs(mxc): correct the Windows streaming fallback comment
191bc49 
spawn_process_container claimed it "mirrors the run-to-completion selection",
but the streaming path does not route through dispatch_with_fallback: there is
no AppContainer-BFS / AppContainer-DACL fallback for streaming (that dispatcher
returns a run-to-completion runner plus a DaclManager guard, neither of which
fits a streaming handle). The behaviour is fail-closed and safe — no
containment weakening and the BaseContainer tier applies no host DACLs — but a
config that runs via spawn_sandbox_from_config on a host lacking the native
BaseContainer API would fail via spawn_sandbox.

Replace the inaccurate comment with the real contract and document the
divergence in the crate README. No behaviour change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(seatbelt): group-kill on streaming timeout to reap descendants
4c859b8 
SeatbeltSandboxProcess::wait()'s timeout branch called self.child.kill(),
which signals only the direct child and orphans sandboxed descendants — even
though kill() already does the correct setsid + killpg(SIGTERM->SIGKILL) group
kill. Reuse self.kill() in the timeout branch so the whole process group dies
(and the captured pipe write-ends are released), then reap the child.

Adds a macOS test that backgrounds a descendant and asserts it is killed when
the streaming wait times out.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): terminate and reap the child before network/policy teardown…
5bf9578 
… on Drop

Dropping a streaming handle while the sandboxed process was still running ran
the teardown (iptables/proxy on Bubblewrap; firewall/BFS on AppContainer;
proxy/sandbox state on BaseContainer) but never terminated the child. An
abandoned-but-running sandbox would then lose its host enforcement (firewall
mode -> unrestricted egress) and leak as a zombie/orphan.

Drop now terminates and reaps the child (reusing each handle's kill()) before
removing enforcement, across all three streaming backends. The post-wait Drop
path stays a no-op (try_wait early-return + teardown_done guard).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): use correct clipboard wire values in build_request
55bd704 
ClipboardPolicy::wire() emitted "readOnly"/"readWrite", but the config parser
only accepts "none"/"read"/"write"/"all" — so every non-None clipboard policy
built via build_request was silently dropped to None (fail-closed but wrong).
The enum was also missing write-only and read+write(all) levels.

Redefine ClipboardPolicy as None/Read/Write/All with wire strings
none/read/write/all, matching the SDK type and the parser. Adds a regression
test asserting all four levels survive build_request -> ExecutionRequest.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): resolve policy paths lexically so the system-critical filte…
e50599f 
…r works

available_tools_policy resolved paths via Path::canonicalize, which on Windows
yields a \\?\ verbatim prefix (and resolves symlinks). is_system_critical_path
compares against C:\Windows, so the prefixed form never matched and System32
(on PATH) was not filtered out of readonly_paths — weakening the protective
filter. canonicalize also hits the filesystem, unlike the SDK's lexical
path.resolve.

Replace canonicalize with lexical absolutization (join with cwd if relative,
then collapse ./.. via components, preserving the prefix/root). This matches
path.resolve, keeps the plain C:\... form, and touches no filesystem. Adds a
test that a system-critical dir on PATH is filtered.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
docs(mxc): document the SandboxProcess pipe-deadlock contract
912780b 
The deadlock-avoidance rule was unstated, so every backend author re-derived
it. Document on the SandboxProcess trait that stdout and stderr are
independent bounded pipes and must be consumed concurrently: wait()
implementors must drain not-taken streams on separate threads, and callers
taking both streams must read them concurrently (taking only one is always
safe). No code change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(mxc): address low-severity review items
efaee61 
- dispatch: support LXC on the run-to-completion path (select_runner);
  reject dry_run on the streaming path (no process to stream)
- dispatch: restore wxc-exec diagnostic parity for the BaseContainer
  fallback (the "Using BaseContainer-fallback dispatcher (reason)" line
  and the "warning: " prefix) inside select_process_container
- lib: correct the env-injection doc (replaces, vs the SDK's append), and
  the spawn_sandbox doc (dry_run is rejected, not ignored); drop the
  logger buffer from error strings (may contain config/env); remove the
  dead capture_output assignment on the streaming entrypoint
- policy: read SystemDrive from the process environment (SDK parity);
  drop the logger buffer from the build_request error string
- sandbox_process: clarify id() is only meaningful while the child is
  alive (Unix PID reuse after reap)
- process_util: add SAFETY comments to the ReadFile/WriteFile blocks
- README: fix proc.wait() (returns ScriptResponse, not Result) and mark
  the live spawning examples no_run
- lxc: drop the now-unused bwrap_common dependency

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
test(mxc): cover the gaps that let review items #1/microsoft#2 slip t…
d2df21f 
…hrough

Add the missing coverage called out in review:

- run path: dry_run validates without executing; finite scriptTimeout
  fires; working_directory override; env override replaces a config value;
  is_base64 input; stderr-only capture; version-out-of-range and malformed
  JSON rejection
- streaming: take_* second call returns None; try_wait reports the exit
  code after completion
- sdk helpers: build_request maps allowed/blocked hosts and local-network;
  platform_support assertions gated for Linux and Windows
- Windows ProcessContainer integration tests (AppContainer + BaseContainer
  capture, AppContainer finite-timeout-with-descendant, streaming stdio) as
  the regression guards for #1 (closed process handle) and microsoft#2 (timeout only
  reaching the direct child). They run a real sandbox, so they require an
  elevated, host-prepped Windows host and are #[ignore]d.

Also split the macOS-only test imports behind cfg so the test crate compiles
cleanly when cross-targeting Windows and Linux.

Verified: cargo test -p mxc (macOS, 37 run + 4 ignored Windows), fmt, clippy
-D warnings; cargo check -p mxc --tests cross-targeted to Windows and Linux.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 12, 2026 13:43
Copilot AI reviewed Jun 12, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new in-process mxc Rust library plus a cross-backend streaming (“handle-based”) sandbox API, while refactoring executor binaries to share backend-selection logic and introducing an opt-in output-capture switch in ExecutionRequest.

Changes:

  • Introduce wxc_common::sandbox_process traits (SandboxProcess, StreamingRunner) for streaming sandbox execution.
  • Add ExecutionRequest::capture_output and implement capture/streaming across Seatbelt, Bubblewrap, and Windows ProcessContainer (AppContainer/BaseContainer).
  • Add new mxc library crate (dispatch, policy builder, platform support) and update wxc/lxc/mxc_darwin binaries to reuse shared dispatch.
Show a summary per file
File Description
src/core/wxc_common/src/sandbox_process.rs Defines streaming handle traits and deadlock contract for concurrent drain.
src/core/wxc_common/src/process_util.rs Adds Windows pipe wrapper types implementing Read/Write over Win32 handles.
src/core/wxc_common/src/models.rs Adds ExecutionRequest::capture_output flag.
src/core/wxc_common/src/lib.rs Exposes the new sandbox_process module.
src/core/wxc_common/src/config_parser.rs Sets default capture_output: false when building requests from config.
src/core/wxc/src/main.rs Uses mxc::select_runner for ProcessContainer backend selection/guarding.
src/core/wxc/Cargo.toml Adds dependency on the new mxc crate.
src/core/mxc_darwin/src/main.rs Uses mxc::select_runner for shared backend selection.
src/core/mxc_darwin/Cargo.toml Switches to mxc dependency instead of direct Seatbelt dependency.
src/core/mxc/tests/streaming.rs Adds streaming API tests (stdio, kill, wait capture).
src/core/mxc/tests/sdk_helpers.rs Adds tests for policy helpers, platform support, and request builder.
src/core/mxc/tests/sandbox.rs Adds end-to-end mxc library tests (config parsing, capture, timeout, Windows ignores).
src/core/mxc/src/policy.rs Implements SDK-like policy discovery and SandboxPolicy→ExecutionRequest builder.
src/core/mxc/src/platform.rs Implements host platform support detection (SDK-like).
src/core/mxc/src/lib.rs Provides public mxc library API: spawn (capture) + spawn (streaming) + helpers.
src/core/mxc/src/dispatch.rs Centralizes backend runner selection and streaming spawn for library/binaries.
src/core/mxc/README.md Documents mxc usage, policy builder, and streaming API.
src/core/mxc/Cargo.toml Defines the new mxc crate and OS-specific backend deps.
src/core/lxc/src/main.rs Routes Bubblewrap selection through mxc::select_runner.
src/core/lxc/Cargo.toml Adds mxc dependency and removes direct bwrap_common dep.
src/backends/seatbelt/common/src/seatbelt_runner.rs Adds capture-mode execution, fixes cwd behavior, implements StreamingRunner + SandboxProcess.
src/backends/bubblewrap/common/src/bwrap_runner.rs Refactors into spawnable child + adds StreamingRunner + SandboxProcess.
src/backends/appcontainer/common/src/probe.rs Makes UiCapabilitySupport clonable for reuse in mxc::platform.
src/backends/appcontainer/common/src/job_object.rs Adds UiJobObject::terminate helper for process-tree kill.
src/backends/appcontainer/common/src/base_container_runner.rs Refactors to spawnable child + adds capture pipes and StreamingRunner + SandboxProcess.
src/backends/appcontainer/common/src/appcontainer_runner.rs Refactors to suspended spawn + adds capture/streaming handle + shared prepare/teardown.
src/Cargo.toml Adds core/mxc to workspace and workspace dependency entries.
.github/copilot-instructions.md Documents new mxc crate and shared-dispatch architecture.

Copilot's findings

  • Files reviewed: 28/29 changed files
  • Comments generated: 6

Comment thread src/backends/appcontainer/common/src/base_container_runner.rs Outdated
Comment thread src/backends/appcontainer/common/src/base_container_runner.rs Outdated
Comment thread src/core/mxc/src/dispatch.rs Outdated
Comment thread src/core/mxc/src/policy.rs
Comment thread src/backends/bubblewrap/common/src/bwrap_runner.rs Outdated
Comment thread src/backends/appcontainer/common/src/appcontainer_runner.rs Outdated
caarlos0 and others added 8 commits June 12, 2026 10:50
@caarlos0
Merge remote-tracking branch 'upstream/main' into mxc-crate
fecf167 
# Conflicts:
#	src/backends/appcontainer/common/src/base_container_runner.rs
@caarlos0
refactor(mxc): drop LXC from the library crate
20dc3d5 
LXC was never a good fit for the in-process library: the LXC backend has no
non-pty capture path (attach_run always runs via run_with_pty and returns
empty stdout/stderr while streaming to the host terminal), so the run path
violated the crate's "capture output, no pty" contract, and it had no
streaming/SandboxProcess impl at all. Rather than half-support it, the crate
now rejects LXC with unsupported_containment like the other unsupported
backends; Bubblewrap remains the fully-supported Linux backend (captured run
+ streaming + tree-kill). The lxc-exec binary keeps its native LXC support.

- dispatch: remove the ContainmentBackend::Lxc arm and select_lxc
- Cargo: drop the now-unused lxc_common dependency
- platform_support: report only bubblewrap on Linux (no lxc-ls probe)
- tests: assert Linux reports bubblewrap only

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): address PR review feedback on timeouts and dispatch warnings
6ce7304 
Resolve the six inline review comments on microsoft#524:

- base_container streaming wait(): drop an untaken stdin before draining,
  and on timeout tree-terminate via the job object (reuse kill()) instead
  of only the root process, so descendants release the captured pipe
  write-ends and the drain threads can finish.
- base_container capture wait_exit(): tree-terminate via the job object on
  timeout (fall back to the root only when no job exists), same deadlock
  fix for the non-streaming capture path.
- appcontainer streaming wait(): drop an untaken stdin so interactive
  children observe EOF and exit reliably.
- bubblewrap streaming wait(): on timeout use the process-group terminate
  (reuse kill()) instead of a single-process signal, matching the
  SandboxProcess contract.
- dispatch: return the BaseContainer-fallback diagnostics (fallback notice,
  dispatcher warnings, selected tier) via Selection.warnings instead of
  writing them into a logger the library drops; select_runner no longer
  takes a logger and callers surface the warnings themselves.
- policy normalize_lexically(): never pop past a root/prefix on "..", so
  "/a/../../b" stays "/b" and "C:\\.." stays "C:\\" (keeps the
  system-critical-path filter and dedup correct).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): reject empty policy version in build_request
d8446cb 
The shared config parser treats an empty schema version as "unset" and
accepts it, but the SDK requires a version ("Policy version is required").
build_request now rejects an empty `SandboxPolicy.version` with
MalformedRequest for parity. Added a test.

This was the only still-valid item from the latest review pass; the other
findings (LXC dispatch contract, bwrap streaming timeout group-kill, dead
Selection.warnings field, LXC doc drift) were already resolved by the
earlier LXC removal and review-feedback commits.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): deserialize SDK proxy union shape in ProxySpec
9a79212 
The externally-tagged derive only accepted the bare string
"builtinTestServer" for the unit variant, but the SDK wire shape is
`{ "builtinTestServer": true }`. Replace the derive with a custom
Deserialize over an untagged intermediate so all three proxy forms
(`{builtinTestServer:true}` / `{localhost:n}` / `{url:s}`) round-trip.
Serialize (proxy_to_wire) was already correct. (review O8)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
test(mxc): harden streaming/sdk-helper test probes
42dc281 
- pid_alive: use signal-0 `libc::kill(pid, 0)` instead of spawning `ps`,
  removing the PID-reuse race and no longer treating a probe failure as
  "dead" (only ESRCH means gone; EPERM etc. means alive). Adds a macOS
  dev-dependency on libc.
- available_tools_policy_filters_nonexistent_and_dedups: assert the full
  resolved cwd is discovered rather than the near-tautological basename
  substring. (review O11)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): bounded, lossy, non-blocking output capture
9bd53cd 
The capture drains were unsafe for large or binary output:
- Unix `read_to_string` was uncapped (OOM on unbounded child output) and
  failed on the first invalid UTF-8 byte, discarding the whole stream.
- Windows `read_from_pipe` stopped draining once it hit the 1 MB char cap,
  so a child emitting more would block forever on a full pipe (and with the
  library forcing capture_output + INFINITE default timeout, the call hung).
  It also decoded per 4 KB chunk, corrupting multibyte UTF-8 at boundaries.

Add a shared `wxc_common::capture_io::read_capped_lossy` that reads to EOF
(discarding past a 1 MiB byte cap so the child never stalls) and decodes once
with from_utf8_lossy. Both Unix runners delegate to it; `read_from_pipe` is
rewritten to mirror the behaviour over its HANDLE. (review O1, O6, O7)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(seatbelt): never inherit the host environment
326c986 
std::process::Command inherits the parent env by default, and the env was
only cleared when request.env was non-empty. The mxc library emits an empty
process.env by default, so untrusted sandboxed code received the embedding
host process's entire environment (cloud creds, API tokens) and could read
and exfiltrate it. Always env_clear() and set only a baseline PATH plus the
request's vars, matching bubblewrap (--clearenv) and AppContainer. Applied to
both the run and streaming paths via a shared helper; adds a regression test.
(review O2)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
caarlos0 and others added 9 commits June 15, 2026 15:32
@caarlos0
fix(lxc): gate Runner import behind cfg(target_os = "linux")
54f92bd 
The Runner adapter is only referenced in the Linux-gated Bubblewrap
dispatch arm, so on Windows/macOS the ungated import tripped
-D unused-imports and broke the workspace build. Gate the import the
same way as the adjacent BubblewrapScriptRunner import.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
style(lxc): sort the cfg-gated Runner import
97a84d0 
rustfmt orders the cfg-gated use into the alphabetical wxc_common slot;
keep the source fmt-clean after the previous import-gating fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(seatbelt): drop the now-unused mxc_pty dependency
2db6d05 
Unifying the backend on spawn(StdioMode) removed seatbelt's last call
to mxc_pty::run_with_pty: the child now inherits the executor binary's
own stdio under StdioMode::Inherit (a real TTY when the binary runs
under a pty) or exposes pipes under StdioMode::Pipes. The mxc_pty
dependency was left dead. Remove it and refresh the stale pty comments
that still referenced run_with_pty. mxc_pty stays a workspace member;
LXC remains its only consumer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
docs: sync copilot-instructions to the unified SandboxBackend model
4b1681a 
The library backends were unified on wxc_common::sandbox_process:
StreamingRunner is gone, replaced by the SandboxBackend trait
(validate + spawn(StdioMode) + diagnose_exit) plus the generic
Runner<B> adapter that drives run-to-completion for the executor
binaries. Update the architecture notes accordingly and record that
mxc_pty is now LXC-only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(mxc): drop unused public API surface
8c51b1f 
Audited the crate's public surface against its only consumer
(copilot-agent-runtime) and the crate's own tests, and removed what
nothing reads:

- PlatformSupport's isolation_tier / isolation_warnings / ui_capabilities
  fields, the UiCapabilitySupport struct, and the populate_isolation_from_probe
  wiring. Consumers read only is_supported / reason (tests also read
  available_methods); the probe richness was written on Windows but never
  read. platform_support() no longer runs the AppContainer probe.
- The Containment enum and build_request's containment parameter. No caller
  ever passed Containment::Bubblewrap, and on Linux it resolves identically
  to Process. build_request is now build_request(policy, container_name);
  the host-filtering check, the dead Linux proxy branch, and apply_backend
  collapse accordingly.

Net -90 lines. README + tests updated to match. The build_request signature
change requires a matching one-line update in the consumer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
chore: simplify
f391507 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(mxc): take a typed Config instead of a JSON string
046385b 
spawn_sandbox now accepts a typed mxc::Config (the clean public mirror
of the SDK's wire ContainerConfig) and converts it straight to an
ExecutionRequest with no JSON round-trip — callers build the config
programmatically rather than serialising/parsing JSON.

- Config and its sections live in a new wxc_common::config_parser::config
  child module so they map 1:1 onto the internal Raw* parse types and
  call the converter without those needing crate-wide visibility; the
  parser itself is untouched apart from one `pub mod config;` line.
  Re-exported as wxc_common::config and mxc::Config.
- Config carries no JSON: no serde, no from_json/from_base64. The library
  is typed-only (callers never hand it JSON), so SpawnOptions::is_base64
  and the JSON ingestion entry are gone.
- Rewrote the mxc end-to-end tests to build typed Config literals.
- Inlined the now single-use load_and_prepare helper into spawn_sandbox.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(mxc): collapse to a single spawn_sandbox(ExecutionRequest) e…
b4fa048 
…ntry

The consumer drives the library through build_request + the streaming
spawn and never the typed Config / spawn_sandbox(Config) path, so remove
that whole surface and unify on one entrypoint.

- Delete the typed wire Config mirror (wxc_common's config_parser::config
  child module) and its conversion; config_parser.rs and wxc_common/lib.rs
  are back to a zero-line diff vs upstream. Drop the mxc Config* re-exports,
  SpawnOptions, apply_options, and execution_request_from_config.
- Rename the surviving spawn_streaming_from_request to
  spawn_sandbox(request: ExecutionRequest) — now the sole public spawn
  entry. Callers build the request via build_request, fill in script_code
  / working_directory / env, then spawn.
- Migrate the end-to-end tests onto build_request + spawn_sandbox, keeping
  the SandboxProcess-contract and seatbelt regression coverage.
- Drop the now-unused serde::Serialize derive on PlatformSupport (nothing
  serialises it; the consumer reads its fields directly).
- Sync the mxc README and .github/copilot-instructions.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(appcontainer): dedup proxy-var injection and unexpose a helper
0ab9245 
- Extract the identical "strip pre-existing proxy vars, then inject
  HTTP_PROXY / HTTPS_PROXY" block — used by both the explicit-env and
  default-env paths — into a private inject_proxy_vars helper.
- Make is_api_not_implemented private; it is only referenced within
  base_container_runner.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0 caarlos0 marked this pull request as ready for review June 16, 2026 17:23
@caarlos0 caarlos0 requested a review from a team as a code owner June 16, 2026 17:23
@caarlos0

caarlos0 commented Jun 16, 2026

Copy link
Copy Markdown
Author

Manually tested end-to-end through Copilot on macOS, Linux, and Windows — exercising the Seatbelt, Bubblewrap, and Windows ProcessContainer backends respectively via the in-process library path.

caarlos0 and others added 15 commits June 18, 2026 12:37
@caarlos0
Merge remote-tracking branch 'upstream/main' into mxc-crate
a7d9962
@caarlos0
feat(mxc): add stdout/stderr close affordance to streamed sandboxes
b997798 
The streaming `SandboxProcess` handle only ended a stdout/stderr read at
pipe EOF or `kill()`. A backgrounded descendant that inherits a pipe can
hold it open past the foreground command's exit, leaving a reader parked
indefinitely; killing the child to unblock it defeats any grace window
for that descendant.

Add a per-stream `StreamCloser` (via `SandboxProcess::stdout_closer` /
`stderr_closer`) that makes an in-flight or subsequent read return EOF
promptly, without terminating the child:

- Unix (Seatbelt, Bubblewrap): non-blocking pipe + poll(2) + self-pipe
  wakeup (`wxc_common::interruptible_reader`).
- Windows (AppContainer, BaseContainer): CancelIoEx + an Arc-shared
  handle + a cancelled flag (`InterruptiblePipeReader`).

The common streaming path (raw pipe reads) is unchanged; closers default
to `None` for inherited stdio. Re-exported from `mxc` for consumers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): harden streaming stdout/stderr read cancellation
73387bb 
Two latent hangs in the streaming SandboxProcess read path. Both are narrow
(the in-process napi consumer takes both streams, so neither is hit in
practice), but each is an unrecoverable hang when it triggers.

- Windows InterruptiblePipeReader had a lost-cancellation race: CancelIoEx
  is edge-triggered, so a close() landing between read()'s cancelled-flag
  check and its ReadFile entering the kernel cancelled nothing and never
  retried, parking the read until real EOF. Replace the bare AtomicBool with
  a Mutex<{cancelled, reading}> handshake: read() announces `reading` under
  the lock before blocking; close() sets `cancelled` then loops CancelIoEx
  while `reading` is set. The mutex's total order guarantees the cancel can't
  be lost. The common read path stays a plain blocking ReadFile; only close()
  (at teardown) spins briefly.

- wait()'s internal drain of a *not-taken* stdout/stderr ran the discard
  io::copy to real EOF, so a backgrounded descendant holding the pipe open
  past the child's exit wedged the drain (and wait()) under a wait-forever
  (scriptTimeout==0) timeout. wait() now fires the stored canceller for any
  stream it drains before joining (new cancel_drained_stream helper); the
  output is discarded anyway. Applied across Seatbelt, Bubblewrap,
  AppContainer, and BaseContainer.

Also tightens the stdout_closer/stderr_closer docs: a closer is meant for a
stream the caller has taken — wait() handles not-taken drains itself.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): reject network.proxy.builtinTestServer:false
1d83456 
The policy port's `ProxySpec` deserializer matched any object with a
`builtinTestServer` key and discarded the bool, so `{ builtinTestServer:
false }` silently selected the experimental, deliberately-permissive
built-in test proxy — a fail-open against the policy author's intent.

Reject an explicit `false` with a clear error instead (fail-closed),
matching the wire-config parser (config_parser.rs), which already rejects
it. The SDK union type is `{ builtinTestServer: true }`, so no real producer
emits `false`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
chore: simplify
d8b1bb8 
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(wxc_common): harden process-group kill and pipe stream primitives
ad72629 
Five confirmed issues in the cross-platform streaming/kill foundation:

- group_kill (Unix): a leader that had already exited made it return without
  signalling the group, so descendants sharing the pgid survived un-reaped;
  and a transient try_wait error in the grace loop could skip the final
  SIGKILL. Capture the pgid up front and always SIGTERM→SIGKILL the group
  (tolerating ESRCH), treating a wait error as "still running".

- InterruptibleReader::read (Unix): a zero-length buffer entered poll() and
  blocked instead of returning Ok(0) as the Read contract requires.

- PipeReadCanceller (Windows): held a strong Arc over the pipe handle, so a
  stored canceller kept a taken stdout/stderr handle open after its reader was
  dropped (a sandboxed writer could block on a full pipe instead of seeing a
  broken pipe). Hold a Weak reference and upgrade only for the cancel.

- InterruptiblePipeReader::read (Windows): could deliver one
  completed-but-undelivered chunk after close() fired; now snapshots the
  cancelled flag after the read and reports EOF, matching the Unix reader.

- PipeWriter::write (Windows): didn't map ERROR_BROKEN_PIPE to
  io::ErrorKind::BrokenPipe, so callers' graceful broken-pipe handling never
  fired.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(seatbelt): orphan-free spawn, inherit tree-kill, small timeouts, …
9d1f91a 
…~ cwd

Five confirmed Seatbelt issues:

- spawn_exec orphaned the just-spawned sandboxed child if wrap_pipe failed
  after the spawn (std::process::Child::drop doesn't kill). Kill and reap the
  child before returning the error.

- Inherit-mode timeout (the run-to-completion / executor path) killed only the
  direct /bin/sh and left descendants running while reporting a timeout. Give
  inherit-with-timeout runs their own process group (setpgid, same session so
  the controlling terminal is kept) so the timeout branch tree-kills the group.
  Limited to timeout-bounded runs since a backgrounded group reading the TTY
  can SIGTTIN-stop. Verified end-to-end via mxc-exec-mac.

- The streaming path rejected timeouts below the 500ms poll interval, refusing
  small timeouts the API exposes. Drop the rejection and clamp each poll sleep
  to the time remaining so sub-interval timeouts are honored precisely.

- Exit-detection latency: the fixed 500ms poll is now 50ms, cutting up to half
  a second of latency on the timeout path.

- resolve_working_directory could hand Command::current_dir a literal `~` cwd;
  run the policy-derived default through profile_builder::expand_tilde first.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(bubblewrap): scope process_group to Pipes mode; tighten timeout poll
08ec195 
- The run-to-completion (Inherit) path called process_group(0)
  unconditionally, placing bwrap in a background process group while it still
  shares the controlling terminal — POSIX job control can SIGTTIN-stop it and
  hang wait(). Restrict process_group(0) to Pipes mode (matching Seatbelt).
  bwrap is pid 1 of the --unshare-pid namespace, so Inherit-mode kill()/timeout
  now tears the sandbox down by killing the root process (namespace teardown)
  instead of a group signal that would hit the executor.

- Exit-detection poll lowered from 500ms to 50ms and each sleep clamped to the
  time remaining, and the "timeout below 500ms" rejection dropped, so small
  timeouts the API exposes are honored precisely (mirrors the Seatbelt fix).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): reject conflicting proxy modes; drop build_request round-trip
c1798f4 
- ProxySpec deserialized through an untagged enum, so an object setting several
  conflicting modes (e.g. { builtinTestServer: true, localhost: 8080 }) silently
  kept the first instead of being rejected like the shared wire-config parser
  does. Parse all recognised modes and require exactly one (also keeps the
  builtinTestServer:false fail-closed rejection).

- build_request serialised the wire config to JSON, base64-encoded it, then
  decoded + re-parsed it — needless work on an in-process path. Add
  config_parser::load_request_from_value (Value -> request, same validation and
  wire->model mapping) and use it, dropping the base64/string round-trip.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
refactor(mxc): private Runner field; idiomatic rustdoc example
32ada5d 
- Runner<B>(pub B) leaked its inner backend even though Runner::new is the
  intended constructor (and the only caller path). Make the field private,
  per the repo's "structs should have private fields" guidance.

- The crate-level rustdoc example used proc.wait().expect(...); switch it to
  proc.wait()? with a Box<dyn Error> tail, matching the README and the repo's
  preference for ? over expect().

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
docs: sync Seatbelt env/cwd behavior and the streaming-mechanism notes
44abe8c 
- Document the Seatbelt backend's unconditional environment clearing (default
  PATH, process.env overrides) and the omitted-process.cwd fallback
  (readwritePaths[0] -> readonlyPaths[0] -> /, tilde-expanded, PWD exported) in
  the Seatbelt guide — both were undocumented.
- Correct the LXC guide's now-stale claim that Seatbelt clears env only when
  process.env is non-empty; Seatbelt now always clears.
- The mxc crate README still grouped Seatbelt with the pty path; only LXC uses
  mxc_pty now, so reword so Seatbelt/Bubblewrap/AppContainer are described as
  inheriting the executor's stdio directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(appcontainer): tree-kill before teardown on the wait() success path
d237530 
The Windows AppContainer and BaseContainer wait() success path ran the stdio
drains and run_teardown() but never tree-killed the job first, so a backgrounded
descendant could keep running after run_teardown() removed the firewall / BFS /
proxy enforcement (Drop's teardown was then a no-op). Both backends now
tree-kill (via kill(), matching each backend's own Drop invariant) and reap the
root before teardown on every path — killing the tree also closes the
descendant's pipe write-ends so the drains finish. WAIT_FAILED and
GetExitCodeProcess failures are surfaced as errors instead of a swallowed Ok(-1)
/ Ok(u32::MAX as -1).

Also aligns the host-filtering test with the cr-003 policy fix: only Linux has a
real host-filtering backend, so macOS/Windows reject host rules without
allowOutbound.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(appcontainer): fail closed when BaseContainer job assignment fails
5a1f947 
The BaseContainer child is created running and was then assigned to a job
object best-effort: on failure `job` was left `None`, after which
`kill()`/timeout/`Drop` could only `TerminateProcess` the root and no
descendant was tree-killed at all -- a sandbox that could not be reliably
contained.

Fail closed instead: if `UiJobObject::new()` or `assign_process` fails,
terminate and reap the just-launched child, run sandbox cleanup (and
`unregister_ctrl_c_cleanup`, matching `run_teardown`) when
`destroy_on_exit`, stop the proxy coordinator, and reject the spawn with a
`LaunchFailed` error rather than run an uncontainable sandbox. `job` is
therefore always present on a spawned child; the `Option` and the
root-only fallback in `kill()` are kept purely as defense-in-depth.

The create API (`Experimental_CreateProcessInSandbox`) does not, on the
builds validated to date, honor a suspended start the way the AppContainer
path does, so the tiny race between the call returning and
`assign_process` completing is documented rather than closed (the child is
a shell that has not yet run the user command, so the window is empty). A
create-suspended path would need verification on an elevated, host-prepped
build, which was not available in this environment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(mxc): scope the PowerShell read-only grant to what pwsh needs
94ef6d1 
When `pwsh.exe` was found on PATH, the policy granted read-only access to
the entire system drive (`C:\`), widening the sandbox's read surface far
past what PowerShell requires.

Replace the whole-drive grant with a scoped read-only set: `$PSHOME` (the
install dir, which carries pwsh.exe, its self-contained .NET runtime, and
the shipped modules), `%WINDIR%\System32`, and the module search roots
(`$PSModulePath`, `%ProgramFiles*%\{PowerShell,WindowsPowerShell}\Modules`,
`$PSHOME\Modules`, and `%USERPROFILE%\Documents\PowerShell\Modules`),
filtered to existing directories. The PSReadLine history dir stays
read-write and unfiltered so pwsh can create it on first use.

The SDK's `getPowerShellPolicy` -- the source the Rust port mirrors -- is
narrowed identically so the two stay in parity, and the unit tests in both
ports are updated to assert the scoped grant and that no whole-drive root
is ever exposed.

Runtime confirmation that pwsh launches under the narrowed policy needs an
elevated, host-prepped AppContainer host and was not possible in this
environment; the policy-construction logic is covered by the new unit
tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@caarlos0
fix(appcontainer): make firewall COM apartment-self-contained
5287c60 
`NetworkManager` cached an STA `INetFwPolicy2` interface and its
`CoInitializeEx` state across the handle's lifetime and reused them at
teardown. The in-process `mxc` library drives `wait()`/`kill()`/teardown
on tokio `spawn_blocking` workers -- potentially a different thread than
the one that initialized COM -- so using the STA interface from another
apartment, and pairing `CoInitialize`/`CoUninitialize` across threads, was
unsound. That is exactly the state the `unsafe impl Send` on the sandbox
handle was asserting over.

Make each firewall operation self-contained instead: a new `ComApartment`
RAII guard does `CoInitializeEx` on creation and `CoUninitialize` on drop,
and both `apply_firewall_rules` and `remove_firewall_rules` open their own
apartment and create a fresh `INetFwPolicy2` on whichever thread runs them.
Windows Firewall rules persist by name, so removal re-acquires a fresh
interface (rules are tracked by name in `created_rule_names`). No COM
interface or apartment state is cached across calls, so nothing
thread-affine is moved across threads and the `unsafe impl Send` SAFETY
comment is now honest -- the only remaining manager state is the
thread-agnostic Winsock refcount.

A small `RuleContext` bundles the per-apply firewall interface and the
principal id so the rule helpers stay within the argument-count lint.

Verified the cross-thread COM mechanics on a Windows host with a throwaway
probe (apply-side enumeration on one thread, fresh-interface re-acquisition
on another); driving the full apply/remove rule lifecycle needs an
elevated, host-prepped AppContainer host, which was not available here.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
This was referenced Jun 19, 2026
Closed
Open
@MGudgin MGudgin mentioned this pull request Jun 19, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@caarlos0