Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions src/backends/appcontainer/common/src/appcontainer_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1137,17 +1137,12 @@ impl AppContainerScriptRunner {
FileSystemBfsManager::new(self.app_container_name.clone(), bfscfg_path);
if self.filesystem_mode == FilesystemMode::Bfs {
if let Err(e) = bfs_manager.configure(&request.policy, logger) {
let msg = if matches!(&e, WxcError::BfsNotAvailable)
&& request.schema_version.starts_with("0.4.0")
{
format!(
"Filesystem policy error: bfscfg.exe is not available on this Windows build. \
Your config uses schema version '{}', which requires BFS support. \
Either update your Windows build to one that includes bfscfg.exe, \
or update your config to schema version '0.6.0-alpha' or later \
(which uses the BaseContainer backend and does not require bfscfg.exe).",
request.schema_version
)
let msg = if matches!(&e, WxcError::BfsNotAvailable) {
"Filesystem policy error: bfscfg.exe is not available on this Windows \
build, so the AppContainer + BFS filesystem tier cannot enforce your \
policy. Use an OS build that includes bfscfg.exe, or run on a host that \
supports the BaseContainer backend (which does not require bfscfg.exe)."
.to_string()
} else {
e.to_string()
};
Expand Down
23 changes: 7 additions & 16 deletions src/backends/appcontainer/common/src/base_container_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1234,22 +1234,13 @@ impl SandboxBackend for BaseContainerRunner {
));
}
Self::is_base_container_api_present().map_err(|e| {
let hint = if !request.experimental_enabled {
format!(
"BaseContainer API unavailable: {e}\n\
Hint: Config schema version '{}' requires the BaseContainer backend, \
but this OS build does not support it. \
Use schema version '0.4.0-alpha' to fall back to AppContainer.",
request.schema_version
)
} else {
format!(
"BaseContainer API unavailable: {e}\n\
Hint: --experimental requested BaseContainer, but this OS build \
does not support it. Remove --experimental to use the AppContainer \
backend, or use an OS build with BaseContainer support."
)
};
let hint = format!(
"BaseContainer API unavailable: {e}\n\
Hint: this OS build does not support the BaseContainer backend. \
MXC selects BaseContainer automatically when the host supports it \
Comment thread
MGudgin marked this conversation as resolved.
and otherwise uses AppContainer; use an OS build with BaseContainer \
support if you require it."
);
ScriptResponse {
// Symbol absent: report BackendUnavailable, not a hard error.
failure_phase: FailurePhase::BackendUnavailable,
Expand Down
24 changes: 15 additions & 9 deletions src/backends/appcontainer/common/src/launch_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,25 +167,29 @@ fn diagnose_api_not_implemented() -> LaunchDiagnostic {
let message = if key_status.is_empty() {
"Experimental_CreateProcessInSandbox returned E_NOTIMPL. \
The BaseContainer feature is not enabled on this OS build. \
Ensure the required velocity keys are enabled, or use schema \
version '0.4.0-alpha' to fall back to the AppContainer backend."
It may be possible to enable it through the Windows experimental \
features settings, or run on a host that supports the BaseContainer \
backend (MXC falls back to AppContainer automatically on builds \
without it)."
.to_string()
} else {
let disabled: Vec<_> = key_status.iter().filter(|(_, enabled)| !enabled).collect();
if disabled.is_empty() {
"Experimental_CreateProcessInSandbox returned E_NOTIMPL. \
All known velocity keys appear enabled, but the feature is \
still gated. The OS build may require additional enablement. \
Use schema version '0.4.0-alpha' to fall back to the AppContainer backend."
The BaseContainer feature is not enabled on this OS build; it may \
require additional enablement. MXC falls back to AppContainer \
automatically on builds without BaseContainer support."
.to_string()
} else {
let disabled_list: Vec<String> =
disabled.iter().map(|(id, _)| id.to_string()).collect();
format!(
"Experimental_CreateProcessInSandbox returned E_NOTIMPL. \
The following velocity keys are not enabled: {}. \
Enable them and retry, or use schema version '0.4.0-alpha' \
to fall back to the AppContainer backend.",
The BaseContainer feature is not enabled on this OS build \
(disabled feature flags: {}). It may be possible to enable it \
through the Windows experimental features settings, or run on a \
host that supports the BaseContainer backend (MXC falls back to \
AppContainer automatically on builds without it).",
disabled_list.join(", ")
)
}
Expand Down Expand Up @@ -424,7 +428,9 @@ mod tests {
fn api_not_implemented_triggers_feature_diagnostic() {
let diag = diagnose_create_process_failure(ERROR_CALL_NOT_IMPLEMENTED.0, "pwsh.exe", &[]);
assert_eq!(diag.kind, "feature_not_enabled");
assert!(diag.message.contains("velocity"));
assert!(diag
.message
.contains("BaseContainer feature is not enabled"));
}

#[test]
Expand Down
42 changes: 22 additions & 20 deletions src/core/mxc-sdk/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,35 +148,37 @@ fn spawn_process_container(
request: &ExecutionRequest,
logger: &mut Logger,
) -> Result<Box<dyn SandboxProcess>, MxcError> {
use appcontainer_common::appcontainer_runner::AppContainerScriptRunner;
use wxc_common::config_parser::is_base_container_version;
use appcontainer_common::appcontainer_runner::{AppContainerScriptRunner, FilesystemMode};
use appcontainer_common::fallback_detector::is_base_container_usable;
use wxc_common::sandbox_process::{SandboxBackend, StdioMode};

// The AppContainer fast path vs the native BaseContainer (OS sandbox API):
// unlike the executor binaries' run-to-completion fallback, streaming does
// NOT route through `dispatch_with_fallback` — there is no AppContainer-BFS
// / AppContainer-DACL fallback for streaming.
// ProcessContainer resolves to a concrete backend purely by host
// capability: prefer the native BaseContainer (OS sandbox API) when usable,
// otherwise AppContainer. The schema version does not influence this choice.
//
// Why: `dispatch_with_fallback` yields a run-to-completion
// `Box<dyn ScriptRunner>` plus a `DaclManager` guard, neither of which
// fits the streaming handle (the DACL tier would require the returned
// `SandboxProcess` to own the guard so ACE restore outlives the child).
//
// Consequence (intentional, fail-closed): an experimental / newer-schema
// config on a host that lacks the native BaseContainer API fails here with
// a clear "BaseContainer API unavailable" error from
// `BaseContainerRunner`'s validation, whereas the binaries' fallback would
// drop to an AppContainer tier. Streaming therefore requires the native
// BaseContainer API for those configs.
let version_implies_base_container = is_base_container_version(&request.schema_version);
if request.experimental_enabled || version_implies_base_container {
// Unlike the executor binaries' run-to-completion path, streaming does NOT
// route through `dispatch_with_fallback` — that yields a run-to-completion
// `Box<dyn ScriptRunner>` plus a `DaclManager` guard, neither of which fits
// the streaming handle (the DACL tier would require the returned
// `SandboxProcess` to own the guard so ACE restore outlives the child). So
// streaming offers only two of the dispatcher's tiers: BaseContainer when
// the API is usable, otherwise AppContainer in BFS mode (equivalent to the
// dispatcher's Tier 2 AppContainer-BFS path, which still requires
// `bfscfg.exe`). The Tier 3 AppContainer-DACL fallback is not available on
// the streaming path. The BaseContainer-vs-AppContainer choice uses the same
// `is_base_container_usable()` probe as the dispatcher's Tier 1 selection, so
// the two paths agree on that decision.
if is_base_container_usable() {
let mut runner = appcontainer_common::base_container_runner::BaseContainerRunner::new();
return runner
.spawn(request, logger, StdioMode::Pipes)
.map_err(map_spawn_error);
}

let mut runner = AppContainerScriptRunner::new();
// Select BFS mode explicitly so this does not silently change if
// `AppContainerScriptRunner::new()`'s default filesystem mode is ever
// altered.
let mut runner = AppContainerScriptRunner::with_filesystem_mode(FilesystemMode::Bfs);
runner
.spawn(request, logger, StdioMode::Pipes)
.map_err(map_spawn_error)
Expand Down
3 changes: 2 additions & 1 deletion src/core/mxc-sdk/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,8 @@ fn proxy_to_wire(proxy: &ProxySpec) -> serde_json::Value {

/// Apply backend-specific fields, resolving the abstract `Process` intent the
/// same way the SDK does (Bubblewrap on Linux, Seatbelt on macOS,
/// BaseContainer on Windows).
/// ProcessContainer on Windows — which itself resolves to BaseContainer or
/// AppContainer at runtime by host capability).
fn apply_backend(config: &mut serde_json::Value, policy: &SandboxPolicy, container_id: &str) {
use serde_json::json;

Expand Down
50 changes: 16 additions & 34 deletions src/core/mxc-sdk/tests/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn seatbelt_request(command: &str, timeout_ms: u32) -> SandboxRequest {
}

/// A Windows ProcessContainer request exposing `C:\Windows\Temp` read-write.
/// The policy `version` selects the tier (>= 0.5 implies BaseContainer).
/// `version` is the schema version stamped on the policy, not a backend selector.
#[cfg(target_os = "windows")]
fn process_container_request(version: &str, command: &str, timeout_ms: u32) -> SandboxRequest {
let policy = SandboxPolicy {
Expand Down Expand Up @@ -240,9 +240,11 @@ fn seatbelt_defaults_cwd_to_allowed_path_without_getcwd_leak() {
// ---------------------------------------------------------------------------
// Windows ProcessContainer (AppContainer + BaseContainer) — integration tests.
//
// These exercise the capture and timeout paths that regressed as review items
// #1 (BaseContainer ran with an already-closed process handle) and #2
// (AppContainer timeout killed only the direct child, so it never fired).
// These exercise two capture/timeout regressions: the process handle being
// closed before the wait completed, and a finite timeout killing only the
// direct child so it never fired. ProcessContainer resolves to BaseContainer or
// AppContainer by host capability; these guards hold for whichever backend the
// host selects.
// They run a real sandbox, so they require an elevated, host-prepped Windows
// host (see docs/host-prep.md) and are therefore `#[ignore]`d — run them with
// `cargo test -p mxc-sdk -- --ignored` on such a host.
Expand All @@ -251,19 +253,18 @@ fn seatbelt_defaults_cwd_to_allowed_path_without_getcwd_leak() {
#[cfg(target_os = "windows")]
#[test]
#[ignore = "requires an elevated, host-prepped Windows host (see docs/host-prep.md)"]
fn base_container_captures_stdout() {
// Schema >= 0.5 implies the BaseContainer fallback. Regression guard for
// #1: a valid exit code and captured stdout prove the process handle was
// not closed out from under the wait.
fn process_container_captures_stdout() {
// Regression guard: a valid exit code and captured stdout prove the
// process handle was not closed out from under the wait.
let result = spawn_and_wait(process_container_request(
"0.7.0-alpha",
"cmd /c echo hello-base-container",
"cmd /c echo hello-process-container",
30000,
))
.expect("BaseContainer run should succeed");
.expect("ProcessContainer run should succeed");
assert_eq!(result.exit_code, 0, "stderr: {}", result.standard_err);
assert!(
result.standard_out.contains("hello-base-container"),
result.standard_out.contains("hello-process-container"),
"stdout should be captured, got: {:?}",
result.standard_out
);
Expand All @@ -272,36 +273,17 @@ fn base_container_captures_stdout() {
#[cfg(target_os = "windows")]
#[test]
#[ignore = "requires an elevated, host-prepped Windows host (see docs/host-prep.md)"]
fn appcontainer_captures_stdout() {
// Schema 0.4 keeps us on the AppContainer fast path (no BaseContainer).
let result = spawn_and_wait(process_container_request(
"0.4.0-alpha",
"cmd /c echo hello-appcontainer",
30000,
))
.expect("AppContainer run should succeed");
assert_eq!(result.exit_code, 0, "stderr: {}", result.standard_err);
assert!(
result.standard_out.contains("hello-appcontainer"),
"stdout should be captured, got: {:?}",
result.standard_out
);
}

#[cfg(target_os = "windows")]
#[test]
#[ignore = "requires an elevated, host-prepped Windows host (see docs/host-prep.md)"]
fn appcontainer_finite_timeout_fires() {
// Regression guard for #2: a finite timeout must fire even when the command
fn process_container_finite_timeout_fires() {
// Regression guard: a finite timeout must fire even when the command
// spawns a descendant that keeps the inherited stdout write-end open. If
// the timeout only killed the direct child, the capture reader would block
// forever and this test would hang past the bounded wall-clock below.
let result = spawn_and_wait(process_container_request(
"0.4.0-alpha",
"0.7.0-alpha",
"cmd /c start /b ping -n 60 127.0.0.1 >nul & ping -n 60 127.0.0.1 >nul",
2000,
))
.expect("AppContainer run should return a response");
.expect("ProcessContainer run should return a response");
assert!(result.timed_out, "a timed-out run must report a timeout");
// The bounded wait is enforced by the test harness; a hang here is the
// failure mode the regression guards against.
Expand Down
Loading
Loading