fix(policy): stop granting the system drive when pwsh.exe is on PATH

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.
@@ -306,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, ...pwshPolicy.readonlyPaths]),
-        readwritePaths: deduplicatePaths([...pwshPolicy.readwritePaths]),
+        readonlyPaths: deduplicatePaths(filtered),
+        readwritePaths: pwshWritePaths,
     };
 }
 

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;
@@ -46,14 +53,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,11 +122,34 @@ 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',
         );
     });
+
+    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',
+        );
+    });
 });

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<LaunchDiagnostic> {
     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<u32>,
-) -> Option<LaunchDiagnostic> {
+/// 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<u32>) -> Option<LaunchDiagnostic> {
     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());