Update Get-Service to ignore common errors when retrieving non-critical properties for a service (#24245)

src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs

@@ -610,146 +610,163 @@ namespace Microsoft.PowerShell.Commands
         /// </summary>
         protected override void ProcessRecord()
         {
-            foreach (ServiceController service in MatchingServices())
+            nint scManagerHandle = nint.Zero;
+            if (!DependentServices && !RequiredServices)
             {
-                if (!DependentServices.IsPresent && !RequiredServices.IsPresent)
+                // As Get-Service only works on local services we get this once
+                // to retrieve extra properties added by PowerShell.
+                scManagerHandle = NativeMethods.OpenSCManagerW(
+                    lpMachineName: null,
+                    lpDatabaseName: null,
+                    dwDesiredAccess: NativeMethods.SC_MANAGER_CONNECT);
+                if (scManagerHandle == nint.Zero)
                 {
-                    WriteObject(AddProperties(service));
+                    Win32Exception exception = new();
+                    string message = StringUtil.Format(ServiceResources.FailToOpenServiceControlManager, exception.Message);
+                    ServiceCommandException serviceException = new ServiceCommandException(message, exception);
+                    ErrorRecord err = new ErrorRecord(
+                        serviceException,
+                        "FailToOpenServiceControlManager",
+                        ErrorCategory.PermissionDenied,
+                        null);
+                    ThrowTerminatingError(err);
                 }
-                else
-                {
-                    if (DependentServices.IsPresent)
-                    {
-                        foreach (ServiceController dependantserv in service.DependentServices)
-                        {
-                            WriteObject(dependantserv);
-                        }
-                    }
+            }
 
-                    if (RequiredServices.IsPresent)
+            try
+            {
+                foreach (ServiceController service in MatchingServices())
+                {
+                    if (!DependentServices.IsPresent && !RequiredServices.IsPresent)
                     {
-                        foreach (ServiceController servicedependedon in service.ServicesDependedOn)
+                        WriteObject(AddProperties(scManagerHandle, service));
+                    }
+                    else
+                    {
+                        if (DependentServices.IsPresent)
                         {
-                            WriteObject(servicedependedon);
+                            foreach (ServiceController dependantserv in service.DependentServices)
+                            {
+                                WriteObject(dependantserv);
+                            }
+                        }
+
+                        if (RequiredServices.IsPresent)
+                        {
+                            foreach (ServiceController servicedependedon in service.ServicesDependedOn)
+                            {
+                                WriteObject(servicedependedon);
+                            }
                         }
                     }
                 }
             }
+            finally
+            {
+                if (scManagerHandle != nint.Zero)
+                {
+                    bool succeeded = NativeMethods.CloseServiceHandle(scManagerHandle);
+                    Diagnostics.Assert(succeeded, "SCManager handle close failed");
+                }
+            }
         }
 
         #endregion Overrides
 
+#nullable enable
         /// <summary>
         /// Adds UserName, Description, BinaryPathName, DelayedAutoStart and StartupType to a ServiceController object.
         /// </summary>
+        /// <param name="scManagerHandle">Handle to the local SCManager instance.</param>
         /// <param name="service"></param>
         /// <returns>ServiceController as PSObject with UserName, Description and StartupType added.</returns>
-        private PSObject AddProperties(ServiceController service)
+        private static PSObject AddProperties(nint scManagerHandle, ServiceController service)
         {
-            NakedWin32Handle hScManager = IntPtr.Zero;
-            NakedWin32Handle hService = IntPtr.Zero;
-            int lastError = 0;
-            PSObject serviceAsPSObj = PSObject.AsPSObject(service);
+            NakedWin32Handle hService = nint.Zero;
+
+            // As these are optional values, a failure due to permissions or
+            // other problem is ignored and the properties are set to null.
+            bool? isDelayedAutoStart = null;
+            string? binPath = null;
+            string? description = null;
+            string? startName = null;
+            ServiceStartupType startupType = ServiceStartupType.InvalidValue;
             try
             {
-                hScManager = NativeMethods.OpenSCManagerW(
-                    lpMachineName: service.MachineName,
-                    lpDatabaseName: null,
-                    dwDesiredAccess: NativeMethods.SC_MANAGER_CONNECT
-                );
-                if (hScManager == IntPtr.Zero)
-                {
-                    lastError = Marshal.GetLastWin32Error();
-                    Win32Exception exception = new(lastError);
-                    WriteNonTerminatingError(
-                        service,
-                        exception,
-                        "FailToOpenServiceControlManager",
-                        ServiceResources.FailToOpenServiceControlManager,
-                        ErrorCategory.PermissionDenied);
-                }
-
+                // We don't use service.ServiceHandle as that requests
+                // SERVICE_ALL_ACCESS when we only need SERVICE_QUERY_CONFIG.
                 hService = NativeMethods.OpenServiceW(
-                    hScManager,
+                    scManagerHandle,
                     service.ServiceName,
                     NativeMethods.SERVICE_QUERY_CONFIG
                 );
-                if (hService == IntPtr.Zero)
+                if (hService != nint.Zero)
                 {
-                    lastError = Marshal.GetLastWin32Error();
-                    Win32Exception exception = new(lastError);
-                    WriteNonTerminatingError(
-                        service,
-                        exception,
-                        "CouldNotGetServiceInfo",
-                        ServiceResources.CouldNotGetServiceInfo,
-                        ErrorCategory.PermissionDenied);
+                    if (NativeMethods.QueryServiceConfig2(
+                        hService,
+                        NativeMethods.SERVICE_CONFIG_DESCRIPTION,
+                        out NativeMethods.SERVICE_DESCRIPTIONW descriptionInfo))
+                    {
+                        description = descriptionInfo.lpDescription;
+                    }
+
+                    if (NativeMethods.QueryServiceConfig2(
+                        hService,
+                        NativeMethods.SERVICE_CONFIG_DELAYED_AUTO_START_INFO,
+                        out NativeMethods.SERVICE_DELAYED_AUTO_START_INFO autostartInfo))
+                    {
+                        isDelayedAutoStart = autostartInfo.fDelayedAutostart;
+                    }
+
+                    if (NativeMethods.QueryServiceConfig(
+                        hService,
+                        out NativeMethods.QUERY_SERVICE_CONFIG serviceInfo))
+                    {
+                        binPath = serviceInfo.lpBinaryPathName;
+                        startName = serviceInfo.lpServiceStartName;
+                        if (isDelayedAutoStart.HasValue)
+                        {
+                            startupType = NativeMethods.GetServiceStartupType(
+                                (ServiceStartMode)serviceInfo.dwStartType,
+                                isDelayedAutoStart.Value);
+                        }
+                    }
                 }
-
-                NativeMethods.SERVICE_DESCRIPTIONW description = new();
-                bool querySuccessful = NativeMethods.QueryServiceConfig2<NativeMethods.SERVICE_DESCRIPTIONW>(hService, NativeMethods.SERVICE_CONFIG_DESCRIPTION, out description);
-
-                NativeMethods.SERVICE_DELAYED_AUTO_START_INFO autostartInfo = new();
-                querySuccessful = querySuccessful && NativeMethods.QueryServiceConfig2<NativeMethods.SERVICE_DELAYED_AUTO_START_INFO>(hService, NativeMethods.SERVICE_CONFIG_DELAYED_AUTO_START_INFO, out autostartInfo);
-
-                NativeMethods.QUERY_SERVICE_CONFIG serviceInfo = new();
-                querySuccessful = querySuccessful && NativeMethods.QueryServiceConfig(hService, out serviceInfo);
-
-                if (!querySuccessful)
-                {
-                    WriteNonTerminatingError(
-                        service: service,
-                        innerException: null,
-                        errorId: "CouldNotGetServiceInfo",
-                        errorMessage: ServiceResources.CouldNotGetServiceInfo,
-                        category: ErrorCategory.PermissionDenied
-                        );
-                }
-
-                PSProperty noteProperty = new("UserName", serviceInfo.lpServiceStartName);
-                serviceAsPSObj.Properties.Add(noteProperty, true);
-                serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#UserName");
-
-                noteProperty = new PSProperty("Description", description.lpDescription);
-                serviceAsPSObj.Properties.Add(noteProperty, true);
-                serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#Description");
-
-                noteProperty = new PSProperty("DelayedAutoStart", autostartInfo.fDelayedAutostart);
-                serviceAsPSObj.Properties.Add(noteProperty, true);
-                serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#DelayedAutoStart");
-
-                noteProperty = new PSProperty("BinaryPathName", serviceInfo.lpBinaryPathName);
-                serviceAsPSObj.Properties.Add(noteProperty, true);
-                serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#BinaryPathName");
-
-                noteProperty = new PSProperty("StartupType", NativeMethods.GetServiceStartupType(service.StartType, autostartInfo.fDelayedAutostart));
-                serviceAsPSObj.Properties.Add(noteProperty, true);
-                serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#StartupType");
             }
             finally
             {
                 if (hService != IntPtr.Zero)
                 {
                     bool succeeded = NativeMethods.CloseServiceHandle(hService);
-                    if (!succeeded)
-                    {
-                        Diagnostics.Assert(lastError != 0, "ErrorCode not success");
-                    }
-                }
-
-                if (hScManager != IntPtr.Zero)
-                {
-                    bool succeeded = NativeMethods.CloseServiceHandle(hScManager);
-                    if (!succeeded)
-                    {
-                        Diagnostics.Assert(lastError != 0, "ErrorCode not success");
-                    }
+                    Diagnostics.Assert(succeeded, "Failed to close service handle");
                 }
             }
 
+            PSObject serviceAsPSObj = PSObject.AsPSObject(service);
+            PSNoteProperty noteProperty = new("UserName", startName);
+            serviceAsPSObj.Properties.Add(noteProperty, true);
+            serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#UserName");
+
+            noteProperty = new PSNoteProperty("Description", description);
+            serviceAsPSObj.Properties.Add(noteProperty, true);
+            serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#Description");
+
+            noteProperty = new PSNoteProperty("DelayedAutoStart", isDelayedAutoStart);
+            serviceAsPSObj.Properties.Add(noteProperty, true);
+            serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#DelayedAutoStart");
+
+            noteProperty = new PSNoteProperty("BinaryPathName", binPath);
+            serviceAsPSObj.Properties.Add(noteProperty, true);
+            serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#BinaryPathName");
+
+            noteProperty = new PSNoteProperty("StartupType", startupType);
+            serviceAsPSObj.Properties.Add(noteProperty, true);
+            serviceAsPSObj.TypeNames.Insert(0, "System.Service.ServiceController#StartupType");
+
             return serviceAsPSObj;
         }
     }
+#nullable disable
     #endregion GetServiceCommand
 
     #region ServiceOperationBaseCommand

src/Microsoft.PowerShell.Commands.Management/resources/ServiceResources.resx

@@ -159,9 +159,6 @@
   <data name="CouldNotSetService" xml:space="preserve">
     <value>Service '{1} ({0})' cannot be configured due to the following error: {2}</value>
   </data>
-  <data name="CouldNotGetServiceInfo" xml:space="preserve">
-    <value>Service '{1} ({0})' cannot be queried due to the following error: {2}</value>
-  </data>
   <data name="CouldNotSetServiceDescription" xml:space="preserve">
     <value>Service '{1} ({0})' description cannot be configured due to the following error: {2}</value>
   </data>
@@ -211,7 +208,7 @@
     <value>Service '{1} ({0})' resume failed.</value>
   </data>
   <data name="FailToOpenServiceControlManager" xml:space="preserve">
-    <value>Failed to configure the service '{1} ({0})' due to the following error: {2}. Run PowerShell as admin and run your command again.</value>
+    <value>Failed to open SCManager due to the following error: {0}. Run PowerShell as admin and run your command again.</value>
   </data>
   <data name="UnsupportedStartupType" xml:space="preserve">
     <value>The startup type '{0}' is not supported by {1}.</value>

test/powershell/Modules/Microsoft.PowerShell.Management/Get-Service.Tests.ps1

@@ -82,3 +82,60 @@ Describe "Get-Service cmdlet tests" -Tags "CI" {
     { & $script } | Should -Throw -ErrorId $errorid
   }
 }
