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

[Carlos Alexandro Becker (caarlos0)]

• I have signed the Contributor License Agreement.
• This pull request is related to an issue.
• If this PR changes build commands, project architecture, or key conventions, I have updated .github/copilot-instructions.md.

---

Summary

powershell_policy (src/core/mxc_engine/src/policy.rs) and its SDK mirror getPowerShellPolicy (sdk/node/src/policy.ts) returned the system-drive root (C:\) as a read-only grant the moment pwsh.exe was found in any PATH directory. Worse, available_tools_policy appended that grant after its directory_exists(dir) && !is_system_critical_path(dir) filter — so the one path most deserving the system-critical check was the only path that never got it.

Any sandbox seeded from these discovery helpers could read the entire volume: ~/.ssh, ~/.aws/credentials, .npmrc / .netrc tokens, browser profiles, and other users' profile directories — defeating the deny-by-default read confinement the AppContainer identity otherwise provides.

The fix

1. Grant $PSHOME, not the volume root. The policy now grants the PATH directory that actually contains pwsh.exe. Module trees already reach the policy through PSModulePath discovery.

   The legitimate need behind the original grant — pwsh.exe calling GetFileAttributesW("C:\\") during startup — is already served host-wide, and safely, by wxc-host-prep prepare-system-drive, which stamps metadata-only, non-inheriting ACEs (FILE_READ_ATTRIBUTES | FILE_READ_EA | READ_CONTROL | SYNCHRONIZE, no FILE_LIST_DIRECTORY) on the drive root. See docs/host-prep.md. The policy grant was a recursive, massively over-broad duplicate of a problem that already had a narrow solution.

2. Merge before filtering. The PowerShell grant is now folded into the collected set before the existence + system-critical filter, so it is held to the same bar as every other discovered directory.

3. Reject filesystem roots outright. is_system_critical_path / isSystemCriticalPath previously only looked at %WINDIR% (Windows) and /bin-style paths (Unix) — a root was never rejected. Now C:\, \\server\share, and / are always critical, closing the same hole for a root that arrives via PATH or any known tool/SDK environment variable. In Rust this is expressed as "an absolute path with no Component::Normal"; in TypeScript as path.parse(resolved).root === resolved.

Hardening found while fixing the above

• Drive-relative paths. resolve_path hand-rolled is_absolute() + current_dir().join(), and PathBuf::push with a prefixed path replaces the buffer — so C: was misclassified as a root and C:Windows bypassed the %WINDIR% check. It now uses std::path::absolute (GetFullPathNameW on Windows), which handles per-drive current directories correctly. It fails closed: the only error case is the empty path, which then classifies as critical.

• Verbatim / device namespaces. \\?\C:\Windows\System32 and \\.\C:\Windows slipped past the %WINDIR% comparison. Both implementations now strip \\?\, \\?\UNC\, and \\.\ prefixes for that comparison — after the root test, so stripping can never turn an absolute root into a cwd-relative path that escapes the guard.

• Misleading diagnostic. The missing_filesystem_access launch diagnostic told users to add the drive root to readonlyPaths, i.e. to re-introduce this exact vulnerability by hand. It now points at wxc-host-prep prepare-system-drive or upgrading to pwsh 7.7+, and explicitly warns against granting the root.

  That diagnostic also needed a better trigger. It inferred "pwsh cannot read the root" purely from the absence of a root grant in readonlyPaths — a usable signal only while the discovery helpers still produced one. Now that they never do, the predicate would be true for every pwsh.exe run, so an ordinary nonzero exit (a script error) would be reported as missing_filesystem_access even on a fully prepared host. It is now gated on the real machine state via the system_drive_prepared root-DACL probe that fallback_detector already runs for its host-prep warnings. The probe is passed as a closure, so it is skipped for non-PowerShell executables and for policies that already grant the root, and the unit test can pin a deterministic host state instead of depending on whichever machine runs CI.

• Tests that never ran. sdk/node/src/policy.ts called os.platform(), but the unit tests mock process.platform. The Windows-branch tests were therefore skipped on Linux (with a stale TODO) and failing on macOS. Switching the module to process.platform — identical semantics, and what the rest of the SDK already uses — makes them exercise the real code path on every platform. The skip: isLinux guards are gone.

  Mocking process.platform still does not turn the imported path module into path.win32, so the Windows namespace handling above could only be tested on a Windows runner. The path flavor and platform are now parameters of the predicate (isSystemCriticalPathWith, @internal, not re-exported from index.ts), and a Windows-semantics matrix runs on every host.

• The write grant escaped the same check. Only the PowerShell read-only path was merged into the guarded collection; the read-write PSReadLine path bypassed the filter entirely. It is derived from USERPROFILE, which can legitimately sit inside %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, strictly worse than the read grant this PR removes. The write paths now go through the same system-critical filter. They stay deliberately 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.

