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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 64 additions & 8 deletions src/backends/appcontainer/common/src/appcontainer_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use std::ptr;
use std::sync::Arc;

use windows::Win32::Foundation::{
CloseHandle, GetLastError, LocalFree, SetHandleInformation, ERROR_ALREADY_EXISTS, HANDLE,
HANDLE_FLAG_INHERIT, HLOCAL, WAIT_OBJECT_0, WAIT_TIMEOUT,
CloseHandle, GetLastError, LocalFree, SetHandleInformation, ERROR_ACCESS_DISABLED_BY_POLICY,
ERROR_ALREADY_EXISTS, HANDLE, HANDLE_FLAG_INHERIT, HLOCAL, WAIT_OBJECT_0, WAIT_TIMEOUT,
};
use windows::Win32::Security::Authorization::ConvertSidToStringSidW;
use windows::Win32::Security::Isolation::{
Expand All @@ -34,6 +34,7 @@ use windows_core::{PCWSTR, PWSTR};
use crate::capture_output;
use crate::guarded_capture::{GuardedCaptureFactory, GuardedCaptureSession};
use crate::job_object::UiJobObject;
use crate::launch_diagnostics::diagnose_create_process_failure;
use crate::process_mitigation;
use wxc_common::error::WxcError;
use wxc_common::logger::Logger;
Expand Down Expand Up @@ -67,6 +68,28 @@ const PROCESS_CREATION_ALL_APPLICATION_PACKAGES_OPT_OUT: u32 = 1;
/// Proxy-related env var names to strip/override when building the child env block.
const PROXY_VAR_NAMES: &[&str] = &["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "ALL_PROXY"];

fn create_process_failure(
err: &windows_core::Error,
command_line: &str,
readonly_paths: &[String],
working_directory: &str,
) -> WxcError {
let message = if err.code() == ERROR_ACCESS_DISABLED_BY_POLICY.to_hresult() {
diagnose_create_process_failure(
ERROR_ACCESS_DISABLED_BY_POLICY.0,
command_line,
readonly_paths,
)
.message
} else {
format!("CreateProcessW failed: {err}")
};

WxcError::Process(format!(
"{message} (working directory: {working_directory})"
))
}

/// Serialize `KEY=VALUE` pairs into a double-null-terminated UTF-16 environment block.
///
/// Entries are sorted case-insensitively by key as required by `CreateProcessW`.
Expand Down Expand Up @@ -1063,11 +1086,12 @@ impl AppContainerScriptRunner {
)
}
.map_err(|err| {
WxcError::Process(format!(
"CreateProcessW failed: {} (working directory: {})",
err,
working_directory.describe()
))
create_process_failure(
&err,
&request.script_code,
&request.policy.readonly_paths,
&working_directory.describe(),
)
})?;

logger.log_line(&format!(
Expand Down Expand Up @@ -2144,11 +2168,13 @@ mod tests {
// ---- validate_runner: unsupported policy fields surface as errors. ----

use super::{
AppContainerScriptRunner, FilesystemMode, CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG,
create_process_failure, AppContainerScriptRunner, FilesystemMode,
CAPTURE_DENIALS_FALLBACK_UNSUPPORTED_MSG,
};
use crate::guarded_capture::{GuardedCaptureFactory, GuardedCaptureSession};
use learning_mode_core::AnalysisResult;
use std::sync::Arc;
use windows::Win32::Foundation::{ERROR_ACCESS_DISABLED_BY_POLICY, ERROR_CALL_NOT_IMPLEMENTED};
use wxc_common::models::{ExecutionRequest, FailurePhase};
use wxc_common::sandbox_process::SandboxBackend;

Expand Down Expand Up @@ -2180,6 +2206,36 @@ mod tests {
}
}

#[test]
fn appcontainer_policy_block_uses_launch_diagnostic() {
let err = windows_core::Error::from_hresult(ERROR_ACCESS_DISABLED_BY_POLICY.to_hresult());
let mapped = create_process_failure(
&err,
r#""C:\Program Files\PowerShell\7\pwsh.exe" -NoProfile"#,
&[],
r"C:\work",
);
let message = mapped.to_string();

assert!(message.contains("IT-managed policy rule"));
assert!(message.contains("1260"));
assert!(message.contains("system administrator"));
assert!(message.contains(r"working directory: C:\work"));
assert!(!message.contains("readonlyPaths"));
}

#[test]
fn appcontainer_other_win32_error_preserves_create_process_message() {
let err = windows_core::Error::from_hresult(ERROR_CALL_NOT_IMPLEMENTED.to_hresult());
let mapped = create_process_failure(&err, "cmd.exe", &[], r"C:\work");
let message = mapped.to_string();

assert!(message.contains("CreateProcessW failed"));
assert!(message.contains(r"working directory: C:\work"));
assert!(!message.contains("BaseContainer"));
assert!(!message.contains("Experimental_CreateProcessInSandbox"));
}

#[test]
fn validate_runner_rejects_denied_paths_in_bfs_mode() {
let runner = AppContainerScriptRunner::with_filesystem_mode(FilesystemMode::Bfs);
Expand Down
28 changes: 27 additions & 1 deletion src/backends/appcontainer/common/src/launch_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ pub fn diagnose_create_process_failure(
command_line: &str,
readonly_paths: &[String],
) -> LaunchDiagnostic {
if win32_error == ERROR_ACCESS_DISABLED_BY_POLICY.0 {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6d70474. AppContainer CreateProcessW failures now decode HRESULT_FROM_WIN32 values and route them through the shared launch diagnostic while preserving working-directory context. Added caller-level regression coverage; cargo fmt, all 236 appcontainer_common lib tests, and targeted clippy pass.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refined in 12e1c07: AppContainer delegates to the shared diagnostic only for ERROR_ACCESS_DISABLED_BY_POLICY (1260). All other CreateProcessW failures retain the existing AppContainer error text. Added a regression test using ERROR_CALL_NOT_IMPLEMENTED to ensure BaseContainer-specific guidance is not emitted. Formatting, all 237 appcontainer_common lib tests, and targeted clippy pass.

return LaunchDiagnostic {
kind: "launch_blocked_by_policy",
message:
"Windows blocked the sandboxed process launch because of an IT-managed policy rule \
(ERROR_ACCESS_DISABLED_BY_POLICY, 1260). Contact your system administrator \
to allow the target executable to run in an MXC sandbox."
.to_string(),
};
}

// Check for feature-not-enabled (velocity keys).
if win32_error == ERROR_CALL_NOT_IMPLEMENTED.0 || win32_error == E_NOTIMPL.0 as u32 {
return diagnose_api_not_implemented();
Expand Down Expand Up @@ -108,7 +119,8 @@ const REQUIRED_VELOCITY_KEYS: &[(u32, &str)] = &[
// flow through `u32`, which matches the existing public surface of
// this module (`diagnose_create_process_failure` takes `u32`).
use windows::Win32::Foundation::{
ERROR_CALL_NOT_IMPLEMENTED, ERROR_NOT_SUPPORTED, E_NOTIMPL, STATUS_DLL_INIT_FAILED,
ERROR_ACCESS_DISABLED_BY_POLICY, ERROR_CALL_NOT_IMPLEMENTED, ERROR_NOT_SUPPORTED, E_NOTIMPL,
STATUS_DLL_INIT_FAILED,
};

// -- Internal heuristics -----------------------------------------------------
Expand Down Expand Up @@ -331,6 +343,20 @@ mod tests {
assert_eq!(diag.kind, "feature_not_enabled");
}

#[test]
fn policy_block_takes_priority_over_executable_heuristics() {
let diag = diagnose_create_process_failure(
ERROR_ACCESS_DISABLED_BY_POLICY.0,
r#""C:\Program Files\PowerShell\7\pwsh.exe" -NoProfile"#,
&[],
);
assert_eq!(diag.kind, "launch_blocked_by_policy");
assert!(diag.message.contains("IT-managed"));
assert!(diag.message.contains("1260"));
assert!(diag.message.contains("system administrator"));
assert!(!diag.message.contains("readonlyPaths"));
}

#[test]
fn packaged_app_detected_from_command_line() {
let cmd =
Expand Down