diff --git a/docs/backends/bwrap/bubblewrap-backend.md b/docs/backends/bwrap/bubblewrap-backend.md index f0122d384..92d55b1d0 100644 --- a/docs/backends/bwrap/bubblewrap-backend.md +++ b/docs/backends/bwrap/bubblewrap-backend.md @@ -991,6 +991,21 @@ isolated wrapper that removes `--die-with-parent`, then checks explicit kill, timeout, and handle-drop cleanup. This reproduces the absence of parent-death protection during startup without relying on a scheduling race. +## 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 eee82f05d..553b6f67f 100644 --- a/docs/backends/lxc/lxc-backend.md +++ b/docs/backends/lxc/lxc-backend.md @@ -324,6 +324,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 4a4ef5166..972f6d397 100644 --- a/docs/backends/wslc/wsl-container-getting-started.md +++ b/docs/backends/wslc/wsl-container-getting-started.md @@ -325,8 +325,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.** MXC does not forward stdin to a WSL container, so `Sandbox::take_stdin()` returns `None` for it. - **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 12ef61bb3..530f42e81 100644 --- a/docs/development/architecture/backends/isolation-session/oneshot.md +++ b/docs/development/architecture/backends/isolation-session/oneshot.md @@ -254,11 +254,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 bace284f5..0f62f454a 100644 --- a/docs/development/architecture/backends/isolation-session/state-aware-rust.md +++ b/docs/development/architecture/backends/isolation-session/state-aware-rust.md @@ -341,10 +341,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 @@ -496,13 +499,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. @@ -550,7 +551,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 c1aac9731..6cf65aaec 100644 --- a/docs/development/architecture/container-lifecycle.md +++ b/docs/development/architecture/container-lifecycle.md @@ -613,9 +613,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. @@ -668,7 +668,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 20920c7ca..006e211ae 100644 --- a/docs/development/architecture/versioning.md +++ b/docs/development/architecture/versioning.md @@ -694,6 +694,45 @@ 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 host +cannot enforce (for example ProcessContainer `filesystem.enumeratePaths`, +`filesystem.deniedPaths`, or LPAC) is a `policy_validation` rejection naming +that field, never a silent drop; but a missing host capability the policy +never reached (for example no native PSEC, or no denial-capture provider) is +`backend_unavailable`. 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 a1b070d75..a878ce147 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, @@ -524,7 +538,7 @@ impl BubblewrapScriptRunner { Ok(file) => file, Err(error) => { stop_proxy_network(&mut proxy_network, logger); - return Err(ScriptResponse::error(&error)); + return Err(ScriptResponse::failed(FailurePhase::LaunchFailed, &error)); } }; let mut args = bwrap_command::build_args_classified_with_mode( @@ -543,7 +557,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, @@ -587,9 +601,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}"), + )); } }, }; @@ -616,10 +631,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), + )); } }; @@ -642,7 +657,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, + )); } } @@ -665,10 +683,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 { @@ -2145,6 +2166,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 72489cb3d..8a3b896ba 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 164fdae5e..dcec019ad 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", @@ -120,8 +121,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(); @@ -157,13 +158,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"), + ), } } @@ -174,6 +189,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 @@ -249,6 +280,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 e587e9cec..886ae8ed6 100644 --- a/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs +++ b/src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs @@ -11,7 +11,8 @@ use std::time::{Duration, Instant}; use crate::mxc_common::host_is_canonical_loopback; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{ - ContainerPolicy, ExecutionRequest, LifecycleConfig, LxcConfig, NetworkAction, ScriptResponse, + ContainerPolicy, ExecutionRequest, FailurePhase, LifecycleConfig, LxcConfig, NetworkAction, + ScriptResponse, }; use crate::mxc_common::proxy_env::{apply_proxy_env, proxy_url_has_credentials, redact_proxy_url}; use crate::mxc_common::sandbox_process::{SandboxBackend, SandboxProcess, StdioMode}; @@ -31,6 +32,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::loopback_relay::LoopbackRelay; use crate::lxc_common::lxc_bindings::{ContainerFirewall, LxcContainer, StartNetwork}; @@ -207,13 +209,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 + ), + ) }) } @@ -225,7 +230,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.", @@ -398,13 +404,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 @@ -428,14 +437,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\")", )); @@ -447,7 +456,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 { @@ -476,10 +499,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; @@ -493,10 +516,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 @@ -510,11 +533,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 + ), + )); } } @@ -535,10 +561,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."); @@ -567,7 +593,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.", @@ -593,7 +620,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)); } }; @@ -613,7 +640,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)); } } @@ -632,12 +659,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. @@ -659,11 +689,14 @@ impl LxcScriptRunner { Ok(relay) => Some(relay), Err(e) => { self.release_after_failure(&container, container_created, logger); - return Err(ScriptResponse::error(&format!( - "LXC: cannot relay runtimeConfig.networkProxy into the container: \ + return Err(ScriptResponse::failed( + FailurePhase::PostLaunchFailed, + &format!( + "LXC: cannot relay runtimeConfig.networkProxy into the container: \ {}. The script was not run.", - e - ))); + e + ), + )); } } } @@ -975,6 +1008,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() && !policy.runtime_network_proxy_specified { Some("network.proxy") @@ -995,7 +1033,7 @@ impl LxcScriptRunner { fn validate_runtime_proxy(&self, policy: &ContainerPolicy) -> Result<(), ScriptResponse> { if let Some(address) = &policy.network_proxy.address { if !host_is_canonical_loopback(address.host()) { - return Err(ScriptResponse::error(&format!( + return Err(ScriptResponse::rejected(&format!( "LXC: runtimeConfig.networkProxy host '{}' must be localhost, 127.0.0.1, or \ ::1. The proxy is relayed into the container at the same loopback address \ and port.", @@ -1004,7 +1042,7 @@ impl LxcScriptRunner { } let url = address.to_url(); if proxy_url_has_credentials(&url) { - return Err(ScriptResponse::error(&format!( + return Err(ScriptResponse::rejected(&format!( "LXC: runtimeConfig.networkProxy must not carry credentials ('{}'). LXC \ passes the proxy URL to lxc-attach as a --set-var command-line argument, \ and process arguments are world-readable through /proc//cmdline.", @@ -1017,7 +1055,7 @@ impl LxcScriptRunner { || !egress.allow.is_empty() || !egress.deny.is_empty() }) { - return Err(ScriptResponse::error( + return Err(ScriptResponse::rejected( "LXC: runtimeConfig.networkProxy cannot be combined with network.egress rules or \ default='allow'. The relayed proxy port is the container's only outbound path. \ Use network.egress.default='deny' with no rules, or remove the proxy.", @@ -1033,7 +1071,7 @@ impl ScriptRunner for LxcScriptRunner { self.validate_runtime_proxy(&request.policy)?; } 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." ))); @@ -1092,7 +1130,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 => {} } @@ -1150,10 +1188,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), + )); } }; @@ -1180,13 +1218,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, - ))); + )); } }; @@ -1216,7 +1257,7 @@ impl LxcScriptRunner { _logger: &mut Logger, _stdio: StdioMode, ) -> Result, ScriptResponse> { - Err(ScriptResponse::error(LXC_STREAMING_LINUX_ONLY)) + Err(ScriptResponse::unavailable(LXC_STREAMING_LINUX_ONLY)) } } @@ -2728,6 +2769,7 @@ mod tests { .validate_runner(&runtime_proxy_request(host)) .expect_err("a wildcard host is not a loopback endpoint the relay can serve"); + assert_eq!(response.failure_phase, FailurePhase::Rejected); assert!( response .error_message @@ -2768,6 +2810,7 @@ mod tests { "with a runtime proxy, the relayed port is the container's only outbound path", ); + assert_eq!(response.failure_phase, FailurePhase::Rejected); assert!( response.error_message.contains("network.egress"), "got: {}", @@ -2812,6 +2855,7 @@ mod tests { .validate_runner(&request) .expect_err("credentials would be visible in /proc//cmdline"); + assert_eq!(response.failure_phase, FailurePhase::Rejected); assert!( response.error_message.contains("credentials") && !response.error_message.contains("secret"), @@ -2976,9 +3020,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" ); } @@ -3177,6 +3221,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/base_container_runner.rs b/src/mxc-sdk/src/backends/process_container/common/base_container_runner.rs index 47e1da14b..aa4aca8f6 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 @@ -43,6 +43,7 @@ use crate::mxc_common::models::{ CaptureDenialsErrorOutput, CaptureDenialsOutput, ContainmentBackend, ExecutionRequest, FailurePhase, SandboxOutputMetadata, ScriptResponse, }; +use crate::mxc_common::mxc_error::MxcError; use crate::mxc_common::process_util::{ create_std_pipes, InterruptiblePipeReader, OwnedHandle, PipeReadCanceller, PipeWriter, SendOwnedHandle, @@ -146,6 +147,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)"; @@ -276,6 +280,22 @@ impl BaseContainerRequestDecision { ), } } + + /// The typed error for a request native PSEC cannot service: a missing + /// host capability is `backend_unavailable`, a field PSEC cannot enforce is + /// `policy_validation`. + pub(crate) fn rejection_error(self) -> Option { + let message = self.rejection_message()?; + Some(match self { + Self::Serviceable => return None, + Self::Unavailable | Self::NativeCaptureUnavailable => { + MxcError::backend_unavailable(message) + } + Self::PolicyIncompatible + | Self::DeniedPathsUnsupported + | Self::EnumeratePathsUnsupported => MxcError::policy_validation(message), + }) + } } /// Script runner that launches through a process security environment. @@ -301,9 +321,9 @@ impl BaseContainerRunner { /// Construct a runner for `request`, or return why native PSEC cannot fully /// enforce it. The serviceability decision is made once here, so /// validation during execution does not repeat the OS capability check. - pub(crate) fn for_request(request: &ExecutionRequest) -> Result { - match Self::can_backend_service_request(request).rejection_message() { - Some(reason) => Err(reason), + pub(crate) fn for_request(request: &ExecutionRequest) -> Result { + match Self::can_backend_service_request(request).rejection_error() { + Some(error) => Err(error), None => Ok(Self { request_serviceability_confirmed: true, ..Self::default() @@ -427,6 +447,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. @@ -494,20 +531,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, )); } @@ -779,13 +811,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)); } } } else { @@ -813,13 +839,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)); } } } @@ -872,13 +892,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)); } }; let environment = (!current_env_ptr.is_null()).then_some(current_env_ptr); @@ -960,12 +974,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) }); } @@ -1037,12 +1047,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) }); } }; @@ -1076,10 +1082,7 @@ impl BaseContainerRunner { "; additionally failed to discard the learning-mode trace: {error}" ); } - 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( @@ -1183,13 +1186,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( @@ -2052,6 +2052,7 @@ mod tests { NetworkCidr, NetworkEgressPolicy, NetworkIngressPolicy, NetworkPeer, NetworkPort, NetworkProtocol, NetworkRule, ProxyAddress, ProxyConfig, UiPolicy, }; + use crate::mxc_common::mxc_error::MxcErrorCode; use crate::mxc_common::network_parser::{parse_network_policy, NetworkSections}; use crate::mxc_common::ui_policy::EffectiveUiRestrictions; use crate::mxc_common::wire; @@ -2059,6 +2060,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( @@ -3725,11 +3756,13 @@ mod tests { crate::process_container_common::test_env::CaptureCapabilityGuard::set(true, true); let mut request = ExecutionRequest::default(); request.policy.least_privilege_mode = true; + let error = BaseContainerRunner::for_request(&request) + .err() + .expect("LPAC must be rejected"); + assert_eq!(error.code, MxcErrorCode::PolicyValidation); assert_eq!( - BaseContainerRunner::for_request(&request).err(), - Some( - "Windows ProcessContainer cannot enforce the requested LPAC or legacy network.proxy policy" - ) + error.message, + "Windows ProcessContainer cannot enforce the requested LPAC or legacy network.proxy policy" ); } @@ -3741,18 +3774,21 @@ mod tests { DeniedPathsUnsupported, EnumeratePathsUnsupported, NativeCaptureUnavailable, }; - for (decision, message) in [ + for (decision, message, code) in [ ( DeniedPathsUnsupported, "Windows ProcessContainer requires native PSEC denied-path support for this request", + MxcErrorCode::PolicyValidation, ), ( EnumeratePathsUnsupported, "Windows ProcessContainer requires native PSEC enumeration-only access support for this request", + MxcErrorCode::PolicyValidation, ), ( NativeCaptureUnavailable, "Windows ProcessContainer requires native Learning Mode capture support for this request", + MxcErrorCode::BackendUnavailable, ), ] { let _guard = PolicyCapabilityGuard::set( @@ -3771,7 +3807,10 @@ mod tests { BaseContainerRunner::can_backend_service_request(&request), decision ); - assert_eq!(BaseContainerRunner::for_request(&request).err(), Some(message)); + let error = BaseContainerRunner::for_request(&request) + .err() + .expect("an unsupported capability must be rejected"); + assert_eq!((error.code, error.message.as_str()), (code, message)); let probe = run_probe(&request); assert_eq!(probe.tier, None); assert_eq!(probe.needs_dacl_augmentation, None); 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 10cfde4e3..a5629f86c 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 626a8e707..3e0d1f630 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 e465fded1..9da7864c0 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 { @@ -59,6 +59,36 @@ impl From for WxcError { } } +impl WxcError { + /// The wire code this failure is reported under, grouped by who can act on + /// it: the request, the host, or neither. + pub fn error_code(&self) -> MxcErrorCode { + match self { + WxcError::StateAware(error) => error.code, + WxcError::ConfigParse(_) | WxcError::StringConversion(_) => { + MxcErrorCode::MalformedRequest + } + WxcError::Validation(_) => MxcErrorCode::PolicyValidation, + 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()) @@ -71,3 +101,35 @@ pub const DENIED_PATHS_FEATURE_DISABLED_MSG: &str = OS build: it does not advertise PSE_SUPPORT_FS_DENY through \ QueryProcessSecurityEnvironmentSupport. Run on a build with \ BaseContainer deny support or remove filesystem.deniedPaths."; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_refused_policy_is_told_apart_from_a_host_fault() { + assert_eq!( + WxcError::Validation("denied".into()).error_code(), + MxcErrorCode::PolicyValidation + ); + assert_eq!( + WxcError::Io("disk gone".into()).error_code(), + MxcErrorCode::BackendError + ); + } + + #[test] + fn an_unreadable_request_is_malformed() { + assert_eq!( + WxcError::ConfigParse("bad json".into()).error_code(), + MxcErrorCode::MalformedRequest + ); + } + + #[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 d9524696a..66524216c 100644 --- a/src/mxc-sdk/src/core/mxc_common/models.rs +++ b/src/mxc-sdk/src/core/mxc_common/models.rs @@ -1110,9 +1110,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 @@ -1125,26 +1124,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)] @@ -1162,6 +1173,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>, @@ -1176,6 +1198,7 @@ impl Default for ScriptResponse { error_message: String::new(), extended_error: String::new(), failure_phase: FailurePhase::None, + warnings: None, output_metadata: None, } } @@ -1250,13 +1273,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)] @@ -1264,18 +1321,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(), @@ -1285,6 +1345,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"); @@ -1297,6 +1414,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 2eeb7027b..5525752f1 100644 --- a/src/mxc-sdk/src/core/mxc_engine/dispatch.rs +++ b/src/mxc-sdk/src/core/mxc_engine/dispatch.rs @@ -276,8 +276,7 @@ fn spawn_process_container( use crate::mxc_common::sandbox_process::{SandboxBackend, StdioMode}; use crate::process_container_common::base_container_runner::BaseContainerRunner; - let mut backend = - BaseContainerRunner::for_request(request).map_err(MxcError::backend_unavailable)?; + let mut backend = BaseContainerRunner::for_request(request)?; backend .spawn(request, logger, StdioMode::Pipes) .map_err(map_spawn_error) @@ -315,7 +314,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.", )) } @@ -369,7 +368,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.", )) } @@ -389,7 +388,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.", )) } @@ -551,6 +550,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 80d81a911..d37d4ec93 100644 --- a/src/mxc-sdk/src/core/mxc_engine/run.rs +++ b/src/mxc-sdk/src/core/mxc_engine/run.rs @@ -53,9 +53,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, @@ -161,11 +164,21 @@ pub fn resolve_runner_for_audit( /// Resolve `request`'s backend and run it to completion. /// /// Convenience over [`resolve_runner`] that runs the resolved backend to -/// completion. +/// completion. 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); - 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); + response.extend_warnings(logger.take_warnings()); + response } // --------------------------------------------------------------------------- @@ -184,11 +197,9 @@ fn resolve_runner_inner_windows( use crate::mxc_common::sandbox_process::Runner; use crate::process_container_common::base_container_runner::BaseContainerRunner; - BaseContainerRunner::for_request(request) - .map(|backend| ResolvedRunner { - runner: Box::new(Runner::new(backend)), - }) - .map_err(MxcError::backend_unavailable) + BaseContainerRunner::for_request(request).map(|backend| ResolvedRunner { + runner: Box::new(Runner::new(backend)), + }) } ContainmentBackend::Wslc => { #[cfg(feature = "wslc")] @@ -202,7 +213,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.", )) } @@ -228,7 +239,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)", )) } @@ -240,11 +251,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", ); } } @@ -261,7 +270,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.", )) } @@ -292,7 +301,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)", )) } @@ -434,8 +443,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).", )) } } @@ -499,6 +509,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()); } } @@ -513,7 +524,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")); @@ -533,4 +544,72 @@ 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 an_unenforceable_process_container_field_is_a_policy_validation_error() { + let _guard = crate::process_container_common::test_env::PolicyCapabilityGuard::set( + true, false, true, + ); + 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 25f296a83..a67676722 100644 --- a/src/tools/wxc/src/main.rs +++ b/src/tools/wxc/src/main.rs @@ -16,8 +16,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; @@ -366,7 +368,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}")]); } } @@ -584,9 +586,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); @@ -668,15 +668,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); } } } @@ -690,10 +690,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 @@ -709,22 +709,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<'_> { @@ -732,8 +738,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), } } @@ -742,10 +748,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) { @@ -798,18 +802,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)> { @@ -894,15 +889,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 { @@ -910,17 +907,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}" + ))), } if decoded_config.is_some() && output.error.is_some() { process::exit(1); @@ -1079,9 +1072,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 => { @@ -1096,8 +1087,7 @@ fn main() { eprint!("{}", logger.get_buffer()); process::exit(1); } - eprintln!("Error: {message}"); - process::exit(1); + emit_mxc_error_exit(&MxcError::malformed_request(message)); } }; if let Some(operation) = cli.operation { @@ -1172,15 +1162,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); } }; @@ -1231,35 +1222,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(), + ]); } } @@ -1273,9 +1263,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, @@ -1283,7 +1270,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. @@ -1382,22 +1371,23 @@ fn main() { let mut runner: Box = match resolved_runner { Ok(resolved) => 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); } }; @@ -1417,22 +1407,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(); } } } @@ -1444,11 +1440,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( @@ -1766,7 +1762,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) @@ -1856,28 +1852,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(_) )); } @@ -2111,7 +2112,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" ); @@ -2150,7 +2151,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 = @@ -2171,7 +2172,7 @@ mod tests { .unwrap_err(); assert!(matches!( request_error_route(&error), - RequestErrorRoute::Diagnostic + RequestErrorRoute::Stderr(_) )); assert!(logger.get_buffer().contains("duplicate field `telemetry`")); } @@ -2926,11 +2927,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:?}"), } @@ -2941,10 +2943,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:?}"), } @@ -2956,7 +2959,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/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 6440ffde2..b41ead422 100644 --- a/tests/scripts/run_isolation_session_tests.ps1 +++ b/tests/scripts/run_isolation_session_tests.ps1 @@ -361,11 +361,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. @@ -430,7 +429,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 @@ -438,11 +437,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_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