• Sample configs that taught the anti-pattern. tests/examples/08_pwsh.json and tests/configs/pwsh_setlocation.json both handed the sandbox "readonlyPaths": ["C:\\"]. Being hand-written configs, nothing filtered them, and as the canonical worked examples of running pwsh under MXC they taught the very pattern this PR removes. Neither needs it — PowerShell is reached through the explicit C:\Program Files\PowerShell\7 entry, and the startup root-metadata access comes from wxc-host-prep prepare-system-drive. The example (documentation only; tests/examples is schema-validated but never executed) also gets $PSHOME demoted to read-only and loses a broad C:\Users grant plus a hardcoded C:\Users\stscha\...\PSReadLine path that the command line's -HistorySaveStyle SaveNothing already makes unnecessary. pwsh_setlocation.json is executed by run_test_configs.ps1, so it gets the minimal change. The isolation_session_* configs that also mention C:\ deliberately pass over-broad paths to prove protected_paths_filter rejects them, and are untouched.

Testing

• cargo test -p mxc_engine --lib — 19 pass (17 before; powershell_policy_grants_system_drive_root is renamed and inverted to ..._grants_pshome_not_the_drive_root, plus new filesystem_roots_are_system_critical and tool_paths_never_grant_a_filesystem_root; a 20th, tool_paths_never_grant_write_access_under_windir, is Windows-gated and runs on the Windows CI job).
• cargo fmt --all -- --check and cargo clippy -p appcontainer_common -p mxc_engine --all-targets --target x86_64-pc-windows-msvc -- -D warnings — clean, so the #[cfg(target_os = "windows")] code and its tests are checked from a non-Windows dev host too.
• node scripts/versioning/validate-configs.js — 192 configs validate against the dev schema.
• npm test in sdk/node — 212 tests, 185 pass, 0 fail (3 were failing before this change), 27 skipped.
• The new isSystemCriticalPath - Windows path semantics suite covers drive and UNC share roots; the verbatim and device namespaces (\\?\C:\, \\?\C:, \\.\C:, \\?\Volume{GUID}\, \\?\UNC\server\share); %WINDIR% in each spelling; and non-critical controls including C:\WindowsApps\vendor, a string prefix of %WINDIR% that must not be treated as under it.
• The SYSTEM-profile write grant is driven end to end through discovery by a Windows-gated Rust test and a Windows-only SDK test, and its path shape is asserted on every host by the injectable path-semantics suite.
• missing_root_metadata_access is unit-tested for both host-prep states, and asserts the DACL probe is never invoked on either short-circuit path.

Follow-up (deliberately out of scope)

temporary_files_policy / getTemporaryFilesPolicy put %TEMP% / $TMPDIR straight into readwritePaths with no system-critical check at all — so a TEMP=C:\ environment yields a read-write whole-volume grant, strictly worse than the bug fixed here. Separately, the TypeScript docstring there claims a unique per-sandbox subdirectory is created (and randomBytes is imported for it), but the body returns the bare temp root. Both are pre-existing and belong in their own change.

[Copilot]

🟡 Changes recommended

PowerShell’s ordinary nonzero exits can now be misreported as missing root access, and Windows path hardening lacks effective automated coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Hardens tool discovery policies to avoid granting filesystem roots while retaining PowerShell functionality.

Changes:

• Grants $PSHOME instead of the system-drive root.
• Filters filesystem roots and Windows namespace variants.
• Updates diagnostics and cross-platform policy tests.

File summaries

File	Description
src/core/mxc_engine/src/policy.rs	Hardens Rust path filtering and PowerShell discovery.
src/backends/appcontainer/common/src/launch_diagnostics.rs	Revises PowerShell remediation guidance.
sdk/node/src/policy.ts	Mirrors policy hardening in the TypeScript SDK.
sdk/node/tests/unit/policy.test.ts	Updates PowerShell and root-filter tests.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

PowerShell diagnostics can still misidentify failures and inconsistently compare the executable drive with the system drive.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

src/backends/appcontainer/common/src/launch_diagnostics.rs:148

