From 2e8da4bbf59d2f47c5eec717cc6fd5ad387f81f4 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 18 Sep 2026 10:27:15 -0300 Subject: [PATCH 1/2] fix(policy): stop granting the system drive when pwsh.exe is on PATH The policy-discovery helpers returned the system-drive root as a read-only grant whenever pwsh.exe was found in any PATH directory. That grant is recursive, so any sandbox seeded from these helpers could read the whole volume: ~/.ssh, ~/.aws/credentials, .npmrc and .netrc tokens, browser profiles, and other users' profile directories. The grant existed to satisfy a metadata-only need: pwsh.exe before 7.7 stats the drive root during startup. That need is already served host-wide, and narrowly, by `wxc-host-prep prepare-system-drive`, which stamps metadata-only, non-inheriting ACEs on the root. Removing the grant costs nothing else. The directory that holds pwsh.exe is by definition a PATH directory, so the ordinary discovery filter already grants $PSHOME read-only. Also removes the `missing_root_readonly` launch diagnostic. Its trigger was "the drive root is absent from readonlyPaths", which is now true for every pwsh.exe run, so an ordinary script error would have been reported as `missing_filesystem_access`. Its remediation text also told users to add the root to readonlyPaths by hand, recreating the vulnerability. The correctly gated form of that hint already exists in `fallback_detector`, keyed on the real host-prep DACL state and naming pwsh.exe explicitly. The two shipped pwsh configs granted the drive root as well. As the canonical worked examples of running pwsh under MXC they taught the very pattern this change removes, and neither needs it. Deliberately not moved to processContainer.filesystem.enumeratePaths: that field is backend-specific, requires schema 0.9.0-alpha exactly, is rejected outright on hosts without PSEC 1.1 PSE_SUPPORT_FS_ENUMERATE, and cannot combine with leastPrivilege. A cross-backend discovery default must never make a run fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65848df5-ce23-4b2a-8697-2f7f6fafe106 Signed-off-by: Carlos Alexandro Becker --- sdk/node/src/policy.ts | 32 +++--- sdk/node/tests/unit/policy.test.ts | 21 +++- .../common/src/launch_diagnostics.rs | 80 +++------------ src/core/mxc_engine/src/policy.rs | 99 +++++++++++++------ tests/configs/pwsh_setlocation.json | 3 - tests/examples/08_pwsh.json | 3 - 6 files changed, 122 insertions(+), 116 deletions(-) diff --git a/sdk/node/src/policy.ts b/sdk/node/src/policy.ts index bdbcb3cfa..355466ddd 100644 --- a/sdk/node/src/policy.ts +++ b/sdk/node/src/policy.ts @@ -193,10 +193,19 @@ function deduplicatePaths(paths: string[]): string[] { * Check whether PowerShell (pwsh.exe) is available on the machine by scanning * the supplied PATH directories for a `pwsh.exe` binary. * - * When PowerShell is found, return a policy fragment with: - * - `C:\` in `readonlyPaths` — pwsh.exe enumerates the drive root on startup. - * - The PSReadLine history directory in `readwritePaths` so the PSReadLine - * module can persist command history. + * When PowerShell is found, return a policy fragment with the PSReadLine + * history directory in `readwritePaths` so the PSReadLine module can persist + * command history. + * + * Deliberately grants no read access to the system-drive root. `pwsh.exe` + * before 7.7 stats the root at startup, but that is a *metadata-only* need, and + * a recursive `readonlyPaths` grant on `C:\` would expose every file on the + * volume (`~/.ssh`, `~/.aws/credentials`, other users' profiles) to satisfy it. + * The narrow, host-wide answer is `wxc-host-prep prepare-system-drive`, which + * stamps non-inheriting metadata ACEs on the root. `$PSHOME` itself needs no + * special handling here: the directory holding `pwsh.exe` is by definition a + * PATH directory, so {@link getAvailableToolsPolicy} already grants it + * read-only. * * On non-Windows platforms or when pwsh.exe is not found on PATH, returns an * empty policy. @@ -224,9 +233,6 @@ function getPowerShellPolicy( return { readonlyPaths: [], readwritePaths: [] }; } - const systemDrive = process.env["SystemDrive"] || 'C:'; - const systemRoot = systemDrive + "\\"; - const readonlyPaths: string[] = [systemRoot]; const readwritePaths: string[] = []; const userProfile = env['USERPROFILE']; @@ -237,7 +243,7 @@ function getPowerShellPolicy( readwritePaths.push(psReadLineDir); } - return { readonlyPaths, readwritePaths }; + return { readonlyPaths: [], readwritePaths }; } // --------------------------------------------------------------------------- @@ -257,10 +263,10 @@ function getPowerShellPolicy( * already grant access to `ALL_APPLICATION_PACKAGES` are removed because * AppContainer processes can see them without explicit brokering. * - * Additionally, if PowerShell (`pwsh.exe`) is found on PATH, the drive root - * (`C:\`) is added to `readonlyPaths` and the PSReadLine history directory - * is added to `readwritePaths` so that interactive PowerShell sessions work - * correctly inside the container. + * Additionally, if PowerShell (`pwsh.exe`) is found on PATH, the PSReadLine + * history directory is added to `readwritePaths` so that interactive PowerShell + * sessions can persist command history. `$PSHOME` needs no special case: it is + * a PATH directory and so is already covered by the filters above. * * @param env - Environment variable map. Defaults to `process.env`. * @param options - Filtering options. @@ -307,7 +313,7 @@ export function getAvailableToolsPolicy( const pwshPolicy = getPowerShellPolicy(pathDirs, environment); return { - readonlyPaths: deduplicatePaths([...filtered, ...pwshPolicy.readonlyPaths]), + readonlyPaths: deduplicatePaths(filtered), readwritePaths: deduplicatePaths([...pwshPolicy.readwritePaths]), }; } diff --git a/sdk/node/tests/unit/policy.test.ts b/sdk/node/tests/unit/policy.test.ts index 7432d996b..2ccfb06b4 100644 --- a/sdk/node/tests/unit/policy.test.ts +++ b/sdk/node/tests/unit/policy.test.ts @@ -46,14 +46,25 @@ describe('getAvailableToolsPolicy - PowerShell discovery', () => { } }); - it('should add system root to readonlyPaths when pwsh.exe is on PATH', { skip: isLinux }, () => { + it('should never add the drive root to readonlyPaths when pwsh.exe is on PATH', { skip: isLinux }, () => { mockWindows(); const pwshDir = createFakePwshDir(); const env = { PATH: pwshDir, USERPROFILE: 'C:\\Users\\TestUser' }; const result = getAvailableToolsPolicy(env); assert.ok( - result.readonlyPaths.some(p => /^[a-z]:\\$/i.test(p)), - 'System root (e.g. C:\\) should be in readonlyPaths when pwsh.exe is on PATH', + !result.readonlyPaths.some(p => /^[a-z]:\\$/i.test(p)), + 'Finding pwsh.exe must not grant a recursive read of the whole volume', + ); + }); + + it('should still grant $PSHOME read-only via PATH discovery', { skip: isLinux }, () => { + mockWindows(); + const pwshDir = createFakePwshDir(); + const env = { PATH: pwshDir, USERPROFILE: 'C:\\Users\\TestUser' }; + const result = getAvailableToolsPolicy(env); + assert.ok( + result.readonlyPaths.some(p => p.toLowerCase() === pwshDir.toLowerCase()), + 'The directory holding pwsh.exe is a PATH directory and stays granted', ); }); @@ -104,8 +115,8 @@ describe('getAvailableToolsPolicy - PowerShell discovery', () => { const env = { PATH: pwshDir }; const result = getAvailableToolsPolicy(env); assert.ok( - result.readonlyPaths.some(p => /^[a-z]:\\$/i.test(p)), - 'System root should still be in readonlyPaths', + !result.readonlyPaths.some(p => /^[a-z]:\\$/i.test(p)), + 'System root must not be in readonlyPaths', ); assert.strictEqual(result.readwritePaths.length, 0, 'readwritePaths should be empty without USERPROFILE', diff --git a/src/backends/process_container/common/src/launch_diagnostics.rs b/src/backends/process_container/common/src/launch_diagnostics.rs index cde1cae9a..e177e419e 100644 --- a/src/backends/process_container/common/src/launch_diagnostics.rs +++ b/src/backends/process_container/common/src/launch_diagnostics.rs @@ -23,7 +23,7 @@ use wxc_common::models::{ExecutionRequest, FailurePhase, ScriptResponse}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct LaunchDiagnostic { /// Machine-readable discriminator (e.g. `"packaged_app"`, - /// `"missing_filesystem_access"`). + /// `"dll_init_failed_ui_required"`). pub kind: &'static str, /// Human-readable explanation of the failure including remediation guidance. pub message: String, @@ -137,7 +137,7 @@ pub fn diagnose_missing_required_env( pub fn diagnose_create_process_failure( win32_error: u32, command_line: &str, - readonly_paths: &[String], + _readonly_paths: &[String], ) -> LaunchDiagnostic { if win32_error == ERROR_ACCESS_DISABLED_BY_POLICY.0 { return LaunchDiagnostic { @@ -159,7 +159,7 @@ pub fn diagnose_create_process_failure( let bare_exe = Path::new(extract_exe_from_command_line(command_line)); let resolved_exe = resolve_exe_on_path(bare_exe); - if let Some(diag) = check_exe_heuristics(&resolved_exe, readonly_paths, None) { + if let Some(diag) = check_exe_heuristics(&resolved_exe, None) { return diag; } @@ -176,13 +176,13 @@ pub fn diagnose_create_process_failure( /// code. Returns `None` when no recognized condition matches. pub fn diagnose_process_exit( command_line: &str, - readonly_paths: &[String], + _readonly_paths: &[String], _readwrite_paths: &[String], exit_code: u32, ) -> Option { let bare_exe = Path::new(extract_exe_from_command_line(command_line)); let resolved_exe = resolve_exe_on_path(bare_exe); - if let Some(diag) = check_exe_heuristics(&resolved_exe, readonly_paths, Some(exit_code)) { + if let Some(diag) = check_exe_heuristics(&resolved_exe, Some(exit_code)) { return Some(diag); } None @@ -207,13 +207,9 @@ use windows::Win32::Foundation::{ // -- Internal heuristics ----------------------------------------------------- -/// Checks exe-path-based heuristics (packaged app, DLL init failure, missing -/// root access). Returns `None` if nothing matches. -fn check_exe_heuristics( - exe_path: &Path, - readonly_paths: &[String], - exit_code: Option, -) -> Option { +/// Checks exe-path-based heuristics (packaged app, DLL init failure). +/// Returns `None` if nothing matches. +fn check_exe_heuristics(exe_path: &Path, exit_code: Option) -> Option { if is_packaged_app(exe_path) { return Some(LaunchDiagnostic { kind: "packaged_app", @@ -239,19 +235,6 @@ fn check_exe_heuristics( }); } - if missing_root_readonly(exe_path, readonly_paths) { - let root = drive_root(exe_path); - return Some(LaunchDiagnostic { - kind: "missing_filesystem_access", - message: format!( - "pwsh.exe versions before 7.7 require read-only access to the \ - root drive ({root}) to start. The current sandbox policy does \ - not grant this access. Add \"{root}\" to `readonlyPaths` in your \ - sandbox policy, or upgrade to pwsh 7.7+." - ), - }); - } - None } @@ -371,30 +354,6 @@ fn is_packaged_app(exe_path: &Path) -> bool { normalized.contains("\\windowsapps\\") || normalized.contains("/windowsapps/") } -fn missing_root_readonly(exe_path: &Path, readonly_paths: &[String]) -> bool { - let filename = exe_path - .file_name() - .unwrap_or_default() - .to_string_lossy() - .to_lowercase(); - if filename != "pwsh.exe" { - return false; - } - let root = drive_root(exe_path); - !readonly_paths - .iter() - .any(|p| p.eq_ignore_ascii_case(&root) || p == "\\") -} - -fn drive_root(exe_path: &Path) -> String { - let s = exe_path.to_string_lossy(); - if s.len() >= 3 && s.as_bytes()[1] == b':' { - format!("{}\\", &s[..2]) - } else { - "C:\\".to_string() - } -} - // -- Tests ------------------------------------------------------------------- #[cfg(test)] @@ -595,27 +554,20 @@ mod tests { assert!(diag.is_none()); } + /// A failing pwsh script must not be reported as a filesystem-policy + /// problem. No policy grants the volume root any more, so a trigger keyed + /// on the absence of that grant would fire on every non-zero pwsh exit. + /// The root-metadata hint lives in `fallback_detector`, gated on the real + /// `wxc-host-prep prepare-system-drive` state. #[test] - fn missing_root_readonly_from_exit() { + fn pwsh_nonzero_exit_reports_no_filesystem_diagnostic() { let diag = diagnose_process_exit(r#""C:\Program Files\PowerShell\7\pwsh.exe""#, &[], &[], 1); - assert!(diag.is_some()); - assert_eq!(diag.unwrap().kind, "missing_filesystem_access"); - } - - #[test] - fn pwsh_with_root_readonly_no_diagnostic() { - let diag = diagnose_process_exit( - r#""C:\Program Files\PowerShell\7\pwsh.exe""#, - &["C:\\".to_string()], - &[], - 1, - ); - assert!(diag.is_none()); + assert!(diag.is_none(), "unexpected diagnostic: {diag:?}"); } #[test] - fn packaged_app_takes_priority_over_missing_access() { + fn packaged_app_detected_on_nonzero_exit() { let cmd = r#""C:\Program Files\WindowsApps\Microsoft.PowerShell_7.4.0\pwsh.exe""#; let diag = diagnose_process_exit(cmd, &[], &[], 1); assert!(diag.is_some()); diff --git a/src/core/mxc_engine/src/policy.rs b/src/core/mxc_engine/src/policy.rs index 60fc2cb02..e33c03b19 100644 --- a/src/core/mxc_engine/src/policy.rs +++ b/src/core/mxc_engine/src/policy.rs @@ -273,13 +273,21 @@ fn apply_environment_overrides( } /// PowerShell-specific policy: when `pwsh.exe` is found on `path_dirs` -/// (Windows only), grant the system-drive root (`C:\`) read-only — `pwsh.exe` -/// enumerates the drive root on startup — plus the PSReadLine history directory -/// read-write so the module can persist command history. +/// (Windows only), grant the PSReadLine history directory read-write so the +/// module can persist command history. /// -/// Mirrors the SDK's `getPowerShellPolicy`. The system drive is read from the -/// process environment (`SystemDrive`, defaulting to `C:`); the user-scoped -/// `USERPROFILE` comes from the passed-in `env`. +/// Deliberately grants no read access to the system-drive root. `pwsh.exe` +/// before 7.7 stats the root at startup, but that is a *metadata-only* need, +/// and a recursive `readonlyPaths` grant on `C:\` would expose every file on +/// the volume (`~/.ssh`, `~/.aws/credentials`, other users' profiles) to +/// satisfy it. The narrow, host-wide answer is `wxc-host-prep +/// prepare-system-drive`, which stamps non-inheriting metadata ACEs on the +/// root. `$PSHOME` itself needs no special handling here: the directory +/// holding `pwsh.exe` is by definition a `PATH` directory, so +/// [`available_tools_policy`] already grants it read-only. +/// +/// Mirrors the SDK's `getPowerShellPolicy`. `USERPROFILE` comes from the +/// passed-in `env`. /// /// On non-Windows, or when `pwsh.exe` is not on `path_dirs`, returns an empty /// policy. @@ -295,12 +303,6 @@ fn powershell_policy(path_dirs: &[String], env: &[(String, String)]) -> Filesyst return FilesystemPolicyResult::default(); } - let system_drive = std::env::var("SystemDrive") - .ok() - .filter(|s| !s.is_empty()) - .unwrap_or_else(|| "C:".to_string()); - let readonly_paths = vec![format!("{system_drive}\\")]; - let mut readwrite_paths: Vec = Vec::new(); if let Some(user_profile) = env_get(env, "USERPROFILE") { // PSReadLine command-history directory (read-write). @@ -318,8 +320,8 @@ fn powershell_policy(path_dirs: &[String], env: &[(String, String)]) -> Filesyst } FilesystemPolicyResult { - readonly_paths, readwrite_paths, + ..Default::default() } } @@ -327,8 +329,9 @@ fn powershell_policy(path_dirs: &[String], env: &[(String, String)]) -> Filesyst /// environment) as read-only policy paths. /// /// Reads `PATH` plus a registry of well-known tool/SDK variables, then filters -/// out non-existent and system-critical directories, and adds PowerShell paths -/// when `pwsh.exe` is on `PATH`. The Rust port of `getAvailableToolsPolicy`. +/// out non-existent and system-critical directories, and adds the PowerShell +/// write paths when `pwsh.exe` is on `PATH`. The Rust port of +/// `getAvailableToolsPolicy`. /// (The SDK's `processcontainer` AAP-ACL filter is Windows-runtime-specific and /// is applied server-side; it is not replicated here.) pub fn available_tools_policy(env: Option<&[(String, String)]>) -> FilesystemPolicyResult { @@ -360,11 +363,8 @@ pub fn available_tools_policy(env: Option<&[(String, String)]>) -> FilesystemPol let pwsh = powershell_policy(&path_dirs, env); - let mut readonly = filtered; - readonly.extend(pwsh.readonly_paths); - FilesystemPolicyResult { - readonly_paths: deduplicate_paths(&readonly), + readonly_paths: deduplicate_paths(&filtered), readwrite_paths: deduplicate_paths(&pwsh.readwrite_paths), } } @@ -1594,7 +1594,7 @@ mod tests { #[cfg(target_os = "windows")] #[test] - fn powershell_policy_grants_system_drive_root() { + fn powershell_policy_never_grants_the_drive_root() { use super::powershell_policy; use std::fs; use std::path::PathBuf; @@ -1619,15 +1619,13 @@ mod tests { // Clean up before asserting so a failing assertion still leaves nothing. let _ = fs::remove_dir_all(&ps_home); - // The system-drive root (e.g. `C:\`) is granted read-only — pwsh - // enumerates the drive root on startup (mirrors `getPowerShellPolicy`). - // A bare drive root normalizes to a 2-char `X:` after trimming separators. + // Finding pwsh.exe must never hand out a recursive read grant on the + // volume root: that would expose every file on the drive to satisfy a + // metadata-only startup stat. A bare drive root normalizes to a 2-char + // `X:` after trimming separators. assert!( - result.readonly_paths.iter().any(|p| { - let trimmed = p.trim_end_matches(['\\', '/']); - trimmed.len() == 2 && trimmed.ends_with(':') - }), - "expected system-drive root in readonly paths: {:?}", + result.readonly_paths.is_empty(), + "pwsh discovery must grant no read paths of its own: {:?}", result.readonly_paths ); // PSReadLine command history stays read-write. @@ -1641,6 +1639,51 @@ mod tests { ); } + /// Dropping the root grant must not cost `$PSHOME` itself: the directory + /// holding `pwsh.exe` is a `PATH` directory, so the ordinary discovery + /// filter already grants it read-only. + #[cfg(target_os = "windows")] + #[test] + fn available_tools_policy_still_grants_pshome_via_path() { + use super::available_tools_policy; + use std::fs; + + let unique = format!( + "mxc_pwsh_pshome_test_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let ps_home = std::env::temp_dir().join(unique); + fs::create_dir_all(&ps_home).expect("create temp $PSHOME"); + fs::write(ps_home.join("pwsh.exe"), b"").expect("create fake pwsh.exe"); + let ps_home_str = ps_home.to_string_lossy().into_owned(); + + let env = vec![("PATH".to_string(), ps_home_str.clone())]; + let result = available_tools_policy(Some(&env)); + + let _ = fs::remove_dir_all(&ps_home); + + assert!( + result + .readonly_paths + .iter() + .any(|p| p.eq_ignore_ascii_case(&ps_home_str)), + "expected $PSHOME granted via PATH: {:?}", + result.readonly_paths + ); + assert!( + !result.readonly_paths.iter().any(|p| { + let trimmed = p.trim_end_matches(['\\', '/']); + trimmed.len() == 2 && trimmed.ends_with(':') + }), + "no drive root may be granted: {:?}", + result.readonly_paths + ); + } + use super::{ build_request, CaptureDenials, CaptureDenialsMode, NetworkAction, NetworkEgressSection, NetworkIngressSection, NetworkPeerSection, NetworkPortSection, NetworkProtocol, diff --git a/tests/configs/pwsh_setlocation.json b/tests/configs/pwsh_setlocation.json index a4b86ffdb..4aafe7987 100644 --- a/tests/configs/pwsh_setlocation.json +++ b/tests/configs/pwsh_setlocation.json @@ -11,9 +11,6 @@ "C:\\Program Files\\PowerShell\\7", "C:\\temp", "C:\\Users" - ], - "readonlyPaths": [ - "C:\\" ] }, "ui": { diff --git a/tests/examples/08_pwsh.json b/tests/examples/08_pwsh.json index 5e6a1a239..1fa064240 100644 --- a/tests/examples/08_pwsh.json +++ b/tests/examples/08_pwsh.json @@ -11,9 +11,6 @@ "C:\\temp", "C:\\Users", "C:\\Users\\stscha\\AppData\\Roaming\\Microsoft\\Windows\\PowerShell\\PSReadLine" - ], - "readonlyPaths": [ - "C:\\" ] } } \ No newline at end of file From eedb3d0bd47d31e17eefef5d76132a69708f038a Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 18 Sep 2026 10:34:46 -0300 Subject: [PATCH 2/2] fix(policy): hold the PSReadLine write grant to the system-critical bar The read-only discovery list was filtered through is_system_critical_path / isSystemCriticalPath, but the PowerShell write grant bypassed that check entirely. USERPROFILE can legitimately sit under %WINDIR% -- the SYSTEM account's profile is C:\Windows\System32\config\systemprofile -- so a service-hosted run could hand the sandbox WRITE access beneath a protected system directory. That is strictly worse than the read grant this branch removes, and after that removal the write path is the helper's entire output. The write paths now go through the same system-critical filter in both the engine and the SDK. They are deliberately left out of the existence filter: PowerShell creates the PSReadLine history directory on first use, so requiring it to pre-exist would silently drop a legitimate grant. A regression test pins each half. Addresses the two unresolved review threads on sdk/node/src/policy.ts and src/core/mxc_engine/src/policy.rs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 65848df5-ce23-4b2a-8697-2f7f6fafe106 Signed-off-by: Carlos Alexandro Becker --- sdk/node/src/policy.ts | 12 +++- sdk/node/tests/unit/policy.test.ts | 30 +++++++++ src/core/mxc_engine/src/policy.rs | 97 +++++++++++++++++++++++++++++- 3 files changed, 137 insertions(+), 2 deletions(-) diff --git a/sdk/node/src/policy.ts b/sdk/node/src/policy.ts index 355466ddd..f4edf8545 100644 --- a/sdk/node/src/policy.ts +++ b/sdk/node/src/policy.ts @@ -312,9 +312,19 @@ export function getAvailableToolsPolicy( // Merge PowerShell-specific paths when pwsh.exe is available const pwshPolicy = getPowerShellPolicy(pathDirs, environment); + // The write paths are held to the same system-critical bar as the read + // paths: `USERPROFILE` can legitimately sit under `%WINDIR%` (the SYSTEM + // account's profile is `C:\Windows\System32\config\systemprofile`), and a + // *write* grant there would be strictly worse than the read grant this + // discovery no longer emits. They are deliberately NOT existence-filtered: + // PowerShell creates the PSReadLine history directory on first use, so + // requiring it to pre-exist would silently drop a legitimate grant. + const pwshWritePaths = deduplicatePaths(pwshPolicy.readwritePaths) + .filter(dirPath => !isSystemCriticalPath(dirPath)); + return { readonlyPaths: deduplicatePaths(filtered), - readwritePaths: deduplicatePaths([...pwshPolicy.readwritePaths]), + readwritePaths: pwshWritePaths, }; } diff --git a/sdk/node/tests/unit/policy.test.ts b/sdk/node/tests/unit/policy.test.ts index 2ccfb06b4..f4336adee 100644 --- a/sdk/node/tests/unit/policy.test.ts +++ b/sdk/node/tests/unit/policy.test.ts @@ -14,6 +14,13 @@ import * as path from 'path'; // is understood. const isLinux = process.platform === 'linux'; +// `isSystemCriticalPath` resolves `%WINDIR%` from `process.env` and normalizes +// with the *host's* path flavor, so the write-path filter can only be exercised +// truthfully on a real Windows host — mocking `process.platform` does not turn +// the imported `path` module into `path.win32`. +const isWindowsHost = process.platform === 'win32'; +const getWinDir = (): string => process.env['WINDIR'] || process.env['windir'] || 'C:\\Windows'; + describe('getAvailableToolsPolicy - PowerShell discovery', () => { let originalPlatform: PropertyDescriptor | undefined; let tmpDir: string | undefined; @@ -122,4 +129,27 @@ describe('getAvailableToolsPolicy - PowerShell discovery', () => { 'readwritePaths should be empty without USERPROFILE', ); }); + + it('should not grant write access under %WINDIR% (SYSTEM profile)', { skip: !isWindowsHost }, () => { + const pwshDir = createFakePwshDir(); + const env = { + PATH: pwshDir, + // The SYSTEM account's profile legitimately lives under %WINDIR%. + USERPROFILE: path.join(getWinDir(), 'System32', 'config', 'systemprofile'), + }; + const result = getAvailableToolsPolicy(env); + assert.deepStrictEqual(result.readwritePaths, [], + 'A PSReadLine write grant must never land beneath %WINDIR%', + ); + }); + + it('should keep the PSReadLine grant when the directory does not exist yet', { skip: !isWindowsHost }, () => { + const pwshDir = createFakePwshDir(); + const env = { PATH: pwshDir, USERPROFILE: 'C:\\Users\\mxc-nonexistent-profile' }; + const result = getAvailableToolsPolicy(env); + assert.ok( + result.readwritePaths.some(p => p.includes('PSReadLine')), + 'PowerShell creates the history directory on first use, so it need not pre-exist', + ); + }); }); diff --git a/src/core/mxc_engine/src/policy.rs b/src/core/mxc_engine/src/policy.rs index e33c03b19..3c67d20b7 100644 --- a/src/core/mxc_engine/src/policy.rs +++ b/src/core/mxc_engine/src/policy.rs @@ -363,9 +363,21 @@ pub fn available_tools_policy(env: Option<&[(String, String)]>) -> FilesystemPol let pwsh = powershell_policy(&path_dirs, env); + // The write paths are held to the same system-critical bar as the read + // paths: `USERPROFILE` can legitimately sit under `%WINDIR%` (the SYSTEM + // account's profile is `C:\Windows\System32\config\systemprofile`), and a + // *write* grant there would be strictly worse than the read grant this + // discovery no longer emits. They are deliberately NOT existence-filtered: + // PowerShell creates the PSReadLine history directory on first use, so + // requiring it to pre-exist would silently drop a legitimate grant. + let pwsh_readwrite: Vec = deduplicate_paths(&pwsh.readwrite_paths) + .into_iter() + .filter(|dir| !is_system_critical_path(dir)) + .collect(); + FilesystemPolicyResult { readonly_paths: deduplicate_paths(&filtered), - readwrite_paths: deduplicate_paths(&pwsh.readwrite_paths), + readwrite_paths: pwsh_readwrite, } } @@ -1684,6 +1696,89 @@ mod tests { ); } + /// `USERPROFILE` can legitimately sit under `%WINDIR%` — the SYSTEM + /// account's profile is `C:\Windows\System32\config\systemprofile`. A + /// read-write PSReadLine grant there would breach the same system-critical + /// boundary the read paths are held to, so it must be filtered out. + #[cfg(target_os = "windows")] + #[test] + fn tool_paths_never_grant_write_access_under_windir() { + use super::available_tools_policy; + use std::fs; + + let unique = format!( + "mxc_pwsh_rw_test_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let ps_home = std::env::temp_dir().join(unique); + fs::create_dir_all(&ps_home).expect("create temp $PSHOME"); + fs::write(ps_home.join("pwsh.exe"), b"").expect("create fake pwsh.exe"); + + let win_dir = std::env::var("WINDIR").unwrap_or_else(|_| "C:\\Windows".to_string()); + let env = vec![ + ("PATH".to_string(), ps_home.to_string_lossy().into_owned()), + ( + "USERPROFILE".to_string(), + format!("{win_dir}\\System32\\config\\systemprofile"), + ), + ]; + let result = available_tools_policy(Some(&env)); + + let _ = fs::remove_dir_all(&ps_home); + + assert!( + result.readwrite_paths.is_empty(), + "no write grant may land under %WINDIR%: {:?}", + result.readwrite_paths + ); + } + + /// The write-path filter must not require the directory to exist: + /// PowerShell creates the PSReadLine history directory on first use. + #[cfg(target_os = "windows")] + #[test] + fn psreadline_write_grant_survives_when_the_directory_is_absent() { + use super::available_tools_policy; + use std::fs; + + let unique = format!( + "mxc_pwsh_rw_keep_test_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let ps_home = std::env::temp_dir().join(unique); + fs::create_dir_all(&ps_home).expect("create temp $PSHOME"); + fs::write(ps_home.join("pwsh.exe"), b"").expect("create fake pwsh.exe"); + + // A profile that does not exist on disk and is not system-critical. + let env = vec![ + ("PATH".to_string(), ps_home.to_string_lossy().into_owned()), + ( + "USERPROFILE".to_string(), + "C:\\Users\\mxc-nonexistent-profile".to_string(), + ), + ]; + let result = available_tools_policy(Some(&env)); + + let _ = fs::remove_dir_all(&ps_home); + + assert!( + result + .readwrite_paths + .iter() + .any(|p| p.contains("PSReadLine")), + "PSReadLine grant must survive a not-yet-created directory: {:?}", + result.readwrite_paths + ); + } + use super::{ build_request, CaptureDenials, CaptureDenialsMode, NetworkAction, NetworkEgressSection, NetworkIngressSection, NetworkPeerSection, NetworkPortSection, NetworkProtocol,