From de05a60427ec4fc6cb332de6a34a3ebc09ae306a Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 9 Oct 2026 23:27:29 -0700 Subject: [PATCH 1/2] require opt-in for PowerShell whole-drive read access The drive root is added only when allowPowerShellDriveRootRead is true; the default is false. Finding pwsh.exe still adds the PSReadLine history directory to readwritePaths, and a drive root that is already a PATH entry is still returned as a tool directory. The README and the helper comments state that the opt-in permits recursive whole-drive reads on BaseContainer, and they point at wxc-host-prep prepare-system-drive or enumeratePaths for a root check that does not grant subtree reads. FilesystemPolicyResult still only carries readonlyPaths and readwritePaths. Fixes #1455 --- sdk/node/README.md | 23 +++- sdk/node/src/v1/policy/filesystem.ts | 48 +++++++-- sdk/node/tests/unit/policy.test.ts | 152 +++++++++++++++++++-------- 3 files changed, 165 insertions(+), 58 deletions(-) diff --git a/sdk/node/README.md b/sdk/node/README.md index 6d49b8491..22843ade5 100644 --- a/sdk/node/README.md +++ b/sdk/node/README.md @@ -264,6 +264,23 @@ Filesystem discovery helpers and their result/options types are grouped under `ToolsPolicyOptions`; set `containerType: 'processcontainer'` to exclude directories with ALL APPLICATION PACKAGES access on Windows. ACL inspection is bounded to five seconds per directory; failures retain the directory and -emit a diagnostic warning. `getUserProfilePolicy` uses the supplied environment, -and `getTemporaryFilesPolicy` returns existing temporary storage without creating -directories. +emit a diagnostic warning. When `pwsh.exe` is on `PATH`, discovery adds the +PSReadLine history directory to `readwritePaths` and leaves the system-drive +root out of `readonlyPaths`. Set `allowPowerShellDriveRootRead: true` to +restore that root for callers who accept the compatibility grant. On +BaseContainer, a drive-root `readonlyPaths` entry permits recursive reads of +the whole drive, including unrelated user files. `getUserProfilePolicy` uses +the supplied environment, and `getTemporaryFilesPolicy` returns existing +temporary storage without creating directories. + +PowerShell startup may still need to stat the system-drive root. Use explicit +host preparation or tier-supported configuration for that check. +[`wxc-host-prep prepare-system-drive`](https://github.com/microsoft/mxc/blob/main/docs/backends/process-container/host-prep.md) +adds persistent, non-inheriting metadata ACEs on the drive root for the +AppContainer package SIDs: `FILE_READ_ATTRIBUTES`, `FILE_READ_EA`, +`READ_CONTROL`, and `SYNCHRONIZE`. The ACEs apply only to the root itself. +They include no directory-enumeration right, no file-content read, and no +write. Where the isolation tier supports a narrower query grant, set +`enumeratePaths` on ProcessContainer containment configuration. +`FilesystemPolicyResult` stays limited to `readonlyPaths` and `readwritePaths`, +so this helper cannot carry `enumeratePaths` for every tier. diff --git a/sdk/node/src/v1/policy/filesystem.ts b/sdk/node/src/v1/policy/filesystem.ts index 83e3dc5c7..c79409f11 100644 --- a/sdk/node/src/v1/policy/filesystem.ts +++ b/sdk/node/src/v1/policy/filesystem.ts @@ -36,6 +36,16 @@ export interface ToolsPolicyOptions { * because AppContainer processes can already see them implicitly. */ containerType?: 'processcontainer'; + + /** + * When `true`, and `pwsh.exe` is found on PATH, add the system-drive root + * (`SystemDrive`, falling back to `C:`) to `readonlyPaths`. + * + * Defaults to `false`. Opting in permits recursive whole-drive reads on + * BaseContainer. PowerShell startup that only needs to stat the drive root + * should use host preparation or tier-supported configuration instead. + */ + allowPowerShellDriveRootRead?: boolean; } // --------------------------------------------------------------------------- @@ -205,19 +215,25 @@ function deduplicatePaths(paths: string[]): string[] { * 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. + * - The system-drive root in `readonlyPaths` only when + * `allowPowerShellDriveRootRead` is true. The root comes from `SystemDrive`, + * falling back to `C:`. That opt-in permits recursive whole-drive reads on + * BaseContainer. The default omits the root; PowerShell startup may need + * explicit host preparation or tier-supported configuration instead. * * On non-Windows platforms or when pwsh.exe is not found on PATH, returns an - * empty policy. + * empty policy. The opt-in does not add a drive root in either case. * * @param pathDirs - The list of PATH directories already collected by the caller. * @param environment - Environment variable map. + * @param allowPowerShellDriveRootRead - Whether to add the synthetic system-drive root. */ function getPowerShellPolicy( pathDirs: string[], environment: { [key: string]: string | undefined }, + allowPowerShellDriveRootRead: boolean, ): FilesystemPolicyResult { if (os.platform() !== 'win32') { return { readonlyPaths: [], readwritePaths: [] }; @@ -235,9 +251,11 @@ function getPowerShellPolicy( return { readonlyPaths: [], readwritePaths: [] }; } - const systemDrive = environmentValue(environment, 'SystemDrive') || 'C:'; - const systemRoot = systemDrive + "\\"; - const readonlyPaths: string[] = [systemRoot]; + const readonlyPaths: string[] = []; + if (allowPowerShellDriveRootRead) { + const systemDrive = environmentValue(environment, 'SystemDrive') || 'C:'; + readonlyPaths.push(systemDrive + "\\"); + } const readwritePaths: string[] = []; const userProfile = environmentValue(environment, 'USERPROFILE'); @@ -268,10 +286,13 @@ 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`. The system-drive root is + * added to `readonlyPaths` only when `options.allowPowerShellDriveRootRead` + * is true (the default is `false`). That opt-in permits recursive whole-drive + * reads on BaseContainer. PowerShell startup that stats the drive root may + * need explicit host preparation or tier-supported configuration; the opt-in + * is the compatibility path for callers who accept the broad read. * * ACL inspection failures retain the directory and emit a diagnostic warning. * @@ -316,8 +337,13 @@ export function getAvailableToolsPolicy( return true; }); - // Merge PowerShell-specific paths when pwsh.exe is available - const pwshPolicy = getPowerShellPolicy(pathDirs, environment); + // Merge PowerShell-specific paths when pwsh.exe is available. + // The drive-root read is opt-in; tool directories and PSReadLine are not. + const pwshPolicy = getPowerShellPolicy( + pathDirs, + environment, + options?.allowPowerShellDriveRootRead === true, + ); return { readonlyPaths: deduplicatePaths([...filtered, ...pwshPolicy.readonlyPaths]), diff --git a/sdk/node/tests/unit/policy.test.ts b/sdk/node/tests/unit/policy.test.ts index fe2a8a31e..bd3b19c40 100644 --- a/sdk/node/tests/unit/policy.test.ts +++ b/sdk/node/tests/unit/policy.test.ts @@ -33,6 +33,10 @@ describe('filesystem helper environment and options', () => { it('does not substitute the host environment for an empty map', () => { assert.deepStrictEqual(getAvailableToolsPolicy({}), { readonlyPaths: [], readwritePaths: [] }); + assert.deepStrictEqual( + getAvailableToolsPolicy({}, { allowPowerShellDriveRootRead: true }), + { readonlyPaths: [], readwritePaths: [] }, + ); assert.deepStrictEqual(getUserProfilePolicy({}), { readonlyPaths: [], readwritePaths: [] }); }); @@ -68,15 +72,55 @@ describe('filesystem helper environment and options', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'mxc-tools & paths-')); try { execFileSync('icacls.exe', [root, '/grant', '*S-1-15-2-1:(OI)(CI)(RX)'], { windowsHide: true }); + fs.writeFileSync(path.join(root, 'pwsh.exe'), ''); const environment = { path: root }; + const driveRoot = path.resolve('C:\\'); assert.deepStrictEqual(getAvailableToolsPolicy(environment).readonlyPaths, [root]); assert.deepStrictEqual( getAvailableToolsPolicy(environment, { containerType: 'processcontainer' }).readonlyPaths, [], ); + // containerType filters ACLs and does not opt in to the drive-root read. + assert.deepStrictEqual( + getAvailableToolsPolicy(environment, { + containerType: 'processcontainer', + allowPowerShellDriveRootRead: false, + }).readonlyPaths, + [], + ); + assert.deepStrictEqual( + getAvailableToolsPolicy(environment, { + containerType: 'processcontainer', + allowPowerShellDriveRootRead: true, + }).readonlyPaths, + [driveRoot], + ); } finally { fs.rmSync(root, { recursive: true }); } }); + + it('preserves a drive root explicitly supplied on PATH', { skip: process.platform !== 'win32' }, () => { + const pwshDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mxc-test-')); + try { + fs.writeFileSync(path.join(pwshDir, 'pwsh.exe'), ''); + const driveRoot = path.resolve((process.env['SystemDrive'] || 'C:') + '\\'); + assert.strictEqual(fs.statSync(driveRoot).isDirectory(), true); + const environment = { PATH: `${driveRoot};${pwshDir}`, USERPROFILE: 'C:\\Users\\TestUser' }; + const history = path.resolve(path.join( + 'C:\\Users\\TestUser', 'AppData', 'Roaming', 'Microsoft', 'Windows', 'PowerShell', 'PSReadLine', + )); + const result = getAvailableToolsPolicy(environment); + const toolDir = path.resolve(pwshDir); + assert.deepStrictEqual(result.readonlyPaths, [driveRoot, toolDir]); + assert.deepStrictEqual(result.readwritePaths, [history]); + assert.deepStrictEqual( + getAvailableToolsPolicy({ PATH: pwshDir, USERPROFILE: 'C:\\Users\\TestUser' }).readonlyPaths, + [toolDir], + ); + } finally { + fs.rmSync(pwshDir, { recursive: true, force: true }); + } + }); }); describe('getAvailableToolsPolicy - PowerShell discovery', () => { @@ -100,6 +144,12 @@ describe('getAvailableToolsPolicy - PowerShell discovery', () => { return tmpDir; }; + const psReadLine = (profile: string): string => path.resolve(path.join( + profile, 'AppData', 'Roaming', 'Microsoft', 'Windows', 'PowerShell', 'PSReadLine', + )); + + const driveRoot = (drive: string): string => path.resolve(drive + '\\'); + afterEach(() => { if (originalPlatform) { Object.defineProperty(process, 'platform', originalPlatform); @@ -111,69 +161,83 @@ describe('getAvailableToolsPolicy - PowerShell discovery', () => { } }); - it('should add system root to readonlyPaths when pwsh.exe is on PATH', { skip: isLinux }, () => { + it('should keep the tool directory and PSReadLine history without a drive root', { 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', - ); + const toolDir = path.resolve(pwshDir); + const history = psReadLine('C:\\Users\\TestUser'); + const optionSets = [undefined, {}, { allowPowerShellDriveRootRead: false }] as const; + for (const options of optionSets) { + const result = getAvailableToolsPolicy(env, options); + assert.deepStrictEqual(result.readonlyPaths, [toolDir]); + assert.deepStrictEqual(result.readwritePaths, [history]); + } }); - it('should add PSReadLine dir to readwritePaths when pwsh.exe is on PATH', { skip: isLinux }, () => { + it('should restore the drive root only when allowPowerShellDriveRootRead is true', { skip: isLinux }, () => { mockWindows(); const pwshDir = createFakePwshDir(); - const env = { PATH: pwshDir, USERPROFILE: 'C:\\Users\\TestUser' }; - const result = getAvailableToolsPolicy(env); - const expected = path.join( - 'C:\\Users\\TestUser', 'AppData', 'Roaming', 'Microsoft', 'Windows', 'PowerShell', 'PSReadLine', + const toolDir = path.resolve(pwshDir); + const history = psReadLine('C:\\Users\\TestUser'); + + const nonDefaultDrive = getAvailableToolsPolicy( + { PATH: pwshDir, SystemDrive: 'D:', USERPROFILE: 'C:\\Users\\TestUser' }, + { allowPowerShellDriveRootRead: true }, ); - assert.ok( - result.readwritePaths.some(p => p.toLowerCase() === expected.toLowerCase()), - 'PSReadLine directory should be in readwritePaths', + assert.deepStrictEqual(nonDefaultDrive.readonlyPaths, [toolDir, driveRoot('D:')]); + assert.deepStrictEqual(nonDefaultDrive.readwritePaths, [history]); + + for (const systemDrive of [undefined, '']) { + const environment: { [key: string]: string | undefined } = { + PATH: pwshDir, + USERPROFILE: 'C:\\Users\\TestUser', + }; + if (systemDrive !== undefined) { + environment['SystemDrive'] = systemDrive; + } + const fallback = getAvailableToolsPolicy(environment, { allowPowerShellDriveRootRead: true }); + assert.deepStrictEqual(fallback.readonlyPaths, [toolDir, driveRoot('C:')]); + assert.deepStrictEqual(fallback.readwritePaths, [history]); + } + + const caseInsensitive = getAvailableToolsPolicy( + { path: pwshDir, systemdrive: 'E:', userprofile: 'C:\\Users\\Other' }, + { allowPowerShellDriveRootRead: true }, ); + assert.deepStrictEqual(caseInsensitive.readonlyPaths, [toolDir, driveRoot('E:')]); + assert.deepStrictEqual(caseInsensitive.readwritePaths, [psReadLine('C:\\Users\\Other')]); }); - it('should not add PowerShell paths when pwsh.exe is not on PATH', { skip: isLinux }, () => { + it('should not add a drive root without pwsh.exe even when opted in', { skip: isLinux }, () => { mockWindows(); - const env = { PATH: 'C:\\Windows\\System32', USERPROFILE: 'C:\\Users\\TestUser' }; - const result = getAvailableToolsPolicy(env); - assert.ok( - !result.readonlyPaths.some(p => /^[a-z]:\\$/i.test(p)), - 'System root should not be in readonlyPaths when pwsh.exe is not on PATH', - ); - assert.strictEqual(result.readwritePaths.length, 0, - 'readwritePaths should be empty when pwsh.exe is not on PATH', - ); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mxc-test-')); + const env = { PATH: tmpDir, USERPROFILE: 'C:\\Users\\TestUser', SystemDrive: 'D:' }; + const result = getAvailableToolsPolicy(env, { allowPowerShellDriveRootRead: true }); + assert.deepStrictEqual(result.readonlyPaths, [path.resolve(tmpDir)]); + assert.deepStrictEqual(result.readwritePaths, []); }); - it('should return empty policy on non-Windows even when pwsh.exe is on PATH', { skip: isLinux }, () => { + it('should return no PowerShell grant on non-Windows even when opted in', { skip: isLinux }, () => { mockLinux(); 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 not be in readonlyPaths on Linux', - ); - assert.strictEqual(result.readwritePaths.length, 0, - 'readwritePaths should be empty on Linux', - ); + const env = { PATH: pwshDir, USERPROFILE: 'C:\\Users\\TestUser', SystemDrive: 'D:' }; + const result = getAvailableToolsPolicy(env, { allowPowerShellDriveRootRead: true }); + assert.ok(!result.readonlyPaths.includes(driveRoot('D:'))); + assert.ok(!result.readonlyPaths.includes(driveRoot('C:'))); + assert.deepStrictEqual(result.readwritePaths, []); }); - it('should not add PSReadLine path when USERPROFILE is not set', { skip: isLinux }, () => { + it('should not add PSReadLine history when USERPROFILE is not set', { skip: isLinux }, () => { mockWindows(); const pwshDir = createFakePwshDir(); - 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', - ); - assert.strictEqual(result.readwritePaths.length, 0, - 'readwritePaths should be empty without USERPROFILE', - ); + const env = { PATH: pwshDir, SystemDrive: 'D:' }; + const toolDir = path.resolve(pwshDir); + const withoutRoot = getAvailableToolsPolicy(env, { allowPowerShellDriveRootRead: false }); + assert.deepStrictEqual(withoutRoot.readonlyPaths, [toolDir]); + assert.deepStrictEqual(withoutRoot.readwritePaths, []); + const withRoot = getAvailableToolsPolicy(env, { allowPowerShellDriveRootRead: true }); + assert.deepStrictEqual(withRoot.readonlyPaths, [toolDir, driveRoot('D:')]); + assert.deepStrictEqual(withRoot.readwritePaths, []); }); }); From 2bb93f94fffff1e68b193e9b213bce3d7bd8271c Mon Sep 17 00:00:00 2001 From: Huzaifa Danish Date: Mon, 12 Oct 2026 15:27:03 -0700 Subject: [PATCH 2/2] address PowerShell policy review findings Apply the safe-by-default PowerShell drive-root behavior to the Rust V1 helper through an additive opt-in entrypoint, preserve explicit PATH roots, and complete the Node and Rust public documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/api-reference/node/v1/types.md | 1 + docs/api-reference/rust/v1/api.md | 8 +++ sdk/node/README.md | 9 ++- sdk/node/src/v1/policy/filesystem.ts | 8 ++- src/mxc-sdk/README.md | 11 +++- src/mxc-sdk/src/lib.rs | 5 +- src/mxc-sdk/src/policy.rs | 86 +++++++++++++++++++--------- src/mxc-sdk/tests/sdk_helpers.rs | 55 +++++++++++++++++- 8 files changed, 146 insertions(+), 37 deletions(-) diff --git a/docs/api-reference/node/v1/types.md b/docs/api-reference/node/v1/types.md index 3517465d2..965b54ba4 100644 --- a/docs/api-reference/node/v1/types.md +++ b/docs/api-reference/node/v1/types.md @@ -954,6 +954,7 @@ Options for getAvailableToolsPolicy. ```typescript export interface ToolsPolicyOptions { containerType?: 'processcontainer'; + allowPowerShellDriveRootRead?: boolean; } ``` diff --git a/docs/api-reference/rust/v1/api.md b/docs/api-reference/rust/v1/api.md index 90b613703..cb9885edb 100644 --- a/docs/api-reference/rust/v1/api.md +++ b/docs/api-reference/rust/v1/api.md @@ -56,6 +56,14 @@ Discover tool and SDK directories from environment (defaults to the process envi pub fn available_tools_policy(environment: Option<&[(String, String)]>, options: ToolsPolicyOptions) -> FilesystemPolicyResult; ``` +## `mxc_sdk::v1::policy::filesystem::available_tools_policy_with_powershell_drive_root_read` + +Discover tool and SDK directories while opting into the legacy PowerShell system-drive compatibility grant. + +```rust +pub fn available_tools_policy_with_powershell_drive_root_read(environment: Option<&[(String, String)]>, options: ToolsPolicyOptions) -> FilesystemPolicyResult; +``` + ## `mxc_sdk::v1::container::deprovision_container` diff --git a/sdk/node/README.md b/sdk/node/README.md index 22843ade5..98af234c8 100644 --- a/sdk/node/README.md +++ b/sdk/node/README.md @@ -269,9 +269,12 @@ PSReadLine history directory to `readwritePaths` and leaves the system-drive root out of `readonlyPaths`. Set `allowPowerShellDriveRootRead: true` to restore that root for callers who accept the compatibility grant. On BaseContainer, a drive-root `readonlyPaths` entry permits recursive reads of -the whole drive, including unrelated user files. `getUserProfilePolicy` uses -the supplied environment, and `getTemporaryFilesPolicy` returns existing -temporary storage without creating directories. +the whole drive, including unrelated user files. This option controls only the +synthetic root added for PowerShell compatibility; a drive root explicitly +supplied through `PATH` or another discovered environment variable is still +returned as a tool path. `getUserProfilePolicy` uses the supplied environment, +and `getTemporaryFilesPolicy` returns existing temporary storage without +creating directories. PowerShell startup may still need to stat the system-drive root. Use explicit host preparation or tier-supported configuration for that check. diff --git a/sdk/node/src/v1/policy/filesystem.ts b/sdk/node/src/v1/policy/filesystem.ts index c79409f11..03ceb0a3e 100644 --- a/sdk/node/src/v1/policy/filesystem.ts +++ b/sdk/node/src/v1/policy/filesystem.ts @@ -43,7 +43,9 @@ export interface ToolsPolicyOptions { * * Defaults to `false`. Opting in permits recursive whole-drive reads on * BaseContainer. PowerShell startup that only needs to stat the drive root - * should use host preparation or tier-supported configuration instead. + * should use host preparation or tier-supported configuration instead. A + * drive root explicitly supplied through PATH or another discovered + * environment variable remains in `readonlyPaths`. */ allowPowerShellDriveRootRead?: boolean; } @@ -292,7 +294,9 @@ function getPowerShellPolicy( * is true (the default is `false`). That opt-in permits recursive whole-drive * reads on BaseContainer. PowerShell startup that stats the drive root may * need explicit host preparation or tier-supported configuration; the opt-in - * is the compatibility path for callers who accept the broad read. + * is the compatibility path for callers who accept the broad read. This option + * controls only the synthetic PowerShell grant; an explicitly discovered drive + * root remains in the result. * * ACL inspection failures retain the directory and emit a diagnostic warning. * diff --git a/src/mxc-sdk/README.md b/src/mxc-sdk/README.md index b5b68029e..4b044cb6b 100644 --- a/src/mxc-sdk/README.md +++ b/src/mxc-sdk/README.md @@ -171,9 +171,14 @@ Filesystem discovery helpers and their result/options types live under `container_type: Some(ToolsPolicyContainerType::ProcessContainer)` to exclude directories with ALL APPLICATION PACKAGES access on Windows. ACL inspection is bounded to five seconds per directory; failures retain the directory and use -the configured diagnostic sink. `user_profile_policy` discovers standard -per-user tool locations, and `temporary_files_policy` returns existing -temporary storage without creating directories. +the configured diagnostic sink. PowerShell discovery omits the synthetic +system-drive root by default; use +`available_tools_policy_with_powershell_drive_root_read` only when recursive +whole-drive reads on BaseContainer are acceptable. A drive root explicitly +supplied through `PATH` or another discovered environment variable is still +returned as a tool path. `user_profile_policy` discovers standard per-user tool +locations, and `temporary_files_policy` returns existing temporary storage +without creating directories. ## Internal modules and executor binaries diff --git a/src/mxc-sdk/src/lib.rs b/src/mxc-sdk/src/lib.rs index cef215ddf..3f4e89fb3 100644 --- a/src/mxc-sdk/src/lib.rs +++ b/src/mxc-sdk/src/lib.rs @@ -256,8 +256,9 @@ pub mod v1 { /// Host filesystem-policy discovery helpers and their options. pub mod filesystem { pub use crate::policy::{ - available_tools_policy, temporary_files_policy, user_profile_policy, - FilesystemPolicyResult, ToolsPolicyContainerType, ToolsPolicyOptions, + available_tools_policy, available_tools_policy_with_powershell_drive_root_read, + temporary_files_policy, user_profile_policy, FilesystemPolicyResult, + ToolsPolicyContainerType, ToolsPolicyOptions, }; } diff --git a/src/mxc-sdk/src/policy.rs b/src/mxc-sdk/src/policy.rs index 86b23be93..7d8ead4aa 100644 --- a/src/mxc-sdk/src/policy.rs +++ b/src/mxc-sdk/src/policy.rs @@ -281,16 +281,20 @@ 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. Add the synthetic system-drive root +/// (`C:\`) read-only only when `allow_drive_root_read` is true. /// /// Mirrors the SDK's `getPowerShellPolicy`. The system drive is read from the /// supplied environment (`SystemDrive`, defaulting to `C:`), as does `USERPROFILE`. /// /// On non-Windows, or when `pwsh.exe` is not on `path_dirs`, returns an empty /// policy. -fn powershell_policy(path_dirs: &[String], env: &[(String, String)]) -> FilesystemPolicyResult { +fn powershell_policy( + path_dirs: &[String], + env: &[(String, String)], + allow_drive_root_read: bool, +) -> FilesystemPolicyResult { if !is_windows() { return FilesystemPolicyResult::default(); } @@ -302,10 +306,14 @@ fn powershell_policy(path_dirs: &[String], env: &[(String, String)]) -> Filesyst return FilesystemPolicyResult::default(); } - let system_drive = env_get(env, "SystemDrive") - .filter(|s| !s.is_empty()) - .unwrap_or("C:"); - let readonly_paths = vec![format!("{system_drive}\\")]; + let readonly_paths = if allow_drive_root_read { + let system_drive = env_get(env, "SystemDrive") + .filter(|s| !s.is_empty()) + .unwrap_or("C:"); + vec![format!("{system_drive}\\")] + } else { + Vec::new() + }; let mut readwrite_paths: Vec = Vec::new(); if let Some(user_profile) = env_get(env, "USERPROFILE") { @@ -337,10 +345,36 @@ fn powershell_policy(path_dirs: &[String], env: &[(String, String)]) -> Filesyst /// when `pwsh.exe` is on `PATH`. The Rust port of `getAvailableToolsPolicy`. /// ProcessContainer filtering excludes existing ALL APPLICATION PACKAGES /// grants on Windows. If ACL inspection fails, retain the directory and emit -/// a diagnostic warning rather than assume it is already accessible. +/// a diagnostic warning rather than assume it is already accessible. The +/// synthetic system-drive root is omitted; a drive root explicitly present in +/// the supplied environment remains a discovered tool path. Use +/// [`available_tools_policy_with_powershell_drive_root_read`] only when the +/// compatibility grant is required. pub fn available_tools_policy( environment: Option<&[(String, String)]>, options: ToolsPolicyOptions, +) -> FilesystemPolicyResult { + available_tools_policy_impl(environment, options, false) +} + +/// Discover tool and SDK directories while opting into the legacy PowerShell +/// system-drive compatibility grant. +/// +/// When `pwsh.exe` is discovered, this adds the system-drive root to +/// `readonly_paths`. On BaseContainer that permits recursive whole-drive reads, +/// including unrelated user files. Prefer [`available_tools_policy`] unless +/// that broad access is explicitly acceptable. +pub fn available_tools_policy_with_powershell_drive_root_read( + environment: Option<&[(String, String)]>, + options: ToolsPolicyOptions, +) -> FilesystemPolicyResult { + available_tools_policy_impl(environment, options, true) +} + +fn available_tools_policy_impl( + environment: Option<&[(String, String)]>, + options: ToolsPolicyOptions, + allow_powershell_drive_root_read: bool, ) -> FilesystemPolicyResult { let env = env_or_process(environment); let env: &[(String, String)] = &env; @@ -372,7 +406,7 @@ pub fn available_tools_policy( }) .collect(); - let pwsh = powershell_policy(&path_dirs, env); + let pwsh = powershell_policy(&path_dirs, env, allow_powershell_drive_root_read); let mut readonly = filtered; readonly.extend(pwsh.readonly_paths); @@ -1183,7 +1217,7 @@ mod tests { #[cfg(target_os = "windows")] #[test] - fn powershell_policy_grants_system_drive_root() { + fn powershell_policy_grants_system_drive_root_only_when_opted_in() { use super::powershell_policy; use std::fs; use std::path::PathBuf; @@ -1203,31 +1237,31 @@ mod tests { let ps_home_str = ps_home.to_string_lossy().into_owned(); let env = vec![("USERPROFILE".to_string(), "C:\\Users\\example".to_string())]; - let result = powershell_policy(std::slice::from_ref(&ps_home_str), &env); + let default_result = powershell_policy(std::slice::from_ref(&ps_home_str), &env, false); + let opted_in_result = powershell_policy(std::slice::from_ref(&ps_home_str), &env, true); // 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. + assert!(default_result.readonly_paths.is_empty()); assert!( - result.readonly_paths.iter().any(|p| { + opted_in_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 - ); - // PSReadLine command history stays read-write. - assert!( - result - .readwrite_paths - .iter() - .any(|p| p.contains("PSReadLine")), - "expected PSReadLine history in readwrite paths: {:?}", - result.readwrite_paths + opted_in_result.readonly_paths ); + for result in [default_result, opted_in_result] { + assert!( + result + .readwrite_paths + .iter() + .any(|p| p.contains("PSReadLine")), + "expected PSReadLine history in readwrite paths: {:?}", + result.readwrite_paths + ); + } } use super::{ diff --git a/src/mxc-sdk/tests/sdk_helpers.rs b/src/mxc-sdk/tests/sdk_helpers.rs index 35b897f33..4252ac049 100644 --- a/src/mxc-sdk/tests/sdk_helpers.rs +++ b/src/mxc-sdk/tests/sdk_helpers.rs @@ -5,7 +5,8 @@ use mxc_sdk::v1::platform_support; use mxc_sdk::v1::policy::filesystem::{ - available_tools_policy, temporary_files_policy, user_profile_policy, + available_tools_policy, available_tools_policy_with_powershell_drive_root_read, + temporary_files_policy, user_profile_policy, }; #[cfg(any(target_os = "windows", target_os = "macos"))] use mxc_sdk::v1::ContainerRequest; @@ -171,6 +172,7 @@ fn tool_options_filter_all_application_packages_without_changing_default() { Some(&environment), ToolsPolicyOptions { container_type: Some(ToolsPolicyContainerType::ProcessContainer), + ..Default::default() }, ); std::fs::remove_dir_all(&directory).unwrap(); @@ -180,6 +182,57 @@ fn tool_options_filter_all_application_packages_without_changing_default() { assert!(filtered.readonly_paths.is_empty()); } +#[cfg(target_os = "windows")] +#[test] +fn powershell_drive_root_read_is_opt_in() { + let directory = std::env::temp_dir().join(format!("mxc-pwsh-{}", std::process::id())); + std::fs::create_dir_all(&directory).unwrap(); + std::fs::write(directory.join("pwsh.exe"), []).unwrap(); + let environment = env_pairs(&[ + ("PATH", &directory.to_string_lossy()), + ("SystemDrive", "D:"), + ("USERPROFILE", r"C:\Users\TestUser"), + ]); + + let default_result = available_tools_policy(Some(&environment), Default::default()); + let opted_in_result = available_tools_policy_with_powershell_drive_root_read( + Some(&environment), + Default::default(), + ); + std::fs::remove_dir_all(&directory).unwrap(); + + let tool_directory = directory.to_string_lossy().into_owned(); + assert_eq!(default_result.readonly_paths, [tool_directory.clone()]); + assert_eq!( + opted_in_result.readonly_paths, + [tool_directory, r"D:\".to_string()] + ); + let history = r"C:\Users\TestUser\AppData\Roaming\Microsoft\Windows\PowerShell\PSReadLine"; + assert_eq!(default_result.readwrite_paths, [history.to_string()]); + assert_eq!(opted_in_result.readwrite_paths, [history.to_string()]); +} + +#[cfg(target_os = "windows")] +#[test] +fn available_tools_policy_preserves_explicit_drive_root() { + let pwsh_directory = + std::env::temp_dir().join(format!("mxc-pwsh-explicit-{}", std::process::id())); + std::fs::create_dir_all(&pwsh_directory).unwrap(); + std::fs::write(pwsh_directory.join("pwsh.exe"), []).unwrap(); + let system_drive = std::env::var("SystemDrive").unwrap_or_else(|_| "C:".to_string()); + let drive_root = format!("{system_drive}\\"); + let path = format!("{drive_root};{}", pwsh_directory.to_string_lossy()); + let environment = env_pairs(&[("PATH", &path)]); + + let result = available_tools_policy(Some(&environment), Default::default()); + std::fs::remove_dir_all(&pwsh_directory).unwrap(); + + assert_eq!( + result.readonly_paths, + [drive_root, pwsh_directory.to_string_lossy().into_owned()] + ); +} + #[cfg(target_os = "macos")] #[test] fn build_request_then_run_seatbelt() {