diff --git a/docs/backends/bwrap/bubblewrap-backend.md b/docs/backends/bwrap/bubblewrap-backend.md index 8debd76e9..f1909527c 100644 --- a/docs/backends/bwrap/bubblewrap-backend.md +++ b/docs/backends/bwrap/bubblewrap-backend.md @@ -926,6 +926,21 @@ tests/scripts/run_bwrap_all_tests.sh Test configs are in `tests/configs/bubblewrap_*.json`. +## Troubleshooting + +Failures carry an error code: `policy_validation` (exit 1) when the request +asks for something the backend refuses, `backend_unavailable` (exit -1) when +the host cannot provide the backend at all, and `backend_error` (exit -1) for +setup and runtime faults. + +| Message | Meaning | +|---|---| +| `Bubblewrap (bwrap) is not installed or not on PATH` / version too old | The host has no usable Bubblewrap (`backend_unavailable`); install `bubblewrap` 0.5+ | +| `requires 'slirp4netns' on PATH` / `requires unprivileged user and network namespaces` | The host cannot provide the sandbox network (`backend_unavailable`) | +| `script_code is empty` / `must not carry credentials` / an egress or proxy refusal | The request is at fault (`policy_validation`); change it and retry | +| `failed to spawn bwrap` / `failed to allocate PTY` / proxy namespace start failure | The request was accepted but the launch faulted (`backend_error`) | +| `failed to wrap stdio pipes` | The sandbox started and then faulted (`backend_error`) | + ## Limitations - **Linux only** — Bubblewrap requires Linux kernel namespaces diff --git a/docs/backends/hyperlight/hyperlight-backend.md b/docs/backends/hyperlight/hyperlight-backend.md index ad22aa2db..c64f06789 100644 --- a/docs/backends/hyperlight/hyperlight-backend.md +++ b/docs/backends/hyperlight/hyperlight-backend.md @@ -115,7 +115,7 @@ to the snapshot between calls. | Aspect | Behaviour | |---|---| -| Exit code | The script's: `sys.exit(N)`, `process.exit(N)` or `exit N` gives `N`, an uncaught exception gives `1`, a runner error gives `-1` with the reason in `error_message` | +| Exit code | The script's: `sys.exit(N)`, `process.exit(N)` or `exit N` gives `N`, an uncaught exception gives `1`. A refused request gives `1` and a host or runtime fault gives `-1`, with the reason in `error_message` | | Timeout | `process.timeout` bounds the run; a guest that overruns is interrupted and the run reports `execution timed out` | | stdout / stderr | Inherited by the guest; `ScriptResponse.standard_out` stays empty, so capture at the process level | @@ -155,11 +155,18 @@ contain whitespace, `:` or brackets, and two mounts may not share one. ## Troubleshooting +Failures carry an error code: `policy_validation` (exit 1) when the request +asks for something the backend refuses, `backend_unavailable` (exit -1) when +the host cannot provide the backend at all, and `backend_error` (exit -1) for +setup and runtime faults. + | Message | Meaning | |---|---| -| `no hyperlight image found for the runtime` | No home holds that runtime; run `--setup-hyperlight=`, or set `MXC_HYPERLIGHT_HOME` | +| `hyperlight policy error: …` | The request is at fault (`policy_validation`); change it and retry | +| `no hyperlight image found for the runtime` | No home holds that runtime (`backend_unavailable`); run `--setup-hyperlight=`, or set `MXC_HYPERLIGHT_HOME` | | `holds a rootfs from another hyperlight-unikraft release` | The directory predates the crate the binary was built with; `--setup-hyperlight=` rebuilds it | -| `Hyperlight requires KVM` / `Hyperlight requires Windows Hypervisor Platform` | No hypervisor for this process (error code `backend_unavailable`): on Linux `/dev/kvm` is missing or not readable and writable, on Windows WHP is not enabled | +| `Hyperlight requires KVM` / `Hyperlight requires Windows Hypervisor Platform` | No hypervisor for this process (`backend_unavailable`): on Linux `/dev/kvm` is missing or not readable and writable, on Windows WHP is not enabled | +| `hyperlight setup error: …` / `hyperlight runtime error: …` | The run was accepted but faulted (`backend_error`) | | `execution timed out after N s` | The script overran `process.timeout` | Run with `--debug` to see the home chosen, restore and call timings, and diff --git a/docs/backends/lxc/lxc-backend.md b/docs/backends/lxc/lxc-backend.md index e96360d9e..9399645a3 100644 --- a/docs/backends/lxc/lxc-backend.md +++ b/docs/backends/lxc/lxc-backend.md @@ -305,6 +305,21 @@ streaming in a long-lived service. sudo lxc-checkconfig ``` +## Troubleshooting + +Failures carry an error code: `policy_validation` (exit 1) when the request +asks for something the backend refuses, `backend_unavailable` (exit -1) when +the host cannot provide the backend at all, and `backend_error` (exit -1) for +setup and runtime faults. + +| Message | Meaning | +|---|---| +| `the LXC runtime is not available on this host` | `lxc-ls --version` did not run (`backend_unavailable`); install the LXC tools | +| `sandboxes can only be launched on Linux` | Wrong OS for the backend (`backend_unavailable`) | +| `distribution and release are required` / a proxy, egress or inherited-stdio refusal | The request is at fault (`policy_validation`); change it and retry | +| `Failed to create container` / `Failed to start container` / `container '…' is already in use` | The request was accepted but the launch faulted (`backend_error`) | +| `did not receive an IPv4 address` / `Failed to discover the container init PID` | The container started and then faulted before the script ran (`backend_error`) | + ## Limitations - **The workload can send only ICMP Echo.** A `protocol: "icmp"` rule lets every diff --git a/docs/backends/seatbelt/seatbelt-backend.md b/docs/backends/seatbelt/seatbelt-backend.md index c061f8eb2..6890eeff8 100644 --- a/docs/backends/seatbelt/seatbelt-backend.md +++ b/docs/backends/seatbelt/seatbelt-backend.md @@ -593,7 +593,9 @@ once you can see the rules that were emitted. ### What gets rejected MXC refuses any config it cannot faithfully enforce, rather than quietly -approximating it. This is the complete list. +approximating it. This is the complete list. A refusal carries the +`policy_validation` error code and exit code `1`: nothing ran, and only a +changed config can succeed. One documented exception: `ingress.hostLoopback: "deny"` under `ingress.default: "allow"` is accepted with the container-to-host half enforced diff --git a/docs/backends/wslc/wsl-container-getting-started.md b/docs/backends/wslc/wsl-container-getting-started.md index 05c5894e3..94bb12a3e 100644 --- a/docs/backends/wslc/wsl-container-getting-started.md +++ b/docs/backends/wslc/wsl-container-getting-started.md @@ -326,8 +326,9 @@ request is validated by the same native engine as executor requests. Notes and limits: -- **Windows only.** Selecting `Containment::Wslc` on Linux/macOS, or without the - `wslc` feature, fails with `ErrorCode::UnsupportedContainment`. +- **Windows only.** Selecting `Containment::Wslc` on Linux/macOS fails with + `ErrorCode::UnsupportedContainment`; on Windows without the `wslc` feature it + fails with `ErrorCode::BackendUnavailable`. - **No stdin.** The WSLC SDK exposes no process-input API, so `Sandbox::take_stdin()` returns `None` for a WSL container. - **No host pid.** The process runs inside the WSL VM, so `Sandbox::id()` is `0`; diff --git a/docs/development/architecture/backends/isolation-session/oneshot.md b/docs/development/architecture/backends/isolation-session/oneshot.md index 7cdbbb4ca..dab34c2e5 100644 --- a/docs/development/architecture/backends/isolation-session/oneshot.md +++ b/docs/development/architecture/backends/isolation-session/oneshot.md @@ -255,11 +255,10 @@ the rationale for each disposition, and the error mapping live in | `isolationSession` / one-shot `appId` | rejected as `malformed_request` — IsolationSession one-shot configuration uses only the stable top-level policy | | `processContainer` / `lxc` / `seatbelt` / another backend's section | rejected — only the section matching `containment` is accepted | -Refusals surface as a non-zero exit with the reason on stderr. One-shot has no -typed policy error code: the envelope carries `error.code = "backend_error"` with -the reason in the message. On the state-aware surface, structurally -representable backend policy failures emit `policy_validation`; fields excluded -by an exact phase root fail earlier as `malformed_request`. +Refusals surface as exit `1` with a `policy_validation` error envelope on +stderr naming the reason. An unreachable IsolationSession service reports +`backend_unavailable`. On the state-aware surface, fields excluded by an exact +phase root fail earlier as `malformed_request`. **Why every supplied `ui` is refused.** The `ui` section states intent about the contained code's relationship to the *user's* environment, and was modelled on a diff --git a/docs/development/architecture/backends/isolation-session/state-aware-rust.md b/docs/development/architecture/backends/isolation-session/state-aware-rust.md index f7ac7d06c..c175d05f6 100644 --- a/docs/development/architecture/backends/isolation-session/state-aware-rust.md +++ b/docs/development/architecture/backends/isolation-session/state-aware-rust.md @@ -347,10 +347,13 @@ as `malformed_request`. Requests that pass the exact structural contract but violate a backend semantic invariant surface as `policy_validation`; a structurally valid but oversized `appId` is one such case. -On the **one-shot** surface the backend's typed policy variant is discarded -(`ScriptResponse::error`) and the envelope carries `error.code = -"backend_error"` with the reason in the message. A supplied `network.proxy` is -also structurally refused as `malformed_request`. +On the **one-shot** surface the backend's typed policy variant becomes +`ScriptResponse::rejected`, so the envelope carries `error.code = +"policy_validation"` and exits 1, matching the state-aware mapping. A supplied +`network.proxy` is also structurally refused as `malformed_request`. An +activation failure of the runtime API becomes `ScriptResponse::unavailable` +(`backend_unavailable`, exit -1); every other lifecycle failure stays +`backend_error`. ## Mode-specific fields @@ -502,13 +505,11 @@ State-aware exec (and other phases) use OS-level cancellation in v1: - On the `wxc-exec` route, cancellation is process termination. - The agent process's pipes EOF, the relay threads exit. - The OS-side service's per-process timer (set from - `process.timeout`) reaps the agent if the runner does not. On the - streaming path that timer is armed with a **margin** past the caller's - deadline, so it acts purely as a watchdog: the service kills with an - ordinary exit code (the host suite pins it as exit code 1), so if it - fired first a genuine timeout would be indistinguishable from a normal - exit and could not be reported as one. The run-to-completion path arms - it unchanged — it has no timeout channel to report through. + `process.timeout`) reaps the agent if the runner does not. That timer is + armed with a **margin** past the caller's deadline, so it acts purely as + a watchdog: the service kills with an ordinary exit code, so if it fired + first a genuine timeout would be indistinguishable from a normal exit + and could not be reported as one. - The runner's existing 3-tier shutdown (`CloseStandardInput` → `SendCtrlClose` → `Terminate`) handles the timeout case from inside the agent process before returning. @@ -556,7 +557,8 @@ An exited process is never routed through the shutdown ladder, which reads only and so cannot tell a `259` exit from a live process. The adapter maps `TimedOut` onto `ErrorKind::TimedOut`, which is what `mxc_sdk::v1::MxcProcess::wait` reads as `WaitResult::TimedOut`. That outcome -is reachable only under `Piped`; the `Relayed` arm reports `Exited`. +is reachable only under `Piped`; the `Relayed` arm reports a timeout as a +`backend_error` instead. Teardown is bounded only insofar as the kill is: the streaming adapter's `Drop` joins the waiter when the kill was accepted, and abandons the diff --git a/docs/development/architecture/container-lifecycle.md b/docs/development/architecture/container-lifecycle.md index 25dc0cee9..75d9a16b3 100644 --- a/docs/development/architecture/container-lifecycle.md +++ b/docs/development/architecture/container-lifecycle.md @@ -612,9 +612,9 @@ diagnostic. The rejection is recorded through the same `ConfigRejected` audit pa Diagnostics are never duplicated to stdout, and lifecycle stderr is not gated on `--debug`. -Without `--operation`, `wxc-exec` accepts only one-shot requests. Input-source and -pre-parse failures on that path retain the legacy primary diagnostic on stderr and do -not emit a lifecycle envelope. Raw SDK and FFI lifecycle calls do not use this CLI +Without `--operation`, `wxc-exec` accepts only create-and-run requests. Input-source +and pre-parse failures on that path emit a typed `{error}` envelope on stderr rather +than the lifecycle envelope on stdout. Raw SDK and FFI lifecycle calls do not use this CLI stream protocol; they return their status and error data through their binding result surfaces. @@ -667,7 +667,10 @@ because a script's stderr may not end in a newline. `code` and `message` are always present. `code` is the machine-readable category a consumer branches on; `message` is the human-readable description, and for a failure raised by an underlying platform API it is that API's own message, passed through -verbatim rather than concatenated with the other fields. +verbatim rather than concatenated with the other fields. The executor's own exit +code follows `code`: a refused request exits `1`, and an unavailable host or a +runtime fault exits `-1`. See the error contract in +[versioning.md](versioning.md#error-contract). The three optional named fields carry structured failure detail. `operation` and `nativeCode` describe an underlying platform call; `remediation` describes the failure: diff --git a/docs/development/architecture/versioning.md b/docs/development/architecture/versioning.md index 2db7a64b4..681ad991a 100644 --- a/docs/development/architecture/versioning.md +++ b/docs/development/architecture/versioning.md @@ -708,6 +708,47 @@ Negotiation failures are **typed and actionable** — never a silent fallback: unsupported field. Security policy is deterministic — no relaxation, no fuzzy fallback. +Every executor failure is classified by **which side can act on it**, and that +classification drives both the wire code and the process exit code: + +| Side at fault | Failure phase | Wire code | Exit code | +|---|---|---|---| +| The request — a different request succeeds | `Rejected` | `policy_validation` | `1` | +| The host — no request change helps | `BackendUnavailable` | `backend_unavailable` | `-1` | +| The runtime, during an accepted run | all other phases | `backend_error` | `-1` | + +A backend this build did not compile in reports `backend_unavailable`, since +only a rebuilt executor can serve it. On the create-and-run path, a backend +belonging to another operating system reports `unsupported_containment`, since +naming a supported backend succeeds; the container lifecycle operations report +such a backend as `backend_unavailable`. Within a backend, a field the selected +tier cannot represent (for example `filesystem.enumeratePaths` or +`network.ingress.hostLoopback` on an AppContainer fallback), or a fallback the +request forbids (`fallback.allowDaclMutation: false` when only the DACL tier is +usable), is a `policy_validation` rejection naming that field, never a silent +drop; but a tier that could not be built at all, or an absent denial-capture +provider, is `backend_unavailable`, because the policy was accepted and never +judged. An experimental backend named without the experimental opt-in is also +`backend_unavailable`: the opt-in is an executor invocation mode rather than a +policy field, so no change to the authored request reaches it. Earlier +phase-specific codes (`malformed_request`, `unsupported_containment`, +`unsupported_phase`, `malformed_id`) are unchanged and also exit `1`. + +Envelope lines share stderr with the workload's own output, so a workload can +print text that looks like an envelope. Treat stderr envelopes as diagnostics, +not as an authenticated result; the exit code and a held `ScriptResponse` are +authoritative. + +`ScriptResponse` carries an optional `warnings` array of diagnostic strings. +Warnings are advisory and never change the exit code; they are omitted from the +wire entirely when a run produced none. The serialized response and the +executor's stderr stream are two distinct shapes of the same warnings: on the +response they are a `warnings: string[]` field, while an executor writes each +one to stderr as its own line-delimited JSON object, +`{"warning":{"message":"..."}}`, matching the error envelope +`{"error":{"code":"...","message":"..."}}`. A caller reading stderr must parse +the per-line objects; the array exists only on a response it holds directly. + The MXC ↔ OS contract therefore reports: which feature failed, whether it was a version mismatch or a runtime/capability unavailability (e.g. Hyper-V off), and what the user should do (upgrade OS, enable feature, change the config). diff --git a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs index 2e5df33d0..4cbac1d8e 100644 --- a/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs +++ b/src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs @@ -25,7 +25,7 @@ use std::time::{Duration, Instant}; use crate::mxc_common::filesystem_symlink::resolve_through_symlinks; use crate::mxc_common::interruptible_reader::{wrap_pipe, InterruptibleReader, ReadCanceller}; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ExecutionRequest, ScriptResponse}; +use crate::mxc_common::models::{ExecutionRequest, FailurePhase, ScriptResponse}; use crate::mxc_common::sandbox_process::{ boxed_closer, cancel_and_join_discard, duplicate_and_take_native_stdio, group_kill, spawn_discard, take_boxed_read, take_boxed_write, wait_with_timeout, NativeStdio, PtySize, @@ -45,6 +45,16 @@ use crate::bwrap_common::{ const DNS_PORT: u16 = 53; +/// A host lookup failure is a launch fault, not a refusal of the policy. +fn proxy_resolve_failure_response(error: proxy_network::ProxyResolveError) -> ScriptResponse { + match error { + proxy_network::ProxyResolveError::Rejected(message) => ScriptResponse::rejected(&message), + proxy_network::ProxyResolveError::Lookup(message) => { + ScriptResponse::failed(FailurePhase::LaunchFailed, &message) + } + } +} + /// Bubblewrap sandbox runner. Uses only shared `ContainerPolicy` fields — /// no backend-specific config struct required. #[derive(Default)] @@ -114,13 +124,13 @@ impl SandboxBackend for BubblewrapScriptRunner { &normalized } Ok(None) => request, - Err(msg) => return Err(ScriptResponse::error(&msg)), + Err(error) => return Err(error.into_response()), }; // Reject any policy path the invoking user cannot access, so the sandbox // never gains access the caller lacks. Runs after object normalization // so it sees the already-tightened intents. if let Err(msg) = crate::mxc_common::filesystem_access::check_delegation(&request.policy) { - return Err(ScriptResponse::error(&msg)); + return Err(ScriptResponse::rejected(&msg)); } // Resolve denied paths that traverse a symlink to their real host path // and classify each as a file/dir mask (see [`resolve_denied_paths`]). @@ -128,7 +138,7 @@ impl SandboxBackend for BubblewrapScriptRunner { // none). See docs/backends/bwrap/bubblewrap-backend.md. let plan = match resolve_denied_paths(&request.policy, logger) { Ok(plan) => plan, - Err(msg) => return Err(ScriptResponse::error(&msg)), + Err(msg) => return Err(ScriptResponse::rejected(&msg)), }; let resolved; let request = match plan.paths { @@ -146,7 +156,7 @@ impl SandboxBackend for BubblewrapScriptRunner { // The masks are built from the list above, not from the one the caller // wrote, so the pin conflict has to be judged against it too. if let Err(msg) = check_pin_against_denied_hosts(request) { - return Err(ScriptResponse::error(&msg)); + return Err(ScriptResponse::rejected(&msg)); } warn_resolver_mount_hint(request, logger); warn_cwd_mount_hint(request, logger); @@ -189,7 +199,7 @@ impl BubblewrapScriptRunner { // probe so config errors are reported deterministically even on // hosts without bwrap installed. if request.script_code.is_empty() { - return Err(ScriptResponse::error( + return Err(ScriptResponse::rejected( "script_code is empty — nothing to execute.", )); } @@ -217,7 +227,7 @@ impl BubblewrapScriptRunner { if crate::mxc_common::proxy_env::proxy_url_has_credentials(&url) { // Built from the redacted form so the rejection cannot become // the leak it is rejecting. - return Err(ScriptResponse::error(&format!( + return Err(ScriptResponse::rejected(&format!( "Bubblewrap: runtimeConfig.networkProxy must not carry credentials ('{}'). \ Bubblewrap passes the proxy URL to bwrap as a --setenv command-line \ argument, and process arguments are world-readable through \ @@ -233,10 +243,10 @@ impl BubblewrapScriptRunner { // Programmatic requests also need the parser's proxy-only restriction: // direct egress must not be accepted and then discarded at launch. if let Some(reason) = bwrap_command::proxy_with_egress_rejection(request) { - return Err(ScriptResponse::error(reason)); + return Err(ScriptResponse::rejected(reason)); } if let Some(reason) = bwrap_command::directional_network_rejection(request) { - return Err(ScriptResponse::error(reason)); + return Err(ScriptResponse::rejected(reason)); } // Proxy-only networking derives the sandbox-visible endpoint from the @@ -258,12 +268,12 @@ impl BubblewrapScriptRunner { if proxy_only { if let Some(address) = request.policy.network_proxy.address.as_ref() { if let Err(error) = proxy_network::SandboxProxy::check_without_resolving(address) { - return Err(ScriptResponse::error(&error)); + return Err(ScriptResponse::rejected(&error)); } if let Err(error) = proxy_network::check_hosts_pin_against_policy(address, &request.policy) { - return Err(ScriptResponse::error(&error)); + return Err(ScriptResponse::rejected(&error)); } // Repeated in `spawn` against the normalized policy. } @@ -277,7 +287,7 @@ impl BubblewrapScriptRunner { let egress_plan = if firewall_enforced { match network_rules::EgressPlan::for_request(request) { Ok(plan) => Some(plan), - Err(error) => return Err(ScriptResponse::error(&error)), + Err(error) => return Err(ScriptResponse::rejected(&error)), } } else { None @@ -287,7 +297,7 @@ impl BubblewrapScriptRunner { // builder emits — an old binary would otherwise fail at spawn time with // an opaque "unknown option" error. if let Err(err) = probe() { - return Err(ScriptResponse::error(&err.to_string())); + return Err(ScriptResponse::unavailable(&err.to_string())); } if proxy_only || firewall_enforced { // Proxy and firewall are mutually exclusive, so this names the one @@ -298,7 +308,7 @@ impl BubblewrapScriptRunner { proxy_network::PrivateNetworkUse::FirewallEnforcement }; if let Err(error) = proxy_network::probe_dependencies(use_case) { - return Err(ScriptResponse::error(&error)); + return Err(ScriptResponse::unavailable(&error)); } } @@ -457,7 +467,7 @@ impl BubblewrapScriptRunner { })?; Some( proxy_network::SandboxProxy::resolve(address) - .map_err(|error| ScriptResponse::error(&error))?, + .map_err(proxy_resolve_failure_response)?, ) } else { None @@ -485,7 +495,9 @@ impl BubblewrapScriptRunner { request.script_timeout, ) { Ok(network) => Some(network), - Err(error) => return Err(ScriptResponse::error(&error)), + Err(error) => { + return Err(ScriptResponse::failed(FailurePhase::LaunchFailed, &error)) + } } } // Direct directional egress uses the same private namespace. @@ -498,7 +510,7 @@ impl BubblewrapScriptRunner { Some(plan) => plan, None => match network_rules::EgressPlan::for_request(request) { Ok(plan) => plan, - Err(error) => return Err(ScriptResponse::error(&error)), + Err(error) => return Err(ScriptResponse::rejected(&error)), }, }; let resolver = proxy_network::ResolverPin::for_host(&request.policy, logger); @@ -514,7 +526,9 @@ impl BubblewrapScriptRunner { warn_resolver_blocked_by_egress(&plan, network.resolver_pinned(), logger); Some(network) } - Err(error) => return Err(ScriptResponse::error(&error)), + Err(error) => { + return Err(ScriptResponse::failed(FailurePhase::LaunchFailed, &error)) + } } } None => None, @@ -534,7 +548,7 @@ impl BubblewrapScriptRunner { Ok(startup) => Some(startup), Err(error) => { stop_proxy_network(&mut proxy_network, logger); - return Err(ScriptResponse::error(&error)); + return Err(ScriptResponse::failed(FailurePhase::LaunchFailed, &error)); } }, None => None, @@ -578,9 +592,10 @@ impl BubblewrapScriptRunner { Ok(pty) => Some(pty), Err(error) => { stop_proxy_network(&mut proxy_network, logger); - return Err(ScriptResponse::error(&format!( - "Bubblewrap: failed to allocate PTY: {error}" - ))); + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + &format!("Bubblewrap: failed to allocate PTY: {error}"), + )); } }, }; @@ -600,10 +615,10 @@ impl BubblewrapScriptRunner { Ok(process) => process, Err(error) => { stop_proxy_network(&mut proxy_network, logger); - return Err(ScriptResponse::error(&format!( - "Bubblewrap: failed to spawn bwrap: {}", - error - ))); + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + &format!("Bubblewrap: failed to spawn bwrap: {}", error), + )); } }; @@ -626,7 +641,10 @@ impl BubblewrapScriptRunner { let _ = child.kill(); let _ = child.wait(); stop_proxy_network(&mut proxy_network, logger); - return Err(ScriptResponse::error(&error)); + return Err(ScriptResponse::failed( + FailurePhase::PostLaunchFailed, + &error, + )); } } @@ -649,10 +667,13 @@ impl BubblewrapScriptRunner { let _ = child.wait(); stop_proxy_network(&mut proxy_network, logger); let error = out_result.err().or(err_result.err()); - return Err(ScriptResponse::error(&format!( - "Bubblewrap: failed to wrap stdio pipes: {}", - error.map_or_else(|| "unknown error".to_string(), |e| e.to_string()), - ))); + return Err(ScriptResponse::failed( + FailurePhase::PostLaunchFailed, + &format!( + "Bubblewrap: failed to wrap stdio pipes: {}", + error.map_or_else(|| "unknown error".to_string(), |e| e.to_string()), + ), + )); } }; let timeout = if request.script_timeout == 0 { @@ -2128,6 +2149,88 @@ mod tests { } } + /// A host without a usable bwrap is not a request the caller can rewrite, so + /// it must not be reported as a policy problem. + #[test] + fn an_unusable_bwrap_is_blamed_on_the_host() { + let request = base_request(); + let failures = [ + bwrap_version::BwrapUnavailable::NotFound, + bwrap_version::BwrapUnavailable::ProbeFailed { + status: Some(126), + detail: "permission denied".to_string(), + }, + bwrap_version::BwrapUnavailable::UnrecognizedVersion("junk".to_string()), + bwrap_version::BwrapUnavailable::TooOld(bwrap_version::BwrapVersion::new(0, 4, 1)), + ]; + + for failure in failures { + let detail = failure.to_string(); + let error = BubblewrapScriptRunner::new() + .validate_prepared_with_probe(&request, || Err(failure)) + .unwrap_err(); + + assert_eq!( + error.failure_phase, + FailurePhase::BackendUnavailable, + "input={detail}; output={error:?}" + ); + assert_eq!(error.exit_code, -1, "input={detail}; output={error:?}"); + } + } + + #[test] + fn a_failed_proxy_lookup_is_a_launch_fault_not_a_refusal() { + use crate::mxc_common::mxc_error::MxcErrorCode; + use proxy_network::ProxyResolveError; + + let lookup = + proxy_resolve_failure_response(ProxyResolveError::Lookup("dns down".to_string())); + assert_eq!(lookup.failure_phase, FailurePhase::LaunchFailed); + assert_eq!( + lookup.failure_phase.error_code(), + MxcErrorCode::BackendError + ); + assert_eq!(lookup.exit_code, -1); + + let refused = + proxy_resolve_failure_response(ProxyResolveError::Rejected("ipv6".to_string())); + assert_eq!(refused.failure_phase, FailurePhase::Rejected); + assert_eq!(refused.exit_code, 1); + } + + #[test] + fn a_policy_this_backend_will_not_run_is_blamed_on_the_request() { + let runner = BubblewrapScriptRunner::new(); + let empty_script = ExecutionRequest { + script_code: String::new(), + ..base_request() + }; + + for (what, request) in [ + ("empty script_code", empty_script), + ( + "proxy credentials", + request_with_a_credential_bearing_proxy(), + ), + ] { + let refusal = runner + .validate(&request) + .err() + .unwrap_or_else(|| panic!("input={what}; expected a refusal")); + + assert_eq!( + refusal.failure_phase, + FailurePhase::Rejected, + "input={what}; output={refusal:?}" + ); + assert_eq!( + refusal.exit_code, 1, + "input={what}; a refusal exits 1; output={refusal:?}" + ); + } + } + /// A denied symlink pointing at a **directory** is rewritten to its canonical /// target so the mask lands on the real directory (bwrap cannot mount a mask /// over a symlink whose parent is bound). The resolved directory is diff --git a/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs b/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs index 54798332e..df909036e 100644 --- a/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs +++ b/src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs @@ -289,6 +289,35 @@ pub(crate) struct SandboxProxy { egress: ProxyEgress, } +/// Why [`SandboxProxy::resolve`] failed. +#[derive(Debug)] +pub(crate) enum ProxyResolveError { + /// The endpoint, or the address it resolves to, cannot be enforced. + Rejected(String), + /// The host lookup itself did not produce an answer. + Lookup(String), +} + +impl ProxyResolveError { + pub(crate) fn message(&self) -> &str { + match self { + Self::Rejected(message) | Self::Lookup(message) => message, + } + } +} + +impl std::fmt::Display for ProxyResolveError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.message()) + } +} + +impl From for ProxyResolveError { + fn from(message: String) -> Self { + Self::Rejected(message) + } +} + impl SandboxProxy { /// Derive the sandbox-visible proxy from the configured one. /// @@ -296,7 +325,7 @@ impl SandboxProxy { /// an address: the workload then presents the `Host` header and proxy-auth /// realm the operator configured. An IP literal has no name to pin, so a /// loopback literal is rewritten to slirp's gateway instead. - pub(crate) fn resolve(configured: &ProxyAddress) -> Result { + pub(crate) fn resolve(configured: &ProxyAddress) -> Result { Self::resolve_with(configured, resolve_ipv4) } @@ -311,7 +340,7 @@ impl SandboxProxy { if needs_lookup { return Ok(()); } - checked + checked.map_err(|error| error.message().to_string()) } /// Run the static checks, reporting whether a lookup would have followed. @@ -320,11 +349,13 @@ impl SandboxProxy { /// also exactly when a hosts pin is created -- an IP literal resolves to /// `Ok(None)` from [`ProxyAddress::host_pin`]. Callers use the flag to /// reason about the pin without duplicating that classification. - fn inspect_without_resolving(configured: &ProxyAddress) -> (bool, Result<(), String>) { + fn inspect_without_resolving( + configured: &ProxyAddress, + ) -> (bool, Result<(), ProxyResolveError>) { let asked_to_resolve = Cell::new(false); let checked = Self::resolve_with(configured, |_, _| { asked_to_resolve.set(true); - Err(String::new()) + Err(ProxyResolveError::Lookup(String::new())) }); (asked_to_resolve.get(), checked.map(|_| ())) @@ -337,10 +368,14 @@ impl SandboxProxy { /// a lookup whose answer the test does not control. fn resolve_with( configured: &ProxyAddress, - resolve: impl Fn(&str, u16) -> Result, - ) -> Result { + resolve: impl Fn(&str, u16) -> Result, + ) -> Result { if configured.port() == 0 { - return Err("Bubblewrap: proxy-only egress requires a non-zero proxy port".to_string()); + return Err( + "Bubblewrap: proxy-only egress requires a non-zero proxy port" + .to_string() + .into(), + ); } let host = configured.host().trim_matches(['[', ']']); @@ -376,7 +411,7 @@ impl SandboxProxy { // an address nothing answers on -- failing at connect time rather // than at policy time. if ip.is_unspecified() { - return Self::rewritten(configured); + return Ok(Self::rewritten(configured)?); } if ip.is_loopback() { return Err(format!( @@ -384,13 +419,14 @@ impl SandboxProxy { network namespace cannot reach; bind the proxy to 127.0.0.1 or a dual-stack \ wildcard address instead. The egress rule is emitted with IPv4 iptables.", configured.host() - )); + ) + .into()); } - return Err(ipv6_unsupported(host)); + return Err(ipv6_unsupported(host).into()); } if is_reserved_loopback_name { - return Self::rewritten(configured); + return Ok(Self::rewritten(configured)?); } let resolved = resolve(host, configured.port())?; @@ -515,27 +551,29 @@ fn with_deadline( /// `getaddrinfo` cannot be cancelled, so the lookup is moved to a thread and /// abandoned once the bound expires -- the wait is bounded even though the /// query itself keeps running until the resolver gives up. -fn resolve_ipv4(host: &str, port: u16) -> Result { +fn resolve_ipv4(host: &str, port: u16) -> Result { let query = host.to_string(); let waited = with_deadline(RESOLVE_TIMEOUT_LABEL, RESOLVE_TIMEOUT, move || { lookup_ipv4(&query, port) }) .map_err(|error| { - format!("Bubblewrap: could not start the lookup of proxy host '{host}': {error}") + ProxyResolveError::Lookup(format!( + "Bubblewrap: could not start the lookup of proxy host '{host}': {error}" + )) })?; match waited { Ok(resolved) => resolved, - Err(mpsc::RecvTimeoutError::Timeout) => Err(format!( + Err(mpsc::RecvTimeoutError::Timeout) => Err(ProxyResolveError::Lookup(format!( "Bubblewrap: resolving proxy host '{host}' exceeded {}s. The proxy endpoint is \ resolved on the host because DNS is closed inside the sandbox, so an unresponsive \ host resolver blocks the sandbox from starting. Check the host's DNS configuration, \ or give the proxy as an IPv4 address to skip resolution.", RESOLVE_TIMEOUT.as_secs() - )), - Err(mpsc::RecvTimeoutError::Disconnected) => Err(format!( + ))), + Err(mpsc::RecvTimeoutError::Disconnected) => Err(ProxyResolveError::Lookup(format!( "Bubblewrap: the lookup of proxy host '{host}' ended without an answer." - )), + ))), } } @@ -544,14 +582,14 @@ fn resolve_ipv4(host: &str, port: u16) -> Result { /// Only the first IPv4 answer is used, and it is the same one the sandbox is /// pinned to. Opening the rest would widen the chain to addresses the sandbox /// can no longer select: with DNS closed, the pin is its only resolution path. -fn lookup_ipv4(host: &str, port: u16) -> Result { +fn lookup_ipv4(host: &str, port: u16) -> Result { let resolved: Vec = (host, port) .to_socket_addrs() .map_err(|error| { - format!( + ProxyResolveError::Lookup(format!( "Bubblewrap: could not resolve proxy host '{host}': {error}. The proxy endpoint \ is resolved on the host because DNS is closed inside the sandbox." - ) + )) })? .map(|addr| addr.ip()) .collect(); @@ -563,11 +601,11 @@ fn lookup_ipv4(host: &str, port: u16) -> Result { // A name with AAAA records and no A records is the same unenforceable case // as an IPv6 literal, so say so rather than claiming it does not resolve. if resolved.is_empty() { - Err(format!( + Err(ProxyResolveError::Lookup(format!( "Bubblewrap: proxy host '{host}' did not resolve to any address." - )) + ))) } else { - Err(ipv6_unsupported(host)) + Err(ipv6_unsupported(host).into()) } } @@ -2510,6 +2548,9 @@ mod tests { let error = SandboxProxy::resolve_with(&address, resolver([10, 0, 2, 2])) .expect_err("the slirp gateway must not be pinned as if it were a host address"); + let ProxyResolveError::Rejected(error) = error else { + panic!("a reserved answer is a policy refusal: {error}"); + }; assert!( error.contains("10.0.2.0/24") && error.contains("127.0.0.1"), "error should name the reserved range and the supported alternative: {error}" @@ -2583,7 +2624,7 @@ mod tests { /// A resolver that answers every name with `ip`, so pin decisions can be /// tested without a lookup the test does not control. - fn resolver(ip: [u8; 4]) -> impl Fn(&str, u16) -> Result { + fn resolver(ip: [u8; 4]) -> impl Fn(&str, u16) -> Result { move |_, _| Ok(Ipv4Addr::from(ip)) } @@ -2725,7 +2766,7 @@ mod tests { let loopback = ProxyAddress::new("[::1]".into(), 8080); let error = SandboxProxy::resolve(&loopback).unwrap_err(); assert!( - error.contains("IPv4"), + error.message().contains("IPv4"), "error should explain the IPv4 requirement: {error}" ); @@ -2745,12 +2786,39 @@ mod tests { ); let error = SandboxProxy::resolve(&address).unwrap_err(); + // A lookup failure is a host fault, not a refusal of the policy. + let ProxyResolveError::Lookup(error) = error else { + panic!("an unresolvable name is a lookup failure: {error}"); + }; assert!( error.contains("proxy.corp.invalid"), "error should name the offending host: {error}" ); } + #[test] + fn a_failed_lookup_is_not_reported_as_a_refusal() { + let address = ProxyAddress::new("proxy.example".into(), 3128); + let error = SandboxProxy::resolve_with(&address, |_, _| { + Err(ProxyResolveError::Lookup("timed out".into())) + }) + .unwrap_err(); + + assert!(matches!(error, ProxyResolveError::Lookup(_)), "{error}"); + } + + #[test] + fn static_rejections_are_refusals() { + for host in ["[::1]", "2001:db8::1", "10.0.2.2"] { + let address = ProxyAddress::new(host.into(), 8080); + let error = SandboxProxy::resolve(&address).unwrap_err(); + assert!( + matches!(error, ProxyResolveError::Rejected(_)), + "{host}: {error}" + ); + } + } + /// `::1` used to be rewritten to the IPv4 gateway, which handed the sandbox /// an address the proxy never listens on: an IPv6-loopback listener does /// not accept the IPv4 connection slirp's gateway produces. Rejecting at @@ -2762,7 +2830,7 @@ mod tests { let error = SandboxProxy::resolve(&address).unwrap_err(); assert!( - error.contains("IPv6 loopback"), + error.message().contains("IPv6 loopback"), "error should name the unreachable address family: {error}" ); } @@ -2784,7 +2852,7 @@ mod tests { let error = SandboxProxy::resolve(&address).unwrap_err(); assert!( - error.contains("non-zero"), + error.message().contains("non-zero"), "error should explain the port requirement: {error}" ); } diff --git a/src/mxc-sdk/src/backends/hyperlight/common/mod.rs b/src/mxc-sdk/src/backends/hyperlight/common/mod.rs index 133f2dd22..7c5ab84d6 100644 --- a/src/mxc-sdk/src/backends/hyperlight/common/mod.rs +++ b/src/mxc-sdk/src/backends/hyperlight/common/mod.rs @@ -89,9 +89,10 @@ //! ## Exit codes //! //! Guest exit code on clean completion (0 for normal exit, non-zero for -//! `sys.exit(N)` or unhandled exceptions), -1 on any runner error -//! (preflight, install, runtime, guest crash, timeout). The specific -//! failure mode is in `error_message`. +//! `sys.exit(N)` or unhandled exceptions). A refused request exits 1; a +//! host that cannot provide the backend, a failed install or boot, a guest +//! crash and a timeout all exit -1. The specific failure mode is in +//! `error_message`, and `failure_phase` says which side is at fault. use std::path::{Path, PathBuf}; use std::sync::mpsc::{self, RecvTimeoutError}; @@ -99,7 +100,9 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ExecutionRequest, HyperlightRuntime, ScriptResponse}; +use crate::mxc_common::models::{ + ExecutionRequest, FailurePhase, HyperlightRuntime, ScriptResponse, +}; use crate::mxc_common::script_runner::ScriptRunner; use crate::mxc_common::validator::{ validate_common, validate_network_policy_support, NetworkPolicySupport, @@ -137,32 +140,53 @@ pub fn is_whp_available() -> bool { #[derive(Debug)] enum RunnerError { - /// Pre-spawn validation failures (missing image, unsupported policy). - Preflight(String), - /// Runtime construction, install, or execution failure. - Runtime(String), + /// The request asks for something the guest cannot do. + Policy(String), + /// This host cannot provide the backend: no usable guest image for the + /// requested runtime, and nothing to install one from. + Unavailable(String), + /// Host-side preparation of a run failed (mount staging, path resolution). + Setup(String), + /// Boot, install, or snapshot bring-up failed. + Launch(String), + /// The guest failed while running the call. + Exec(String), + /// The call outlived `scriptTimeout` and was cut short. + Timeout(String), } impl std::fmt::Display for RunnerError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - RunnerError::Preflight(msg) => write!(f, "hyperlight preflight error: {msg}"), - RunnerError::Runtime(msg) => write!(f, "hyperlight runtime error: {msg}"), + RunnerError::Policy(msg) => write!(f, "hyperlight policy error: {msg}"), + RunnerError::Unavailable(msg) => { + write!(f, "hyperlight is not available on this host: {msg}") + } + RunnerError::Setup(msg) => write!(f, "hyperlight setup error: {msg}"), + RunnerError::Launch(msg) | RunnerError::Exec(msg) | RunnerError::Timeout(msg) => { + write!(f, "hyperlight runtime error: {msg}") + } } } } impl RunnerError { - fn to_response(&self) -> ScriptResponse { - ScriptResponse { - exit_code: ERROR_EXIT_CODE, - error_message: self.to_string(), - ..Default::default() + /// Which side a failure is attributed to: the request, the host, or the + /// run itself. + fn failure_phase(&self) -> FailurePhase { + match self { + RunnerError::Policy(_) => FailurePhase::Rejected, + RunnerError::Unavailable(_) => FailurePhase::BackendUnavailable, + RunnerError::Setup(_) | RunnerError::Launch(_) => FailurePhase::LaunchFailed, + RunnerError::Exec(_) => FailurePhase::PostLaunchFailed, + RunnerError::Timeout(_) => FailurePhase::Timeout, } } -} -const ERROR_EXIT_CODE: i32 = -1; + fn to_response(&self) -> ScriptResponse { + ScriptResponse::failed(self.failure_phase(), &self.to_string()) + } +} /// Env var override for the Hyperlight image home. Set this to force a /// specific location; otherwise the runner uses a standard OS-local @@ -406,7 +430,7 @@ impl HyperlightScriptRunner { default.join(name) ), }; - Err(RunnerError::Preflight(format!( + Err(RunnerError::Unavailable(format!( "no hyperlight image found for the {name} runtime. searched ${HOME_ENV}, \ {default:?}, /{EXE_RELATIVE_HOME}/, /{CWD_RELATIVE_HOME}/, each under \ `{name}/`. {hint}" @@ -444,10 +468,10 @@ impl HyperlightScriptRunner { /// Reject policies the guest cannot enforce before any sandbox is booted. fn validate_policies(request: &ExecutionRequest) -> Result<(), RunnerError> { if request.policy.network_proxy.is_enabled() { - return Err(RunnerError::Preflight(ERR_PROXY_POLICY.to_string())); + return Err(RunnerError::Policy(ERR_PROXY_POLICY.to_string())); } if !request.working_directory.is_empty() { - return Err(RunnerError::Preflight(ERR_WORKDIR.to_string())); + return Err(RunnerError::Policy(ERR_WORKDIR.to_string())); } // Denied paths: block early if any appears in the allow lists. for denied in &request.policy.denied_paths { @@ -459,7 +483,7 @@ impl HyperlightScriptRunner { { match path_relationship(allowed, denied) { PathRelationship::Overlaps => { - return Err(RunnerError::Preflight(format!( + return Err(RunnerError::Policy(format!( "deniedPaths entry {denied:?} overlaps allow list path {allowed:?}; \ Hyperlight cannot safely expose overlapping allowed and denied \ host paths" @@ -467,7 +491,7 @@ impl HyperlightScriptRunner { } PathRelationship::Separate => {} PathRelationship::Indeterminate => { - return Err(RunnerError::Preflight(format!( + return Err(RunnerError::Policy(format!( "Hyperlight: cannot verify deniedPaths against allow lists because \ {allowed:?} or {denied:?} could not be resolved to a canonical \ location. With deniedPaths present, MXC fails closed rather than \ @@ -511,13 +535,13 @@ impl HyperlightScriptRunner { .map(|p| p.as_os_str().is_empty() || p.exists()) .unwrap_or(false); if !parent_ok { - return Err(RunnerError::Preflight(format!( + return Err(RunnerError::Policy(format!( "mount path {host:?} does not exist and its parent doesn't either; \ refusing to auto-create (fix the path or `mkdir -p` manually)" ))); } std::fs::create_dir_all(&host_path).map_err(|e| { - RunnerError::Preflight(format!("auto-create mount dir {host:?}: {e}")) + RunnerError::Setup(format!("auto-create mount dir {host:?}: {e}")) })?; } @@ -525,12 +549,12 @@ impl HyperlightScriptRunner { // so a relative path keeps meaning the same directory after a // cwd change. let host_path = std::fs::canonicalize(&host_path) - .map_err(|e| RunnerError::Preflight(format!("resolve mount path {host:?}: {e}")))?; + .map_err(|e| RunnerError::Setup(format!("resolve mount path {host:?}: {e}")))?; let basename = host_path .file_name() .and_then(|n| n.to_str()) .ok_or_else(|| { - RunnerError::Preflight(format!("mount path {host:?} has no filename component")) + RunnerError::Policy(format!("mount path {host:?} has no filename component")) })?; // The guest path travels to the kernel in its `vfs.fstab` // list, which these characters would break; the library @@ -539,14 +563,14 @@ impl HyperlightScriptRunner { .chars() .any(|c| c.is_whitespace() || matches!(c, ':' | '[' | ']')) { - return Err(RunnerError::Preflight(format!( + return Err(RunnerError::Policy(format!( "mount path {host:?}: the directory name may not contain whitespace, \ ':' or brackets (it names the guest mount point)" ))); } let guest_path = format!("{GUEST_MOUNT_ROOT}/{basename}"); if !seen_guest_paths.insert(guest_path.clone()) { - return Err(RunnerError::Preflight(format!( + return Err(RunnerError::Policy(format!( "two mount paths collide on guest path {guest_path:?}; \ rename one of the host directories" ))); @@ -616,7 +640,7 @@ impl HyperlightScriptRunner { let builder = SandboxBuilder::from_snapshot(rewind).mounts(mounts.iter().cloned()); let sandbox = builder .boot() - .map_err(|e| RunnerError::Runtime(format!("restore hyperlight snapshot: {e}")))?; + .map_err(|e| RunnerError::Launch(format!("restore hyperlight snapshot: {e}")))?; Ok(Guest { sandbox, fresh: true, @@ -724,11 +748,11 @@ impl HyperlightScriptRunner { // next call boots another from the rewind point. self.guest = None; let err = match failure { - RunError::TimedOut(timeout) => RunnerError::Runtime(format!( + RunError::TimedOut(timeout) => RunnerError::Timeout(format!( "execution timed out after {:.1}s", timeout.as_secs_f64() )), - RunError::Failed(msg) => RunnerError::Runtime(format!("run: {msg}")), + RunError::Failed(msg) => RunnerError::Exec(format!("run: {msg}")), }; logger.log_line(&err.to_string()); err.to_response() @@ -873,7 +897,7 @@ fn warm_snapshot( configure_surrogates(); let mut sandbox = rootfs_builder(home, runtime) .boot() - .map_err(|e| RunnerError::Runtime(format!("boot hyperlight rootfs: {e}")))?; + .map_err(|e| RunnerError::Launch(format!("boot hyperlight rootfs: {e}")))?; // Saved beside the snapshot directory, so a failed save leaves whatever // is there standing, then published by rename. let staged = home.join(format!(".{SNAPSHOT_DIR}.{}.part", std::process::id())); @@ -882,7 +906,7 @@ fn warm_snapshot( Ok(snapshot) => snapshot, Err(e) => { let _ = std::fs::remove_dir_all(&staged); - return Err(RunnerError::Runtime(format!( + return Err(RunnerError::Launch(format!( "save snapshot to {staged:?}: {e}" ))); } @@ -975,7 +999,7 @@ fn load_persisted_snapshot( Err(e) => e, }; if !has_install_source(home, runtime) { - return Err(RunnerError::Preflight(no_install_source(runtime))); + return Err(RunnerError::Unavailable(no_install_source(runtime))); } logger.log_line(&match unloadable { hyperlight_unikraft::Error::SnapshotRelease { saved_by, .. } => format!( @@ -1851,7 +1875,8 @@ mod tests { }; let mut logger = Logger::new(Mode::Buffer); let resp = r.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, FailurePhase::Rejected.failure_exit_code()); assert_denied_overlap_message(&resp.error_message); } @@ -1955,7 +1980,8 @@ mod tests { }; let mut logger = Logger::new(Mode::Buffer); let resp = r.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, FailurePhase::Rejected.failure_exit_code()); assert_denied_overlap_message(&resp.error_message); } @@ -1976,7 +2002,8 @@ mod tests { }; let mut logger = Logger::new(Mode::Buffer); let resp = r.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, FailurePhase::Rejected.failure_exit_code()); assert_denied_overlap_message(&resp.error_message); } @@ -2173,7 +2200,43 @@ mod tests { }; let mut logger = Logger::new(Mode::Buffer); let resp = r.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, FailurePhase::Rejected.failure_exit_code()); assert!(resp.error_message.contains(ERR_WORKDIR)); } + + #[test] + fn a_host_without_an_image_blames_the_host() { + let resp = + RunnerError::Unavailable(no_install_source(HyperlightRuntime::Agent)).to_response(); + assert_eq!(resp.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!(resp.exit_code, -1); + assert!(resp.error_message.contains("not available on this host")); + } + + #[test] + fn run_faults_keep_the_generic_backend_error_code() { + for (err, phase) in [ + ( + RunnerError::Setup("mount".to_string()), + FailurePhase::LaunchFailed, + ), + ( + RunnerError::Launch("boot".to_string()), + FailurePhase::LaunchFailed, + ), + ( + RunnerError::Exec("run".to_string()), + FailurePhase::PostLaunchFailed, + ), + ( + RunnerError::Timeout("deadline".to_string()), + FailurePhase::Timeout, + ), + ] { + let resp = err.to_response(); + assert_eq!(resp.failure_phase, phase, "{err}"); + assert_eq!(resp.exit_code, -1, "{err}"); + } + } } diff --git a/src/mxc-sdk/src/backends/isolation_session/common/error.rs b/src/mxc-sdk/src/backends/isolation_session/common/error.rs index ad6e3c9bf..173e4311a 100644 --- a/src/mxc-sdk/src/backends/isolation_session/common/error.rs +++ b/src/mxc-sdk/src/backends/isolation_session/common/error.rs @@ -251,7 +251,14 @@ impl std::fmt::Display for IsolationSessionError { impl From for ScriptResponse { fn from(err: IsolationSessionError) -> Self { - ScriptResponse::error(&err.to_string()) + let message = err.to_string(); + match err { + IsolationSessionError::Policy(_) => ScriptResponse::rejected(&message), + IsolationSessionError::ServiceUnavailable(_) => ScriptResponse::unavailable(&message), + IsolationSessionError::Lifecycle(_) | IsolationSessionError::Stale(_) => { + ScriptResponse::error(&message) + } + } } } @@ -498,6 +505,7 @@ pub(crate) mod test_support { #[cfg(test)] mod tests { use super::*; + use crate::mxc_common::models::FailurePhase; fn api_failure(code: Option) -> IsoApiFailure { IsoApiFailure::new( @@ -952,6 +960,23 @@ mod tests { assert!(response.error_message.contains("remediation")); } + /// The one-shot path has no structured envelope, so the phase on the + /// response is the only thing that tells the caller who is at fault. + #[test] + fn script_response_conversion_blames_the_same_side_as_the_wire_error() { + let refused: ScriptResponse = IsolationSessionError::Policy("no proxy".into()).into(); + assert_eq!(refused.failure_phase, FailurePhase::Rejected); + assert_eq!(refused.exit_code, 1); + + let unavailable: ScriptResponse = activation_error(REGDB_E_CLASSNOTREG_HRESULT, "x").into(); + assert_eq!(unavailable.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!(unavailable.exit_code, -1); + + let runtime: ScriptResponse = lifecycle_err("create stdout relay: out of memory").into(); + assert_eq!(runtime.failure_phase, FailurePhase::None); + assert_eq!(runtime.exit_code, -1); + } + // ── operation values ───────────────────────────────────────────────── /// `operation` must stay interface-qualified and free of call diff --git a/src/mxc-sdk/src/backends/isolation_session/common/manager.rs b/src/mxc-sdk/src/backends/isolation_session/common/manager.rs index 7ec4c8b79..505334c0b 100644 --- a/src/mxc-sdk/src/backends/isolation_session/common/manager.rs +++ b/src/mxc-sdk/src/backends/isolation_session/common/manager.rs @@ -37,7 +37,9 @@ use super::owned_thread::{self, Impersonation}; use super::pipe_relay::{ create_relay_thread, create_relay_thread_with_stop, duplicate_handle, PipeRelayWithStopParams, }; -use super::process_options::{build_iso_process_options, ProcessOptions}; +use super::process_options::{ + build_iso_process_options, with_service_timeout_grace, ProcessOptions, +}; // The OS account name is lifecycle addressing data, not diagnostic identity. const AGENT_IDENTITY_MARKER: &str = "isolation-session-agent"; @@ -398,16 +400,22 @@ impl IsolationSessionManager { /// Step 3: Create a process inside the started isolation session and run it /// to completion. Output is streamed live to the calling process's stdio via - /// internal relay threads; only the exit code is returned. + /// internal relay threads; only the exit code or timeout is returned. + /// + /// `options.timeout_ms` is enforced locally; the service-side timer is armed + /// with [`with_service_timeout_grace`] so a timeout is never reported as an + /// ordinary exit. /// /// This is the **relay** path, reached by `wxc-exec` and by an in-process /// caller that asked to attach. A caller that wants the streams handed back /// uses [`Self::piped_exec_handle`] instead. + /// + /// [`with_service_timeout_grace`]: super::process_options::with_service_timeout_grace pub(super) fn create_process( &self, options: &ProcessOptions, logger: Option<&mut Logger>, - ) -> Result { + ) -> Result { owned_thread::call(&self.impersonation, || { // Everything fallible that does not need the workload runs first, so no // failure can strand a running one. @@ -448,7 +456,8 @@ impl IsolationSessionManager { // Lifetime: each relay's param struct is moved to the heap and owned // by its thread, which frees it on exit, so joining is not required // for memory safety. - let started = self.start_process(options, logger.as_deref())?; + let service_options = with_service_timeout_grace(options.clone()); + let started = self.start_process(&service_options, logger.as_deref())?; let process = &started.process; let stdout_handle_val = started.stdout; let stderr_handle_val = started.stderr; @@ -583,10 +592,7 @@ impl IsolationSessionManager { scope.stop_stdin(); scope.finish(); - Ok(match outcome { - ExecOutcome::Exited(exit_code) => exit_code, - ExecOutcome::TimedOut => WAIT_FOR_EXIT_TIMEOUT, - }) + Ok(outcome) }) } diff --git a/src/mxc-sdk/src/backends/isolation_session/common/one_shot.rs b/src/mxc-sdk/src/backends/isolation_session/common/one_shot.rs index 68161e291..544fd87c7 100644 --- a/src/mxc-sdk/src/backends/isolation_session/common/one_shot.rs +++ b/src/mxc-sdk/src/backends/isolation_session/common/one_shot.rs @@ -9,8 +9,9 @@ use std::fmt::Write; use std::io::IsTerminal; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ExecutionRequest, ScriptResponse}; +use crate::mxc_common::models::{ExecutionRequest, FailurePhase, ScriptResponse}; use crate::mxc_common::script_runner::ScriptRunner; +use crate::mxc_common::state_aware_backend::ExecOutcome; use crate::mxc_common::validator::{validate_network_policy_support, NetworkPolicySupport}; use super::manager::{log_sandbox_torn_down, IsolationSessionManager, TeardownOutcome}; @@ -34,13 +35,13 @@ use super::IsolationSessionRunner; /// anything applied — so there is nothing to retain. fn reject_unsupported_lifecycle(request: &ExecutionRequest) -> Result<(), ScriptResponse> { if !request.lifecycle.destroy_on_exit { - return Err(ScriptResponse::error( + return Err(ScriptResponse::rejected( "lifecycle.destroyOnExit=false is not supported by the isolation session backend; \ the session is always stopped and the agent user removed", )); } if request.lifecycle.preserve_policy { - return Err(ScriptResponse::error( + return Err(ScriptResponse::rejected( "lifecycle.preservePolicy=true is not supported by the isolation session backend; \ it installs no persistent filesystem or network enforcement, so there is none \ to preserve", @@ -121,8 +122,8 @@ impl ScriptRunner for IsolationSessionRunner { return e.into(); } - let exit_code = match manager.create_process(&options, Some(logger)) { - Ok(code) => code, + let exec_outcome = match manager.create_process(&options, Some(logger)) { + Ok(exec_outcome) => exec_outcome, Err(e) => { let stopped = manager.stop_session().is_ok(); let deprovisioned = manager.deprovision_agent_user().is_ok(); @@ -158,13 +159,27 @@ impl ScriptRunner for IsolationSessionRunner { // Output already streamed live to wxc-exec's stdio via relay // threads in `create_process` — captured fields intentionally // empty (same pattern as AppContainer). - ScriptResponse { + completed_response(exec_outcome, options.timeout_ms) + } +} + +/// The response for a completed run, so a workload that failed or timed out is +/// told apart from the sandbox failing around it. +fn completed_response(outcome: ExecOutcome, timeout_ms: u32) -> ScriptResponse { + match outcome { + ExecOutcome::Exited(0) => ScriptResponse { + exit_code: 0, + ..Default::default() + }, + ExecOutcome::Exited(exit_code) => ScriptResponse { exit_code, - standard_out: String::new(), - standard_err: String::new(), - error_message: String::new(), + failure_phase: FailurePhase::ProcessExited, ..Default::default() - } + }, + ExecOutcome::TimedOut => ScriptResponse::failed( + FailurePhase::Timeout, + &format!("script timed out after {timeout_ms}ms"), + ), } } @@ -175,6 +190,22 @@ mod tests { ContainerPolicy, LifecycleConfig, NetworkAction, NetworkEgressPolicy, NetworkIngressPolicy, }; + #[test] + fn completed_runs_distinguish_exit_from_timeout() { + let ok = completed_response(ExecOutcome::Exited(0), 5_000); + assert_eq!((ok.exit_code, ok.failure_phase), (0, FailurePhase::None)); + + let exited = completed_response(ExecOutcome::Exited(-1), 5_000); + assert_eq!( + (exited.exit_code, exited.failure_phase), + (-1, FailurePhase::ProcessExited) + ); + + let timed_out = completed_response(ExecOutcome::TimedOut, 5_000); + assert_eq!(timed_out.failure_phase, FailurePhase::Timeout); + assert_eq!(timed_out.error_message, "script timed out after 5000ms"); + } + #[test] fn validate_runner_one_shot_rejects_default_network() { // No user, but the default (absent → `Block`) network is not the @@ -250,6 +281,29 @@ mod tests { ); } + #[test] + fn validate_runner_one_shot_lifecycle_refusals_blame_the_request() { + let runner = IsolationSessionRunner::new(); + for lifecycle in [ + LifecycleConfig { + destroy_on_exit: false, + preserve_policy: false, + }, + LifecycleConfig { + destroy_on_exit: true, + preserve_policy: true, + }, + ] { + let req = ExecutionRequest { + lifecycle, + ..canonical_request() + }; + let resp = runner.validate_runner(&req).unwrap_err(); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, 1); + } + } + #[test] fn validate_runner_one_shot_accepts_default_lifecycle() { // `destroyOnExit: true` (the default) is exactly what the backend does, diff --git a/src/mxc-sdk/src/backends/isolation_session/common/process_options.rs b/src/mxc-sdk/src/backends/isolation_session/common/process_options.rs index d40558ca9..e0968cc39 100644 --- a/src/mxc-sdk/src/backends/isolation_session/common/process_options.rs +++ b/src/mxc-sdk/src/backends/isolation_session/common/process_options.rs @@ -135,10 +135,8 @@ pub(super) const SERVICE_TIMEOUT_GRACE_MS: u32 = 5_000; /// Relaxes the service-side timer so the caller's deadline is enforced locally. /// -/// Only for the streaming path, which has somewhere to report a timeout. The -/// run-to-completion path keeps the service timer exactly as it was: it has no -/// timeout channel, and its observable behaviour (exit code 1 on an OS-side -/// timeout) is pinned by the host suite. +/// Without the margin the two deadlines coincide, and the service's kill (an +/// ordinary exit code 1) could win the race and hide the timeout. /// /// Two deadlines are left alone rather than shifted: /// diff --git a/src/mxc-sdk/src/backends/isolation_session/common/sandbox.rs b/src/mxc-sdk/src/backends/isolation_session/common/sandbox.rs index 31166252a..56de32eec 100644 --- a/src/mxc-sdk/src/backends/isolation_session/common/sandbox.rs +++ b/src/mxc-sdk/src/backends/isolation_session/common/sandbox.rs @@ -21,7 +21,7 @@ use std::thread::JoinHandle; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ExecutionRequest, FailurePhase, ScriptResponse}; -use crate::mxc_common::mxc_error::MxcError; +use crate::mxc_common::mxc_error::{MxcError, MxcErrorCode}; use crate::mxc_common::process_util::{ InterruptiblePipeReader, OwnedHandle as ProcessOwnedHandle, PipeReadCanceller, }; @@ -75,13 +75,24 @@ pub fn spawn_one_shot( /// workload exits, and whose relay scope borrows the process — into a /// non-blocking start with detached relays. fn inherit_not_served() -> ScriptResponse { - ScriptResponse { - failure_phase: FailurePhase::Rejected, - ..ScriptResponse::error( - "the isolation session backend does not yet support inherited stdio; it serves piped \ - stdio, which is what an in-process caller receives. Run it through wxc-exec to have \ - the workload relayed onto this process's own stdio.", - ) + ScriptResponse::rejected( + "the isolation session backend does not yet support inherited stdio; it serves piped \ + stdio, which is what an in-process caller receives. Run it through wxc-exec to have \ + the workload relayed onto this process's own stdio.", + ) +} + +/// Flattens a launch failure onto a [`ScriptResponse`], keeping the side the +/// typed error blames. +/// +/// A launch reaches the OS API, so it can fail because that API is not there at +/// all. Reporting that as a launch failure would point the caller at a retry +/// that cannot help. +fn launch_failure_response(error: &MxcError) -> ScriptResponse { + match error.code { + MxcErrorCode::BackendUnavailable => ScriptResponse::unavailable(&error.message), + MxcErrorCode::PolicyValidation => ScriptResponse::rejected(&error.message), + _ => ScriptResponse::failed(FailurePhase::LaunchFailed, &error.message), } } @@ -89,13 +100,7 @@ impl SandboxBackend for IsolationSessionRunner { fn validate(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { // One-shot runs the whole lifecycle, so provision-phase rules apply to // the whole call — the same check the run-to-completion path makes. - // - // Everything it refuses is a caller-fixable request problem, and the - // phase is what the dispatcher reads back to classify it. - ScriptRunner::validate_runner(self, request).map_err(|resp| ScriptResponse { - failure_phase: FailurePhase::Rejected, - ..resp - }) + ScriptRunner::validate_runner(self, request) } /// Provision, start, and hand back the exec's live pipes. @@ -114,19 +119,13 @@ impl SandboxBackend for IsolationSessionRunner { StdioMode::Pty(size) => { spawn_one_shot_pty(request, logger, size).map_err(|error| match error { OneShotSpawnFailure::Refused(response) => response, - OneShotSpawnFailure::Launch(error) => ScriptResponse { - failure_phase: FailurePhase::LaunchFailed, - ..ScriptResponse::error(&error.message) - }, + OneShotSpawnFailure::Launch(error) => launch_failure_response(&error), }) } // Delegated so both entry points share one validation prologue. StdioMode::Pipes => spawn_one_shot(request, logger).map_err(|e| match e { OneShotSpawnFailure::Refused(resp) => resp, - OneShotSpawnFailure::Launch(err) => ScriptResponse { - failure_phase: FailurePhase::LaunchFailed, - ..ScriptResponse::error(&err.message) - }, + OneShotSpawnFailure::Launch(err) => launch_failure_response(&err), }), } } @@ -801,7 +800,6 @@ impl Drop for IsolationPtyProcess { #[cfg(test)] mod tests { use super::*; - use crate::mxc_common::mxc_error::MxcErrorCode; /// A launch failure that also failed to clean up keeps its own /// classification, so a caller still branches on the real cause. @@ -837,4 +835,29 @@ mod tests { let after = with_cleanup_failures(launch, &[]).to_string(); assert_eq!(before, after); } + + /// Flattening onto a `ScriptResponse` must not turn an absent API into a + /// launch failure the caller would retry. + #[test] + fn a_launch_response_keeps_the_side_the_typed_error_blames() { + let unavailable = + launch_failure_response(&MxcError::backend_unavailable("the API is not registered")); + assert_eq!(unavailable.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!(unavailable.exit_code, -1); + + let refused = launch_failure_response(&MxcError::policy_validation("no proxy")); + assert_eq!(refused.failure_phase, FailurePhase::Rejected); + assert_eq!(refused.exit_code, 1); + + let launch = launch_failure_response(&MxcError::backend_error("StartSessionAsync failed")); + assert_eq!(launch.failure_phase, FailurePhase::LaunchFailed); + assert_eq!(launch.exit_code, -1); + } + + #[test] + fn inherited_stdio_is_refused_as_a_request_problem() { + let refused = inherit_not_served(); + assert_eq!(refused.failure_phase, FailurePhase::Rejected); + assert_eq!(refused.exit_code, 1); + } } diff --git a/src/mxc-sdk/src/backends/isolation_session/common/state_aware.rs b/src/mxc-sdk/src/backends/isolation_session/common/state_aware.rs index b31c8a9b8..43681271a 100644 --- a/src/mxc-sdk/src/backends/isolation_session/common/state_aware.rs +++ b/src/mxc-sdk/src/backends/isolation_session/common/state_aware.rs @@ -316,9 +316,10 @@ impl StatefulSandboxBackend for IsolationSessionRunner { ); let mut logger = Logger::inherit_thread_diagnostic_sink(); - let exit_code = manager + let outcome = manager .create_process(&options, Some(&mut logger)) .map_err(map_lifecycle_error)?; + let exit_code = relayed_exit_code(outcome, options.timeout_ms)?; // The output relay completed inside `create_process`. The // dispatcher sees zero pipe handles, skips its own relay setup, @@ -367,6 +368,17 @@ impl StatefulSandboxBackend for IsolationSessionRunner { } } +/// A relayed exec reports an exit code, so a timeout must surface as an error +/// rather than an exit code a workload could also return. +fn relayed_exit_code(outcome: ExecOutcome, timeout_ms: u32) -> Result { + match outcome { + ExecOutcome::Exited(exit_code) => Ok(exit_code), + ExecOutcome::TimedOut => Err(MxcError::backend_error(format!( + "script timed out after {timeout_ms}ms" + ))), + } +} + #[cfg(test)] mod tests { use super::*; @@ -376,6 +388,18 @@ mod tests { }; use crate::mxc_common::mxc_error::MxcErrorCode; + #[test] + fn a_relayed_timeout_is_an_error_not_an_exit_code() { + assert_eq!( + relayed_exit_code(ExecOutcome::Exited(-1), 5_000).unwrap(), + -1 + ); + + let error = relayed_exit_code(ExecOutcome::TimedOut, 5_000).unwrap_err(); + assert_eq!(error.code, MxcErrorCode::BackendError); + assert_eq!(error.message, "script timed out after 5000ms"); + } + // ====== Wire-format constants ====== // Checked binding verifies BACKEND_KEY before delivering configuration. diff --git a/src/mxc-sdk/src/backends/lxc/common/availability.rs b/src/mxc-sdk/src/backends/lxc/common/availability.rs index fbca924c7..8d69b787b 100644 --- a/src/mxc-sdk/src/backends/lxc/common/availability.rs +++ b/src/mxc-sdk/src/backends/lxc/common/availability.rs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +use std::io::ErrorKind; use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::OnceLock; use std::time::{Duration, Instant}; @@ -14,16 +16,43 @@ const POLL_INTERVAL: Duration = Duration::from_millis(25); enum LxcLsOutcome { ExitedSuccess, ExitedFailure, + NotFound, SpawnFailed, TimedOut, } +/// What execution should conclude from probing the LXC tooling. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LxcTooling { + Available, + /// `lxc-ls` is absent or reports itself broken. + Unavailable, + /// The probe itself failed (timeout, spawn error); worth retrying. + ProbeFailed, +} + /// Runs `lxc-ls --version`; only a clean exit counts as available. pub fn is_lxc_available() -> bool { static AVAILABLE: OnceLock = OnceLock::new(); *AVAILABLE.get_or_init(|| available_from(probe_lxc_ls())) } +/// The tooling verdict for execution, separate from the discovery cache. +/// +/// Only success is cached: a host that gains the LXC tools later, or whose +/// probe was transiently slow, is re-probed on the next run. +pub fn lxc_tooling_for_execution() -> LxcTooling { + static CONFIRMED: AtomicBool = AtomicBool::new(false); + if CONFIRMED.load(Ordering::Relaxed) { + return LxcTooling::Available; + } + let tooling = tooling_from(probe_lxc_ls()); + if tooling == LxcTooling::Available { + CONFIRMED.store(true, Ordering::Relaxed); + } + tooling +} + fn probe_lxc_ls() -> LxcLsOutcome { let child = Command::new("lxc-ls") .arg("--version") @@ -33,6 +62,7 @@ fn probe_lxc_ls() -> LxcLsOutcome { .spawn(); match child { Ok(mut child) => wait_bounded(&mut child, PROBE_TIMEOUT), + Err(error) if error.kind() == ErrorKind::NotFound => LxcLsOutcome::NotFound, Err(_) => LxcLsOutcome::SpawnFailed, } } @@ -60,6 +90,14 @@ fn available_from(outcome: LxcLsOutcome) -> bool { matches!(outcome, LxcLsOutcome::ExitedSuccess) } +fn tooling_from(outcome: LxcLsOutcome) -> LxcTooling { + match outcome { + LxcLsOutcome::ExitedSuccess => LxcTooling::Available, + LxcLsOutcome::NotFound | LxcLsOutcome::ExitedFailure => LxcTooling::Unavailable, + LxcLsOutcome::SpawnFailed | LxcLsOutcome::TimedOut => LxcTooling::ProbeFailed, + } +} + #[cfg(test)] mod tests { use super::*; @@ -68,7 +106,32 @@ mod tests { fn only_a_clean_exit_means_available() { assert!(available_from(LxcLsOutcome::ExitedSuccess)); assert!(!available_from(LxcLsOutcome::ExitedFailure)); + assert!(!available_from(LxcLsOutcome::NotFound)); assert!(!available_from(LxcLsOutcome::SpawnFailed)); assert!(!available_from(LxcLsOutcome::TimedOut)); } + + #[test] + fn a_missing_tool_is_unavailable_but_a_failed_probe_is_not() { + assert_eq!( + tooling_from(LxcLsOutcome::ExitedSuccess), + LxcTooling::Available + ); + assert_eq!( + tooling_from(LxcLsOutcome::NotFound), + LxcTooling::Unavailable + ); + assert_eq!( + tooling_from(LxcLsOutcome::ExitedFailure), + LxcTooling::Unavailable + ); + assert_eq!( + tooling_from(LxcLsOutcome::TimedOut), + LxcTooling::ProbeFailed + ); + assert_eq!( + tooling_from(LxcLsOutcome::SpawnFailed), + LxcTooling::ProbeFailed + ); + } } diff --git a/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs b/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs index ec3cefd4a..57f110fde 100644 --- a/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs +++ b/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs @@ -10,7 +10,7 @@ use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ - ContainerPolicy, ExecutionRequest, LifecycleConfig, LxcConfig, ScriptResponse, + ContainerPolicy, ExecutionRequest, FailurePhase, LifecycleConfig, LxcConfig, ScriptResponse, }; use crate::mxc_common::sandbox_process::{SandboxBackend, SandboxProcess, StdioMode}; use crate::mxc_common::script_runner::ScriptRunner; @@ -29,6 +29,7 @@ use crate::mxc_common::sandbox_process::{ #[cfg(target_os = "linux")] use crate::mxc_pty::{LivePty, PtySize as UnixPtySize}; +use crate::lxc_common::availability::LxcTooling; use crate::lxc_common::filesystem_mounts; use crate::lxc_common::lxc_bindings::{ContainerFirewall, LxcContainer, StartNetwork}; use crate::lxc_common::network_ingress::IngressManager; @@ -201,13 +202,16 @@ impl LxcScriptRunner { return self.claim_generated_name(); } ContainerNameClaim::acquire(&self.container_id).ok_or_else(|| { - ScriptResponse::error(&format!( - "LXC: container '{}' is already in use by a run in this process. LXC reads a \ - run's network section only when the container starts, so a second run would \ - stop the first one's workload to restart it under its own policy. Let the \ - first run finish, or give this one its own containerId.", - self.container_id - )) + ScriptResponse::failed( + FailurePhase::LaunchFailed, + &format!( + "LXC: container '{}' is already in use by a run in this process. LXC reads a \ + run's network section only when the container starts, so a second run would \ + stop the first one's workload to restart it under its own policy. Let the \ + first run finish, or give this one its own containerId.", + self.container_id + ), + ) }) } @@ -219,7 +223,8 @@ impl LxcScriptRunner { return Ok(claim); } } - Err(ScriptResponse::error( + Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, "LXC: could not find a free container name for this run; every generated name was \ already in use by another run in this process. Retry, or set containerId to a name \ of your own.", @@ -390,13 +395,16 @@ impl LxcScriptRunner { let release = self.release_kind(container_created); Self::report_release_failure(release, release_container(release), logger); - Some(ScriptResponse::error(&format!( - "Container did not receive an IPv4 address within {:.0}s; \ - check that lxc-net/dnsmasq is running and able to assign one. \ - A bridge offering only IPv6 leaves the container unable to reach \ - the IPv4 destinations its policy names.", - timeout.as_secs_f64() - ))) + Some(ScriptResponse::failed( + FailurePhase::PostLaunchFailed, + &format!( + "Container did not receive an IPv4 address within {:.0}s; \ + check that lxc-net/dnsmasq is running and able to assign one. \ + A bridge offering only IPv6 leaves the container unable to reach \ + the IPv4 destinations its policy names.", + timeout.as_secs_f64() + ), + )) } /// Everything that must succeed before a workload can launch; a failure @@ -420,14 +428,14 @@ impl LxcScriptRunner { &normalized } Ok(None) => request, - Err(msg) => return Err(ScriptResponse::error(&msg)), + Err(error) => return Err(error.into_response()), }; if let Err(msg) = crate::mxc_common::filesystem_access::check_delegation(&request.policy) { - return Err(ScriptResponse::error(&msg)); + return Err(ScriptResponse::rejected(&msg)); } if self.config.distribution.is_empty() || self.config.release.is_empty() { - return Err(ScriptResponse::error( + return Err(ScriptResponse::rejected( "LXC distribution and release are required \ (e.g., \"distribution\": \"alpine\", \"release\": \"3.23\")", )); @@ -439,7 +447,21 @@ impl LxcScriptRunner { // The policy lowers to the same rules with or without a container, so // one that cannot be programmed is refused before a container exists. if let Err(msg) = NetworkIptablesManager::validate_egress_lowering(&request.policy) { - return Err(ScriptResponse::error(&msg)); + return Err(ScriptResponse::rejected(&msg)); + } + + match crate::lxc_common::availability::lxc_tooling_for_execution() { + LxcTooling::Available => {} + LxcTooling::Unavailable => { + return Err(ScriptResponse::unavailable(LXC_TOOLING_UNAVAILABLE)); + } + LxcTooling::ProbeFailed => { + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + "LXC: probing the LXC tooling (`lxc-ls --version`) timed out or \ + could not run; retry once the host is responsive", + )); + } } if self.destroy_on_exit { @@ -468,10 +490,10 @@ impl LxcScriptRunner { if !container.is_defined() { let _ = writeln!(logger, "Creating LXC container..."); if let Err(e) = container.create(&self.config.distribution, &self.config.release) { - return Err(ScriptResponse::error(&format!( - "Failed to create container: {}", - e - ))); + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + &format!("Failed to create container: {}", e), + )); } let _ = writeln!(logger, "Container created successfully."); container_created = true; @@ -485,10 +507,10 @@ impl LxcScriptRunner { if self.destroy_on_exit || container_created { let _ = container.destroy(); } - return Err(ScriptResponse::error(&format!( - "Failed to configure filesystem: {}", - e - ))); + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + &format!("Failed to configure filesystem: {}", e), + )); } // LXC reads the network section only at start. A container an earlier @@ -502,11 +524,14 @@ impl LxcScriptRunner { if self.destroy_on_exit || container_created { let _ = container.destroy(); } - return Err(ScriptResponse::error(&format!( - "Failed to stop a container left running by an earlier run: {}. \ - Its network policy is the earlier run's, so the script was not run.", - e - ))); + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + &format!( + "Failed to stop a container left running by an earlier run: {}. \ + Its network policy is the earlier run's, so the script was not run.", + e + ), + )); } } @@ -527,10 +552,10 @@ impl LxcScriptRunner { if self.destroy_on_exit || container_created { let _ = container.destroy(); } - return Err(ScriptResponse::error(&format!( - "Failed to start container: {}", - e - ))); + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + &format!("Failed to start container: {}", e), + )); } let _ = writeln!(logger, "Container started successfully."); @@ -559,7 +584,8 @@ impl LxcScriptRunner { let netns_pid = container.init_pid(); let Some(hook_point) = egress_hook_point(netns_pid, plan.installs_firewall()) else { self.release_after_failure(&container, container_created, logger); - return Err(ScriptResponse::error( + return Err(ScriptResponse::failed( + FailurePhase::PostLaunchFailed, "Failed to discover the container init PID; cannot enter the container \ network namespace to install its firewall. Aborting rather than \ running without it.", @@ -585,7 +611,7 @@ impl LxcScriptRunner { Ok(managers) => managers, Err(e) => { self.release_after_failure(&container, container_created, logger); - return Err(ScriptResponse::error(&e)); + return Err(ScriptResponse::failed(FailurePhase::PostLaunchFailed, &e)); } }; @@ -605,7 +631,7 @@ impl LxcScriptRunner { if is_firewall_installed == ContainerFirewall::Installed { if let Err(e) = fw_manager.allow_ping_sockets(logger) { self.release_after_failure(&container, container_created, logger); - return Err(ScriptResponse::error(&e)); + return Err(ScriptResponse::failed(FailurePhase::PostLaunchFailed, &e)); } } @@ -624,12 +650,15 @@ impl LxcScriptRunner { }); if let Err(reason) = stale_pin_error { self.release_after_failure(&container, container_created, logger); - return Err(ScriptResponse::error(&format!( - "Failed to clear a stale network proxy pin from the container's \ - /etc/hosts: {}. The script was not run, because it could have resolved \ - the pinned hostname to an address this policy did not authorize.", - reason - ))); + return Err(ScriptResponse::failed( + FailurePhase::PostLaunchFailed, + &format!( + "Failed to clear a stale network proxy pin from the container's \ + /etc/hosts: {}. The script was not run, because it could have resolved \ + the pinned hostname to an address this policy did not authorize.", + reason + ), + )); } // `script_timeout == 0` means "no timeout" per the SDK contract. @@ -944,6 +973,11 @@ pub const LXC_INHERIT_STDIO_UNSUPPORTED: &str = pub const LXC_STREAMING_LINUX_ONLY: &str = "LXC: sandboxes can only be launched on Linux."; +pub const LXC_TOOLING_UNAVAILABLE: &str = + "LXC: the LXC runtime is not available on this host — `lxc-ls --version` did not run. \ + Install the LXC tools (e.g. apt install lxc) and make sure they are on PATH, or select \ + a backend this host can provide."; + fn unsupported_network_proxy(policy: &ContainerPolicy) -> Option<&'static str> { if policy.network_proxy.is_enabled() { Some("network.proxy") @@ -962,10 +996,10 @@ fn lxc_network_policy_support() -> NetworkPolicySupport { impl ScriptRunner for LxcScriptRunner { fn validate_runner(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { if request.policy.runtime_network_proxy_specified { - return Err(ScriptResponse::error(LXC_RUNTIME_PROXY_UNSUPPORTED)); + return Err(ScriptResponse::rejected(LXC_RUNTIME_PROXY_UNSUPPORTED)); } if let Some(field) = unsupported_network_proxy(&request.policy) { - return Err(ScriptResponse::error(&format!( + return Err(ScriptResponse::rejected(&format!( "LXC: {field} is not supported and cannot be enforced; \ remove the proxy request." ))); @@ -1024,7 +1058,7 @@ impl LxcScriptRunner { ) -> Result, ScriptResponse> { match stdio { StdioMode::Inherit => { - return Err(ScriptResponse::error(LXC_INHERIT_STDIO_UNSUPPORTED)); + return Err(ScriptResponse::rejected(LXC_INHERIT_STDIO_UNSUPPORTED)); } StdioMode::Pty(_) | StdioMode::Pipes => {} } @@ -1082,10 +1116,10 @@ impl LxcScriptRunner { Ok(launched) => launched, Err(e) => { let cleanup = self.abandon_prepared(&mut prepared, logger); - return Err(ScriptResponse::error(&launch_failure_message( - &format!("Execution failed: {}", e), - &cleanup, - ))); + return Err(ScriptResponse::failed( + FailurePhase::LaunchFailed, + &launch_failure_message(&format!("Execution failed: {}", e), &cleanup), + )); } }; @@ -1112,13 +1146,16 @@ impl LxcScriptRunner { let cleanup = self.abandon_prepared(&mut prepared, logger); let error = out_result.err().or(err_result.err()); - return Err(ScriptResponse::error(&launch_failure_message( - &format!( - "Failed to wrap the lxc-attach stdio pipes: {}", - error.map_or_else(|| "unknown error".to_string(), |e| e.to_string()) + return Err(ScriptResponse::failed( + FailurePhase::PostLaunchFailed, + &launch_failure_message( + &format!( + "Failed to wrap the lxc-attach stdio pipes: {}", + error.map_or_else(|| "unknown error".to_string(), |e| e.to_string()) + ), + &cleanup, ), - &cleanup, - ))); + )); } }; @@ -1148,7 +1185,7 @@ impl LxcScriptRunner { _logger: &mut Logger, _stdio: StdioMode, ) -> Result, ScriptResponse> { - Err(ScriptResponse::error(LXC_STREAMING_LINUX_ONLY)) + Err(ScriptResponse::unavailable(LXC_STREAMING_LINUX_ONLY)) } } @@ -2827,9 +2864,9 @@ mod tests { "the fail-closed readiness timeout message changed unexpectedly: {}", response.error_message ); - assert_eq!( - response.standard_err, response.error_message, - "error responses should carry the message in stderr as well" + assert!( + response.standard_err.is_empty(), + "the message belongs in the error envelope, not repeated in stderr" ); } @@ -3029,6 +3066,131 @@ mod tests { .contains("Script content must not be empty"), "expected the shared refusal, got: {refusal:?}" ); + // The shared checks judge the request, so they have to reach the + // caller as a rejection on the launch path too, not only through a + // standalone `validate`. + assert_eq!(refusal.failure_phase, FailurePhase::Rejected); + assert_eq!( + refusal.exit_code, + FailurePhase::Rejected.failure_exit_code() + ); + } + + /// A caller can only act on a failure it can attribute: a refused policy is + /// worth rewriting, a host that cannot run LXC at all is not. + #[test] + fn a_policy_this_backend_cannot_enforce_is_blamed_on_the_request() { + let runtime_proxy = ExecutionRequest { + script_code: "echo hello".to_string(), + policy: ContainerPolicy { + runtime_network_proxy_specified: true, + ..Default::default() + }, + ..Default::default() + }; + + for (what, request) in [ + ("runtimeConfig.networkProxy", runtime_proxy), + ( + "network.proxy", + request_with_proxy_url("http://proxy.example.com:8080"), + ), + ( + "unlowerable network.egress", + request_with_unlowerable_egress(), + ), + ] { + let refusal = runner_for_guard_tests("refusal-phase") + .prepare(&request, &mut Logger::new(Mode::Buffer)) + .err() + .unwrap_or_else(|| panic!("input={what}; expected a refusal")); + + assert_eq!( + refusal.failure_phase, + FailurePhase::Rejected, + "input={what}; expected a refusal of the request; output={refusal:?}" + ); + assert_eq!( + refusal.exit_code, 1, + "input={what}; a refusal exits 1; output={refusal:?}" + ); + } + } + + #[test] + fn an_image_the_config_never_named_is_blamed_on_the_request() { + let refusal = validating_runner() + .prepare(&ExecutionRequest::default(), &mut Logger::new(Mode::Buffer)) + .err() + .expect("an empty distribution and release cannot name an image to create"); + + assert_eq!(refusal.failure_phase, FailurePhase::Rejected); + assert_eq!(refusal.exit_code, 1); + } + + #[test] + fn inherited_stdio_is_refused_as_a_request_fault() { + let refusal = validating_runner() + .spawn( + &streamable_request(), + &mut Logger::new(Mode::Buffer), + StdioMode::Inherit, + ) + .err() + .expect("the library path has no pty to hand a workload"); + + assert_eq!( + refusal.failure_phase, + FailurePhase::Rejected, + "pipes and a caller-owned PTY are both reachable, so only the request has to change" + ); + } + + /// A container another live sandbox holds is contention, not a refusal: the + /// same request succeeds once the first run finishes. + #[test] + fn a_container_another_run_holds_is_a_launch_fault() { + let runner = runner_named("mxc-claim-phase-test"); + let _held = runner + .claim_container_name() + .expect("the first sandbox takes a container no other one is using"); + + let refusal = runner + .claim_container_name() + .expect_err("a second sandbox on a live container must be refused"); + + assert_eq!(refusal.failure_phase, FailurePhase::LaunchFailed); + assert_eq!(refusal.exit_code, -1); + } + + #[test] + fn a_container_that_never_got_an_address_is_a_post_launch_fault() { + let runner = runner_for_network_readiness_tests(false); + let response = fail_network_readiness(&runner, false).response; + + assert_eq!( + response.failure_phase, + FailurePhase::PostLaunchFailed, + "the container started; it was the sandbox network that never came up" + ); + } + + /// Only reachable off Linux, where the backend does not exist at all. + #[cfg(not(target_os = "linux"))] + #[test] + fn a_host_that_cannot_run_lxc_is_blamed_instead_of_the_request() { + let refusal = validating_runner() + .launch_streaming( + &streamable_request(), + &mut Logger::new(Mode::Buffer), + StdioMode::Pipes, + ) + .err() + .expect("LXC sandboxes only launch on Linux"); + + assert_eq!(refusal.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!(refusal.exit_code, -1); + assert_eq!(refusal.error_message, LXC_STREAMING_LINUX_ONLY); } #[test] diff --git a/src/mxc-sdk/src/backends/nanvix/runner/mod.rs b/src/mxc-sdk/src/backends/nanvix/runner/mod.rs index aa1dc3bfc..950f06aaf 100644 --- a/src/mxc-sdk/src/backends/nanvix/runner/mod.rs +++ b/src/mxc-sdk/src/backends/nanvix/runner/mod.rs @@ -56,7 +56,7 @@ use std::thread::JoinHandle; use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ExecutionRequest, NetworkAction, ScriptResponse}; +use crate::mxc_common::models::{ExecutionRequest, FailurePhase, NetworkAction, ScriptResponse}; use crate::mxc_common::script_runner::ScriptRunner; use crate::mxc_common::validator::{validate_network_policy_support, NetworkPolicySupport}; @@ -132,8 +132,13 @@ fn exit_code_from_status(status: &std::process::ExitStatus) -> i32 { /// Classifies NanVix runner errors for structured error handling. #[derive(Debug)] enum NanVixError { - /// Pre-spawn validation failures (missing artifacts, invalid config, unsupported policy). - Preflight(String), + /// The request asks for something the NanVix guest will not enforce. + Policy(String), + /// A NanVix runtime artifact the host must supply is missing. + Unavailable(String), + /// Host-side preparation failed before the micro-VM was spawned + /// (snapshot generation, staging, locating the installation). + Setup(String), /// OS/platform failures while spawning/managing the NanVix process (WHP/spawn/handles). Platform(String), /// Stdin broken pipe, VM crash. @@ -148,7 +153,11 @@ enum NanVixError { impl std::fmt::Display for NanVixError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - NanVixError::Preflight(msg) => write!(f, "NanVix preflight error: {}", msg), + NanVixError::Policy(msg) => write!(f, "NanVix policy error: {}", msg), + NanVixError::Unavailable(msg) => { + write!(f, "NanVix is not available on this host: {}", msg) + } + NanVixError::Setup(msg) => write!(f, "NanVix setup error: {}", msg), NanVixError::Platform(msg) => write!(f, "NanVix platform error: {}", msg), NanVixError::Runtime(msg) => write!(f, "NanVix runtime error: {}", msg), NanVixError::Timeout { @@ -165,13 +174,21 @@ impl std::fmt::Display for NanVixError { } impl NanVixError { - fn to_response(&self) -> ScriptResponse { - ScriptResponse { - exit_code: ERROR_EXIT_CODE, - error_message: self.to_string(), - ..Default::default() + /// Which side a failure is attributed to: the request, the host, or the + /// run itself. + fn failure_phase(&self) -> FailurePhase { + match self { + NanVixError::Policy(_) => FailurePhase::Rejected, + NanVixError::Unavailable(_) => FailurePhase::BackendUnavailable, + NanVixError::Setup(_) | NanVixError::Platform(_) => FailurePhase::LaunchFailed, + NanVixError::Runtime(_) => FailurePhase::PostLaunchFailed, + NanVixError::Timeout { .. } => FailurePhase::Timeout, } } + + fn to_response(&self) -> ScriptResponse { + ScriptResponse::failed(self.failure_phase(), &self.to_string()) + } } /// Returns the directory containing the current executable. @@ -180,11 +197,11 @@ impl NanVixError { /// `process_util` is gated to `target_os = "windows"`. fn exe_dir() -> Result { std::env::current_exe() - .map_err(|e| NanVixError::Preflight(format!("cannot determine exe path: {}", e))) + .map_err(|e| NanVixError::Setup(format!("cannot determine exe path: {}", e))) .and_then(|exe| { exe.parent() .map(|p| p.to_path_buf()) - .ok_or_else(|| NanVixError::Preflight("exe has no parent directory".to_string())) + .ok_or_else(|| NanVixError::Setup("exe has no parent directory".to_string())) }) } @@ -321,7 +338,7 @@ impl NanVixScriptRunner { let nanvixd = dir.join(NANVIXD_BINARY); if !nanvixd.exists() { - return Err(NanVixError::Preflight(format!( + return Err(NanVixError::Unavailable(format!( "{} not found in {:?}", NANVIXD_BINARY, dir ))); @@ -329,7 +346,7 @@ impl NanVixScriptRunner { let ramfs = dir.join(RAMFS_IMAGE); if !ramfs.exists() { - return Err(NanVixError::Preflight(format!( + return Err(NanVixError::Unavailable(format!( "{} not found in {:?}", RAMFS_IMAGE, dir ))); @@ -337,7 +354,7 @@ impl NanVixScriptRunner { let initrd = dir.join(INITRD_BINARY); if !initrd.exists() { - return Err(NanVixError::Preflight(format!( + return Err(NanVixError::Unavailable(format!( "{} not found in {:?}", INITRD_BINARY, dir ))); @@ -350,7 +367,7 @@ impl NanVixScriptRunner { for name in crate::nanvix_common::BIN_SUBDIR_FILES { let path = bin_subdir.join(name); if !path.exists() { - return Err(NanVixError::Preflight(format!( + return Err(NanVixError::Unavailable(format!( "{}/{} not found in {:?}", BIN_DIR, name, dir ))); @@ -405,7 +422,7 @@ impl NanVixScriptRunner { let p = PathBuf::from(val); if !p.as_os_str().is_empty() { std::fs::create_dir_all(&p).map_err(|e| { - NanVixError::Preflight(format!( + NanVixError::Setup(format!( "cannot create ${} directory {:?}: {}", NANVIX_HOME_ENV, p, e )) @@ -457,9 +474,8 @@ impl NanVixScriptRunner { ramfs: &Path, initrd: &Path, ) -> Result<(), NanVixError> { - std::fs::create_dir_all(snapshot_home).map_err(|e| { - NanVixError::Preflight(format!("failed to create snapshot home: {}", e)) - })?; + std::fs::create_dir_all(snapshot_home) + .map_err(|e| NanVixError::Setup(format!("failed to create snapshot home: {}", e)))?; eprintln!("nanvix: no snapshot found — generating via cold boot (one-time cost)..."); @@ -471,7 +487,7 @@ impl NanVixScriptRunner { ramfs, initrd, ) - .map_err(NanVixError::Preflight)?; + .map_err(NanVixError::Setup)?; eprintln!( "nanvix: snapshot generated in {:.0?} — subsequent runs will use warm start", @@ -501,7 +517,7 @@ impl NanVixScriptRunner { .as_ref() .is_some_and(|egress| !egress.allow.is_empty() || !egress.deny.is_empty()) { - return Err(NanVixError::Preflight(ERR_DIRECTIONAL_FILTERS.to_string())); + return Err(NanVixError::Policy(ERR_DIRECTIONAL_FILTERS.to_string())); } let egress = policy .network_egress @@ -519,7 +535,7 @@ impl NanVixScriptRunner { .map(|ingress| ingress.host_loopback) .unwrap_or(NetworkAction::Deny); if egress != ingress || ingress != host_loopback { - return Err(NanVixError::Preflight(ERR_DIRECTIONAL_NETWORK.to_string())); + return Err(NanVixError::Policy(ERR_DIRECTIONAL_NETWORK.to_string())); } Ok(egress == NetworkAction::Allow) } @@ -527,13 +543,13 @@ impl NanVixScriptRunner { fn validate_policies(request: &ExecutionRequest) -> Result<(), NanVixError> { // denied_paths is explicitly rejected — microvm has no host visibility. if !request.policy.denied_paths.is_empty() { - return Err(NanVixError::Preflight(ERR_DENIED_PATHS.to_string())); + return Err(NanVixError::Policy(ERR_DENIED_PATHS.to_string())); } if request.policy.network_proxy.is_enabled() { - return Err(NanVixError::Preflight(ERR_PROXY_POLICY.to_string())); + return Err(NanVixError::Policy(ERR_PROXY_POLICY.to_string())); } if !request.working_directory.is_empty() { - return Err(NanVixError::Preflight(ERR_WORKDIR.to_string())); + return Err(NanVixError::Policy(ERR_WORKDIR.to_string())); } Self::resolve_networking_mode(request)?; @@ -806,8 +822,9 @@ impl NanVixScriptRunner { } /// Returns `true` when filesystem copyback should run. /// Copyback runs on any normal process exit (including non-zero exit codes). - /// It is skipped for preflight, spawn, runtime, and timeout errors, and for - /// OS crashes (negative exit codes from NTSTATUS values). + /// It is skipped for every runner error (policy, availability, setup, spawn, + /// runtime, timeout) and for OS crashes (negative exit codes from NTSTATUS + /// values). fn should_copy_back(response: &ScriptResponse) -> bool { response.error_message.is_empty() && response.exit_code >= 0 } @@ -853,7 +870,7 @@ impl ScriptRunner for NanVixScriptRunner { ) { Ok(s) => s, Err(e) => { - let err = NanVixError::Preflight(e.to_string()); + let err = NanVixError::Setup(e.to_string()); let _ = writeln!(logger, "{}", err); return err.to_response(); } @@ -1184,7 +1201,8 @@ mod tests { }; let mut logger = Logger::new(Mode::Buffer); let resp = runner.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, FailurePhase::Rejected.failure_exit_code()); assert!(resp.error_message.contains(ERR_WORKDIR)); } @@ -1225,7 +1243,8 @@ mod tests { }; let mut logger = Logger::new(Mode::Buffer); let resp = runner.run(&request, &mut logger); - assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, FailurePhase::Rejected.failure_exit_code()); assert!(resp.error_message.contains(ERR_PROXY_POLICY)); } @@ -1338,12 +1357,26 @@ mod tests { // -- NanVixError display tests --------------------------------------------- #[test] - fn error_display_preflight() { - let err = NanVixError::Preflight("missing binary".to_string()); - assert!(err.to_string().contains("preflight")); + fn error_display_policy() { + let err = NanVixError::Policy("workingDirectory".to_string()); + assert!(err.to_string().contains("policy")); + assert!(err.to_string().contains("workingDirectory")); + } + + #[test] + fn error_display_unavailable() { + let err = NanVixError::Unavailable("missing binary".to_string()); + assert!(err.to_string().contains("not available on this host")); assert!(err.to_string().contains("missing binary")); } + #[test] + fn error_display_setup() { + let err = NanVixError::Setup("snapshot home".to_string()); + assert!(err.to_string().contains("setup")); + assert!(err.to_string().contains("snapshot home")); + } + #[test] fn error_display_platform() { let err = NanVixError::Platform("spawn failed".to_string()); @@ -1370,9 +1403,53 @@ mod tests { assert!(msg.contains("5000")); } + #[test] + fn refused_policy_is_reported_as_a_rejection() { + let resp = NanVixError::Policy(ERR_WORKDIR.to_string()).to_response(); + assert_eq!(resp.failure_phase, FailurePhase::Rejected); + assert_eq!(resp.exit_code, 1); + assert!(!resp.error_message.is_empty()); + } + + #[test] + fn missing_runtime_artifacts_blame_the_host() { + let resp = NanVixError::Unavailable("nanvixd not found".to_string()).to_response(); + assert_eq!(resp.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!(resp.exit_code, ERROR_EXIT_CODE); + } + + #[test] + fn run_faults_keep_the_generic_backend_error_code() { + for (err, phase) in [ + ( + NanVixError::Setup("staging".to_string()), + FailurePhase::LaunchFailed, + ), + ( + NanVixError::Platform("spawn".to_string()), + FailurePhase::LaunchFailed, + ), + ( + NanVixError::Runtime("crash".to_string()), + FailurePhase::PostLaunchFailed, + ), + ( + NanVixError::Timeout { + script_timeout_ms: 1, + total_ms: 2, + }, + FailurePhase::Timeout, + ), + ] { + let resp = err.to_response(); + assert_eq!(resp.failure_phase, phase, "{err}"); + assert_eq!(resp.exit_code, ERROR_EXIT_CODE, "{err}"); + } + } + #[test] fn error_to_response_has_error_exit_code() { - let err = NanVixError::Preflight("test".to_string()); + let err = NanVixError::Setup("test".to_string()); let resp = err.to_response(); assert_eq!(resp.exit_code, ERROR_EXIT_CODE); assert!(!resp.error_message.is_empty()); diff --git a/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs b/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs index a3cff3cac..88acd0936 100644 --- a/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs +++ b/src/mxc-sdk/src/backends/process_container/common/appcontainer_runner.rs @@ -85,6 +85,26 @@ const PROCESS_CREATION_ALL_APPLICATION_PACKAGES_OPT_OUT: u32 = 1; /// Proxy-related env var names to strip/override when building the child env block. const PROXY_VAR_NAMES: &[&str] = &["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "ALL_PROXY"]; +const BFSCFG_MISSING_MSG: &str = + "Filesystem policy error: bfscfg.exe is not available on this Windows build, so the \ + AppContainer + BFS filesystem tier cannot enforce your policy. Use an OS build that \ + includes bfscfg.exe, or run on a host that supports the BaseContainer backend (which does \ + not require bfscfg.exe)."; + +/// A failure while bringing the sandbox up, before the child could run. +fn launch_failed(message: &str) -> ScriptResponse { + ScriptResponse::failed(FailurePhase::LaunchFailed, message) +} + +/// Attribute a BFS configuration failure: an absent `bfscfg.exe` is a host that +/// cannot provide this tier at all, not a policy this run could restate. +fn bfs_configure_failure(error: &WxcError) -> ScriptResponse { + match error { + WxcError::BfsNotAvailable => ScriptResponse::unavailable(BFSCFG_MISSING_MSG), + other => launch_failed(&other.to_string()), + } +} + fn create_process_failure( err: &windows_core::Error, command_line: &str, @@ -1559,7 +1579,7 @@ impl AppContainerScriptRunner { } if let Err(e) = self.initialize(request) { - return Err(ScriptResponse::error(&e.to_string())); + return Err(launch_failed(&e.to_string())); } let principal_id = self.get_principal_id(); @@ -1570,7 +1590,9 @@ impl AppContainerScriptRunner { let bfscfg_path = if self.filesystem_mode == FilesystemMode::Bfs { match crate::process_container_common::fallback_detector::find_bfscfg_exe() { Ok(p) => p, - Err(e) => return Err(ScriptResponse::error(&e.to_string())), + // The only failure is an OS that cannot report its own install + // directory, which no request can work around. + Err(e) => return Err(ScriptResponse::unavailable(&e.to_string())), } } else { None @@ -1580,16 +1602,7 @@ impl AppContainerScriptRunner { FileSystemBfsManager::new(self.app_container_name.clone(), bfscfg_path); if self.filesystem_mode == FilesystemMode::Bfs { if let Err(e) = bfs_manager.configure(&request.policy, logger) { - let msg = if matches!(&e, WxcError::BfsNotAvailable) { - "Filesystem policy error: bfscfg.exe is not available on this Windows \ - build, so the AppContainer + BFS filesystem tier cannot enforce your \ - policy. Use an OS build that includes bfscfg.exe, or run on a host that \ - supports the BaseContainer backend (which does not require bfscfg.exe)." - .to_string() - } else { - e.to_string() - }; - return Err(ScriptResponse::error(&msg)); + return Err(bfs_configure_failure(&e)); } } @@ -1640,7 +1653,7 @@ impl AppContainerScriptRunner { self.proxy_address = proxy_coordinator.address().cloned(); } Err(err) => { - return Err(ScriptResponse::error(&err.to_string())); + return Err(launch_failed(&err.to_string())); } } @@ -1779,10 +1792,9 @@ impl SandboxBackend for AppContainerScriptRunner { false, )?; if request.policy.capture_denials.is_some() && self.guarded_capture_factory.is_none() { - return Err(ScriptResponse { - failure_phase: FailurePhase::BackendUnavailable, - ..ScriptResponse::error(CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG) - }); + return Err(ScriptResponse::unavailable( + CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG, + )); } if !request.policy.denied_paths.is_empty() && self.filesystem_mode != FilesystemMode::Dacl @@ -1816,12 +1828,12 @@ impl SandboxBackend for AppContainerScriptRunner { Ok(c) => c, Err(e) => { self.teardown(&mut prepared, request.lifecycle.preserve_policy, logger); - return Err(ScriptResponse::error(&e.to_string())); + return Err(launch_failed(&e.to_string())); } }; if let Err(e) = child.resume() { self.teardown(&mut prepared, request.lifecycle.preserve_policy, logger); - return Err(ScriptResponse::error(&e.to_string())); + return Err(launch_failed(&e.to_string())); } Ok(Box::new(AppContainerSandboxProcess::new( @@ -2271,6 +2283,23 @@ impl Drop for AppContainerSandboxProcess { mod tests { use crate::mxc_common::audit::{TeardownSkipReason, TeardownStatus}; + #[test] + fn a_missing_bfscfg_is_an_unavailable_host_not_a_rejected_policy() { + use super::*; + + let unavailable = bfs_configure_failure(&WxcError::BfsNotAvailable); + assert_eq!(unavailable.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!( + unavailable.exit_code, + FailurePhase::BackendUnavailable.failure_exit_code() + ); + assert!(unavailable.error_message.contains("bfscfg.exe")); + + let launch = bfs_configure_failure(&WxcError::FilesystemPolicy("add failed".to_string())); + assert_eq!(launch.failure_phase, FailurePhase::LaunchFailed); + assert!(launch.error_message.contains("add failed")); + } + #[test] fn appcontainer_network_audit_records_proxyless_proxy_and_start_failure() { use super::*; diff --git a/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs b/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs index 27b83a786..a51342d3e 100644 --- a/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs +++ b/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs @@ -149,6 +149,9 @@ const PSEC_ENUMERATE_PATHS_UNSUPPORTED_MSG: &str = and QueryProcessSecurityEnvironmentSupport to advertise PSE_SUPPORT_FS_ENUMERATE; this OS \ build does not support that policy, and enumeration-only access cannot fall back to \ AppContainer enforcement"; +const PSEC_BASELINE_UNSUPPORTED_MSG: &str = + "this host does not support the baseline Process Security Environment contract version, so \ + the BaseContainer backend cannot be provided"; const CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API: &str = "CreateProcessW(PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT)"; @@ -414,6 +417,23 @@ impl BaseContainerRunner { version } + /// Attribute an unsupported PSEC version: a version raised by a requested + /// policy feature is the request's fault, while an unsupported baseline + /// means the host does not implement the contract at all. + fn unsupported_psec_version_response( + request: &ExecutionRequest, + psec_version: SecurityEnvironmentVersion, + ) -> ScriptResponse { + if psec_version == SecurityEnvironmentVersion::V1_0 { + return ScriptResponse::unavailable(PSEC_BASELINE_UNSUPPORTED_MSG); + } + ScriptResponse::rejected(if !request.policy.enumerate_paths.is_empty() { + PSEC_ENUMERATE_PATHS_UNSUPPORTED_MSG + } else { + "the required Process Security Environment schema version is not supported" + }) + } + /// The single BaseContainer serviceability decision point: return the first /// reason the request cannot be enforced, or /// [`BaseContainerRequestDecision::Serviceable`] when every check passes. @@ -481,20 +501,15 @@ impl BaseContainerRunner { ); let psec_version = Self::choose_min_required_psec_version_for_request(request); - let version_supported = - secenv::supports_version(psec_version).map_err(|error| ScriptResponse { - failure_phase: FailurePhase::BackendUnavailable, - ..ScriptResponse::error(&format!( - "failed to query Process Security Environment contract support: {error}" - )) - })?; + let version_supported = secenv::supports_version(psec_version).map_err(|error| { + ScriptResponse::unavailable(&format!( + "failed to query Process Security Environment contract support: {error}" + )) + })?; if !version_supported { - return Err(ScriptResponse::rejected( - if !request.policy.enumerate_paths.is_empty() { - PSEC_ENUMERATE_PATHS_UNSUPPORTED_MSG - } else { - "the required Process Security Environment schema version is not supported" - }, + return Err(Self::unsupported_psec_version_response( + request, + psec_version, )); } @@ -774,13 +789,7 @@ impl BaseContainerRunner { FailurePhase::LaunchFailed }; self.cleanup_capture_begin_failure(logger); - return Err(ScriptResponse { - exit_code: -1, - error_message: msg.clone(), - standard_err: msg, - failure_phase, - ..Default::default() - }); + return Err(ScriptResponse::failed(failure_phase, &msg)); } } } else { @@ -808,13 +817,7 @@ impl BaseContainerRunner { } else { FailurePhase::LaunchFailed }; - return Err(ScriptResponse { - exit_code: -1, - error_message: msg.clone(), - standard_err: msg, - failure_phase, - ..Default::default() - }); + return Err(ScriptResponse::failed(failure_phase, &msg)); } } } @@ -870,13 +873,7 @@ impl BaseContainerRunner { if capture_denials.is_some() { self.cleanup_capture_begin_failure(logger); } - return Err(ScriptResponse { - exit_code: -1, - error_message: msg.clone(), - standard_err: msg, - failure_phase, - ..Default::default() - }); + return Err(ScriptResponse::failed(failure_phase, &msg)); } }; let environment = (!current_env_ptr.is_null()).then_some(current_env_ptr); @@ -961,12 +958,8 @@ impl BaseContainerRunner { }; return Err(ScriptResponse { - exit_code: -1, - error_message: diag.message.clone(), - standard_err: diag.message, extended_error, - failure_phase, - ..Default::default() + ..ScriptResponse::failed(failure_phase, &diag.message) }); } @@ -1042,12 +1035,8 @@ impl BaseContainerRunner { ); } return Err(ScriptResponse { - exit_code: -1, - error_message: JOB_SETUP_FAILED_MSG.to_string(), - standard_err: JOB_SETUP_FAILED_MSG.to_string(), extended_error, - failure_phase: FailurePhase::LaunchFailed, - ..Default::default() + ..ScriptResponse::failed(FailurePhase::LaunchFailed, JOB_SETUP_FAILED_MSG) }); } }; @@ -1082,10 +1071,7 @@ impl BaseContainerRunner { ); } self.proxy_coordinator.stop(logger); - return Err(ScriptResponse { - failure_phase: FailurePhase::LaunchFailed, - ..ScriptResponse::error(&message) - }); + return Err(ScriptResponse::failed(FailurePhase::LaunchFailed, &message)); } crate::mxc_common::telemetry::log_network_policy_applied( @@ -1191,13 +1177,10 @@ impl SandboxBackend for BaseContainerRunner { if !self.request_serviceability_confirmed && !Self::can_backend_service_request(request).can_service_request() { - return Err(ScriptResponse { - failure_phase: FailurePhase::BackendUnavailable, - ..ScriptResponse::error( - "the request cannot be represented by the process security environment \ - available on this host", - ) - }); + return Err(ScriptResponse::unavailable( + "the request cannot be represented by the process security environment \ + available on this host", + )); } if request.policy.least_privilege_mode { return Err(ScriptResponse::rejected( @@ -2076,6 +2059,36 @@ mod tests { use crate::process_security_environment_spec::process_security_environment_layout as psec_layout; use std::sync::atomic::{AtomicUsize, Ordering}; + /// A host that does not implement the baseline contract cannot be fixed by + /// rewriting the request; a version raised by a requested feature can. + #[test] + fn an_unsupported_psec_baseline_is_unavailable_not_rejected() { + let plain = ExecutionRequest::default(); + let baseline = BaseContainerRunner::unsupported_psec_version_response( + &plain, + SecurityEnvironmentVersion::V1_0, + ); + assert_eq!(baseline.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!( + baseline.exit_code, + FailurePhase::BackendUnavailable.failure_exit_code() + ); + + let enumerate = ExecutionRequest { + policy: ContainerPolicy { + enumerate_paths: vec![r"C:\project".to_string()], + ..Default::default() + }, + ..Default::default() + }; + let raised = BaseContainerRunner::unsupported_psec_version_response( + &enumerate, + SecurityEnvironmentVersion::V1_1, + ); + assert_eq!(raised.failure_phase, FailurePhase::Rejected); + assert_eq!(raised.error_message, PSEC_ENUMERATE_PATHS_UNSUPPORTED_MSG); + } + #[test] fn native_capture_probe_requires_base_container_and_complete_learning_mode_abi() { assert!(!BaseContainerRunner::native_capture_available_with( diff --git a/src/mxc-sdk/src/backends/process_container/common/dispatcher.rs b/src/mxc-sdk/src/backends/process_container/common/dispatcher.rs index 322cfcf5c..c490b8ada 100644 --- a/src/mxc-sdk/src/backends/process_container/common/dispatcher.rs +++ b/src/mxc-sdk/src/backends/process_container/common/dispatcher.rs @@ -75,6 +75,7 @@ use crate::mxc_common::error::WxcError; use crate::mxc_common::filesystem_dacl::{DaclError, DaclManager, RO_MASK, RW_MASK}; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ContainmentBackend, ExecutionRequest, ScriptResponse}; +use crate::mxc_common::mxc_error::{MxcError, MxcErrorCode}; use crate::mxc_common::sandbox_process::{Runner, SandboxBackend, SandboxProcess, StdioMode}; use crate::mxc_common::script_runner::ScriptRunner; use crate::mxc_common::validator::NetworkPolicySupport; @@ -284,6 +285,44 @@ impl std::fmt::Display for DispatchError { impl std::error::Error for DispatchError {} +impl DispatchError { + /// The wire code this dispatch failure is reported under. + /// + /// A field the selected tier cannot represent, or an authored + /// `fallback.allowDaclMutation: false` the tier would have to override, is + /// a refusal of the policy: a changed request can run. The rest are the + /// tier itself being unavailable -- none could be built, or a required + /// provider is absent -- so the policy was never judged, which is the same + /// distinction the [`RejectionReason::RunnerUnavailable`] audit reason + /// draws. A SID that could not be derived carries its own code, and ACEs + /// that could not be applied are a runtime fault. + /// + /// [`RejectionReason::RunnerUnavailable`]: crate::mxc_common::audit::RejectionReason + pub fn error_code(&self) -> MxcErrorCode { + match self { + DispatchError::Fallback( + FallbackError::EnumeratePathsUnsupported + | FallbackError::IngressUnsupported + | FallbackError::DaclFallbackDisabled, + ) => MxcErrorCode::PolicyValidation, + DispatchError::Fallback( + FallbackError::WriteDacUnavailable { .. } + | FallbackError::SystemRootUnresolved { .. }, + ) + | DispatchError::CaptureDenialsUnsupported { .. } + | DispatchError::CaptureDenialsUnavailable { .. } => MxcErrorCode::BackendUnavailable, + DispatchError::Sid(error) => error.error_code(), + DispatchError::Dacl { .. } => MxcErrorCode::BackendError, + } + } + + /// Convert to the typed error carried on the wire, keeping the `Display` + /// text as the message. + pub fn to_mxc_error(&self) -> MxcError { + MxcError::new(self.error_code(), self.to_string()) + } +} + impl From for DispatchError { fn from(e: FallbackError) -> Self { DispatchError::Fallback(e) @@ -820,6 +859,86 @@ mod tests { BcUsableGuard, CaptureCapabilityGuard, ForceTierGuard, ENV_LOCK, }; + #[test] + fn a_field_the_tier_cannot_represent_is_a_refusal_of_the_policy() { + // Documented as a rejection rather than a silent drop, and the caller + // is the one who can act: a request without the field runs. + for error in [ + FallbackError::EnumeratePathsUnsupported, + FallbackError::IngressUnsupported, + FallbackError::DaclFallbackDisabled, + ] { + let dispatch = DispatchError::Fallback(error); + assert_eq!( + dispatch.error_code(), + MxcErrorCode::PolicyValidation, + "{dispatch}" + ); + assert_eq!(dispatch.error_code().exit_code(), 1, "{dispatch}"); + } + } + + #[test] + fn a_tier_that_could_not_be_built_is_not_blamed_on_the_policy() { + // No tier exists to judge the policy against, so reporting a rejection + // would send the caller editing a config that was never the problem. + for error in [ + FallbackError::WriteDacUnavailable { + path: PathBuf::from("C:\\work"), + reason: "access denied".to_string(), + }, + FallbackError::SystemRootUnresolved { + reason: "GetWindowsDirectoryW failed".to_string(), + }, + ] { + let dispatch = DispatchError::Fallback(error); + assert_eq!( + dispatch.error_code(), + MxcErrorCode::BackendUnavailable, + "{dispatch}" + ); + assert_eq!(dispatch.error_code().exit_code(), -1, "{dispatch}"); + } + } + + #[test] + fn a_host_without_a_capture_provider_has_not_rejected_capture_denials() { + // The suites read acceptance as "not rejected during validation", and + // a host with no provider never got as far as judging the section. + for dispatch in [ + DispatchError::CaptureDenialsUnsupported { + tier: IsolationTier::AppContainerDacl, + }, + DispatchError::CaptureDenialsUnavailable { + tier: IsolationTier::AppContainerDacl, + reason: "WPR is not installed".to_string(), + }, + ] { + assert_eq!( + dispatch.error_code(), + MxcErrorCode::BackendUnavailable, + "{dispatch}" + ); + assert_eq!(dispatch.error_code().exit_code(), -1, "{dispatch}"); + } + } + + #[test] + fn a_failed_ace_apply_is_a_runtime_fault() { + let dispatch = DispatchError::Dacl { + error: DaclError::PathNotFound(PathBuf::from("C:\\work")), + warnings: Vec::new(), + }; + assert_eq!(dispatch.error_code(), MxcErrorCode::BackendError); + assert_eq!(dispatch.error_code().exit_code(), -1); + } + + #[test] + fn the_message_the_caller_already_sees_is_the_one_carried_on_the_wire() { + let dispatch = DispatchError::Fallback(FallbackError::IngressUnsupported); + assert_eq!(dispatch.to_mxc_error().message, dispatch.to_string()); + } + fn test_request(policy: ContainerPolicy) -> ExecutionRequest { ExecutionRequest { container_id: "MxcDispatcherTest".to_string(), diff --git a/src/mxc-sdk/src/backends/process_container/common/guarded_capture.rs b/src/mxc-sdk/src/backends/process_container/common/guarded_capture.rs index 37cc119cb..299d1ac73 100644 --- a/src/mxc-sdk/src/backends/process_container/common/guarded_capture.rs +++ b/src/mxc-sdk/src/backends/process_container/common/guarded_capture.rs @@ -25,9 +25,7 @@ use std::path::Path; use crate::learning_mode_core::AnalysisResult; -use crate::mxc_common::models::{ - CaptureDenialsErrorOutput, FailurePhase, SandboxOutputMetadata, ScriptResponse, -}; +use crate::mxc_common::models::{CaptureDenialsErrorOutput, SandboxOutputMetadata, ScriptResponse}; /// Error returned when an injected guarded-capture implementation cannot /// transfer retained ETL requested by the caller. @@ -170,10 +168,7 @@ pub fn validate_retain_etl_supported( native_capture_retains_etl: bool, ) -> Result<(), ScriptResponse> { if retain_etl && !native_capture_retains_etl && !provider_allows_trace_transfer { - return Err(ScriptResponse { - failure_phase: FailurePhase::BackendUnavailable, - ..ScriptResponse::error(RETAIN_ETL_UNSUPPORTED_MSG) - }); + return Err(ScriptResponse::unavailable(RETAIN_ETL_UNSUPPORTED_MSG)); } Ok(()) } @@ -700,7 +695,10 @@ mod tests { #[test] fn retain_etl_validation_rejects_when_no_transfer_and_no_native() { let error = validate_retain_etl_supported(true, false, false).unwrap_err(); - assert_eq!(error.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!( + error.failure_phase, + crate::mxc_common::models::FailurePhase::BackendUnavailable + ); assert_eq!(error.error_message, RETAIN_ETL_UNSUPPORTED_MSG); } diff --git a/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs b/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs index 40aae6bc5..44e0282a0 100644 --- a/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs +++ b/src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs @@ -34,6 +34,23 @@ use crate::mxc_common::host_is_canonical_loopback; use crate::mxc_common::models::{ClipboardPolicy, ContainerPolicy, ExecutionRequest, ProxyAddress}; use crate::seatbelt_common::seatbelt_policy; +/// Why a profile could not be built: the request, or the executor's host. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ProfileError { + /// The request names something the profile cannot express. + Policy(String), + /// The executor's environment lacks something the profile needs. + Host(String), +} + +impl std::fmt::Display for ProfileError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Policy(message) | Self::Host(message) => f.write_str(message), + } + } +} + /// Build a complete Seatbelt sandbox profile, scoping cooperative proxy /// reachability to the resolved address supplied by the runner. /// @@ -44,7 +61,7 @@ use crate::seatbelt_common::seatbelt_policy; pub fn build_profile_with_proxy( request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>, -) -> Result { +) -> Result { build_profile_with_terminals(request, proxy_address, &[]) } @@ -53,7 +70,7 @@ pub(crate) fn build_profile_with_terminals( request: &ExecutionRequest, proxy_address: Option<&ProxyAddress>, terminal_paths: &[PathBuf], -) -> Result { +) -> Result { if let Some(override_profile) = request .seatbelt .as_ref() @@ -81,9 +98,9 @@ pub(crate) fn build_profile_with_terminals( // Keep the controlling terminal usable without granting other host PTYs. out.push_str(TTY_ALLOW); for path in terminal_paths { - let path = path - .to_str() - .ok_or_else(|| "Seatbelt: assigned terminal path is not valid UTF-8".to_string())?; + let path = path.to_str().ok_or_else(|| { + ProfileError::Host("Seatbelt: assigned terminal path is not valid UTF-8".into()) + })?; let _ = writeln!( out, "(allow file-read* file-write* file-ioctl (literal {}))", @@ -294,7 +311,7 @@ struct ResolvedPaths { } impl ResolvedPaths { - fn from_policy(policy: &ContainerPolicy) -> Result { + fn from_policy(policy: &ContainerPolicy) -> Result { let denied = resolve_all(&policy.denied_paths)?; let readonly = resolve_all(&policy.readonly_paths)?; let mut readwrite = resolve_all(&policy.readwrite_paths)?; @@ -314,7 +331,7 @@ impl ResolvedPaths { } } -fn resolve_all(paths: &[String]) -> Result, String> { +fn resolve_all(paths: &[String]) -> Result, ProfileError> { paths.iter().map(|p| resolve_policy_path(p)).collect() } @@ -644,7 +661,7 @@ fn write_nested_pty_rules(out: &mut String, request: &ExecutionRequest) { /// stores under `/Library/Keychains` and `/System/Library/Keychains` /// are already covered by the baseline `/Library` and `/System` /// read-only allows, so we don't re-add them here. -fn write_keychain_rules(out: &mut String, request: &ExecutionRequest) -> Result<(), String> { +fn write_keychain_rules(out: &mut String, request: &ExecutionRequest) -> Result<(), ProfileError> { let enabled = request.seatbelt.as_ref().is_some_and(|c| c.keychain_access); if !enabled { return Ok(()); @@ -681,8 +698,11 @@ fn write_keychain_rules(out: &mut String, request: &ExecutionRequest) -> Result< out.push_str(" (subpath \"/private/var/db/mds\"))\n"); let home = std::env::var("HOME").map_err(|_| { - "HOME environment variable not set; cannot expand '~/Library/Keychains' for keychainAccess" - .to_string() + ProfileError::Host( + "HOME environment variable not set; cannot expand '~/Library/Keychains' for \ + keychainAccess" + .to_string(), + ) })?; let user_keychains = format!("{home}/Library/Keychains"); out.push_str(";; --- keychainAccess: user keychain DB + XPC/folder caches (read+write) ---\n"); @@ -716,9 +736,10 @@ fn write_extra_seatbelt_rules(out: &mut String, request: &ExecutionRequest) { /// /// All three steps are required. See [`expand_tilde`], [`normalize_lexical`] /// and [`resolve_macos_root_symlinks`]. -pub(crate) fn resolve_policy_path(path: &str) -> Result { - let expanded = expand_tilde(path)?; - Ok(resolve_macos_root_symlinks(&normalize_lexical(&expanded)?)) +pub(crate) fn resolve_policy_path(path: &str) -> Result { + let expanded = expand_tilde(path).map_err(ProfileError::Host)?; + let normalized = normalize_lexical(&expanded).map_err(ProfileError::Policy)?; + Ok(resolve_macos_root_symlinks(&normalized)) } /// Collapse the lexical spellings of a path that leave a Seatbelt rule dead: @@ -838,7 +859,7 @@ mod tests { use super::*; use crate::mxc_common::models::{NetworkAction, SeatbeltConfig, UiPolicy}; - fn build_profile(request: &ExecutionRequest) -> Result { + fn build_profile(request: &ExecutionRequest) -> Result { build_profile_with_proxy(request, request.policy.network_proxy.address.as_ref()) } @@ -1527,7 +1548,10 @@ mod tests { // would silently widen an allow and leave a deny dead. for path in ["/private/var/../tmp", "/tmp/..", "/..", "/a/b/../c"] { let err = resolve_policy_path(path).unwrap_err(); - assert!(err.contains(".."), "{path} must be rejected, got {err}"); + assert!( + matches!(&err, ProfileError::Policy(message) if message.contains("..")), + "{path} must be rejected as policy, got {err:?}" + ); } // A path merely *containing* dots in a segment name is fine. assert_eq!( diff --git a/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs b/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs index e53f46af1..e1e9a5d61 100644 --- a/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs +++ b/src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs @@ -27,7 +27,7 @@ use std::time::{Duration, Instant}; use crate::mxc_common::interruptible_reader::{wrap_pipe, InterruptibleReader, ReadCanceller}; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::{ExecutionRequest, ProxyAddress, ScriptResponse}; +use crate::mxc_common::models::{ExecutionRequest, FailurePhase, ProxyAddress, ScriptResponse}; use crate::mxc_common::sandbox_process::{ boxed_closer, cancel_and_join_discard, duplicate_and_take_native_stdio, group_kill, spawn_discard, take_boxed_read, take_boxed_write, wait_with_timeout, NativeStdio, PtySize, @@ -43,7 +43,7 @@ use crate::seatbelt_common::default_env::DEFAULT_SANDBOX_PATH; use crate::seatbelt_common::default_env::{env_pairs, resolved_env}; #[cfg(test)] use crate::seatbelt_common::profile_builder::build_profile_with_proxy; -use crate::seatbelt_common::profile_builder::build_profile_with_terminals; +use crate::seatbelt_common::profile_builder::{build_profile_with_terminals, ProfileError}; #[link(name = "proc")] unsafe extern "C" { @@ -282,9 +282,9 @@ impl SandboxBackend for SeatbeltScriptRunner { // Seatbelt's own invariants — the only home for them, so a caller that // builds an ExecutionRequest directly gets the same rules. crate::seatbelt_common::seatbelt_policy::validate_seatbelt_network_policy(&request.policy) - .map_err(error_response)?; + .map_err(rejected_response)?; crate::seatbelt_common::seatbelt_policy::validate_seatbelt_ui_policy(request) - .map_err(error_response)?; + .map_err(rejected_response)?; Ok(()) } @@ -340,7 +340,7 @@ fn spawn_exec( } else { "a caller-owned PTY" }; - return Err(error_response(format!( + return Err(rejected_response(format!( "Seatbelt guiAccess requires inherited stdio and cannot use {mode}" ))); } @@ -382,7 +382,7 @@ fn spawn_exec( ) { Ok(cwd) => cwd, Err(e) => { - return Err(error_response(format!( + return Err(launch_failed_response(format!( "failed to read the current directory to anchor the relative seatbelt working directory: {e}" ))) } @@ -417,7 +417,7 @@ fn spawn_exec( let mut child = command .spawn() - .map_err(|error| error_response(spawn_error(&error)))?; + .map_err(|error| launch_failed_response(spawn_error(&error)))?; let (stdin, stdout, stderr) = match stdio { StdioMode::Pipes => (child.stdin.take(), child.stdout.take(), child.stderr.take()), @@ -438,7 +438,7 @@ fn spawn_exec( let _ = child.kill(); let _ = child.wait(); let error = out_result.err().or(err_result.err()); - return Err(error_response(format!( + return Err(launch_failed_response(format!( "Seatbelt: failed to wrap stdio pipes: {}", error.map_or_else(|| "unknown error".to_string(), |e| e.to_string()), ))); @@ -834,24 +834,27 @@ fn build_sandbox_command( }, &[], ) - .map_err(|error| error_response(format!("Seatbelt: failed to allocate PTY: {error}"))) + .map_err(|error| { + launch_failed_response(format!("Seatbelt: failed to allocate PTY: {error}")) + }) }) .transpose()?; let terminal_paths = match &pty { Some(pty) => vec![pty.secondary_path().to_path_buf()], None if !new_session => inherited_terminal_paths().map_err(|error| { - error_response(format!( + launch_failed_response(format!( "Seatbelt: failed to resolve inherited terminals: {error}" )) })?, None => Vec::new(), }; let profile = build_profile_with_terminals(request, proxy_address, &terminal_paths) - .map_err(error_response)?; + .map_err(profile_error_response)?; log_generated_profile(&profile, logger); - let profile_cstr = CString::new(profile) - .map_err(|e| error_response(format!("seatbelt profile contains embedded NUL byte: {e}")))?; + let profile_cstr = CString::new(profile).map_err(|e| { + rejected_response(format!("seatbelt profile contains embedded NUL byte: {e}")) + })?; let _ = writeln!(logger, "Seatbelt: applying sandbox via sandbox_init"); // SAFETY: The closure runs after fork(), before exec(). We only call @@ -910,11 +913,22 @@ fn log_generated_profile(profile: &str, logger: &mut Logger) { let _ = writeln!(logger, "{PROFILE_LOG_END}"); } -fn error_response(message: String) -> ScriptResponse { - ScriptResponse { - exit_code: -1, - error_message: message, - ..Default::default() +/// A request this backend refuses: the host is fine, so only a changed request +/// can succeed. +fn rejected_response(message: String) -> ScriptResponse { + ScriptResponse::rejected(&message) +} + +/// A fault while bringing the sandboxed child up, after the request was +/// accepted. +fn launch_failed_response(message: String) -> ScriptResponse { + ScriptResponse::failed(FailurePhase::LaunchFailed, &message) +} + +fn profile_error_response(error: ProfileError) -> ScriptResponse { + match error { + ProfileError::Policy(message) => rejected_response(message), + ProfileError::Host(message) => launch_failed_response(message), } } @@ -1219,6 +1233,19 @@ os.close(fd) .err() .expect("guiAccess PTY must be rejected"); assert!(error.error_message.contains("caller-owned PTY")); + assert_eq!(error.failure_phase, FailurePhase::Rejected); + assert_eq!(error.exit_code, FailurePhase::Rejected.failure_exit_code()); + } + + #[test] + fn a_failed_launch_keeps_the_generic_backend_error_code() { + let response = launch_failed_response("failed to spawn sandboxed process".into()); + assert_eq!(response.failure_phase, FailurePhase::LaunchFailed); + assert_eq!( + response.failure_phase.error_code(), + crate::mxc_common::mxc_error::MxcErrorCode::BackendError + ); + assert_eq!(response.exit_code, -1); } #[test] @@ -1401,6 +1428,11 @@ os.close(fd) assert!(response.error_message.contains("hostLoopback='allow'")); assert!(response.error_message.contains("default='deny'")); assert!(!response.error_message.contains("inherit 'default'")); + assert_eq!(response.failure_phase, FailurePhase::Rejected); + assert_eq!( + response.exit_code, + FailurePhase::Rejected.failure_exit_code() + ); } /// The other divergent pair is expressible and must be accepted: it is the diff --git a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/error.rs b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/error.rs index cbe9d4186..227888e09 100644 --- a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/error.rs +++ b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/error.rs @@ -59,12 +59,9 @@ impl OneShotError { }; ScriptResponse { - exit_code: -1, standard_err: summary.to_string(), - error_message: summary.to_string(), extended_error: detail, - failure_phase, - ..Default::default() + ..ScriptResponse::failed(failure_phase, summary) } } @@ -72,10 +69,11 @@ impl OneShotError { /// whether a retry could ever succeed. fn failure_phase(&self) -> FailurePhase { match self { - // Non-retryable preflight: the request/config cannot be honored or a - // required host prerequisite is missing. Retrying the same input on - // the same host will not succeed. - OneShotError::SandboxUnavailable(_) | OneShotError::Policy(_) => FailurePhase::Rejected, + // The optional OS feature is absent or disabled, so the host has to + // be provisioned; no change to the request reaches it. + OneShotError::SandboxUnavailable(_) => FailurePhase::BackendUnavailable, + // The request asks for something this backend will not do. + OneShotError::Policy(_) => FailurePhase::Rejected, // Launch attempt failed (incl. transient single-instance contention, // async-runtime setup, capture-proof, rendezvous wait, and the // initial guest connect) — generally worth retrying. @@ -91,6 +89,7 @@ impl OneShotError { #[cfg(test)] mod tests { use super::*; + use crate::mxc_common::mxc_error::MxcErrorCode; #[test] fn busy_maps_to_launch_failed_with_detail() { @@ -102,18 +101,23 @@ mod tests { } #[test] - fn policy_and_prereq_errors_map_to_rejected() { - for err in [ - OneShotError::Policy("denied path in share".to_string()), - OneShotError::SandboxUnavailable("feature off".to_string()), - ] { - let resp = err.into_response(); - assert_eq!( - resp.failure_phase, - FailurePhase::Rejected, - "non-retryable preflight should map to Rejected" - ); - } + fn a_refused_policy_is_told_apart_from_a_host_that_cannot_run_it() { + let refused = OneShotError::Policy("denied path in share".to_string()).into_response(); + assert_eq!(refused.failure_phase, FailurePhase::Rejected); + assert_eq!(refused.exit_code, 1); + assert_eq!( + refused.failure_phase.error_code(), + MxcErrorCode::PolicyValidation + ); + + let unavailable = + OneShotError::SandboxUnavailable("feature off".to_string()).into_response(); + assert_eq!(unavailable.failure_phase, FailurePhase::BackendUnavailable); + assert_eq!(unavailable.exit_code, -1); + assert_eq!( + unavailable.failure_phase.error_code(), + MxcErrorCode::BackendUnavailable + ); } #[test] diff --git a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs index 8ca669f7d..0e106ee8d 100644 --- a/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs +++ b/src/mxc-sdk/src/backends/windows_sandbox/lifecycle/state_aware.rs @@ -210,10 +210,12 @@ fn is_valid_sandbox_token(token: &str) -> bool { } /// Map a [`OneShotError`] from policy planning to the wire error model. Policy -/// rejections become `policy_validation`; anything else is a backend error. +/// rejections become `policy_validation`, a missing host feature becomes +/// `backend_unavailable`, and anything else is a backend error. fn map_policy_error(e: OneShotError) -> MxcError { match e { OneShotError::Policy(msg) => MxcError::policy_validation(msg), + OneShotError::SandboxUnavailable(msg) => MxcError::backend_unavailable(msg), other => MxcError::backend_error(format!("{other:?}")), } } @@ -1325,6 +1327,12 @@ mod tests { assert_eq!(err.code, MxcErrorCode::PolicyValidation); } + #[test] + fn map_policy_error_blames_the_host_for_a_missing_sandbox_feature() { + let err = map_policy_error(OneShotError::SandboxUnavailable("feature off".into())); + assert_eq!(err.code, MxcErrorCode::BackendUnavailable); + } + #[test] fn validate_start_rejects_non_default_network() { let backend = WindowsSandboxRunner::new(); diff --git a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs index 02901d143..c7526b3bf 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -792,14 +792,15 @@ pub unsafe fn load_sdk_checked(logger: &mut Logger) -> Result, pub store_may_be_held: bool, } impl From for SessionCreateFailure { fn from(response: ScriptResponse) -> Self { Self { - response, + response: Box::new(response), store_may_be_held: false, } } @@ -875,7 +876,7 @@ pub unsafe fn create_daemon_session( } }; return Err(SessionCreateFailure { - response, + response: Box::new(response), store_may_be_held, }); } diff --git a/src/mxc-sdk/src/backends/wslc/common/error.rs b/src/mxc-sdk/src/backends/wslc/common/error.rs index 5a3c9f5b2..4e284b676 100644 --- a/src/mxc-sdk/src/backends/wslc/common/error.rs +++ b/src/mxc-sdk/src/backends/wslc/common/error.rs @@ -11,10 +11,8 @@ //! the message. Each variant here attributes the failure to a lifecycle phase //! instead. //! -//! `Display` reproduces the pre-existing message verbatim and -//! [`WslcError::into_response`] fills the same fields `ScriptResponse::error` -//! did, so the text a user sees is unchanged; `failure_phase` is the only -//! addition. +//! `Display` reproduces the pre-existing message verbatim, so the text a user +//! sees is unchanged. use std::fmt; @@ -68,18 +66,9 @@ impl WslcError { } } - /// Render as a [`ScriptResponse`], preserving the exact message text the - /// untyped construction produced. + /// Render as a [`ScriptResponse`] carrying the message verbatim. pub(crate) fn into_response(self) -> ScriptResponse { - let failure_phase = self.failure_phase(); - let msg = self.to_string(); - ScriptResponse { - exit_code: -1, - standard_err: msg.clone(), - error_message: msg, - failure_phase, - ..Default::default() - } + ScriptResponse::failed(self.failure_phase(), &self.to_string()) } } @@ -161,16 +150,34 @@ mod tests { } #[test] - fn response_matches_the_untyped_shape_apart_from_the_phase() { - let typed = WslcError::Rejected("bad path".to_string()).into_response(); - let untyped = ScriptResponse::error("bad path"); - - assert_eq!(typed.exit_code, untyped.exit_code); - assert_eq!(typed.standard_err, untyped.standard_err); - assert_eq!(typed.error_message, untyped.error_message); - // The untyped helper left this at the default, which is the bug. - assert_eq!(untyped.failure_phase, FailurePhase::None); - assert_eq!(typed.failure_phase, FailurePhase::Rejected); + fn response_carries_the_message_and_the_phase() { + let typed = WslcError::Host("bring-up failed".to_string()).into_response(); + + assert_eq!(typed.exit_code, -1); + assert_eq!(typed.error_message, "bring-up failed"); + assert_eq!(typed.failure_phase, FailurePhase::LaunchFailed); + } + + /// A WSLc rejection has to cost the caller the same exit code a rejection + /// from any other backend does. + #[test] + fn exit_codes_follow_the_shared_phase_contract() { + let s = || "x".to_string(); + assert_eq!( + WslcError::Rejected(s()).into_response().exit_code, + ScriptResponse::rejected("x").exit_code + ); + assert_eq!( + WslcError::Unavailable(s()).into_response().exit_code, + ScriptResponse::unavailable("x").exit_code + ); + for err in [ + WslcError::Host(s()), + WslcError::Runtime(s()), + WslcError::sdk("op", -1, ""), + ] { + assert_eq!(err.into_response().exit_code, -1); + } } #[test] diff --git a/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs b/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs index d26035b9e..96c75ec64 100644 --- a/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs +++ b/src/mxc-sdk/src/backends/wslc/common/policy_mapping.rs @@ -10,6 +10,7 @@ //! injected, so its logic stays testable without touching disk. use crate::mxc_common::filesystem_canonical::{canonicalize_allowing_absent_tail, PathCanonical}; +use crate::mxc_common::filesystem_object::FilesystemPolicyError; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ContainerPolicy, ExecutionRequest}; use crate::wslc_common::wslc_bindings::WslcContainerNetworkingMode; @@ -331,22 +332,24 @@ pub fn validate_denied_path_overlap( /// /// Returns the tightened policy when normalization changed something (the caller /// rebuilds the request around it) or `None` when the policy was already -/// conflict-free. Any failure is returned as a `String`; callers map it to their -/// own error envelope (`ScriptResponse` / `MxcError`). Normalization diagnostics +/// conflict-free. Callers map a failure to their own error envelope +/// (`ScriptResponse` / `MxcError`). Normalization diagnostics /// are written to `logger`; callers decide how to surface them. pub fn apply_provision_policy_gate( request: &ExecutionRequest, logger: &mut Logger, -) -> Result, String> { +) -> Result, FilesystemPolicyError> { let normalized = crate::mxc_common::filesystem_object::normalize_object_conflicts(&request.policy, logger)?; let effective = normalized.as_ref().unwrap_or(&request.policy); - crate::mxc_common::filesystem_access::check_delegation(effective)?; + crate::mxc_common::filesystem_access::check_delegation(effective) + .map_err(FilesystemPolicyError::Policy)?; validate_denied_path_overlap( &effective.readwrite_paths, &effective.readonly_paths, &effective.denied_paths, - )?; + ) + .map_err(FilesystemPolicyError::Policy)?; Ok(normalized) } diff --git a/src/mxc-sdk/src/backends/wslc/common/state_aware.rs b/src/mxc-sdk/src/backends/wslc/common/state_aware.rs index 6b8a994be..1b7318ce5 100644 --- a/src/mxc-sdk/src/backends/wslc/common/state_aware.rs +++ b/src/mxc-sdk/src/backends/wslc/common/state_aware.rs @@ -21,6 +21,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; #[cfg(windows)] use std::sync::Arc; +use crate::mxc_common::filesystem_object::FilesystemPolicyError; use crate::mxc_common::logger::{Logger, Mode}; use crate::mxc_common::models::{ ContainerPolicy, ExecutionRequest, WslcProvisionConfig, WslcProvisionSessionConfig, @@ -683,9 +684,10 @@ fn build_provision_config( /// ([`crate::wslc_common::policy_mapping::apply_provision_policy_gate`]): runs the full /// three-step gate (D6 normalization → D3 delegation → denied-path overlap), /// buffering normalization diagnostics and surfacing them on stderr (stdout -/// carries the phase envelope), and maps the gate's `String` error to a -/// `policy_validation` [`MxcError`]. Returns the tightened policy when -/// normalization changed something, else `None`. +/// carries the phase envelope), and maps a gate failure to a `policy_validation` +/// or, for a path that could not be examined, a `backend_error` [`MxcError`]. +/// Returns the tightened policy when normalization changed something, else +/// `None`. fn normalize_and_check_delegation( request: &ExecutionRequest, ) -> Result, MxcError> { @@ -698,7 +700,10 @@ fn normalize_and_check_delegation( if !notes.is_empty() { eprint!("{notes}"); } - result.map_err(MxcError::policy_validation) + result.map_err(|error| match error { + FilesystemPolicyError::Policy(message) => MxcError::policy_validation(message), + FilesystemPolicyError::Host(message) => MxcError::backend_error(message), + }) } /// Build daemon volume mounts from the request's filesystem policy. Overlapping diff --git a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs index 51172a6f4..645328371 100644 --- a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs +++ b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs @@ -23,6 +23,7 @@ use std::ptr; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Condvar, Mutex}; +use crate::mxc_common::filesystem_object::FilesystemPolicyError; use crate::mxc_common::logger::{Logger, Mode}; use crate::mxc_common::models::{ExecutionRequest, ScriptResponse, WslcConfig}; use crate::mxc_common::mxc_error::MxcError; @@ -846,9 +847,13 @@ impl WSLContainerRunner { &normalized } Ok(None) => request, - Err(msg) => { - let _ = writeln!(logger, "[WSLC] {}", msg); - return Err(WslcError::Rejected(msg).into_response()); + Err(error) => { + let _ = writeln!(logger, "[WSLC] {}", error.message()); + return Err(match error { + FilesystemPolicyError::Policy(message) => WslcError::Rejected(message), + FilesystemPolicyError::Host(message) => WslcError::Host(message), + } + .into_response()); } }; @@ -1643,7 +1648,15 @@ mod tests { let mut runner = WSLContainerRunner::new(&WslcConfig::default()); let response = runner.execute(&request, &mut logger); - assert_eq!(response.exit_code, -1, "overlap must fail the run"); + assert_eq!( + response.exit_code, + crate::mxc_common::models::FailurePhase::Rejected.failure_exit_code(), + "overlap must fail the run" + ); + assert_eq!( + response.failure_phase, + crate::mxc_common::models::FailurePhase::Rejected + ); assert!( response.error_message.contains("cannot be enforced"), "expected the overlap error, got: {}", @@ -1692,9 +1705,14 @@ mod tests { let response = runner.execute(&request, &mut logger); assert_eq!( - response.exit_code, -1, + response.exit_code, + crate::mxc_common::models::FailurePhase::Rejected.failure_exit_code(), "junction-aliased deny must fail the run" ); + assert_eq!( + response.failure_phase, + crate::mxc_common::models::FailurePhase::Rejected + ); assert!( response.error_message.contains("cannot be enforced"), "expected the overlap error, got: {}", @@ -1742,9 +1760,14 @@ mod tests { let response = runner.execute(&request, &mut logger); assert_eq!( - response.exit_code, -1, + response.exit_code, + crate::mxc_common::models::FailurePhase::Rejected.failure_exit_code(), "absent junction-aliased deny must fail the run" ); + assert_eq!( + response.failure_phase, + crate::mxc_common::models::FailurePhase::Rejected + ); assert!( response.error_message.contains("cannot be enforced"), "expected the overlap error, got: {}", diff --git a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs index fb99ee622..5c8d80512 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs @@ -1012,7 +1012,7 @@ fn start_session_boot( } .map(BootedSession) .map_err(|failure| BootFailure { - error: sr_err(failure.response), + error: sr_err(*failure.response), store_may_be_held: failure.store_may_be_held, }) } diff --git a/src/mxc-sdk/src/core/mxc_common/config_parser.rs b/src/mxc-sdk/src/core/mxc_common/config_parser.rs index 622e56149..10fd10789 100644 --- a/src/mxc-sdk/src/core/mxc_common/config_parser.rs +++ b/src/mxc-sdk/src/core/mxc_common/config_parser.rs @@ -24,9 +24,10 @@ use crate::mxc_contract::{probe_version, supported_versions, ContractVersion, Ve use std::fs; /// Categorised error from `load_mxc_request`. The `wxc-exec` driver uses the -/// variant to choose the failure-output convention: state-aware failures -/// emit a JSON `{"error": ...}` envelope on stdout, while one-shot and -/// pre-discrimination failures keep the existing diagnostic-on-stderr path. +/// variant to choose where the failure is written: state-aware failures emit a +/// JSON `{"error": ...}` envelope on stdout, because there the envelope *is* +/// the operation's response document, while one-shot and pre-discrimination +/// failures emit theirs on stderr alongside the diagnostics. #[derive(Debug)] pub enum ParseError { /// I/O, base64-decode, or top-level JSON parse failure — the input could @@ -44,6 +45,23 @@ pub enum ParseError { StateAware(MxcError), } +impl ParseError { + /// The typed error this failure is reported to the caller as. + /// + /// Every variant resolves to a code, so a request that never reached a + /// backend is still classified rather than being reported as an + /// unexplained non-zero exit. + pub fn to_mxc_error(&self) -> MxcError { + match self { + ParseError::Decode(error) + | ParseError::Version(error) + | ParseError::OneShot(error) + | ParseError::OneShotMalformed(error) => error.to_mxc_error(), + ParseError::StateAware(error) => error.clone(), + } + } +} + #[derive(Debug, Clone, Copy)] enum ErrorOutput { Primary, diff --git a/src/mxc-sdk/src/core/mxc_common/error.rs b/src/mxc-sdk/src/core/mxc_common/error.rs index 37cb5ed5e..f7a83e6d7 100644 --- a/src/mxc-sdk/src/core/mxc_common/error.rs +++ b/src/mxc-sdk/src/core/mxc_common/error.rs @@ -3,7 +3,7 @@ use thiserror::Error; -use crate::mxc_common::mxc_error::MxcError; +use crate::mxc_common::mxc_error::{MxcError, MxcErrorCode}; #[derive(Debug, Error)] pub enum WxcError { @@ -62,6 +62,42 @@ impl From for WxcError { } } +impl WxcError { + /// The wire code this failure is reported under. + /// + /// The grouping is by *whose* problem it is, because that is what decides + /// what the caller can do next: fix the request, fix the host, or report a + /// bug. `BfsNotAvailable` is the one that is easy to get wrong -- nothing + /// the caller writes makes `bfscfg.exe` appear, so it is a property of the + /// host rather than a judgement of the policy. + pub fn error_code(&self) -> MxcErrorCode { + match self { + WxcError::StateAware(error) => error.code, + WxcError::ConfigParse(_) | WxcError::StringConversion(_) => { + MxcErrorCode::MalformedRequest + } + WxcError::Validation(_) => MxcErrorCode::PolicyValidation, + WxcError::BfsNotAvailable => MxcErrorCode::BackendUnavailable, + WxcError::Process(_) + | WxcError::Io(_) + | WxcError::Firewall(_) + | WxcError::FilesystemPolicy(_) + | WxcError::Initialization(_) + | WxcError::NetworkProxy(_) => MxcErrorCode::BackendError, + } + } + + /// Convert to the typed error carried on the wire, keeping the `Display` + /// text as the message so the variant's prefix survives. A wrapped + /// [`MxcError`] is returned unchanged. + pub fn to_mxc_error(&self) -> MxcError { + match self { + WxcError::StateAware(error) => (**error).clone(), + error => MxcError::new(error.error_code(), error.to_string()), + } + } +} + impl From for WxcError { fn from(err: std::io::Error) -> Self { WxcError::Io(err.to_string()) @@ -82,3 +118,39 @@ pub const DENIED_PATHS_FEATURE_DISABLED_MSG: &str = QueryProcessSecurityEnvironmentSupport. Run on a build with \ BaseContainer deny support, or use the ProcessContainer dispatcher so it can select \ the AppContainer + DACL fallback (which enforces deniedPaths via DENY ACEs)."; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_missing_host_tool_is_not_reported_as_a_bad_policy() { + assert_eq!( + WxcError::BfsNotAvailable.error_code(), + MxcErrorCode::BackendUnavailable + ); + assert_eq!( + WxcError::Validation("denied".into()).error_code(), + MxcErrorCode::PolicyValidation + ); + } + + #[test] + fn an_unreadable_request_is_told_apart_from_a_refused_one() { + assert_eq!( + WxcError::ConfigParse("bad json".into()).error_code(), + MxcErrorCode::MalformedRequest + ); + assert_eq!( + WxcError::Io("disk gone".into()).error_code(), + MxcErrorCode::BackendError + ); + } + + #[test] + fn conversion_keeps_the_variants_prefix_in_the_message() { + let error = WxcError::Validation("portRange is empty".into()).to_mxc_error(); + assert_eq!(error.code, MxcErrorCode::PolicyValidation); + assert_eq!(error.message, "Validation error: portRange is empty"); + } +} diff --git a/src/mxc-sdk/src/core/mxc_common/filesystem_object.rs b/src/mxc-sdk/src/core/mxc_common/filesystem_object.rs index 3e634a668..6c96460c9 100644 --- a/src/mxc-sdk/src/core/mxc_common/filesystem_object.rs +++ b/src/mxc-sdk/src/core/mxc_common/filesystem_object.rs @@ -25,8 +25,10 @@ //! denied, unreachable mount, I/O error — as opposed to *cleanly missing*) and //! `deniedPaths` are present, MXC cannot prove that path is not an undetectable //! alias that would bypass a deny. Rather than proceed looser than intended, the -//! config is **rejected** (fail closed). Cleanly-missing paths do not trigger -//! this — they are genuinely absent, so there is nothing to alias. +//! run is **refused** (fail closed): a path the caller cannot access is a policy +//! rejection, and any other unexaminable path is a host fault. Cleanly-missing +//! paths do not trigger this — they are genuinely absent, so there is nothing to +//! alias. //! //! This does file I/O (`stat`/`CreateFile`), so — per design review — it lives //! here in `mxc_common` and is invoked by each backend runner close to the @@ -35,7 +37,7 @@ //! and the backend actually building its mounts. use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::ContainerPolicy; +use crate::mxc_common::models::{ContainerPolicy, FailurePhase, ScriptResponse}; use std::path::Path; /// Intent class for a policy path, ordered least → most restrictive so that @@ -89,12 +91,37 @@ enum PathResolution { /// Nothing to alias; safe to skip. (Existence is surfaced separately by the /// parse-time existence warning.) Absent, + /// The caller is denied access to the path, so its identity cannot be read. + Inaccessible, /// The path exists (or may exist) but its identity could not be determined — - /// permission denied, an unreachable mount, or another I/O error. This is - /// the fail-closed trigger when `deniedPaths` are present. + /// an unreachable mount or another I/O error. Unknown, } +/// Why a filesystem policy was refused. +#[derive(Debug, PartialEq, Eq)] +pub enum FilesystemPolicyError { + /// The request is at fault, e.g. a policy path the caller cannot access. + Policy(String), + /// A policy path could not be examined; the same request may succeed later. + Host(String), +} + +impl FilesystemPolicyError { + pub fn message(&self) -> &str { + match self { + Self::Policy(message) | Self::Host(message) => message, + } + } + + pub fn into_response(self) -> ScriptResponse { + match self { + Self::Policy(message) => ScriptResponse::rejected(&message), + Self::Host(message) => ScriptResponse::failed(FailurePhase::LaunchFailed, &message), + } + } +} + /// Result of comparing two paths by filesystem-object identity. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum ExistingObjectComparison { @@ -128,6 +155,7 @@ fn resolve_object(path: &Path) -> PathResolution { // could not examine the path. Err(e) => match e.raw_os_error() { Some(libc::ENOENT) | Some(libc::ENOTDIR) => PathResolution::Absent, + Some(libc::EACCES) | Some(libc::EPERM) => PathResolution::Inaccessible, _ => PathResolution::Unknown, }, } @@ -138,7 +166,7 @@ fn resolve_object(path: &Path) -> PathResolution { use std::os::windows::ffi::OsStrExt; use windows::core::PCWSTR; use windows::Win32::Foundation::{ - CloseHandle, GetLastError, ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, + CloseHandle, GetLastError, ERROR_ACCESS_DENIED, ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, }; use windows::Win32::Storage::FileSystem::{ CreateFileW, FileIdInfo, GetFileInformationByHandleEx, FILE_FLAG_BACKUP_SEMANTICS, @@ -178,6 +206,8 @@ fn resolve_object(path: &Path) -> PathResolution { let err = unsafe { GetLastError() }; return if err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND { classify_not_found_leaf_windows(path) + } else if err == ERROR_ACCESS_DENIED { + PathResolution::Inaccessible } else { PathResolution::Unknown }; @@ -253,7 +283,8 @@ fn compare_resolved_filesystem_objects( // Unknown must take precedence over Absent on every platform. Treating // an unexaminable path as merely different would let callers proceed // without proving that the policy paths refer to distinct objects. - (PathResolution::Unknown, _) | (_, PathResolution::Unknown) => { + (PathResolution::Unknown | PathResolution::Inaccessible, _) + | (_, PathResolution::Unknown | PathResolution::Inaccessible) => { ExistingObjectComparison::Unknown } (PathResolution::Absent, _) | (_, PathResolution::Absent) => { @@ -277,18 +308,17 @@ fn compare_resolved_filesystem_objects( /// - `Ok(None)` when there is nothing to change (the common case — no /// symlink / hard-link / bind-mount aliases), so callers can avoid cloning the /// request entirely. -/// - `Err(message)` when the config must be **rejected**: a path could not be -/// resolved to an object (permission denied, unreachable mount, I/O error — -/// *not* cleanly missing) while `deniedPaths` are present, so an alias that -/// would bypass a deny cannot be ruled out (fail closed). The message is -/// suitable for surfacing as a backend error. +/// - `Err(_)` when the run must be **refused**: a path could not be resolved to +/// an object (permission denied, unreachable mount, I/O error — *not* cleanly +/// missing) while `deniedPaths` are present, so an alias that would bypass a +/// deny cannot be ruled out (fail closed). /// /// Run this *after* the string-level normalization in `config_parser`, close to /// where the backend builds its mounts. pub fn normalize_object_conflicts( policy: &ContainerPolicy, logger: &mut Logger, -) -> Result, String> { +) -> Result, FilesystemPolicyError> { let total = policy.readwrite_paths.len() + policy.readonly_paths.len() + policy.denied_paths.len(); // A conflict needs at least two paths resolving to the same object, so a @@ -314,10 +344,11 @@ pub fn normalize_object_conflicts( // Group entry indices by object identity. Resolution has three outcomes: // - `Object`: grouped for conflict detection. // - `Absent` (cleanly missing): skipped — nothing to alias. - // - `Unknown` (exists/maybe-exists but unexaminable): when `deniedPaths` are - // present we cannot prove this path is not an undetectable alias that would - // bypass a deny, so we FAIL CLOSED and reject. Without `deniedPaths` there - // is no deny to bypass, so we log it and leave it in place. + // - `Inaccessible` / `Unknown` (exists/maybe-exists but unexaminable): when + // `deniedPaths` are present we cannot prove this path is not an + // undetectable alias that would bypass a deny, so we FAIL CLOSED. Without + // `deniedPaths` there is no deny to bypass, so we log it and leave it in + // place. let has_denied = !policy.denied_paths.is_empty(); let mut groups: HashMap> = HashMap::new(); for (i, (path, intent)) in entries.iter().enumerate() { @@ -326,18 +357,23 @@ pub fn normalize_object_conflicts( groups.entry(id).or_default().push(i); } PathResolution::Absent => {} + PathResolution::Inaccessible if has_denied => { + return Err(FilesystemPolicyError::Policy(unresolvable_message( + path, + *intent, + "permission denied", + "Grant access to the path, or remove the offending entry.", + ))); + } PathResolution::Unknown if has_denied => { - return Err(format!( - "Filesystem path '{}' ({}) could not be resolved to a filesystem object \ - (permission denied, unreachable mount, or I/O error). Because deniedPaths \ - are present, MXC cannot verify it does not alias a denied object and rejects \ - the config rather than risk bypassing the deny. Ensure the path is reachable, \ - or remove the offending entry.", + return Err(FilesystemPolicyError::Host(unresolvable_message( path, - intent.list_name(), - )); + *intent, + "unreachable mount or I/O error", + "Retry once the path is reachable, or remove the offending entry.", + ))); } - PathResolution::Unknown => { + PathResolution::Inaccessible | PathResolution::Unknown => { logger.log_line(&format!( "WARNING: filesystem path '{}' ({}) could not be resolved to a filesystem \ object and was not checked for aliasing", @@ -430,10 +466,30 @@ pub fn normalize_object_conflicts( Ok(Some(new_policy)) } +fn unresolvable_message(path: &str, intent: Intent, cause: &str, remedy: &str) -> String { + format!( + "Filesystem path '{path}' ({}) could not be resolved to a filesystem object ({cause}). \ + Because deniedPaths are present, MXC cannot verify it does not alias a denied object \ + and refuses to run rather than risk bypassing the deny. {remedy}", + intent.list_name(), + ) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn an_unexaminable_path_is_a_host_fault_but_denied_access_is_a_rejection() { + let host = FilesystemPolicyError::Host("stale mount".into()).into_response(); + assert_eq!(host.failure_phase, FailurePhase::LaunchFailed); + assert_eq!(host.exit_code, -1); + + let policy = FilesystemPolicyError::Policy("permission denied".into()).into_response(); + assert_eq!(policy.failure_phase, FailurePhase::Rejected); + assert_eq!(policy.exit_code, 1); + } + #[test] fn unknown_resolution_takes_precedence_over_absent() { assert_eq!( @@ -675,8 +731,9 @@ mod tests { let err = normalize_object_conflicts(&p, &mut logger) .expect_err("an unexaminable path with deniedPaths present must fail closed"); assert!( - err.contains("could not be resolved") && err.contains("deniedPaths"), - "expected a fail-closed rejection message, got: {err}" + matches!(&err, FilesystemPolicyError::Policy(message) + if message.contains("could not be resolved") && message.contains("deniedPaths")), + "expected a fail-closed policy rejection, got: {err:?}" ); // Restore traversal so the tempdir can recurse in during cleanup. @@ -742,8 +799,9 @@ mod tests { let err = normalize_object_conflicts(&p, &mut logger) .expect_err("an unexaminable path with deniedPaths present must fail closed"); assert!( - err.contains("could not be resolved") && err.contains("deniedPaths"), - "expected a fail-closed rejection message, got: {err}" + matches!(&err, FilesystemPolicyError::Policy(message) + if message.contains("could not be resolved") && message.contains("deniedPaths")), + "expected a fail-closed policy rejection, got: {err:?}" ); // Remove the deny ACE so the tempdir can clean up. diff --git a/src/mxc-sdk/src/core/mxc_common/models.rs b/src/mxc-sdk/src/core/mxc_common/models.rs index 5a352cd50..2e34807df 100644 --- a/src/mxc-sdk/src/core/mxc_common/models.rs +++ b/src/mxc-sdk/src/core/mxc_common/models.rs @@ -1139,9 +1139,8 @@ pub enum FailurePhase { /// the VM/sandbox bring-up, or a transient resource contention (e.g. a /// single-instance backend already running). Generally worth retrying. LaunchFailed, - /// The request cannot be honored and will not succeed on a blind retry - /// without changing the input or host: a policy rejection, or a missing - /// host prerequisite (backend/runtime not installed). + /// The request itself is refusable: the policy asks for something the + /// selected backend cannot enforce. Only a changed request can succeed. Rejected, /// The launch command succeeded but the guest/sandbox infrastructure failed /// before or while running user code (agent rendezvous, channel connect, or @@ -1154,26 +1153,38 @@ pub enum FailurePhase { /// can detect a timeout uniformly across backends rather than inferring it /// from `exit_code == -1` (which collides with other failures). Timeout, - /// The selected containment backend is unavailable on this host: the API is - /// missing, or present but not usable (e.g. feature-disabled). Distinct from - /// [`LaunchFailed`] so callers can fall back to a lower tier rather than - /// hard-fail. + /// The host cannot provide the selected containment backend at all: the API + /// is missing, the runtime is not installed, or an OS feature it depends on + /// is disabled. No change to the request can satisfy it — the host has to + /// be provisioned, or a different backend selected. Distinct from + /// [`LaunchFailed`], which is a backend that is present and failed. BackendUnavailable, } impl FailurePhase { /// The wire error code a failure in this phase is reported as. /// - /// [`Rejected`](FailurePhase::Rejected) is the one phase a caller can act on - /// by changing the request, so it is the only one that earns a code of its - /// own; every other failure is an infrastructure problem the caller cannot - /// distinguish usefully and stays `backend_error`. + /// Two phases earn a code of their own because they tell the caller which + /// side is at fault: [`Rejected`](FailurePhase::Rejected) blames the + /// request, and [`BackendUnavailable`](FailurePhase::BackendUnavailable) + /// blames the host. Every other failure is a runtime fault the caller + /// cannot distinguish usefully and stays `backend_error`. pub fn error_code(&self) -> MxcErrorCode { match self { FailurePhase::Rejected => MxcErrorCode::PolicyValidation, + FailurePhase::BackendUnavailable => MxcErrorCode::BackendUnavailable, _ => MxcErrorCode::BackendError, } } + + /// The process exit status a failure in this phase terminates with. + /// + /// Derived from [`error_code`](FailurePhase::error_code) so a response and + /// an entry-point failure carrying the same code always exit alike: a + /// refusal exits 1, and a host or runtime fault exits -1. + pub fn failure_exit_code(&self) -> i32 { + self.error_code().exit_code() + } } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -1191,6 +1202,17 @@ pub struct ScriptResponse { /// Indicates at what phase the failure occurred. #[serde(default)] pub failure_phase: FailurePhase, + /// Non-fatal diagnostics raised while building or running the sandbox, + /// carried on the response so a programmatic caller can filter them + /// instead of scraping stderr. + /// + /// Boxed, like `output_metadata`, to keep `ScriptResponse` small enough to + /// serve as a `Result` error type. Reach for it through + /// [`warnings`](ScriptResponse::warnings) and + /// [`extend_warnings`](ScriptResponse::extend_warnings) rather than the + /// field, so the "absent" and "empty" spellings cannot drift apart. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub warnings: Option>>, /// Structured metadata produced after the sandboxed process exits. #[serde(default, skip_serializing_if = "Option::is_none")] pub output_metadata: Option>, @@ -1205,6 +1227,7 @@ impl Default for ScriptResponse { error_message: String::new(), extended_error: String::new(), failure_phase: FailurePhase::None, + warnings: None, output_metadata: None, } } @@ -1279,13 +1302,47 @@ impl ScriptResponse { /// copying it would print the message bare and then again inside the JSON /// envelope. pub fn rejected(msg: &str) -> Self { + Self::failed(FailurePhase::Rejected, msg) + } + + /// Create a response for a backend this host cannot provide. + /// + /// Reports `backend_unavailable`, which tells the caller the host is at + /// fault: no change to the request will satisfy it. Like + /// [`ScriptResponse::rejected`] nothing ran, so the message is left to the + /// envelope rather than copied into `standard_err`. + pub fn unavailable(msg: &str) -> Self { + Self::failed(FailurePhase::BackendUnavailable, msg) + } + + /// Create a response for a failure attributed to `phase`. + /// + /// The exit code follows the phase, and the message is left to the error + /// envelope rather than copied into `standard_err`. + pub fn failed(phase: FailurePhase, msg: &str) -> Self { ScriptResponse { - exit_code: 1, + exit_code: phase.failure_exit_code(), error_message: msg.to_string(), - failure_phase: FailurePhase::Rejected, + failure_phase: phase, ..Default::default() } } + + /// The non-fatal diagnostics this run recorded. + pub fn warnings(&self) -> &[String] { + self.warnings.as_deref().map_or(&[], Vec::as_slice) + } + + /// Append diagnostics, allocating only once there is something to record. + pub fn extend_warnings(&mut self, warnings: impl IntoIterator) { + let mut incoming = warnings.into_iter().peekable(); + if incoming.peek().is_none() { + return; + } + self.warnings + .get_or_insert_with(Box::default) + .extend(incoming); + } } #[cfg(test)] @@ -1293,18 +1350,21 @@ mod tests { use super::*; #[test] - fn only_rejection_earns_a_caller_actionable_error_code() { + fn each_caller_actionable_phase_earns_its_own_error_code() { assert_eq!( FailurePhase::Rejected.error_code(), MxcErrorCode::PolicyValidation ); + assert_eq!( + FailurePhase::BackendUnavailable.error_code(), + MxcErrorCode::BackendUnavailable + ); for phase in [ FailurePhase::None, FailurePhase::LaunchFailed, FailurePhase::PostLaunchFailed, FailurePhase::ProcessExited, FailurePhase::Timeout, - FailurePhase::BackendUnavailable, ] { assert_eq!( phase.error_code(), @@ -1314,6 +1374,63 @@ mod tests { } } + #[test] + fn only_a_refusal_exits_one() { + assert_eq!(FailurePhase::Rejected.failure_exit_code(), 1); + for phase in [ + FailurePhase::None, + FailurePhase::LaunchFailed, + FailurePhase::PostLaunchFailed, + FailurePhase::ProcessExited, + FailurePhase::Timeout, + FailurePhase::BackendUnavailable, + ] { + assert_eq!(phase.failure_exit_code(), -1, "phase {phase:?}"); + } + } + + #[test] + fn an_unavailable_host_is_told_apart_from_a_refusal_and_a_crash() { + let unavailable = ScriptResponse::unavailable("WSL is not installed"); + + assert_eq!(unavailable.exit_code, -1); + assert_eq!( + unavailable.failure_phase.error_code(), + MxcErrorCode::BackendUnavailable + ); + assert!(unavailable.standard_err.is_empty()); + } + + #[test] + fn warnings_are_omitted_from_the_wire_when_there_are_none() { + let quiet = serde_json::to_string(&ScriptResponse::default()).unwrap(); + assert!(!quiet.contains("warnings"), "{quiet}"); + + let mut noisy = ScriptResponse::default(); + noisy.extend_warnings(["dacl augmentation fell back".to_string()]); + let noisy = serde_json::to_string(&noisy).unwrap(); + assert!(noisy.contains("dacl augmentation fell back"), "{noisy}"); + } + + #[test] + fn recording_nothing_allocates_nothing() { + let mut response = ScriptResponse::default(); + response.extend_warnings(Vec::new()); + assert!(response.warnings.is_none()); + assert!(response.warnings().is_empty()); + } + + // `ScriptResponse` is used as a `Result` error type, so it has to stay + // under clippy's `result_large_err` threshold. + #[test] + fn the_response_stays_small_enough_to_be_an_error_type() { + assert!( + std::mem::size_of::() <= 128, + "{} bytes", + std::mem::size_of::() + ); + } + #[test] fn rejection_exits_one_and_leaves_stderr_to_the_envelope() { let rejected = ScriptResponse::rejected("policy not supported"); @@ -1326,6 +1443,16 @@ mod tests { assert!(rejected.standard_err.is_empty()); } + #[test] + fn a_phased_failure_takes_its_exit_code_from_the_phase() { + let launch = ScriptResponse::failed(FailurePhase::LaunchFailed, "spawn failed"); + + assert_eq!(launch.exit_code, -1); + assert_eq!(launch.failure_phase, FailurePhase::LaunchFailed); + assert_eq!(launch.error_message, "spawn failed"); + assert!(launch.standard_err.is_empty()); + } + #[test] fn sandbox_paths_normalize_against_the_root() { for (input, expected) in [ diff --git a/src/mxc-sdk/src/core/mxc_common/mxc_error.rs b/src/mxc-sdk/src/core/mxc_common/mxc_error.rs index 756e65df2..928ee11c2 100644 --- a/src/mxc-sdk/src/core/mxc_common/mxc_error.rs +++ b/src/mxc-sdk/src/core/mxc_common/mxc_error.rs @@ -49,6 +49,28 @@ impl MxcErrorCode { Self::BackendError => "backend_error", } } + + /// The process exit status a failure with this code terminates with. + /// + /// Every caller-fault code exits 1: the request asked for something that + /// will not be done, and only a changed request can succeed. A host fault + /// (`backend_unavailable`) and a runtime fault (`backend_error`) exit -1, + /// and are told apart by the code rather than by the status. + pub fn exit_code(self) -> i32 { + match self { + Self::BackendUnavailable | Self::BackendError => -1, + Self::MalformedRequest + | Self::UnsupportedContainment + | Self::UnsupportedPhase + | Self::MalformedId + | Self::StaleId + | Self::NotProvisioned + | Self::NotStarted + | Self::AlreadyStarted + | Self::AlreadyStopped + | Self::PolicyValidation => 1, + } + } } impl std::fmt::Display for MxcErrorCode { @@ -187,6 +209,18 @@ impl MxcError { self.api_failure.as_ref().map(|f| f.operation.as_str()) } + /// Serialise to the JSON response envelope callers parse. + /// + /// Falls back to a minimal hand-written envelope if serialisation itself + /// fails, so a consumer that requires `error.code` still parses something. + pub fn to_envelope_string(&self) -> String { + let envelope: ResponseEnvelope<()> = ResponseEnvelope::from_error(self); + serde_json::to_string(&envelope).unwrap_or_else(|_| { + r#"{"error":{"code":"backend_error","message":"failed to serialise error envelope"}}"# + .to_string() + }) + } + /// The underlying platform status, when known. Never present without /// [`MxcError::operation`]. pub fn native_code(&self) -> Option<&str> { @@ -322,6 +356,27 @@ mod tests { } } + #[test] + fn only_host_and_runtime_faults_exit_minus_one() { + let cases = [ + (MxcErrorCode::MalformedRequest, 1), + (MxcErrorCode::UnsupportedContainment, 1), + (MxcErrorCode::UnsupportedPhase, 1), + (MxcErrorCode::BackendUnavailable, -1), + (MxcErrorCode::MalformedId, 1), + (MxcErrorCode::StaleId, 1), + (MxcErrorCode::NotProvisioned, 1), + (MxcErrorCode::NotStarted, 1), + (MxcErrorCode::AlreadyStarted, 1), + (MxcErrorCode::AlreadyStopped, 1), + (MxcErrorCode::PolicyValidation, 1), + (MxcErrorCode::BackendError, -1), + ]; + for (code, exit) in cases { + assert_eq!(code.exit_code(), exit, "{code}"); + } + } + #[test] fn convenience_constructors_set_correct_codes() { assert_eq!( diff --git a/src/mxc-sdk/src/core/mxc_common/sandbox_process.rs b/src/mxc-sdk/src/core/mxc_common/sandbox_process.rs index bb0d530b5..5f38a8a6e 100644 --- a/src/mxc-sdk/src/core/mxc_common/sandbox_process.rs +++ b/src/mxc-sdk/src/core/mxc_common/sandbox_process.rs @@ -934,7 +934,13 @@ mod runner_tests { let response = runner.execute(&request, &mut logger); - assert_eq!(response.exit_code, -1); + // An empty script is a fault of the request, so the shared checks + // refuse it before the backend is engaged at all. + assert_eq!(response.failure_phase, FailurePhase::Rejected); + assert_eq!( + response.exit_code, + FailurePhase::Rejected.failure_exit_code() + ); assert_eq!(validations.load(Ordering::Relaxed), 0); assert_eq!(direct_spawns.load(Ordering::Relaxed), 0); } diff --git a/src/mxc-sdk/src/core/mxc_common/script_runner.rs b/src/mxc-sdk/src/core/mxc_common/script_runner.rs index 7e94f4365..8c9670088 100644 --- a/src/mxc-sdk/src/core/mxc_common/script_runner.rs +++ b/src/mxc-sdk/src/core/mxc_common/script_runner.rs @@ -3,6 +3,7 @@ use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ExecutionRequest, ScriptResponse}; +use crate::mxc_common::mxc_error::MxcError; use crate::mxc_common::validator::{ validate_common, validate_network_policy_support, NetworkPolicySupport, }; @@ -75,10 +76,12 @@ fn dry_run_summary(response: &ScriptResponse) -> String { } /// Print a dry-run result message to the logger, flush, and exit the process. +/// A failed dry run also emits its typed error envelope on stderr. pub fn handle_dry_run_exit(response: &ScriptResponse, logger: &mut Logger) -> ! { use std::fmt::Write; let _ = writeln!(logger, "{}", dry_run_summary(response)); print!("{}", logger.get_buffer()); + emit_backend_error_envelope(response); std::process::exit(response.exit_code); } @@ -97,11 +100,20 @@ pub fn handle_dry_run_exit(response: &ScriptResponse, logger: &mut Logger) -> ! /// /// The `code` comes from the response's /// [`FailurePhase`](crate::mxc_common::models::FailurePhase), so a refused policy is -/// reported as `policy_validation` rather than being flattened into the generic -/// `backend_error` every other failure uses. +/// reported as `policy_validation` and a host that cannot provide the backend +/// as `backend_unavailable`, rather than both being flattened into the generic +/// `backend_error` every other failure uses. The envelope starts on a fresh line +/// because the workload's stderr may not end in a newline. pub fn emit_backend_error_envelope(response: &ScriptResponse) { + if let Some(line) = backend_error_envelope_line(response) { + eprintln!("\n{line}"); + } +} + +/// The stderr line [`emit_backend_error_envelope`] writes, if any. +fn backend_error_envelope_line(response: &ScriptResponse) -> Option { if response.exit_code == 0 || response.error_message.is_empty() { - return; + return None; } let mut envelope = serde_json::json!({ @@ -114,11 +126,48 @@ pub fn emit_backend_error_envelope(response: &ScriptResponse) { envelope["error"]["extended_error"] = serde_json::Value::String(response.extended_error.clone()); } - if let Ok(json) = serde_json::to_string(&envelope) { - eprintln!("{json}"); + serde_json::to_string(&envelope).ok() +} + +/// The stderr lines a run's warnings are emitted as, one JSON object each. +/// +/// Kept separate from [`emit_warning_envelopes`] so the exact bytes a caller +/// has to parse are testable without capturing the process's stderr. +fn warning_envelope_lines(warnings: &[String]) -> Vec { + warnings + .iter() + .filter(|warning| !warning.trim().is_empty()) + .filter_map(|warning| { + serde_json::to_string(&serde_json::json!({ "warning": { "message": warning } })).ok() + }) + .collect() +} + +/// Emit each warning as its own structured JSON line on stderr. +/// +/// Warnings are reported on success as well as on failure, so this is separate +/// from [`emit_backend_error_envelope`] rather than folded into it. One object +/// per line keeps the stream parseable a line at a time, matching the error +/// envelope. The batch starts on a fresh line because the workload's stderr may +/// not end in a newline. +pub fn emit_warning_envelopes(warnings: &[String]) { + let lines = warning_envelope_lines(warnings); + if !lines.is_empty() { + eprintln!("\n{}", lines.join("\n")); } } +/// Emit a typed JSON error envelope on stderr for a failure raised before any +/// [`ScriptResponse`] exists, then exit with the status its code implies. +/// +/// Shared by every executor entry point. Without it such a failure prints a +/// bare line and exits 1, which a caller cannot tell from a typed policy +/// rejection. +pub fn emit_mxc_error_exit(error: &MxcError) -> ! { + eprintln!("{}", error.to_envelope_string()); + std::process::exit(error.code.exit_code()); +} + #[cfg(test)] mod tests { use super::get_timeout_milliseconds; @@ -140,28 +189,73 @@ mod tests { fn error_envelope_is_noop_without_error() { use crate::mxc_common::models::ScriptResponse; // exit 0 => no-op; non-zero but empty message (clean sandbox exit) => no-op. - super::emit_backend_error_envelope(&ScriptResponse { + assert!(super::backend_error_envelope_line(&ScriptResponse { exit_code: 0, error_message: "ignored on success".to_string(), ..Default::default() - }); - super::emit_backend_error_envelope(&ScriptResponse { + }) + .is_none()); + assert!(super::backend_error_envelope_line(&ScriptResponse { exit_code: 1, error_message: String::new(), ..Default::default() - }); + }) + .is_none()); } #[test] - fn error_envelope_emits_on_infra_failure() { + fn error_envelope_carries_the_code_for_each_failure_kind() { use crate::mxc_common::models::ScriptResponse; - // Exercises the serialization branch (writes to stderr); must not panic. - super::emit_backend_error_envelope(&ScriptResponse { - exit_code: 1, - error_message: "backend unavailable".to_string(), + let cases = [ + (ScriptResponse::rejected("refused"), "policy_validation"), + ( + ScriptResponse::unavailable("missing"), + "backend_unavailable", + ), + (ScriptResponse::error("broke"), "backend_error"), + ]; + for (response, code) in cases { + let line = super::backend_error_envelope_line(&response).expect(code); + let parsed: serde_json::Value = serde_json::from_str(&line).unwrap(); + assert_eq!(parsed["error"]["code"], code, "{line}"); + assert_eq!(parsed["error"]["message"], response.error_message); + } + } + + #[test] + fn error_envelope_includes_extended_error_when_present() { + use crate::mxc_common::models::ScriptResponse; + let line = super::backend_error_envelope_line(&ScriptResponse { extended_error: "WIN32_ERROR(1920)".to_string(), - ..Default::default() - }); + ..ScriptResponse::unavailable("backend unavailable") + }) + .unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&line).unwrap(); + assert_eq!(parsed["error"]["extended_error"], "WIN32_ERROR(1920)"); + } + + #[test] + fn warning_lines_are_one_parseable_object_each() { + let lines = super::warning_envelope_lines(&[ + "dacl augmentation fell back".to_string(), + "quoted \"name\" and \\ backslash".to_string(), + ]); + + assert_eq!( + lines[0], + r#"{"warning":{"message":"dacl augmentation fell back"}}"# + ); + // A warning is free-form text, so it has to survive JSON escaping. + let parsed: serde_json::Value = serde_json::from_str(&lines[1]).unwrap(); + assert_eq!( + parsed["warning"]["message"], + "quoted \"name\" and \\ backslash" + ); + } + + #[test] + fn blank_warnings_do_not_reach_the_stream() { + assert!(super::warning_envelope_lines(&[String::new(), " ".to_string()]).is_empty()); } #[test] diff --git a/src/mxc-sdk/src/core/mxc_common/validator.rs b/src/mxc-sdk/src/core/mxc_common/validator.rs index b3b359d17..07c9b5793 100644 --- a/src/mxc-sdk/src/core/mxc_common/validator.rs +++ b/src/mxc-sdk/src/core/mxc_common/validator.rs @@ -147,13 +147,15 @@ pub fn validate_state_aware_network_policy_support( /// Validates non-backend-specific parts of the request (e.g. non-empty script). pub fn validate_common(request: &ExecutionRequest) -> Result<(), ScriptResponse> { if request.script_code.is_empty() { - return Err(ScriptResponse::error("Script content must not be empty.")); + return Err(ScriptResponse::rejected( + "Script content must not be empty.", + )); } if !request.policy.enumerate_paths.is_empty() && request.containment != crate::mxc_common::models::ContainmentBackend::ProcessContainer { - return Err(ScriptResponse::error( + return Err(ScriptResponse::rejected( "processContainer.filesystem.enumeratePaths is supported only by the Windows \ ProcessContainer backend", )); diff --git a/src/mxc-sdk/src/core/mxc_engine/dispatch.rs b/src/mxc-sdk/src/core/mxc_engine/dispatch.rs index 914a3aad9..f4445540b 100644 --- a/src/mxc-sdk/src/core/mxc_engine/dispatch.rs +++ b/src/mxc-sdk/src/core/mxc_engine/dispatch.rs @@ -297,7 +297,7 @@ fn spawn_process_container( match spawn_with_fallback(request, logger, StdioMode::Pipes, capture_factory) { Ok(dispatched) => { for w in &dispatched.warnings { - let _ = writeln!(logger, "warning: {w}"); + logger.warning_line(w); } let _ = writeln!( logger, @@ -307,15 +307,13 @@ fn spawn_process_container( Ok(dispatched.process) } Err(SpawnDispatchError::Dispatch(e)) => { - // Surface any retained-entry DACL warnings through the logger so the - // caller's buffer flush still reports them — mirroring the - // run-to-completion `resolve_runner_inner`. + // Retain DACL warnings so a failed dispatch still reports them. if let DispatchError::Dacl { warnings, .. } = &e { for w in warnings { - let _ = writeln!(logger, "dacl warning: {w}"); + logger.warning_line(w); } } - Err(MxcError::backend_unavailable(format!("{e}"))) + Err(e.to_mxc_error()) } Err(SpawnDispatchError::Spawn { response, @@ -328,7 +326,7 @@ fn spawn_process_container( // before its separate spawn attempt, so a spawn failure never loses // them there either. for w in &warnings { - let _ = writeln!(logger, "warning: {w}"); + logger.warning_line(w); } let _ = writeln!(logger, "selected isolation tier: {}", tier.as_str()); Err(map_spawn_error(*response)) @@ -368,7 +366,7 @@ fn spawn_wslc( ) -> Result, MxcError> { #[cfg(target_os = "windows")] { - Err(MxcError::unsupported_containment( + Err(MxcError::backend_unavailable( "WSLC backend not compiled. Rebuild with --features wslc.", )) } @@ -420,7 +418,7 @@ fn spawn_isolation_session( ) -> Result, MxcError> { #[cfg(target_os = "windows")] { - Err(MxcError::unsupported_containment( + Err(MxcError::backend_unavailable( "IsolationSession backend not compiled. Rebuild with --features isolation_session.", )) } @@ -440,7 +438,7 @@ fn spawn_isolation_session_pty( ) -> Result, MxcError> { #[cfg(target_os = "windows")] { - Err(MxcError::unsupported_containment( + Err(MxcError::backend_unavailable( "IsolationSession backend not compiled. Rebuild with --features isolation_session.", )) } @@ -602,6 +600,22 @@ mod tests { assert!(err.message.contains("Windows"), "got: {}", err.message); } + #[cfg(all(target_os = "windows", not(feature = "wslc")))] + #[test] + fn streaming_reports_a_backend_this_build_omits_as_unavailable() { + // Run-to-completion resolution calls the same build/host pair + // `backend_unavailable`, so streaming must not hand the caller a + // different code for it. + let mut request = minimal_request(); + request.containment = ContainmentBackend::Wslc; + let mut logger = Logger::new(Mode::Buffer); + let err = match spawn_runner(&request, &mut logger) { + Ok(_) => panic!("a backend this build omits must not spawn"), + Err(e) => e, + }; + assert_eq!(err.code, MxcErrorCode::BackendUnavailable); + } + #[cfg(not(target_os = "linux"))] #[test] fn streaming_rejects_lxc_off_linux() { diff --git a/src/mxc-sdk/src/core/mxc_engine/error.rs b/src/mxc-sdk/src/core/mxc_engine/error.rs index 9135ecb22..bfcb3e904 100644 --- a/src/mxc-sdk/src/core/mxc_engine/error.rs +++ b/src/mxc-sdk/src/core/mxc_engine/error.rs @@ -4,7 +4,7 @@ //! The SDK's own error type — a crate-owned facade over the internal //! `mxc_common` error, so the public API never exposes the foundation crate. -use crate::mxc_common::mxc_error::{MxcError, MxcErrorCode}; +use crate::mxc_common::mxc_error::{ApiFailure, MxcError, MxcErrorCode}; /// Error codes returned by the SDK, corresponding one-for-one to the /// snake_case codes reported by MXC. @@ -50,6 +50,26 @@ impl std::fmt::Display for ErrorCode { } } +impl ErrorCode { + /// The internal code this facade code stands for. + fn to_internal(self) -> MxcErrorCode { + match self { + Self::MalformedRequest => MxcErrorCode::MalformedRequest, + Self::UnsupportedContainment => MxcErrorCode::UnsupportedContainment, + Self::UnsupportedPhase => MxcErrorCode::UnsupportedPhase, + Self::BackendUnavailable => MxcErrorCode::BackendUnavailable, + Self::MalformedId => MxcErrorCode::MalformedId, + Self::StaleId => MxcErrorCode::StaleId, + Self::NotProvisioned => MxcErrorCode::NotProvisioned, + Self::NotStarted => MxcErrorCode::NotStarted, + Self::AlreadyStarted => MxcErrorCode::AlreadyStarted, + Self::AlreadyStopped => MxcErrorCode::AlreadyStopped, + Self::PolicyValidation => MxcErrorCode::PolicyValidation, + Self::BackendError => MxcErrorCode::BackendError, + } + } +} + impl From for ErrorCode { fn from(code: MxcErrorCode) -> Self { match code { @@ -147,11 +167,36 @@ impl From for Error { } } +impl Error { + /// Recover the internal error this facade error stands for, so an + /// in-repo binary can emit the typed envelope and exit status its code + /// implies. + /// + /// Deliberately an inherent method rather than `impl From for + /// MxcError`: a second `MxcError: From<_>` impl makes `?` ambiguous + /// wherever the error type is inferred. + /// + /// `MxcError::details` has no facade counterpart and does not survive the + /// round trip. + #[doc(hidden)] + pub fn into_mxc_error(self) -> MxcError { + let mut error = MxcError::new(self.code.to_internal(), self.message); + if let Some(operation) = self.operation { + let mut failure = ApiFailure::new(operation); + failure.native_code = self.native_code; + error = error.with_api_failure(failure); + } + if let Some(remediation) = self.remediation { + error = error.with_remediation(remediation); + } + error + } +} + #[cfg(test)] mod tests { use super::*; use crate::mxc_common::mxc_error::{ApiFailure as InnerFailure, MxcError}; - /// The conversion carries the API detail across, rather than keeping only /// the code and message. /// diff --git a/src/mxc-sdk/src/core/mxc_engine/run.rs b/src/mxc-sdk/src/core/mxc_engine/run.rs index f10e51128..74a92ba11 100644 --- a/src/mxc-sdk/src/core/mxc_engine/run.rs +++ b/src/mxc-sdk/src/core/mxc_engine/run.rs @@ -80,9 +80,12 @@ impl ResolvedRunner { /// Experimental backends, classified by the engine's backend registry, require /// `request.experimental_enabled`; without it they return a /// [`backend_unavailable`](MxcError::backend_unavailable) error before any -/// host-specific resolution. Backends that are not available on this host / -/// not compiled in return an -/// [`unsupported_containment`](MxcError::unsupported_containment) error. +/// host-specific resolution. A backend that this build did not compile in +/// reports the same code, since only a rebuilt executor can serve it. A +/// backend that belongs to a different operating system, or one that does not +/// exist yet, reports +/// [`unsupported_containment`](MxcError::unsupported_containment) instead, +/// because naming a backend this host does support will succeed. pub fn resolve_runner( request: &ExecutionRequest, logger: &mut Logger, @@ -190,13 +193,24 @@ pub fn resolve_runner_for_audit( /// Convenience over [`resolve_runner`] for callers without external guard / /// signal machinery: it runs the resolved runner and drops it — then, on /// Windows, the DACL guard — in the correct order before returning the -/// [`ScriptResponse`]. +/// [`ScriptResponse`]. Warnings the logger recorded are moved onto the +/// response; nothing is written to the terminal. pub fn run(request: &ExecutionRequest, logger: &mut Logger) -> Result { - let mut resolved = resolve_runner(request, logger)?; - let response = resolved.runner.run(request, logger); - // `resolved` drops here: `runner` first (releasing child handles), then — - // on Windows — `dacl_manager` (restoring host ACEs). - Ok(response) + let resolved = resolve_runner(request, logger)?; + Ok(run_resolved(resolved, request, logger)) +} + +fn run_resolved( + mut resolved: ResolvedRunner, + request: &ExecutionRequest, + logger: &mut Logger, +) -> ScriptResponse { + let mut response = resolved.runner.run(request, logger); + // `runner` drops first (releasing child handles), then — on Windows — + // `dacl_manager` (restoring host ACEs). + drop(resolved); + response.extend_warnings(logger.take_warnings()); + response } // --------------------------------------------------------------------------- @@ -228,7 +242,7 @@ fn resolve_runner_inner_windows( ) { Ok(dispatched) => { for w in &dispatched.warnings { - let _ = writeln!(logger, "warning: {w}"); + logger.warning_line(w); } let _ = writeln!( logger, @@ -243,18 +257,17 @@ fn resolve_runner_inner_windows( }) } Err(e) => { - // Surface any retained-entry DACL warnings through the - // logger so the caller's buffer flush still reports them. + // Retain DACL warnings so a failed dispatch still reports them. if let crate::process_container_common::dispatcher::DispatchError::Dacl { warnings, .. } = &e { for w in warnings { - let _ = writeln!(logger, "dacl warning: {w}"); + logger.warning_line(w); } } - Err(MxcError::backend_unavailable(format!("{e}"))) + Err(e.to_mxc_error()) } } } @@ -270,7 +283,7 @@ fn resolve_runner_inner_windows( #[cfg(not(feature = "wslc"))] { let _ = logger; - Err(MxcError::unsupported_containment( + Err(MxcError::backend_unavailable( "WSLC backend not compiled. Rebuild with --features wslc.", )) } @@ -296,7 +309,7 @@ fn resolve_runner_inner_windows( } #[cfg(not(feature = "microvm"))] { - Err(MxcError::unsupported_containment( + Err(MxcError::backend_unavailable( "MicroVM backend not compiled in (build with --features microvm)", )) } @@ -308,11 +321,9 @@ fn resolve_runner_inner_windows( if ws.idle_timeout_ms != default.idle_timeout_ms || ws.daemon_pipe_name != default.daemon_pipe_name { - let _ = writeln!( - logger, - "warning: windowsSandbox.idleTimeoutMs and daemonPipeName \ - are ignored by the one-shot backend; each invocation launches and tears \ - down a fresh VM" + logger.warning_line( + "windowsSandbox.idleTimeoutMs and daemonPipeName are ignored by the \ + one-shot backend; each invocation launches and tears down a fresh VM", ); } } @@ -329,7 +340,7 @@ fn resolve_runner_inner_windows( } #[cfg(not(feature = "isolation_session"))] { - Err(MxcError::unsupported_containment( + Err(MxcError::backend_unavailable( "IsolationSession backend not compiled. Rebuild with --features isolation_session.", )) } @@ -360,7 +371,7 @@ fn resolve_runner_inner( } #[cfg(not(feature = "microvm"))] { - Err(MxcError::unsupported_containment( + Err(MxcError::backend_unavailable( "MicroVM backend not compiled in (build with --features microvm)", )) } @@ -502,8 +513,9 @@ fn resolve_hyperlight() -> Result { } #[cfg(not(all(feature = "hyperlight", target_arch = "x86_64")))] { - Err(MxcError::unsupported_containment( - "Hyperlight backend requires x86_64 (Hyperlight needs KVM or WHP)", + Err(MxcError::backend_unavailable( + "Hyperlight backend not compiled in; it requires x86_64 and --features hyperlight \ + (Hyperlight needs KVM or WHP).", )) } } @@ -570,6 +582,7 @@ mod tests { let mut logger = Logger::new(Mode::Buffer); resolve_runner_inner_windows(&request, &mut logger).unwrap(); assert!(logger.get_buffer().is_empty()); + assert!(logger.warnings().is_empty()); } } @@ -584,7 +597,7 @@ mod tests { resolve_runner_inner_windows(&request, &mut logger).unwrap(); - let warning = logger.get_buffer(); + let warning = logger.warnings().join("\n"); assert!(warning.contains("idleTimeoutMs")); assert!(warning.contains("daemonPipeName")); assert!(warning.contains("fresh VM")); @@ -604,4 +617,70 @@ mod tests { assert!(!logger.get_buffer().contains("experimental")); } + + fn resolve_error(request: &ExecutionRequest) -> Error { + let mut logger = Logger::new(Mode::Buffer); + match resolve_runner(request, &mut logger) { + Ok(_) => panic!("{:?} must not resolve", request.containment), + Err(error) => error, + } + } + + #[test] + fn a_policy_no_tier_can_enforce_is_a_policy_validation_error() { + let _guard = crate::process_container_common::test_env::BcUsableGuard::set(false); + let mut request = ExecutionRequest { + containment: ContainmentBackend::ProcessContainer, + ..Default::default() + }; + request.policy.enumerate_paths = vec!["C:\\tools".to_string()]; + + let error = resolve_error(&request); + + assert_eq!(error.code, crate::ErrorCode::PolicyValidation, "{error}"); + } + + #[cfg(not(feature = "microvm"))] + #[test] + fn a_backend_not_compiled_in_is_unavailable() { + let request = ExecutionRequest { + containment: ContainmentBackend::MicroVm, + experimental_enabled: true, + ..Default::default() + }; + + let error = resolve_error(&request); + + assert_eq!(error.code, crate::ErrorCode::BackendUnavailable, "{error}"); + } +} + +#[cfg(test)] +mod run_tests { + use super::*; + use crate::mxc_common::logger::Mode; + + struct WarningRunner; + + impl ScriptRunner for WarningRunner { + fn execute(&mut self, _request: &ExecutionRequest, logger: &mut Logger) -> ScriptResponse { + logger.warning_line("advisory"); + ScriptResponse::default() + } + } + + #[test] + fn run_moves_logger_warnings_onto_the_response() { + let mut logger = Logger::new(Mode::Buffer); + let response = run_resolved( + ResolvedRunner::without_guard(Box::new(WarningRunner)), + &ExecutionRequest { + script_code: "echo hi".to_string(), + ..Default::default() + }, + &mut logger, + ); + assert_eq!(response.warnings(), ["advisory"]); + assert!(logger.warnings().is_empty()); + } } diff --git a/src/tools/lxc/src/linux_executor_arguments.rs b/src/tools/lxc/src/linux_executor_arguments.rs index bbe5fcdc1..d3188f4e4 100644 --- a/src/tools/lxc/src/linux_executor_arguments.rs +++ b/src/tools/lxc/src/linux_executor_arguments.rs @@ -5,7 +5,8 @@ use clap::Parser; use mxc_sdk::mxc_common::config_parser::load_one_shot_request; use mxc_sdk::mxc_common::logger::Logger; use mxc_sdk::mxc_common::models::ExecutionRequest; -use std::process; +use mxc_sdk::mxc_common::mxc_error::MxcError; +use mxc_sdk::mxc_common::script_runner::emit_mxc_error_exit; #[derive(Parser)] #[command(name = "lxc-exec", about = "Linux Container Executor")] @@ -99,19 +100,17 @@ impl LinuxExecutorArguments { } else if let Some(ref path) = self.config_path { (path.clone(), false) } else { - eprintln!( - "Error: No config provided. Use a positional path, --config, or --config-base64" - ); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request( + "No config provided. Use a positional path, --config, or --config-base64", + )); }; let mut request = match load_one_shot_request(&config_data, logger, is_base64) { Ok(r) => r, - Err(_) => { - eprintln!("Request error"); + Err(error) => { crate::emit_warnings(logger); eprint!("{}", logger.get_buffer()); - process::exit(1); + emit_mxc_error_exit(&error.to_mxc_error()); } }; @@ -187,8 +186,9 @@ impl LinuxExecutorArguments { let name = match self.containername { Some(ref n) => n, None => { - eprintln!("Error: --containername is required with --delete"); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request( + "--containername is required with --delete", + )); } }; diff --git a/src/tools/lxc/src/main.rs b/src/tools/lxc/src/main.rs index 1b482ad86..91fa1e1d2 100644 --- a/src/tools/lxc/src/main.rs +++ b/src/tools/lxc/src/main.rs @@ -10,7 +10,8 @@ use std::{eprint, process}; use mxc_sdk::mxc_common::logger::{Logger, Mode}; use mxc_sdk::mxc_common::models::{ExecutionRequest, ScriptResponse}; -use mxc_sdk::mxc_common::script_runner; +use mxc_sdk::mxc_common::mxc_error::MxcError; +use mxc_sdk::mxc_common::script_runner::{self, emit_mxc_error_exit, emit_warning_envelopes}; use mxc_sdk::mxc_common::telemetry; use mxc_sdk::lxc_common::signal_cleanup; @@ -28,7 +29,7 @@ fn main() { // Enable the file sink if needed if let Some(ref log_path) = arguments.get_log_path() { if let Err(e) = logger.enable_file_sink(std::path::Path::new(log_path)) { - eprintln!("Warning: could not open log file '{}': {}", log_path, e); + emit_warning_envelopes(&[format!("could not open log file '{log_path}': {e}")]); } } @@ -40,8 +41,9 @@ fn main() { process::exit(0); } Err(e) => { - eprintln!("Error: probe serialization failed: {e}"); - process::exit(1); + emit_mxc_error_exit(&MxcError::backend_error(format!( + "probe serialization failed: {e}" + ))); } } } else if arguments.does_user_want_hyperlight_setup() { @@ -85,8 +87,9 @@ fn main() { // Setup watchdog for unexpected shutdowns if let Err(e) = signal_cleanup::install() { - eprintln!("Error: failed to install signal cleanup handler: {e}"); - process::exit(1); + emit_mxc_error_exit(&MxcError::backend_error(format!( + "failed to install signal cleanup handler: {e}" + ))); } let run_start = Instant::now(); @@ -95,7 +98,6 @@ fn main() { let response = match mxc_sdk::mxc_engine::run(&request, &mut logger) { Ok(response) => response, Err(e) => { - eprintln!("error: {}", e.message); emit_warnings(&logger); eprint!("{}", logger.get_buffer()); telemetry::emit_early_exit_with_kind( @@ -104,14 +106,15 @@ fn main() { requested_sandbox_kind, telemetry::FailureReason::InitError, ); - process::exit(1); + emit_mxc_error_exit(&e.into_mxc_error()); } }; let run_elapsed = run_start.elapsed(); let _ = writeln!(logger, "Runner completed in {}ms", run_elapsed.as_millis()); - emit_warnings(&logger); + // Emitted before the dry-run branch below, which exits the process. + emit_warning_envelopes(response.warnings()); // Handle dry_run request. if arguments.parse_and_validate() { @@ -145,9 +148,7 @@ fn log_request(request: &ExecutionRequest, logger: &mut Logger) { } fn emit_warnings(logger: &Logger) { - for warning in logger.warnings() { - eprintln!("{warning}"); - } + emit_warning_envelopes(logger.warnings()); } fn display_script_results(response: &ScriptResponse, logger: &mut Logger) { diff --git a/src/tools/mxc_darwin/src/main.rs b/src/tools/mxc_darwin/src/main.rs index f017a763a..72a83979e 100644 --- a/src/tools/mxc_darwin/src/main.rs +++ b/src/tools/mxc_darwin/src/main.rs @@ -10,12 +10,13 @@ //! use still requires macOS. use std::fmt::Write; -use std::process; use clap::Parser; use mxc_sdk::mxc_common::config_parser::load_one_shot_request; use mxc_sdk::mxc_common::logger::{Logger, Mode}; use mxc_sdk::mxc_common::models::ExecutionRequest; +use mxc_sdk::mxc_common::mxc_error::MxcError; +use mxc_sdk::mxc_common::script_runner::{emit_mxc_error_exit, emit_warning_envelopes}; #[cfg(target_os = "macos")] use mxc_sdk::mxc_common::models::ScriptResponse; @@ -84,8 +85,9 @@ fn main() { } else if let Some(ref path) = cli.config_path { (path.clone(), false) } else { - eprintln!("Error: No config provided. Use a positional path, --config, or --config-base64"); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request( + "No config provided. Use a positional path, --config, or --config-base64", + )); }; let mut logger = Logger::new(if cli.debug { @@ -96,15 +98,15 @@ fn main() { if let Some(ref log_path) = cli.log_file { if let Err(e) = logger.enable_file_sink(std::path::Path::new(log_path)) { - eprintln!("Warning: could not open log file '{}': {}", log_path, e); + emit_warning_envelopes(&[format!("could not open log file '{log_path}': {e}")]); } } let mut request = match load_one_shot_request(&config_data, &mut logger, is_base64) { Ok(r) => r, - Err(_) => { - eprint!("Request error\n{}", logger.get_buffer()); - process::exit(1); + Err(error) => { + eprint!("{}", logger.get_buffer()); + emit_mxc_error_exit(&error.to_mxc_error()); } }; @@ -150,14 +152,17 @@ fn run_seatbelt(request: &ExecutionRequest, logger: &mut Logger) -> ! { let response = match mxc_sdk::mxc_engine::run(request, logger) { Ok(response) => response, Err(e) => { - eprintln!("error: {}", e.message); + emit_warning_envelopes(logger.warnings()); eprint!("{}", logger.get_buffer()); - process::exit(1); + emit_mxc_error_exit(&e.into_mxc_error()); } }; let run_elapsed = run_start.elapsed(); let _ = writeln!(logger, "Runner completed in {}ms", run_elapsed.as_millis()); + // Emitted before the dry-run branch below, which exits the process. + emit_warning_envelopes(response.warnings()); + if request.dry_run { handle_dry_run_exit(&response, logger); } @@ -182,15 +187,14 @@ fn run_seatbelt(request: &ExecutionRequest, logger: &mut Logger) -> ! { // with lxc-exec / wxc-exec (issue #564). mxc_sdk::mxc_common::script_runner::emit_backend_error_envelope(&response); - process::exit(response.exit_code); + std::process::exit(response.exit_code); } #[cfg(not(target_os = "macos"))] fn run_seatbelt(_request: &ExecutionRequest, logger: &mut Logger) -> ! { - eprintln!( - "mxc-exec-mac: the macOS sandbox backend is only available on macOS. \ - This binary was built for a non-Darwin target and cannot execute scripts." - ); print!("{}", logger.get_buffer()); - process::exit(1); + emit_mxc_error_exit(&MxcError::backend_unavailable( + "mxc-exec-mac: the macOS sandbox backend is only available on macOS. \ + This binary was built for a non-Darwin target and cannot execute scripts.", + )); } diff --git a/src/tools/wxc/src/main.rs b/src/tools/wxc/src/main.rs index a30ad4fd0..8f67529bc 100644 --- a/src/tools/wxc/src/main.rs +++ b/src/tools/wxc/src/main.rs @@ -17,8 +17,10 @@ use mxc_sdk::mxc_common::diagnostic::DiagnosticConfig; use mxc_sdk::mxc_common::error::WxcError; use mxc_sdk::mxc_common::logger::{Logger, Mode}; use mxc_sdk::mxc_common::models::{ContainmentBackend, ExecutionRequest, ScriptResponse}; -use mxc_sdk::mxc_common::mxc_error::{MxcError, MxcErrorCode, ResponseEnvelope}; -use mxc_sdk::mxc_common::script_runner::{handle_dry_run_exit, ScriptRunner}; +use mxc_sdk::mxc_common::mxc_error::{MxcError, MxcErrorCode}; +use mxc_sdk::mxc_common::script_runner::{ + emit_mxc_error_exit, emit_warning_envelopes, handle_dry_run_exit, ScriptRunner, +}; use mxc_sdk::mxc_common::state_aware_dispatch::{resolve_backend, DispatchOutcome}; use mxc_sdk::mxc_common::state_aware_request::{MxcRequest, ParsedStateAwareRequest, Phase}; use mxc_sdk::mxc_common::telemetry; @@ -379,7 +381,7 @@ fn logger_for_cli(cli: &Cli) -> Logger { if let Some(ref log_path) = cli.log_file { if let Err(error) = logger.enable_file_sink(std::path::Path::new(log_path)) { - eprintln!("Warning: could not open log file '{log_path}': {error}"); + emit_warning_envelopes(&[format!("could not open log file '{log_path}': {error}")]); } } @@ -597,9 +599,7 @@ fn run_state_aware_main( diagnostic_logger.log_audit_event(&record); } } - for warning in logger.take_warnings() { - eprintln!("{warning}"); - } + mxc_sdk::mxc_common::script_runner::emit_warning_envelopes(&logger.take_warnings()); let buffered = logger.get_buffer().to_string(); if !buffered.is_empty() { eprint!("{}", buffered); @@ -681,15 +681,15 @@ fn run_state_aware_main( process::exit(0); } StateAwareExit::ExecCode(exit_code) => process::exit(exit_code), - StateAwareExit::Error(json) => { + StateAwareExit::Error(json, exit_code) => { println!("{}", json); - process::exit(1); + process::exit(exit_code); } - StateAwareExit::ExecError(json) => { + StateAwareExit::ExecError(json, exit_code) => { // The script's own stderr may not end in a newline, and gluing the // envelope onto those bytes would leave it unparseable. eprintln!("\n{}", json); - process::exit(1); + process::exit(exit_code); } } } @@ -703,10 +703,10 @@ enum StateAwareExit { Envelope(String), /// Exit with this script exit code (its output was already streamed). ExecCode(i32), - /// Print this JSON error envelope to stdout, then exit 1. - Error(String), - /// Print this JSON error envelope to stderr, then exit 1. - ExecError(String), + /// Print this JSON error envelope to stdout, then exit with the code's status. + Error(String, i32), + /// Print this JSON error envelope to stderr, then exit with the code's status. + ExecError(String, i32), } /// Map a dispatch outcome to its terminal exit action. Pure — it neither prints @@ -722,22 +722,28 @@ fn finalize_state_aware_outcome( Ok(DispatchOutcome::ExecCompleted { exit_code }) => StateAwareExit::ExecCode(exit_code), Err(e) => { let json = error_envelope_string(&e); + let exit_code = e.code.exit_code(); // A failed exec has already streamed the script's output to stdout, // so an envelope appended there would corrupt both. A dry run // streams nothing and keeps its envelope on stdout. if phase == "exec" && !dry_run { - StateAwareExit::ExecError(json) + StateAwareExit::ExecError(json, exit_code) } else { - StateAwareExit::Error(json) + StateAwareExit::Error(json, exit_code) } } } } +/// Where a request failure is written, which differs by request shape. enum RequestErrorRoute<'a> { - Diagnostic, - Envelope(&'a MxcError), + /// One-shot: a typed envelope on stderr, beside the diagnostics, because + /// stdout belongs to the script's own output. + Stderr(MxcError), + /// State-aware: the envelope *is* the operation's response document, so it + /// goes to stdout where the caller parses the result. + Stdout(&'a MxcError), } fn request_error_route(error: &ParseError) -> RequestErrorRoute<'_> { @@ -745,8 +751,8 @@ fn request_error_route(error: &ParseError) -> RequestErrorRoute<'_> { ParseError::Decode(_) | ParseError::Version(_) | ParseError::OneShot(_) - | ParseError::OneShotMalformed(_) => RequestErrorRoute::Diagnostic, - ParseError::StateAware(e) => RequestErrorRoute::Envelope(e), + | ParseError::OneShotMalformed(_) => RequestErrorRoute::Stderr(error.to_mxc_error()), + ParseError::StateAware(e) => RequestErrorRoute::Stdout(e), } } @@ -755,10 +761,8 @@ fn reject_state_aware_without_operation(logger: &mut Logger) -> ! { remove 'phase' and 'sandboxId' from the config JSON"; let error = ParseError::OneShot(WxcError::ConfigParse(message.to_string())); log_request_parse_rejection(logger, &error, None); - eprintln!("Request error"); - eprintln!("{message}"); eprint!("{}", logger.get_buffer()); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request(message)); } fn log_request_parse_rejection(logger: &mut Logger, error: &ParseError, phase: Option) { @@ -811,18 +815,9 @@ fn print_error_envelope(error: &MxcError) { /// Serialise `error` to its JSON response-envelope string. Split out of /// [`print_error_envelope`] so [`finalize_state_aware_outcome`] can build the -/// error-exit envelope without printing, and so both the print path and the -/// state-aware exit path share one serialisation (including the last-resort -/// fallback when the envelope itself fails to serialise). +/// error-exit envelope without printing. fn error_envelope_string(error: &MxcError) -> String { - let envelope: ResponseEnvelope<()> = ResponseEnvelope::from_error(error); - serde_json::to_string(&envelope).unwrap_or_else(|_| { - // Last-resort path: serialisation of the envelope itself failed — emit a - // minimal structurally-valid envelope so consumers that require - // `error.code` still parse something. - r#"{"error":{"code":"backend_error","message":"failed to serialise error envelope"}}"# - .to_string() - }) + error.to_envelope_string() } fn config_input(cli: &Cli) -> Option<(String, bool)> { @@ -1065,19 +1060,23 @@ fn main() { match mxc_sdk::mxc_common::filesystem_dacl::recover_orphaned_state() { Ok(report) => { if report.files_processed > 0 || !report.errors.is_empty() { - eprintln!( + let mut warnings = vec![format!( "DACL recovery: {} file(s), {} ACE(s) restored, {} pruned (missing), {} error(s)", report.files_processed, report.aces_restored, report.aces_pruned_missing, report.errors.len() + )]; + warnings.extend( + report + .errors + .iter() + .map(|e| format!("DACL recovery error: {e}")), ); - for e in &report.errors { - eprintln!(" {e}"); - } + emit_warning_envelopes(&warnings); } } - Err(e) => eprintln!("DACL recovery failed: {e}"), + Err(e) => emit_warning_envelopes(&[format!("DACL recovery failed: {e}")]), } // --probe is a detection-only fast path used by SDK @@ -1101,15 +1100,17 @@ fn main() { ) { Ok(MxcRequest::OneShot(request)) => request, Ok(MxcRequest::StateAware(_)) | Err(_) => { - eprintln!("Error: failed to load probe config"); eprint!("{}", probe_logger.get_buffer()); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request( + "failed to load probe config", + )); } }, Err(_) => { - eprintln!("Error: failed to load probe config"); eprint!("{}", probe_logger.get_buffer()); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request( + "failed to load probe config", + )); } } } else { @@ -1117,17 +1118,13 @@ fn main() { }; let output = match mxc_sdk::mxc_engine::probe_execution_request(Some(&request)) { Ok(output) => output, - Err(error) => { - eprintln!("Error: {}", error.message); - process::exit(1); - } + Err(error) => emit_mxc_error_exit(&error.into_mxc_error()), }; match mxc_sdk::process_container_common::probe::to_json_pretty(&output) { Ok(s) => println!("{s}"), - Err(e) => { - eprintln!("Error: probe serialization failed: {e}"); - process::exit(1); - } + Err(e) => emit_mxc_error_exit(&MxcError::backend_error(format!( + "probe serialization failed: {e}" + ))), } return; } @@ -1296,9 +1293,7 @@ fn main() { process::exit(1); } None => { - eprintln!("Request error"); - eprintln!("{}", error.into_error()); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request(error.into_error().to_string())); } }, None => { @@ -1315,8 +1310,7 @@ fn main() { eprint!("{}", logger.get_buffer()); process::exit(1); } - eprintln!("Error: {message}"); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request(message)); } None } @@ -1326,10 +1320,9 @@ fn main() { if cli.delete { let name = match cli.containername { Some(ref n) => n.as_str(), - None => { - eprintln!("Error: --containername is required with --delete"); - process::exit(1); - } + None => emit_mxc_error_exit(&MxcError::malformed_request( + "--containername is required with --delete", + )), }; let success = delete_app_container_profile(name, &mut logger); print!("{}", logger.get_buffer()); @@ -1411,15 +1404,16 @@ fn main() { Err(error) => { log_request_parse_rejection(&mut logger, &error, None); match request_error_route(&error) { - RequestErrorRoute::Diagnostic => { - eprint!("Request error\n{}", logger.get_buffer()); + RequestErrorRoute::Stderr(typed) => { + eprint!("{}", logger.get_buffer()); + emit_mxc_error_exit(&typed); } - RequestErrorRoute::Envelope(e) => { + RequestErrorRoute::Stdout(e) => { print_error_envelope(e); eprint!("{}", logger.get_buffer()); + process::exit(e.code.exit_code()); } } - process::exit(1); } }; @@ -1470,35 +1464,34 @@ fn main() { "containment", "", ); - eprintln!("Error: {message}"); telemetry::emit_early_exit_with_kind( telemetry_active, &request.containment, requested_sandbox_kind, telemetry::FailureReason::ConfigError, ); - process::exit(1); + emit_mxc_error_exit(&MxcError::policy_validation(message)); } let injected = apply_permissive_learning_mode(&mut request.policy.capabilities); let context = match audit::prepare_request(&mut request, config_file_path(&cli)) { Ok(context) => context, Err(message) => { - eprintln!("Error: {message}"); telemetry::emit_early_exit_with_kind( telemetry_active, &request.containment, requested_sandbox_kind, telemetry::FailureReason::ConfigError, ); - process::exit(1); + emit_mxc_error_exit(&MxcError::backend_error(message)); } }; audit_context = Some(context); logger.log("WARNING: --audit enabled - AppContainer restrictions will NOT be enforced\n"); if injected && cli.audit_verbose { - eprintln!( - "[mxc] permissiveLearningMode injected via --audit - AppContainer restrictions are NOT enforced" - ); + emit_warning_envelopes(&[ + "permissiveLearningMode injected via --audit - AppContainer restrictions are NOT enforced" + .to_string(), + ]); } } @@ -1512,9 +1505,6 @@ fn main() { "process.commandLine", "", ); - eprintln!( - "Error: no command to run. Provide `process.commandLine` in the policy or pass the command as arguments after the config path." - ); eprint!("{}", logger.get_buffer()); telemetry::emit_early_exit_with_kind( telemetry_active, @@ -1522,7 +1512,9 @@ fn main() { requested_sandbox_kind, telemetry::FailureReason::ConfigError, ); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request( + "no command to run. Provide `process.commandLine` in the policy or pass the command as arguments after the config path.", + )); } // Inject learningModeLogging capability when diagnostic console is enabled. @@ -1631,22 +1623,23 @@ fn main() { resolved.runner } Err(e) => { + let error = e.into_mxc_error(); log_config_rejected( &mut logger, - RejectionReason::RunnerUnavailable, + rejection_reason_for(&error), request.containment.wire_name(), "containment", "", ); - eprintln!("error: {}", e.message); eprint!("{}", logger.get_buffer()); + emit_warning_envelopes(&logger.take_warnings()); telemetry::emit_early_exit_with_kind( telemetry_active, &request.containment, requested_sandbox_kind, telemetry::FailureReason::InitError, ); - process::exit(1); + emit_mxc_error_exit(&error); } }; @@ -1666,22 +1659,28 @@ fn main() { if !cli.dry_run { match audit::finalize(&mut response, context, &exe_dir, cli.audit_verbose) { Ok(cleanup_warnings) => { - for warning in cleanup_warnings { - let message = format!("[audit] warning: {warning}"); - let _ = writeln!(logger, "{message}"); - eprintln!("{message}"); + for warning in &cleanup_warnings { + let _ = writeln!(logger, "[audit] warning: {warning}"); } + response.extend_warnings( + cleanup_warnings + .into_iter() + .map(|warning| format!("[audit] {warning}")), + ); eprintln!("[audit] artifacts written to {}", context.log_dir.display()); } Err(error) => { let message = format!("audit finalization failed: {error}"); let _ = writeln!(logger, "[audit] {message}"); - eprintln!("error: {message}"); - response.exit_code = -1; response.error_message = match response.error_message.is_empty() { true => message, false => format!("{}; {message}", response.error_message), }; + // Derived rather than hard-coded so the status keeps + // agreeing with the code the envelope reports: an + // otherwise-clean run carries `backend_error`, and a run + // the backend already refused stays a rejection. + response.exit_code = response.failure_phase.failure_exit_code(); } } } @@ -1706,11 +1705,11 @@ fn main() { // deny-by-default under --audit). The logger only records these rather than // writing them itself, so that `mxc_engine` embedders don't get unannounced // writes to a terminal they own. wxc-exec *does* own its terminal, so it - // opts in here. Messages carry their own banner; print them verbatim. + // opts in here. Warnings travel on the response so a programmatic caller + // reads them from one structured place rather than scraping stderr. // Emitted before the dry-run branch below, which exits the process. - for warning in logger.warnings() { - eprintln!("{warning}"); - } + response.extend_warnings(logger.take_warnings()); + emit_warning_envelopes(response.warnings()); #[cfg(target_os = "windows")] if let Err(error) = mxc_sdk::mxc_engine::emit_verbose_telemetry( @@ -2044,7 +2043,7 @@ mod tests { impl ResolveError { fn from_parse(e: ParseError) -> Self { - let envelope_routed = matches!(request_error_route(&e), RequestErrorRoute::Envelope(_)); + let envelope_routed = matches!(request_error_route(&e), RequestErrorRoute::Stdout(_)); let message = match e { ParseError::Decode(err) @@ -2134,28 +2133,33 @@ mod tests { #[test] fn request_error_route_matches_the_output_conventions() { - let decode = ParseError::Decode(WxcError::ConfigParse("decode".to_string())); - assert!(matches!( - request_error_route(&decode), - RequestErrorRoute::Diagnostic - )); - - let version = ParseError::Version(WxcError::ConfigParse("version".to_string())); - assert!(matches!( - request_error_route(&version), - RequestErrorRoute::Diagnostic - )); - - let oneshot = ParseError::OneShot(WxcError::ConfigParse("oneshot".to_string())); - assert!(matches!( - request_error_route(&oneshot), - RequestErrorRoute::Diagnostic - )); + // The stderr route still classifies: a one-shot parse failure blames + // the request, so it reports a code rather than an untyped exit. + for (case, error) in [ + ( + "decode", + ParseError::Decode(WxcError::ConfigParse("decode".to_string())), + ), + ( + "version", + ParseError::Version(WxcError::ConfigParse("version".to_string())), + ), + ( + "oneshot", + ParseError::OneShot(WxcError::ConfigParse("oneshot".to_string())), + ), + ] { + let RequestErrorRoute::Stderr(typed) = request_error_route(&error) else { + panic!("{case}: one-shot failures belong on stderr"); + }; + assert_eq!(typed.code, MxcErrorCode::MalformedRequest, "{case}"); + assert_eq!(typed.code.exit_code(), 1, "{case}"); + } let stateaware = ParseError::StateAware(MxcError::malformed_request("stateaware")); assert!(matches!( request_error_route(&stateaware), - RequestErrorRoute::Envelope(_) + RequestErrorRoute::Stdout(_) )); } @@ -2389,7 +2393,7 @@ mod tests { ) .unwrap_err(); assert!( - matches!(request_error_route(&error), RequestErrorRoute::Diagnostic), + matches!(request_error_route(&error), RequestErrorRoute::Stderr(_)), "{case}: pre-discrimination failures must remain stderr diagnostics" ); @@ -2428,7 +2432,7 @@ mod tests { &mut logger, ) .unwrap_err(); - let RequestErrorRoute::Envelope(error) = request_error_route(&error) else { + let RequestErrorRoute::Stdout(error) = request_error_route(&error) else { panic!("identified lifecycle requests must retain their JSON error envelope"); }; let envelope: serde_json::Value = @@ -2449,7 +2453,7 @@ mod tests { .unwrap_err(); assert!(matches!( request_error_route(&error), - RequestErrorRoute::Diagnostic + RequestErrorRoute::Stderr(_) )); assert!(logger.get_buffer().contains("duplicate field `telemetry`")); } @@ -3250,11 +3254,12 @@ mod tests { let outcome: Result = Err(MxcError::malformed_request("boom".to_string())); match finalize_state_aware_outcome(outcome, "provision", false) { - StateAwareExit::Error(json) => { + StateAwareExit::Error(json, exit_code) => { // Must be a parseable envelope carrying the error code so consumers // that key off `error.code` still work. let parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); assert!(parsed["error"]["code"].is_string(), "{json}"); + assert_eq!(exit_code, 1); } other => panic!("expected Error, got {other:?}"), } @@ -3265,10 +3270,11 @@ mod tests { let outcome: Result = Err(MxcError::backend_error("output was truncated".to_string())); match finalize_state_aware_outcome(outcome, "exec", false) { - StateAwareExit::ExecError(json) => { + StateAwareExit::ExecError(json, exit_code) => { // Still a parseable envelope; only the stream it lands on changes. let parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); assert!(parsed["error"]["code"].is_string(), "{json}"); + assert_eq!(exit_code, -1); } other => panic!("expected ExecError, got {other:?}"), } @@ -3280,7 +3286,7 @@ mod tests { Err(MxcError::malformed_request("boom".to_string())); assert!(matches!( finalize_state_aware_outcome(outcome, "exec", true), - StateAwareExit::Error(_) + StateAwareExit::Error(_, 1) )); } } diff --git a/tests/scripts/lib/WinProcessContainer.Common.ps1 b/tests/scripts/lib/WinProcessContainer.Common.ps1 index 509bc144a..8cc89c807 100644 --- a/tests/scripts/lib/WinProcessContainer.Common.ps1 +++ b/tests/scripts/lib/WinProcessContainer.Common.ps1 @@ -948,18 +948,41 @@ function Test-WasRejected { if ($both -match '(?i)create_process_failed|CreateProcessSecurityEnvironment failed') { return $false } - # Validation failures surface as config_parse or policy_validation. - if ($both -match '"code"\s*:\s*"backend_error"') { return $false } + # A runtime fault and a host that cannot provide the backend are both + # failures of the run, not judgements of the policy. + if ($both -match '"code"\s*:\s*"(backend_error|backend_unavailable)"') { return $false } # No tier could be built on this host, so the policy was never judged. # Emitted as a ConfigRejected event, which the typed checks above miss. if ($both -match '"reason"\s*:\s*"runner_unavailable"') { return $false } - # Positive evidence. wxc-exec prints this banner for every request it - # refuses, at parse stage and at backend validate. Requiring it means an - # unexplained non-zero exit is NOT counted as a rejection. + # Positive evidence. Every code below blames the request, so each one is a + # refusal and exits 1 per MxcErrorCode::exit_code (lifecycle-state codes + # are omitted because one-shot runs cannot produce them). The two text + # banners cover the paths that still print prose. Requiring one of these + # means an unexplained non-zero exit is NOT counted as a rejection. + $rejectionCodes = 'policy_validation|malformed_request|' + + 'unsupported_containment|unsupported_phase|malformed_id' return [bool]($both -match '(?im)^\s*Request error\s*$' -or $both -match '(?i)Configuration parse error' -or - $both -match '"code"\s*:\s*"(config_parse|policy_validation)"') + $both -match ('"code"\s*:\s*"(' + $rejectionCodes + ')"')) +} + +# A run the host could not service at all, reported as a typed +# `backend_unavailable` envelope on stderr. +# +# Host capability probes answer whether an API is present, which is not the +# same question as whether it is implemented: an export can resolve and still +# return E_NOTIMPL when called. Behavioural assertions that need a working +# provider therefore gate on what a real run reported rather than on the probe +# alone, so such a host skips instead of failing on absent output. +function Test-BackendUnavailable { + param( + [Parameter(Mandatory)] [object]$Run, + [AllowEmptyString()] [string]$Log + ) + $result = $(if ($Run.PSObject.Properties['Result']) { $Run.Result } else { $Run }) + $err = "$(if ($result.PSObject.Properties['Stderr']) { $result.Stderr })" + return [bool]("$Log`n$err" -match '"code"\s*:\s*"backend_unavailable"') } # The non-network counterpart of Get-NetVerdict's NORUN state. diff --git a/tests/scripts/lib/seatbelt_common.sh b/tests/scripts/lib/seatbelt_common.sh index 3e1a1d8f8..e50f503a3 100644 --- a/tests/scripts/lib/seatbelt_common.sh +++ b/tests/scripts/lib/seatbelt_common.sh @@ -189,6 +189,10 @@ expect_rejected() { fail "$label (the workload ran before the config was refused)" "$OUT" fi grep -qF "$marker" <<<"$OUT" || fail "$label (refused, but not for '$marker')" "$OUT" + # A refusal is a request fault: exit 1 with a request-side error code. + if [ "$RC" != 1 ] || ! grep -qE '"code":"(policy_validation|malformed_request)"' <<<"$OUT"; then + fail "$label (expected exit 1 with a policy_validation or malformed_request envelope, got exit $RC)" "$OUT" + fi pass "$label" } diff --git a/tests/scripts/run_bwrap_cwd_advisory_test.sh b/tests/scripts/run_bwrap_cwd_advisory_test.sh index 9e974eaf2..a5eab23dd 100644 --- a/tests/scripts/run_bwrap_cwd_advisory_test.sh +++ b/tests/scripts/run_bwrap_cwd_advisory_test.sh @@ -37,10 +37,21 @@ EOF for case in uncovered unrelated; do rc=0 output=$("$LXC_EXEC" --experimental "$WORK_DIR/$case.json" 2>"$WORK_DIR/$case.stderr") || rc=$? + # One warning envelope must carry the whole advisory, path included. if [ "$rc" -eq 0 ] \ - || ! grep -Fq "WARNING: Bubblewrap: process.cwd \"$CWD\"" "$WORK_DIR/$case.stderr" \ - || ! grep -Fq 'filesystem.readonlyPaths' "$WORK_DIR/$case.stderr" \ + || ! python3 - "$WORK_DIR/$case.stderr" "$CWD" <<'PY' \ || ! grep -Eq '(^|[[:space:]])bwrap:' "$WORK_DIR/$case.stderr"; then +import json, sys +path, cwd = sys.argv[1], sys.argv[2] +for line in open(path, encoding="utf-8"): + try: + message = json.loads(line)["warning"]["message"] + except (ValueError, KeyError, TypeError): + continue + if all(s in message for s in ("Bubblewrap: process.cwd", cwd, "filesystem.readonlyPaths")): + sys.exit(0) +sys.exit(1) +PY echo "$case cwd did not report the hint and Bubblewrap's original error: $output" cat "$WORK_DIR/$case.stderr" exit 1 @@ -48,7 +59,7 @@ for case in uncovered unrelated; do done output=$("$LXC_EXEC" --experimental "$WORK_DIR/covered.json" 2>"$WORK_DIR/covered.stderr") if ! grep -qFx "$CWD" <<<"$output" \ - || grep -Fq 'WARNING: Bubblewrap: process.cwd ' "$WORK_DIR/covered.stderr"; then + || grep -Fq 'Bubblewrap: process.cwd' "$WORK_DIR/covered.stderr"; then echo "Covered cwd did not run without an advisory: $output" cat "$WORK_DIR/covered.stderr" exit 1 diff --git a/tests/scripts/run_bwrap_directional_test.sh b/tests/scripts/run_bwrap_directional_test.sh index 2f3219162..c95a28967 100644 --- a/tests/scripts/run_bwrap_directional_test.sh +++ b/tests/scripts/run_bwrap_directional_test.sh @@ -70,6 +70,11 @@ run_rejected() { echo "FAIL: $label (refused, but not for the expected reason: '$marker')" exit 1 fi + if [ "$rc" != 1 ] || ! grep -qF '"code":"policy_validation"' <<<"$out"; then + echo "$out" + echo "FAIL: $label (expected exit 1 with a policy_validation envelope, got exit $rc)" + exit 1 + fi echo "PASS: $label" } diff --git a/tests/scripts/run_bwrap_version_gate_test.sh b/tests/scripts/run_bwrap_version_gate_test.sh index 4bd45b150..56a12b424 100644 --- a/tests/scripts/run_bwrap_version_gate_test.sh +++ b/tests/scripts/run_bwrap_version_gate_test.sh @@ -56,6 +56,10 @@ assert_gated() { echo "$out" fail "$label (refused, but not for the expected reason: '$marker')" fi + if [ "$rc" != 255 ] || ! grep -qF '"code":"backend_unavailable"' <<<"$out"; then + echo "$out" + fail "$label (expected exit 255 with a backend_unavailable envelope, got exit $rc)" + fi echo "PASS: $label" } diff --git a/tests/scripts/run_isolation_session_tests.ps1 b/tests/scripts/run_isolation_session_tests.ps1 index ea0f65904..8571c13cd 100644 --- a/tests/scripts/run_isolation_session_tests.ps1 +++ b/tests/scripts/run_isolation_session_tests.ps1 @@ -365,11 +365,10 @@ $null = $results.Add((Run-IsolationSessionTest "isolation_session_stderr.json" ` # must appear in the captured output (proves streams aren't crossed or dropped mid-run). $null = $results.Add((Run-IsolationSessionTest "isolation_session_stdout_stderr_interleaved.json" ` -OutputContains @("OUT_A", "ERR_A", "OUT_B", "ERR_B", "OUT_C"))) -# Timeout: ping runs ~30s against a 1500ms deadline. The service-side timer and -# the local wait report different codes, and either can win the race to end the -# run. +# Timeout: ping runs ~30s against a 1500ms deadline. The local wait enforces the +# deadline, so the run reports a typed timeout rather than the service's kill. $null = $results.Add((Run-IsolationSessionTest "isolation_session_timeout.json" ` - -ExpectedExitAnyOf 1, -1)) + -ExpectedExit -1 -OutputContains @('"code":"backend_error"', 'timed out after'))) # Any IsolationSession-specific object is rejected at the same closed exact # contract boundary, before the command can run. @@ -434,7 +433,7 @@ $null = $results.Add((Run-IsolationSessionTest "legacy network posture rejected" # than accepted and dropped. Presence drives the refusal (UiPolicy's default is # full lockdown, so an explicit lockdown `ui` is indistinguishable by value). $null = $results.Add((Run-IsolationSessionTest "isolation_session_one_shot_ui_rejected.json" ` - -ExpectedExit -1 ` + -ExpectedExit 1 ` -OutputContains @("UI policy is not supported"))) # One-shot lifecycle rejection: the in-proc API exposes no session-lifetime @@ -442,11 +441,11 @@ $null = $results.Add((Run-IsolationSessionTest "isolation_session_one_shot_ui_re # returning. `destroyOnExit: true` (the default) matches that and is accepted; # `false` asks for something the backend cannot deliver. $null = $results.Add((Run-IsolationSessionTest "isolation_session_one_shot_lifecycle_rejected.json" ` - -ExpectedExit -1 ` + -ExpectedExit 1 ` -OutputContains @("lifecycle.destroyOnExit=false"))) $null = $results.Add((Run-IsolationSessionTest "isolation_session_one_shot_env_rejected.json" ` - -ExpectedExit -1 ` + -ExpectedExit 1 ` -OutputContains @("process.env without process.inheritDefaultEnv=true is not supported"))) # ---------------- Concurrent one-shot test ---------------- diff --git a/tests/scripts/run_processcontainer_capture_denials_test.ps1 b/tests/scripts/run_processcontainer_capture_denials_test.ps1 index 60195d9b7..dfdf7a35d 100644 --- a/tests/scripts/run_processcontainer_capture_denials_test.ps1 +++ b/tests/scripts/run_processcontainer_capture_denials_test.ps1 @@ -6,8 +6,9 @@ # `outputPath` is validated by validate_capture_denials_output_path during # config parsing, so Phase 14a's four rejections never reach the launch API — # rare process-container coverage that still produces signal on a host which -# cannot start a container. The behavioral half (14b/14c) is host-dependent -# and fails honestly where a run cannot start. +# cannot start a container. The behavioral half (14b/14c) is host-dependent: +# it skips where the host reports `backend_unavailable` for capture, and fails +# honestly where a run cannot start for any other reason. # # Documented contract (docs/schema.md, updated 2026-09-10): # * mode "block" (default) keeps access denied and logs it; "allow" permits @@ -162,7 +163,13 @@ function Phase-CaptureDenialsModes { -CaptureDenialsMode 'allow' $log = Join-Path $ScratchRoot 'logs\cd-allow-warning.log' $r = Invoke-Wxc -Wxc $WxcDebug -ConfigPath $cfg -LogPath $log -TimeoutSec 60 - $all = "$(Read-Log $log)`n$($r.Stderr)" + $logText = Read-Log $log + if (Test-BackendUnavailable -Run $r -Log $logText) { + Record-Result -Phase 'P14b' -Name 'captureDenials mode=allow emits a security warning' -Status 'skip' ` + -Detail 'the host reported backend_unavailable for capture, so no capture session exists to warn about' + return + } + $all = "$logText`n$($r.Stderr)" $sawWarning = [bool]($all -match '(?i)warn.{0,80}(denial|deny|audit|allow)|(?i)(denial|audit).{0,80}warn') Record-Result -Phase 'P14b' -Name 'captureDenials mode=allow emits a security warning' ` -Pass $sawWarning ` @@ -182,11 +189,13 @@ function Phase-CaptureDenialsModes { function Phase-CaptureDenialsStampedPath { Section 'Phase 14c: captureDenials per-run output path' + $assertions = @( + 'the actual denials output path is reported on stderr', + 'the reported path is not the verbatim requested path (a per-run id is stamped in)', + 'two runs of the same config report different output paths') + if (-not $Script:Caps.CaptureDenialsUsable) { - foreach ($name in @( - 'the actual denials output path is reported on stderr', - 'the reported path is not the verbatim requested path (a per-run id is stamped in)', - 'two runs of the same config report different output paths')) { + foreach ($name in $assertions) { Record-Result -Phase 'P14c' -Name $name -Status 'skip' ` -Detail 'no capture provider on this host, so the request fails before an output path is resolved' } @@ -207,6 +216,14 @@ function Phase-CaptureDenialsStampedPath { -CaptureDenialsOutputPath $requested $log = Join-Path $ScratchRoot "logs\cd-stamped-$i.log" $r = Invoke-Wxc -Wxc $WxcDebug -ConfigPath $cfg -LogPath $log -TimeoutSec 60 + if (Test-BackendUnavailable -Run $r -Log (Read-Log $log)) { + foreach ($name in $assertions) { + Record-Result -Phase 'P14c' -Name $name -Status 'skip' ` + -Detail ("the host reported backend_unavailable for capture (run $i), " + + 'so no output path is resolved') + } + return + } # stderr only: the one-line pointer is the CLI-boundary contract under # test, and the backend log carries its own copy of the resolved path, # so including the log would keep this green if the pointer were diff --git a/tests/scripts/run_processcontainer_dacl_disabled_test.ps1 b/tests/scripts/run_processcontainer_dacl_disabled_test.ps1 index 50aa1189f..04e758abf 100644 --- a/tests/scripts/run_processcontainer_dacl_disabled_test.ps1 +++ b/tests/scripts/run_processcontainer_dacl_disabled_test.ps1 @@ -50,16 +50,13 @@ function Phase-DaclDisabled { $stateAfter = @(Get-NewStateFiles) # A bare `exit != 0` is satisfied by a launch failure or a harness timeout, - # which is the false-green pattern this suite exists to remove. This refusal - # is raised during runner resolution, before any launch API, so wxc-exec - # logs a ConfigRejected event with reason `runner_unavailable` and exits 1. - # Test-WasRejected does not apply here: it deliberately treats - # `runner_unavailable` as a non-rejection, because everywhere else it means - # the host could not build a tier and the policy was never judged. + # which is the false-green pattern this suite exists to remove. The authored + # allowDaclMutation=false is what forbids the tier, so this is a typed + # policy refusal raised during tier selection, before any launch API. $stderrOrLog = ($r.Stderr + "`n" + $logContent) - $typedRefusal = [bool]($stderrOrLog -match '"reason"\s*:\s*"runner_unavailable"') + $typedRefusal = [bool]($stderrOrLog -match '"code"\s*:\s*"policy_validation"') $refused = (-not $r.TimedOut) -and ($r.ExitCode -eq 1) -and $typedRefusal - Record-Result -Phase 'P5' -Name 'dispatch refused before launch (typed runner_unavailable, exit 1)' ` + Record-Result -Phase 'P5' -Name 'dispatch refused before launch (typed policy_validation, exit 1)' ` -Pass $refused ` -Detail "exit=$($r.ExitCode); timedOut=$($r.TimedOut); typedRefusal=$typedRefusal" Record-Result -Phase 'P5' -Name 'rw ACL untouched' -Pass ($aclBefore -eq $aclAfter) diff --git a/tests/scripts/run_processcontainer_filesystem_enumerate_test.ps1 b/tests/scripts/run_processcontainer_filesystem_enumerate_test.ps1 index 6b5deb46d..ffbeb3f3a 100644 --- a/tests/scripts/run_processcontainer_filesystem_enumerate_test.ps1 +++ b/tests/scripts/run_processcontainer_filesystem_enumerate_test.ps1 @@ -111,8 +111,9 @@ function Phase-FsEnumerate { $r = Invoke-Wxc -Wxc $WxcDebug -ConfigPath $cfg -LogPath $log -TimeoutSec 30 $combined = "$($r.Stdout)`n$($r.Stderr)`n$(Read-Log $log)" - # wxc-exec exits 1 when it refuses a request; -1 means a launch API - # failed, which would mean the policy was never judged. + # enumeratePaths cannot degrade to an AppContainer tier, so the refusal + # names the field and exits 1; -1 would mean a launch API failed, which + # would mean the policy was never judged. Record-Result -Phase 'P4e' -Name 'unsupported host refuses the run (exit=1)' ` -Pass (-not $r.TimedOut -and $r.ExitCode -eq 1) ` -Detail "exit=$($r.ExitCode); timedOut=$($r.TimedOut)" diff --git a/tests/scripts/run_wslc_all_tests.ps1 b/tests/scripts/run_wslc_all_tests.ps1 index 4c3fa8dbc..19dcb905d 100644 --- a/tests/scripts/run_wslc_all_tests.ps1 +++ b/tests/scripts/run_wslc_all_tests.ps1 @@ -365,13 +365,13 @@ $null = $results.Add((Run-WslcTest "wslc_network_bridged.json" ` # rules, so a per-destination egress rule must be refused rather than quietly # widened to the posture's default. foreach ($ruleConfig in @("wslc_network_egress_rules_rejected.json", "wslc_network_egress_deny_rules_rejected.json")) { - $null = $results.Add((Run-WslcTest $ruleConfig -ExpectedExit -1 ` + $null = $results.Add((Run-WslcTest $ruleConfig -ExpectedExit 1 ` -OutputContains "network.egress allow/deny rules" ` -OutputNotContains "WSLC_EGRESS_RULES_SHOULD_NOT_RUN")) } # `process.env` without `inheritDefaultEnv` is launched through `env -i NAME=VALUE`, # which puts the injected proxy URL in argv and therefore in /proc//cmdline. -$null = $results.Add((Run-WslcTest "wslc_network_proxy_credentials_rejected.json" -ExpectedExit -1 ` +$null = $results.Add((Run-WslcTest "wslc_network_proxy_credentials_rejected.json" -ExpectedExit 1 ` -OutputContains "must not carry credentials" ` -OutputNotContains "WSLC_PROXY_CREDENTIALS_SHOULD_NOT_RUN")) # Delegate the cooperative proxy fixture to its owning script, which asserts @@ -431,11 +431,11 @@ $null = $results.Add((Run-WslcTest "wslc_cold_cache_pull.json" -ForceDebug ` -OutputContains "Image 'busybox:latest' found" -OutputNotContains "Pulling image")) # The store is warm now, so a miss here can only come from the denied posture. Clear-ColdCacheStore -$null = $results.Add((Run-WslcTest "wslc_cold_cache_denied_egress.json" -ExpectedExit -1 ` +$null = $results.Add((Run-WslcTest "wslc_cold_cache_denied_egress.json" -ExpectedExit 1 ` -OutputContains "declares no egress" -OutputNotContains "SHOULD_NOT_RUN")) # `Check the image name and tag` is unique to the non-retryable arm; every arm # says `could not be pulled`. -$null = $results.Add((Run-WslcTest "wslc_cold_cache_unresolvable.json" -ExpectedExit -1 ` +$null = $results.Add((Run-WslcTest "wslc_cold_cache_unresolvable.json" -ExpectedExit 1 ` -OutputContains "Check the image name and tag")) Clear-ColdCacheStore @@ -465,7 +465,7 @@ $null = $results.Add((Run-WslcTest "wslc_destroy_on_exit_true.json" ` # deliver. It is refused before any container is created, so the payload must # never run. $null = $results.Add((Run-WslcTest "wslc_destroy_on_exit_false_rejected.json" ` - -ExpectedExit -1 ` + -ExpectedExit 1 ` -OutputContains "destroyOnExit=false")) Write-Host "`n--- State-Aware Lifecycle Tests ---" -ForegroundColor Cyan