commit e99152389bed782b8f86188ec48b39382dc3c4a5 parent a10f1ab366296ceec95491161c7d5b02bb6c7206 Author: Ruben Guerrero <rubengu@microsoft.com> Date: Mon, 23 Oct 2023 18:28:29 -0700 Inform user if a module requires elevation (#3758) Fix #3747 Add a new HRESULT for failed importing module because the module requires to run as admin. Also fix places where WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT was missing. We can't have automated tests because we don't currently have a way to run winget as non admin in our tests. I created a UT with a simple test that is skipped in ADO. I also removed printing details for WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE because I noticed that "error code could not be found" message is coming the IConfigurationUnitResultInformation description. Apply :: VSComponents Loading the module for the configuration unit failed. The text associated with this error code could not be found. Some of the configuration was not applied successfully. In the configuration module, the IConfigurationUnitResultInformation description is the actual error message that we set in ImportModuleException. My guess is that cppwinrt use some method to get message of well-known errors and set that, but I didn't want to go that deep, so removing it because we are literally printing a line before the text associated with that error code. Diffstat:
18 files changed, 179 insertions(+), 10 deletions(-)
diff --git a/doc/windows/package-manager/winget/returnCodes.md b/doc/windows/package-manager/winget/returnCodes.md @@ -192,4 +192,6 @@ Installation failed. Restart your PC then try again. | | 0x8A15C106 | -1978285818 | WINGET_CONFIG_ERROR_UNIT_INVOKE_SET | The configuration unit failed while attempting to apply the desired state. | | 0x8A15C107 | -1978285817 | WINGET_CONFIG_ERROR_UNIT_MODULE_CONFLICT | The module for the configuration unit is available in multiple locations with the same version. | | 0x8A15C108 | -1978285816 | WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE | Loading the module for the configuration unit failed. | -| 0x8A15C109 | -1978285815 | WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT | The configuration unit returned an unexpected result during execution. |- \ No newline at end of file +| 0x8A15C109 | -1978285815 | WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT | The configuration unit returned an unexpected result during execution. | +| 0x8A15C110 | -1978285814 | WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT | A unit contains a setting that requires the config root. | +| 0x8A15C111 | -1978285813 | WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN | Loading the module for the configuration unit failed because it requires administrator privileges to run. |+ \ No newline at end of file diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h @@ -104,6 +104,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitFailedUnitProcessing); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitHasDuplicateIdentifier); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitHasMissingDependency); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitImportModuleAdmin); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitIsPartOfDependencyCycle); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitManuallySkipped); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitModuleConflict); @@ -118,6 +119,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitNotRunDueToDependency); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitNotRunDueToFailedAssert); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitReturnedInvalidResult); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitSettingConfigRoot); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationUnitSkipped); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationValidationFoundNoIssues); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWaitingOnAnother); diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -407,8 +407,10 @@ namespace AppInstaller::CLI::Workflow case WINGET_CONFIG_ERROR_UNIT_INVOKE_TEST: return { Resource::String::ConfigurationUnitFailedDuringTest(), true }; case WINGET_CONFIG_ERROR_UNIT_INVOKE_SET: return { Resource::String::ConfigurationUnitFailedDuringSet(), true }; case WINGET_CONFIG_ERROR_UNIT_MODULE_CONFLICT: return { Resource::String::ConfigurationUnitModuleConflict(), false }; - case WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE: return { Resource::String::ConfigurationUnitModuleImportFailed(), true }; + case WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE: return { Resource::String::ConfigurationUnitModuleImportFailed(), false }; case WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT: return { Resource::String::ConfigurationUnitReturnedInvalidResult(), false }; + case WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT: return { Resource::String::ConfigurationUnitSettingConfigRoot(), false }; + case WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN: return { Resource::String::ConfigurationUnitImportModuleAdmin(), false }; } switch (resultInformation.ResultSource()) diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs @@ -317,6 +317,7 @@ namespace AppInstallerCLIE2ETests public const int CONFIG_ERROR_UNIT_IMPORT_MODULE = unchecked((int)0x8A15C108); public const int CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT = unchecked((int)0x8A15C109); public const int CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT = unchecked((int)0x8A15C110); + public const int CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN = unchecked((int)0x8A15C111); } #pragma warning restore SA1310 // Field names should not contain underscore diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -2587,6 +2587,15 @@ Please specify one of them using the --source option to proceed.</value> <data name="EnableWindowsFeaturesSuccess" xml:space="preserve"> <value>Successfully enabled Windows Features dependencies</value> </data> + <data name="WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN" xml:space="preserve"> + <value>Loading the module for the configuration unit failed because it requires administrator privileges to run.</value> + </data> + <data name="ConfigurationUnitSettingConfigRoot" xml:space="preserve"> + <value>A unit contains a setting that requires the config root.</value> + </data> + <data name="ConfigurationUnitImportModuleAdmin" xml:space="preserve"> + <value>Loading the module for the configuration unit failed because it requires administrator privileges to run.</value> + </data> <data name="ResumeCommandLongDescription" xml:space="preserve"> <value>Resumes execution of a previously saved command by passing in the unique identifier of the saved command. This is used to resume an executed command that may have been terminated due to a reboot.</value> </data> diff --git a/src/AppInstallerSharedLib/Errors.cpp b/src/AppInstallerSharedLib/Errors.cpp @@ -261,7 +261,8 @@ namespace AppInstaller WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE, "Loading the module for the configuration unit failed."), WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT, "The configuration unit returned an unexpected result during execution."), WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT, "A unit contains a setting that requires the config root."), - + WINGET_HRESULT_INFO(WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN, "Loading the module for the configuration unit failed because it requires administrator privileges to run."), + // Errors without the error bit set WINGET_HRESULT_INFO(WINGET_INSTALLED_STATUS_INSTALL_LOCATION_NOT_APPLICABLE, "The install location is not applicable."), WINGET_HRESULT_INFO(WINGET_INSTALLED_STATUS_FILE_FOUND_WITHOUT_HASH_CHECK, "The file was found but the hash was not checked."), diff --git a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h @@ -194,6 +194,7 @@ #define WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE ((HRESULT)0x8A15C108) #define WINGET_CONFIG_ERROR_UNIT_INVOKE_INVALID_RESULT ((HRESULT)0x8A15C109) #define WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT ((HRESULT)0x8A15C110) +#define WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN ((HRESULT)0x8A15C111) namespace AppInstaller { diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/ErrorCodes.cs @@ -65,5 +65,10 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// The unit contains a setting that requires config root. /// </summary> internal const int WinGetConfigUnitSettingConfigRoot = unchecked((int)0x8A15C110); + + /// <summary> + /// The module where the DSC resource is implemented requires admin. + /// </summary> + internal const int WinGetConfigUnitImportModuleAdmin = unchecked((int)0x8A15C111); } } diff --git a/src/Microsoft.Management.Configuration.Processor/Exceptions/ImportModuleException.cs b/src/Microsoft.Management.Configuration.Processor/Exceptions/ImportModuleException.cs @@ -7,6 +7,7 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions { using System; + using System.Management.Automation; /// <summary> /// Import-Module threw an exception. @@ -17,11 +18,11 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// Initializes a new instance of the <see cref="ImportModuleException"/> class. /// </summary> /// <param name="moduleName">Module name.</param> - /// <param name="inner">Inner exception.</param> - public ImportModuleException(string? moduleName, Exception inner) - : base($"Could not import module: {moduleName?.ToString() ?? "<no module>"}", inner) + /// <param name="pwshEx">Inner exception.</param> + public ImportModuleException(string? moduleName, Exception pwshEx) + : base($"Could not import module: {moduleName?.ToString() ?? "<no module>"}", pwshEx) { - this.HResult = ErrorCodes.WinGetConfigUnitImportModule; + this.HResult = this.GetHResult(pwshEx); this.ModuleName = moduleName; } @@ -29,5 +30,22 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions /// Gets the module name. /// </summary> public string? ModuleName { get; } + + private int GetHResult(Exception pwshEx) + { + if (pwshEx.InnerException is not null) + { + var scriptEx = pwshEx.InnerException as ScriptRequiresException; + if (scriptEx is not null) + { + if (scriptEx.ErrorRecord.CategoryInfo.Category == ErrorCategory.PermissionDenied) + { + return ErrorCodes.WinGetConfigUnitImportModuleAdmin; + } + } + } + + return ErrorCodes.WinGetConfigUnitImportModule; + } } } diff --git a/src/Microsoft.Management.Configuration.UnitTests/Microsoft.Management.Configuration.UnitTests.csproj b/src/Microsoft.Management.Configuration.UnitTests/Microsoft.Management.Configuration.UnitTests.csproj @@ -35,7 +35,7 @@ <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> </PackageReference> <PackageReference Include="Moq" Version="4.18.2" /> - <PackageReference Include="Microsoft.PowerShell.SDK" Version="7.2.8" GeneratePathProperty="true"/> + <PackageReference Include="Microsoft.PowerShell.SDK" Version="7.2.8" GeneratePathProperty="true" /> </ItemGroup> <ItemGroup> @@ -53,7 +53,7 @@ </ProjectReference> </ItemGroup> - <Target Name="PwshFiles" AfterTargets="AfterBuild" > + <Target Name="PwshFiles" AfterTargets="AfterBuild"> <ItemGroup> <RefFiles Include="$(OutputPath)..\..\..\..\..\AnyCPU\$(Configuration)\Microsoft.Management.Configuration.Processor\$(TargetFramework)\win\ref\**\*.*" /> <WinModuleFiles Include="$(OutputPath)..\..\..\..\..\AnyCPU\$(Configuration)\Microsoft.Management.Configuration.Processor\$(TargetFramework)\win\Modules\**\*.*" /> @@ -63,6 +63,12 @@ </Target> <ItemGroup> + <None Update="TestCollateral\PowerShellModules\xAdminTestResource\xAdminTestResource.psd1"> + <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> + </None> + <None Update="TestCollateral\PowerShellModules\xAdminTestResource\xAdminTestResource.psm1"> + <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> + </None> <None Update="TestCollateral\PowerShellModules\xSimpleTestResource\xSimpleTestResource.psd1"> <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> </None> diff --git a/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xAdminTestResource/xAdminTestResource.psd1 b/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xAdminTestResource/xAdminTestResource.psd1 @@ -0,0 +1,37 @@ +# +# Module manifest for module 'xAdminTestResource' +# +# Generated by: Luffytaro +# +# Generated on: 10/11/2023 +# + +@{ + +RootModule = 'xAdminTestResource.psm1' +ModuleVersion = '0.0.0.1' +GUID = 'a0be43e8-ac22-4244-8efc-7263dfa58b8c' +CompatiblePSEditions = 'Core' +Author = 'Luffytaro' +CompanyName = 'Microsoft Corporation' +Copyright = '(c) Microsoft Corporation. All rights reserved.' +Description = 'PowerShell module with DSC resources for unit tests that requires admin' +PowerShellVersion = '7.2' +FunctionsToExport = @() +CmdletsToExport = @() +DscResourcesToExport = @( + 'AdminResource' +) +HelpInfoURI = 'https://www.contoso.com/help' + +# Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell. +PrivateData = @{ + + PSData = @{ + ProjectUri = 'https://github.com/microsoft/winget-cli' + IconUri = 'https://www.contoso.com/icons/icon.png' + } + +} + +} diff --git a/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xAdminTestResource/xAdminTestResource.psm1 b/src/Microsoft.Management.Configuration.UnitTests/TestCollateral/PowerShellModules/xAdminTestResource/xAdminTestResource.psm1 @@ -0,0 +1,29 @@ +# Simple module that requires admin. +#Requires -RunAsAdministrator +enum Ensure +{ + Absent + Present +} + +[DscResource()] +class AdminResource +{ + [DscProperty(Key)] + [string] $key + + [AdminResource] Get() + { + return $this + } + + [bool] Test() + { + return $false + } + + [void] Set() + { + } +} + diff --git a/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs b/src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationSetProcessorTests.cs @@ -791,6 +791,30 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests unitProcessor.TestSettings(); } + /// <summary> + /// Tests a module that requires admin is loaded from non admin. + /// </summary> + [FactSkipIfCI] + public void CreateUnitProcessor_ModuleRequiresAdmin() + { + var processorEnv = this.fixture.PrepareTestProcessorEnvironment(); + + var setProcessor = new ConfigurationSetProcessor(processorEnv, new ConfigurationSet()); + + var unit = new ConfigurationUnit + { + Type = "AdminResource", + Intent = ConfigurationUnitIntent.Assert, + }; + unit.Metadata.Add("module", "xAdminTestResource"); + unit.Metadata.Add("version", "0.0.0.1"); + + unit.Settings.Add("key", "key"); + + var importModuleException = Assert.Throws<ImportModuleException>(() => setProcessor.CreateUnitProcessor(unit)); + Assert.Equal(ErrorCodes.WinGetConfigUnitImportModuleAdmin, importModuleException.HResult); + } + private ConfigurationUnit CreateConfigurationUnit() { var unit = new ConfigurationUnit(); diff --git a/src/Microsoft.Management.Configuration/ConfigurationUnitResultInformation.cpp b/src/Microsoft.Management.Configuration/ConfigurationUnitResultInformation.cpp @@ -16,6 +16,8 @@ namespace winrt::Microsoft::Management::Configuration::implementation case WINGET_CONFIG_ERROR_UNIT_NOT_FOUND_REPOSITORY: case WINGET_CONFIG_ERROR_UNIT_MULTIPLE_MATCHES: case WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE: + case WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT: + case WINGET_CONFIG_ERROR_UNIT_IMPORT_MODULE_ADMIN: return ConfigurationUnitResultSource::ConfigurationSet; case WINGET_CONFIG_ERROR_UNIT_MODULE_CONFLICT: return ConfigurationUnitResultSource::SystemState; diff --git a/src/PowerShell/Microsoft.WinGet.Configuration.Engine/Exceptions/ErrorCodes.cs b/src/PowerShell/Microsoft.WinGet.Configuration.Engine/Exceptions/ErrorCodes.cs @@ -39,6 +39,8 @@ namespace Microsoft.WinGet.Configuration.Engine.Exceptions internal const int WinGetConfigUnitModuleConflict = unchecked((int)0x8A15C107); internal const int WinGetConfigUnitImportModule = unchecked((int)0x8A15C108); internal const int WinGetConfigUnitInvokeInvalidResult = unchecked((int)0x8A15C109); + internal const int WinGetConfigUnitSettingConfigRoot = unchecked((int)0x8A15C110); + internal const int WinGetConfigUnitImportModuleAdmin = unchecked((int)0x8A15C111); #pragma warning restore SA1310 // Field names should not contain underscore #pragma warning restore SA1600 // ElementsMustBeDocumented } diff --git a/src/PowerShell/Microsoft.WinGet.Configuration.Engine/PSObjects/PSUnitResult.cs b/src/PowerShell/Microsoft.WinGet.Configuration.Engine/PSObjects/PSUnitResult.cs @@ -101,6 +101,10 @@ namespace Microsoft.WinGet.Configuration.Engine.PSObjects return Resources.ConfigurationUnitManuallySkipped; case ErrorCodes.WingetConfigErrorDependencyUnsatisfied: return Resources.ConfigurationUnitNotRunDueToDependency; + case ErrorCodes.WinGetConfigUnitSettingConfigRoot: + return Resources.WinGetConfigUnitSettingConfigRoot; + case ErrorCodes.WinGetConfigUnitImportModuleAdmin: + return Resources.WinGetConfigUnitImportModuleAdmin; } switch (resultInfo.ResultSource) diff --git a/src/PowerShell/Microsoft.WinGet.Configuration.Engine/Resources/Resources.Designer.cs b/src/PowerShell/Microsoft.WinGet.Configuration.Engine/Resources/Resources.Designer.cs @@ -554,5 +554,23 @@ namespace Microsoft.WinGet.Configuration.Engine.Resources { return ResourceManager.GetString("SeeLineAndColumn", resourceCulture); } } + + /// <summary> + /// Looks up a localized string similar to Loading the module for the configuration unit failed because it requires administrator privileges to run.. + /// </summary> + internal static string WinGetConfigUnitImportModuleAdmin { + get { + return ResourceManager.GetString("WinGetConfigUnitImportModuleAdmin", resourceCulture); + } + } + + /// <summary> + /// Looks up a localized string similar to A unit contains a setting that requires the config root.. + /// </summary> + internal static string WinGetConfigUnitSettingConfigRoot { + get { + return ResourceManager.GetString("WinGetConfigUnitSettingConfigRoot", resourceCulture); + } + } } } diff --git a/src/PowerShell/Microsoft.WinGet.Configuration.Engine/Resources/Resources.resx b/src/PowerShell/Microsoft.WinGet.Configuration.Engine/Resources/Resources.resx @@ -309,4 +309,10 @@ <data name="ConfigurationWarningPromptTest" xml:space="preserve"> <value>Have you reviewed the configuration and would you like to proceed verifying it against the system?</value> </data> + <data name="WinGetConfigUnitImportModuleAdmin" xml:space="preserve"> + <value>Loading the module for the configuration unit failed because it requires administrator privileges to run.</value> + </data> + <data name="WinGetConfigUnitSettingConfigRoot" xml:space="preserve"> + <value>A unit contains a setting that requires the config root.</value> + </data> </root> \ No newline at end of file