diff --git a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md index 1b70f9451f..cbf29be452 100644 --- a/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md +++ b/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md @@ -80,6 +80,7 @@ applyTo: "**/*.ps?(m|d)1" - Place ShouldProcess check immediately before each state-change - `$PSCmdlet.ShouldProcess` must use required pattern - Never use backtick as line continuation in production code. +- Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'`; restore after ## Output streams @@ -181,6 +182,7 @@ function Get-Something - Limit piping to one pipe per line - Assign function results to variables rather than inline calls - Return a single, consistent object type per function + - return `$null` for no objects/non-terminating errors ### Security & Safety diff --git a/CHANGELOG.md b/CHANGELOG.md index fe76c11419..a840d8f5c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `SqlRSSetup` - Re-added `ReportServerEdition` enum and updated class to use enum instead of ValidateSet for the Edition property. +- Fixed commands continuing execution after `Assert-ElevatedUser` elevation + errors by setting `$ErrorActionPreference = 'Stop'` [issue #2070](https://github.com/dsccommunity/SqlServerDsc/issues/2070) ### Added diff --git a/source/Private/Invoke-ReportServerSetupAction.ps1 b/source/Private/Invoke-ReportServerSetupAction.ps1 index 1bd3e26fc1..1e56053a88 100644 --- a/source/Private/Invoke-ReportServerSetupAction.ps1 +++ b/source/Private/Invoke-ReportServerSetupAction.ps1 @@ -196,6 +196,10 @@ function Invoke-ReportServerSetupAction $PassThru ) + $previousErrorActionPreference = $ErrorActionPreference + + $ErrorActionPreference = 'Stop' + if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' @@ -226,6 +230,8 @@ function Invoke-ReportServerSetupAction Assert-BoundParameter @assertBoundParameters + $ErrorActionPreference = $previousErrorActionPreference + # Sensitive values. $sensitiveValue = @() diff --git a/source/Private/Invoke-SetupAction.ps1 b/source/Private/Invoke-SetupAction.ps1 index e0a2cdb30e..949eabd88b 100644 --- a/source/Private/Invoke-SetupAction.ps1 +++ b/source/Private/Invoke-SetupAction.ps1 @@ -1372,6 +1372,10 @@ function Invoke-SetupAction $Force ) + $previousErrorActionPreference = $ErrorActionPreference + + $ErrorActionPreference = 'Stop' + if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' @@ -1411,6 +1415,8 @@ function Invoke-SetupAction Assert-SetupActionProperties -Property $PSBoundParameters -SetupAction $setupAction -ErrorAction 'Stop' + $ErrorActionPreference = $previousErrorActionPreference + $setupArgument = '/QUIET /ACTION={0}' -f $setupAction if ($DebugPreference -in @('Continue', 'Inquire')) diff --git a/source/Public/Get-SqlDscStartupParameter.ps1 b/source/Public/Get-SqlDscStartupParameter.ps1 index e03716f9ae..1f99897c73 100644 --- a/source/Public/Get-SqlDscStartupParameter.ps1 +++ b/source/Public/Get-SqlDscStartupParameter.ps1 @@ -65,6 +65,10 @@ function Get-SqlDscStartupParameter $InstanceName = 'MSSQLSERVER' ) + $previousErrorActionPreference = $ErrorActionPreference + + $ErrorActionPreference = 'Stop' + Assert-ElevatedUser -ErrorAction 'Stop' if ($PSCmdlet.ParameterSetName -eq 'ByServiceObject') @@ -72,6 +76,8 @@ function Get-SqlDscStartupParameter $ServiceObject | Assert-ManagedServiceType -ServiceType 'DatabaseEngine' } + $ErrorActionPreference = $previousErrorActionPreference + if ($PSCmdlet.ParameterSetName -eq 'ByServerName') { $getSqlDscManagedComputerServiceParameters = @{ diff --git a/source/Public/Set-SqlDscStartupParameter.ps1 b/source/Public/Set-SqlDscStartupParameter.ps1 index f31e8ddaa3..e36e84ae81 100644 --- a/source/Public/Set-SqlDscStartupParameter.ps1 +++ b/source/Public/Set-SqlDscStartupParameter.ps1 @@ -91,12 +91,18 @@ function Set-SqlDscStartupParameter begin { + $previousErrorActionPreference = $ErrorActionPreference + + $ErrorActionPreference = 'Stop' + Assert-ElevatedUser -ErrorAction 'Stop' if ($Force.IsPresent -and -not $Confirm) { $ConfirmPreference = 'None' } + + $ErrorActionPreference = $previousErrorActionPreference } process diff --git a/tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1 b/tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1 index 8f1b7820cc..cfb8312fd2 100644 --- a/tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1 +++ b/tests/Unit/Private/Invoke-ReportServerSetupAction.Tests.ps1 @@ -99,6 +99,46 @@ Describe 'Invoke-ReportServerSetupAction' -Tag 'Private' { } } + Context 'When user is not elevated' { + BeforeAll { + # Mock Assert-ElevatedUser to throw the same error it would in a real scenario + Mock -CommandName Assert-ElevatedUser -MockWith { + $PSCmdlet.ThrowTerminatingError( + [System.Management.Automation.ErrorRecord]::new( + 'This command must run in an elevated PowerShell session. (DRC0043)', + 'UserNotElevated', + [System.Management.Automation.ErrorCategory]::InvalidOperation, + 'Command parameters' + ) + ) + } + + # Create a valid executable file for the test + New-Item -Path "$TestDrive/ssrs.exe" -ItemType File -Force | Out-Null + + InModuleScope -ScriptBlock { + $script:mockDefaultParameters = @{ + Install = $true + AcceptLicensingTerms = $true + MediaPath = "$TestDrive/ssrs.exe" + Force = $true + } + } + } + + It 'Should throw a terminating error and not continue execution' { + InModuleScope -ScriptBlock { + # This test verifies the fix for issue #2070 where Assert-ElevatedUser + # would throw an error but the function would continue executing + { Invoke-ReportServerSetupAction @mockDefaultParameters } | + Should -Throw -ExpectedMessage '*This command must run in an elevated PowerShell session*' + } + + # Ensure Assert-ElevatedUser was called + Should -Invoke -CommandName Assert-ElevatedUser -Exactly -Times 1 -Scope It + } + } + Context 'When passing no existent path to parameter MediaPath' { BeforeAll { Mock -CommandName Assert-ElevatedUser diff --git a/tests/Unit/Public/Get-SqlDscStartupParameter.Tests.ps1 b/tests/Unit/Public/Get-SqlDscStartupParameter.Tests.ps1 index 1c4db5706d..94677cb532 100644 --- a/tests/Unit/Public/Get-SqlDscStartupParameter.Tests.ps1 +++ b/tests/Unit/Public/Get-SqlDscStartupParameter.Tests.ps1 @@ -125,7 +125,6 @@ Describe 'Get-SqlDscStartupParameter' -Tag 'Public' { Context 'When passing server name but an Managed Computer Service object is not returned' { BeforeAll { Mock -CommandName Assert-ElevatedUser - Mock -CommandName Get-SqlDscManagedComputerService -MockWith { return $null }