Skip to content

SqlRole: Fix parameter checks #2459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dan-hughes wants to merge 2 commits into
base: main
Choose a base branch
from
Open

SqlRole: Fix parameter checks #2459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fcbc28
Fix if statement checks
dan-hughes Apr 27, 2026
43ef0c7
Update changelog
dan-hughes Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- SqlDatabase
- Fixed logic in `Set-SqlDscDatabaseProperty` so pipeline input is properly
handled in the cmdlet's `Process` block. ([issue #2456](https://github.com/dsccommunity/SqlServerDsc/issues/2456))
- SqlRole
- Fix if statements checking the parameters are present. ([issue #1702](https://github.com/dsccommunity/SqlServerDsc/issues/1702))

### Changed

Expand Down
21 changes: 10 additions & 11 deletions source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ function Get-TargetResource
if ($sqlServerObject)
{
Write-Verbose -Message (
$script:localizedData.GetProperties `
-f $ServerRoleName
$script:localizedData.GetProperties -f $ServerRoleName
)

if ($sqlServerRoleObject = $sqlServerObject.Roles[$ServerRoleName])
Expand All @@ -84,8 +83,8 @@ function Get-TargetResource
ServerName = $ServerName
InstanceName = $InstanceName
Members = $membersInRole
MembersToInclude = $null
MembersToExclude = $null
MembersToInclude = @()
MembersToExclude = @()
}
}

Expand Down Expand Up @@ -237,7 +236,7 @@ function Set-TargetResource

$correctedParameters = Get-CorrectedMemberParameters @originalParameters

if ($Members)
if ($PSBoundParameters.ContainsKey('Members'))
{
$memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames()

Expand All @@ -263,7 +262,7 @@ function Set-TargetResource
}
else
{
if ($MembersToInclude)
if ($PSBoundParameters.ContainsKey('MembersToInclude'))
{
$memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames()

Expand All @@ -278,7 +277,7 @@ function Set-TargetResource
}
}

if ($MembersToExclude)
if ($PSBoundParameters.ContainsKey('MembersToExclude'))
{
$memberNamesInRoleObject = $sqlServerObject.Roles[$ServerRoleName].EnumMemberNames()

Expand Down Expand Up @@ -428,9 +427,9 @@ function Test-TargetResource
}
else
{
if ($Members)
if ($PSBoundParameters.ContainsKey('Members'))
{
if ( $null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members))
if ($null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members))
Comment on lines +430 to +432
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify PowerShell Compare-Object behavior with empty/null desired member lists.
# Expected: These commands demonstrate why the resource should normalize/guard empty lists.

pwsh -NoProfile -Command "Compare-Object -ReferenceObject @('CONTOSO\User') -DifferenceObject @()"
pwsh -NoProfile -Command "Compare-Object -ReferenceObject @('CONTOSO\User') -DifferenceObject \$null"

Repository: dsccommunity/SqlServerDsc

Length of output: 246

🏁 Script executed:

cat -n source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1 | head -450 | tail -50

Repository: dsccommunity/SqlServerDsc

Length of output: 2282

🏁 Script executed:

# Find Get-TargetResource function and how Members is returned
grep -n "function Get-TargetResource" source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1

Repository: dsccommunity/SqlServerDsc

Length of output: 99

🏁 Script executed:

sed -n '31,150p' source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1

Repository: dsccommunity/SqlServerDsc

Length of output: 3181

🏁 Script executed:

grep -n "function Test-TargetResource" source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1

Repository: dsccommunity/SqlServerDsc

Length of output: 101

🏁 Script executed:

sed -n '325,410p' source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1

Repository: dsccommunity/SqlServerDsc

Length of output: 2396

🏁 Script executed:

grep -n "function Get-CorrectedMemberParameters" source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1

Repository: dsccommunity/SqlServerDsc

Length of output: 111

🏁 Script executed:

sed -n '650,750p' source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1

Repository: dsccommunity/SqlServerDsc

Length of output: 1029

🏁 Script executed:

# Verify the behavior of type casting $null to [System.String[]]
pwsh -NoProfile -Command '[System.String[]]$null | ForEach-Object { "Value: $_"; "Length: $($_.Length)" } ; if ($null -eq ([System.String[]]$null)) { "Result is null" } else { "Result is not null" }'

Repository: dsccommunity/SqlServerDsc

Length of output: 101

🏁 Script executed:

# Verify Compare-Object with $null values specifically
pwsh -NoProfile -Command "
try {
    Compare-Object -ReferenceObject @('CONTOSO\User') -DifferenceObject \$null
} catch {
    Write-Host 'Error caught: ' + \$_.Exception.Message
}
"

Repository: dsccommunity/SqlServerDsc

Length of output: 158

🏁 Script executed:

# Verify the actual scenario: what happens at line 430 when either side is $null
pwsh -NoProfile -Command "
# Simulate the scenario
\$getTargetResourceResult = @{ Members = \$null }
\$correctedParameters = @{ Members = \$null }

# Try the current code path
try {
    \$result = \$null -ne (Compare-Object -ReferenceObject \$getTargetResourceResult.Members -DifferenceObject \$correctedParameters.Members)
    Write-Host \"Result: \$result\"
} catch {
    Write-Host \"Error: \$(\$_.Exception.Message)\"
}

# Now test with empty arrays instead
\$getTargetResourceResult.Members = @()
\$correctedParameters.Members = @()
\$result = \$null -ne (Compare-Object -ReferenceObject \$getTargetResourceResult.Members -DifferenceObject \$correctedParameters.Members)
Write-Host \"With empty arrays, result: \$result\"
"

Repository: dsccommunity/SqlServerDsc

Length of output: 180

🏁 Script executed:

# Verify what happens with actual sysadmin role with no members
pwsh -NoProfile -Command "
# Simulating the actual code flow for sysadmin role with no members
\$Members = \$null
\$MembersToInclude = \$null
\$MembersToExclude = \$null
\$ServerRoleName = 'sysadmin'

# This simulates Get-CorrectedMemberParameters
\$correctedParameters = @{
    Members = [System.String[]](\$Members)
    MembersToInclude = [System.String[]](\$MembersToInclude)
    MembersToExclude = [System.String[]](\$MembersToExclude)
}

Write-Host \"Members type: \$(\$correctedParameters.Members.GetType())\"
Write-Host \"Members is null: \$(\$null -eq \$correctedParameters.Members)\"
Write-Host \"Members value: '\$(\$correctedParameters.Members)'\"
"

Repository: dsccommunity/SqlServerDsc

Length of output: 379

🏁 Script executed:

# Check if there are tests for this scenario and if the issue manifests
rg -A 5 "Compare-Object" source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1

Repository: dsccommunity/SqlServerDsc

Length of output: 442

🏁 Script executed:

# Check if there are unit tests for this scenario
find tests -name "*SqlRole*" -type f

Repository: dsccommunity/SqlServerDsc

Length of output: 214

🏁 Script executed:

# Search for test cases with Members parameter
rg -B 2 -A 5 "Members.*\`$null\|Members.*@\(\)" tests/Unit/DSC_SqlRole.Tests.ps1 | head -100

Repository: dsccommunity/SqlServerDsc

Length of output: 51

🏁 Script executed:

# Check for test cases involving Members parameter
rg "Members" tests/Unit/DSC_SqlRole.Tests.ps1 -c

Repository: dsccommunity/SqlServerDsc

Length of output: 72

🏁 Script executed:

grep -n "Members" tests/Unit/DSC_SqlRole.Tests.ps1 | head -30

Repository: dsccommunity/SqlServerDsc

Length of output: 2821

Normalize empty/null member lists before Compare-Object.

When comparing members, $getTargetResourceResult.Members can be $null (when a role has no members), and Get-CorrectedMemberParameters returns Members = [System.String[]]$null which remains $null. This causes Compare-Object at line 430 to fail with a terminating error: "Cannot bind argument to parameter 'DifferenceObject' because it is null." Normalize both sides to empty arrays before the comparison.

Proposed fix 
                 if ($PSBoundParameters.ContainsKey('Members'))
                 {
-                    if ($null -ne (Compare-Object -ReferenceObject $getTargetResourceResult.Members -DifferenceObject $correctedParameters.Members))
+                    $currentMembers = if ($null -eq $getTargetResourceResult.Members)
+                    {
+                        @()
+                    }
+                    else
+                    {
+                        @($getTargetResourceResult.Members)
+                    }
+
+                    $desiredMembers = if ($null -eq $correctedParameters.Members)
+                    {
+                        @()
+                    }
+                    else
+                    {
+                        @($correctedParameters.Members)
+                    }
+
+                    $membersAreDifferent = $currentMembers.Count -ne $desiredMembers.Count
+
+                    if (-not $membersAreDifferent -and $currentMembers.Count -gt 0)
+                    {
+                        $membersAreDifferent = $null -ne (
+                            Compare-Object -ReferenceObject $currentMembers -DifferenceObject $desiredMembers
+                        )
+                    }
+
+                    if ($membersAreDifferent)
                     {
                         Write-Verbose -Message (
                             $script:localizedData.DesiredMembersNotPresent `
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@source/DSCResources/DSC_SqlRole/DSC_SqlRole.psm1` around lines 428 - 430,
Normalize null/empty member lists to empty arrays before calling Compare-Object:
when PSBoundParameters contains 'Members', wrap both
$getTargetResourceResult.Members and $correctedParameters.Members with an array
coercion (e.g., use @(...)) so neither side is $null; then call Compare-Object
on those normalized arrays (reference the symbols
$getTargetResourceResult.Members, correctedParameters.Members, Compare-Object
and Get-CorrectedMemberParameters).

{
Write-Verbose -Message (
$script:localizedData.DesiredMembersNotPresent `
Expand All @@ -442,7 +441,7 @@ function Test-TargetResource
}
else
{
if ($MembersToInclude)
if ($PSBoundParameters.ContainsKey('MembersToInclude'))
{
foreach ($memberToInclude in $correctedParameters.MembersToInclude)
{
Expand All @@ -458,7 +457,7 @@ function Test-TargetResource
}
}

if ($MembersToExclude)
if ($PSBoundParameters.ContainsKey('MembersToExclude'))
{
foreach ($memberToExclude in $correctedParameters.MembersToExclude)
{
Expand Down
Loading