• This still reports missing_filesystem_access for every nonzero pwsh.exe exit on an unprepared host, although the message itself says only versions before 7.7 need this access. For example, a pwsh 7.7+ script that intentionally exits 1 is misdiagnosed because neither the executable version nor access-denied evidence is checked. Please gate the process-exit heuristic on evidence that the affected startup path was hit; host-prep state alone is insufficient.

    if missing_root_metadata_access(exe_path, readonly_paths, || {
        crate::fallback_detector::system_drive_prepared()
    }) {

src/core/mxc_engine/src/policy.rs:196

• The Rust tests only exercise C:\/C:/; they do not cover the newly added UNC, verbatim, or device-namespace stripping here, nor drive-relative resolution through std::path::absolute. The TypeScript path.win32 tests cannot validate Rust Path::components behavior. Please add Windows-target cases for these security-boundary spellings, including non-root/non-%WINDIR% controls.

        let n = n
            .strip_prefix(r"\\?\unc\")
            .or_else(|| n.strip_prefix(r"\\?\"))
            .or_else(|| n.strip_prefix(r"\\.\"))

src/backends/appcontainer/common/src/launch_diagnostics.rs:431

• root is derived from the executable's drive, but drive_prepared() now probes only %SystemDrive% in fallback_detector. If pwsh is installed on D:, prepared C: ACEs suppress a diagnostic whose message names D:, while an explicit C:\ grant is not recognized when the system-drive probe fails. Use the same root for the policy check, DACL probe, and message—per docs/host-prep.md:61-68, the startup access being prepared is the system-drive root—or parameterize the probe with the root actually being tested.

    let root = drive_root(exe_path);
    let policy_grants_root = readonly_paths
        .iter()
        .any(|p| p.eq_ignore_ascii_case(&root) || p == "\\");
    !policy_grants_root && !drive_prepared()

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

PowerShell write paths still bypass critical-path filtering, and root-access diagnostics can evaluate inconsistent or equivalent root spellings incorrectly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

src/backends/appcontainer/common/src/launch_diagnostics.rs:431

• These two checks can refer to different drives: root comes from the executable path, while drive_prepared() always probes %SystemDrive%. For pwsh.exe installed on D:, a prepared C: suppresses the diagnostic even though D: was never probed, and an explicit C:\ grant does not short-circuit it. Make the DACL probe accept/check the same root used for the policy comparison, or consistently base both checks and the message on the system-drive root that PowerShell actually stats.

    let root = drive_root(exe_path);
    let policy_grants_root = readonly_paths
        .iter()
        .any(|p| p.eq_ignore_ascii_case(&root) || p == "\\");
    !policy_grants_root && !drive_prepared()

src/backends/appcontainer/common/src/launch_diagnostics.rs:430

• This textual comparison misses equivalent recursive root grants such as C:/, C:\., and verbatim-root spellings. Filesystem policy paths are not lexically normalized by the parser, so on an unprepared host a normal nonzero PowerShell exit with one of those grants is still misreported as missing root access. Normalize each policy path before deciding whether it denotes the relevant root; the new root-classification tests already establish these spellings as equivalent roots.

    let policy_grants_root = readonly_paths
        .iter()
        .any(|p| p.eq_ignore_ascii_case(&root) || p == "\\");

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

PowerShell diagnostics still misidentify some failures, and the executable test fixture lacks the newly required host-preparation setup.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (4)

src/backends/appcontainer/common/src/launch_diagnostics.rs:431

• The predicate still classifies every nonzero pwsh.exe exit on an unprepared host as missing root access, regardless of the executable version. That contradicts the new message's statement that 7.7+ does not require this access: a normal script error from 7.7+ will be replaced by this diagnostic whenever the host is intentionally unprepared. Gate this on the affected PowerShell versions or on failure evidence specific to the root metadata access.

    let policy_grants_root = readonly_paths
        .iter()
        .any(|p| p.eq_ignore_ascii_case(&root) || p == "\\");
    !policy_grants_root && !drive_prepared()

src/backends/appcontainer/common/src/launch_diagnostics.rs:149

• This mixes two different roots: system_drive_prepared() probes %SystemDrive%, while drive_root(exe_path) and the policy check use the volume containing pwsh.exe. For a PowerShell installation on D:, an unprepared C: host reports that prepare-system-drive will add ACEs to D: (it will not), and an explicit D:\ grant suppresses the diagnostic even though the required system-drive metadata is still unavailable. Derive the displayed and policy-checked root from the same system-drive value used by the probe.

    if missing_root_metadata_access(exe_path, readonly_paths, || {
        crate::fallback_detector::system_drive_prepared()
    }) {
        let root = drive_root(exe_path);

src/core/mxc_engine/src/policy.rs:196

• No Rust test exercises this newly added device-namespace branch (or the adjacent verbatim %WINDIR% handling); filesystem_roots_are_system_critical only checks plain drive/POSIX roots, while the comprehensive namespace matrix is TypeScript-only. Add Windows-targeted Rust assertions for \\?\C:\Windows, \\.\C:\Windows, and verbatim/device roots so the two policy implementations cannot silently diverge at this security boundary.

            .or_else(|| n.strip_prefix(r"\\.\"))

tests/configs/pwsh_setlocation.json:14

• Removing the root grant makes this executable fixture depend on the host-prep ACEs for PowerShell versions before 7.7, but the documented test setup does not establish that prerequisite: scripts/setup-test-prereqs.ps1:122-136 reports all prerequisites met after only finding PowerShell 7, and run_pwsh_test.ps1 runs this config without checking host prep. Consequently the supported setup can still fail at PowerShell startup. Update the prerequisite setup/check to run or verify wxc-host-prep prepare-system-drive (or gate this fixture on a version that no longer needs it).

      "C:\\Users"
    ]

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.