From 7574d9d98c101b3397dfcf15a9056cd2a2117207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Wed, 28 Oct 2026 09:45:06 -0400 Subject: [PATCH 1/3] fix(agent): find Store pwsh and fix PowerShell arguments The package broker only reported PowerShell 7 as available when the MSI installation existed under Program Files, so machines with the Microsoft Store installation could not broker PowerShell 7 operations. The broker now falls back to the user's pwsh.exe app execution alias, validated against the Microsoft.PowerShell package family like the winget.exe alias, and launches the verified WindowsApps target. Several generated PowerShell commands passed parameters the cmdlets do not accept, so those operations always failed: -Repository on Uninstall-PSResource, Uninstall-Module and Update-Module, and -SkipPublisherCheck on Update-Module. Update-Module updates from the repository a module was installed from, so the Windows PowerShell update first checks that this repository is the policy-matched source and fails otherwise. Co-Authored-By: Claude Opus 5.5 --- .../src/command_builder/powershell.rs | 78 +++++++++++++++-- .../src/executor/windows/mod.rs | 43 +++++++++- .../now-package-broker/src/policy_security.rs | 85 +++++++++++++++++-- 3 files changed, 187 insertions(+), 19 deletions(-) diff --git a/crates/now-package-broker/src/command_builder/powershell.rs b/crates/now-package-broker/src/command_builder/powershell.rs index a4ebe172c0..d7cdcd50a5 100644 --- a/crates/now-package-broker/src/command_builder/powershell.rs +++ b/crates/now-package-broker/src/command_builder/powershell.rs @@ -9,8 +9,11 @@ //! //! The Windows executor materializes these scripts into temporary `.ps1` files before launch. //! -//! Both builders bind the policy-matched request source as the PowerShell repository -//! and pin the invariant culture so repository lookup does not depend on the host locale. +//! Both builders bind the policy-matched request source as the PowerShell repository and +//! pin the invariant culture so repository lookup does not depend on the host locale. +//! Uninstall removes the local module and takes no `-Repository`. `Update-Module` takes no +//! `-Repository` either, as it updates from the repository the module was installed from, +//! so the PowerShell 5 update first checks that this repository is the requested source. //! Options that don't apply to PowerShell modules (interactive, custom install //! location, winget's no-upgrade / uninstall-previous) are intentionally omitted. @@ -33,6 +36,10 @@ pub fn build_powershell5_command(request: &PackageRequest) -> anyhow::Result "Install-Module", Operation::Update => "Update-Module", @@ -41,7 +48,9 @@ pub fn build_powershell5_command(request: &PackageRequest) -> anyhow::Result anyhow::Result anyhow::Result anyhow::R Ok(()) } +/// Fail the script unless the installed module was installed from the requested source, +/// which is the repository `Update-Module` updates from. +fn append_installed_repository_check(script: &mut String, request: &PackageRequest) -> anyhow::Result<()> { + let repository = request.source.name.trim(); + if repository.is_empty() { + bail!("PowerShell package source name is required"); + } + append_raw( + script, + &format!( + "if (@(Get-InstalledModule -Name {} -ErrorAction Stop | Where-Object {{ $_.Repository -ne {} }}).Count -ne 0) {{ throw 'The installed module was not installed from the requested repository.' }};", + quote_powershell_literal(&request.package.id.0), + quote_powershell_literal(repository), + ), + ); + Ok(()) +} + fn append_raw(script: &mut String, value: &str) { if !script.is_empty() { script.push(' '); @@ -279,14 +309,46 @@ mod tests { request.operation = Operation::Uninstall; let cmd = build_powershell5_command(&request).expect("build command"); let script = script_of(&cmd); + // `Uninstall-Module` has no `-Repository` parameter. assert!(script.starts_with(&format!( - "{INVARIANT_CULTURE_PREFIX} Uninstall-Module -Name 'Pester' -Repository 'PSGallery'" + "{INVARIANT_CULTURE_PREFIX} Uninstall-Module -Name 'Pester' -Confirm:$false -Force" ))); + assert!(!script.contains("-Repository")); assert!(!script.contains("-Scope")); assert!(!script.contains("-RequiredVersion")); assert!(!script.contains("-SkipPublisherCheck")); } + #[test] + fn powershell5_update_checks_installed_repository_instead_of_passing_it() { + let mut request = make_request(ManagerName::PowerShell); + request.operation = Operation::Update; + request.options.skip_hash_check = true; + let cmd = build_powershell5_command(&request).expect("build command"); + let script = script_of(&cmd); + assert!(script.starts_with(&format!( + "{INVARIANT_CULTURE_PREFIX} if (@(Get-InstalledModule -Name 'Pester' -ErrorAction Stop | Where-Object {{ $_.Repository -ne 'PSGallery' }}).Count -ne 0) {{ throw " + ))); + // `Update-Module` has neither `-Repository` nor `-SkipPublisherCheck`. + let update = &script[script.find("Update-Module").expect("Update-Module call")..]; + assert!(update.starts_with("Update-Module -Name 'Pester' -Confirm:$false -Force")); + assert!(!update.contains("-Repository")); + assert!(!update.contains("-SkipPublisherCheck")); + assert!(update.contains("-Scope CurrentUser")); + assert!(update.contains("-RequiredVersion '5.6.0'")); + } + + #[test] + fn powershell5_update_repository_check_escapes_literals() { + let mut request = make_request(ManagerName::PowerShell); + request.operation = Operation::Update; + request.package.id = PackageIdentifier::from("Vendor's.Module".to_owned()); + let cmd = build_powershell5_command(&request).expect("build command"); + let script = script_of(&cmd); + assert!(script.contains("Get-InstalledModule -Name 'Vendor''s.Module'")); + assert!(script.contains("Update-Module -Name 'Vendor''s.Module'")); + } + #[test] fn powershell7_install_matches_package_broker_client_semantics() { let request = make_request(ManagerName::PowerShell7); @@ -320,9 +382,11 @@ mod tests { request.operation = Operation::Uninstall; let cmd = build_powershell7_command(&request).expect("build command"); let script = script_of(&cmd); + // `Uninstall-PSResource` has no `-Repository` parameter. assert!(script.starts_with(&format!( - "{INVARIANT_CULTURE_PREFIX} Uninstall-PSResource -Name 'Pester' -Repository 'PSGallery' -Confirm:$false" + "{INVARIANT_CULTURE_PREFIX} Uninstall-PSResource -Name 'Pester' -Confirm:$false" ))); + assert!(!script.contains("-Repository")); // Uninstall must not pin a version (it would remove only the matching version, // or fail if it does not match what is installed). assert!(!script.contains("-Version")); diff --git a/crates/now-package-broker/src/executor/windows/mod.rs b/crates/now-package-broker/src/executor/windows/mod.rs index 1d4657be89..a0cdec5826 100644 --- a/crates/now-package-broker/src/executor/windows/mod.rs +++ b/crates/now-package-broker/src/executor/windows/mod.rs @@ -181,7 +181,7 @@ fn manager_is_available(manager: ManagerName, user_env: &UserEnv<'_>) -> bool { && path_contains_executable(user_env, &["npm.cmd", "npm.exe"]) } ManagerName::PowerShell => Path::new(&trusted_windows_powershell_executable()).is_file(), - ManagerName::PowerShell7 => Path::new(&trusted_powershell7_executable()).is_file(), + ManagerName::PowerShell7 => resolve_powershell7_executable(user_env).is_ok(), // Scoop is driven through the user's `scoop.ps1` shim (resolved via `Get-Command // -CommandType ExternalScript`) inside the trusted Windows PowerShell wrapper. ManagerName::Scoop => { @@ -468,7 +468,7 @@ fn prepare_main_command_in( requires_elevation: bool, ) -> anyhow::Result { if let Some((script, command_arg_index)) = powershell_inline_script(command) { - return prepare_powershell_script(command, command_arg_index, script, temp_dir); + return prepare_powershell_script(command, command_arg_index, script, temp_dir, user_env); } if executable_is(command, "winget.exe") { @@ -542,6 +542,7 @@ fn prepare_powershell_script( command_arg_index: usize, script: &str, temp_dir: Option<&Path>, + user_env: Option<&UserEnv<'_>>, ) -> anyhow::Result { command.first().context("empty PowerShell command")?; let is_windows_powershell = executable_is(command, "powershell.exe"); @@ -567,7 +568,13 @@ fn prepare_powershell_script( prepared[0] = if is_windows_powershell { trusted_windows_powershell_executable() } else { - trusted_powershell7_executable() + user_env + .map_or_else( + || Ok(PathBuf::from(trusted_powershell7_executable())), + resolve_powershell7_executable, + )? + .display() + .to_string() }; prepared.push("-Command".to_owned()); prepared.push(format!("& {}", quote_powershell_literal(&temp_script.path_string()))); @@ -1116,6 +1123,36 @@ fn trusted_powershell7_executable() -> String { .to_string() } +/// Resolve PowerShell 7 for the target user, mirroring how the client finds `pwsh.exe`. +/// +/// The machine-wide MSI installation is preferred. Otherwise, the Microsoft Store +/// installation is used through the user's `pwsh.exe` app execution alias; it is only +/// accepted when it validates as an alias of the Microsoft PowerShell package, and the +/// launch path substitutes the verified `WindowsApps` target for it. +fn resolve_powershell7_executable(user_env: &UserEnv<'_>) -> anyhow::Result { + let msi = PathBuf::from(trusted_powershell7_executable()); + if msi.is_file() { + return Ok(msi); + } + + let alias = user_env + .var("LOCALAPPDATA") + .filter(|value| !value.trim().is_empty()) + .map(|local_app_data| PathBuf::from(local_app_data).join(r"Microsoft\WindowsApps\pwsh.exe")); + if let Some(alias) = alias + && policy_security::app_exec_alias_kind(user_env, &alias) == Some(policy_security::AppExecAliasKind::App) + { + policy_security::pin_launch_executable(user_env, &alias, false) + .with_context(|| format!("PowerShell 7 app execution alias is not usable: {}", alias.display()))?; + return Ok(alias); + } + + bail!( + "PowerShell 7 was not found at {} nor as a Microsoft Store installation for the target user", + msi.display() + ); +} + fn system_program_data_dir() -> anyhow::Result { use windows::Win32::System::Com::CoTaskMemFree; use windows::Win32::UI::Shell::{FOLDERID_ProgramData, KF_FLAG_DEFAULT, SHGetKnownFolderPath}; diff --git a/crates/now-package-broker/src/policy_security.rs b/crates/now-package-broker/src/policy_security.rs index 78f4aa44b0..c8a9fad6fb 100644 --- a/crates/now-package-broker/src/policy_security.rs +++ b/crates/now-package-broker/src/policy_security.rs @@ -149,6 +149,20 @@ const MAXIMUM_REPARSE_DATA_BUFFER_SIZE: usize = 16 * 1024; /// publisher can install a package under this family. const WINGET_PACKAGE_FAMILY: &str = "Microsoft.DesktopAppInstaller_8wekyb3d8bbwe"; +/// Package family name of the Microsoft Store PowerShell 7 package that delivers `pwsh.exe`. +const POWERSHELL7_PACKAGE_FAMILY: &str = "Microsoft.PowerShell_8wekyb3d8bbwe"; + +/// Package family a supported app execution alias must belong to, by alias file name. +fn trusted_alias_package_family(alias_name: &str) -> Option<&'static str> { + if alias_name.eq_ignore_ascii_case("winget.exe") { + Some(WINGET_PACKAGE_FAMILY) + } else if alias_name.eq_ignore_ascii_case("pwsh.exe") { + Some(POWERSHELL7_PACKAGE_FAMILY) + } else { + None + } +} + /// Security descriptor allocated by `GetSecurityInfo`, freed with `LocalFree` on drop. struct OwnedSecurityDescriptor(PSECURITY_DESCRIPTOR); @@ -955,9 +969,10 @@ fn resolve_app_exec_alias(opener: &dyn PathOpener, path: &Path) -> Option Date: Wed, 28 Oct 2026 09:51:10 -0400 Subject: [PATCH 2/3] test(agent): mark the sources of the new PowerShell tests Co-Authored-By: Claude Opus 5.5 --- .../now-package-broker/src/command_builder/powershell.rs | 8 +++++--- crates/now-package-broker/src/policy_security.rs | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/now-package-broker/src/command_builder/powershell.rs b/crates/now-package-broker/src/command_builder/powershell.rs index d7cdcd50a5..894e2f85ad 100644 --- a/crates/now-package-broker/src/command_builder/powershell.rs +++ b/crates/now-package-broker/src/command_builder/powershell.rs @@ -309,7 +309,7 @@ mod tests { request.operation = Operation::Uninstall; let cmd = build_powershell5_command(&request).expect("build command"); let script = script_of(&cmd); - // `Uninstall-Module` has no `-Repository` parameter. + // oracle: reference PowerShellGet 2.2.5 and 1.0.0.1 `Uninstall-Module`, which have no `-Repository` parameter assert!(script.starts_with(&format!( "{INVARIANT_CULTURE_PREFIX} Uninstall-Module -Name 'Pester' -Confirm:$false -Force" ))); @@ -326,18 +326,20 @@ mod tests { request.options.skip_hash_check = true; let cmd = build_powershell5_command(&request).expect("build command"); let script = script_of(&cmd); + // oracle: mutant dropping the installed-repository check that binds `Update-Module` to the requested source assert!(script.starts_with(&format!( "{INVARIANT_CULTURE_PREFIX} if (@(Get-InstalledModule -Name 'Pester' -ErrorAction Stop | Where-Object {{ $_.Repository -ne 'PSGallery' }}).Count -ne 0) {{ throw " ))); - // `Update-Module` has neither `-Repository` nor `-SkipPublisherCheck`. let update = &script[script.find("Update-Module").expect("Update-Module call")..]; assert!(update.starts_with("Update-Module -Name 'Pester' -Confirm:$false -Force")); + // oracle: reference PowerShellGet 2.2.5 and 1.0.0.1 `Update-Module`, which have neither a `-Repository` nor a `-SkipPublisherCheck` parameter assert!(!update.contains("-Repository")); assert!(!update.contains("-SkipPublisherCheck")); assert!(update.contains("-Scope CurrentUser")); assert!(update.contains("-RequiredVersion '5.6.0'")); } + // oracle: spec PowerShell about_Quoting_Rules §Single-quoted strings #[test] fn powershell5_update_repository_check_escapes_literals() { let mut request = make_request(ManagerName::PowerShell); @@ -382,7 +384,7 @@ mod tests { request.operation = Operation::Uninstall; let cmd = build_powershell7_command(&request).expect("build command"); let script = script_of(&cmd); - // `Uninstall-PSResource` has no `-Repository` parameter. + // oracle: reference PSResourceGet 1.2.0 `Uninstall-PSResource`, which has no `-Repository` parameter assert!(script.starts_with(&format!( "{INVARIANT_CULTURE_PREFIX} Uninstall-PSResource -Name 'Pester' -Confirm:$false" ))); diff --git a/crates/now-package-broker/src/policy_security.rs b/crates/now-package-broker/src/policy_security.rs index c8a9fad6fb..22c05144b6 100644 --- a/crates/now-package-broker/src/policy_security.rs +++ b/crates/now-package-broker/src/policy_security.rs @@ -1944,6 +1944,7 @@ mod tests { assert!(target.ends_with("winget.exe")); } + // oracle: reference the `pwsh.exe` app execution alias of Microsoft Store PowerShell 7.6.9, read with `fsutil reparsepoint query` #[test] fn pwsh_alias_with_expected_identity_is_validated() { let alias_path = Path::new(r"C:\Users\user\AppData\Local\Microsoft\WindowsApps\pwsh.exe"); @@ -1957,6 +1958,7 @@ mod tests { assert!(target.ends_with("pwsh.exe")); } + // oracle: mutant accepting a `pwsh.exe` alias bound to the App Installer package or to the PowerShell Preview package #[test] fn pwsh_alias_bound_to_another_package_is_rejected() { let alias_path = Path::new(r"C:\Users\user\AppData\Local\Microsoft\WindowsApps\pwsh.exe"); @@ -1980,6 +1982,7 @@ mod tests { } } + // oracle: reference the `pwsh.exe` app execution alias Microsoft Store PowerShell 7 installs, compared live #[test] fn pwsh_app_exec_alias_resolves_to_windowsapps_target() { // Opportunistic: only runs where the Microsoft Store PowerShell 7 alias is installed. From fd490b16f3b381f1e09501fd2e763139e82185c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Wed, 28 Oct 2026 10:02:20 -0400 Subject: [PATCH 3/3] fix(agent): bind PowerShell uninstalls and pwsh launch Check the installed repository before Uninstall-Module and Uninstall-PSResource, which take no -Repository, and list prerelease versions in the Windows PowerShell check. Launch the Store pwsh from the alias target pinned when the alias was validated, so replacing the alias afterwards cannot redirect the launch. Co-Authored-By: Claude Opus 5.5 --- .../src/command_builder/powershell.rs | 147 +++++++++++++----- .../src/executor/windows/mod.rs | 50 +++--- .../now-package-broker/src/policy_security.rs | 60 ++++++- 3 files changed, 202 insertions(+), 55 deletions(-) diff --git a/crates/now-package-broker/src/command_builder/powershell.rs b/crates/now-package-broker/src/command_builder/powershell.rs index 894e2f85ad..13572c6dc7 100644 --- a/crates/now-package-broker/src/command_builder/powershell.rs +++ b/crates/now-package-broker/src/command_builder/powershell.rs @@ -11,9 +11,9 @@ //! //! Both builders bind the policy-matched request source as the PowerShell repository and //! pin the invariant culture so repository lookup does not depend on the host locale. -//! Uninstall removes the local module and takes no `-Repository`. `Update-Module` takes no -//! `-Repository` either, as it updates from the repository the module was installed from, -//! so the PowerShell 5 update first checks that this repository is the requested source. +//! Uninstall and `Update-Module` take no `-Repository`: they act on installed modules, and +//! `Update-Module` updates from the repository a module was installed from. Those commands +//! first check that every installed version came from the requested source instead. //! Options that don't apply to PowerShell modules (interactive, custom install //! location, winget's no-upgrade / uninstall-previous) are intentionally omitted. @@ -36,8 +36,15 @@ pub fn build_powershell5_command(request: &PackageRequest) -> anyhow::Result anyhow::Result "Install-PSResource", Operation::Update => "Update-PSResource", @@ -181,9 +196,14 @@ fn append_repository(script: &mut String, request: &PackageRequest) -> anyhow::R Ok(()) } -/// Fail the script unless the installed module was installed from the requested source, -/// which is the repository `Update-Module` updates from. -fn append_installed_repository_check(script: &mut String, request: &PackageRequest) -> anyhow::Result<()> { +/// Fail the script unless every installed version that `lookup` lists was installed from +/// the requested source, so a command without `-Repository` only acts on modules from the +/// source that policy evaluation matched. +fn append_installed_repository_check( + script: &mut String, + lookup: &str, + request: &PackageRequest, +) -> anyhow::Result<()> { let repository = request.source.name.trim(); if repository.is_empty() { bail!("PowerShell package source name is required"); @@ -191,8 +211,7 @@ fn append_installed_repository_check(script: &mut String, request: &PackageReque append_raw( script, &format!( - "if (@(Get-InstalledModule -Name {} -ErrorAction Stop | Where-Object {{ $_.Repository -ne {} }}).Count -ne 0) {{ throw 'The installed module was not installed from the requested repository.' }};", - quote_powershell_literal(&request.package.id.0), + "if (@({lookup} | Where-Object {{ $_.Repository -ne {} }}).Count -ne 0) {{ throw 'The installed module was not installed from the requested repository.' }};", quote_powershell_literal(repository), ), ); @@ -269,6 +288,11 @@ mod tests { &cmd[3] } + /// Split a script into the statements before the `verb` call and the call itself. + fn split_at_call<'a>(script: &'a str, verb: &str) -> (&'a str, &'a str) { + script.split_at(script.find(verb).expect("package command call")) + } + #[test] fn powershell5_install_matches_package_broker_client_semantics() { let request = make_request(ManagerName::PowerShell); @@ -308,29 +332,84 @@ mod tests { let mut request = make_request(ManagerName::PowerShell); request.operation = Operation::Uninstall; let cmd = build_powershell5_command(&request).expect("build command"); - let script = script_of(&cmd); + let (_, uninstall) = split_at_call(script_of(&cmd), "Uninstall-Module"); // oracle: reference PowerShellGet 2.2.5 and 1.0.0.1 `Uninstall-Module`, which have no `-Repository` parameter - assert!(script.starts_with(&format!( - "{INVARIANT_CULTURE_PREFIX} Uninstall-Module -Name 'Pester' -Confirm:$false -Force" - ))); - assert!(!script.contains("-Repository")); - assert!(!script.contains("-Scope")); - assert!(!script.contains("-RequiredVersion")); - assert!(!script.contains("-SkipPublisherCheck")); + assert!(uninstall.starts_with("Uninstall-Module -Name 'Pester' -Confirm:$false -Force")); + assert!(!uninstall.contains("-Repository")); + assert!(!uninstall.contains("-Scope")); + assert!(!uninstall.contains("-RequiredVersion")); + assert!(!uninstall.contains("-SkipPublisherCheck")); } + // oracle: mutant dropping the installed-repository check that binds a command without `-Repository` to the requested source #[test] - fn powershell5_update_checks_installed_repository_instead_of_passing_it() { + fn powershell_commands_without_repository_check_installed_repository() { + for (manager, operation, verb, lookup) in [ + ( + ManagerName::PowerShell, + Operation::Update, + "Update-Module", + "Get-InstalledModule @installedModuleQuery", + ), + ( + ManagerName::PowerShell, + Operation::Uninstall, + "Uninstall-Module", + "Get-InstalledModule @installedModuleQuery", + ), + ( + ManagerName::PowerShell7, + Operation::Uninstall, + "Uninstall-PSResource", + "Get-InstalledPSResource -Name 'Pester' -ErrorAction Stop", + ), + ] { + let mut request = make_request(manager); + request.operation = operation; + let cmd = crate::command_builder::build_command(&request).expect("build command"); + let (check, _) = split_at_call(script_of(&cmd), verb); + assert!( + check.contains(&format!( + "if (@({lookup} | Where-Object {{ $_.Repository -ne 'PSGallery' }}).Count -ne 0) {{ throw " + )), + "{verb}: {check}" + ); + } + + for (manager, operation) in [ + (ManagerName::PowerShell, Operation::Install), + (ManagerName::PowerShell7, Operation::Install), + (ManagerName::PowerShell7, Operation::Update), + ] { + let mut request = make_request(manager); + request.operation = operation; + let cmd = crate::command_builder::build_command(&request).expect("build command"); + assert!(!script_of(&cmd).contains("Get-Installed"), "{manager:?} {operation:?}"); + } + } + + #[test] + fn powershell5_installed_module_lookup_lists_every_version() { + for operation in [Operation::Update, Operation::Uninstall] { + let mut request = make_request(ManagerName::PowerShell); + request.operation = operation; + let cmd = build_powershell5_command(&request).expect("build command"); + let script = script_of(&cmd); + assert!(script.contains("$installedModuleQuery = @{ Name = 'Pester' ; AllVersions = $true;")); + // oracle: reference PowerShellGet 2.2.5 `Get-InstalledModule`, which lists prerelease versions only with `-AllowPrerelease`, a parameter PowerShellGet 1.0.0.1 lacks + assert!(script.contains( + "if ((Get-Command Get-InstalledModule).Parameters.ContainsKey('AllowPrerelease')) { $installedModuleQuery.AllowPrerelease = $true };" + )); + } + } + + #[test] + fn powershell5_update_omits_unsupported_parameters() { let mut request = make_request(ManagerName::PowerShell); request.operation = Operation::Update; request.options.skip_hash_check = true; let cmd = build_powershell5_command(&request).expect("build command"); - let script = script_of(&cmd); - // oracle: mutant dropping the installed-repository check that binds `Update-Module` to the requested source - assert!(script.starts_with(&format!( - "{INVARIANT_CULTURE_PREFIX} if (@(Get-InstalledModule -Name 'Pester' -ErrorAction Stop | Where-Object {{ $_.Repository -ne 'PSGallery' }}).Count -ne 0) {{ throw " - ))); - let update = &script[script.find("Update-Module").expect("Update-Module call")..]; + let (_, update) = split_at_call(script_of(&cmd), "Update-Module"); assert!(update.starts_with("Update-Module -Name 'Pester' -Confirm:$false -Force")); // oracle: reference PowerShellGet 2.2.5 and 1.0.0.1 `Update-Module`, which have neither a `-Repository` nor a `-SkipPublisherCheck` parameter assert!(!update.contains("-Repository")); @@ -347,7 +426,7 @@ mod tests { request.package.id = PackageIdentifier::from("Vendor's.Module".to_owned()); let cmd = build_powershell5_command(&request).expect("build command"); let script = script_of(&cmd); - assert!(script.contains("Get-InstalledModule -Name 'Vendor''s.Module'")); + assert!(script.contains("$installedModuleQuery = @{ Name = 'Vendor''s.Module' ;")); assert!(script.contains("Update-Module -Name 'Vendor''s.Module'")); } @@ -383,18 +462,16 @@ mod tests { let mut request = make_request(ManagerName::PowerShell7); request.operation = Operation::Uninstall; let cmd = build_powershell7_command(&request).expect("build command"); - let script = script_of(&cmd); + let (_, uninstall) = split_at_call(script_of(&cmd), "Uninstall-PSResource"); // oracle: reference PSResourceGet 1.2.0 `Uninstall-PSResource`, which has no `-Repository` parameter - assert!(script.starts_with(&format!( - "{INVARIANT_CULTURE_PREFIX} Uninstall-PSResource -Name 'Pester' -Confirm:$false" - ))); - assert!(!script.contains("-Repository")); + assert!(uninstall.starts_with("Uninstall-PSResource -Name 'Pester' -Confirm:$false")); + assert!(!uninstall.contains("-Repository")); // Uninstall must not pin a version (it would remove only the matching version, // or fail if it does not match what is installed). - assert!(!script.contains("-Version")); - assert!(!script.contains("-TrustRepository")); - assert!(!script.contains("-AcceptLicense")); - assert!(!script.contains("-Scope")); + assert!(!uninstall.contains("-Version")); + assert!(!uninstall.contains("-TrustRepository")); + assert!(!uninstall.contains("-AcceptLicense")); + assert!(!uninstall.contains("-Scope")); } #[test] diff --git a/crates/now-package-broker/src/executor/windows/mod.rs b/crates/now-package-broker/src/executor/windows/mod.rs index a0cdec5826..757fc015f8 100644 --- a/crates/now-package-broker/src/executor/windows/mod.rs +++ b/crates/now-package-broker/src/executor/windows/mod.rs @@ -181,7 +181,7 @@ fn manager_is_available(manager: ManagerName, user_env: &UserEnv<'_>) -> bool { && path_contains_executable(user_env, &["npm.cmd", "npm.exe"]) } ManagerName::PowerShell => Path::new(&trusted_windows_powershell_executable()).is_file(), - ManagerName::PowerShell7 => resolve_powershell7_executable(user_env).is_ok(), + ManagerName::PowerShell7 => resolve_powershell7_executable(user_env, false).is_ok(), // Scoop is driven through the user's `scoop.ps1` shim (resolved via `Get-Command // -CommandType ExternalScript`) inside the trusted Windows PowerShell wrapper. ManagerName::Scoop => { @@ -468,7 +468,14 @@ fn prepare_main_command_in( requires_elevation: bool, ) -> anyhow::Result { if let Some((script, command_arg_index)) = powershell_inline_script(command) { - return prepare_powershell_script(command, command_arg_index, script, temp_dir, user_env); + return prepare_powershell_script( + command, + command_arg_index, + script, + temp_dir, + user_env, + requires_elevation, + ); } if executable_is(command, "winget.exe") { @@ -543,6 +550,7 @@ fn prepare_powershell_script( script: &str, temp_dir: Option<&Path>, user_env: Option<&UserEnv<'_>>, + requires_elevation: bool, ) -> anyhow::Result { command.first().context("empty PowerShell command")?; let is_windows_powershell = executable_is(command, "powershell.exe"); @@ -565,21 +573,21 @@ fn prepare_powershell_script( } let mut prepared = command[..command_arg_index].to_vec(); + let mut exe_guard = None; prepared[0] = if is_windows_powershell { trusted_windows_powershell_executable() } else { - user_env - .map_or_else( - || Ok(PathBuf::from(trusted_powershell7_executable())), - resolve_powershell7_executable, - )? - .display() - .to_string() + let (executable, guard) = user_env.map_or_else( + || Ok((PathBuf::from(trusted_powershell7_executable()), None)), + |env| resolve_powershell7_executable(env, requires_elevation), + )?; + exe_guard = guard; + executable.display().to_string() }; prepared.push("-Command".to_owned()); prepared.push(format!("& {}", quote_powershell_literal(&temp_script.path_string()))); - Ok(PreparedCommand::with_script(prepared, temp_script)) + Ok(PreparedCommand::with_script(prepared, temp_script).with_exe_guard(exe_guard)) } fn powershell_script_with_utf8_preamble(script: &str) -> String { @@ -1127,24 +1135,28 @@ fn trusted_powershell7_executable() -> String { /// /// The machine-wide MSI installation is preferred. Otherwise, the Microsoft Store /// installation is used through the user's `pwsh.exe` app execution alias; it is only -/// accepted when it validates as an alias of the Microsoft PowerShell package, and the -/// launch path substitutes the verified `WindowsApps` target for it. -fn resolve_powershell7_executable(user_env: &UserEnv<'_>) -> anyhow::Result { +/// accepted when it validates as an alias of the Microsoft PowerShell package. The +/// returned path is then the pinned `WindowsApps` target, and the guard keeps it pinned +/// until the command has finished running. +fn resolve_powershell7_executable( + user_env: &UserEnv<'_>, + requires_elevation: bool, +) -> anyhow::Result<(PathBuf, Option)> { let msi = PathBuf::from(trusted_powershell7_executable()); if msi.is_file() { - return Ok(msi); + return Ok((msi, None)); } let alias = user_env .var("LOCALAPPDATA") .filter(|value| !value.trim().is_empty()) .map(|local_app_data| PathBuf::from(local_app_data).join(r"Microsoft\WindowsApps\pwsh.exe")); - if let Some(alias) = alias - && policy_security::app_exec_alias_kind(user_env, &alias) == Some(policy_security::AppExecAliasKind::App) - { - policy_security::pin_launch_executable(user_env, &alias, false) + if let Some(alias) = alias { + let guard = policy_security::pin_app_exec_alias_target(user_env, &alias, requires_elevation) .with_context(|| format!("PowerShell 7 app execution alias is not usable: {}", alias.display()))?; - return Ok(alias); + if let Some(guard) = guard { + return Ok((guard.path().to_owned(), Some(guard))); + } } bail!( diff --git a/crates/now-package-broker/src/policy_security.rs b/crates/now-package-broker/src/policy_security.rs index 22c05144b6..22a86f3487 100644 --- a/crates/now-package-broker/src/policy_security.rs +++ b/crates/now-package-broker/src/policy_security.rs @@ -832,6 +832,33 @@ pub(crate) fn pin_launch_executable( ) } +/// Pin the target of the Microsoft Store app execution alias at `path`, verifying it when it +/// runs elevated. +/// +/// Returns `None` when `path` is not an alias. The alias is read once, and only the target +/// that `validate_app_exec_alias` bound to the identity expected for the alias name is +/// pinned, so replacing the alias afterwards cannot redirect the launch. The returned guard +/// must stay alive until the process has been created. +pub(crate) fn pin_app_exec_alias_target( + opener: &dyn PathOpener, + path: &Path, + requires_elevation: bool, +) -> anyhow::Result> { + let Some(alias) = resolve_app_exec_alias(opener, path) else { + return Ok(None); + }; + let target = validate_app_exec_alias(path, alias, requires_elevation)?; + if let Some(verified) = verify_elevated_executable_security(opener, &target, requires_elevation)? { + return Ok(Some(verified)); + } + pin_executable( + opener, + &target, + &format!("package-manager executable '{}'", target.display()), + ) + .map(Some) +} + /// Kind of Microsoft Store app execution alias found at a path. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum AppExecAliasKind { @@ -1994,11 +2021,42 @@ mod tests { return; } - let guard = pin_launch_executable(&ServiceOpener, &alias_path, false).expect("pwsh alias must be launchable"); + let guard = pin_app_exec_alias_target(&ServiceOpener, &alias_path, false) + .expect("pwsh alias must be launchable") + .expect("pwsh alias must be an alias"); assert!(guard.path().ends_with("pwsh.exe")); assert_ne!(guard.path(), alias_path); } + // oracle: mutant pinning a regular file or an alias of another package in place of a validated alias target + #[test] + fn alias_target_pinning_requires_a_validated_alias() { + let root = tempfile::tempdir().expect("create alias directory"); + let plain = root.path().join("pwsh.exe"); + std::fs::write(&plain, b"").expect("write plain file"); + assert!( + pin_app_exec_alias_target(&ServiceOpener, &plain, false) + .expect("a regular file is not an alias") + .is_none() + ); + + let foreign = root.path().join("foreign").join("pwsh.exe"); + std::fs::create_dir_all(foreign.parent().expect("alias parent")).expect("create alias parent"); + create_app_exec_alias_for_tests( + &foreign, + WINGET_PACKAGE_FAMILY, + Path::new( + r"C:\Program Files\WindowsApps\Microsoft.DesktopAppInstaller_1.0.0.0_x64__8wekyb3d8bbwe\pwsh.exe", + ), + ); + let error = + pin_app_exec_alias_target(&ServiceOpener, &foreign, false).expect_err("foreign alias must be rejected"); + assert!( + error.to_string().contains("package family"), + "unexpected error: {error:#}" + ); + } + #[test] fn alias_with_unexpected_package_family_is_rejected() { // A crafted alias claiming another (even Microsoft-published) package family