Skip to content

Commit

Permalink
Inform user if a module requires elevation (#3758)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
msftrubengu authored Oct 24, 2023
1 parent a10f1ab commit e991523
Show file tree
Hide file tree
Showing 18 changed files with 178 additions and 9 deletions.
4 changes: 3 additions & 1 deletion doc/windows/package-manager/winget/returnCodes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
| 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. |
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLIE2ETests/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ public class ErrorCode
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
Expand Down
9 changes: 9 additions & 0 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
3 changes: 2 additions & 1 deletion src/AppInstallerSharedLib/Errors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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."),
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerSharedLib/Public/AppInstallerErrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,10 @@ internal static class ErrorCodes
/// 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace Microsoft.Management.Configuration.Processor.Exceptions
{
using System;
using System.Management.Automation;

/// <summary>
/// Import-Module threw an exception.
Expand All @@ -17,17 +18,34 @@ internal class ImportModuleException : Exception
/// 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;
}

/// <summary>
/// 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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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\**\*.*" />
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
}

}

}
Original file line number Diff line number Diff line change
@@ -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()
{
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,30 @@ public void CreateUnitProcessor_TestTypes()
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ internal static class ErrorCodes
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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ private string GetUnitMessage(ConfigurationUnit unit, IConfigurationUnitResultIn
return Resources.ConfigurationUnitManuallySkipped;
case ErrorCodes.WingetConfigErrorDependencyUnsatisfied:
return Resources.ConfigurationUnitNotRunDueToDependency;
case ErrorCodes.WinGetConfigUnitSettingConfigRoot:
return Resources.WinGetConfigUnitSettingConfigRoot;
case ErrorCodes.WinGetConfigUnitImportModuleAdmin:
return Resources.WinGetConfigUnitImportModuleAdmin;
}

switch (resultInfo.ResultSource)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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>

0 comments on commit e991523

Please sign in to comment.