diff --git a/README.md b/README.md index 19b909578..8033b462c 100644 --- a/README.md +++ b/README.md @@ -147,8 +147,19 @@ wxc-exec.exe --config-base64 # Debug output wxc-exec.exe --debug config.json + +# Supply or replace process.commandLine from trailing arguments +wxc-exec.exe config.json -- python --version ``` +For `wxc-exec.exe`, arguments after the required `--` separator are rendered +for the selected backend and spliced into `process.commandLine` before the +request is parsed. They may supply a missing command or replace the policy's +command. This form is supported for one-shot requests and state-aware `exec`; +other state-aware phases reject it. A policy that relies on trailing arguments +is a CLI template rather than a complete request that can be executed +independently. + On Linux: `./lxc-exec config.json` On macOS: `./mxc-exec-mac --experimental config.json` diff --git a/docs/schema.md b/docs/schema.md index 74094f9d9..08dae8a8f 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -80,6 +80,13 @@ schema 0.6 and 0.7. During the additive schema 0.8 transition, requests may continue to use those legacy fields or use the directional fields above, but cannot mix both formats in one request. +Every complete request that carries a process requires a non-empty +`process.commandLine`. The Windows native CLI may accept a template without +that field when the command is supplied after `--`; `wxc-exec.exe` inserts or +replaces `process.commandLine` before schema and typed request validation. That +entry-point transform does not make the unmodified template a complete request +that can be executed independently. + ### Full Schema ```json diff --git a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md index c424efde4..8e4a9ec31 100644 --- a/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md +++ b/docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md @@ -1006,11 +1006,14 @@ fields (`filesystem` / `network` / `ui`) on a per-(backend, phase) Config map di to top-level wire fields — they are already wire-format-aligned in the Config, so the SDK passes them through unchanged. Cross-backend exec fields (`commandLine`, `cwd`, `env`, `timeout`) flow through the top-level `process` block, not through -`experimental`. For non-exec phases the executor emits a single JSON envelope on -stdout; for exec the script's output streams raw and the SDK constructs the result -from PTY events. Responses unwrap any `result` envelope at the SDK boundary so the -caller sees a plain `ProvisionResult` / `StartResult` / `ExecResult` / `StopResult` / -`DeprovisionResult`. +`experimental`. The typed SDK requires `commandLine`. The native `wxc-exec.exe` +entry point may instead complete an `exec` template from arguments after `--`; +it inserts or replaces `process.commandLine` before parsing. Trailing commands +are rejected for every non-exec phase. For non-exec phases the executor emits a +single JSON envelope on stdout; for exec the script's output streams raw and the +SDK constructs the result from PTY events. Responses unwrap any `result` +envelope at the SDK boundary so the caller sees a plain `ProvisionResult` / +`StartResult` / `ExecResult` / `StopResult` / `DeprovisionResult`. ## 8. Error model @@ -1102,24 +1105,39 @@ domain types for trivial enum/struct conversions). The state-aware path reuses this wire model while retaining its per-backend `experimental` subtree for dispatch-time typing. +When the native CLI supplies trailing command arguments, the loader first +splices the rendered command into `process.commandLine` and then parses that +effective document. Diagnostics and retained state-aware source text are +therefore relative to the effective document; replacing or inserting the +command may shift a later same-line column from its position in the caller's +original bytes. + ```rust -// In config_parser.rs — discrimination is by presence of the `phase` key in -// the source JSON without building a full untyped request tree. +// In load_mxc_request_with_options — complete a CLI template first. +let (json_str, override_log) = if opts.cli_command.is_empty() { + (json_str, None) +} else { + apply_cli_command(&json_str, opts.cli_command)? +}; +let request = parse_mxc_request_json(&json_str, logger)?; + +// In parse_mxc_request_json — discriminate and run the normal typed parser. let discriminator: RequestDiscriminator<'_> = - config_deserialize::from_str(&json_str)?; + config_deserialize::from_str(json_str)?; if discriminator.phase.is_some() { - convert_wire_state_aware( - &json_str, - discriminator.experimental, - logger, - allow_missing_command, - ) + convert_wire_state_aware(json_str, discriminator.experimental, logger) } else { - let cfg: wire::MxcConfig = config_deserialize::from_str(&json_str)?; - convert_wire_config(cfg, logger, true, allow_missing_command) + let cfg: wire::MxcConfig = config_deserialize::from_str(json_str)?; + convert_wire_config(cfg, logger, true, false) } ``` +The trailing-command path keeps the exact phase probe separate because it owns +error routing. Backend selection and command editing share one +duplicate-preserving raw root parse, and the resulting effective document then +uses the authoritative parser above. The ordinary path with no trailing command +enters `parse_mxc_request_json` directly. + `wire::MxcConfig` is closed (`deny_unknown_fields`) on its stable surface, so unknown fields are rejected at the trust boundary. `phase` maps to the `wire::Phase` enum. The `experimental` block stays permissive and is captured as @@ -1598,6 +1616,11 @@ shapes. | MXC dispatch common (Rust) | Cross-backend per-phase invariants (e.g., `validate_exec_common` checks `process.commandLine` non-empty) | `error.code: malformed_request`, `policy_validation` | | Backend `validate_` hooks (Rust) | Per-backend per-phase invariants: config field values, cross-cutting policy honor (per the matrix in §10.3), id format checks beyond prefix matching | `error.code: policy_validation`, `malformed_id`, `stale_id`, `backend_error`, `backend_unavailable` | +The native CLI template form is resolved before these layers: a trailing +command on state-aware `exec` supplies or replaces `process.commandLine`, while +other phases reject trailing commands. The effective request presented to the +Rust parser still contains the required non-empty command. + Each layer validates only what it cheaply can. The SDK's typed config catches structural errors at compile time. The dispatch layer catches structural errors that escaped the SDK (e.g., from non-TypeScript callers). The backend catches semantic errors that depend diff --git a/docs/wsl/wsl-container-getting-started.md b/docs/wsl/wsl-container-getting-started.md index 304ba0696..50f9a9034 100644 --- a/docs/wsl/wsl-container-getting-started.md +++ b/docs/wsl/wsl-container-getting-started.md @@ -198,9 +198,8 @@ let wslc = WslcSection { ..Default::default() }; -let mut request = build_request_with_containment(&policy, &Containment::Wslc(wslc), None)?; +let mut request = build_request_with_containment(&policy, &Containment::Wslc(wslc), "python3 -c \"print('Hello from WSLC')\"", None)?; request - .set_script("python3 -c \"print('Hello from WSLC')\"") .set_experimental(true); // Run to completion, capturing output… diff --git a/src/core/mxc-sdk/README.md b/src/core/mxc-sdk/README.md index 8ac2036f9..dfaceae4a 100644 --- a/src/core/mxc-sdk/README.md +++ b/src/core/mxc-sdk/README.md @@ -24,8 +24,8 @@ let policy = SandboxPolicy { ui: None, timeout_ms: Some(10_000), }; -let mut request = build_request(&policy, None)?; -request.set_script("echo hello").set_telemetry_opt_in(true); +let mut request = build_request(&policy, "echo hello", None)?; +request.set_telemetry_opt_in(true); let output = run(request)?; assert_eq!(output.outcome, WaitOutcome::Exited(0)); @@ -40,9 +40,9 @@ Ok(()) [`build_request`] resolves the host's default containment backend (see [Supported backends](#supported-backends)), builds the wire config, and runs it -through the shared parser. The returned [`SandboxRequest`] has an empty command -line — set the command with [`SandboxRequest::set_script`] (and any working -directory / env) before spawning. +through the shared parser. The command is supplied to [`build_request`], so the +returned [`SandboxRequest`] is complete; optionally adjust its working directory +or environment before spawning. Telemetry remains off unless `SandboxRequest::set_telemetry_opt_in(true)` is called. Enabling that per-invocation switch still requires persisted user @@ -84,6 +84,7 @@ process_container.capture_denials = Some(CaptureDenials::default()); let request = build_request_with_containment( &policy, &Containment::ProcessContainer(process_container), + "echo hello", None, )?; # Ok::<(), mxc_sdk::Error>(()) @@ -242,9 +243,8 @@ let policy = SandboxPolicy { ui: None, timeout_ms: None, }; -let mut request = build_request(&policy, None)?; -request.set_script("cat"); // echoes stdin until EOF - +// echoes stdin until EOF +let request = build_request(&policy, "cat", None)?; let mut proc = spawn_sandbox(request)?; let mut stdin = proc.take_stdin().unwrap(); let mut stdout = proc.take_stdout().unwrap(); @@ -437,8 +437,13 @@ let policy = SandboxPolicy { filesystem: None, network: None, ui: None, timeout_ms: None, }; let wslc = WslcSection { image: "python:3.12".to_string(), ..Default::default() }; -let mut request = build_request_with_containment(&policy, &Containment::Wslc(wslc), None)?; -request.set_script("python3 -c 'print(42)'").set_experimental(true); +let mut request = build_request_with_containment( + &policy, + &Containment::Wslc(wslc), + "python3 -c 'print(42)'", + None, +)?; +request.set_experimental(true); let output = run(request)?; let _ = output; Ok(()) diff --git a/src/core/mxc-sdk/src/lib.rs b/src/core/mxc-sdk/src/lib.rs index d3960ee21..17d403b91 100644 --- a/src/core/mxc-sdk/src/lib.rs +++ b/src/core/mxc-sdk/src/lib.rs @@ -25,8 +25,7 @@ //! ui: None, //! timeout_ms: None, //! }; -//! let mut request = build_request(&policy, None)?; -//! request.set_script("echo hi"); +//! let request = build_request(&policy, "echo hi", None)?; //! let output = run(request)?; //! match output.outcome { //! WaitOutcome::Exited(code) => println!("exit={code}"), @@ -93,8 +92,8 @@ //! # }; //! // Run a command inside a WSL container (Windows, --features wslc). //! let wslc = WslcSection { image: "python:3.12".to_string(), ..Default::default() }; -//! let mut request = build_request_with_containment(&policy, &Containment::Wslc(wslc), None)?; -//! request.set_script("python3 -c 'print(42)'").set_experimental(true); +//! let mut request = build_request_with_containment(&policy, &Containment::Wslc(wslc), "python3 -c 'print(42)'", None)?; +//! request.set_experimental(true); //! let output = run(request)?; //! # Ok::<(), mxc_sdk::Error>(()) //! ``` diff --git a/src/core/mxc-sdk/tests/isolation_session.rs b/src/core/mxc-sdk/tests/isolation_session.rs index 70641bd19..a0015f2b5 100644 --- a/src/core/mxc-sdk/tests/isolation_session.rs +++ b/src/core/mxc-sdk/tests/isolation_session.rs @@ -107,12 +107,14 @@ fn a_single_threaded_apartment_is_refused_before_the_service_is_reached() { #[test] fn one_shot_run_refuses_the_backend() { - let mut request = - build_request_with_containment(&iso_policy(), &Containment::IsolationSession, None) - .expect("building the request must succeed — the refusal is at dispatch, not build"); - request - .set_script("cmd.exe /c echo hi") - .set_experimental(true); + let mut request = build_request_with_containment( + &iso_policy(), + &Containment::IsolationSession, + "cmd.exe /c echo hi", + None, + ) + .expect("building the request must succeed — the refusal is at dispatch, not build"); + request.set_experimental(true); let err = mxc_sdk::run(request).expect_err("one-shot run must refuse IsolationSession"); assert_eq!( @@ -126,12 +128,14 @@ fn one_shot_run_refuses_the_backend() { #[test] fn one_shot_spawn_refuses_the_backend() { - let mut request = - build_request_with_containment(&iso_policy(), &Containment::IsolationSession, None) - .expect("building the request must succeed — the refusal is at dispatch, not build"); - request - .set_script("cmd.exe /c echo hi") - .set_experimental(true); + let mut request = build_request_with_containment( + &iso_policy(), + &Containment::IsolationSession, + "cmd.exe /c echo hi", + None, + ) + .expect("building the request must succeed — the refusal is at dispatch, not build"); + request.set_experimental(true); // `Sandbox` is not `Debug`, so match rather than `expect_err`. match mxc_sdk::spawn_sandbox(request) { diff --git a/src/core/mxc-sdk/tests/sandbox.rs b/src/core/mxc-sdk/tests/sandbox.rs index 807d55def..5cff9b441 100644 --- a/src/core/mxc-sdk/tests/sandbox.rs +++ b/src/core/mxc-sdk/tests/sandbox.rs @@ -33,9 +33,7 @@ fn seatbelt_request(command: &str, timeout_ms: u32) -> SandboxRequest { Some(timeout_ms) }, }; - let mut request = build_request(&policy, None).expect("build_request should succeed"); - request.set_script(command); - request + build_request(&policy, command, None).expect("build_request should succeed") } /// A Windows ProcessContainer request exposing `C:\Windows\Temp` read-write. @@ -58,9 +56,7 @@ fn process_container_request(version: &str, command: &str, timeout_ms: u32) -> S Some(timeout_ms) }, }; - let mut request = build_request(&policy, None).expect("build_request should succeed"); - request.set_script(command); - request + build_request(&policy, command, None).expect("build_request should succeed") } /// Outcome of running a sandbox to completion via the streaming API. @@ -125,8 +121,8 @@ fn version_older_than_supported_is_rejected() { timeout_ms: None, }; - let err = - build_request(&policy, None).expect_err("an out-of-range schema version must be rejected"); + let err = build_request(&policy, "echo hello", None) + .expect_err("an out-of-range schema version must be rejected"); assert_eq!(err.code, ErrorCode::MalformedRequest); } diff --git a/src/core/mxc-sdk/tests/sdk_helpers.rs b/src/core/mxc-sdk/tests/sdk_helpers.rs index 07cc840d9..522e976af 100644 --- a/src/core/mxc-sdk/tests/sdk_helpers.rs +++ b/src/core/mxc-sdk/tests/sdk_helpers.rs @@ -129,7 +129,8 @@ fn build_request_rejects_empty_version() { timeout_ms: None, }; - let err = build_request(&policy, None).expect_err("an empty policy version must be rejected"); + let err = build_request(&policy, "echo hello", None) + .expect_err("an empty policy version must be rejected"); assert_eq!(err.code, mxc_sdk::ErrorCode::MalformedRequest); } @@ -148,7 +149,7 @@ fn build_request_host_rules_require_outbound() { // Unix backends accept host rules without `allowOutbound`; only Windows // ProcessContainer requires it. Either way this must not panic. - let result = build_request(&policy, None); + let result = build_request(&policy, "echo hello", None); if cfg!(any(target_os = "linux", target_os = "macos")) { assert!( result.is_ok(), @@ -180,7 +181,8 @@ fn rust_sdk_builds_legacy_networking() { timeout_ms: None, }; - build_request(&policy, None).expect("the Rust SDK should build legacy networking"); + build_request(&policy, "echo hello", None) + .expect("the Rust SDK should build legacy networking"); } #[test] @@ -206,7 +208,8 @@ fn rust_sdk_builds_directional_networking() { timeout_ms: None, }; - build_request(&policy, None).expect("the Rust SDK should build directional networking"); + build_request(&policy, "echo hello", None) + .expect("the Rust SDK should build directional networking"); } #[test] @@ -246,6 +249,7 @@ fn rust_sdk_builds_directional_process_container_networking_and_capture() { build_request_with_containment( &policy, &Containment::ProcessContainer(process_container), + "echo hello", None, ) .expect("public re-exports should build a schema 0.8 ProcessContainer request"); @@ -267,8 +271,8 @@ fn build_request_then_run_seatbelt() { timeout_ms: Some(10000), }; - let mut request = build_request(&policy, None).expect("build_request should succeed"); - request.set_script("echo built-from-policy"); + let request = build_request(&policy, "echo built-from-policy", None) + .expect("build_request should succeed"); let mut proc = spawn_sandbox(request).expect("spawn should succeed"); let mut out = String::new(); diff --git a/src/core/mxc-sdk/tests/streaming.rs b/src/core/mxc-sdk/tests/streaming.rs index 60668fd1a..12fa92740 100644 --- a/src/core/mxc-sdk/tests/streaming.rs +++ b/src/core/mxc-sdk/tests/streaming.rs @@ -32,8 +32,7 @@ fn seatbelt_request(command: &str, timeout_ms: u32) -> SandboxRequest { Some(timeout_ms) }, }; - let mut request = build_request(&policy, None).expect("build_request should succeed"); - request.set_script(command); + let request = build_request(&policy, command, None).expect("build_request should succeed"); request } diff --git a/src/core/mxc-sdk/tests/streaming_bubblewrap.rs b/src/core/mxc-sdk/tests/streaming_bubblewrap.rs index 6f9c99ec3..3fb631d00 100644 --- a/src/core/mxc-sdk/tests/streaming_bubblewrap.rs +++ b/src/core/mxc-sdk/tests/streaming_bubblewrap.rs @@ -45,9 +45,7 @@ fn bwrap_request(command: &str, timeout_ms: u32) -> SandboxRequest { Some(timeout_ms) }, }; - let mut request = build_request(&policy, None).expect("build_request should succeed"); - request.set_script(command); - request + build_request(&policy, command, None).expect("build_request should succeed") } /// Whether `pid` still has a `/proc` entry. An exited child stays a zombie -- diff --git a/src/core/mxc-sdk/tests/streaming_processcontainer.rs b/src/core/mxc-sdk/tests/streaming_processcontainer.rs index d7941ce19..8c3e4a691 100644 --- a/src/core/mxc-sdk/tests/streaming_processcontainer.rs +++ b/src/core/mxc-sdk/tests/streaming_processcontainer.rs @@ -28,9 +28,8 @@ fn streaming_processcontainer_bidirectional_stdio() { ui: None, timeout_ms: None, }; - let mut request = build_request(&policy, None).expect("build_request"); // `cmd /c more` echoes stdin to stdout until EOF, then exits. - request.set_script("cmd /c more"); + let request = build_request(&policy, "cmd /c more", None).expect("build_request"); let mut proc = spawn_sandbox(request).expect("spawn"); let mut stdin = proc.take_stdin().expect("stdin available"); diff --git a/src/core/mxc_config_contract/src/lib.rs b/src/core/mxc_config_contract/src/lib.rs index b427342f9..a9dfe948b 100644 --- a/src/core/mxc_config_contract/src/lib.rs +++ b/src/core/mxc_config_contract/src/lib.rs @@ -12,9 +12,10 @@ //! not validate the remainder of the selected configuration contract. //! //! This crate must not depend on MXC runtime, execution-engine, or containment -//! backend crates. It is not yet consumed by the production configuration -//! parser; version-specific request types and parser dispatch will be added in -//! later phases. +//! backend crates. The production configuration parser reuses the development +//! contract's narrow phase probe when preparing a trailing CLI command. +//! Version-specific request types and exact parser dispatch remain +//! non-authoritative until the later cutover phase. mod registry; mod version; diff --git a/src/core/mxc_engine/src/configs/process_container.rs b/src/core/mxc_engine/src/configs/process_container.rs index 84ca3fdc7..fd4f875e0 100644 --- a/src/core/mxc_engine/src/configs/process_container.rs +++ b/src/core/mxc_engine/src/configs/process_container.rs @@ -274,6 +274,8 @@ mod tests { RuntimeConfigSection, }; + const TEST_COMMAND: &str = "echo hello"; + fn policy(network: Option) -> SandboxPolicy { policy_for_version("0.8.0-alpha", network) } @@ -313,6 +315,7 @@ mod tests { let config = build_wire_config( &policy(None), &crate::policy::Containment::ProcessContainer(process_container), + TEST_COMMAND, Some("sdk-test"), ) .expect("ProcessContainer config should build"); @@ -370,6 +373,7 @@ mod tests { let config = build_wire_config( &policy(Some(network)), &crate::policy::Containment::ProcessContainer(process_container), + TEST_COMMAND, None, ) .expect("directional network config should build"); @@ -400,6 +404,7 @@ mod tests { let config = build_wire_config( &policy(Some(network)), &crate::policy::Containment::ProcessContainer(ProcessContainer::default()), + TEST_COMMAND, None, ) .expect("directional network config should build"); @@ -427,6 +432,7 @@ mod tests { let config = build_wire_config( &policy(Some(network)), &crate::policy::Containment::ProcessContainer(process_container), + TEST_COMMAND, None, ) .expect("schema 0.8 config should build"); @@ -461,6 +467,7 @@ mod tests { let error = build_wire_config( &policy_for_version("0.7.0-alpha", None), &crate::policy::Containment::ProcessContainer(process_container), + TEST_COMMAND, None, ) .expect_err("schema 0.7 must reject schema 0.8 ProcessContainer fields"); @@ -475,6 +482,7 @@ mod tests { let config = build_wire_config( &policy_for_version("0.7.0-alpha", None), &crate::policy::Containment::ProcessContainer(ProcessContainer::default()), + TEST_COMMAND, None, ) .expect("default ProcessContainer should remain valid for schema 0.7"); @@ -499,6 +507,7 @@ mod tests { let error = build_wire_config( &policy(Some(network)), &crate::policy::Containment::ProcessContainer(process_container), + TEST_COMMAND, None, ) .expect_err("legacy and ProcessContainer directional networking must not mix"); diff --git a/src/core/mxc_engine/src/dispatch.rs b/src/core/mxc_engine/src/dispatch.rs index 2e65f3114..389771688 100644 --- a/src/core/mxc_engine/src/dispatch.rs +++ b/src/core/mxc_engine/src/dispatch.rs @@ -330,7 +330,8 @@ mod tests { // `dry_run` ("validate, don't execute") has no process to stream, so the // streaming spawn rejects it. The public `SandboxRequest` can't set it, // so drive the dispatch directly with the internal model. - let mut request = build_request(&minimal_policy(), None).expect("build_request"); + let mut request = + build_request(&minimal_policy(), "echo hello", None).expect("build_request"); request.inner.dry_run = true; let mut logger = Logger::new(Mode::Buffer); let err = match spawn_runner(&request.inner, &mut logger) { @@ -346,7 +347,8 @@ mod tests { // clear `UnsupportedContainment` rather than spawning. The public // `SandboxRequest` can't choose a backend, so drive dispatch with the // internal model. - let mut request = build_request(&minimal_policy(), None).expect("build_request"); + let mut request = + build_request(&minimal_policy(), "echo hello", None).expect("build_request"); request.inner.containment = ContainmentBackend::Lxc; let mut logger = Logger::new(Mode::Buffer); let err = match spawn_runner(&request.inner, &mut logger) { @@ -383,8 +385,7 @@ mod tests { ui: None, timeout_ms: None, }; - let mut request = build_request(&policy, None).expect("build_request"); - request.set_script("echo hi"); + let mut request = build_request(&policy, "echo hi", None).expect("build_request"); request .inner .seatbelt @@ -404,7 +405,8 @@ mod tests { fn streaming_rejects_wslc_off_windows() { // WSLC is a Windows-host backend; selecting it anywhere else must be a // clear `UnsupportedContainment` rather than a confusing spawn failure. - let mut request = build_request(&minimal_policy(), None).expect("build_request"); + let mut request = + build_request(&minimal_policy(), "echo hello", None).expect("build_request"); request.inner.containment = ContainmentBackend::Wslc; request.set_experimental(true); let mut logger = Logger::new(Mode::Buffer); @@ -421,7 +423,8 @@ mod tests { fn streaming_rejects_wslc_without_experimental() { // The experimental gate is fail-closed: selecting WSLC without opting // in must be rejected before any container is created. - let mut request = build_request(&minimal_policy(), None).expect("build_request"); + let mut request = + build_request(&minimal_policy(), "echo hello", None).expect("build_request"); request.inner.containment = ContainmentBackend::Wslc; let mut logger = Logger::new(Mode::Buffer); let err = match spawn_runner(&request.inner, &mut logger) { diff --git a/src/core/mxc_engine/src/policy.rs b/src/core/mxc_engine/src/policy.rs index f13a4b5ed..819287795 100644 --- a/src/core/mxc_engine/src/policy.rs +++ b/src/core/mxc_engine/src/policy.rs @@ -609,11 +609,9 @@ pub struct SandboxPolicy { pub timeout_ms: Option, } -/// A spawnable sandbox request, built from a [`SandboxPolicy`] by -/// [`build_request`]. Fill in the command with -/// [`set_script`](Self::set_script) — and optionally a working -/// directory or environment — then hand it to -/// [`spawn`](crate::spawn). +/// A spawnable sandbox request, built from a [`SandboxPolicy`] and a command by +/// [`build_request`]. Optionally adjust the working directory or environment, +/// then hand it to [`spawn`](crate::spawn). /// /// This is the SDK's own request type; the internal execution model it maps to /// is an implementation detail callers don't depend on. @@ -626,18 +624,6 @@ pub struct SandboxRequest { } impl SandboxRequest { - /// Set the command the sandbox runs — the `/bin/sh -c` body on Unix, the - /// command line on Windows. - /// - /// This is the raw command string, mapped to the same `script_code` the - /// executor binaries run, so it is interpreted exactly as the SDK's - /// `spawnSandbox(script)` / `process.commandLine` is — behavior is identical - /// across the SDK and this crate. - pub fn set_script(&mut self, script: impl Into) -> &mut Self { - self.inner.script_code = script.into(); - self - } - /// Override the working directory the sandboxed child starts in. Left unset, /// it defaults to the policy's resolution. pub fn set_working_directory(&mut self, working_directory: impl Into) -> &mut Self { @@ -728,9 +714,9 @@ impl SandboxRequest { /// Build a [`SandboxRequest`] from a [`SandboxPolicy`], resolving the host's /// containment backend — the Rust port of the SDK's `createConfigFromPolicy`. /// -/// The returned request has an empty command line; set the command with -/// [`SandboxRequest::set_script`] (and any working directory / env) before -/// streaming it via [`crate::spawn`]. +/// The `script` becomes the request's command line, so the returned request is +/// complete and needs no post-build patching before streaming it via +/// [`crate::spawn`]. An empty script is rejected. /// /// Mirrors the SDK field mapping and validation (network proxy/host-filtering /// constraints) for the supported backends. Internally it builds the same @@ -741,9 +727,10 @@ impl SandboxRequest { /// [`build_request_with_containment`] to select a specific backend. pub fn build_request( policy: &SandboxPolicy, + script: &str, container_name: Option<&str>, ) -> Result { - build_request_with_containment(policy, &Containment::Process, container_name) + build_request_with_containment(policy, &Containment::Process, script, container_name) } /// Build a [`SandboxRequest`] for an explicitly chosen [`Containment`] backend @@ -766,23 +753,22 @@ pub fn build_request( /// timeout_ms: None, /// }; /// let wslc = WslcSection { image: "python:3.12".to_string(), ..Default::default() }; -/// let mut request = build_request_with_containment(&policy, &Containment::Wslc(wslc), None)?; -/// request.set_script("python3 -c 'print(1)'").set_experimental(true); +/// let mut request = build_request_with_containment(&policy, &Containment::Wslc(wslc), "python3 -c 'print(1)'", None)?; +/// request.set_experimental(true); /// # Ok::<(), mxc_engine::Error>(()) /// ``` pub fn build_request_with_containment( policy: &SandboxPolicy, containment: &Containment, + script: &str, container_name: Option<&str>, ) -> Result { require_sdk_policy_version(policy)?; - let config = build_wire_config(policy, containment, container_name)?; + let config = build_wire_config(policy, containment, script, container_name)?; let mut logger = Logger::new(Mode::Buffer); - // Map the wire config straight to a request — no base64/file round-trip. - // The command line is intentionally empty here (the caller fills - // `script_code` before running), so tolerate a missing command. - let inner = wxc_common::config_parser::load_request_from_value(config, &mut logger, true) + // Map the wire config straight to a request + let inner = wxc_common::config_parser::load_request_from_value(config, &mut logger) .map_err(|e| MxcError::malformed_request(format!("failed to build request: {e}")))?; Ok(SandboxRequest { inner, @@ -804,19 +790,25 @@ fn require_sdk_policy_version(policy: &SandboxPolicy) -> Result<(), crate::Error pub(crate) fn build_wire_config( policy: &SandboxPolicy, containment: &Containment, + script: &str, container_name: Option<&str>, ) -> Result { - build_wire_config_with_network_format(policy, containment, container_name) + build_wire_config_with_network_format(policy, containment, script, container_name) .map(|(config, _)| config) } fn build_wire_config_with_network_format( policy: &SandboxPolicy, containment: &Containment, + script: &str, container_name: Option<&str>, ) -> Result<(serde_json::Value, NetworkFormat), MxcError> { use serde_json::json; + if script.is_empty() { + return Err(MxcError::malformed_request("script parameter is required")); + } + let container_id = container_name .map(str::to_string) .unwrap_or_else(wxc_common::id::mint_random_token); @@ -828,7 +820,7 @@ fn build_wire_config_with_network_format( "version": policy.version, "containerId": container_id, "lifecycle": { "destroyOnExit": true, "preservePolicy": !clear_policy }, - "process": { "commandLine": "", "timeout": policy.timeout_ms.unwrap_or(0) }, + "process": { "commandLine": script, "timeout": policy.timeout_ms.unwrap_or(0) }, "filesystem": { "readwritePaths": fs.readwrite_paths, "readonlyPaths": fs.readonly_paths, @@ -1051,6 +1043,8 @@ fn apply_linux_network_policy(config: &mut serde_json::Value) { #[cfg(test)] mod tests { + const TEST_COMMAND: &str = "echo hello"; + // `ui` must be emitted only when the caller supplied one. // // These pin the fix for a defect that was invisible by value: the builder @@ -1070,8 +1064,9 @@ mod tests { serde_json::from_str(r#"{ "version": "0.7.0-alpha" }"#).expect("minimal policy parses"); assert!(policy.ui.is_none(), "precondition: no ui supplied"); - let config = super::build_wire_config(&policy, &super::Containment::Process, None) - .expect("minimal policy builds a wire config"); + let config = + super::build_wire_config(&policy, &super::Containment::Process, TEST_COMMAND, None) + .expect("minimal policy builds a wire config"); assert!( config.get("ui").is_none(), @@ -1088,8 +1083,9 @@ mod tests { .expect("policy with ui parses"); assert!(policy.ui.is_some(), "precondition: ui supplied"); - let config = super::build_wire_config(&policy, &super::Containment::Process, None) - .expect("policy with ui builds a wire config"); + let config = + super::build_wire_config(&policy, &super::Containment::Process, TEST_COMMAND, None) + .expect("policy with ui builds a wire config"); assert!( config.get("ui").is_some(), @@ -1170,7 +1166,7 @@ mod tests { }); policy.version = "0.8x".to_string(); - let error = build_request(&policy, None) + let error = build_request(&policy, TEST_COMMAND, None) .expect_err("a malformed schema version must be rejected") .to_string(); @@ -1194,7 +1190,7 @@ mod tests { ..Default::default() }); assert!( - build_request(&policy, None).is_ok(), + build_request(&policy, TEST_COMMAND, None).is_ok(), "macOS must accept allowedHosts without allowOutbound, matching the SDK" ); } @@ -1210,7 +1206,7 @@ mod tests { ..Default::default() }); assert!( - build_request(&policy, None).is_ok(), + build_request(&policy, TEST_COMMAND, None).is_ok(), "outbound-allowed host filter should build" ); } @@ -1222,8 +1218,8 @@ mod tests { proxy: Some(ProxySpec::Localhost(8080)), ..Default::default() }); - let request = - build_request(&policy, None).expect("macOS must accept Seatbelt proxy configuration"); + let request = build_request(&policy, TEST_COMMAND, None) + .expect("macOS must accept Seatbelt proxy configuration"); let proxy = &request.inner.policy.network_proxy; assert!(proxy.is_enabled()); @@ -1250,15 +1246,16 @@ mod tests { // Inspect the internal model the SDK maps to — a unit concern; the public // API only hands back the opaque `SandboxRequest`. - let request = - build_request(&policy, Some("test-container")).expect("build_request should succeed"); + let request = build_request(&policy, TEST_COMMAND, Some("test-container")) + .expect("build_request should succeed"); assert_eq!(request.inner.script_timeout, 5000); assert!(request .inner .policy .readwrite_paths .contains(&"/tmp".to_string())); - assert!(request.inner.script_code.is_empty()); + assert!(request.inner.script_code.contains(TEST_COMMAND)); + assert_eq!(request.inner.container_id, "test-container".to_string()); } #[test] @@ -1273,7 +1270,8 @@ mod tests { ui: None, timeout_ms: None, }; - let mut request = build_request(&policy, None).expect("build_request should succeed"); + let mut request = + build_request(&policy, TEST_COMMAND, None).expect("build_request should succeed"); request.set_env([("FIRST", "1"), ("SECOND", "2")]); assert_eq!(request.inner.env, vec!["FIRST=1", "SECOND=2"]); } @@ -1300,7 +1298,8 @@ mod tests { }), timeout_ms: None, }; - let request = build_request(&policy, None).expect("build_request should succeed"); + let request = + build_request(&policy, TEST_COMMAND, None).expect("build_request should succeed"); assert_eq!( request.inner.policy.ui.clipboard, expected, "clipboard {input:?} should map to {expected:?}" @@ -1317,7 +1316,7 @@ mod tests { blocked_hosts: vec!["198.51.100.10".to_string()], ..Default::default() }); - let request = build_request(&policy, None) + let request = build_request(&policy, TEST_COMMAND, None) .expect("build_request should accept host rules with allowOutbound"); assert!(request .inner @@ -1339,7 +1338,7 @@ mod tests { ..Default::default() }); - let error = build_request(&policy, None) + let error = build_request(&policy, TEST_COMMAND, None) .expect_err("the in-process SDK cannot start the built-in test proxy"); assert!(error.message.contains("builtinTestServer")); @@ -1347,6 +1346,36 @@ mod tests { assert!(error.message.contains("localhost or url")); } + #[test] + fn request_builders_reject_an_empty_script() { + let policy: SandboxPolicy = + serde_json::from_str(r#"{ "version": "0.7.0-alpha" }"#).expect("minimal policy parses"); + + let errors = [ + ( + "build_request", + build_request(&policy, "", None).expect_err("empty script should be rejected"), + ), + ( + "build_request_with_containment", + build_request_with_containment(&policy, &Containment::Process, "", None) + .expect_err("empty script should be rejected"), + ), + ]; + + for (entry_point, error) in errors { + assert_eq!( + error.code, + crate::ErrorCode::MalformedRequest, + "{entry_point}" + ); + assert!( + error.message.contains("script parameter is required"), + "{entry_point} unexpected error: {error:?}" + ); + } + } + #[cfg(target_os = "macos")] #[test] fn seatbelt_extra_mach_lookups_and_keychain_round_trip() { @@ -1359,7 +1388,7 @@ mod tests { }; // build_request resolves Seatbelt on macOS, so the config is present and // the consumer can read its defaults and write back. - let mut request = build_request(&policy, None).expect("build_request"); + let mut request = build_request(&policy, TEST_COMMAND, None).expect("build_request"); let mut union: Vec = request.seatbelt_extra_mach_lookups().to_vec(); union.push("com.example.service".to_string()); request.set_seatbelt_extra_mach_lookups(union.clone()); @@ -1424,7 +1453,7 @@ mod tests { output_path: Some(expected.clone()), retain_etl: true, }); - let request = build_request_with_containment(&policy, &containment, None) + let request = build_request_with_containment(&policy, &containment, TEST_COMMAND, None) .expect("build_request_with_containment"); let captured = request @@ -1444,13 +1473,14 @@ mod tests { let mut policy = minimal_policy(); policy.version = "0.8.0-alpha".to_string(); let containment = process_container_with_capture_denials(CaptureDenials::default()); - let request = build_request_with_containment(&policy, &containment, None) + let request = build_request_with_containment(&policy, &containment, TEST_COMMAND, None) .expect("build_request_with_containment"); assert!(request.inner.policy.capture_denials.is_some()); let request = build_request_with_containment( &policy, &Containment::ProcessContainer(ProcessContainer::default()), + TEST_COMMAND, None, ) .expect("build_request_with_containment"); @@ -1500,7 +1530,7 @@ mod tests { #[test] fn wire_contract_accepts_capture_denials_together_with_a_network_proxy() { let config = serde_json::json!({ - "process": { "commandLine": "echo hello" }, + "process": { "commandLine": TEST_COMMAND }, "containment": "processcontainer", "network": { "defaultPolicy": "allow", @@ -1512,7 +1542,7 @@ mod tests { }); let mut logger = super::Logger::new(super::Mode::Buffer); - let request = wxc_common::config_parser::load_request_from_value(config, &mut logger, true) + let request = wxc_common::config_parser::load_request_from_value(config, &mut logger) .expect("captureDenials alongside network.proxy satisfies the wire contract"); assert!( @@ -1547,7 +1577,7 @@ mod tests { retain_etl: true, }); - let request = build_request_with_containment(&policy, &containment, None) + let request = build_request_with_containment(&policy, &containment, TEST_COMMAND, None) .expect("captureDenials and network.proxy are accepted together"); let captured = request @@ -1578,7 +1608,7 @@ mod tests { fn default_containment_resolves_the_host_backend() { // `build_request` must keep resolving the host's native backend — the // WSLC selection is strictly opt-in and must not change the default. - let request = build_request(&minimal_policy(), None).expect("build_request"); + let request = build_request(&minimal_policy(), TEST_COMMAND, None).expect("build_request"); assert_ne!(request.inner.containment, ContainmentBackend::Wslc); } @@ -1596,9 +1626,13 @@ mod tests { port_mappings: vec![(8080, 80)], ..Default::default() }; - let request = - build_request_with_containment(&minimal_policy(), &Containment::Wslc(wslc), None) - .expect("build_request_with_containment"); + let request = build_request_with_containment( + &minimal_policy(), + &Containment::Wslc(wslc), + TEST_COMMAND, + None, + ) + .expect("build_request_with_containment"); assert_eq!(request.inner.containment, ContainmentBackend::Wslc); let config = request @@ -1625,6 +1659,7 @@ mod tests { let request = build_request_with_containment( &minimal_policy(), &Containment::Wslc(WslcSection::default()), + TEST_COMMAND, None, ) .expect("build_request_with_containment"); @@ -1648,6 +1683,7 @@ mod tests { let mut request = build_request_with_containment( &minimal_policy(), &Containment::Wslc(WslcSection::default()), + TEST_COMMAND, None, ) .expect("build_request_with_containment"); @@ -1658,7 +1694,8 @@ mod tests { #[test] fn telemetry_enablement_is_stable_and_independent_of_experimental_mode() { - let mut request = build_request(&minimal_policy(), None).expect("build_request"); + let mut request = + build_request(&minimal_policy(), TEST_COMMAND, None).expect("build_request"); assert!(request.inner.telemetry.is_none()); assert!(!request.inner.experimental_enabled); @@ -1688,6 +1725,7 @@ mod tests { let mut request = build_request_with_containment( &minimal_policy(), &Containment::ProcessContainer(containment), + TEST_COMMAND, None, ) .expect("build_request_with_containment"); @@ -1712,8 +1750,13 @@ mod tests { port_mappings: vec![(8080, 80), (8080, 81)], ..Default::default() }; - let err = build_request_with_containment(&minimal_policy(), &Containment::Wslc(wslc), None) - .expect_err("duplicate windowsPort must be rejected"); + let err = build_request_with_containment( + &minimal_policy(), + &Containment::Wslc(wslc), + TEST_COMMAND, + None, + ) + .expect_err("duplicate windowsPort must be rejected"); assert!( err.message.contains("duplicate windowsPort"), "got: {}", @@ -1734,6 +1777,7 @@ mod tests { let err = build_request_with_containment( &policy, &Containment::Wslc(WslcSection::default()), + TEST_COMMAND, None, ) .expect_err("WSLc must reject per-host egress filtering"); @@ -1761,8 +1805,9 @@ mod tests { // wire config must name the backend and add nothing else — unlike // WSLc, which also writes an `experimental.wslc` block. let policy = policy_with_network(isolation_session_network()); - let config = super::build_wire_config(&policy, &Containment::IsolationSession, None) - .expect("build_wire_config"); + let config = + super::build_wire_config(&policy, &Containment::IsolationSession, TEST_COMMAND, None) + .expect("build_wire_config"); assert_eq!(config["containment"], "isolation_session"); assert!( config.get("experimental").is_none(), @@ -1775,8 +1820,9 @@ mod tests { // The backend accepts *only* this shape and refuses an absent policy, // so the SDK types must be able to express it. let policy = policy_with_network(isolation_session_network()); - let config = super::build_wire_config(&policy, &Containment::IsolationSession, None) - .expect("build_wire_config"); + let config = + super::build_wire_config(&policy, &Containment::IsolationSession, TEST_COMMAND, None) + .expect("build_wire_config"); assert_eq!(config["network"]["defaultPolicy"], "allow"); assert_eq!(config["network"]["allowLocalNetwork"], true); assert_eq!( @@ -1798,9 +1844,13 @@ mod tests { // Selecting an experimental backend must not silently satisfy the // experimental gate. Mirrors `wslc_is_not_experimental_enabled_by_default`. let policy = policy_with_network(isolation_session_network()); - let mut request = - build_request_with_containment(&policy, &Containment::IsolationSession, None) - .expect("build_request_with_containment"); + let mut request = build_request_with_containment( + &policy, + &Containment::IsolationSession, + TEST_COMMAND, + None, + ) + .expect("build_request_with_containment"); assert!(!request.inner.experimental_enabled); request.set_experimental(true); assert!(request.inner.experimental_enabled); @@ -1809,8 +1859,13 @@ mod tests { #[test] fn isolation_session_selects_the_backend() { let policy = policy_with_network(isolation_session_network()); - let request = build_request_with_containment(&policy, &Containment::IsolationSession, None) - .expect("build_request_with_containment"); + let request = build_request_with_containment( + &policy, + &Containment::IsolationSession, + TEST_COMMAND, + None, + ) + .expect("build_request_with_containment"); assert_eq!( request.inner.containment, ContainmentBackend::IsolationSession diff --git a/src/core/wxc/src/main.rs b/src/core/wxc/src/main.rs index 572f38ea2..9b2cdb315 100644 --- a/src/core/wxc/src/main.rs +++ b/src/core/wxc/src/main.rs @@ -7,12 +7,11 @@ mod audit; use std::fmt::Write; use std::process; use std::sync::{Mutex, OnceLock}; -use std::time::{Duration, Instant}; +use std::time::Instant; use appcontainer_common::appcontainer_runner::delete_app_container_profile; use clap::Parser; use wxc_common::audit::{AuditEvent, AuditEventName, RejectionReason}; -use wxc_common::cmdline::{cmdline_from_argv_for_context, CommandLineContext, CommandLineError}; use wxc_common::config_parser::{LoadOptions, ParseError}; #[cfg(target_os = "windows")] use wxc_common::diagnostic::DiagnosticConfig; @@ -21,7 +20,7 @@ use wxc_common::models::{ContainmentBackend, ExecutionRequest, ScriptResponse}; use wxc_common::mxc_error::{MxcError, MxcErrorCode, ResponseEnvelope}; use wxc_common::script_runner::{handle_dry_run_exit, ScriptRunner}; use wxc_common::state_aware_dispatch::{resolve_backend, DispatchOutcome}; -use wxc_common::state_aware_request::{MxcRequest, ParsedStateAwareRequest, Phase}; +use wxc_common::state_aware_request::{MxcRequest, ParsedStateAwareRequest}; use wxc_common::telemetry; #[derive(Parser)] @@ -227,10 +226,6 @@ fn display_script_results(response: &ScriptResponse, logger: &mut Logger) { } } -fn has_cli_command(cli: &Cli) -> bool { - !cli.command.is_empty() -} - fn apply_permissive_learning_mode(capabilities: &mut Vec) -> bool { capabilities.retain(|capability| !capability.eq_ignore_ascii_case("learningModeLogging")); if capabilities @@ -271,49 +266,6 @@ fn decode_config_input_once(cli: &Cli) -> Option Result, CommandLineError> { - if cli.command.is_empty() { - Ok(None) - } else { - cmdline_from_argv_for_context(&cli.command, context).map(Some) - } -} - -fn command_override_context_for_state_aware( - parsed: &ParsedStateAwareRequest, - has_command_override: bool, -) -> Result, MxcError> { - if !has_command_override { - return Ok(None); - } - if parsed.phase != Phase::Exec { - return Err(MxcError::malformed_request( - "CLI command override is only supported for state-aware exec requests", - )); - } - resolve_backend(parsed).map(|backend| Some(CommandLineContext::for_backend(&backend))) -} - -fn apply_command_override( - request: &mut ExecutionRequest, - command_override: Option<&str>, - logger: &mut Logger, -) { - if let Some(cmd) = command_override { - if !request.script_code.is_empty() { - let _ = writeln!( - logger, - "Overriding policy process.commandLine with CLI command: {}", - cmd - ); - } - request.script_code = cmd.to_string(); - } -} - /// On a state-aware dispatch failure, record the error only on the auxiliary /// diagnostic sinks (`--log-file` and the diagnostic pipe) via /// [`Logger::log_diagnostic_line`]. It is deliberately kept out of the primary @@ -384,14 +336,6 @@ fn offending_field_from_message(message: &str) -> &str { /// resolution. const UNKNOWN_BACKEND: &str = "unknown"; -/// Resolve the wire backend name for a parsed state-aware request, falling back -/// to [`UNKNOWN_BACKEND`] when the request is too malformed to name one. -fn backend_name_for_state_aware(parsed: &ParsedStateAwareRequest) -> String { - resolve_backend(parsed) - .map(|b| b.wire_name().to_string()) - .unwrap_or_else(|_| UNKNOWN_BACKEND.to_string()) -} - /// Map a state-aware [`MxcError`] to its bounded [`RejectionReason`]. /// /// Driven by the error's own `code`, which is already an exhaustive closed set — @@ -436,31 +380,6 @@ fn state_aware_policy_identity(sandbox_id: Option<&str>) -> String { } } -fn emit_state_aware_early_rejection( - telemetry_active: bool, - parsed: &ParsedStateAwareRequest, - error: &MxcError, -) { - let backend = backend_name_for_state_aware(parsed); - let requested_sandbox_kind = parsed - .request - .telemetry - .as_ref() - .and_then(|config| config.requested_sandbox_kind); - let outcome = Err(error.clone()); - telemetry::emit_state_aware_with_kind( - telemetry_active, - requested_sandbox_kind, - telemetry::TelemetryContext { - backend: &backend, - phase: parsed.phase.as_str(), - correlation_vector: "", - }, - &outcome, - Duration::ZERO, - ); -} - /// Resolve the sandbox id to report on `mxc.SandboxIdentity` for a completed /// state-aware dispatch. /// @@ -713,6 +632,20 @@ fn finalize_state_aware_outcome(outcome: Result) -> S } } +enum RequestErrorRoute<'a> { + Diagnostic, + Envelope(&'a MxcError), +} + +fn request_error_route(error: &ParseError) -> RequestErrorRoute<'_> { + match error { + ParseError::Decode(_) | ParseError::OneShot(_) | ParseError::OneShotMalformed(_) => { + RequestErrorRoute::Diagnostic + } + ParseError::StateAware(e) => RequestErrorRoute::Envelope(e), + } +} + fn print_error_envelope(error: &MxcError) { println!("{}", error_envelope_string(error)); } @@ -1239,10 +1172,9 @@ fn main() { // one-shot. State-aware failures emit a JSON envelope on stdout; one-shot // and pre-discrimination failures keep the existing diagnostic-on-stderr // convention. - let has_command_override = has_cli_command(&cli); let load_opts = LoadOptions { is_base64: false, - allow_missing_command: has_command_override, + cli_command: &cli.command, }; let parsed_request = wxc_common::config_parser::load_mxc_request_from_json_with_options( &config_json, @@ -1258,49 +1190,6 @@ fn main() { .as_ref() .map(|config| telemetry::init(config, &mut logger)) .unwrap_or(false); - let context = - match command_override_context_for_state_aware(&parsed, has_command_override) { - Ok(context) => context, - Err(e) => { - log_config_rejected( - &mut logger, - rejection_reason_for(&e), - &backend_name_for_state_aware(&parsed), - "", - parsed.phase.as_str(), - ); - print_error_envelope(&e); - eprint!("{}", logger.get_buffer()); - emit_state_aware_early_rejection(telemetry_active, &parsed, &e); - process::exit(1); - } - }; - let command_override = match context - .map(|context| command_override_from_cli(&cli, context)) - .transpose() - { - Ok(command_override) => command_override.flatten(), - Err(e) => { - log_config_rejected( - &mut logger, - RejectionReason::InvalidCommandOverride, - &backend_name_for_state_aware(&parsed), - "process.commandLine", - parsed.phase.as_str(), - ); - let error = - MxcError::malformed_request(format!("invalid CLI command override: {e}")); - print_error_envelope(&error); - eprint!("{}", logger.get_buffer()); - emit_state_aware_early_rejection(telemetry_active, &parsed, &error); - process::exit(1); - } - }; - apply_command_override( - &mut parsed.request, - command_override.as_deref(), - &mut logger, - ); // Mirror what the one-shot path does at the post-dispatch stage // below: copy the CLI `--experimental` flag into the parsed // request so backends that gate on it (e.g. Windows Sandbox @@ -1313,55 +1202,58 @@ fn main() { parsed.request.dry_run = cli.dry_run; run_state_aware_main(parsed, cli.dry_run, telemetry_active, &mut logger) } - Err(ParseError::Decode(_)) => { - // The payload could not even be decoded into JSON, so no backend or - // field path is known — the record still exists so a rejected run - // is never invisible. - log_config_rejected( - &mut logger, - RejectionReason::MalformedJson, - UNKNOWN_BACKEND, - "", - "", - ); - eprint!("Request error\n{}", logger.get_buffer()); - process::exit(1); - } - Err(ParseError::OneShotMalformed(error)) => { - let message = error.to_string(); - log_config_rejected( - &mut logger, - RejectionReason::MalformedJson, - UNKNOWN_BACKEND, - offending_field_from_message(&message), - "", - ); - eprint!("Request error\n{}", logger.get_buffer()); - process::exit(1); - } - Err(ParseError::OneShot(error)) => { - let message = error.to_string(); - log_config_rejected( - &mut logger, - RejectionReason::SchemaViolation, - UNKNOWN_BACKEND, - offending_field_from_message(&message), - "", - ); - eprint!("Request error\n{}", logger.get_buffer()); - process::exit(1); - } - Err(ParseError::StateAware(e)) => { - let offending_field = offending_field_from_message(&e.message); - log_config_rejected( - &mut logger, - rejection_reason_for(&e), - UNKNOWN_BACKEND, - offending_field, - "", - ); - print_error_envelope(&e); - eprint!("{}", logger.get_buffer()); + Err(error) => { + match &error { + ParseError::Decode(_) => { + // The payload could not even be decoded into JSON, so no + // backend or field path is known. + log_config_rejected( + &mut logger, + RejectionReason::MalformedJson, + UNKNOWN_BACKEND, + "", + "", + ); + } + ParseError::OneShotMalformed(error) => { + let message = error.to_string(); + log_config_rejected( + &mut logger, + RejectionReason::MalformedJson, + UNKNOWN_BACKEND, + offending_field_from_message(&message), + "", + ); + } + ParseError::OneShot(error) => { + let message = error.to_string(); + log_config_rejected( + &mut logger, + RejectionReason::SchemaViolation, + UNKNOWN_BACKEND, + offending_field_from_message(&message), + "", + ); + } + ParseError::StateAware(error) => { + log_config_rejected( + &mut logger, + rejection_reason_for(error), + UNKNOWN_BACKEND, + offending_field_from_message(&error.message), + "", + ); + } + } + match request_error_route(&error) { + RequestErrorRoute::Diagnostic => { + eprint!("Request error\n{}", logger.get_buffer()); + } + RequestErrorRoute::Envelope(e) => { + print_error_envelope(e); + eprint!("{}", logger.get_buffer()); + } + } process::exit(1); } }; @@ -1391,34 +1283,6 @@ fn main() { telemetry::install_panic_hook(); } - // Apply the CLI command-line override to one-shot requests. State-aware - // exec is handled above before dispatch. - let command_override = match command_override_from_cli( - &cli, - CommandLineContext::for_backend(&request.containment), - ) { - Ok(command_override) => command_override, - Err(e) => { - log_config_rejected( - &mut logger, - RejectionReason::InvalidCommandOverride, - request.containment.wire_name(), - "process.commandLine", - "", - ); - eprintln!("Request error\ninvalid CLI command override: {e}"); - eprint!("{}", logger.get_buffer()); - telemetry::emit_early_exit_with_kind( - telemetry_active, - &request.containment, - requested_sandbox_kind, - telemetry::FailureReason::ConfigError, - ); - process::exit(1); - } - }; - apply_command_override(&mut request, command_override.as_deref(), &mut logger); - // --audit injects permissiveLearningMode so denied operations are logged // but allowed, and drives the WPR/ETW PLM trace pipeline below. This is the // developer inner-loop flow. The capability names are reserved from direct @@ -1746,13 +1610,28 @@ mod tests { use super::*; use clap::{CommandFactory, Parser}; + use wxc_common::cmdline::{cmdline_from_argv_for_context, CommandLineContext}; use wxc_common::config_parser::load_mxc_request_with_options; use wxc_common::encoding::base64_encode; + use wxc_common::error::WxcError; use wxc_common::logger::Mode; - use wxc_common::mxc_error::MxcErrorCode; use wxc_common::state_aware_request::MxcRequest; use wxc_common::telemetry::correlation_state::test_support::StoreDirGuard; + const CLI_OVERRIDE_PROGRAM: &str = "cli-app.exe"; + const CLI_OVERRIDE_FLAG: &str = "--from-cli"; + const CLI_OVERRIDE_ARGV: &[&str] = &[ + "wxc-exec", + "policy.json", + "--", + CLI_OVERRIDE_PROGRAM, + CLI_OVERRIDE_FLAG, + ]; + + fn cli_override_command() -> String { + format!("{CLI_OVERRIDE_PROGRAM} {CLI_OVERRIDE_FLAG}") + } + fn parse_cli(argv: &[&str]) -> Cli { Cli::try_parse_from(argv) .unwrap() @@ -1829,6 +1708,30 @@ mod tests { } } + #[derive(Debug)] + struct ResolveError { + message: String, + envelope_routed: bool, + } + + impl ResolveError { + fn from_parse(e: ParseError) -> Self { + let envelope_routed = matches!(request_error_route(&e), RequestErrorRoute::Envelope(_)); + + let message = match e { + ParseError::Decode(err) + | ParseError::OneShot(err) + | ParseError::OneShotMalformed(err) => err.to_string(), + ParseError::StateAware(err) => err.to_string(), + }; + + Self { + message, + envelope_routed, + } + } + } + #[test] fn offending_field_is_extracted_without_error_text() { assert_eq!( @@ -1931,6 +1834,47 @@ 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 oneshot = ParseError::OneShot(WxcError::ConfigParse("oneshot".to_string())); + assert!(matches!( + request_error_route(&oneshot), + RequestErrorRoute::Diagnostic + )); + + let stateaware = ParseError::StateAware(MxcError::malformed_request("stateaware")); + assert!(matches!( + request_error_route(&stateaware), + RequestErrorRoute::Envelope(_) + )); + } + + fn resolve_with_cli( + argv: &[&str], + policy_json: &str, + ) -> (Result, String) { + let cli = parse_cli(argv); + let mut logger = test_logger(); + let opts = LoadOptions { + is_base64: true, + cli_command: &cli.command, + }; + + let result = load_mxc_request_with_options(&encoded_policy(policy_json), &mut logger, opts) + .map(|r| match r { + MxcRequest::OneShot(q) => q, + MxcRequest::StateAware(p) => p.request, + }) + .map_err(ResolveError::from_parse); + (result, logger.get_buffer().to_string()) + } + #[test] fn audit_mode_replaces_deny_and_record_capability() { let mut capabilities = vec![ @@ -2086,10 +2030,9 @@ mod tests { vec!["python".to_string(), "--version".to_string()] ); assert_eq!( - command_override_from_cli(&cli, CommandLineContext::WindowsCreateProcess) - .unwrap() - .as_deref(), - Some("python --version") + cmdline_from_argv_for_context(&cli.command, CommandLineContext::WindowsCreateProcess) + .unwrap(), + "python --version" ); } @@ -2111,10 +2054,9 @@ mod tests { vec!["python".to_string(), "--version".to_string()] ); assert_eq!( - command_override_from_cli(&cli, CommandLineContext::WindowsCreateProcess) - .unwrap() - .as_deref(), - Some("python --version") + cmdline_from_argv_for_context(&cli.command, CommandLineContext::WindowsCreateProcess) + .unwrap(), + "python --version" ); } @@ -2138,10 +2080,9 @@ mod tests { vec!["python".to_string(), "--version".to_string()] ); assert_eq!( - command_override_from_cli(&cli, CommandLineContext::WindowsCreateProcess) - .unwrap() - .as_deref(), - Some("python --version") + cmdline_from_argv_for_context(&cli.command, CommandLineContext::WindowsCreateProcess) + .unwrap(), + "python --version" ); } @@ -2164,10 +2105,9 @@ mod tests { vec!["python".to_string(), "--version".to_string()] ); assert_eq!( - command_override_from_cli(&cli, CommandLineContext::WindowsCreateProcess) - .unwrap() - .as_deref(), - Some("python --version") + cmdline_from_argv_for_context(&cli.command, CommandLineContext::WindowsCreateProcess) + .unwrap(), + "python --version" ); } @@ -2254,10 +2194,9 @@ mod tests { vec!["-command".to_string(), "value".to_string()] ); assert_eq!( - command_override_from_cli(&cli, CommandLineContext::WindowsCreateProcess) - .unwrap() - .as_deref(), - Some("-command value") + cmdline_from_argv_for_context(&cli.command, CommandLineContext::WindowsCreateProcess) + .unwrap(), + "-command value" ); } @@ -2282,10 +2221,6 @@ mod tests { #[test] fn cli_command_overrides_policy_command_line_in_resolved_request() { - let cli = parse_cli(&["wxc-exec", "policy.json", "--", "cli-app.exe", "--from-cli"]); - let command_override = - command_override_from_cli(&cli, CommandLineContext::WindowsCreateProcess).unwrap(); - let mut logger = test_logger(); let policy = r#"{ "process": { "commandLine": "policy-app.exe --from-policy", @@ -2295,29 +2230,142 @@ mod tests { "readwritePaths": ["C:\\workspace"] } }"#; - let opts = LoadOptions { - is_base64: true, - allow_missing_command: command_override.is_some(), - }; - let mut request = match load_mxc_request_with_options( - &encoded_policy(policy), - &mut logger, - opts, - ) - .unwrap() - { - MxcRequest::OneShot(req) => req, - MxcRequest::StateAware(_) => panic!("expected one-shot"), - }; - apply_command_override(&mut request, command_override.as_deref(), &mut logger); + let (resolved_request, log) = resolve_with_cli(CLI_OVERRIDE_ARGV, policy); + + let request = resolved_request.unwrap(); + let expected_command = cli_override_command(); - assert_eq!(request.script_code, "cli-app.exe --from-cli"); + assert_eq!(request.script_code, expected_command); assert_eq!(request.working_directory, "C:\\workspace"); assert_eq!(request.policy.readwrite_paths, vec!["C:\\workspace"]); - assert!(logger.get_buffer().contains( - "Overriding policy process.commandLine with CLI command: cli-app.exe --from-cli" - )); + assert!(log.contains(&format!( + "Overriding policy process.commandLine with CLI command: {expected_command}" + ))); + } + + #[test] + fn cli_command_fills_absent_policy_command_line_without_override_log() { + let policy = r#"{ + "process": { + "cwd": "C:\\workspace" + }, + "filesystem": { + "readwritePaths": ["C:\\workspace"] + } + }"#; + + let (resolved_request, log) = resolve_with_cli(CLI_OVERRIDE_ARGV, policy); + + let request = resolved_request.unwrap(); + + assert_eq!(request.script_code, cli_override_command()); + assert_eq!(request.working_directory, "C:\\workspace"); + assert_eq!(request.policy.readwrite_paths, vec!["C:\\workspace"]); + assert!( + !log.contains("Overriding policy process.commandLine"), + "no override log should be emitted when the policy command line is absent" + ); + } + + #[test] + fn policy_command_line_survives_without_cli_command() { + let argv = &["wxc-exec", "policy.json"]; + let policy = r#"{ + "process": { + "commandLine": "policy-app.exe --from-policy", + "cwd": "C:\\workspace" + } + }"#; + + let (resolved_request, log) = resolve_with_cli(argv, policy); + + let request = resolved_request.unwrap(); + + assert_eq!(request.script_code, "policy-app.exe --from-policy"); + assert!( + !log.contains("Overriding policy process.commandLine"), + "no override log should be emitted when no CLI command is supplied" + ); + } + + // The three quoting contexts are distinguished by a single argument + // containing `&`: CreateProcess leaves it bare, cmd.exe quotes it because + // `&` is a command separator, and a POSIX shell single-quotes it. These + // assert the value that reaches `script_code` after the whole pipeline, + // unlike the tests above that assert what `command_override_from_cli` + // renders in isolation. + const QUOTING_ARGV: &[&str] = &["wxc-exec", "policy.json", "--", "app.exe", "a&b"]; + + #[test] + fn cli_command_quoting_for_windows_create_process_in_resolved_request() { + let policy = r#"{ + "containment": "processcontainer", + "process": {} + }"#; + + let (resolved_request, _log) = resolve_with_cli(QUOTING_ARGV, policy); + + assert_eq!(resolved_request.unwrap().script_code, "app.exe a&b"); + } + + #[test] + fn cli_command_quoting_for_command_processor_in_resolved_request() { + let policy = r#"{ + "containment": "windows_sandbox", + "process": {} + }"#; + + let (resolved_request, _log) = resolve_with_cli(QUOTING_ARGV, policy); + + assert_eq!(resolved_request.unwrap().script_code, "app.exe \"a&b\""); + } + + #[test] + fn cli_command_quoting_for_posix_shell_in_resolved_request() { + let policy = r#"{ + "containment": "bubblewrap", + "process": {} + }"#; + + let (resolved_request, _log) = resolve_with_cli(QUOTING_ARGV, policy); + + assert_eq!(resolved_request.unwrap().script_code, "app.exe 'a&b'"); + } + + #[test] + fn non_object_process_section_is_rejected() { + let argv = &["wxc-exec", "policy.json", "--", "echo", "hi"]; + let policy = r#"{ + "process": 42 + }"#; + + let (result, _log) = resolve_with_cli(argv, policy); + + let err = result.unwrap_err(); + assert!( + !err.envelope_routed, + "one-shot failures use the stderr diagnostic convention" + ); + assert!( + err.message.contains("process"), + "error should name the offending section: {}", + err.message + ); + } + + #[test] + fn malformed_policy_json_is_rejected_before_splicing() { + let argv = &["wxc-exec", "policy.json", "--", "echo", "hi"]; + let policy = r#"{ "process": "#; + + let (result, _log) = resolve_with_cli(argv, policy); + + let err = result.unwrap_err(); + assert!( + !err.envelope_routed, + "an undiscriminated decode failure uses the stderr diagnostic convention" + ); } #[test] @@ -2330,10 +2378,6 @@ mod tests { "--message", "hello world", ]); - let command_override = - command_override_from_cli(&cli, CommandLineContext::WindowsCreateProcess) - .unwrap() - .unwrap(); let mut policy_logger = test_logger(); let mut cli_logger = test_logger(); let policy = r#"{ @@ -2353,7 +2397,7 @@ mod tests { &mut policy_logger, LoadOptions { is_base64: true, - allow_missing_command: false, + cli_command: &[], }, ) .unwrap() @@ -2361,12 +2405,12 @@ mod tests { MxcRequest::OneShot(req) => req, MxcRequest::StateAware(_) => panic!("expected one-shot"), }; - let mut cli_request = match load_mxc_request_with_options( + let cli_request = match load_mxc_request_with_options( &encoded_policy(cli_policy), &mut cli_logger, LoadOptions { is_base64: true, - allow_missing_command: true, + cli_command: &cli.command, }, ) .unwrap() @@ -2375,8 +2419,6 @@ mod tests { MxcRequest::StateAware(_) => panic!("expected one-shot"), }; - apply_command_override(&mut cli_request, Some(&command_override), &mut cli_logger); - assert_eq!(cli_request.script_code, policy_request.script_code); assert_eq!( cli_request.working_directory, @@ -2394,10 +2436,11 @@ mod tests { "-c", "if 5 < 10: print('hello')", ]); - let command_override = - command_override_from_cli(&cli, CommandLineContext::WindowsCommandProcessor) - .unwrap() - .unwrap(); + let command_override = cmdline_from_argv_for_context( + &cli.command, + CommandLineContext::WindowsCommandProcessor, + ) + .unwrap(); assert_eq!(command_override, "python -c \"if 5 < 10: print('hello')\""); } @@ -2405,32 +2448,97 @@ mod tests { #[test] fn wslc_cli_command_uses_posix_shell_quoting() { let cli = parse_cli(&["wxc-exec", "policy.json", "--", "echo", "safe&whoami"]); - let command_override = command_override_from_cli(&cli, CommandLineContext::PosixShell) - .unwrap() - .unwrap(); + let command_override = + cmdline_from_argv_for_context(&cli.command, CommandLineContext::PosixShell).unwrap(); assert_eq!(command_override, "echo 'safe&whoami'"); } #[test] - fn state_aware_command_override_only_applies_to_exec_phase() { - let parsed = ParsedStateAwareRequest { - request: ExecutionRequest::default(), - phase: Phase::Start, - containment: None, - sandbox_id: Some("iso:wxc-1234".into()), - experimental_raw: None, - source_text: None, - }; + fn state_aware_exec_cli_command_overrides_policy_command_line() { + let argv = &["wxc-exec", "policy.json", "--", "echo", "hi"]; + let policy = r#"{ + "phase": "exec", + "sandboxId": "iso:abcd1234", + "process": { + "commandLine": "echo from policy" + } + }"#; - let err = command_override_context_for_state_aware(&parsed, true).unwrap_err(); + let (resolved_request, log) = resolve_with_cli(argv, policy); - assert_eq!(err.code, MxcErrorCode::MalformedRequest); + let request = resolved_request.unwrap(); + + assert_eq!(request.script_code, "echo hi"); + assert!( + log.contains("Overriding policy process.commandLine"), + "override log should be emitted when the policy command line is present" + ); + } + + #[test] + fn state_aware_exec_cli_command_fills_absent_policy_command_line_without_override_log() { + let argv = &["wxc-exec", "policy.json", "--", "echo", "hi"]; + let policy = r#"{ + "phase": "exec", + "sandboxId": "iso:abcd1234" + }"#; + + let (resolved_request, log) = resolve_with_cli(argv, policy); + + let request = resolved_request.unwrap(); + + assert_eq!(request.script_code, "echo hi"); + assert!( + !log.contains("Overriding policy process.commandLine"), + "no override log should be emitted when the policy command line is absent" + ); + } + + #[test] + fn state_aware_non_exec_cli_command_error_routes_to_envelope() { + let argv = &["wxc-exec", "policy.json", "--", "echo", "hi"]; + let policy = r#"{ + "phase": "start", + "sandboxId": "iso:abcd1234" + }"#; + + let (result, _log) = resolve_with_cli(argv, policy); + + let err = result.unwrap_err(); + assert!( + err.envelope_routed, + "state-aware failures emit an envelope on stdout" + ); assert!(err .message .contains("only supported for state-aware exec requests")); } + #[test] + fn state_aware_exec_unconvertible_cli_command_routes_to_envelope() { + let argv = &[ + "wxc-exec", + "policy.json", + "--", + "app.exe", + "hidden\0payload", + ]; + let policy = r#"{ + "phase": "exec", + "sandboxId": "iso:abcd1234" + }"#; + + let (result, _log) = resolve_with_cli(argv, policy); + + let err = result.unwrap_err(); + assert!( + err.envelope_routed, + "state-aware command-rendering failures emit an envelope on stdout" + ); + assert!(err.message.contains("invalid CLI command override")); + } + #[cfg(target_os = "windows")] #[test] fn cancel_then_cleanup_emits_before_cleanup() { diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index ea07e366e..1895eb281 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -1,8 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::{borrow::Cow, fs}; - +use crate::cmdline::{cmdline_from_argv_for_context, CommandLineContext}; use crate::config_deserialize; use crate::encoding::base64_decode; use crate::error::WxcError; @@ -20,8 +19,10 @@ use crate::network_parser::{ }; use crate::state_aware_request::{MxcRequest, ParsedStateAwareRequest, Phase}; use crate::wire; +use mxc_config_contract::dev::{probe_phase, Phase as ContractPhase}; use serde::{Deserialize, Deserializer}; use serde_json::value::RawValue; +use std::{borrow::Cow, fs}; /// Categorised error from `load_mxc_request`. The `wxc-exec` driver uses the /// variant to choose the failure-output convention: state-aware failures @@ -123,16 +124,12 @@ fn reject_legacy_telemetry_value(config: &serde_json::Value) -> Result<(), WxcEr /// loader-tuning knobs can be threaded through without re-spinning every /// caller. #[derive(Debug, Clone, Copy, Default)] -pub struct LoadOptions { +pub struct LoadOptions<'a> { /// Treat `input` as a base64-encoded JSON blob rather than a file path. pub is_base64: bool, - /// Allow `process.commandLine` to be absent or empty in the policy. - /// - /// The driver sets this when it has a CLI-provided command-line - /// override to splice into `script_code` after parsing. Without it, - /// missing/empty `commandLine` is a hard parse error in one-shot - /// and state-aware exec requests (matching the legacy contract). - pub allow_missing_command: bool, + /// Trailing CLI argv spliced into `process.commandLine` before parsing. + /// Empty means no override. + pub cli_command: &'a [String], } /// Loads and parses a JSON-based code execution request. @@ -143,26 +140,9 @@ pub fn load_request( input: &str, logger: &mut Logger, is_base64: bool, -) -> Result { - load_request_with_options( - input, - logger, - LoadOptions { - is_base64, - allow_missing_command: false, - }, - ) -} - -/// Options-aware variant of [`load_request`] used by drivers that may -/// override `process.commandLine` from the CLI. See [`LoadOptions`]. -pub fn load_request_with_options( - input: &str, - logger: &mut Logger, - opts: LoadOptions, ) -> Result { let result = (|| { - let json_str = decode_request_input(input, opts.is_base64)?; + let json_str = decode_request_input(input, is_base64)?; let discriminator: RequestDiscriminator<'_> = config_deserialize::from_str(&json_str) .map_err(|error| WxcError::ConfigParse(error.to_string()))?; reject_legacy_telemetry_raw(discriminator.experimental.map(|raw| raw.get()))?; @@ -173,7 +153,7 @@ pub fn load_request_with_options( .map_err(|error| WxcError::ConfigParse(error.to_string()))?; validate_versioned_fields(&raw)?; - convert_wire_config(cfg, logger, true, opts.allow_missing_command, false) + convert_wire_config(cfg, logger, true, false) })(); log_one_shot_error(logger, &result); result @@ -193,7 +173,7 @@ pub fn load_request_from_json( logger, LoadOptions { is_base64: false, - allow_missing_command: false, + cli_command: &[], }, ) } @@ -226,23 +206,22 @@ pub(crate) fn load_request_from_json_with_options( .map_err(|error| WxcError::ConfigParse(error.to_string()))?; validate_versioned_fields(&raw)?; - convert_wire_config(cfg, logger, true, opts.allow_missing_command, false) + convert_wire_config(cfg, logger, true, false) })(); log_one_shot_error(logger, &result); result } /// Build a request from an already-parsed wire-format config [`Value`], running -/// the same validation and wire→model mapping as [`load_request_with_options`] -/// but without a base64 (or file) round-trip. For in-process callers (e.g. the -/// `mxc` crate) that already hold the config as JSON and would otherwise pay to +/// the same validation and wire→model mapping as [`load_request`] but without a +/// base64 (or file) round-trip. For in-process callers (e.g. the `mxc` crate) +/// that already hold the config as JSON and would otherwise pay to /// serialise → base64 → decode → re-parse it. /// /// [`Value`]: serde_json::Value pub fn load_request_from_value( config: serde_json::Value, logger: &mut Logger, - allow_missing_command: bool, ) -> Result { let result = (|| { let raw = config.clone(); @@ -251,7 +230,7 @@ pub fn load_request_from_value( .map_err(|error| WxcError::ConfigParse(error.to_string()))?; validate_versioned_fields(&raw)?; - convert_wire_config(cfg, logger, true, allow_missing_command, false) + convert_wire_config(cfg, logger, true, false) })(); log_one_shot_error(logger, &result); result @@ -269,28 +248,29 @@ pub fn load_mxc_request( logger, LoadOptions { is_base64, - allow_missing_command: false, + cli_command: &[], }, ) } /// Options-aware variant of [`load_mxc_request`]. When -/// `LoadOptions::allow_missing_command` is set, a missing or empty -/// `process.commandLine` in the policy is tolerated and `script_code` -/// is left empty for the driver to fill in from a CLI override. +/// `LoadOptions::cli_command` is non-empty, it is rendered for the request's +/// backend and spliced into `process.commandLine` before parsing, so the +/// parsed request is complete rather than being patched afterwards. pub fn load_mxc_request_with_options( input: &str, logger: &mut Logger, - opts: LoadOptions, + opts: LoadOptions<'_>, ) -> Result { - let result = (|| { + let result: Result = (|| { let json_str = decode_request_input(input, opts.is_base64).map_err(ParseError::Decode)?; - parse_mxc_request_json(&json_str, logger, opts.allow_missing_command) + parse_mxc_request_json_with_cli(&json_str, logger, opts.cli_command) })(); if let Err(error) = &result { log_error(logger, &error.message(), error.output()); } + result } @@ -307,15 +287,12 @@ pub fn load_mxc_request_from_json( logger, LoadOptions { is_base64: false, - allow_missing_command: false, + cli_command: &[], }, ) } -/// Options-aware variant of [`load_mxc_request_from_json`]. When -/// `LoadOptions::allow_missing_command` is set, a missing or empty -/// `process.commandLine` in the policy is tolerated and `script_code` is left -/// empty for the driver to fill in from a CLI override. +/// Options-aware variant of [`load_mxc_request_from_json`]. /// /// Executor binaries call this after [`decode_request_input`] to avoid a /// second read of the input source (file / named pipe / `/dev/stdin` / @@ -324,28 +301,116 @@ pub fn load_mxc_request_from_json( pub fn load_mxc_request_from_json_with_options( json_str: &str, logger: &mut Logger, - opts: LoadOptions, + opts: LoadOptions<'_>, ) -> Result { // `is_base64` is meaningless on an already-decoded JSON string; the field // is kept in `LoadOptions` for signature parity with the from-input path. let _ = opts.is_base64; - let result = parse_mxc_request_json(json_str, logger, opts.allow_missing_command); + let result = parse_mxc_request_json_with_cli(json_str, logger, opts.cli_command); if let Err(error) = &result { log_error(logger, &error.message(), error.output()); } result } +fn parse_mxc_request_json_with_cli( + json_str: &str, + logger: &mut Logger, + cli_command: &[String], +) -> Result { + if cli_command.is_empty() { + return parse_mxc_request_json(json_str, logger); + } + + let (json_str, override_log) = apply_cli_command(json_str, cli_command)?; + let request = parse_mxc_request_json(&json_str, logger)?; + if let Some(message) = override_log { + logger.log_line(&message); + } + Ok(request) +} + +/// Resolves a CLI command override by splicing it into the request source, +/// returning the effective document to parse. +/// +/// Returns the input **unchanged** whenever the override cannot be applied for +/// a reason the parser will itself report. Those inputs then keep today's error +/// text and output routing rather than inheriting a probe's stricter, and +/// differently routed, diagnostic. +/// +/// Probing the phase first is load-bearing: it selects which [`ParseError`] +/// variant a later failure uses, and that selects the stdout envelope versus +/// the stderr diagnostic. +/// +/// [`load_mxc_request_with_options`] calls this only after confirming that +/// `LoadOptions::cli_command` is non-empty. The explicit empty-command +/// rejection remains defensive for direct internal callers and future call +/// sites rather than relying solely on that upstream guard. +fn apply_cli_command(json: &str, argv: &[String]) -> Result<(String, Option), ParseError> { + // An unreadable phase declaration is the parser's to report. + let Ok(phase) = probe_phase(json) else { + return Ok((json.to_string(), None)); + }; + + let Some(command_source) = crate::splice::CommandSource::parse(json) else { + return Ok((json.to_string(), None)); + }; + + let context = match phase { + None => match command_source.one_shot_backend() { + Some(backend) => CommandLineContext::for_backend(&backend), + // Likewise an unreadable containment: the typed parse rejects it. + None => return Ok((json.to_string(), None)), + }, + Some(ContractPhase::Exec) => { + // Not a passthrough: `resolve_backend` raises this same error after + // parsing today, so surfacing it here preserves current behavior. + // Swallowing it would silently drop the caller's override. + let backend = command_source + .state_aware_backend() + .map_err(ParseError::StateAware)?; + CommandLineContext::for_backend(&backend) + } + Some(_) => { + return Err(ParseError::StateAware(MxcError::malformed_request( + "CLI command override is only supported for state-aware exec requests", + ))) + } + }; + + let command = cmdline_from_argv_for_context(argv, context).map_err(|e| match phase { + None => ParseError::Decode(WxcError::ConfigParse(format!( + "invalid CLI command override: {e}" + ))), + Some(_) => ParseError::StateAware(MxcError::malformed_request(format!( + "invalid CLI command override: {e}" + ))), + })?; + + if command.is_empty() { + return Err(ParseError::Decode(WxcError::ConfigParse( + "CLI command override must not be empty".to_string(), + ))); + } + + // A document the splice cannot transform is one the parser rejects anyway. + let Some(spliced) = command_source.splice_command(&command) else { + return Ok((json.to_string(), None)); + }; + + let override_log = spliced + .replaced_existing + .then(|| format!("Overriding policy process.commandLine with CLI command: {command}")); + + Ok((spliced.json, override_log)) +} + /// Shared parse core over an already-decoded JSON string. /// /// Borrows only the discriminator and the raw state-aware backend block, then /// deserialises the typed model directly from source text so policy errors /// retain line and column information (`serde_json::Value` would discard it). -fn parse_mxc_request_json( - json_str: &str, - logger: &mut Logger, - allow_missing_command: bool, -) -> Result { +fn parse_mxc_request_json(json_str: &str, logger: &mut Logger) -> Result { let discriminator: RequestDiscriminator<'_> = config_deserialize::from_str(json_str) .map_err(|error| ParseError::Decode(WxcError::ConfigParse(error.to_string())))?; if discriminator.phase.is_some() { @@ -359,15 +424,9 @@ fn parse_mxc_request_json( validate_versioned_fields(&raw).map_err(|error| { ParseError::StateAware(MxcError::malformed_request(error.to_string())) })?; - convert_wire_state_aware( - json_str, - experimental, - experimental_span, - logger, - allow_missing_command, - ) - .map(MxcRequest::StateAware) - .map_err(|e| ParseError::StateAware(MxcError::malformed_request(e.to_string()))) + convert_wire_state_aware(json_str, experimental, experimental_span, logger) + .map(MxcRequest::StateAware) + .map_err(|e| ParseError::StateAware(MxcError::malformed_request(e.to_string()))) } else { reject_legacy_telemetry_raw(discriminator.experimental.map(|raw| raw.get())) .map_err(ParseError::OneShot)?; @@ -383,7 +442,7 @@ fn parse_mxc_request_json( let raw: serde_json::Value = config_deserialize::from_str(json_str) .map_err(|error| ParseError::OneShot(WxcError::ConfigParse(error.to_string())))?; validate_versioned_fields(&raw).map_err(ParseError::OneShot)?; - convert_wire_config(cfg, logger, true, allow_missing_command, false) + convert_wire_config(cfg, logger, true, false) .map(MxcRequest::OneShot) .map_err(ParseError::OneShot) } @@ -794,7 +853,7 @@ fn make_seatbelt_config(sb: wire::Seatbelt) -> SeatbeltConfig { /// An omitted `containment` (`None`) resolves identically to the abstract /// `process` intent: the OS-native process sandbox. Concrete and abstract /// variants are mapped by `From`. -fn map_wire_containment(c: Option<&wire::Containment>) -> ContainmentBackend { +pub(crate) fn map_wire_containment(c: Option<&wire::Containment>) -> ContainmentBackend { match c { Some(c) => c.clone().into(), None => wire::Containment::Process.into(), @@ -880,11 +939,6 @@ fn validate_capture_denials_output_path(path: &str, logger: &mut Logger) -> Resu } } -// `allow_missing_command` relaxes the `require_process == true` arms so that a -// CLI command-line override (provided by the driver after parsing) can stand in -// for `process.commandLine`. When set, a missing or empty `commandLine` is -// silently accepted and `script_code` is left empty. -// // `state_aware_wslc_exec` identifies the state-aware exec exception: network // mode was fixed at provision, so a proxy-only exec inherits that mode rather // than restating `defaultPolicy`. Backend phase validation still rejects every @@ -893,7 +947,6 @@ fn convert_wire_config( cfg: wire::MxcConfig, logger: &mut Logger, require_process: bool, - allow_missing_command: bool, state_aware_wslc_exec: bool, ) -> Result { // `phase` / `sandboxId` are state-aware-only fields. The state-aware path @@ -921,19 +974,17 @@ fn convert_wire_config( let container_id = cfg.container_id.unwrap_or_default(); // Process section: required for one-shot and state-aware exec; optional for - // non-exec state-aware phases (require_process == false) or when the driver - // signalled a CLI command-line override (allow_missing_command). - let command_required = require_process && !allow_missing_command; + // non-exec state-aware phases (require_process == false) let (script_code, working_directory, script_timeout, env) = match cfg.process { Some(process) => { let script_code = match process.command_line { Some(s) if !s.is_empty() => s, - Some(_) if command_required => { + Some(_) if require_process => { return Err(WxcError::ConfigParse( "process.commandLine cannot be empty".to_string(), )); } - None if command_required => { + None if require_process => { return Err(WxcError::ConfigParse( "Missing required field: process.commandLine".to_string(), )); @@ -955,7 +1006,7 @@ fn convert_wire_config( process.env.unwrap_or_default(), ) } - None if command_required => { + None if require_process => { return Err(WxcError::ConfigParse( "'process' section is required".into(), )); @@ -1543,7 +1594,6 @@ fn convert_wire_state_aware( experimental: Option<&str>, experimental_span: Option<(usize, usize)>, logger: &mut Logger, - allow_missing_command: bool, ) -> Result { let experimental_raw = experimental .map(|raw| { @@ -1681,13 +1731,7 @@ fn convert_wire_state_aware( .containment .as_ref() .is_some_and(|value| map_wire_containment(Some(value)) == ContainmentBackend::Wslc); - let mut request = convert_wire_config( - cfg, - logger, - require_process, - allow_missing_command, - state_aware_wslc_exec, - )?; + let mut request = convert_wire_config(cfg, logger, require_process, state_aware_wslc_exec)?; if phase != Phase::Provision && !network_supplied { request.policy.network_egress = None; request.policy.network_ingress = None; @@ -1790,11 +1834,48 @@ mod tests { use crate::encoding::base64_encode; use crate::logger::Mode; use crate::models::{ClipboardPolicy, NetworkAction, ProxyAddress}; + use crate::mxc_error::MxcErrorCode; + + fn argv(args: &[&str]) -> Vec { + args.iter().map(|s| s.to_string()).collect() + } fn test_logger() -> Logger { Logger::new(Mode::Buffer) } + #[test] + fn reused_phase_probe_classifies_one_shot_and_state_aware_phases() { + assert_eq!( + probe_phase(r#"{"process":{"commandLine":"echo hi"}}"#).unwrap(), + None + ); + assert_eq!( + probe_phase(r#"{"phase":"exec","sandboxId":"iso:abcd1234"}"#).unwrap(), + Some(ContractPhase::Exec), + ); + + for (phase, expected) in [ + ("provision", ContractPhase::Provision), + ("start", ContractPhase::Start), + ("stop", ContractPhase::Stop), + ("deprovision", ContractPhase::Deprovision), + ] { + assert_eq!( + probe_phase(&format!(r#"{{"phase":"{phase}"}}"#)).unwrap(), + Some(expected), + ); + } + } + + #[test] + fn reused_phase_probe_is_stricter_than_the_rolling_parser() { + assert!(probe_phase(r#"{"phase":null}"#).is_err()); + assert!(probe_phase(r#"{"phase":42}"#).is_err()); + assert!(probe_phase(r#"{"phase":"start","phase":"exec"}"#).is_err()); + assert!(probe_phase(r#"{"phase":"nope"}"#).is_err()); + } + #[test] fn state_aware_wslc_exec_accepts_proxy_without_redeclaring_network_mode() { let json = r#"{ @@ -1822,7 +1903,7 @@ mod tests { load_mxc_request(&encoded, &mut logger, true) } - fn load_mxc_with_opts(json: &str, opts: LoadOptions) -> Result { + fn load_mxc_with_cli(json: &str, cli_command: &[String]) -> Result { let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); load_mxc_request_with_options( @@ -1830,24 +1911,17 @@ mod tests { &mut logger, LoadOptions { is_base64: true, - ..opts + cli_command, }, ) } #[test] - fn allow_missing_command_lets_one_shot_skip_command_line() { - // No process.commandLine in the policy — without the flag this would - // be a parse error; with allow_missing_command set the parser yields - // an empty script_code for the driver to fill in. + fn cli_command_supplies_a_missing_one_shot_command_line() { let json = r#"{"process": {"cwd": "C:\\tmp"}}"#; - let opts = LoadOptions { - is_base64: true, - allow_missing_command: true, - }; - match load_mxc_with_opts(json, opts).unwrap() { + match load_mxc_with_cli(json, &argv(&["app.exe", "--flag"])).unwrap() { MxcRequest::OneShot(req) => { - assert!(req.script_code.is_empty()); + assert_eq!(req.script_code, "app.exe --flag"); assert_eq!(req.working_directory, "C:\\tmp"); } MxcRequest::StateAware(_) => panic!("expected one-shot"), @@ -1855,48 +1929,419 @@ mod tests { } #[test] - fn allow_missing_command_lets_one_shot_skip_process_block_entirely() { + fn cli_command_supplies_an_absent_one_shot_process_block() { let json = r#"{"containment": "processcontainer"}"#; - let opts = LoadOptions { - is_base64: true, - allow_missing_command: true, - }; - match load_mxc_with_opts(json, opts).unwrap() { - MxcRequest::OneShot(req) => assert!(req.script_code.is_empty()), + match load_mxc_with_cli(json, &argv(&["app.exe", "--flag"])).unwrap() { + MxcRequest::OneShot(req) => assert_eq!(req.script_code, "app.exe --flag"), MxcRequest::StateAware(_) => panic!("expected one-shot"), } } #[test] - fn allow_missing_command_lets_state_aware_exec_skip_command_line() { + fn cli_command_supplies_a_missing_state_aware_exec_command_line() { + let json = r#"{ + "phase": "exec", + "sandboxId": "iso:abcd1234", + "process": {"cwd": "C:\\tmp"} + }"#; + match load_mxc_with_cli(json, &argv(&["app.exe", "--flag"])).unwrap() { + MxcRequest::StateAware(p) => { + assert_eq!(p.phase, Phase::Exec); + assert_eq!(p.request.script_code, "app.exe --flag"); + assert_eq!(p.request.working_directory, "C:\\tmp"); + } + MxcRequest::OneShot(_) => panic!("expected state-aware"), + } + } + + #[test] + fn cli_command_replaces_a_state_aware_exec_command_line() { let json = r#"{ "phase": "exec", "sandboxId": "iso:abcd1234", - "process": {"cwd": "C:\\tmp"} + "process": {"commandLine": "policy.exe", "cwd": "C:\\tmp"} }"#; - let opts = LoadOptions { - is_base64: true, - allow_missing_command: true, - }; - match load_mxc_with_opts(json, opts).unwrap() { + match load_mxc_with_cli(json, &argv(&["app.exe", "--flag"])).unwrap() { MxcRequest::StateAware(p) => { assert_eq!(p.phase, Phase::Exec); - assert!(p.request.script_code.is_empty()); + assert_eq!(p.request.script_code, "app.exe --flag"); + assert_eq!(p.request.working_directory, "C:\\tmp"); } MxcRequest::OneShot(_) => panic!("expected state-aware"), } } #[test] - fn default_options_still_reject_missing_command_line() { + fn state_aware_exec_cli_command_preserves_duplicate_field_errors() { + for (field, json) in [ + ( + "process", + r#"{ + "phase": "exec", + "sandboxId": "iso:abcd1234", + "process": {"commandLine": "first.exe"}, + "process": {"commandLine": "second.exe"} + }"#, + ), + ( + "commandLine", + r#"{ + "phase": "exec", + "sandboxId": "iso:abcd1234", + "process": { + "commandLine": "first.exe", + "commandLine": "second.exe" + } + }"#, + ), + ( + "_comment", + r#"{ + "phase": "exec", + "sandboxId": "iso:abcd1234", + "process": {"commandLine": "policy.exe"}, + "_comment": "first", + "_comment": "second" + }"#, + ), + ] { + let error = load_mxc_with_cli(json, &argv(&["cli.exe"])) + .expect_err("CLI command must not hide the duplicate field"); + + assert!( + matches!(error, ParseError::StateAware(_)), + "{field}: expected state-aware error, got {error:?}" + ); + assert!( + error.message().contains("duplicate field"), + "{field}: unexpected error: {}", + error.message() + ); + } + } + + #[test] + fn state_aware_exec_cli_diagnostics_match_the_effective_document() { + for (case, json, command) in [ + ( + "replacement", + r#"{ + "phase":"exec", + "sandboxId":"iso:abcd1234", + "process":{"commandLine":"x","cwd":42} + }"#, + argv(&["a-much-longer-command.exe"]), + ), + ( + "insertion", + r#"{ + "phase":"exec", + "sandboxId":"iso:abcd1234", + "process":{"cwd":42} + }"#, + argv(&["cli.exe"]), + ), + ] { + let (effective_json, _) = + apply_cli_command(json, &command).expect("command preparation"); + + let expected = load_mxc(&effective_json) + .expect_err("effective document should retain the policy error"); + let actual = load_mxc_with_cli(json, &command) + .expect_err("CLI path should retain the effective-document error"); + + assert!( + matches!(&actual, ParseError::StateAware(_)), + "{case}: expected state-aware error, got {actual:?}" + ); + assert_eq!(actual.message(), expected.message(), "{case}"); + } + } + + #[test] + fn cli_command_preserves_duplicate_field_errors() { + for (field, json) in [ + ( + "filesystem", + r#"{ + "process": {"commandLine": "policy.exe"}, + "filesystem": {"readwritePaths": ["first"]}, + "filesystem": {"readwritePaths": ["second"]} + }"#, + ), + ( + "process", + r#"{ + "process": {"commandLine": "first.exe"}, + "process": {"commandLine": "second.exe"} + }"#, + ), + ( + "commandLine", + r#"{ + "process": { + "commandLine": "first.exe", + "commandLine": "second.exe" + } + }"#, + ), + ] { + assert!(load_mxc(json).is_err(), "sanity: duplicate {field}"); + assert!( + load_mxc_with_cli(json, &argv(&["cli.exe"])).is_err(), + "CLI override must not hide duplicate {field}" + ); + } + } + + #[test] + fn cli_command_preserves_invalid_command_line_type_errors() { + for value in ["42", "true", "[]", "{}"] { + let json = format!(r#"{{"process":{{"commandLine":{value}}}}}"#); + + assert!(load_mxc(&json).is_err(), "sanity: commandLine={value}"); + assert!( + load_mxc_with_cli(&json, &argv(&["cli.exe"])).is_err(), + "CLI override must not hide commandLine={value}" + ); + } + } + + #[test] + fn cli_command_diagnostics_match_the_effective_document() { + for (case, json, command) in [ + ( + "longer replacement", + r#"{"process":{"commandLine":"x","cwd":42}}"#, + argv(&["a-much-longer-command.exe"]), + ), + ( + "shorter replacement", + r#"{"process":{"commandLine":"a-much-longer-policy-command.exe","cwd":42}}"#, + argv(&["x"]), + ), + ("insertion", r#"{"process":{"cwd":42}}"#, argv(&["cli.exe"])), + ] { + let (effective_json, _) = + apply_cli_command(json, &command).expect("command preparation"); + let expected = load_mxc(&effective_json) + .expect_err("effective document should retain the policy error") + .message(); + let actual = load_mxc_with_cli(json, &command) + .expect_err("CLI path should retain the effective-document error") + .message(); + + assert_eq!(actual, expected, "{case}"); + } + } + + #[test] + fn cli_command_render_error_precedes_an_unrelated_one_shot_policy_error() { + let json = r#"{"process":{"commandLine":"policy.exe","cwd":42}}"#; + let error = load_mxc_with_cli(json, &argv(&["cli.exe", "hidden\0payload"])) + .expect_err("command rendering should fail before typed policy validation"); + + assert!(matches!(error, ParseError::Decode(_))); + assert!( + error.message().contains("invalid CLI command override"), + "unexpected error: {}", + error.message() + ); + } + + #[test] + fn state_aware_backend_probe_errors_precede_unrelated_policy_errors() { + for (case, json, expected_code) in [ + ( + "missing sandbox id", + r#"{"phase":"exec","process":{"commandLine":42}}"#, + MxcErrorCode::MalformedRequest, + ), + ( + "unsupported sandbox id", + r#"{"phase":"exec","sandboxId":"zzz:abcd","process":{"commandLine":42}}"#, + MxcErrorCode::UnsupportedContainment, + ), + ] { + let error = load_mxc_with_cli(json, &argv(&["cli.exe"])) + .expect_err("backend probing should fail before typed policy validation"); + + match error { + ParseError::StateAware(error) => { + assert_eq!(error.code, expected_code, "{case}"); + } + other => panic!("{case}: expected state-aware error, got {other:?}"), + } + } + } + + #[test] + fn missing_command_line_is_rejected_without_a_cli_command() { // Sanity: without the flag, the legacy contract holds — missing // commandLine is a hard parse error. let json = r#"{"process": {"cwd": "C:\\tmp"}}"#; - let opts = LoadOptions { - is_base64: true, - allow_missing_command: false, - }; - assert!(load_mxc_with_opts(json, opts).is_err()); + assert!(load_mxc_with_cli(json, &[]).is_err()); + } + + #[test] + fn apply_cli_command_returns_override_log_for_a_one_shot_replacement() { + let (out, override_log) = apply_cli_command( + r#"{"process":{"commandLine":"policy.exe"}}"#, + &argv(&["app.exe", "--flag"]), + ) + .unwrap(); + + let doc: serde_json::Value = serde_json::from_str(&out).unwrap(); + assert_eq!(doc["process"]["commandLine"], "app.exe --flag"); + assert_eq!( + override_log.as_deref(), + Some("Overriding policy process.commandLine with CLI command: app.exe --flag") + ); + } + + #[test] + fn apply_cli_command_returns_no_override_log_when_the_policy_had_no_command() { + let (out, override_log) = apply_cli_command( + r#"{"process":{"cwd":"/usr/tmp"}}"#, + &argv(&["app.exe", "--flag"]), + ) + .unwrap(); + + let doc: serde_json::Value = serde_json::from_str(&out).unwrap(); + assert_eq!(doc["process"]["commandLine"], "app.exe --flag"); + assert!(override_log.is_none()); + } + + #[test] + fn cli_command_does_not_log_override_when_the_effective_policy_is_invalid() { + let json = r#"{ + "process": { + "commandLine": "policy.exe", + "cwd": 42 + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let command = argv(&["app.exe", "--flag"]); + let mut logger = test_logger(); + + let result = load_mxc_request_with_options( + &encoded, + &mut logger, + LoadOptions { + is_base64: true, + cli_command: &command, + }, + ); + + assert!(result.is_err()); + assert!(!logger + .get_buffer() + .contains("Overriding policy process.commandLine")); + } + + #[test] + fn apply_cli_command_splices_into_a_state_aware_exec_request() { + // The `sandboxId` prefix selects the quoting context, so the argument + // carries `&`: cmd.exe quotes it because `&` separates commands, a + // POSIX shell single-quotes it, and the direct Windows path leaves it + // bare. An argument needing no quoting would render identically under + // all three and prove nothing about the prefix. + for (sandbox_id, expected) in [ + // iso -> IsolationSession -> WindowsCommandProcessor + ("iso:abcd1234", "app.exe \"a&b\""), + // wslc -> Wslc -> PosixShell + ("wslc:abcd1234", "app.exe 'a&b'"), + ] { + let json = format!(r#"{{"phase":"exec","sandboxId":"{sandbox_id}"}}"#); + let (out, override_log) = apply_cli_command(&json, &argv(&["app.exe", "a&b"])).unwrap(); + + let doc: serde_json::Value = serde_json::from_str(&out).unwrap(); + assert_eq!( + doc["process"]["commandLine"], expected, + "wrong quoting context for {sandbox_id}" + ); + assert!(override_log.is_none()); + } + } + + #[test] + fn apply_cli_command_rejects_a_non_exec_phase_with_an_envelope_error() { + let err = apply_cli_command( + r#"{"phase":"start","sandboxId":"iso:abcd1234"}"#, + &argv(&["echo", "hi"]), + ) + .unwrap_err(); + assert!(matches!(err, ParseError::StateAware(_))); + } + + #[test] + fn apply_cli_command_surfaces_an_unregistered_sandbox_id_prefix() { + let err = apply_cli_command( + r#"{"phase":"exec","sandboxId":"zzz:abcd"}"#, + &argv(&["app.exe", "--flag"]), + ) + .unwrap_err(); + + assert!(matches!(err, ParseError::StateAware(_))); + } + + #[test] + fn apply_cli_command_returns_the_source_unchanged_when_it_cannot_classify() { + // Each passthrough path: unreadable phase ({"phase":null}), unreadable + // containment ({"containment":"nope"}), unspliceable document + // ({"process":42}). Assert the output is byte-identical to the input. + + for json in [ + r#"{"phase":null,"sandboxId":"iso:abcd1234"}"#, + r#"{"containment":"nope"}"#, + r#"{"process":42}"#, + ] { + let (out, override_log) = + apply_cli_command(json, &argv(&["app.exe", "--flag"])).unwrap(); + assert_eq!(out, json); + assert!(override_log.is_none()); + } + } + + #[test] + fn apply_cli_command_rejects_an_empty_argv() { + let err = + apply_cli_command(r#"{"process":{"commandLine":"policy.exe"}}"#, &[]).unwrap_err(); + assert!(matches!(err, ParseError::Decode(_))); + } + + #[test] + fn apply_cli_command_rejects_an_unconvertible_command() { + // A null byte fails argv rendering, which is an entry-point error + // rather than a parse error: the document is never spliced. + let err = apply_cli_command( + r#"{"process":{"commandLine":"policy.exe"}}"#, + &argv(&["app.exe", "hidden\0payload"]), + ) + .unwrap_err(); + + assert!(matches!(err, ParseError::Decode(_))); + assert!( + err.message().contains("invalid CLI command override"), + "unexpected message: {}", + err.message() + ); + } + + #[test] + fn apply_cli_command_routes_an_unconvertible_state_aware_exec_command_to_an_envelope() { + let err = apply_cli_command( + r#"{"phase":"exec","sandboxId":"iso:abcd1234"}"#, + &argv(&["app.exe", "hidden\0payload"]), + ) + .unwrap_err(); + + assert!(matches!(err, ParseError::StateAware(_))); + assert!( + err.message().contains("invalid CLI command override"), + "unexpected message: {}", + err.message() + ); } #[test] @@ -2034,14 +2479,11 @@ mod tests { r#"{{ "phase": "{phase}", "sandboxId": "iso:abcd1234", - "containment": "wslc" + "containment": "wslc", + "process": {{"commandLine": "echo hi"}} }}"# ); - let opts = LoadOptions { - is_base64: true, - allow_missing_command: true, - }; - let r = load_mxc_with_opts(&json, opts); + let r = load_mxc_with_cli(&json, &[]); assert!( matches!(r, Err(ParseError::StateAware(_))), "phase {phase}: expected state-aware rejection, got {:?}", @@ -2473,7 +2915,7 @@ mod tests { }); let mut logger = test_logger(); - let error = load_request_from_value(config, &mut logger, false).unwrap_err(); + let error = load_request_from_value(config, &mut logger).unwrap_err(); let message = error.to_string(); assert!(message.contains("Invalid configuration at `process.timeout`")); assert!(message.contains("expected u32")); @@ -2506,7 +2948,7 @@ mod tests { }), ] { let mut logger = test_logger(); - assert!(load_request_from_value(config, &mut logger, false).is_err()); + assert!(load_request_from_value(config, &mut logger).is_err()); } } @@ -6775,8 +7217,7 @@ mod tests { let mut logger = test_logger(); let error = - load_request_from_value(serde_json::from_str(json).unwrap(), &mut logger, false) - .unwrap_err(); + load_request_from_value(serde_json::from_str(json).unwrap(), &mut logger).unwrap_err(); assert!(error.to_string().contains(expected), "got {error:?}"); } @@ -6877,7 +7318,7 @@ mod tests { "experimental": { "telemetry": { "enabled": true } } }); let mut logger = test_logger(); - let error = load_request_from_value(config, &mut logger, false).unwrap_err(); + let error = load_request_from_value(config, &mut logger).unwrap_err(); assert!( error .to_string() @@ -6894,7 +7335,7 @@ mod tests { "experimental": { "telemetry": { "enabled": true } } }); let mut logger = test_logger(); - let error = load_request_from_value(config, &mut logger, false).unwrap_err(); + let error = load_request_from_value(config, &mut logger).unwrap_err(); assert!(error .to_string() .contains("'experimental.telemetry' has moved")); diff --git a/src/core/wxc_common/src/lib.rs b/src/core/wxc_common/src/lib.rs index aac9ff919..f8cc247c4 100644 --- a/src/core/wxc_common/src/lib.rs +++ b/src/core/wxc_common/src/lib.rs @@ -29,6 +29,7 @@ pub use network_parser::supports_directional_network; pub mod proxy_env; pub mod sandbox_process; pub mod script_runner; +pub(crate) mod splice; pub mod state_aware_backend; pub mod state_aware_dispatch; pub mod state_aware_request; diff --git a/src/core/wxc_common/src/splice.rs b/src/core/wxc_common/src/splice.rs new file mode 100644 index 000000000..4a3fcdb9d --- /dev/null +++ b/src/core/wxc_common/src/splice.rs @@ -0,0 +1,453 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use crate::id::parse_sandbox_id_prefix; +use crate::models::ContainmentBackend; +use crate::mxc_error::MxcError; +use crate::wire; +use serde::de::{MapAccess, Visitor}; +use serde::{Deserialize, Deserializer}; +use serde_json::value::RawValue; +use std::fmt; +use std::ops::Range; + +pub(crate) struct Spliced { + pub json: String, + /// True when the document already carried a non-empty `process.commandLine`. + /// Drives the "Overriding policy process.commandLine" log. + pub replaced_existing: bool, +} + +struct RawMember<'a> { + key: String, + value: &'a RawValue, +} + +// Keep members in source order rather than a map so duplicate keys survive +// this structural probe for the typed parser to reject later. +struct RawObject<'a> { + members: Vec>, +} + +impl<'de> Deserialize<'de> for RawObject<'de> { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct RawObjectVisitor; + + impl<'de> Visitor<'de> for RawObjectVisitor { + type Value = RawObject<'de>; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a JSON object") + } + + fn visit_map(self, mut map: A) -> Result + where + A: MapAccess<'de>, + { + let mut members = Vec::new(); + while let Some((key, value)) = map.next_entry::()? { + members.push(RawMember { key, value }); + } + Ok(RawObject { members }) + } + } + + deserializer.deserialize_map(RawObjectVisitor) + } +} + +pub(crate) struct CommandSource<'a> { + json: &'a str, + root: RawObject<'a>, +} + +impl<'a> CommandSource<'a> { + pub(crate) fn parse(json: &'a str) -> Option { + let root = serde_json::from_str(json).ok()?; + Some(Self { json, root }) + } + + pub(crate) fn one_shot_backend(&self) -> Option { + let containment = match find_member(&self.root, "containment") { + MemberMatch::Missing => None, + MemberMatch::Duplicate => return None, + MemberMatch::Unique(value) => { + serde_json::from_str::>(value.get()).ok()? + } + }; + + Some(containment.unwrap_or(wire::Containment::Process).into()) + } + + pub(crate) fn state_aware_backend(&self) -> Result { + let sandbox_id = match find_member(&self.root, "sandboxId") { + MemberMatch::Missing => None, + MemberMatch::Duplicate => { + return Err(MxcError::malformed_request("duplicate field `sandboxId`")) + } + MemberMatch::Unique(value) => serde_json::from_str::>(value.get()) + .map_err(|_| MxcError::malformed_request("'sandboxId' must be a string"))?, + } + .ok_or_else(|| MxcError::malformed_request("state-aware requests require 'sandboxId'"))?; + + crate::state_aware_dispatch::backend_from_prefix(parse_sandbox_id_prefix(&sandbox_id)?) + } +} + +enum MemberMatch<'a> { + Missing, + Unique(&'a RawValue), + Duplicate, +} + +fn find_member<'a>(object: &'a RawObject<'a>, key: &str) -> MemberMatch<'a> { + let mut matches = object + .members + .iter() + .filter(|member| member.key == key) + .map(|member| member.value); + let Some(value) = matches.next() else { + return MemberMatch::Missing; + }; + if matches.next().is_some() { + MemberMatch::Duplicate + } else { + MemberMatch::Unique(value) + } +} + +fn raw_value_range(source: &str, value: &RawValue) -> Option> { + // Borrowed RawValues point into `source`; convert that subslice into the + // byte range replaced by the localized edit. + let start = (value.get().as_ptr() as usize).checked_sub(source.as_ptr() as usize)?; + let end = start.checked_add(value.get().len())?; + (end <= source.len()).then_some(start..end) +} + +fn replace_range(source: &str, range: Range, replacement: &str) -> String { + let mut output = String::with_capacity(source.len() - range.len() + replacement.len()); + output.push_str(&source[..range.start]); + output.push_str(replacement); + output.push_str(&source[range.end..]); + output +} + +fn object_opening_brace(source: &str) -> Option { + source + .char_indices() + .find(|(_, character)| !character.is_whitespace()) + .and_then(|(index, character)| (character == '{').then_some(index)) +} + +fn insert_member(source: &str, object: &RawObject<'_>, member: &str) -> Option { + let (offset, separator) = match object.members.last() { + Some(last) => (raw_value_range(source, last.value)?.end, ","), + None => (object_opening_brace(source)? + 1, ""), + }; + Some(replace_range( + source, + offset..offset, + &format!("{separator}{member}"), + )) +} + +impl CommandSource<'_> { + pub(crate) fn splice_command(&self, command: &str) -> Option { + let command = serde_json::to_string(command).ok()?; + + match find_member(&self.root, "process") { + MemberMatch::Missing => { + let process = format!(r#""process":{{"commandLine":{command}}}"#); + Some(Spliced { + json: insert_member(self.json, &self.root, &process)?, + replaced_existing: false, + }) + } + MemberMatch::Duplicate => None, + MemberMatch::Unique(process_raw) => { + let process_source = process_raw.get(); + let process: RawObject<'_> = serde_json::from_str(process_source).ok()?; + + let (process_json, replaced_existing) = match find_member(&process, "commandLine") { + MemberMatch::Missing => { + let member = format!(r#""commandLine":{command}"#); + (insert_member(process_source, &process, &member)?, false) + } + MemberMatch::Duplicate => return None, + MemberMatch::Unique(command_line_raw) => { + let replaced_existing = if command_line_raw.get().trim() == "null" { + false + } else { + let existing: String = + serde_json::from_str(command_line_raw.get()).ok()?; + !existing.is_empty() + }; + let range = raw_value_range(process_source, command_line_raw)?; + ( + replace_range(process_source, range, &command), + replaced_existing, + ) + } + }; + + let process_range = raw_value_range(self.json, process_raw)?; + Some(Spliced { + json: replace_range(self.json, process_range, &process_json), + replaced_existing, + }) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config_parser::load_mxc_request_from_json; + use crate::logger::{Logger, Mode}; + use crate::mxc_error::MxcErrorCode; + use crate::state_aware_request::MxcRequest; + use serde_json::Value; + + fn splice_command(json: &str, command: &str) -> Option { + CommandSource::parse(json)?.splice_command(command) + } + + #[test] + fn one_shot_backend_agrees_with_the_parser_for_every_spelling() { + for spelling in [ + "process", + "processcontainer", + "appcontainer", + "vm", + "windows_sandbox", + "lxc", + "microvm", + "hyperlight", + "wslc", + "seatbelt", + "macos_sandbox", + "isolation_session", + ] { + let wire: wire::Containment = + serde_json::from_str(&format!(r#""{spelling}""#)).unwrap(); + let json = format!(r#"{{"containment":"{spelling}"}}"#); + let source = CommandSource::parse(&json).unwrap(); + assert_eq!( + source.one_shot_backend(), + Some(crate::config_parser::map_wire_containment(Some(&wire))), + "raw command source and parser disagree on {spelling}", + ); + } + + let source = CommandSource::parse("{}").unwrap(); + assert_eq!( + source.one_shot_backend(), + Some(crate::config_parser::map_wire_containment(None)), + ); + } + + #[test] + fn state_aware_backend_resolves_every_registered_prefix() { + for (id, expected) in [ + ("iso:abcd1234", ContainmentBackend::IsolationSession), + ("wsb:abcd1234", ContainmentBackend::WindowsSandbox), + ("wslc:abcd1234", ContainmentBackend::Wslc), + ] { + let json = format!(r#"{{"phase":"exec","sandboxId":"{id}"}}"#); + let source = CommandSource::parse(&json).unwrap(); + assert_eq!(source.state_aware_backend().unwrap(), expected); + } + } + + #[test] + fn state_aware_backend_rejects_missing_malformed_and_unregistered_ids() { + for json in [ + r#"{"phase":"exec"}"#, + r#"{"sandboxId":"no-colon"}"#, + r#"{"sandboxId":":abcd"}"#, + ] { + let source = CommandSource::parse(json).unwrap(); + assert!(source.state_aware_backend().is_err(), "{json}"); + } + + let source = CommandSource::parse(r#"{"sandboxId":"zzz:abcd"}"#).unwrap(); + let error = source.state_aware_backend().unwrap_err(); + assert_eq!(error.code, MxcErrorCode::UnsupportedContainment); + } + + #[test] + fn one_shot_backend_returns_none_for_malformed_declarations() { + for json in [ + r#"{"containment":"nope"}"#, + r#"{"containment":42}"#, + r#"{"containment":"process","containment":"lxc"}"#, + ] { + let source = CommandSource::parse(json).unwrap(); + assert!(source.one_shot_backend().is_none(), "{json}"); + } + + assert!(CommandSource::parse(r#"{ "process": "#).is_none()); + + let source = CommandSource::parse(r#"{"containment":null}"#).unwrap(); + assert_eq!( + source.one_shot_backend(), + Some(crate::config_parser::map_wire_containment(None)), + ); + } + + #[test] + fn splice_overwrites_an_existing_command_line() { + let original = r#"{"process":{"commandLine":"echo hi"}}"#; + let spliced = splice_command(original, "echo bye").unwrap(); + assert_eq!(spliced.json, r#"{"process":{"commandLine":"echo bye"}}"#); + assert!(spliced.replaced_existing); + } + + #[test] + fn splice_creates_an_absent_process_object() { + let original = r#"{}"#; + let spliced = splice_command(original, "echo bye").unwrap(); + assert_eq!(spliced.json, r#"{"process":{"commandLine":"echo bye"}}"#); + assert!(!spliced.replaced_existing); + } + + #[test] + fn splice_reports_an_empty_command_line_as_not_replaced() { + let original = r#"{"process":{"commandLine":""}}"#; + let spliced = splice_command(original, "echo bye").unwrap(); + assert_eq!(spliced.json, r#"{"process":{"commandLine":"echo bye"}}"#); + assert!(!spliced.replaced_existing); + } + + #[test] + fn splice_reports_a_null_command_line_as_not_replaced() { + let original = r#"{"process":{"commandLine":null}}"#; + let spliced = splice_command(original, "echo bye").unwrap(); + assert_eq!(spliced.json, r#"{"process":{"commandLine":"echo bye"}}"#); + assert!(!spliced.replaced_existing); + } + + #[test] + fn splice_rejects_invalid_existing_command_line_types() { + for value in ["42", "true", "[]", "{}"] { + let original = format!(r#"{{"process":{{"commandLine":{value}}}}}"#); + assert!( + splice_command(&original, "echo bye").is_none(), + "invalid commandLine value should be left for the parser: {value}" + ); + } + } + + #[test] + fn splice_rejects_duplicate_process_members() { + let original = r#"{"process":{},"process":{}}"#; + assert!(splice_command(original, "echo bye").is_none()); + } + + #[test] + fn splice_rejects_duplicate_command_line_members() { + let original = r#"{"process":{"commandLine":"first.exe","commandLine":"second.exe"}}"#; + assert!(splice_command(original, "echo bye").is_none()); + } + + #[test] + fn splice_preserves_unrelated_duplicate_members_for_typed_validation() { + let original = r#"{ + "process": {"commandLine": "policy.exe"}, + "filesystem": {"readwritePaths": ["first"]}, + "filesystem": {"readwritePaths": ["second"]} + }"#; + + let spliced = splice_command(original, "cli.exe").unwrap(); + + assert_eq!(spliced.json.matches("\"filesystem\"").count(), 2); + } + + const RICH_POLICY: &str = r#"{ + "$schema": "https://example.com/mxc-config.schema.json", + "_comment": null, + "version": "0.6.0-alpha", + "containerId": "test-container", + "containment": "processcontainer", + "lifecycle": { "destroyOnExit": false, "preservePolicy": true }, + "process": { + "commandLine": "policy-app.exe --from-policy", + "cwd": "C:\\work space\\proj", + "env": ["PATH=C:\\bin", "GREETING=héllo \"world\"", "EMPTY="], + "timeout": 4294967295 + }, + "filesystem": { + "readwritePaths": ["C:\\rw"], + "readonlyPaths": [], + "deniedPaths": ["C:\\secrets"] + }, + "network": { + "defaultPolicy": "allow", + "allowLocalNetwork": false, + "allowedHosts": ["example.com", "*.contoso.com"], + "proxy": { "localhost": 8080 } + }, + "processContainer": { + "leastPrivilege": true, + "capabilities": [] + } + }"#; + + #[test] + fn splice_preserves_every_other_field() { + let over = "cli-app.exe --from-cli"; + let spliced = splice_command(RICH_POLICY, over).unwrap(); + + let expected = + RICH_POLICY.replacen("policy-app.exe --from-policy", "cli-app.exe --from-cli", 1); + assert_eq!(spliced.json, expected); + + let actual = serde_json::from_str::(&spliced.json).unwrap(); + let mut expected: Value = serde_json::from_str(RICH_POLICY).unwrap(); + expected["process"]["commandLine"] = Value::String(over.to_string()); + + assert_eq!(actual, expected); + assert!(spliced.replaced_existing); + } + + #[test] + fn splice_rejects_a_non_object_process() { + let original = r#"{"process":42}"#; + assert!(splice_command(original, "echo bye").is_none()); + } + + #[test] + fn splice_rejects_a_non_object_root() { + for original in [r#"42"#, r#""a string""#, r#"[]"#, r#"[{"process":{}}]"#] { + assert!( + splice_command(original, "echo bye").is_none(), + "non-object root should not splice: {original}" + ); + } + } + + #[test] + fn splice_output_reparses_into_the_same_request() { + let over = "cli-app.exe --from-cli"; + let original = r#"{ + "process": { "cwd": "C:\\workspace" }, + "filesystem": { "readwritePaths": ["C:\\workspace"] } + }"#; + + let spliced = splice_command(original, over).unwrap(); + + let mut logger = Logger::new(Mode::Buffer); + let request = match load_mxc_request_from_json(&spliced.json, &mut logger).unwrap() { + MxcRequest::OneShot(request) => request, + MxcRequest::StateAware(_) => panic!("expected a one-shot request"), + }; + + // The spliced document is a complete request in its own right. + assert_eq!(request.script_code, over); + assert_eq!(request.working_directory, "C:\\workspace"); + } +} diff --git a/src/core/wxc_common/src/state_aware_dispatch.rs b/src/core/wxc_common/src/state_aware_dispatch.rs index fead6e3c4..4f49eef45 100644 --- a/src/core/wxc_common/src/state_aware_dispatch.rs +++ b/src/core/wxc_common/src/state_aware_dispatch.rs @@ -183,7 +183,7 @@ pub fn resolve_backend(parsed: &ParsedStateAwareRequest) -> Result Result { +pub(crate) fn backend_from_prefix(prefix: &str) -> Result { match prefix { "iso" => Ok(ContainmentBackend::IsolationSession), "wsb" => Ok(ContainmentBackend::WindowsSandbox), diff --git a/src/ffi/mxc_ffi/src/lib.rs b/src/ffi/mxc_ffi/src/lib.rs index d4c376375..510ffb6b2 100644 --- a/src/ffi/mxc_ffi/src/lib.rs +++ b/src/ffi/mxc_ffi/src/lib.rs @@ -907,6 +907,24 @@ mod tests { assert!(out.error.message_utf8.is_null()); } + #[test] + fn empty_command_reports_malformed_request() { + let mut out = run_with(r#"{"policy":{"version":"0.7.0-alpha"},"command":""}"#); + assert_eq!(out.status, MXC_STATUS_MALFORMED_REQUEST); + assert!(!out.error.message_utf8.is_null()); + assert!(out.stdout_utf8.is_null()); + assert!(out.stderr_utf8.is_null()); + + // SAFETY: `out` was filled by `mxc_run_request`. + let message = unsafe { CStr::from_ptr(out.error.message_utf8) } + .to_str() + .unwrap(); + assert_eq!(message, "script parameter is required"); + + // SAFETY: `out` was filled by `mxc_run_request`. + unsafe { mxc_run_result_free(&mut out) }; + } + #[test] fn null_out_pointer_reports_null_argument_without_leaking() { let request = diff --git a/src/ffi/mxc_ffi/src/request.rs b/src/ffi/mxc_ffi/src/request.rs index 5c32aa5b4..441e0c54c 100644 --- a/src/ffi/mxc_ffi/src/request.rs +++ b/src/ffi/mxc_ffi/src/request.rs @@ -332,9 +332,12 @@ pub(crate) fn build_request_from_json(request_json: &str) -> Result