From e608b8b9fbf9dfb85196ff83a89dc080c9f19e95 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Tue, 21 Jul 2026 13:38:25 -0700 Subject: [PATCH 01/15] integ: inject mode-mapped learning-mode capability for captureDenials When processContainer.captureDenials is set, additively inject the learning-mode capability the runner's capture path requires onto policy.capabilities (block-and-log -> learningModeLogging, allow-and-log -> permissiveLearningMode), preserving the workload's real capabilities and de-duping if already present. Also add the learning_mode_windows crate as a Windows-only dependency of appcontainer_common so the runner can drive the capture lifecycle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/backends/appcontainer/common/Cargo.toml | 1 + src/core/wxc_common/src/config_parser.rs | 114 ++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/src/backends/appcontainer/common/Cargo.toml b/src/backends/appcontainer/common/Cargo.toml index f79e9fec4..7e3d3ccf6 100644 --- a/src/backends/appcontainer/common/Cargo.toml +++ b/src/backends/appcontainer/common/Cargo.toml @@ -26,6 +26,7 @@ flatbuffers = { workspace = true } sandbox_spec = { workspace = true } widestring = { workspace = true } winreg = { workspace = true } +learning_mode_windows = { workspace = true } [dev-dependencies] tempfile.workspace = true diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 0cbefd500..c008e4a7e 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -863,6 +863,23 @@ enforced; access denials are recorded for diagnostics.\n", Some(wire::CaptureDenialsMode::Allow) => CaptureDenialsMode::Allow, Some(wire::CaptureDenialsMode::Block) | None => CaptureDenialsMode::Block, }; + + // captureDenials drives the learning-mode ETL capture in the runner, + // which requires the corresponding learning-mode capability on the + // child token so the OS emits the access-check records the capture + // path collects. Inject it additively (preserving the workload's real + // capabilities) if it is not already present. `block-and-log` keeps + // deny-by-default via `learningModeLogging`; `allow-and-log` relaxes it + // via `permissiveLearningMode` (the runner flags that with a security + // warning). + let capture_capability = match mode { + CaptureDenialsMode::BlockAndLog => "learningModeLogging", + CaptureDenialsMode::AllowAndLog => "permissiveLearningMode", + }; + if !policy.capabilities.iter().any(|c| c == capture_capability) { + policy.capabilities.push(capture_capability.to_string()); + } + policy.capture_denials = Some(CaptureDenialsConfig { mode, output_path: cd.output_path, @@ -2433,6 +2450,103 @@ mod tests { assert_eq!(cd.mode, CaptureDenialsMode::Allow); } + #[test] + fn capture_denials_block_and_log_injects_learning_mode_logging_capability() { + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": {"captureDenials": {"mode": "block-and-log"}} + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"learningModeLogging".to_string()), + "block-and-log capture must additively inject learningModeLogging: {:?}", + req.policy.capabilities + ); + assert!( + !req.policy + .capabilities + .contains(&"permissiveLearningMode".to_string()), + "block-and-log must not inject permissiveLearningMode" + ); + } + + #[test] + fn capture_denials_allow_and_log_injects_permissive_learning_mode_capability() { + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": {"captureDenials": {"mode": "allow-and-log"}} + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"permissiveLearningMode".to_string()), + "allow-and-log capture must additively inject permissiveLearningMode: {:?}", + req.policy.capabilities + ); + } + + #[test] + fn capture_denials_default_injects_learning_mode_logging_capability() { + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": {"captureDenials": {}} + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"learningModeLogging".to_string()), + "default (block-and-log) capture must inject learningModeLogging: {:?}", + req.policy.capabilities + ); + } + + #[test] + fn capture_denials_capability_injection_is_additive_and_deduped() { + // The workload's own capabilities are preserved, and the injected + // learning-mode capability is not duplicated when already present. + let json = r#"{ + "process": {"commandLine": "print('test')"}, + "containment": "processcontainer", + "processContainer": { + "capabilities": ["internetClient", "permissiveLearningMode"], + "captureDenials": {"mode": "allow-and-log"} + } + }"#; + let encoded = base64_encode(json.as_bytes()); + let mut logger = test_logger(); + let req = load_request(&encoded, &mut logger, true).unwrap(); + assert!( + req.policy + .capabilities + .contains(&"internetClient".to_string()), + "the workload's own capabilities must be preserved" + ); + let permissive_count = req + .policy + .capabilities + .iter() + .filter(|c| *c == "permissiveLearningMode") + .count(); + assert_eq!( + permissive_count, 1, + "an already-present learning-mode capability must not be duplicated: {:?}", + req.policy.capabilities + ); + } + #[test] fn capture_denials_unknown_mode_rejected() { let json = r#"{ From c4ce0d21b482e189b11b870698016e45862ee493 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Tue, 21 Jul 2026 13:49:10 -0700 Subject: [PATCH 02/15] integ: drive learning-mode ETL capture from the BaseContainer runner When processContainer.captureDenials is configured, spawn_base now launches the child inside a process security environment with a learning-mode trace already started against it, instead of the one-shot CreateProcessInSandbox: - Resolve the security-environment + learning-mode trace APIs from processmodel.dll; if absent, degrade cleanly with a BackendUnavailable error (feature-enabled/GE_CURRENT builds only). - CaptureSession::begin creates the env + starts the trace before launch; the child is launched via CreateProcessAsUserInsideSecurityEnvironment, returning a real PROCESS_INFORMATION owned/waited like the normal path. - Thread the session + resolved ETL output path (caller-specified, else a managed per-run temp file) through BaseChild into the sandbox process; seal the trace in run_teardown after the child exits and surface the ETL file path on stderr. - On any early return the CaptureSession Drop discards the trace and closes the environment (no broker leak). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../common/src/base_container_runner.rs | 275 ++++++++++++++---- 1 file changed, 225 insertions(+), 50 deletions(-) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index e80c5fe52..a72ec3c43 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -12,8 +12,13 @@ use std::ffi::c_void; use std::fmt::Write; use std::io::IsTerminal; +use std::path::PathBuf; use std::ptr; +use learning_mode_windows::{ + CaptureSession, LearningModeApi, SecurityEnvironmentApi, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, +}; + use windows::Win32::Foundation::{ CloseHandle, GetLastError, SetHandleInformation, ERROR_CALL_NOT_IMPLEMENTED, E_NOTIMPL, HANDLE, HANDLE_FLAG_INHERIT, WAIT_OBJECT_0, WAIT_TIMEOUT, @@ -661,6 +666,62 @@ impl BaseContainerRunner { Self::log_sandbox_spec(&spec_bytes, logger); + // Resolve the learning-mode capture APIs when captureDenials is + // configured. The mode-mapped learning-mode capability was already + // injected into policy.capabilities (and hence the spec) by the config + // parser; here we obtain the OS handles that drive the ETL trace. Both + // exports live in the same processmodel.dll the BaseContainer launch API + // is loaded from, and only exist on feature-enabled (GE_CURRENT) builds, + // so a failed load is a clean, actionable "backend unavailable" degrade. + let capture_denials = request.policy.capture_denials.clone(); + let mut capture_apis: Option<(SecurityEnvironmentApi, LearningModeApi)> = None; + if capture_denials.is_some() { + let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: captureDenials"); + match (SecurityEnvironmentApi::load(), LearningModeApi::load()) { + (Ok(se), Ok(lm)) => { + let _ = writeln!( + logger, + "captureDenials: learning-mode trace API available (processmodel.dll)" + ); + capture_apis = Some((se, lm)); + } + (se, lm) => { + let detail = match (&se, &lm) { + (Err(e), _) => format!("security-environment API: {e}"), + (_, Err(e)) => format!("learning-mode trace API: {e}"), + _ => "unknown".to_string(), + }; + let msg = format!( + "captureDenials was requested but the learning-mode trace API is not \ + available on this OS build ({detail}). This feature requires a \ + feature-enabled Windows build that exports the learning-mode trace \ + APIs from processmodel.dll." + ); + let _ = writeln!(logger, "Error: {msg}"); + return Err(ScriptResponse { + exit_code: -1, + error_message: msg.clone(), + standard_err: msg, + failure_phase: FailurePhase::BackendUnavailable, + ..Default::default() + }); + } + } + } + + // Resolve the ETL output path: caller-specified when provided, else a + // managed per-run temp file the runner names (parsed / cleaned up by the + // consume + analyze stages). + let capture_output_path: Option = capture_denials.as_ref().map(|cd| { + cd.output_path + .clone() + .map(PathBuf::from) + .unwrap_or_else(|| { + std::env::temp_dir() + .join(format!("mxc_capture_denials_{}.etl", std::process::id())) + }) + }); + let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: Load API"); // 2. Dynamically load the API from processmodel.dll. @@ -961,71 +1022,148 @@ impl BaseContainerRunner { let mut current_creation_flags = creation_flags; let mut retries_remaining: u32 = 1; - // The loop yields (api_return_code, last_win32_error_on_failure). - let (success, last_error) = loop { - pi = unsafe { std::mem::zeroed() }; + // When captureDenials is active, launch inside a process security + // environment that already has a learning-mode trace started against it, + // instead of the one-shot CreateProcessInSandbox. `begin` creates the + // environment and starts the trace *before* the child launches (so no + // early denials are missed); the child is then launched into that + // environment via CreateProcessAsUserInsideSecurityEnvironment, which + // returns a real PROCESS_INFORMATION we own and wait on exactly like the + // normal path. On any early return below, `capture_session` drops and its + // Drop discards the trace and closes the environment (no broker leak). + let mut capture_session: Option = None; + let mut capture_se_api: Option = None; + if let Some((se_api, lm_api)) = capture_apis { + match CaptureSession::begin( + se_api, + lm_api, + &spec_bytes, + PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, + ) { + Ok(session) => { + let _ = writeln!( + logger, + "captureDenials: security environment created and trace started" + ); + // `SecurityEnvironmentApi` is Copy, so we keep our own copy to + // call `launch_fn` at the launch site below. + capture_se_api = Some(se_api); + capture_session = Some(session); + } + Err(e) => { + let msg = format!("captureDenials: failed to start learning-mode capture: {e}"); + let _ = writeln!(logger, "Error: {msg}"); + return Err(ScriptResponse { + exit_code: -1, + error_message: msg.clone(), + standard_err: msg, + failure_phase: FailurePhase::LaunchFailed, + ..Default::default() + }); + } + } + } - let result = unsafe { - create_process_in_sandbox( - ptr::null(), // applicationName (resolved from commandLine) - cmd_wide.as_mut_ptr(), // commandLine - ptr::null(), // processAttributes (must be NULL) - ptr::null(), // threadAttributes (must be NULL) - // inheritHandles: must be FALSE per the OS sandbox API contract. - // Unlike regular CreateProcess, CreateProcessInSandbox treats the - // explicit STDIO handles in STARTUPINFO (hStdInput/hStdOutput/hStdError) - // as inheritable when STARTF_USESTDHANDLES is set, but does not support - // general handle inheritance. - i32::from(false), // inheritHandles + // The launch yields (api_return_code, last_win32_error_on_failure). + let (success, last_error) = if let (Some(session), Some(se_api)) = + (capture_session.as_ref(), capture_se_api.as_ref()) + { + // Single-attempt in-environment launch. The learning-mode security + // environment only exists on builds that support the `environment` + // parameter, so the CreateProcessInSandbox env-not-supported retry + // does not apply here. + pi = unsafe { std::mem::zeroed() }; + let launch = se_api.launch_fn(); + // SAFETY: `launch` was resolved from processmodel.dll and matches the + // declared C signature. `cmd_wide` is a mutable, null-terminated + // UTF-16 buffer; `si`/`pi` are valid; `session.environment()` is the + // live handle from the just-begun session. When `current_env_ptr` is + // non-null, `current_creation_flags` already carries + // CREATE_UNICODE_ENVIRONMENT. + let ok = unsafe { + launch( + HANDLE(ptr::null_mut()), // userToken: caller context + ptr::null(), // applicationName (from command line) + cmd_wide.as_mut_ptr(), // commandLine current_creation_flags, // creationFlags current_env_ptr, // environment cwd_ptr, // currentDirectory &si, // startupInfo - identity_wide.as_ptr(), // identity - spec_bytes.as_ptr(), // sandboxSpecification - spec_bytes.len() as u32, // sandboxSpecificationSize + session.environment(), // process security environment &mut pi, // processInformation ) }; - - if result != 0 { - break (result, None); + if ok != 0 { + (ok, None) + } else { + (0, Some(unsafe { GetLastError() })) } + } else { + loop { + pi = unsafe { std::mem::zeroed() }; + + let result = unsafe { + create_process_in_sandbox( + ptr::null(), // applicationName (resolved from commandLine) + cmd_wide.as_mut_ptr(), // commandLine + ptr::null(), // processAttributes (must be NULL) + ptr::null(), // threadAttributes (must be NULL) + // inheritHandles: must be FALSE per the OS sandbox API contract. + // Unlike regular CreateProcess, CreateProcessInSandbox treats the + // explicit STDIO handles in STARTUPINFO (hStdInput/hStdOutput/hStdError) + // as inheritable when STARTF_USESTDHANDLES is set, but does not support + // general handle inheritance. + i32::from(false), // inheritHandles + current_creation_flags, // creationFlags + current_env_ptr, // environment + cwd_ptr, // currentDirectory + &si, // startupInfo + identity_wide.as_ptr(), // identity + spec_bytes.as_ptr(), // sandboxSpecification + spec_bytes.len() as u32, // sandboxSpecificationSize + &mut pi, // processInformation + ) + }; - // Call failed -- capture the error before any handle cleanup. - let err = unsafe { GetLastError() }; - - if retries_remaining > 0 - && is_environment_not_supported(err.0, !current_env_ptr.is_null()) - { - retries_remaining -= 1; + if result != 0 { + break (result, None); + } - // Clean up handles from the failed attempt. - unsafe { - if !pi.hProcess.is_invalid() { - let _ = CloseHandle(pi.hProcess); - } - if !pi.hThread.is_invalid() { - let _ = CloseHandle(pi.hThread); + // Call failed -- capture the error before any handle cleanup. + let err = unsafe { GetLastError() }; + + if retries_remaining > 0 + && is_environment_not_supported(err.0, !current_env_ptr.is_null()) + { + retries_remaining -= 1; + + // Clean up handles from the failed attempt. + unsafe { + if !pi.hProcess.is_invalid() { + let _ = CloseHandle(pi.hProcess); + } + if !pi.hThread.is_invalid() { + let _ = CloseHandle(pi.hThread); + } } - } - let diag = diagnose_environment_not_supported(); - let _ = writeln!( - logger, - "{EMOJI_WARNING} Launch diagnostic [{}]: {}", - diag.kind, diag.message - ); + let diag = diagnose_environment_not_supported(); + let _ = writeln!( + logger, + "{EMOJI_WARNING} Launch diagnostic [{}]: {}", + diag.kind, diag.message + ); - // Retry without the environment block, but keep the child - // suspended (resumed after job assignment). - current_env_ptr = ptr::null(); - current_creation_flags = CREATE_SUSPENDED.0 | no_window_flag; - continue; - } + // Retry without the environment block, but keep the child + // suspended (resumed after job assignment). + current_env_ptr = ptr::null(); + current_creation_flags = CREATE_SUSPENDED.0 | no_window_flag; + continue; + } - // Non-retryable failure. - break (result, Some(err)); + // Non-retryable failure. + break (result, Some(err)); + } }; if success == 0 { @@ -1196,6 +1334,8 @@ impl BaseContainerRunner { identity, sid_string, proxy_coordinator: std::mem::take(&mut self.proxy_coordinator), + capture_session, + capture_output_path, }) } } @@ -1222,6 +1362,13 @@ struct BaseChild { identity: String, sid_string: String, proxy_coordinator: ProxyCoordinator, + /// Live learning-mode capture session (`Some` only when `captureDenials` + /// is configured and the OS API is available). Sealed in `run_teardown` + /// after the child exits. + capture_session: Option, + /// Resolved ETL output path for the capture session (caller-specified or a + /// managed per-run temp file). `Some` iff `capture_session` is `Some`. + capture_output_path: Option, } impl SandboxBackend for BaseContainerRunner { @@ -1308,6 +1455,11 @@ struct BaseContainerSandboxProcess { sid_string: String, proxy_coordinator: ProxyCoordinator, teardown_done: bool, + /// Live learning-mode capture session, moved from the `BaseChild`. Sealed + /// in `run_teardown` once the child has exited and been reaped. + capture_session: Option, + /// Resolved ETL output path for the capture session. + capture_output_path: Option, } // SAFETY: the fields are Windows HANDLEs / handle-owning managers and owned @@ -1342,6 +1494,8 @@ impl BaseContainerSandboxProcess { sid_string: std::mem::take(&mut child.sid_string), proxy_coordinator: std::mem::take(&mut child.proxy_coordinator), teardown_done: false, + capture_session: child.capture_session.take(), + capture_output_path: child.capture_output_path.take(), } } @@ -1351,6 +1505,27 @@ impl BaseContainerSandboxProcess { } self.teardown_done = true; let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); + + // Seal the learning-mode ETL trace now that the child has exited and + // been reaped (both `wait` and `Drop` kill + reap before calling this). + // `finish` stops the trace to the resolved output path, then closes the + // security environment. The output-file path is surfaced on stderr so a + // caller can locate the denials (the full NDJSON contract is finalized + // in the output/consume stage). + if let Some(session) = self.capture_session.take() { + let output_path = self.capture_output_path.clone(); + match session.finish(output_path.as_deref()) { + Ok(()) => { + if let Some(path) = &output_path { + eprintln!("captureDenials: denials ETL written to {}", path.display()); + } + } + Err(e) => { + eprintln!("captureDenials: failed to seal denials ETL: {e}"); + } + } + } + if self.destroy_on_exit { run_sandbox_cleanup( &self.identity, From f947bf06619172b9b3a0eafb71d16d9fde8ff9d0 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Tue, 21 Jul 2026 14:08:34 -0700 Subject: [PATCH 03/15] integ: lock learning_mode_windows dep in Cargo.lock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Cargo.lock b/src/Cargo.lock index 831cbfbac..82b872c1a 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -88,6 +88,7 @@ version = "0.7.0" dependencies = [ "flatbuffers", "getrandom 0.2.17", + "learning_mode_windows", "sandbox_spec", "serde", "serde_json", From 99f91ab4e9588eb8072825056e66d47e6d3c5ace Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Tue, 28 Jul 2026 13:26:52 -0700 Subject: [PATCH 04/15] Reconcile learning mode integration contracts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../common/src/appcontainer_runner.rs | 27 ++++++- .../common/src/base_container_runner.rs | 5 +- src/core/wxc_common/src/config_parser.rs | 73 +++++++++++-------- 3 files changed, 71 insertions(+), 34 deletions(-) diff --git a/src/backends/appcontainer/common/src/appcontainer_runner.rs b/src/backends/appcontainer/common/src/appcontainer_runner.rs index 6149b301a..02ced9497 100644 --- a/src/backends/appcontainer/common/src/appcontainer_runner.rs +++ b/src/backends/appcontainer/common/src/appcontainer_runner.rs @@ -34,7 +34,9 @@ use crate::job_object::UiJobObject; use crate::process_mitigation; use wxc_common::error::WxcError; use wxc_common::logger::Logger; -use wxc_common::models::{ExecutionRequest, NetworkEnforcementMode, NetworkPolicy, ScriptResponse}; +use wxc_common::models::{ + ExecutionRequest, FailurePhase, NetworkEnforcementMode, NetworkPolicy, ScriptResponse, +}; use wxc_common::process_util::{ create_std_pipes, InterruptiblePipeReader, OwnedHandle, PipeReadCanceller, PipeWriter, SendOwnedHandle, SidAndAttributes, @@ -1303,6 +1305,14 @@ impl AppContainerScriptRunner { impl SandboxBackend for AppContainerScriptRunner { fn validate(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { + if request.policy.capture_denials.is_some() { + let msg = "captureDenials requires the BaseContainer learning-mode APIs and is not \ + supported by the AppContainer fallback tier"; + return Err(ScriptResponse { + failure_phase: FailurePhase::BackendUnavailable, + ..ScriptResponse::error(msg) + }); + } if !request.policy.denied_paths.is_empty() && self.filesystem_mode != FilesystemMode::Dacl { return Err(ScriptResponse::error( wxc_common::error::DENIED_PATHS_NOT_SUPPORTED_MSG, @@ -1824,7 +1834,7 @@ mod tests { // ---- validate_runner: unsupported policy fields surface as errors. ---- use super::{AppContainerScriptRunner, FilesystemMode}; - use wxc_common::models::ExecutionRequest; + use wxc_common::models::{ExecutionRequest, FailurePhase}; use wxc_common::sandbox_process::SandboxBackend; #[test] @@ -1885,4 +1895,17 @@ mod tests { let request = ExecutionRequest::default(); assert!(runner.validate(&request).is_ok()); } + + #[test] + fn validate_runner_rejects_capture_denials_on_appcontainer_fallback() { + let runner = AppContainerScriptRunner::new(); + let mut request = ExecutionRequest::default(); + request.policy.capture_denials = Some(Default::default()); + + let error = runner + .validate(&request) + .expect_err("AppContainer fallback must not silently ignore captureDenials"); + assert_eq!(error.failure_phase, FailurePhase::BackendUnavailable); + assert!(error.error_message.contains("requires the BaseContainer")); + } } diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index a72ec3c43..4495efa57 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -671,8 +671,9 @@ impl BaseContainerRunner { // injected into policy.capabilities (and hence the spec) by the config // parser; here we obtain the OS handles that drive the ETL trace. Both // exports live in the same processmodel.dll the BaseContainer launch API - // is loaded from, and only exist on feature-enabled (GE_CURRENT) builds, - // so a failed load is a clean, actionable "backend unavailable" degrade. + // is loaded from, and only exist on supported feature-enabled Windows + // builds, so a failed load is a clean, actionable "backend unavailable" + // degrade. let capture_denials = request.policy.capture_denials.clone(); let mut capture_apis: Option<(SecurityEnvironmentApi, LearningModeApi)> = None; if capture_denials.is_some() { diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index c008e4a7e..efd8f9afc 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -868,15 +868,28 @@ enforced; access denials are recorded for diagnostics.\n", // which requires the corresponding learning-mode capability on the // child token so the OS emits the access-check records the capture // path collects. Inject it additively (preserving the workload's real - // capabilities) if it is not already present. `block-and-log` keeps - // deny-by-default via `learningModeLogging`; `allow-and-log` relaxes it - // via `permissiveLearningMode` (the runner flags that with a security - // warning). + // capabilities). `block` keeps deny-by-default via + // `learningModeLogging`; `allow` replaces deny-and-record with + // `permissiveLearningMode` (the runner surfaces the security warning). let capture_capability = match mode { - CaptureDenialsMode::BlockAndLog => "learningModeLogging", - CaptureDenialsMode::AllowAndLog => "permissiveLearningMode", + CaptureDenialsMode::Block => { + policy.capabilities.retain(|capability| { + !capability.eq_ignore_ascii_case("permissiveLearningMode") + }); + "learningModeLogging" + } + CaptureDenialsMode::Allow => { + policy.capabilities.retain(|capability| { + !capability.eq_ignore_ascii_case("learningModeLogging") + }); + "permissiveLearningMode" + } }; - if !policy.capabilities.iter().any(|c| c == capture_capability) { + if !policy + .capabilities + .iter() + .any(|capability| capability.eq_ignore_ascii_case(capture_capability)) + { policy.capabilities.push(capture_capability.to_string()); } @@ -2451,11 +2464,11 @@ mod tests { } #[test] - fn capture_denials_block_and_log_injects_learning_mode_logging_capability() { + fn capture_denials_block_injects_learning_mode_logging_capability() { let json = r#"{ "process": {"commandLine": "print('test')"}, "containment": "processcontainer", - "processContainer": {"captureDenials": {"mode": "block-and-log"}} + "processContainer": {"captureDenials": {"mode": "block"}} }"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); @@ -2464,23 +2477,23 @@ mod tests { req.policy .capabilities .contains(&"learningModeLogging".to_string()), - "block-and-log capture must additively inject learningModeLogging: {:?}", + "block capture must additively inject learningModeLogging: {:?}", req.policy.capabilities ); assert!( !req.policy .capabilities .contains(&"permissiveLearningMode".to_string()), - "block-and-log must not inject permissiveLearningMode" + "block must not inject permissiveLearningMode" ); } #[test] - fn capture_denials_allow_and_log_injects_permissive_learning_mode_capability() { + fn capture_denials_allow_injects_permissive_learning_mode_capability() { let json = r#"{ "process": {"commandLine": "print('test')"}, "containment": "processcontainer", - "processContainer": {"captureDenials": {"mode": "allow-and-log"}} + "processContainer": {"captureDenials": {"mode": "allow"}} }"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); @@ -2489,7 +2502,7 @@ mod tests { req.policy .capabilities .contains(&"permissiveLearningMode".to_string()), - "allow-and-log capture must additively inject permissiveLearningMode: {:?}", + "allow capture must inject permissiveLearningMode: {:?}", req.policy.capabilities ); } @@ -2508,21 +2521,20 @@ mod tests { req.policy .capabilities .contains(&"learningModeLogging".to_string()), - "default (block-and-log) capture must inject learningModeLogging: {:?}", + "default (block) capture must inject learningModeLogging: {:?}", req.policy.capabilities ); } #[test] - fn capture_denials_capability_injection_is_additive_and_deduped() { - // The workload's own capabilities are preserved, and the injected - // learning-mode capability is not duplicated when already present. + fn capture_denials_allow_overrides_learning_mode_boolean() { let json = r#"{ "process": {"commandLine": "print('test')"}, "containment": "processcontainer", "processContainer": { - "capabilities": ["internetClient", "permissiveLearningMode"], - "captureDenials": {"mode": "allow-and-log"} + "learningMode": true, + "capabilities": ["internetClient"], + "captureDenials": {"mode": "allow"} } }"#; let encoded = base64_encode(json.as_bytes()); @@ -2534,16 +2546,17 @@ mod tests { .contains(&"internetClient".to_string()), "the workload's own capabilities must be preserved" ); - let permissive_count = req - .policy - .capabilities - .iter() - .filter(|c| *c == "permissiveLearningMode") - .count(); - assert_eq!( - permissive_count, 1, - "an already-present learning-mode capability must not be duplicated: {:?}", - req.policy.capabilities + assert!( + req.policy + .capabilities + .contains(&"permissiveLearningMode".to_string()), + "allow capture must inject permissiveLearningMode" + ); + assert!( + !req.policy + .capabilities + .contains(&"learningModeLogging".to_string()), + "allow capture must remove deny-and-record mode" ); } From b405d10ecd02fd52c5c47066ddb51d2299becb1d Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Tue, 28 Jul 2026 14:38:08 -0700 Subject: [PATCH 05/15] Harden denial capture teardown and paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../common/src/base_container_runner.rs | 101 ++++++++++++++---- src/core/wxc_common/src/config_parser.rs | 8 +- 2 files changed, 85 insertions(+), 24 deletions(-) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 4495efa57..2faf14ec7 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -713,15 +713,16 @@ impl BaseContainerRunner { // Resolve the ETL output path: caller-specified when provided, else a // managed per-run temp file the runner names (parsed / cleaned up by the // consume + analyze stages). - let capture_output_path: Option = capture_denials.as_ref().map(|cd| { - cd.output_path - .clone() - .map(PathBuf::from) - .unwrap_or_else(|| { - std::env::temp_dir() - .join(format!("mxc_capture_denials_{}.etl", std::process::id())) - }) - }); + let capture_output_path: Option = capture_denials + .as_ref() + .map(|cd| { + cd.output_path + .clone() + .map(PathBuf::from) + .map(Ok) + .unwrap_or_else(managed_capture_output_path) + }) + .transpose()?; let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: Load API"); @@ -1500,9 +1501,9 @@ impl BaseContainerSandboxProcess { } } - fn run_teardown(&mut self) { + fn run_teardown(&mut self) -> std::io::Result<()> { if self.teardown_done { - return; + return Ok(()); } self.teardown_done = true; let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); @@ -1513,19 +1514,22 @@ impl BaseContainerSandboxProcess { // security environment. The output-file path is surfaced on stderr so a // caller can locate the denials (the full NDJSON contract is finalized // in the output/consume stage). - if let Some(session) = self.capture_session.take() { - let output_path = self.capture_output_path.clone(); + let capture_result = if let Some(session) = self.capture_session.take() { + let output_path = self.capture_output_path.take(); match session.finish(output_path.as_deref()) { Ok(()) => { if let Some(path) = &output_path { eprintln!("captureDenials: denials ETL written to {}", path.display()); } + Ok(()) } - Err(e) => { - eprintln!("captureDenials: failed to seal denials ETL: {e}"); - } + Err(error) => Err(std::io::Error::other(format!( + "captureDenials failed to seal the denial trace: {error}" + ))), } - } + } else { + Ok(()) + }; if self.destroy_on_exit { run_sandbox_cleanup( @@ -1537,6 +1541,7 @@ impl BaseContainerSandboxProcess { sandbox_tracking::unregister_ctrl_c_cleanup(); } self.proxy_coordinator.stop(&mut logger); + capture_result } } @@ -1632,8 +1637,8 @@ impl SandboxProcess for BaseContainerSandboxProcess { } cancel_and_join_discard(stdout_thread, &self.stdout_canceller); cancel_and_join_discard(stderr_thread, &self.stderr_canceller); - self.run_teardown(); - result + let teardown_result = self.run_teardown(); + combine_process_and_teardown_results(result, teardown_result) } } @@ -1646,7 +1651,43 @@ impl Drop for BaseContainerSandboxProcess { unsafe { let _ = WaitForSingleObject(self.process.get(), u32::MAX); } - self.run_teardown(); + if let Err(error) = self.run_teardown() { + eprintln!("captureDenials teardown failed during drop: {error}"); + } + } +} + +fn managed_capture_output_path() -> Result { + let mut nonce = [0u8; 16]; + getrandom::getrandom(&mut nonce).map_err(|error| { + ScriptResponse::error(&format!( + "captureDenials could not generate a unique temporary output path: {error}" + )) + })?; + let suffix = nonce + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + Ok(std::env::temp_dir().join(format!( + "mxc_capture_denials_{}_{suffix}.etl", + std::process::id() + ))) +} + +fn combine_process_and_teardown_results( + process_result: std::io::Result, + teardown_result: std::io::Result<()>, +) -> std::io::Result { + match (process_result, teardown_result) { + (Ok(exit_code), Ok(())) => Ok(exit_code), + (Ok(_), Err(teardown_error)) => Err(teardown_error), + (Err(wait_error), Ok(())) => Err(wait_error), + (Err(wait_error), Err(teardown_error)) => { + eprintln!( + "captureDenials teardown also failed after process wait failure: {teardown_error}" + ); + Err(wait_error) + } } } @@ -1686,6 +1727,26 @@ mod tests { to_job_object_uilimit_mask(&r) as u64 } + #[test] + fn managed_capture_paths_are_unique_per_run() { + let first = managed_capture_output_path().expect("first path"); + let second = managed_capture_output_path().expect("second path"); + + assert_ne!(first, second); + assert_eq!(first.parent(), Some(std::env::temp_dir().as_path())); + assert_eq!(second.parent(), Some(std::env::temp_dir().as_path())); + assert_eq!(first.extension().and_then(|ext| ext.to_str()), Some("etl")); + } + + #[test] + fn successful_process_reports_capture_teardown_failure() { + let error = + combine_process_and_teardown_results(Ok(0), Err(std::io::Error::other("seal failed"))) + .expect_err("capture failure must override successful process exit"); + + assert!(error.to_string().contains("seal failed")); + } + #[test] fn is_api_not_implemented_classifies_disabled_feature() { assert!(is_api_not_implemented(ERROR_CALL_NOT_IMPLEMENTED.0)); diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index efd8f9afc..22c334fa6 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -815,10 +815,6 @@ fn convert_wire_config( // available in every build. if ac.learning_mode.unwrap_or(false) { policy.capabilities.push("learningModeLogging".to_string()); - logger.log( - "NOTE: 'learningModeLogging' enabled - AppContainer restrictions remain \ -enforced; access denials are recorded for diagnostics.\n", - ); } // Learning-mode capability names are reserved for the dedicated entry @@ -2558,6 +2554,10 @@ mod tests { .contains(&"learningModeLogging".to_string()), "allow capture must remove deny-and-record mode" ); + assert!( + !logger.get_buffer().contains("restrictions remain enforced"), + "parser must not log the superseded deny-and-record mode" + ); } #[test] From 9a93c1313c8a7d4b89b8a9d3112cddce4283d71f Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 09:37:21 -0700 Subject: [PATCH 06/15] Reject audit with captureDenials Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- README.md | 2 +- docs/learning-mode/capabilities.md | 2 ++ src/core/wxc/src/main.rs | 55 ++++++++++++++++++++++++------ 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index f14aa0efe..d80f33e4a 100644 --- a/README.md +++ b/README.md @@ -225,7 +225,7 @@ See [docs/diagnostics.md](docs/diagnostics.md) for full diagnostics reference. wxc-exec.exe --audit policy.json ``` -> **Warning:** `--audit` injects `permissiveLearningMode` — AppContainer restrictions are **not** enforced for the duration of the run. Use only for policy authoring. `learningModeLogging` and `permissiveLearningMode` are reserved internal capability names and are rejected in `processContainer.capabilities`; use `processContainer.learningMode: true` for deny-and-record mode or `--audit` for permissive mode. See [docs/learning-mode/capabilities.md](docs/learning-mode/capabilities.md) for the three learning-mode flows. +> **Warning:** `--audit` injects `permissiveLearningMode` — AppContainer restrictions are **not** enforced for the duration of the run. Use only for policy authoring. It cannot be combined with `processContainer.captureDenials`; use `captureDenials.mode: "allow"` for permissive application-driven capture. `learningModeLogging` and `permissiveLearningMode` are reserved internal capability names and are rejected in `processContainer.capabilities`. See [docs/learning-mode/capabilities.md](docs/learning-mode/capabilities.md) for the three learning-mode flows. ## Telemetry (Experimental) diff --git a/docs/learning-mode/capabilities.md b/docs/learning-mode/capabilities.md index c2f104e6a..004eeb85c 100644 --- a/docs/learning-mode/capabilities.md +++ b/docs/learning-mode/capabilities.md @@ -87,6 +87,8 @@ stays enforced: 1. **Developer inner-loop (`--audit`).** A developer runs `wxc-exec --audit` with ProcessContainer containment to discover the capabilities and paths their process needs. `--audit` is rejected for every other Windows backend. + It is also mutually exclusive with `captureDenials`; use + `captureDenials.mode: "allow"` for permissive application-driven capture. It triggers UAC, injects `permissiveLearningMode`, and drives a WPR/ETW permissive-learning-mode trace for the run. This is typically a static config the developer iterates on locally. diff --git a/src/core/wxc/src/main.rs b/src/core/wxc/src/main.rs index aa9529df8..21b9dabbd 100644 --- a/src/core/wxc/src/main.rs +++ b/src/core/wxc/src/main.rs @@ -200,16 +200,22 @@ fn apply_permissive_learning_mode(capabilities: &mut Vec) -> bool { } } -fn validate_audit_containment(containment: &ContainmentBackend) -> Result<(), String> { - if *containment == ContainmentBackend::ProcessContainer { - Ok(()) - } else { - Err(format!( +fn validate_audit_request(request: &ExecutionRequest) -> Result<(), String> { + if request.containment != ContainmentBackend::ProcessContainer { + return Err(format!( "--audit is only supported with the Windows ProcessContainer backend; \ resolved containment is '{}'", - containment.wire_name() - )) + request.containment.wire_name() + )); + } + if request.policy.capture_denials.is_some() { + return Err( + "--audit cannot be combined with processContainer.captureDenials; \ + use captureDenials.mode: \"allow\" for permissive capture" + .to_string(), + ); } + Ok(()) } fn command_override_from_cli( @@ -1074,7 +1080,7 @@ fn main() { // because Windows derives capability SIDs case-insensitively. #[cfg(target_os = "windows")] if cli.audit { - if let Err(message) = validate_audit_containment(&request.containment) { + if let Err(message) = validate_audit_request(&request) { eprintln!("Error: {message}"); telemetry::emit_early_exit( telemetry_active, @@ -1434,7 +1440,11 @@ mod tests { #[test] fn audit_mode_only_accepts_process_container() { - assert!(validate_audit_containment(&ContainmentBackend::ProcessContainer).is_ok()); + let mut request = ExecutionRequest { + containment: ContainmentBackend::ProcessContainer, + ..Default::default() + }; + assert!(validate_audit_request(&request).is_ok()); for containment in [ ContainmentBackend::WindowsSandbox, @@ -1442,9 +1452,32 @@ mod tests { ContainmentBackend::IsolationSession, ContainmentBackend::MicroVm, ] { - let error = validate_audit_containment(&containment) + let containment_name = containment.wire_name(); + request.containment = containment; + let error = validate_audit_request(&request) .expect_err("non-ProcessContainer backend must reject --audit"); - assert!(error.contains(containment.wire_name())); + assert!(error.contains(containment_name)); + } + } + + #[test] + fn audit_mode_rejects_both_capture_denials_modes() { + use wxc_common::models::{CaptureDenialsConfig, CaptureDenialsMode}; + + for mode in [CaptureDenialsMode::Block, CaptureDenialsMode::Allow] { + let mut request = ExecutionRequest { + containment: ContainmentBackend::ProcessContainer, + ..Default::default() + }; + request.policy.capture_denials = Some(CaptureDenialsConfig { + mode, + output_path: None, + }); + + let error = validate_audit_request(&request) + .expect_err("--audit and captureDenials must be mutually exclusive"); + assert!(error.contains("cannot be combined")); + assert!(error.contains(r#"mode: "allow""#)); } } From 7256c5acc0b1f1a420a9ddffedac58c4abf6e79d Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 11:35:28 -0700 Subject: [PATCH 07/15] Finalize capture on terminal try-wait Seal captureDenials before polling waits report process exit, terminate and reap descendants through a shared path, and cache teardown failures so repeated waits cannot hide them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../common/src/base_container_runner.rs | 59 +++++++++++-------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 2faf14ec7..a63c33116 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -1456,7 +1456,9 @@ struct BaseContainerSandboxProcess { identity: String, sid_string: String, proxy_coordinator: ProxyCoordinator, - teardown_done: bool, + /// Cached teardown outcome so repeated terminal waits cannot hide a + /// capture failure after the session has been consumed. + teardown_result: Option>, /// Live learning-mode capture session, moved from the `BaseChild`. Sealed /// in `run_teardown` once the child has exited and been reaped. capture_session: Option, @@ -1495,17 +1497,16 @@ impl BaseContainerSandboxProcess { identity: std::mem::take(&mut child.identity), sid_string: std::mem::take(&mut child.sid_string), proxy_coordinator: std::mem::take(&mut child.proxy_coordinator), - teardown_done: false, + teardown_result: None, capture_session: child.capture_session.take(), capture_output_path: child.capture_output_path.take(), } } fn run_teardown(&mut self) -> std::io::Result<()> { - if self.teardown_done { - return Ok(()); + if let Some(result) = &self.teardown_result { + return result.clone().map_err(std::io::Error::other); } - self.teardown_done = true; let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); // Seal the learning-mode ETL trace now that the child has exited and @@ -1541,7 +1542,27 @@ impl BaseContainerSandboxProcess { sandbox_tracking::unregister_ctrl_c_cleanup(); } self.proxy_coordinator.stop(&mut logger); - capture_result + let result = capture_result.map_err(|error| error.to_string()); + self.teardown_result = Some(result.clone()); + result.map_err(std::io::Error::other) + } + + fn kill_process_tree(&mut self) -> std::io::Result<()> { + if let Some(job) = &self.job { + job.terminate(u32::MAX); + } else { + unsafe { + let _ = TerminateProcess(self.process.get(), u32::MAX); + } + } + Ok(()) + } + + fn terminate_and_reap(&mut self) { + let _ = self.kill_process_tree(); + unsafe { + let _ = WaitForSingleObject(self.process.get(), u32::MAX); + } } } @@ -1573,7 +1594,12 @@ impl SandboxProcess for BaseContainerSandboxProcess { if unsafe { GetExitCodeProcess(self.process.get(), &mut code) }.is_err() { return Err(std::io::Error::other("GetExitCodeProcess failed")); } - Ok(Some(code as i32)) + // A terminal observation must not become visible before + // captureDenials is sealed. This is the path used by polling + // SDK waits, so finalize synchronously before reporting exit. + self.terminate_and_reap(); + let teardown_result = self.run_teardown(); + combine_process_and_teardown_results(Ok(code as i32), teardown_result).map(Some) } WAIT_TIMEOUT => Ok(None), _ => Err(std::io::Error::other("WaitForSingleObject failed")), @@ -1587,14 +1613,7 @@ impl SandboxProcess for BaseContainerSandboxProcess { fn kill(&mut self) -> std::io::Result<()> { // Tree-kill via the job object when the child was successfully assigned // to one; otherwise fall back to terminating the root process. - if let Some(job) = &self.job { - job.terminate(u32::MAX); - } else { - unsafe { - let _ = TerminateProcess(self.process.get(), u32::MAX); - } - } - Ok(()) + self.kill_process_tree() } fn wait(&mut self) -> std::io::Result { @@ -1631,10 +1650,7 @@ impl SandboxProcess for BaseContainerSandboxProcess { // failure this also terminates it. Then reap the root before releasing // the pipe drains — and killing the tree closes the descendant's pipe // write-ends, so the drains can finish. - let _ = self.kill(); - unsafe { - let _ = WaitForSingleObject(self.process.get(), u32::MAX); - } + self.terminate_and_reap(); cancel_and_join_discard(stdout_thread, &self.stdout_canceller); cancel_and_join_discard(stderr_thread, &self.stderr_canceller); let teardown_result = self.run_teardown(); @@ -1647,10 +1663,7 @@ impl Drop for BaseContainerSandboxProcess { // Kill and reap before tearing down proxy / sandbox state, so an // abandoned-but-running sandbox cannot outlive its enforcement (or // leak as an orphan). - let _ = self.kill(); - unsafe { - let _ = WaitForSingleObject(self.process.get(), u32::MAX); - } + self.terminate_and_reap(); if let Err(error) = self.run_teardown() { eprintln!("captureDenials teardown failed during drop: {error}"); } From c7a335cd351779b54186b8f56cc426c20469c5a7 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 14:06:26 -0700 Subject: [PATCH 08/15] Fix capture availability and polling semantics Classify feature-disabled capture initialization as backend unavailable and preserve observational try-wait behavior for ordinary BaseContainer processes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../common/src/base_container_runner.rs | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index a63c33116..6a348ab5c 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -128,6 +128,18 @@ fn is_api_not_implemented(err: u32) -> bool { err == ERROR_CALL_NOT_IMPLEMENTED.0 || err == E_NOTIMPL.0 as u32 } +fn learning_mode_api_not_implemented(error: &learning_mode_windows::LearningModeError) -> bool { + match error { + learning_mode_windows::LearningModeError::ApiCall { code, .. } => { + is_api_not_implemented(*code) + } + learning_mode_windows::LearningModeError::CleanupFailed { primary, .. } => { + learning_mode_api_not_implemented(primary) + } + _ => false, + } +} + /// Script runner that uses `Experimental_CreateProcessInSandbox` API /// to launch a sandboxed process. #[derive(Default)] @@ -1055,11 +1067,16 @@ impl BaseContainerRunner { Err(e) => { let msg = format!("captureDenials: failed to start learning-mode capture: {e}"); let _ = writeln!(logger, "Error: {msg}"); + let failure_phase = if learning_mode_api_not_implemented(&e) { + FailurePhase::BackendUnavailable + } else { + FailurePhase::LaunchFailed + }; return Err(ScriptResponse { exit_code: -1, error_message: msg.clone(), standard_err: msg, - failure_phase: FailurePhase::LaunchFailed, + failure_phase, ..Default::default() }); } @@ -1594,6 +1611,9 @@ impl SandboxProcess for BaseContainerSandboxProcess { if unsafe { GetExitCodeProcess(self.process.get(), &mut code) }.is_err() { return Err(std::io::Error::other("GetExitCodeProcess failed")); } + if self.capture_session.is_none() && self.teardown_result.is_none() { + return Ok(Some(code as i32)); + } // A terminal observation must not become visible before // captureDenials is sealed. This is the path used by polling // SDK waits, so finalize synchronously before reporting exit. @@ -1769,6 +1789,29 @@ mod tests { assert!(!is_api_not_implemented(0)); } + #[test] + fn learning_mode_api_not_implemented_checks_primary_failure() { + use learning_mode_windows::LearningModeError; + + let disabled = LearningModeError::CleanupFailed { + primary: Box::new(LearningModeError::ApiCall { + function: "CreateProcessSecurityEnvironment", + code: ERROR_CALL_NOT_IMPLEMENTED.0, + }), + cleanup: Box::new(LearningModeError::ApiCall { + function: "CloseProcessSecurityEnvironment", + code: 87, + }), + }; + assert!(learning_mode_api_not_implemented(&disabled)); + + let ordinary = LearningModeError::ApiCall { + function: "StartLearningModeTrace", + code: 87, + }; + assert!(!learning_mode_api_not_implemented(&ordinary)); + } + #[test] fn decode_create_capability_table() { let cap = SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX; From 92f4e4b926e7f7a7988230e32f4795536e35858b Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 15:40:51 -0700 Subject: [PATCH 09/15] Clean up failed capture initialization Remove unlaunched profiles and tracking after safe capture-init failures, retain deferred recovery state when environment or profile cleanup cannot be proven complete, and stop registered cleanup state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../common/src/base_container_runner.rs | 50 +++++++++++++++++++ .../common/src/sandbox_tracking.rs | 17 +++++++ 2 files changed, 67 insertions(+) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 6a348ab5c..9bb78527f 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -140,6 +140,13 @@ fn learning_mode_api_not_implemented(error: &learning_mode_windows::LearningMode } } +fn learning_mode_cleanup_failed(error: &learning_mode_windows::LearningModeError) -> bool { + matches!( + error, + learning_mode_windows::LearningModeError::CleanupFailed { .. } + ) +} + /// Script runner that uses `Experimental_CreateProcessInSandbox` API /// to launch a sandboxed process. #[derive(Default)] @@ -1072,6 +1079,26 @@ impl BaseContainerRunner { } else { FailurePhase::LaunchFailed }; + if request.lifecycle.destroy_on_exit { + if learning_mode_cleanup_failed(&e) { + sandbox_tracking::mark_cleanup_deferred( + &sid_string, + "capture initialization failed and the process security environment could not be closed", + logger, + ); + } else { + // No process was launched and the security + // environment is closed, so real profile/tracking + // cleanup is safe. + sandbox_tracking::cleanup_unlaunched_sandbox( + &identity, + &sid_string, + logger, + ); + } + sandbox_tracking::unregister_ctrl_c_cleanup(); + } + self.proxy_coordinator.stop(logger); return Err(ScriptResponse { exit_code: -1, error_message: msg.clone(), @@ -1812,6 +1839,29 @@ mod tests { assert!(!learning_mode_api_not_implemented(&ordinary)); } + #[test] + fn learning_mode_cleanup_failure_is_detected() { + use learning_mode_windows::LearningModeError; + + let error = LearningModeError::CleanupFailed { + primary: Box::new(LearningModeError::ApiCall { + function: "StartLearningModeTrace", + code: 87, + }), + cleanup: Box::new(LearningModeError::ApiCall { + function: "CloseProcessSecurityEnvironment", + code: 5, + }), + }; + assert!(learning_mode_cleanup_failed(&error)); + + let ordinary = LearningModeError::ApiCall { + function: "StartLearningModeTrace", + code: 87, + }; + assert!(!learning_mode_cleanup_failed(&ordinary)); + } + #[test] fn decode_create_capability_table() { let cap = SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX; diff --git a/src/backends/appcontainer/common/src/sandbox_tracking.rs b/src/backends/appcontainer/common/src/sandbox_tracking.rs index ade2b3da3..5ada785b7 100644 --- a/src/backends/appcontainer/common/src/sandbox_tracking.rs +++ b/src/backends/appcontainer/common/src/sandbox_tracking.rs @@ -193,6 +193,23 @@ pub fn cleanup_sandbox(identity: &str, sid_string: &str, logger: &mut Logger) { delete_tracking_key(sid_string, logger); } +/// Cleans up a sandbox whose process was never launched. +/// +/// The tracking entry is removed only after profile deletion succeeds. If +/// deletion fails, the entry is retained and marked deferred so a later +/// maintenance pass still has the identity and SID needed for recovery. +pub fn cleanup_unlaunched_sandbox(identity: &str, sid_string: &str, logger: &mut Logger) { + if crate::appcontainer_runner::delete_app_container_profile(identity, logger) { + delete_tracking_key(sid_string, logger); + } else { + mark_cleanup_deferred( + sid_string, + "capture initialization failed and the AppContainer profile could not be deleted", + logger, + ); + } +} + /// Remove the registry tracking key and all its subkeys (including Active). fn delete_tracking_key(sid_string: &str, logger: &mut Logger) { let key_path = format!("{}\\{}", TRACKING_BASE, sid_string); From 80d7d6109cda064c6d82cfff8b2fb0336ed76e52 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 18:28:52 -0700 Subject: [PATCH 10/15] Reject capture fallback before DACL mutation Select an unstamped AppContainer backend for unsupported captureDenials fallback so normal validation emits the structured backend-unavailable response without modifying host ACLs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../appcontainer/common/src/dispatcher.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/backends/appcontainer/common/src/dispatcher.rs b/src/backends/appcontainer/common/src/dispatcher.rs index 2db648327..d3e67beee 100644 --- a/src/backends/appcontainer/common/src/dispatcher.rs +++ b/src/backends/appcontainer/common/src/dispatcher.rs @@ -330,6 +330,21 @@ fn select_backend_with_fallback( DispatchError, > { let decision = fallback_detector::detect(&request.policy, /*prefer_bc=*/ true)?; + if request.policy.capture_denials.is_some() && decision.tier != IsolationTier::BaseContainer { + let filesystem_mode = match decision.tier { + IsolationTier::AppContainerBfs => FilesystemMode::Bfs, + IsolationTier::AppContainerDacl => FilesystemMode::Dacl, + IsolationTier::BaseContainer => unreachable!(), + }; + return Ok(( + SelectedBackend::AppContainer(AppContainerScriptRunner::with_filesystem_mode( + filesystem_mode, + )), + None, + decision.tier, + decision.warnings, + )); + } let (backend, dacl_manager): (SelectedBackend, Option) = match decision.tier { IsolationTier::BaseContainer => { @@ -673,6 +688,21 @@ mod tests { "T3 always requires DaclManager (grants applied)" ); } + + #[test] + fn capture_denials_defers_fallback_rejection_without_dacl_setup() { + let _g = ForceTierGuard::set("appcontainer-dacl"); + let (mut policy, _tmp) = policy_with_rw_temp(); + policy.capture_denials = Some(Default::default()); + let req = test_request(policy); + + let dispatched = dispatch_with_fallback(&req).expect("validation should own the rejection"); + assert!(matches!(dispatched.tier, IsolationTier::AppContainerDacl)); + assert!( + !dispatched.has_dacl_guard(), + "capture fallback must not apply host DACLs before validation" + ); + } #[test] fn dispatch_fallback_disabled_errors() { let _g = ForceTierGuard::set("appcontainer-dacl"); From a79c6f4f7779b0f30a0dfcdf5fcf8f9f804408a3 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 22:01:32 -0700 Subject: [PATCH 11/15] Address integration review feedback Reuse diagnostic constants, clarify capture API naming and policy matching, extract the environment fallback launch loop, and document the explicit pre-launch cleanup lifecycle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../common/src/appcontainer_runner.rs | 16 +- .../common/src/base_container_runner.rs | 246 ++++++++++-------- .../common/src/sandbox_tracking.rs | 8 +- src/core/wxc/src/main.rs | 11 +- src/core/wxc_common/src/config_parser.rs | 3 + 5 files changed, 162 insertions(+), 122 deletions(-) diff --git a/src/backends/appcontainer/common/src/appcontainer_runner.rs b/src/backends/appcontainer/common/src/appcontainer_runner.rs index 02ced9497..03b052965 100644 --- a/src/backends/appcontainer/common/src/appcontainer_runner.rs +++ b/src/backends/appcontainer/common/src/appcontainer_runner.rs @@ -48,6 +48,9 @@ use wxc_common::sandbox_process::{ use wxc_common::script_runner::get_timeout_milliseconds; use wxc_common::{string_util, ui_policy}; +pub(crate) const CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG: &str = + "captureDenials requires the BaseContainer learning-mode APIs and is not supported by the AppContainer fallback tier"; + /// `UpdateProcThreadAttribute` value for /// `PROC_THREAD_ATTRIBUTE_ALL_APPLICATION_PACKAGES_POLICY` that opts the /// process out of inheriting `ALL APPLICATION PACKAGES` grants. This @@ -1306,11 +1309,9 @@ impl AppContainerScriptRunner { impl SandboxBackend for AppContainerScriptRunner { fn validate(&self, request: &ExecutionRequest) -> Result<(), ScriptResponse> { if request.policy.capture_denials.is_some() { - let msg = "captureDenials requires the BaseContainer learning-mode APIs and is not \ - supported by the AppContainer fallback tier"; return Err(ScriptResponse { failure_phase: FailurePhase::BackendUnavailable, - ..ScriptResponse::error(msg) + ..ScriptResponse::error(CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG) }); } if !request.policy.denied_paths.is_empty() && self.filesystem_mode != FilesystemMode::Dacl { @@ -1833,7 +1834,9 @@ mod tests { // ---- validate_runner: unsupported policy fields surface as errors. ---- - use super::{AppContainerScriptRunner, FilesystemMode}; + use super::{ + AppContainerScriptRunner, FilesystemMode, CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG, + }; use wxc_common::models::{ExecutionRequest, FailurePhase}; use wxc_common::sandbox_process::SandboxBackend; @@ -1906,6 +1909,9 @@ mod tests { .validate(&request) .expect_err("AppContainer fallback must not silently ignore captureDenials"); assert_eq!(error.failure_phase, FailurePhase::BackendUnavailable); - assert!(error.error_message.contains("requires the BaseContainer")); + assert_eq!( + error.error_message, + CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG + ); } } diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 9bb78527f..bbead9b9d 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -112,6 +112,83 @@ type PfnCreateProcessInSandbox = unsafe extern "system" fn( /// ambiguous and must not be read as "sandbox unsupported". type PfnQuerySandboxSupport = unsafe extern "system" fn(capabilities: *mut u64) -> i32; +struct SandboxLaunchArgs<'a> { + api: PfnCreateProcessInSandbox, + command_line: &'a mut [u16], + current_directory: *const u16, + startup_info: &'a STARTUPINFOW, + identity: &'a [u16], + sandbox_specification: &'a [u8], + no_window_flag: u32, +} + +impl SandboxLaunchArgs<'_> { + /// Launches through the one-shot API, retrying once without the + /// environment block on downlevel systems that reject that parameter. + fn launch_with_environment_fallback( + &mut self, + creation_flags: u32, + environment: *const c_void, + process_information: &mut PROCESS_INFORMATION, + logger: &mut Logger, + ) -> (i32, Option) { + let mut current_creation_flags = creation_flags; + let mut current_environment = environment; + let mut retries_remaining = 1; + + loop { + *process_information = unsafe { std::mem::zeroed() }; + let result = unsafe { + (self.api)( + ptr::null(), + self.command_line.as_mut_ptr(), + ptr::null(), + ptr::null(), + i32::from(false), + current_creation_flags, + current_environment, + self.current_directory, + self.startup_info, + self.identity.as_ptr(), + self.sandbox_specification.as_ptr(), + self.sandbox_specification.len() as u32, + process_information, + ) + }; + if result != 0 { + return (result, None); + } + + let error = unsafe { GetLastError() }; + if retries_remaining == 0 + || !is_environment_not_supported(error.0, !current_environment.is_null()) + { + return (result, Some(error)); + } + retries_remaining -= 1; + + unsafe { + if !process_information.hProcess.is_invalid() { + let _ = CloseHandle(process_information.hProcess); + } + if !process_information.hThread.is_invalid() { + let _ = CloseHandle(process_information.hThread); + } + } + + let diagnostic = diagnose_environment_not_supported(); + let _ = writeln!( + logger, + "{EMOJI_WARNING} Launch diagnostic [{}]: {}", + diagnostic.kind, diagnostic.message + ); + + current_environment = ptr::null(); + current_creation_flags = CREATE_SUSPENDED.0 | self.no_window_flag; + } + } +} + /// `SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX`: when clear, Tier 1 is unusable. const SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX: u64 = 0x0000_0000_0000_0001; @@ -121,6 +198,12 @@ const SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX: u64 = 0x0000_0000_0000_0001; /// assumed bit 1). When clear, `deniedPaths` is rejected at launch and callers /// must rely on default-deny plus explicit `readwrite`/`readonly` grants. const SANDBOX_CAP_FS_DENY: u64 = 0x0000_0000_0000_0002; +const CAPTURE_API_AVAILABLE_LOG: &str = + "captureDenials: learning-mode trace API available (processmodel.dll)"; +const CAPTURE_API_UNAVAILABLE_REQUIREMENT: &str = + "This feature requires a feature-enabled Windows build that exports the learning-mode trace APIs from processmodel.dll."; +const CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON: &str = + "capture initialization failed and the process security environment could not be closed"; /// True when a Win32 error code signals the BaseContainer feature is not /// enabled on this build (symbol present, capability gated off). @@ -179,6 +262,32 @@ impl BaseContainerRunner { Self::default() } + fn cleanup_capture_begin_failure( + &mut self, + error: &learning_mode_windows::LearningModeError, + request: &ExecutionRequest, + identity: &str, + sid_string: &str, + logger: &mut Logger, + ) { + // This cannot be deferred to BaseContainerRunner::drop: the runner may + // outlive a failed spawn, and the exact error determines whether + // profile deletion is safe or recovery tracking must be retained. + if request.lifecycle.destroy_on_exit { + if learning_mode_cleanup_failed(error) { + sandbox_tracking::mark_cleanup_deferred( + sid_string, + CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON, + logger, + ); + } else { + sandbox_tracking::cleanup_unlaunched_sandbox(identity, sid_string, logger); + } + sandbox_tracking::unregister_ctrl_c_cleanup(); + } + self.proxy_coordinator.stop(logger); + } + /// Pre-flight probe: check whether the current OS build exports the /// `Experimental_CreateProcessInSandbox` symbol from `processmodel.dll`. /// @@ -685,37 +794,26 @@ impl BaseContainerRunner { Self::log_sandbox_spec(&spec_bytes, logger); - // Resolve the learning-mode capture APIs when captureDenials is - // configured. The mode-mapped learning-mode capability was already - // injected into policy.capabilities (and hence the spec) by the config - // parser; here we obtain the OS handles that drive the ETL trace. Both - // exports live in the same processmodel.dll the BaseContainer launch API - // is loaded from, and only exist on supported feature-enabled Windows - // builds, so a failed load is a clean, actionable "backend unavailable" - // degrade. + // Load the security-environment and trace APIs used by captureDenials. + // Both are feature-gated exports from processmodel.dll. let capture_denials = request.policy.capture_denials.clone(); let mut capture_apis: Option<(SecurityEnvironmentApi, LearningModeApi)> = None; if capture_denials.is_some() { let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: captureDenials"); match (SecurityEnvironmentApi::load(), LearningModeApi::load()) { - (Ok(se), Ok(lm)) => { - let _ = writeln!( - logger, - "captureDenials: learning-mode trace API available (processmodel.dll)" - ); - capture_apis = Some((se, lm)); + (Ok(security_environment_api), Ok(learning_mode_api)) => { + let _ = writeln!(logger, "{CAPTURE_API_AVAILABLE_LOG}"); + capture_apis = Some((security_environment_api, learning_mode_api)); } - (se, lm) => { - let detail = match (&se, &lm) { + (security_environment_api, learning_mode_api) => { + let detail = match (&security_environment_api, &learning_mode_api) { (Err(e), _) => format!("security-environment API: {e}"), (_, Err(e)) => format!("learning-mode trace API: {e}"), _ => "unknown".to_string(), }; let msg = format!( "captureDenials was requested but the learning-mode trace API is not \ - available on this OS build ({detail}). This feature requires a \ - feature-enabled Windows build that exports the learning-mode trace \ - APIs from processmodel.dll." + available on this OS build ({detail}). {CAPTURE_API_UNAVAILABLE_REQUIREMENT}" ); let _ = writeln!(logger, "Error: {msg}"); return Err(ScriptResponse { @@ -1039,9 +1137,8 @@ impl BaseContainerRunner { // If the OS returns ERROR_NOT_SUPPORTED (0x32) and we passed a non-null // environment block, this is a downlevel build that doesn't support the // `environment` parameter. Retry once without it. - let mut current_env_ptr = env_ptr; - let mut current_creation_flags = creation_flags; - let mut retries_remaining: u32 = 1; + let current_env_ptr = env_ptr; + let current_creation_flags = creation_flags; // When captureDenials is active, launch inside a process security // environment that already has a learning-mode trace started against it, @@ -1079,26 +1176,13 @@ impl BaseContainerRunner { } else { FailurePhase::LaunchFailed }; - if request.lifecycle.destroy_on_exit { - if learning_mode_cleanup_failed(&e) { - sandbox_tracking::mark_cleanup_deferred( - &sid_string, - "capture initialization failed and the process security environment could not be closed", - logger, - ); - } else { - // No process was launched and the security - // environment is closed, so real profile/tracking - // cleanup is safe. - sandbox_tracking::cleanup_unlaunched_sandbox( - &identity, - &sid_string, - logger, - ); - } - sandbox_tracking::unregister_ctrl_c_cleanup(); - } - self.proxy_coordinator.stop(logger); + self.cleanup_capture_begin_failure( + &e, + &request, + &identity, + &sid_string, + logger, + ); return Err(ScriptResponse { exit_code: -1, error_message: msg.clone(), @@ -1145,71 +1229,21 @@ impl BaseContainerRunner { (0, Some(unsafe { GetLastError() })) } } else { - loop { - pi = unsafe { std::mem::zeroed() }; - - let result = unsafe { - create_process_in_sandbox( - ptr::null(), // applicationName (resolved from commandLine) - cmd_wide.as_mut_ptr(), // commandLine - ptr::null(), // processAttributes (must be NULL) - ptr::null(), // threadAttributes (must be NULL) - // inheritHandles: must be FALSE per the OS sandbox API contract. - // Unlike regular CreateProcess, CreateProcessInSandbox treats the - // explicit STDIO handles in STARTUPINFO (hStdInput/hStdOutput/hStdError) - // as inheritable when STARTF_USESTDHANDLES is set, but does not support - // general handle inheritance. - i32::from(false), // inheritHandles - current_creation_flags, // creationFlags - current_env_ptr, // environment - cwd_ptr, // currentDirectory - &si, // startupInfo - identity_wide.as_ptr(), // identity - spec_bytes.as_ptr(), // sandboxSpecification - spec_bytes.len() as u32, // sandboxSpecificationSize - &mut pi, // processInformation - ) - }; - - if result != 0 { - break (result, None); - } - - // Call failed -- capture the error before any handle cleanup. - let err = unsafe { GetLastError() }; - - if retries_remaining > 0 - && is_environment_not_supported(err.0, !current_env_ptr.is_null()) - { - retries_remaining -= 1; - - // Clean up handles from the failed attempt. - unsafe { - if !pi.hProcess.is_invalid() { - let _ = CloseHandle(pi.hProcess); - } - if !pi.hThread.is_invalid() { - let _ = CloseHandle(pi.hThread); - } - } - - let diag = diagnose_environment_not_supported(); - let _ = writeln!( - logger, - "{EMOJI_WARNING} Launch diagnostic [{}]: {}", - diag.kind, diag.message - ); - - // Retry without the environment block, but keep the child - // suspended (resumed after job assignment). - current_env_ptr = ptr::null(); - current_creation_flags = CREATE_SUSPENDED.0 | no_window_flag; - continue; - } - - // Non-retryable failure. - break (result, Some(err)); + SandboxLaunchArgs { + api: create_process_in_sandbox, + command_line: &mut cmd_wide, + current_directory: cwd_ptr, + startup_info: &si, + identity: &identity_wide, + sandbox_specification: &spec_bytes, + no_window_flag, } + .launch_with_environment_fallback( + current_creation_flags, + current_env_ptr, + &mut pi, + logger, + ) }; if success == 0 { diff --git a/src/backends/appcontainer/common/src/sandbox_tracking.rs b/src/backends/appcontainer/common/src/sandbox_tracking.rs index 5ada785b7..e7f5df5f4 100644 --- a/src/backends/appcontainer/common/src/sandbox_tracking.rs +++ b/src/backends/appcontainer/common/src/sandbox_tracking.rs @@ -36,6 +36,8 @@ use wxc_common::string_util; /// Registry base path for sandbox tracking entries. const TRACKING_BASE: &str = "Software\\Classes\\Local Settings\\Software\\Microsoft\\Windows\\CurrentVersion\\ProcessSandboxes\\Mappings"; +const CAPTURE_PROFILE_CLEANUP_DEFERRED_REASON: &str = + "capture initialization failed and the AppContainer profile could not be deleted"; /// Generate a unique sandbox identity string: `sandbox-{16 lowercase hex chars}`. /// @@ -202,11 +204,7 @@ pub fn cleanup_unlaunched_sandbox(identity: &str, sid_string: &str, logger: &mut if crate::appcontainer_runner::delete_app_container_profile(identity, logger) { delete_tracking_key(sid_string, logger); } else { - mark_cleanup_deferred( - sid_string, - "capture initialization failed and the AppContainer profile could not be deleted", - logger, - ); + mark_cleanup_deferred(sid_string, CAPTURE_PROFILE_CLEANUP_DEFERRED_REASON, logger); } } diff --git a/src/core/wxc/src/main.rs b/src/core/wxc/src/main.rs index 21b9dabbd..90e8afb06 100644 --- a/src/core/wxc/src/main.rs +++ b/src/core/wxc/src/main.rs @@ -200,6 +200,9 @@ fn apply_permissive_learning_mode(capabilities: &mut Vec) -> bool { } } +const AUDIT_CAPTURE_DENIALS_CONFLICT_MSG: &str = + "--audit cannot be combined with processContainer.captureDenials; use captureDenials.mode: \"allow\" for permissive capture"; + fn validate_audit_request(request: &ExecutionRequest) -> Result<(), String> { if request.containment != ContainmentBackend::ProcessContainer { return Err(format!( @@ -209,11 +212,7 @@ fn validate_audit_request(request: &ExecutionRequest) -> Result<(), String> { )); } if request.policy.capture_denials.is_some() { - return Err( - "--audit cannot be combined with processContainer.captureDenials; \ - use captureDenials.mode: \"allow\" for permissive capture" - .to_string(), - ); + return Err(AUDIT_CAPTURE_DENIALS_CONFLICT_MSG.to_string()); } Ok(()) } @@ -1476,7 +1475,7 @@ mod tests { let error = validate_audit_request(&request) .expect_err("--audit and captureDenials must be mutually exclusive"); - assert!(error.contains("cannot be combined")); + assert_eq!(error, AUDIT_CAPTURE_DENIALS_CONFLICT_MSG); assert!(error.contains(r#"mode: "allow""#)); } } diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 22c334fa6..8f26bbef8 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -869,6 +869,9 @@ fn convert_wire_config( // `permissiveLearningMode` (the runner surfaces the security warning). let capture_capability = match mode { CaptureDenialsMode::Block => { + // Capability entries are exact names. Comma-packed entries + // were rejected above, so substring matching here would + // incorrectly remove unrelated custom capabilities. policy.capabilities.retain(|capability| { !capability.eq_ignore_ascii_case("permissiveLearningMode") }); From 28b457224a5607878a74691c4cd7a64387e6ac69 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 22:23:03 -0700 Subject: [PATCH 12/15] Harden capture cleanup and diagnostics Guard tracking operations without a SID, remove only safe pre-launch tracking state, report the actual capture launch API, and document the feature-enabled BaseContainer requirement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- docs/learning-mode/capabilities.md | 5 +++ .../common/src/base_container_runner.rs | 36 +++++++++------- .../common/src/sandbox_tracking.rs | 42 +++++++++++++------ 3 files changed, 54 insertions(+), 29 deletions(-) diff --git a/docs/learning-mode/capabilities.md b/docs/learning-mode/capabilities.md index 004eeb85c..f496113bf 100644 --- a/docs/learning-mode/capabilities.md +++ b/docs/learning-mode/capabilities.md @@ -117,6 +117,11 @@ Windows-only `captureDenials` config switch drives collecting those events and surfacing the resulting denials to the caller. Its `mode` selects how each ungranted access is handled while it is recorded: +> **Host requirement.** `captureDenials` requires a feature-enabled Windows +> build exposing the BaseContainer security-environment and Learning Mode APIs. +> It is not supported by the AppContainer fallback tiers; unsupported hosts +> return `backend_unavailable`. + - `mode: "block"` (default) maps onto `learningModeLogging` (deny-and-record) — the app / user-configurable flow. - `mode: "allow"` maps onto `permissiveLearningMode` (allow-and-record) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index bbead9b9d..25e5ca593 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -204,6 +204,9 @@ const CAPTURE_API_UNAVAILABLE_REQUIREMENT: &str = "This feature requires a feature-enabled Windows build that exports the learning-mode trace APIs from processmodel.dll."; const CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON: &str = "capture initialization failed and the process security environment could not be closed"; +const CREATE_PROCESS_IN_SANDBOX_API: &str = "Experimental_CreateProcessInSandbox"; +const CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API: &str = + "CreateProcessAsUserInsideSecurityEnvironment"; /// True when a Win32 error code signals the BaseContainer feature is not /// enabled on this build (symbol present, capability gated off). @@ -266,7 +269,6 @@ impl BaseContainerRunner { &mut self, error: &learning_mode_windows::LearningModeError, request: &ExecutionRequest, - identity: &str, sid_string: &str, logger: &mut Logger, ) { @@ -281,7 +283,10 @@ impl BaseContainerRunner { logger, ); } else { - sandbox_tracking::cleanup_unlaunched_sandbox(identity, sid_string, logger); + // CaptureSession::begin receives the sandbox specification, + // not the later process identity. Once its environment is + // closed, only the pre-launch tracking entry needs removal. + sandbox_tracking::remove_tracking_entry(sid_string, logger); } sandbox_tracking::unregister_ctrl_c_cleanup(); } @@ -1176,13 +1181,7 @@ impl BaseContainerRunner { } else { FailurePhase::LaunchFailed }; - self.cleanup_capture_begin_failure( - &e, - &request, - &identity, - &sid_string, - logger, - ); + self.cleanup_capture_begin_failure(&e, &request, &sid_string, logger); return Err(ScriptResponse { exit_code: -1, error_message: msg.clone(), @@ -1195,7 +1194,7 @@ impl BaseContainerRunner { } // The launch yields (api_return_code, last_win32_error_on_failure). - let (success, last_error) = if let (Some(session), Some(se_api)) = + let (success, last_error, launch_api_name) = if let (Some(session), Some(se_api)) = (capture_session.as_ref(), capture_se_api.as_ref()) { // Single-attempt in-environment launch. The learning-mode security @@ -1224,12 +1223,16 @@ impl BaseContainerRunner { ) }; if ok != 0 { - (ok, None) + (ok, None, CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API) } else { - (0, Some(unsafe { GetLastError() })) + ( + 0, + Some(unsafe { GetLastError() }), + CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API, + ) } } else { - SandboxLaunchArgs { + let (success, error) = SandboxLaunchArgs { api: create_process_in_sandbox, command_line: &mut cmd_wide, current_directory: cwd_ptr, @@ -1243,7 +1246,8 @@ impl BaseContainerRunner { current_env_ptr, &mut pi, logger, - ) + ); + (success, error, CREATE_PROCESS_IN_SANDBOX_API) }; if success == 0 { @@ -1277,7 +1281,7 @@ impl BaseContainerRunner { &request.policy.readonly_paths, ); - let extended_error = format!("Experimental_CreateProcessInSandbox failed: {err:?}"); + let extended_error = format!("{launch_api_name} failed: {err:?}"); let _ = writeln!(logger, "Error: {extended_error}"); let _ = writeln!( @@ -1603,7 +1607,7 @@ impl BaseContainerSandboxProcess { Ok(()) } Err(error) => Err(std::io::Error::other(format!( - "captureDenials failed to seal the denial trace: {error}" + "captureDenials failed to finalize the denial capture: {error}" ))), } } else { diff --git a/src/backends/appcontainer/common/src/sandbox_tracking.rs b/src/backends/appcontainer/common/src/sandbox_tracking.rs index e7f5df5f4..6012dd02e 100644 --- a/src/backends/appcontainer/common/src/sandbox_tracking.rs +++ b/src/backends/appcontainer/common/src/sandbox_tracking.rs @@ -36,8 +36,6 @@ use wxc_common::string_util; /// Registry base path for sandbox tracking entries. const TRACKING_BASE: &str = "Software\\Classes\\Local Settings\\Software\\Microsoft\\Windows\\CurrentVersion\\ProcessSandboxes\\Mappings"; -const CAPTURE_PROFILE_CLEANUP_DEFERRED_REASON: &str = - "capture initialization failed and the AppContainer profile could not be deleted"; /// Generate a unique sandbox identity string: `sandbox-{16 lowercase hex chars}`. /// @@ -139,6 +137,13 @@ pub fn write_tracking_entry(entry: &TrackingEntry, logger: &mut Logger) -> Resul /// /// Adds a `CleanupDeferred` REG_SZ value to the existing tracking key. pub fn mark_cleanup_deferred(sid_string: &str, reason: &str, logger: &mut Logger) { + if sid_string.is_empty() { + let _ = writeln!( + logger, + "warning: cleanup could not be tracked because no AppContainer SID was derived" + ); + return; + } let key_path = format!("{}\\{}", TRACKING_BASE, sid_string); let hkcu = RegKey::predef(HKEY_CURRENT_USER); @@ -195,21 +200,20 @@ pub fn cleanup_sandbox(identity: &str, sid_string: &str, logger: &mut Logger) { delete_tracking_key(sid_string, logger); } -/// Cleans up a sandbox whose process was never launched. -/// -/// The tracking entry is removed only after profile deletion succeeds. If -/// deletion fails, the entry is retained and marked deferred so a later -/// maintenance pass still has the identity and SID needed for recovery. -pub fn cleanup_unlaunched_sandbox(identity: &str, sid_string: &str, logger: &mut Logger) { - if crate::appcontainer_runner::delete_app_container_profile(identity, logger) { - delete_tracking_key(sid_string, logger); - } else { - mark_cleanup_deferred(sid_string, CAPTURE_PROFILE_CLEANUP_DEFERRED_REASON, logger); - } +/// Removes the tracking entry for a sandbox that never launched. +pub fn remove_tracking_entry(sid_string: &str, logger: &mut Logger) { + delete_tracking_key(sid_string, logger); } /// Remove the registry tracking key and all its subkeys (including Active). fn delete_tracking_key(sid_string: &str, logger: &mut Logger) { + if sid_string.is_empty() { + let _ = writeln!( + logger, + "warning: refusing to delete sandbox tracking without an AppContainer SID" + ); + return; + } let key_path = format!("{}\\{}", TRACKING_BASE, sid_string); let hkcu = RegKey::predef(HKEY_CURRENT_USER); match hkcu.delete_subkey_all(&key_path) { @@ -366,4 +370,16 @@ mod tests { "unexpected SID: {sid_str}" ); } + + #[test] + fn empty_sid_never_targets_shared_tracking_root() { + let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); + + mark_cleanup_deferred("", "test", &mut logger); + remove_tracking_entry("", &mut logger); + + let output = logger.get_buffer(); + assert!(output.contains("no AppContainer SID was derived")); + assert!(output.contains("refusing to delete sandbox tracking")); + } } From a09ad48988456c12421178c90e0cb85419a7485a Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Thu, 30 Jul 2026 17:08:03 -0700 Subject: [PATCH 13/15] Use process security environment attribute Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a8860a9-bb20-48ef-91a1-3da8e34b92fb --- .../common/src/base_container_runner.rs | 107 +++++--- .../windows/examples/lm_capture.rs | 58 ++-- .../windows/examples/lm_probe.rs | 3 +- src/backends/learning_mode/windows/src/lib.rs | 2 +- .../learning_mode/windows/src/lifecycle.rs | 7 +- .../learning_mode/windows/src/secenv.rs | 254 +++++++++++++----- 6 files changed, 288 insertions(+), 143 deletions(-) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 25e5ca593..112637118 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -16,12 +16,13 @@ use std::path::PathBuf; use std::ptr; use learning_mode_windows::{ - CaptureSession, LearningModeApi, SecurityEnvironmentApi, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, + CaptureSession, LearningModeApi, SecurityEnvironmentApi, SecurityEnvironmentStartupInfo, + PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, }; use windows::Win32::Foundation::{ - CloseHandle, GetLastError, SetHandleInformation, ERROR_CALL_NOT_IMPLEMENTED, E_NOTIMPL, HANDLE, - HANDLE_FLAG_INHERIT, WAIT_OBJECT_0, WAIT_TIMEOUT, + CloseHandle, GetLastError, SetHandleInformation, ERROR_CALL_NOT_IMPLEMENTED, + ERROR_NOT_SUPPORTED, E_NOTIMPL, HANDLE, HANDLE_FLAG_INHERIT, WAIT_OBJECT_0, WAIT_TIMEOUT, }; use windows::Win32::System::Console::{ GetStdHandle, STD_ERROR_HANDLE, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, @@ -30,10 +31,11 @@ use windows::Win32::System::LibraryLoader::{ GetProcAddress, LoadLibraryExW, LOAD_LIBRARY_SEARCH_SYSTEM32, }; use windows::Win32::System::Threading::{ - GetExitCodeProcess, TerminateProcess, WaitForSingleObject, PROCESS_INFORMATION, + CreateProcessW, GetExitCodeProcess, TerminateProcess, WaitForSingleObject, + EXTENDED_STARTUPINFO_PRESENT, PROCESS_CREATION_FLAGS, PROCESS_INFORMATION, STARTF_USESTDHANDLES, STARTUPINFOW, }; -use windows_core::PCWSTR; +use windows_core::{PCWSTR, PWSTR}; use crate::job_object::UiJobObject; use crate::launch_diagnostics::{ @@ -206,7 +208,7 @@ const CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON: &str = "capture initialization failed and the process security environment could not be closed"; const CREATE_PROCESS_IN_SANDBOX_API: &str = "Experimental_CreateProcessInSandbox"; const CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API: &str = - "CreateProcessAsUserInsideSecurityEnvironment"; + "CreateProcessW(PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT)"; /// True when a Win32 error code signals the BaseContainer feature is not /// enabled on this build (symbol present, capability gated off). @@ -217,7 +219,7 @@ fn is_api_not_implemented(err: u32) -> bool { fn learning_mode_api_not_implemented(error: &learning_mode_windows::LearningModeError) -> bool { match error { learning_mode_windows::LearningModeError::ApiCall { code, .. } => { - is_api_not_implemented(*code) + is_api_not_implemented(*code) || *code == ERROR_NOT_SUPPORTED.0 } learning_mode_windows::LearningModeError::CleanupFailed { primary, .. } => { learning_mode_api_not_implemented(primary) @@ -265,7 +267,7 @@ impl BaseContainerRunner { Self::default() } - fn cleanup_capture_begin_failure( + fn cleanup_capture_prelaunch_failure( &mut self, error: &learning_mode_windows::LearningModeError, request: &ExecutionRequest, @@ -1149,13 +1151,11 @@ impl BaseContainerRunner { // environment that already has a learning-mode trace started against it, // instead of the one-shot CreateProcessInSandbox. `begin` creates the // environment and starts the trace *before* the child launches (so no - // early denials are missed); the child is then launched into that - // environment via CreateProcessAsUserInsideSecurityEnvironment, which - // returns a real PROCESS_INFORMATION we own and wait on exactly like the - // normal path. On any early return below, `capture_session` drops and its - // Drop discards the trace and closes the environment (no broker leak). + // early denials are missed); the environment handle is attached to a + // normal CreateProcessW call via PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT. + // On any early return below, `capture_session` drops and its Drop + // discards the trace and closes the environment (no broker leak). let mut capture_session: Option = None; - let mut capture_se_api: Option = None; if let Some((se_api, lm_api)) = capture_apis { match CaptureSession::begin( se_api, @@ -1168,9 +1168,6 @@ impl BaseContainerRunner { logger, "captureDenials: security environment created and trace started" ); - // `SecurityEnvironmentApi` is Copy, so we keep our own copy to - // call `launch_fn` at the launch site below. - capture_se_api = Some(se_api); capture_session = Some(session); } Err(e) => { @@ -1181,7 +1178,7 @@ impl BaseContainerRunner { } else { FailurePhase::LaunchFailed }; - self.cleanup_capture_begin_failure(&e, &request, &sid_string, logger); + self.cleanup_capture_prelaunch_failure(&e, &request, &sid_string, logger); return Err(ScriptResponse { exit_code: -1, error_message: msg.clone(), @@ -1194,36 +1191,60 @@ impl BaseContainerRunner { } // The launch yields (api_return_code, last_win32_error_on_failure). - let (success, last_error, launch_api_name) = if let (Some(session), Some(se_api)) = - (capture_session.as_ref(), capture_se_api.as_ref()) + let (success, last_error, launch_api_name) = if let Some(session) = capture_session.as_ref() { // Single-attempt in-environment launch. The learning-mode security - // environment only exists on builds that support the `environment` - // parameter, so the CreateProcessInSandbox env-not-supported retry - // does not apply here. + // environment is attached as a process-thread attribute; the + // CreateProcessInSandbox environment fallback does not apply here. pi = unsafe { std::mem::zeroed() }; - let launch = se_api.launch_fn(); - // SAFETY: `launch` was resolved from processmodel.dll and matches the - // declared C signature. `cmd_wide` is a mutable, null-terminated - // UTF-16 buffer; `si`/`pi` are valid; `session.environment()` is the - // live handle from the just-begun session. When `current_env_ptr` is - // non-null, `current_creation_flags` already carries - // CREATE_UNICODE_ENVIRONMENT. - let ok = unsafe { - launch( - HANDLE(ptr::null_mut()), // userToken: caller context - ptr::null(), // applicationName (from command line) - cmd_wide.as_mut_ptr(), // commandLine - current_creation_flags, // creationFlags - current_env_ptr, // environment - cwd_ptr, // currentDirectory - &si, // startupInfo - session.environment(), // process security environment - &mut pi, // processInformation + let inherited_handles = if pipe_mode { + vec![h_stdin, h_stdout, h_stderr] + } else { + Vec::new() + }; + let extended_startup = match SecurityEnvironmentStartupInfo::new( + si, + session.environment(), + &inherited_handles, + ) { + Ok(startup) => startup, + Err(error) => { + let msg = format!( + "captureDenials: failed to attach the process security environment: {error}" + ); + let _ = writeln!(logger, "Error: {msg}"); + let failure_phase = if learning_mode_api_not_implemented(&error) { + FailurePhase::BackendUnavailable + } else { + FailurePhase::LaunchFailed + }; + self.cleanup_capture_prelaunch_failure(&error, &request, &sid_string, logger); + return Err(ScriptResponse { + exit_code: -1, + error_message: msg.clone(), + standard_err: msg, + failure_phase, + ..Default::default() + }); + } + }; + let environment = (!current_env_ptr.is_null()).then_some(current_env_ptr); + let result = unsafe { + CreateProcessW( + PCWSTR::null(), + Some(PWSTR(cmd_wide.as_mut_ptr())), + None, + None, + !inherited_handles.is_empty(), + PROCESS_CREATION_FLAGS(current_creation_flags | EXTENDED_STARTUPINFO_PRESENT.0), + environment, + PCWSTR(cwd_ptr), + &extended_startup.startup_info().StartupInfo, + &mut pi, ) }; - if ok != 0 { - (ok, None, CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API) + if result.is_ok() { + (1, None, CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API) } else { ( 0, diff --git a/src/backends/learning_mode/windows/examples/lm_capture.rs b/src/backends/learning_mode/windows/examples/lm_capture.rs index c38a0b5b3..1a925481d 100644 --- a/src/backends/learning_mode/windows/examples/lm_capture.rs +++ b/src/backends/learning_mode/windows/examples/lm_capture.rs @@ -9,8 +9,9 @@ //! 1. build a minimal FlatBuffer sandbox spec with the `permissiveLearningMode` //! capability (the token the OS learning-mode path recognises), //! 2. [`CaptureSession::begin`] — create the security environment + start the trace, -//! 3. launch `cmd.exe` inside the environment via -//! `CreateProcessAsUserInsideSecurityEnvironment`, +//! 3. attach the environment handle through +//! `PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT` and launch `cmd.exe` with +//! `CreateProcessW`, //! 4. wait for it to exit, //! 5. [`CaptureSession::finish`] — seal the ETL to a temp path + close the environment, //! 6. assert the ETL file was produced (non-empty). @@ -41,7 +42,7 @@ mod windows_impl { use flatbuffers::FlatBufferBuilder; use learning_mode_windows::{ - CaptureSession, LearningModeApi, SecurityEnvironmentApi, + CaptureSession, LearningModeApi, SecurityEnvironmentApi, SecurityEnvironmentStartupInfo, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, }; use sandbox_spec::base_container_layout::{ @@ -49,8 +50,10 @@ mod windows_impl { }; use windows::Win32::Foundation::{CloseHandle, HANDLE, WAIT_FAILED, WAIT_OBJECT_0}; use windows::Win32::System::Threading::{ - GetExitCodeProcess, WaitForSingleObject, INFINITE, PROCESS_INFORMATION, STARTUPINFOW, + CreateProcessW, GetExitCodeProcess, WaitForSingleObject, EXTENDED_STARTUPINFO_PRESENT, + INFINITE, PROCESS_INFORMATION, STARTUPINFOW, }; + use windows_core::{PCWSTR, PWSTR}; /// Matches the schema version BaseContainer embeds in every spec payload. const SANDBOX_SPEC_VERSION: &str = "0.1.0"; @@ -118,7 +121,7 @@ mod windows_impl { }; println!("CaptureSession::begin OK — environment + trace live"); - let exit_code = match launch_and_wait(&secenv_api, session.environment()) { + let exit_code = match launch_and_wait(session.environment()) { Ok(code) => { println!("child exited with code {code}"); code @@ -161,11 +164,7 @@ mod windows_impl { /// Launch the child inside `environment` and wait for it to exit, returning its exit /// code. - fn launch_and_wait( - secenv_api: &SecurityEnvironmentApi, - environment: HANDLE, - ) -> Result { - let launch = secenv_api.launch_fn(); + fn launch_and_wait(environment: HANDLE) -> Result { let mut cmd = wide_command_line(); // SAFETY: a zeroed STARTUPINFOW with only `cb` set is valid; the child inherits @@ -173,33 +172,28 @@ mod windows_impl { let mut startup_info: STARTUPINFOW = unsafe { std::mem::zeroed() }; startup_info.cb = u32::try_from(std::mem::size_of::()) .map_err(|_| "STARTUPINFOW size overflow".to_string())?; + let extended_startup = SecurityEnvironmentStartupInfo::new(startup_info, environment, &[]) + .map_err(|error| error.to_string())?; let mut process_information: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; - // SAFETY: `launch` was resolved from processmodel.dll and matches the declared C - // signature. `cmd` is a mutable, null-terminated UTF-16 buffer; `startup_info` - // and `process_information` are valid; `environment` is the live handle from the - // session. `lpEnvironment` is null, so CREATE_UNICODE_ENVIRONMENT is not needed. - let ok = unsafe { - launch( - HANDLE(std::ptr::null_mut()), // userToken: caller context - std::ptr::null(), // applicationName (from command line) - cmd.as_mut_ptr(), // commandLine - 0, // creationFlags - std::ptr::null(), // environment - std::ptr::null(), // currentDirectory - &startup_info, - environment, + // SAFETY: `cmd` is mutable and null-terminated; `extended_startup` + // owns a valid attribute list containing the live environment handle; + // `process_information` is a valid output buffer. + unsafe { + CreateProcessW( + PCWSTR::null(), + Some(PWSTR(cmd.as_mut_ptr())), + None, + None, + false, + EXTENDED_STARTUPINFO_PRESENT, + None, + PCWSTR::null(), + &extended_startup.startup_info().StartupInfo, &mut process_information, ) - }; - if ok == 0 { - // SAFETY: reads the calling thread's last-error slot. - let err = unsafe { windows::Win32::Foundation::GetLastError() }; - return Err(format!( - "CreateProcessAsUserInsideSecurityEnvironment failed (GetLastError = {})", - err.0 - )); } + .map_err(|error| format!("CreateProcessW failed: {error}"))?; // SAFETY: `hProcess` is a valid process handle returned by the launch. let wait = unsafe { WaitForSingleObject(process_information.hProcess, INFINITE) }; diff --git a/src/backends/learning_mode/windows/examples/lm_probe.rs b/src/backends/learning_mode/windows/examples/lm_probe.rs index 2eadf5b0b..1dc3b8e93 100644 --- a/src/backends/learning_mode/windows/examples/lm_probe.rs +++ b/src/backends/learning_mode/windows/examples/lm_probe.rs @@ -6,7 +6,7 @@ //! Prints whether `processmodel.dll` on this machine exposes the Learning Mode trace //! exports (`StartLearningModeTrace` / `StopLearningModeTrace`) and the 2-phase //! security-environment exports (`CreateProcessSecurityEnvironment` / -//! `CreateProcessAsUserInsideSecurityEnvironment` / `CloseProcessSecurityEnvironment`), +//! `CloseProcessSecurityEnvironment`), //! reporting the exact resolved name for each (plain vs `Experimental_`). Intended to //! be run on a feature-enabled Windows build to confirm the runtime FFI resolves //! against the real API. @@ -34,7 +34,6 @@ fn run_probe() -> i32 { let report = learning_mode_windows::probe_security_environment_exports(); println!(" create export = {:?}", report.create); - println!(" launch export = {:?}", report.launch); println!(" close export = {:?}", report.close); match learning_mode_windows::SecurityEnvironmentApi::load() { diff --git a/src/backends/learning_mode/windows/src/lib.rs b/src/backends/learning_mode/windows/src/lib.rs index 8bf6c86d7..14f697b52 100644 --- a/src/backends/learning_mode/windows/src/lib.rs +++ b/src/backends/learning_mode/windows/src/lib.rs @@ -43,7 +43,7 @@ pub use lifecycle::CaptureSession; pub use secenv::{ is_security_environment_api_available, probe_security_environment_exports, ProcessSecurityEnvironment, SecurityEnvironmentApi, SecurityEnvironmentExportReport, - PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, + SecurityEnvironmentStartupInfo, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, }; /// Errors surfaced while loading or invoking the Learning Mode trace API. diff --git a/src/backends/learning_mode/windows/src/lifecycle.rs b/src/backends/learning_mode/windows/src/lifecycle.rs index fd0e7f9bb..b76e03369 100644 --- a/src/backends/learning_mode/windows/src/lifecycle.rs +++ b/src/backends/learning_mode/windows/src/lifecycle.rs @@ -10,8 +10,9 @@ //! 1. `CreateProcessSecurityEnvironment(spec)` → env handle //! 2. `StartLearningModeTrace(env)` → trace handle (**before** the child launches, so no //! early denials are missed) -//! 3. `CreateProcessAsUserInsideSecurityEnvironment(env, …)` → child (**runner's job**; -//! the session exposes the env handle for it via [`CaptureSession::environment`]) +//! 3. attach `env` as `PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT`, then call +//! `CreateProcessW` (**runner's job**; the session exposes the handle via +//! [`CaptureSession::environment`]) //! 4. wait for the child to exit //! 5. `StopLearningModeTrace(trace, outputPath)` → sealed ETL (NULL path discards) //! 6. `CloseProcessSecurityEnvironment(env)` → teardown @@ -91,7 +92,7 @@ impl CaptureSession { } /// The `HPROCESS_SECURITY_ENVIRONMENT` handle to pass to - /// `CreateProcessAsUserInsideSecurityEnvironment`. + /// [`crate::SecurityEnvironmentStartupInfo`]. /// /// # Panics /// Panics only on an internal invariant violation — the environment is present for diff --git a/src/backends/learning_mode/windows/src/secenv.rs b/src/backends/learning_mode/windows/src/secenv.rs index bf4533f2b..610a7ba8d 100644 --- a/src/backends/learning_mode/windows/src/secenv.rs +++ b/src/backends/learning_mode/windows/src/secenv.rs @@ -18,21 +18,15 @@ //! LPCVOID sandboxSpecification, DWORD sandboxSpecificationSize, //! PROCESS_SECURITY_ENVIRONMENT_FLAGS flags, //! HPROCESS_SECURITY_ENVIRONMENT* processSecurityEnvironment); -//! BOOL CreateProcessAsUserInsideSecurityEnvironment( -//! HANDLE userToken, LPCWSTR lpApplicationName, LPWSTR lpCommandLine, -//! DWORD dwCreationFlags, LPCVOID lpEnvironment, LPCWSTR lpCurrentDirectory, -//! LPSTARTUPINFOW lpStartupInfo, HANDLE processSecurityEnvironment, -//! LPPROCESS_INFORMATION lpProcessInformation); //! BOOL CloseProcessSecurityEnvironment(HPROCESS_SECURITY_ENVIRONMENT* processSecurityEnvironment); //! ``` //! //! `sandboxSpecification`/`...Size` is a compiled FlatBuffer sandbox-spec blob (the //! same `"SBOX"` format the BaseContainer runner already builds via `sandbox_spec`); -//! the spec must encode the learning-mode capability. Unlike the RPC one-shot, the -//! launch export returns a real `PROCESS_INFORMATION`, so wxc-exec owns the child -//! handle directly (stdio via `STARTUPINFOW`, wait, and job-object handling behave like -//! the classic path). `Close` tears the environment down; `Detach` (declared by the DLL -//! but not needed here) leaves the child running independently. +//! the spec must encode the learning-mode capability. The environment handle is +//! attached to a normal `CreateProcessW` launch through +//! `PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT`; KernelBase routes that launch +//! through the security environment internally. `Close` tears the environment down. //! //! As with the trace exports, each function is resolved at runtime and tolerates the //! `Experimental_`-prefixed name as a fallback for OS builds that predate the @@ -41,11 +35,14 @@ use std::ffi::c_void; use std::ptr; -use windows::Win32::Foundation::{GetLastError, HANDLE, HMODULE}; +use windows::Win32::Foundation::{GetLastError, ERROR_INSUFFICIENT_BUFFER, HANDLE, HMODULE}; use windows::Win32::System::LibraryLoader::{ GetProcAddress, LoadLibraryExW, LOAD_LIBRARY_SEARCH_SYSTEM32, }; -use windows::Win32::System::Threading::{PROCESS_INFORMATION, STARTUPINFOW}; +use windows::Win32::System::Threading::{ + DeleteProcThreadAttributeList, InitializeProcThreadAttributeList, UpdateProcThreadAttribute, + LPPROC_THREAD_ATTRIBUTE_LIST, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, STARTUPINFOEXW, STARTUPINFOW, +}; use windows_core::{PCSTR, PCWSTR}; use wxc_common::string_util; @@ -75,27 +72,6 @@ type PfnCreateProcessSecurityEnvironment = unsafe extern "system" fn( process_security_environment: *mut HANDLE, ) -> i32; -/// `BOOL CreateProcessAsUserInsideSecurityEnvironment(HANDLE userToken, -/// LPCWSTR lpApplicationName, LPWSTR lpCommandLine, DWORD dwCreationFlags, -/// LPCVOID lpEnvironment, LPCWSTR lpCurrentDirectory, LPSTARTUPINFOW lpStartupInfo, -/// HANDLE processSecurityEnvironment, LPPROCESS_INFORMATION lpProcessInformation)`. -/// -/// `userToken`/`lpApplicationName`/`lpCommandLine`/`lpEnvironment`/`lpCurrentDirectory` -/// are optional; `lpStartupInfo`/`processSecurityEnvironment`/`lpProcessInformation` are -/// required. When `lpEnvironment` is non-null, `dwCreationFlags` must include -/// `CREATE_UNICODE_ENVIRONMENT`. -pub type PfnCreateProcessAsUserInsideSecurityEnvironment = unsafe extern "system" fn( - user_token: HANDLE, - application_name: *const u16, - command_line: *mut u16, - creation_flags: u32, - environment: *const c_void, - current_directory: *const u16, - startup_info: *const STARTUPINFOW, - process_security_environment: HANDLE, - process_information: *mut PROCESS_INFORMATION, -) -> i32; - /// `BOOL CloseProcessSecurityEnvironment(HPROCESS_SECURITY_ENVIRONMENT* processSecurityEnvironment)`. /// /// The export nulls `*processSecurityEnvironment` on success. @@ -160,6 +136,161 @@ impl Drop for ProcessSecurityEnvironment { } } +/// `ProcThreadAttributeSecurityEnvironment` (enum value 35), encoded with the +/// standard `PROC_THREAD_ATTRIBUTE_INPUT` flag. +const PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT: usize = 35 | 0x0002_0000; + +/// Owns the extended startup information required to insert a process into an +/// existing process security environment through `CreateProcessW`. +pub struct SecurityEnvironmentStartupInfo { + storage: Vec, + attribute_list: LPPROC_THREAD_ATTRIBUTE_LIST, + environment_value: Box, + inherited_handles: Vec, + startup_info: STARTUPINFOEXW, +} + +impl SecurityEnvironmentStartupInfo { + /// Add `environment` to an extended copy of `startup_info`. + pub fn new( + mut startup_info: STARTUPINFOW, + environment: HANDLE, + inherited_handles: &[HANDLE], + ) -> Result { + let attribute_count = 1 + u32::from(!inherited_handles.is_empty()); + let mut byte_count = 0usize; + // SAFETY: the documented sizing call uses a null list and writes only + // the required byte count. + let sizing_result = unsafe { + InitializeProcThreadAttributeList(None, attribute_count, None, &mut byte_count) + }; + let sizing_error = last_error(); + if sizing_result.is_ok() || sizing_error != ERROR_INSUFFICIENT_BUFFER.0 || byte_count == 0 { + return Err(LearningModeError::ApiCall { + function: "InitializeProcThreadAttributeList(size)", + code: sizing_error, + }); + } + + let word_count = byte_count.div_ceil(std::mem::size_of::()); + let mut storage = vec![0usize; word_count]; + let attribute_list = LPPROC_THREAD_ATTRIBUTE_LIST(storage.as_mut_ptr().cast()); + + // SAFETY: `storage` is aligned for pointers, has at least + // `byte_count` writable bytes, and remains owned by the returned value. + unsafe { + InitializeProcThreadAttributeList( + Some(attribute_list), + attribute_count, + None, + &mut byte_count, + ) + } + .map_err(|_| LearningModeError::ApiCall { + function: "InitializeProcThreadAttributeList", + code: last_error(), + })?; + + let environment_value = Box::new(environment); + // SAFETY: the attribute list is initialized, and the boxed HANDLE has + // a stable address that outlives every use of the list. + if unsafe { + UpdateProcThreadAttribute( + attribute_list, + 0, + PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT, + Some((&raw const *environment_value).cast()), + std::mem::size_of::(), + None, + None, + ) + } + .is_err() + { + let code = last_error(); + // SAFETY: balances the successful initialization above. + unsafe { DeleteProcThreadAttributeList(attribute_list) }; + return Err(LearningModeError::ApiCall { + function: "UpdateProcThreadAttribute(SecurityEnvironment)", + code, + }); + } + + let inherited_handles = inherited_handles.to_vec(); + if !inherited_handles.is_empty() + && unsafe { + UpdateProcThreadAttribute( + attribute_list, + 0, + PROC_THREAD_ATTRIBUTE_HANDLE_LIST as usize, + Some(inherited_handles.as_ptr().cast()), + std::mem::size_of_val(inherited_handles.as_slice()), + None, + None, + ) + } + .is_err() + { + let code = last_error(); + // SAFETY: balances the successful initialization above. + unsafe { DeleteProcThreadAttributeList(attribute_list) }; + return Err(LearningModeError::ApiCall { + function: "UpdateProcThreadAttribute(HANDLE_LIST)", + code, + }); + } + + startup_info.cb = u32::try_from(std::mem::size_of::()).map_err(|_| { + LearningModeError::ApiCall { + function: "STARTUPINFOEXW size", + code: windows::Win32::Foundation::ERROR_INVALID_PARAMETER.0, + } + })?; + let startup_info = STARTUPINFOEXW { + StartupInfo: startup_info, + lpAttributeList: attribute_list, + }; + + Ok(Self { + storage, + attribute_list, + environment_value, + inherited_handles, + startup_info, + }) + } + + /// Extended startup information to pass to `CreateProcessW`. + #[must_use] + pub fn startup_info(&self) -> &STARTUPINFOEXW { + &self.startup_info + } +} + +impl std::fmt::Debug for SecurityEnvironmentStartupInfo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SecurityEnvironmentStartupInfo") + .field( + "attribute_bytes", + &(self.storage.len() * std::mem::size_of::()), + ) + .field("environment", &self.environment_value) + .field("inherited_handles", &self.inherited_handles) + .finish() + } +} + +impl Drop for SecurityEnvironmentStartupInfo { + fn drop(&mut self) { + if !self.attribute_list.is_invalid() { + // SAFETY: the list was successfully initialized and has not been + // deleted yet. + unsafe { DeleteProcThreadAttributeList(self.attribute_list) }; + self.attribute_list = LPPROC_THREAD_ATTRIBUTE_LIST::default(); + } + } +} + /// Which candidate export name resolved for each function on this machine — a /// diagnostic used by the capability probe to report the exact live surface (plain vs /// `Experimental_`). @@ -167,8 +298,6 @@ impl Drop for ProcessSecurityEnvironment { pub struct SecurityEnvironmentExportReport { /// Resolved name of the create export, if present. pub create: Option<&'static str>, - /// Resolved name of the in-environment launch export, if present. - pub launch: Option<&'static str>, /// Resolved name of the close export, if present. pub close: Option<&'static str>, } @@ -177,7 +306,7 @@ impl SecurityEnvironmentExportReport { /// `true` only when every export required for the 2-phase launch resolved. #[must_use] pub fn is_complete(&self) -> bool { - self.create.is_some() && self.launch.is_some() && self.close.is_some() + self.create.is_some() && self.close.is_some() } } @@ -187,10 +316,6 @@ const CREATE_NAMES: &[&core::ffi::CStr] = &[ c"CreateProcessSecurityEnvironment", c"Experimental_CreateProcessSecurityEnvironment", ]; -const LAUNCH_NAMES: &[&core::ffi::CStr] = &[ - c"CreateProcessAsUserInsideSecurityEnvironment", - c"Experimental_CreateProcessAsUserInsideSecurityEnvironment", -]; const CLOSE_NAMES: &[&core::ffi::CStr] = &[ c"CloseProcessSecurityEnvironment", c"Experimental_CloseProcessSecurityEnvironment", @@ -200,7 +325,6 @@ const CLOSE_NAMES: &[&core::ffi::CStr] = &[ #[derive(Clone, Copy)] pub struct SecurityEnvironmentApi { create: PfnCreateProcessSecurityEnvironment, - launch: PfnCreateProcessAsUserInsideSecurityEnvironment, close: PfnCloseProcessSecurityEnvironment, } @@ -208,7 +332,6 @@ impl std::fmt::Debug for SecurityEnvironmentApi { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("SecurityEnvironmentApi") .field("create", &(self.create as *const ())) - .field("launch", &(self.launch as *const ())) .field("close", &(self.close as *const ())) .finish() } @@ -234,7 +357,6 @@ impl SecurityEnvironmentApi { .map_err(|e| LearningModeError::DllLoad(e.to_string()))?; let create_proc = resolve_any(hmodule, CREATE_NAMES)?; - let launch_proc = resolve_any(hmodule, LAUNCH_NAMES)?; let close_proc = resolve_any(hmodule, CLOSE_NAMES)?; Ok(Self { @@ -242,10 +364,6 @@ impl SecurityEnvironmentApi { unsafe extern "system" fn() -> isize, PfnCreateProcessSecurityEnvironment, >(create_proc), - launch: std::mem::transmute::< - unsafe extern "system" fn() -> isize, - PfnCreateProcessAsUserInsideSecurityEnvironment, - >(launch_proc), close: std::mem::transmute::< unsafe extern "system" fn() -> isize, PfnCloseProcessSecurityEnvironment, @@ -308,20 +426,6 @@ impl SecurityEnvironmentApi { pub fn close(&self, env: &mut ProcessSecurityEnvironment) -> Result<(), LearningModeError> { env.close_with(self.close) } - - /// The resolved in-environment launch export. - /// - /// The stdio/wait/job-object orchestration around - /// `CreateProcessAsUserInsideSecurityEnvironment` belongs to the runner, so the raw - /// function pointer is exposed rather than a fully-wrapped launch here. The runner - /// supplies the `STARTUPINFOW`, receives the real `PROCESS_INFORMATION`, and owns - /// the returned handles. Callers must pass the environment handle from - /// [`ProcessSecurityEnvironment::raw`], and — when supplying an environment block — - /// include `CREATE_UNICODE_ENVIRONMENT` in the creation flags. - #[must_use] - pub fn launch_fn(&self) -> PfnCreateProcessAsUserInsideSecurityEnvironment { - self.launch - } } /// Resolve the first name in `names` that is present in `hmodule`. @@ -379,7 +483,6 @@ pub fn probe_security_environment_exports() -> SecurityEnvironmentExportReport { unsafe { SecurityEnvironmentExportReport { create: first_present(hmodule, CREATE_NAMES), - launch: first_present(hmodule, LAUNCH_NAMES), close: first_present(hmodule, CLOSE_NAMES), } } @@ -459,6 +562,33 @@ mod tests { assert_eq!(PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, 0); } + #[test] + fn startup_info_attaches_security_environment_attribute() { + let startup_info = STARTUPINFOW { + cb: std::mem::size_of::() as u32, + ..Default::default() + }; + let environment = HANDLE(std::ptr::dangling_mut::()); + + match SecurityEnvironmentStartupInfo::new(startup_info, environment, &[]) { + Ok(extended) => { + assert_eq!( + extended.startup_info().StartupInfo.cb as usize, + std::mem::size_of::() + ); + assert!(!extended.startup_info().lpAttributeList.is_invalid()); + } + Err(LearningModeError::ApiCall { code, .. }) + if code == windows::Win32::Foundation::ERROR_NOT_SUPPORTED.0 + || code == windows::Win32::Foundation::ERROR_CALL_NOT_IMPLEMENTED.0 => + { + // Off-feature Windows builds reject the private attribute at + // UpdateProcThreadAttribute time. + } + Err(error) => panic!("unexpected attribute-list error: {error}"), + } + } + #[test] fn failed_close_retains_ownership_and_drop_retries() { CLOSE_CALLS.store(0, Ordering::SeqCst); From a59c0627eff5d8555f13d11b2b87973ae9cd847f Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Fri, 31 Jul 2026 10:02:27 -0700 Subject: [PATCH 14/15] Address capture lifecycle review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a8860a9-bb20-48ef-91a1-3da8e34b92fb --- .../Microsoft.Mxc.Sdk/MxcSandboxProcess.cs | 6 +- .../common/src/base_container_runner.rs | 397 ++++++++++++++---- .../appcontainer/common/src/dispatcher.rs | 43 +- 3 files changed, 347 insertions(+), 99 deletions(-) diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs b/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs index e31d8e44d..464bde7ad 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk/MxcSandboxProcess.cs @@ -228,7 +228,11 @@ private SandboxWaitResult WaitCore(CancellationToken cancellationToken) } if (running == 0) { - return new SandboxWaitResult { ExitCode = exit, TimedOut = false }; + // try_wait is intentionally a non-blocking root-process poll. + // Complete the native wait so descendant termination and any + // backend finalization (including captureDenials) finish before + // the managed Wait/WaitAsync contract returns. + return WaitBlocking(); } // try_wait only reports exited / still-running, never a timeout, and diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 112637118..526888876 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -14,6 +14,7 @@ use std::fmt::Write; use std::io::IsTerminal; use std::path::PathBuf; use std::ptr; +use std::sync::Arc; use learning_mode_windows::{ CaptureSession, LearningModeApi, SecurityEnvironmentApi, SecurityEnvironmentStartupInfo, @@ -202,10 +203,9 @@ const SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX: u64 = 0x0000_0000_0000_0001; const SANDBOX_CAP_FS_DENY: u64 = 0x0000_0000_0000_0002; const CAPTURE_API_AVAILABLE_LOG: &str = "captureDenials: learning-mode trace API available (processmodel.dll)"; -const CAPTURE_API_UNAVAILABLE_REQUIREMENT: &str = - "This feature requires a feature-enabled Windows build that exports the learning-mode trace APIs from processmodel.dll."; const CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON: &str = - "capture initialization failed and the process security environment could not be closed"; + "capture teardown failed and the process security environment may still be live"; +const CLOSE_PROCESS_SECURITY_ENVIRONMENT_API: &str = "CloseProcessSecurityEnvironment"; const CREATE_PROCESS_IN_SANDBOX_API: &str = "Experimental_CreateProcessInSandbox"; const CREATE_PROCESS_IN_SECURITY_ENVIRONMENT_API: &str = "CreateProcessW(PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT)"; @@ -218,6 +218,8 @@ fn is_api_not_implemented(err: u32) -> bool { fn learning_mode_api_not_implemented(error: &learning_mode_windows::LearningModeError) -> bool { match error { + learning_mode_windows::LearningModeError::DllLoad(_) + | learning_mode_windows::LearningModeError::ExportMissing { .. } => true, learning_mode_windows::LearningModeError::ApiCall { code, .. } => { is_api_not_implemented(*code) || *code == ERROR_NOT_SUPPORTED.0 } @@ -229,17 +231,93 @@ fn learning_mode_api_not_implemented(error: &learning_mode_windows::LearningMode } fn learning_mode_cleanup_failed(error: &learning_mode_windows::LearningModeError) -> bool { - matches!( - error, - learning_mode_windows::LearningModeError::CleanupFailed { .. } - ) + match error { + learning_mode_windows::LearningModeError::ApiCall { function, .. } => { + *function == CLOSE_PROCESS_SECURITY_ENVIRONMENT_API + } + learning_mode_windows::LearningModeError::CleanupFailed { primary, cleanup } => { + learning_mode_cleanup_failed(primary) || learning_mode_cleanup_failed(cleanup) + } + _ => false, + } +} + +fn combine_capture_operation_and_cleanup_errors( + primary: learning_mode_windows::LearningModeError, + cleanup: Result<(), learning_mode_windows::LearningModeError>, +) -> learning_mode_windows::LearningModeError { + match cleanup { + Ok(()) => primary, + Err(cleanup) => learning_mode_windows::LearningModeError::CleanupFailed { + primary: Box::new(primary), + cleanup: Box::new(cleanup), + }, + } +} + +trait CaptureSessionOps { + fn environment(&self) -> HANDLE; + fn finish( + self: Box, + output_path: Option<&std::path::Path>, + ) -> Result<(), learning_mode_windows::LearningModeError>; +} + +impl CaptureSessionOps for CaptureSession { + fn environment(&self) -> HANDLE { + self.environment() + } + + fn finish( + self: Box, + output_path: Option<&std::path::Path>, + ) -> Result<(), learning_mode_windows::LearningModeError> { + (*self).finish(output_path) + } +} + +trait CaptureSessionFactory: Send + Sync { + fn begin( + &self, + sandbox_specification: &[u8], + flags: u32, + ) -> Result, learning_mode_windows::LearningModeError>; +} + +struct RealCaptureSessionFactory; + +impl CaptureSessionFactory for RealCaptureSessionFactory { + fn begin( + &self, + sandbox_specification: &[u8], + flags: u32, + ) -> Result, learning_mode_windows::LearningModeError> { + let security_environment_api = SecurityEnvironmentApi::load()?; + let learning_mode_api = LearningModeApi::load()?; + CaptureSession::begin( + security_environment_api, + learning_mode_api, + sandbox_specification, + flags, + ) + .map(|session| Box::new(session) as Box) + } } /// Script runner that uses `Experimental_CreateProcessInSandbox` API /// to launch a sandboxed process. -#[derive(Default)] pub struct BaseContainerRunner { proxy_coordinator: ProxyCoordinator, + capture_factory: Arc, +} + +impl Default for BaseContainerRunner { + fn default() -> Self { + Self { + proxy_coordinator: ProxyCoordinator::default(), + capture_factory: Arc::new(RealCaptureSessionFactory), + } + } } /// SandboxSpec FlatBuffer schema version embedded in every spec payload. @@ -267,9 +345,17 @@ impl BaseContainerRunner { Self::default() } + #[cfg(test)] + fn with_capture_factory(capture_factory: Arc) -> Self { + Self { + proxy_coordinator: ProxyCoordinator::default(), + capture_factory, + } + } + fn cleanup_capture_prelaunch_failure( &mut self, - error: &learning_mode_windows::LearningModeError, + cleanup_error: Option<&learning_mode_windows::LearningModeError>, request: &ExecutionRequest, sid_string: &str, logger: &mut Logger, @@ -278,7 +364,7 @@ impl BaseContainerRunner { // outlive a failed spawn, and the exact error determines whether // profile deletion is safe or recovery tracking must be retained. if request.lifecycle.destroy_on_exit { - if learning_mode_cleanup_failed(error) { + if cleanup_error.is_some_and(learning_mode_cleanup_failed) { sandbox_tracking::mark_cleanup_deferred( sid_string, CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON, @@ -801,37 +887,9 @@ impl BaseContainerRunner { Self::log_sandbox_spec(&spec_bytes, logger); - // Load the security-environment and trace APIs used by captureDenials. - // Both are feature-gated exports from processmodel.dll. let capture_denials = request.policy.capture_denials.clone(); - let mut capture_apis: Option<(SecurityEnvironmentApi, LearningModeApi)> = None; if capture_denials.is_some() { let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: captureDenials"); - match (SecurityEnvironmentApi::load(), LearningModeApi::load()) { - (Ok(security_environment_api), Ok(learning_mode_api)) => { - let _ = writeln!(logger, "{CAPTURE_API_AVAILABLE_LOG}"); - capture_apis = Some((security_environment_api, learning_mode_api)); - } - (security_environment_api, learning_mode_api) => { - let detail = match (&security_environment_api, &learning_mode_api) { - (Err(e), _) => format!("security-environment API: {e}"), - (_, Err(e)) => format!("learning-mode trace API: {e}"), - _ => "unknown".to_string(), - }; - let msg = format!( - "captureDenials was requested but the learning-mode trace API is not \ - available on this OS build ({detail}). {CAPTURE_API_UNAVAILABLE_REQUIREMENT}" - ); - let _ = writeln!(logger, "Error: {msg}"); - return Err(ScriptResponse { - exit_code: -1, - error_message: msg.clone(), - standard_err: msg, - failure_phase: FailurePhase::BackendUnavailable, - ..Default::default() - }); - } - } } // Resolve the ETL output path: caller-specified when provided, else a @@ -1155,18 +1213,16 @@ impl BaseContainerRunner { // normal CreateProcessW call via PROC_THREAD_ATTRIBUTE_SECURITY_ENVIRONMENT. // On any early return below, `capture_session` drops and its Drop // discards the trace and closes the environment (no broker leak). - let mut capture_session: Option = None; - if let Some((se_api, lm_api)) = capture_apis { - match CaptureSession::begin( - se_api, - lm_api, - &spec_bytes, - PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, - ) { + let mut capture_session: Option> = None; + if capture_denials.is_some() { + match self + .capture_factory + .begin(&spec_bytes, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE) + { Ok(session) => { let _ = writeln!( logger, - "captureDenials: security environment created and trace started" + "{CAPTURE_API_AVAILABLE_LOG}; security environment and trace started" ); capture_session = Some(session); } @@ -1178,7 +1234,7 @@ impl BaseContainerRunner { } else { FailurePhase::LaunchFailed }; - self.cleanup_capture_prelaunch_failure(&e, &request, &sid_string, logger); + self.cleanup_capture_prelaunch_failure(Some(&e), &request, &sid_string, logger); return Err(ScriptResponse { exit_code: -1, error_message: msg.clone(), @@ -1208,7 +1264,12 @@ impl BaseContainerRunner { &inherited_handles, ) { Ok(startup) => startup, - Err(error) => { + Err(primary) => { + let cleanup = capture_session + .take() + .map(|session| session.finish(None)) + .unwrap_or(Ok(())); + let error = combine_capture_operation_and_cleanup_errors(primary, cleanup); let msg = format!( "captureDenials: failed to attach the process security environment: {error}" ); @@ -1218,7 +1279,12 @@ impl BaseContainerRunner { } else { FailurePhase::LaunchFailed }; - self.cleanup_capture_prelaunch_failure(&error, &request, &sid_string, logger); + self.cleanup_capture_prelaunch_failure( + Some(&error), + &request, + &sid_string, + logger, + ); return Err(ScriptResponse { exit_code: -1, error_message: msg.clone(), @@ -1272,6 +1338,7 @@ impl BaseContainerRunner { }; if success == 0 { + let err = last_error.unwrap_or_else(|| unsafe { GetLastError() }); // Clean up any partially-populated handles from the failed API call. unsafe { if !pi.hProcess.is_invalid() { @@ -1281,9 +1348,19 @@ impl BaseContainerRunner { let _ = CloseHandle(pi.hThread); } } - // The OS may have created the AppContainer profile before failing, - // so run the same cleanup logic used on normal exit. - if request.lifecycle.destroy_on_exit { + let capture_cleanup_error = capture_session + .take() + .and_then(|session| session.finish(None).err()); + if capture_denials.is_some() { + self.cleanup_capture_prelaunch_failure( + capture_cleanup_error.as_ref(), + &request, + &sid_string, + logger, + ); + } else if request.lifecycle.destroy_on_exit { + // The OS may have created the AppContainer profile before + // failing, so run the same cleanup logic used on normal exit. run_sandbox_cleanup( &identity, &sid_string, @@ -1295,14 +1372,19 @@ impl BaseContainerRunner { // // Diagnose the launch failure (FailurePhase::LaunchFailed). // - let err = last_error.unwrap_or_else(|| unsafe { GetLastError() }); let diag = diagnose_create_process_failure( err.0, &request.script_code, &request.policy.readonly_paths, ); - let extended_error = format!("{launch_api_name} failed: {err:?}"); + let mut extended_error = format!("{launch_api_name} failed: {err:?}"); + if let Some(cleanup_error) = capture_cleanup_error { + let _ = write!( + extended_error, + "; capture teardown also failed: {cleanup_error}" + ); + } let _ = writeln!(logger, "Error: {extended_error}"); let _ = writeln!( @@ -1385,7 +1467,17 @@ impl BaseContainerRunner { let _ = CloseHandle(pi.hProcess); let _ = CloseHandle(pi.hThread); } - if request.lifecycle.destroy_on_exit { + let capture_cleanup_error = capture_session + .take() + .and_then(|session| session.finish(None).err()); + if capture_denials.is_some() { + self.cleanup_capture_prelaunch_failure( + capture_cleanup_error.as_ref(), + &request, + &sid_string, + logger, + ); + } else if request.lifecycle.destroy_on_exit { run_sandbox_cleanup( &identity, &sid_string, @@ -1394,17 +1486,26 @@ impl BaseContainerRunner { ); sandbox_tracking::unregister_ctrl_c_cleanup(); } - self.proxy_coordinator.stop(logger); + if capture_denials.is_none() { + self.proxy_coordinator.stop(logger); + } const JOB_SETUP_FAILED_MSG: &str = "BaseContainer sandbox could not be placed in a job object, so it \ could not be reliably terminated; the launch was rejected to \ avoid running an uncontainable sandbox."; + let mut extended_error = format!("BaseContainer job-object setup failed: {e}"); + if let Some(cleanup_error) = capture_cleanup_error { + let _ = write!( + extended_error, + "; capture teardown also failed: {cleanup_error}" + ); + } return Err(ScriptResponse { exit_code: -1, error_message: JOB_SETUP_FAILED_MSG.to_string(), standard_err: JOB_SETUP_FAILED_MSG.to_string(), - extended_error: format!("BaseContainer job-object setup failed: {e}"), + extended_error, failure_phase: FailurePhase::LaunchFailed, ..Default::default() }); @@ -1470,7 +1571,7 @@ struct BaseChild { /// Live learning-mode capture session (`Some` only when `captureDenials` /// is configured and the OS API is available). Sealed in `run_teardown` /// after the child exits. - capture_session: Option, + capture_session: Option>, /// Resolved ETL output path for the capture session (caller-specified or a /// managed per-run temp file). `Some` iff `capture_session` is `Some`. capture_output_path: Option, @@ -1564,7 +1665,7 @@ struct BaseContainerSandboxProcess { teardown_result: Option>, /// Live learning-mode capture session, moved from the `BaseChild`. Sealed /// in `run_teardown` once the child has exited and been reaped. - capture_session: Option, + capture_session: Option>, /// Resolved ETL output path for the capture session. capture_output_path: Option, } @@ -1618,6 +1719,7 @@ impl BaseContainerSandboxProcess { // security environment. The output-file path is surfaced on stderr so a // caller can locate the denials (the full NDJSON contract is finalized // in the output/consume stage). + let mut defer_capture_cleanup = false; let capture_result = if let Some(session) = self.capture_session.take() { let output_path = self.capture_output_path.take(); match session.finish(output_path.as_deref()) { @@ -1627,21 +1729,32 @@ impl BaseContainerSandboxProcess { } Ok(()) } - Err(error) => Err(std::io::Error::other(format!( - "captureDenials failed to finalize the denial capture: {error}" - ))), + Err(error) => { + defer_capture_cleanup = learning_mode_cleanup_failed(&error); + Err(std::io::Error::other(format!( + "captureDenials failed to finalize the denial capture: {error}" + ))) + } } } else { Ok(()) }; if self.destroy_on_exit { - run_sandbox_cleanup( - &self.identity, - &self.sid_string, - self.proxy_enabled, - &mut logger, - ); + if defer_capture_cleanup { + sandbox_tracking::mark_cleanup_deferred( + &self.sid_string, + CAPTURE_SECURITY_ENVIRONMENT_CLEANUP_DEFERRED_REASON, + &mut logger, + ); + } else { + run_sandbox_cleanup( + &self.identity, + &self.sid_string, + self.proxy_enabled, + &mut logger, + ); + } sandbox_tracking::unregister_ctrl_c_cleanup(); } self.proxy_coordinator.stop(&mut logger); @@ -1697,15 +1810,10 @@ impl SandboxProcess for BaseContainerSandboxProcess { if unsafe { GetExitCodeProcess(self.process.get(), &mut code) }.is_err() { return Err(std::io::Error::other("GetExitCodeProcess failed")); } - if self.capture_session.is_none() && self.teardown_result.is_none() { - return Ok(Some(code as i32)); - } - // A terminal observation must not become visible before - // captureDenials is sealed. This is the path used by polling - // SDK waits, so finalize synchronously before reporting exit. - self.terminate_and_reap(); - let teardown_result = self.run_teardown(); - combine_process_and_teardown_results(Ok(code as i32), teardown_result).map(Some) + // Keep polling non-blocking and independent of captureDenials. + // `wait()` or `Drop` owns descendant termination and capture + // finalization after the root exit becomes observable. + Ok(Some(code as i32)) } WAIT_TIMEOUT => Ok(None), _ => Err(std::io::Error::other("WaitForSingleObject failed")), @@ -1839,9 +1947,58 @@ mod tests { use super::*; use crate::job_object::to_job_object_uilimit_mask; use sandbox_spec::base_container_layout; + use std::sync::atomic::{AtomicUsize, Ordering}; use wxc_common::models::{ClipboardPolicy, ProxyConfig, UiPolicy}; use wxc_common::ui_policy::EffectiveUiRestrictions; + struct FakeCaptureSession { + finish_error: Option<(&'static str, u32)>, + finish_calls: Arc, + } + + impl CaptureSessionOps for FakeCaptureSession { + fn environment(&self) -> HANDLE { + HANDLE(std::ptr::dangling_mut()) + } + + fn finish( + self: Box, + _output_path: Option<&std::path::Path>, + ) -> Result<(), learning_mode_windows::LearningModeError> { + self.finish_calls.fetch_add(1, Ordering::SeqCst); + match self.finish_error { + Some((function, code)) => { + Err(learning_mode_windows::LearningModeError::ApiCall { function, code }) + } + None => Ok(()), + } + } + } + + struct FakeCaptureFactory { + begin_error: Option<(&'static str, u32)>, + finish_error: Option<(&'static str, u32)>, + begin_calls: AtomicUsize, + finish_calls: Arc, + } + + impl CaptureSessionFactory for FakeCaptureFactory { + fn begin( + &self, + _sandbox_specification: &[u8], + _flags: u32, + ) -> Result, learning_mode_windows::LearningModeError> { + self.begin_calls.fetch_add(1, Ordering::SeqCst); + if let Some((function, code)) = self.begin_error { + return Err(learning_mode_windows::LearningModeError::ApiCall { function, code }); + } + Ok(Box::new(FakeCaptureSession { + finish_error: self.finish_error, + finish_calls: Arc::clone(&self.finish_calls), + })) + } + } + fn expected_mask(r: EffectiveUiRestrictions) -> u64 { to_job_object_uilimit_mask(&r) as u64 } @@ -1896,6 +2053,17 @@ mod tests { code: 87, }; assert!(!learning_mode_api_not_implemented(&ordinary)); + + assert!(learning_mode_api_not_implemented( + &LearningModeError::ExportMissing { + api: "Learning Mode trace", + export: "StartLearningModeTrace", + detail: "not found".to_string(), + } + )); + assert!(learning_mode_api_not_implemented( + &LearningModeError::DllLoad("missing processmodel.dll".to_string()) + )); } #[test] @@ -1914,6 +2082,12 @@ mod tests { }; assert!(learning_mode_cleanup_failed(&error)); + let close_only = LearningModeError::ApiCall { + function: CLOSE_PROCESS_SECURITY_ENVIRONMENT_API, + code: 5, + }; + assert!(learning_mode_cleanup_failed(&close_only)); + let ordinary = LearningModeError::ApiCall { function: "StartLearningModeTrace", code: 87, @@ -1921,6 +2095,73 @@ mod tests { assert!(!learning_mode_cleanup_failed(&ordinary)); } + #[test] + fn prelaunch_failure_preserves_capture_cleanup_error() { + use learning_mode_windows::LearningModeError; + + let error = combine_capture_operation_and_cleanup_errors( + LearningModeError::ApiCall { + function: "UpdateProcThreadAttribute(SecurityEnvironment)", + code: 87, + }, + Err(LearningModeError::ApiCall { + function: CLOSE_PROCESS_SECURITY_ENVIRONMENT_API, + code: 5, + }), + ); + + assert!(matches!(error, LearningModeError::CleanupFailed { .. })); + assert!(learning_mode_cleanup_failed(&error)); + assert!(error.to_string().contains("UpdateProcThreadAttribute")); + assert!(error + .to_string() + .contains(CLOSE_PROCESS_SECURITY_ENVIRONMENT_API)); + } + + #[test] + fn capture_factory_injects_begin_failure() { + let factory = Arc::new(FakeCaptureFactory { + begin_error: Some(("StartLearningModeTrace", 5)), + finish_error: None, + begin_calls: AtomicUsize::new(0), + finish_calls: Arc::new(AtomicUsize::new(0)), + }); + let runner = BaseContainerRunner::with_capture_factory(factory.clone()); + + let error = match runner + .capture_factory + .begin(&[], PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE) + { + Ok(_) => panic!("fake begin must fail"), + Err(error) => error, + }; + + assert!(error.to_string().contains("StartLearningModeTrace")); + assert_eq!(factory.begin_calls.load(Ordering::SeqCst), 1); + assert_eq!(factory.finish_calls.load(Ordering::SeqCst), 0); + } + + #[test] + fn capture_factory_injects_finish_failure_once() { + let factory = Arc::new(FakeCaptureFactory { + begin_error: None, + finish_error: Some((CLOSE_PROCESS_SECURITY_ENVIRONMENT_API, 5)), + begin_calls: AtomicUsize::new(0), + finish_calls: Arc::new(AtomicUsize::new(0)), + }); + let runner = BaseContainerRunner::with_capture_factory(factory.clone()); + let session = runner + .capture_factory + .begin(&[], PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE) + .expect("fake begin"); + + let error = session.finish(None).expect_err("fake finish must fail"); + + assert!(learning_mode_cleanup_failed(&error)); + assert_eq!(factory.begin_calls.load(Ordering::SeqCst), 1); + assert_eq!(factory.finish_calls.load(Ordering::SeqCst), 1); + } + #[test] fn decode_create_capability_table() { let cap = SANDBOX_CAP_CREATE_PROCESS_IN_SANDBOX; diff --git a/src/backends/appcontainer/common/src/dispatcher.rs b/src/backends/appcontainer/common/src/dispatcher.rs index d3e67beee..24b9fda76 100644 --- a/src/backends/appcontainer/common/src/dispatcher.rs +++ b/src/backends/appcontainer/common/src/dispatcher.rs @@ -145,6 +145,9 @@ pub enum DispatchError { }, /// AppContainer SID derivation failed. Sid(WxcError), + /// captureDenials requires the native BaseContainer tier and cannot + /// proceed through an AppContainer fallback. + CaptureDenialsUnsupported { tier: IsolationTier }, } impl std::fmt::Display for DispatchError { @@ -171,6 +174,12 @@ impl std::fmt::Display for DispatchError { ), DispatchError::Dacl { error, .. } => write!(f, "Failed to apply DACL ACEs: {error}"), DispatchError::Sid(e) => write!(f, "Failed to derive AppContainer SID: {e}"), + DispatchError::CaptureDenialsUnsupported { tier } => write!( + f, + "captureDenials requires the native BaseContainer backend; \ + the selected fallback tier '{}' does not support denial capture.", + tier.as_str() + ), } } } @@ -331,19 +340,9 @@ fn select_backend_with_fallback( > { let decision = fallback_detector::detect(&request.policy, /*prefer_bc=*/ true)?; if request.policy.capture_denials.is_some() && decision.tier != IsolationTier::BaseContainer { - let filesystem_mode = match decision.tier { - IsolationTier::AppContainerBfs => FilesystemMode::Bfs, - IsolationTier::AppContainerDacl => FilesystemMode::Dacl, - IsolationTier::BaseContainer => unreachable!(), - }; - return Ok(( - SelectedBackend::AppContainer(AppContainerScriptRunner::with_filesystem_mode( - filesystem_mode, - )), - None, - decision.tier, - decision.warnings, - )); + return Err(DispatchError::CaptureDenialsUnsupported { + tier: decision.tier, + }); } let (backend, dacl_manager): (SelectedBackend, Option) = match decision.tier { @@ -690,18 +689,22 @@ mod tests { } #[test] - fn capture_denials_defers_fallback_rejection_without_dacl_setup() { + fn capture_denials_rejects_fallback_before_backend_or_dacl_setup() { let _g = ForceTierGuard::set("appcontainer-dacl"); let (mut policy, _tmp) = policy_with_rw_temp(); policy.capture_denials = Some(Default::default()); let req = test_request(policy); - let dispatched = dispatch_with_fallback(&req).expect("validation should own the rejection"); - assert!(matches!(dispatched.tier, IsolationTier::AppContainerDacl)); - assert!( - !dispatched.has_dacl_guard(), - "capture fallback must not apply host DACLs before validation" - ); + let error = match dispatch_with_fallback(&req) { + Ok(_) => panic!("capture fallback must be rejected"), + Err(error) => error, + }; + assert!(matches!( + error, + DispatchError::CaptureDenialsUnsupported { + tier: IsolationTier::AppContainerDacl + } + )); } #[test] fn dispatch_fallback_disabled_errors() { From 4ca8575e95b87f53b7635f1182ea1458a65f45bf Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Fri, 31 Jul 2026 10:35:22 -0700 Subject: [PATCH 15/15] Prevent capture environment inheritance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a8860a9-bb20-48ef-91a1-3da8e34b92fb --- .../common/src/appcontainer_runner.rs | 4 ++-- .../common/src/base_container_runner.rs | 21 +++++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/backends/appcontainer/common/src/appcontainer_runner.rs b/src/backends/appcontainer/common/src/appcontainer_runner.rs index 03b052965..1539c830e 100644 --- a/src/backends/appcontainer/common/src/appcontainer_runner.rs +++ b/src/backends/appcontainer/common/src/appcontainer_runner.rs @@ -64,7 +64,7 @@ const PROXY_VAR_NAMES: &[&str] = &["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "ALL /// Serialize `KEY=VALUE` pairs into a double-null-terminated UTF-16 environment block. /// /// Entries are sorted case-insensitively by key as required by `CreateProcessW`. -fn encode_env_block(entries: &[(String, String)]) -> Vec { +pub(crate) fn encode_env_block(entries: &[(String, String)]) -> Vec { let mut sorted: Vec<&(String, String)> = entries.iter().collect(); sorted.sort_by(|(a, _), (b, _)| a.to_ascii_uppercase().cmp(&b.to_ascii_uppercase())); @@ -85,7 +85,7 @@ fn encode_env_block(entries: &[(String, String)]) -> Vec { /// Calls `CreateEnvironmentBlock` with `bInherit = FALSE` so that only the /// system/user profile variables are included (no process-level vars leak in). /// Returns the entries as `(key, value)` pairs. -fn create_default_env_entries() -> Result, WxcError> { +pub(crate) fn create_default_env_entries() -> Result, WxcError> { unsafe { let mut token = HANDLE::default(); OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 526888876..4300574db 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -1138,13 +1138,22 @@ impl BaseContainerRunner { let mut pi: PROCESS_INFORMATION = unsafe { std::mem::zeroed() }; // Environment block for the sandboxed child. - // If the caller specified explicit env vars, use only those. - // Otherwise, pass NULL to let the OS provide the default environment - // for the sandbox (CreateProcessInSandbox handles this internally). + // Explicit variables are always isolated from the parent environment. + // The one-shot API supplies its own default when this is NULL, but the + // attribute-based CreateProcessW capture path must receive an explicit + // clean block or it would inherit all wxc-exec process variables. let env_block: Option> = if request.env.is_empty() { - // TODO: consider calling CreateEnvironmentBlock(NULL, FALSE) here - // for a cleansed default env if the OS API doesn't do it for us. - None + if capture_denials.is_some() { + let entries = + crate::appcontainer_runner::create_default_env_entries().map_err(|error| { + ScriptResponse::error(&format!( + "captureDenials failed to create a clean child environment: {error}" + )) + })?; + Some(crate::appcontainer_runner::encode_env_block(&entries)) + } else { + None + } } else { Some(encode_env_block(&request.env)) };