From 7afc57a4ab51a4799068e3d0926f1c4228434d9e Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 26 Apr 2022 16:33:27 -0700 Subject: [PATCH 1/4] Act on elevation requirements in majority cases --- src/AppInstallerCLICore/Resources.h | 2 ++ .../Workflows/InstallFlow.cpp | 8 +++++ .../ShellExecuteInstallerHandler.cpp | 29 +++++++++++++++++-- .../Shared/Strings/en-us/winget.resw | 6 ++++ src/AppInstallerCommonCore/Errors.cpp | 2 ++ .../Public/AppInstallerErrors.h | 1 + 6 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 81f41ea833..a47c8f139e 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -116,6 +116,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(InstallCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(InstalledPackageNotAvailable); WINGET_DEFINE_RESOURCE_STRINGID(InstalledPackageVersionNotAvailable); + WINGET_DEFINE_RESOURCE_STRINGID(InstallerElevationExpected); WINGET_DEFINE_RESOURCE_STRINGID(InstallerBlockedByPolicy); WINGET_DEFINE_RESOURCE_STRINGID(InstallerFailedSecurityCheck); WINGET_DEFINE_RESOURCE_STRINGID(InstallerFailedVirusScan); @@ -126,6 +127,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(InstallerHashMismatchOverrideRequired); WINGET_DEFINE_RESOURCE_STRINGID(InstallerHashVerified); WINGET_DEFINE_RESOURCE_STRINGID(InstallerLogAvailable); + WINGET_DEFINE_RESOURCE_STRINGID(InstallerProhibitsElevation); WINGET_DEFINE_RESOURCE_STRINGID(InstallFlowInstallSuccess); WINGET_DEFINE_RESOURCE_STRINGID(InstallFlowRegistrationDeferred); WINGET_DEFINE_RESOURCE_STRINGID(InstallFlowReturnCodeAlreadyInstalled); diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 6ae888fe4d..bc86ca8128 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -116,6 +116,14 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Error() << Resource::String::NoApplicableInstallers << std::endl; AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_APPLICABLE_INSTALLER); } + + // This installer cannot be run elevated, but we are running elevated. + // Implementation of de-elevation is complex; simply block for now. + if (installer->ElevationRequirement == ElevationRequirementEnum::ElevationProhibited && Runtime::IsRunningAsAdmin()) + { + context.Reporter.Error() << Resource::String::InstallerProhibitsElevation << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALLER_PROHIBITS_ELEVATION); + } } void ShowInstallationDisclaimer(Execution::Context& context) diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index c0849a6825..eda2615957 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -14,7 +14,7 @@ namespace AppInstaller::CLI::Workflow namespace { // ShellExecutes the given path. - std::optional InvokeShellExecute(const std::filesystem::path& filePath, const std::string& args, IProgressCallback& progress) + std::optional InvokeShellExecuteEx(const std::filesystem::path& filePath, const std::string& args, bool useRunAs, IProgressCallback& progress) { AICLI_LOG(CLI, Info, << "Starting: '" << filePath.u8string() << "' with arguments '" << args << '\''); @@ -28,6 +28,13 @@ namespace AppInstaller::CLI::Workflow // Verified setting to SW_SHOW does not hurt silent mode since no UI will be shown. execInfo.nShow = SW_SHOW; + // This installer must be run elevated, but we are not currently. + // Have ShellExecute elevate the installer since it won't do so itself. + if (useRunAs) + { + execInfo.lpVerb = L"runas"; + } + THROW_LAST_ERROR_IF(!ShellExecuteExW(&execInfo) || !execInfo.hProcess); wil::unique_process_handle process{ execInfo.hProcess }; @@ -58,6 +65,11 @@ namespace AppInstaller::CLI::Workflow } } + std::optional InvokeShellExecute(const std::filesystem::path& filePath, const std::string& args, IProgressCallback& progress) + { + return InvokeShellExecuteEx(filePath, args, false, progress); + } + // Gets the escaped installer args. std::string GetInstallerArgsTemplate(Execution::Context& context) { @@ -187,12 +199,25 @@ namespace AppInstaller::CLI::Workflow { context.Reporter.Info() << Resource::String::InstallFlowStartingPackageInstall << std::endl; + const auto& installer = context.Get(); const std::string& installerArgs = context.Get(); + // Enforce elevation requirements + bool isElevated = Runtime::IsRunningAsAdmin(); + + // The installer will run elevated, either by direct request or through the installer itself doing so. + if ((installer->ElevationRequirement == ElevationRequirementEnum::ElevationRequired || + installer->ElevationRequirement == ElevationRequirementEnum::ElevatesSelf) + && !isElevated) + { + context.Reporter.Warn() << Resource::String::InstallerElevationExpected << std::endl; + } + auto installResult = context.Reporter.ExecuteWithProgress( - std::bind(InvokeShellExecute, + std::bind(InvokeShellExecuteEx, context.Get(), installerArgs, + installer->ElevationRequirement == ElevationRequirementEnum::ElevationRequired && !isElevated, std::placeholders::_1)); if (!installResult) diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 3351e02a73..55e5801aac 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1287,4 +1287,10 @@ Please specify one of them using the `--source` option to proceed. Prompts the user to press any key before exiting + + The installer will request to run elevated, expect a prompt. + + + The installer cannot be run from an administrator context. + \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Errors.cpp b/src/AppInstallerCommonCore/Errors.cpp index ca15f06bd1..47e2a9a336 100644 --- a/src/AppInstallerCommonCore/Errors.cpp +++ b/src/AppInstallerCommonCore/Errors.cpp @@ -204,6 +204,8 @@ namespace AppInstaller return "A higher version of this application is already installed."; case APPINSTALLER_CLI_ERROR_INSTALL_BLOCKED_BY_POLICY: return "Organization policies are preventing installation. Contact your admin."; + case APPINSTALLER_CLI_ERROR_INSTALLER_PROHIBITS_ELEVATION: + return "The installer cannot be run from an administrator context."; default: return "Unknown Error Code"; } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h index 1ac01522b4..4be34628a9 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -94,6 +94,7 @@ #define APPINSTALLER_CLI_ERROR_UPGRADE_VERSION_NOT_NEWER ((HRESULT)0x8A15004F) #define APPINSTALLER_CLI_ERROR_UPGRADE_VERSION_UNKNOWN ((HRESULT)0x8A150050) #define APPINSTALLER_CLI_ERROR_ICU_CONVERSION_ERROR ((HRESULT)0x8A150051) +#define APPINSTALLER_CLI_ERROR_INSTALLER_PROHIBITS_ELEVATION ((HRESULT)0x8A150052) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) From 58afc1d2374bd36f6e779c18042051c45f8447f4 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 26 Apr 2022 16:46:39 -0700 Subject: [PATCH 2/4] Spelling --- .github/actions/spelling/allow.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 5630c89465..d15a0f3562 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -442,6 +442,7 @@ Rpc rpc rubengustorage ruleset +runas runsettings runtimes safecast From e1513f3239ff0fffb46fce69ebbe15a5b292dd0e Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 27 Apr 2022 10:23:00 -0700 Subject: [PATCH 3/4] PR feedback and add elevation warning to direct MSI installs that elevate --- src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp | 7 +++++++ .../Workflows/ShellExecuteInstallerHandler.cpp | 2 +- .../Shared/Strings/en-us/winget.resw | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp b/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp index 38c4021cff..03b9b23659 100644 --- a/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp @@ -32,10 +32,17 @@ namespace AppInstaller::CLI::Workflow { context.Reporter.Info() << Resource::String::InstallFlowStartingPackageInstall << std::endl; + const auto& installer = context.Get(); const std::filesystem::path& installerPath = context.Get(); Msi::MsiParsedArguments parsedArgs = Msi::ParseMSIArguments(context.Get()); + // Inform of elevation requirements + if (!Runtime::IsRunningAsAdmin() && installer->ElevationRequirement == Manifest::ElevationRequirementEnum::ElevatesSelf) + { + context.Reporter.Warn() << Resource::String::InstallerElevationExpected << std::endl; + } + auto installResult = context.Reporter.ExecuteWithProgress( std::bind(InvokeMsiInstallProduct, installerPath, diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index eda2615957..a36e36992d 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -202,7 +202,7 @@ namespace AppInstaller::CLI::Workflow const auto& installer = context.Get(); const std::string& installerArgs = context.Get(); - // Enforce elevation requirements + // Inform of elevation requirements bool isElevated = Runtime::IsRunningAsAdmin(); // The installer will run elevated, either by direct request or through the installer itself doing so. diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 55e5801aac..ac2c48b7bd 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1288,7 +1288,7 @@ Please specify one of them using the `--source` option to proceed. Prompts the user to press any key before exiting - The installer will request to run elevated, expect a prompt. + The installer will request to run as administrator, expect a prompt. The installer cannot be run from an administrator context. From 1603f99109c2a8edda0460151cbfd4e689e683f7 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 27 Apr 2022 14:31:42 -0700 Subject: [PATCH 4/4] Align brace --- src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp b/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp index 03b9b23659..00f350320c 100644 --- a/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/MsiInstallFlow.cpp @@ -39,7 +39,7 @@ namespace AppInstaller::CLI::Workflow // Inform of elevation requirements if (!Runtime::IsRunningAsAdmin() && installer->ElevationRequirement == Manifest::ElevationRequirementEnum::ElevatesSelf) - { + { context.Reporter.Warn() << Resource::String::InstallerElevationExpected << std::endl; }