+
+Describe 'Get-Service Admin tests' -Tag CI,RequireAdminOnWindows {
+  BeforeAll {
+    $originalDefaultParameterValues = $PSDefaultParameterValues.Clone()
+    if ( -not $IsWindows ) {
+        $PSDefaultParameterValues["it:skip"] = $true
+    }
+  }
+  AfterAll {
+      $global:PSDefaultParameterValues = $originalDefaultParameterValues
+  }
+
+  BeforeEach {
+    $serviceParams = @{
+      Name = "PowerShellTest-$([Guid]::NewGuid().Guid)"
+      BinaryPathName = "$env:SystemRoot\System32\cmd.exe"
+      StartupType = 'Manual'
+    }
+    $service = New-Service @serviceParams
+  }
+  AfterEach {
+    $service | Remove-Service
+  }
+
+  It "Ignores invalid description MUI entry" {
+    $serviceRegPath = "HKLM:\SYSTEM\CurrentControlSet\Services\$($service.Name)"
+    Set-ItemProperty -LiteralPath $serviceRegPath -Name Description -Value '@Fake.dll,-0'
+    $actual = Get-Service -Name $service.Name -ErrorAction Stop
+
+    $actual.Name | Should -Be $service.Name
+    $actual.Status | Should -Be Stopped
+    $actual.Description | Should -BeNullOrEmpty
+    $actual.UserName | Should -Be LocalSystem
+    $actual.StartupType | Should -Be Manual
+  }
+
+  It "Ignores no SERVICE_QUERY_CONFIG access" {
+    $sddl = ((sc.exe sdshow $service.Name) -join "").Trim()
+    $sd = ConvertFrom-SddlString -Sddl $sddl
+    $sd.RawDescriptor.DiscretionaryAcl.AddAccess(
+      [System.Security.AccessControl.AccessControlType]::Deny,
+      [System.Security.Principal.WindowsIdentity]::GetCurrent().User,
+      0x1,  # SERVICE_QUERY_CONFIG
+      [System.Security.AccessControl.InheritanceFlags]::None,
+      [System.Security.AccessControl.PropagationFlags]::None)
+    $newSddl = $sd.RawDescriptor.GetSddlForm([System.Security.AccessControl.AccessControlSections]::All)
+    $null = sc.exe sdset $service.Name $newSddl
+
+    $actual = Get-Service -Name $service.Name -ErrorAction Stop
+
+    $actual.Name | Should -Be $service.Name
+    $actual.Status | Should -Be Stopped
+    $actual.Description | Should -BeNullOrEmpty
+    $actual.UserName | Should -BeNullOrEmpty
+    $actual.StartupType | Should -Be InvalidValue
+  }
+}