diff --git a/crates/openshell-driver-mxc/README.md b/crates/openshell-driver-mxc/README.md index 81511deabd..b661448aee 100644 --- a/crates/openshell-driver-mxc/README.md +++ b/crates/openshell-driver-mxc/README.md @@ -75,6 +75,14 @@ debug = false etw_audit = false ``` +`wxc_exec_path` is required and must be an absolute path to `wxc-exec.exe`. +The gateway rejects an omitted or relative value (including the bare filename +`wxc-exec.exe`) at startup, before any sandbox is created: `wxc-exec.exe` is +the binary that builds every sandbox, so a relative value would let +PATH-lookup or working-directory-relative resolution execute an unapproved +binary with the gateway's identity instead of the approved `wxc-exec`. There +is no usable default. + When `egress_proxy` is enabled, `egress_proxy_addr` must be a loopback `IP:PORT` seed. For policies with explicit network rules, the driver preserves the configured IP and allocates a unique ephemeral port for that sandbox's diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 8cf5e17a47..412b352b34 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -179,7 +179,12 @@ impl MxcBackend { #[serde(default, deny_unknown_fields)] #[allow(clippy::struct_excessive_bools)] // Independent, existing gateway TOML options. pub struct MxcComputeConfig { - /// Path to `wxc-exec.exe`. Required for live runs. + /// Path to `wxc-exec.exe`. Required for live runs, and must be an + /// absolute path: `wxc-exec` is the binary that builds every sandbox, so + /// a relative path (including the unset default) would let PATH-lookup + /// or working-directory-relative resolution execute a decoy binary with + /// the gateway's identity instead of the approved `wxc-exec`. Enforced + /// at gateway startup by the compute-driver config preflight. pub wxc_exec_path: String, /// Backend to target. Default: `process_container`. pub backend: MxcBackend, @@ -256,7 +261,12 @@ pub struct MxcComputeConfig { impl Default for MxcComputeConfig { fn default() -> Self { Self { - wxc_exec_path: "wxc-exec.exe".into(), + // No usable default: `wxc_exec_path` must be explicitly set to an + // absolute path (see `validate_configuration` and the field doc + // comment above). Shipping a bare relative filename here would + // silently reintroduce the exact PATH/CWD-hijack risk the + // validation exists to reject. + wxc_exec_path: String::new(), backend: MxcBackend::default(), pc_least_privilege: false, pc_capabilities: Vec::new(), @@ -275,6 +285,32 @@ impl Default for MxcComputeConfig { } } +impl MxcComputeConfig { + /// Validate startup configuration without touching `wxc-exec` or the + /// filesystem beyond `Path::is_absolute`. + /// + /// `wxc_exec_path` must be set to an absolute path: it is the binary + /// that builds every sandbox, so a relative path (including an unset, + /// empty value) would let PATH-lookup or working-directory-relative + /// resolution execute a decoy binary with the gateway's identity instead + /// of the approved `wxc-exec`, turning the containment mechanism itself + /// into an arbitrary-code-execution primitive. + pub fn validate_configuration(&self) -> openshell_core::Result<()> { + if self.wxc_exec_path.trim().is_empty() { + return Err(openshell_core::Error::config( + "[openshell.drivers.mxc] wxc_exec_path must be set to an absolute path to wxc-exec.exe", + )); + } + if !Path::new(&self.wxc_exec_path).is_absolute() { + return Err(openshell_core::Error::config(format!( + "[openshell.drivers.mxc] wxc_exec_path must be an absolute path, got '{}'", + self.wxc_exec_path + ))); + } + Ok(()) + } +} + /// Per-sandbox MXC workload settings supplied through /// `template.driver_config.mxc` / `--driver-config-json`. #[derive(Debug, Clone, Deserialize)] @@ -2790,6 +2826,50 @@ mod lifecycle_tests { } } + #[test] + fn validate_configuration_rejects_unset_wxc_exec_path() { + // Regression test: the shipped default used to be the bare relative + // filename "wxc-exec.exe", which is exactly the PATH/CWD-hijack + // primitive this validation exists to reject. The default must stay + // rejected, not silently become a usable-but-insecure fallback. + let config = MxcComputeConfig::default(); + assert!(config.wxc_exec_path.is_empty()); + let error = config.validate_configuration().unwrap_err(); + assert!(error.to_string().contains("wxc_exec_path")); + } + + #[test] + fn validate_configuration_rejects_relative_wxc_exec_path() { + let config = MxcComputeConfig { + wxc_exec_path: "wxc-exec.exe".into(), + ..Default::default() + }; + let error = config.validate_configuration().unwrap_err(); + assert!(error.to_string().contains("wxc_exec_path")); + + let config = MxcComputeConfig { + wxc_exec_path: r"..\wxc-exec.exe".into(), + ..Default::default() + }; + assert!(config.validate_configuration().is_err()); + } + + #[test] + fn validate_configuration_accepts_absolute_wxc_exec_path() { + let config = MxcComputeConfig { + wxc_exec_path: r"C:\mxc-kit\bin\wxc-exec.exe".into(), + ..Default::default() + }; + config.validate_configuration().unwrap(); + } + + #[test] + fn governed_egress_defaults_off_and_allocates_unique_loopback_ports() { + let config = MxcComputeConfig::default(); + assert!(!config.egress_proxy); + assert!(config.egress_proxy_addr.is_empty()); + } + #[test] fn sandbox_proxy_addr_uses_ephemeral_loopback_port() { let configured = "127.0.0.1:18080".parse().unwrap(); diff --git a/crates/openshell-gateway/src/lib.rs b/crates/openshell-gateway/src/lib.rs index 30e1c83727..f5f2074da9 100644 --- a/crates/openshell-gateway/src/lib.rs +++ b/crates/openshell-gateway/src/lib.rs @@ -149,8 +149,8 @@ impl openshell_server::ComputeDriverFactory for MxcFactory { &self, context: openshell_server::ComputeDriverConfigContext<'_>, ) -> openshell_core::Result<()> { - let _: openshell_driver_mxc::MxcComputeConfig = context.driver_config()?; - Ok(()) + let config: openshell_driver_mxc::MxcComputeConfig = context.driver_config()?; + config.validate_configuration() } async fn build( diff --git a/crates/openshell-gateway/tests/mxc_config_preflight.rs b/crates/openshell-gateway/tests/mxc_config_preflight.rs new file mode 100644 index 0000000000..c4314f3079 --- /dev/null +++ b/crates/openshell-gateway/tests/mxc_config_preflight.rs @@ -0,0 +1,100 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Integration test for the actual enforcement boundary of the +//! `wxc_exec_path` absolute-path requirement: the real `openshell-gateway` +//! binary's `config preflight` subcommand, selecting the `mxc` compute +//! driver. +//! +//! `openshell_driver_mxc::MxcComputeConfig::validate_configuration` already +//! has unit coverage, but that only proves the validation function itself is +//! correct -- it says nothing about whether `MxcFactory::validate_config` (in +//! `src/lib.rs`) still calls it. Before this fix, that factory method was a +//! no-op that discarded the parsed config entirely, so a regression back to +//! that shape would leave every unit test passing while a relative +//! `wxc_exec_path` again reached gateway startup. Spawning the real compiled +//! binary through its actual CLI entry point exercises the whole chain: CLI +//! parsing, TOML config loading, driver selection, `MxcFactory::validate_config`, +//! and `MxcComputeConfig::validate_configuration`. +//! +//! These tests assert only pass/fail, not the diagnostic's exact wording: +//! `openshell_server::cli::run_effective_config_preflight` replaces any +//! validation failure with a generic "malformed" message whenever a config +//! file path is in play (`ConfigPreflightError::invalid_current`), so the +//! specific `wxc_exec_path` wording from `validate_configuration` is not +//! observable through this boundary today. That message-masking behavior is +//! pre-existing and unrelated to this fix. + +#![cfg(all(target_os = "windows", feature = "compute-driver-mxc"))] + +use std::io::Write; +use std::process::Command; + +/// Run `openshell-gateway config preflight` against a disposable TOML config +/// with the `mxc` driver selected. Returns whether the process exited +/// successfully and its captured stderr. +fn run_preflight(toml_body: &str) -> (bool, String) { + let mut config_file = tempfile::NamedTempFile::new().expect("create temp config file"); + write!(config_file, "{toml_body}").expect("write temp config file"); + + let output = Command::new(env!("CARGO_BIN_EXE_openshell-gateway")) + .arg("config") + .arg("preflight") + .env("OPENSHELL_GATEWAY_CONFIG", config_file.path()) + .env("OPENSHELL_COMPUTE_DRIVER", "mxc") + .env("OPENSHELL_DISABLE_TLS", "true") + .env_remove("OPENSHELL_DRIVERS") + .output() + .expect("spawn openshell-gateway config preflight"); + + let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + (output.status.success(), stderr) +} + +#[test] +fn config_preflight_rejects_omitted_wxc_exec_path() { + let (ok, stderr) = run_preflight( + r" +[openshell] +version = 2 +", + ); + assert!( + !ok, + "omitted wxc_exec_path must fail preflight, stderr: {stderr}" + ); +} + +#[test] +fn config_preflight_rejects_relative_wxc_exec_path() { + let (ok, stderr) = run_preflight( + r#" +[openshell] +version = 2 + +[openshell.drivers.mxc] +wxc_exec_path = "wxc-exec.exe" +"#, + ); + assert!( + !ok, + "a relative wxc_exec_path must fail preflight, stderr: {stderr}" + ); +} + +#[test] +fn config_preflight_accepts_absolute_wxc_exec_path() { + let (ok, stderr) = run_preflight( + r#" +[openshell] +version = 2 + +[openshell.drivers.mxc] +wxc_exec_path = "C:\\mxc-kit\\bin\\wxc-exec.exe" +"#, + ); + assert!( + ok, + "an absolute wxc_exec_path must pass preflight, stderr: {stderr}" + ); +} diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index a292cb030c..c17f8e2d8c 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -687,6 +687,14 @@ debug = false etw_audit = true ``` +`wxc_exec_path` is required and must be an absolute path to `wxc-exec.exe`. +The gateway rejects an omitted or relative value at startup: `wxc-exec.exe` +is the binary that builds every sandbox, so a relative value would let +PATH-lookup or working-directory-relative resolution execute an unapproved +binary with the gateway's identity instead of the intended `wxc-exec`. There +is no usable default; older configurations that relied on the bare relative +filename `wxc-exec.exe` must set an absolute path before upgrading. + `etw_audit` defaults to `false`. When enabled, the gateway account must be an administrator or belong to the Windows Performance Log Users group. Workload commands and working directories remain sandbox-scoped and must be supplied in @@ -949,6 +957,13 @@ debug = false etw_audit = false ``` +`wxc_exec_path` is required and must be an absolute path; the gateway rejects +an omitted or relative value at startup. `wxc-exec.exe` builds every sandbox, +so a relative value would let PATH-lookup or working-directory-relative +resolution execute an unapproved binary with the gateway's identity. There is +no usable default -- older configurations that relied on the bare relative +filename `wxc-exec.exe` must set an absolute path before upgrading. + The default `pc_least_privilege = false` still runs the workload with an AppContainer token. Set it to `true` to request the stricter Less Privileged AppContainer (LPAC) variant. Windows AppContainer tokens retain the launching