fix(access): stop for a named $null of the five parameters

A named $null of AccessType, AuditFlags, InheritanceFlags,
PropagationFlags, or AppliesTo (a literal $null, a variable that holds
$null such as a misspelled property, the alias -AccessControlType,
splatting, or a null entry of $PSDefaultParameterValues) silently kept
the parameter's default instead of stopping: Add-NTFSAccess -AccessType
$null created an Allow entry, Add-NTFSAudit -AuditFlags $null audited
Success and Failure, and Remove-NTFSAccess -AccessType $null removed
nothing. Before the five parameters became object (Decision 27, fix
B), the same named $null ended with a terminating ParameterBindingFailed.

Each cmdlet's BeginProcessing now checks, for a value that is null,
whether MyInvocation.BoundParameters has the parameter's name; a piped
property that is null is not bound yet at that point and still keeps
the default, but a value that is already bound and null is reported
the same way as a bad named value: a terminating InvalidParameterValue
error naming the parameter and its valid values. AppliesTo is Mandatory
and a named $null of it was already a terminating
ParameterArgumentValidationErrorNullNotAllowed from the binder itself,
unchanged by this fix.

ParameterConversion gains the message for this case next to the
existing blank-value message, so that the wording is built in one
place.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>

NTFSSecurity/AccessCmdlets/AddAccess.cs

@@ -234,11 +234,22 @@ namespace NTFSSecurity
         }
 
         // A bad value named on the command line is already bound when BeginProcessing runs, so it is reported there
-        // as a terminating error, the way the binder itself would have reported it for a typed parameter.
+        // as a terminating error, the way the binder itself would have reported it for a typed parameter. A named
+        // $null is bound too, whether it came from a literal $null, a variable, the alias -AccessControlType,
+        // splatting, or $PSDefaultParameterValues (MyInvocation.BoundParameters always has the canonical name), and
+        // is an error the same way; a $null that a piped object's property carries is not bound yet at this point,
+        // so it keeps the parameter's own default instead, as before.
         private void ValidateNamedValue<T>(object value, string parameterName)
         {
             if (value == null)
             {
+                if (!MyInvocation.BoundParameters.ContainsKey(parameterName))
+                {
+                    return;
+                }
+
+                var nullException = new ArgumentException(ParameterConversion.DescribeNull<T>(parameterName));
+                ThrowTerminatingError(new ErrorRecord(nullException, "InvalidParameterValue", ErrorCategory.InvalidArgument, value));
                 return;
             }
 

NTFSSecurity/AccessCmdlets/RemoveAccess.cs

@@ -245,11 +245,22 @@ namespace NTFSSecurity
         }
 
         // A bad value named on the command line is already bound when BeginProcessing runs, so it is reported there
-        // as a terminating error, the way the binder itself would have reported it for a typed parameter.
+        // as a terminating error, the way the binder itself would have reported it for a typed parameter. A named
+        // $null is bound too, whether it came from a literal $null, a variable, the alias -AccessControlType,
+        // splatting, or $PSDefaultParameterValues (MyInvocation.BoundParameters always has the canonical name), and
+        // is an error the same way; a $null that a piped object's property carries is not bound yet at this point,
+        // so it keeps the parameter's own default instead, as before.
         private void ValidateNamedValue<T>(object value, string parameterName)
         {
             if (value == null)
             {
+                if (!MyInvocation.BoundParameters.ContainsKey(parameterName))
+                {
+                    return;
+                }
+
+                var nullException = new ArgumentException(ParameterConversion.DescribeNull<T>(parameterName));
+                ThrowTerminatingError(new ErrorRecord(nullException, "InvalidParameterValue", ErrorCategory.InvalidArgument, value));
                 return;
             }
 

NTFSSecurity/AuditCmdlets/AddAudit.cs

@@ -238,11 +238,22 @@ namespace NTFSSecurity
         }
 
         // A bad value named on the command line is already bound when BeginProcessing runs, so it is reported there
-        // as a terminating error, the way the binder itself would have reported it for a typed parameter.
+        // as a terminating error, the way the binder itself would have reported it for a typed parameter. A named
+        // $null is bound too, whether it came from a literal $null, a variable, the alias -AccessControlType,
+        // splatting, or $PSDefaultParameterValues (MyInvocation.BoundParameters always has the canonical name), and
+        // is an error the same way; a $null that a piped object's property carries is not bound yet at this point,
+        // so it keeps the parameter's own default instead, as before.
         private void ValidateNamedValue<T>(object value, string parameterName)
         {
             if (value == null)
             {
+                if (!MyInvocation.BoundParameters.ContainsKey(parameterName))
+                {
+                    return;
+                }
+
+                var nullException = new ArgumentException(ParameterConversion.DescribeNull<T>(parameterName));
+                ThrowTerminatingError(new ErrorRecord(nullException, "InvalidParameterValue", ErrorCategory.InvalidArgument, value));
                 return;
             }
 

NTFSSecurity/AuditCmdlets/RemoveAudit.cs

@@ -249,11 +249,22 @@ namespace NTFSSecurity
         }
 
         // A bad value named on the command line is already bound when BeginProcessing runs, so it is reported there
-        // as a terminating error, the way the binder itself would have reported it for a typed parameter.
+        // as a terminating error, the way the binder itself would have reported it for a typed parameter. A named
+        // $null is bound too, whether it came from a literal $null, a variable, the alias -AccessControlType,
+        // splatting, or $PSDefaultParameterValues (MyInvocation.BoundParameters always has the canonical name), and
+        // is an error the same way; a $null that a piped object's property carries is not bound yet at this point,
+        // so it keeps the parameter's own default instead, as before.
         private void ValidateNamedValue<T>(object value, string parameterName)
         {
             if (value == null)
             {
+                if (!MyInvocation.BoundParameters.ContainsKey(parameterName))
+                {
+                    return;
+                }
+
+                var nullException = new ArgumentException(ParameterConversion.DescribeNull<T>(parameterName));
+                ThrowTerminatingError(new ErrorRecord(nullException, "InvalidParameterValue", ErrorCategory.InvalidArgument, value));
                 return;
             }
 

NTFSSecurity/ParameterConversion.cs

@@ -50,6 +50,19 @@ namespace NTFSSecurity
             }
         }
 
+        /// <summary>
+        /// Builds the message for a parameter whose value is explicitly null although it was bound on the command
+        /// line (a literal $null, a variable that holds $null, or a null entry of $PSDefaultParameterValues): "The
+        /// value for parameter -AccessType is null. Valid values: Allow, Deny." A piped property that is null is
+        /// not an error there: it means the property was not specified and the parameter's own default applies;
+        /// only the cmdlet knows, from MyInvocation.BoundParameters, whether a null came from such a named value.
+        /// </summary>
+        /// <param name="parameterName">The name of the parameter, without the leading dash, for the message.</param>
+        internal static string DescribeNull<T>(string parameterName)
+        {
+            return string.Format("The value for parameter -{0} is null. {1}", parameterName, DescribeValidValues(typeof(T)));
+        }
+
         // The valid names of the enumeration, in declaration order, with a note for a flags enumeration that a
         // combination of them is written separated with commas.
         private static string DescribeValidValues(Type enumType)

Tests/ParameterSets.Access.Tests.ps1

@@ -1384,6 +1384,16 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
             @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo' }
         }
 
+        # R1-2: a named $null (a literal $null, a variable, an alias, splatting, or $PSDefaultParameterValues) must
+        # stay a terminating error, the same as a bad named value; only a $null that a piped object's property
+        # carries keeps the parameter's own default.
+        $accessNamedNullCases = foreach ($cmdlet in 'Add-NTFSAccess', 'Remove-NTFSAccess') {
+            @{ Cmdlet = $cmdlet; Parameter = 'AccessType' }
+            @{ Cmdlet = $cmdlet; Parameter = 'InheritanceFlags' }
+            @{ Cmdlet = $cmdlet; Parameter = 'PropagationFlags' }
+            @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo' }
+        }
+
         $accessValidAccessTypeShapeCases = @(
             @{ Description = 'the exact name'; Value = 'Deny' }
             @{ Description = 'a different case'; Value = 'deny' }
@@ -1612,6 +1622,98 @@ Describe 'Invalid and valid values of AccessType, InheritanceFlags, PropagationF
         }
     }
 
+    # R1-2: rc8 ended a named $null (for example a variable that misspells a property, such as -AccessType
+    # $rule.AccessTpye) with a terminating ParameterBindingFailed; the object-typed parameter accepted it and
+    # silently kept the default instead. A named $null is now reported the same way as a bad named value.
+    Context 'A named $null is a terminating error' {
+        It 'Should throw a terminating error for a named $null of <Cmdlet> -<Parameter>, and change nothing' -ForEach $accessNamedNullCases {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'NamedNull' -Directory
+            $before = (Get-Acl -LiteralPath $folder).Sddl
+            $namedArguments = @{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData' }
+            $namedArguments[$Parameter] = $null
+
+            $caughtId = $null
+            $caughtMessage = $null
+            try {
+                & $Cmdlet @namedArguments -ErrorAction Stop
+            }
+            catch {
+                $caughtId = $_.FullyQualifiedErrorId
+                $caughtMessage = $_.Exception.Message
+            }
+
+            if ($Parameter -eq 'AppliesTo') {
+                # AppliesTo is Mandatory: PowerShell's own binder already rejects a named $null before the cmdlet
+                # runs, with its own error id, unrelated to this fix; characterized here so that the row is not
+                # silently missing from the table.
+                $caughtId | Should -BeLike 'ParameterArgumentValidationErrorNullNotAllowed,*'
+            }
+            else {
+                $caughtId | Should -BeLike 'InvalidParameterValue,*'
+                $caughtMessage | Should -Match ([regex]::Escape($Parameter))
+                $caughtMessage | Should -Match 'is null'
+            }
+            (Get-Acl -LiteralPath $folder).Sddl | Should -BeExactly $before
+        }
+
+        It 'Should throw a terminating InvalidParameterValue error for a named $null of <Cmdlet> -AccessControlType (the alias), and change nothing' -ForEach @(
+            @{ Cmdlet = 'Add-NTFSAccess' }
+            @{ Cmdlet = 'Remove-NTFSAccess' }
+        ) {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'NamedNullAlias' -Directory
+            $before = (Get-Acl -LiteralPath $folder).Sddl
+
+            $caughtId = $null
+            try {
+                & $Cmdlet -Path $folder -Account 'S-1-1-0' -AccessRights ReadData -AccessControlType $null -ErrorAction Stop
+            }
+            catch {
+                $caughtId = $_.FullyQualifiedErrorId
+            }
+
+            $caughtId | Should -BeLike 'InvalidParameterValue,*'
+            (Get-Acl -LiteralPath $folder).Sddl | Should -BeExactly $before
+        }
+
+        It 'Should throw a terminating InvalidParameterValue error for a $null of -AccessType from $PSDefaultParameterValues, and change nothing' {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'NamedNullDefault' -Directory
+            $before = (Get-Acl -LiteralPath $folder).Sddl
+            $PSDefaultParameterValues['Add-NTFSAccess:AccessType'] = $null
+
+            $caughtId = $null
+            try {
+                Add-NTFSAccess -Path $folder -Account 'S-1-1-0' -AccessRights ReadData -ErrorAction Stop
+            }
+            catch {
+                $caughtId = $_.FullyQualifiedErrorId
+            }
+            finally {
+                $PSDefaultParameterValues.Remove('Add-NTFSAccess:AccessType')
+            }
+
+            $caughtId | Should -BeLike 'InvalidParameterValue,*'
+            (Get-Acl -LiteralPath $folder).Sddl | Should -BeExactly $before
+        }
+
+        It 'Should still give <Cmdlet> -<Parameter> its own default for a piped $null, unaffected by the named $null fix' -ForEach $accessNamedNullCases {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'PipedNullUnaffected' -Directory
+            $extra = @{ }
+            $extra[$Parameter] = $null
+
+            [pscustomobject] (@{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData' } + $extra) |
+                & $Cmdlet -ErrorVariable pipedErrors -ErrorAction SilentlyContinue
+
+            if ($Parameter -eq 'AppliesTo') {
+                # AppliesTo is Mandatory: a piped $null is the pre-existing binder validation error, unrelated to this fix.
+                $pipedErrors | Should -HaveCount 1
+                $pipedErrors[0].FullyQualifiedErrorId | Should -BeLike 'ParameterArgumentValidationErrorNullNotAllowed,*'
+            }
+            else {
+                $pipedErrors | Should -BeNullOrEmpty
+            }
+        }
+    }
+
     Context 'Choosing the parameter set with a mix of string and enum values' {
         It 'Should reject a piped object whose AppliesTo is <AppliesToShape> combined with InheritanceFlags as <FlagShape> with an AmbiguousParameterSet error' -ForEach $accessAmbiguousValueCases {
             $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'AmbiguousPipedMix' -Directory

Tests/ParameterSets.Audit.Tests.ps1

@@ -1054,6 +1054,16 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
             @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo'; Value = 'Typoo' }
         }
 
+        # R1-2: a named $null (a literal $null, a variable, splatting, or $PSDefaultParameterValues) must stay a
+        # terminating error, the same as a bad named value; only a $null that a piped object's property carries
+        # keeps the parameter's own default.
+        $auditNamedNullCases = foreach ($cmdlet in 'Add-NTFSAudit', 'Remove-NTFSAudit') {
+            @{ Cmdlet = $cmdlet; Parameter = 'AuditFlags' }
+            @{ Cmdlet = $cmdlet; Parameter = 'InheritanceFlags' }
+            @{ Cmdlet = $cmdlet; Parameter = 'PropagationFlags' }
+            @{ Cmdlet = $cmdlet; Parameter = 'AppliesTo' }
+        }
+
         $auditValidAuditFlagsShapeCases = @(
             @{ Description = 'the exact name'; Value = 'Failure' }
             @{ Description = 'a different case'; Value = 'failure' }
@@ -1294,6 +1304,82 @@ Describe 'Invalid and valid values of AuditFlags, InheritanceFlags, PropagationF
         }
     }
 
+    # R1-2: rc8 ended a named $null with a terminating ParameterBindingFailed; the object-typed parameter accepted
+    # it and silently kept the default instead. A named $null is now reported the same way as a bad named value.
+    Context 'A named $null is a terminating error' {
+        It 'Should throw a terminating error for a named $null of <Cmdlet> -<Parameter>, and change nothing' -ForEach $auditNamedNullCases {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'NamedNull' -Directory
+            $before = if ($canReadAudit) { Get-TestAuditSddl -Path $folder }
+            $namedArguments = @{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData' }
+            $namedArguments[$Parameter] = $null
+
+            $caughtId = $null
+            $caughtMessage = $null
+            try {
+                & $Cmdlet @namedArguments -ErrorAction Stop
+            }
+            catch {
+                $caughtId = $_.FullyQualifiedErrorId
+                $caughtMessage = $_.Exception.Message
+            }
+
+            if ($Parameter -eq 'AppliesTo') {
+                # AppliesTo is Mandatory: PowerShell's own binder already rejects a named $null before the cmdlet
+                # runs, with its own error id, unrelated to this fix; characterized here so that the row is not
+                # silently missing from the table.
+                $caughtId | Should -BeLike 'ParameterArgumentValidationErrorNullNotAllowed,*'
+            }
+            else {
+                $caughtId | Should -BeLike 'InvalidParameterValue,*'
+                $caughtMessage | Should -Match ([regex]::Escape($Parameter))
+                $caughtMessage | Should -Match 'is null'
+            }
+            if ($canReadAudit) {
+                Get-TestAuditSddl -Path $folder | Should -BeExactly $before
+            }
+        }
+
+        It 'Should throw a terminating InvalidParameterValue error for a $null of -AuditFlags from $PSDefaultParameterValues, and change nothing' {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'NamedNullDefault' -Directory
+            $before = if ($canReadAudit) { Get-TestAuditSddl -Path $folder }
+            $PSDefaultParameterValues['Add-NTFSAudit:AuditFlags'] = $null
+
+            $caughtId = $null
+            try {
+                Add-NTFSAudit -Path $folder -Account 'S-1-1-0' -AccessRights ReadData -ErrorAction Stop
+            }
+            catch {
+                $caughtId = $_.FullyQualifiedErrorId
+            }
+            finally {
+                $PSDefaultParameterValues.Remove('Add-NTFSAudit:AuditFlags')
+            }
+
+            $caughtId | Should -BeLike 'InvalidParameterValue,*'
+            if ($canReadAudit) {
+                Get-TestAuditSddl -Path $folder | Should -BeExactly $before
+            }
+        }
+
+        It 'Should still give <Cmdlet> -<Parameter> its own default for a piped $null, unaffected by the named $null fix' -ForEach $auditNamedNullCases -Skip:(-not $canReadAudit) {
+            $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'PipedNullUnaffected' -Directory
+            $extra = @{ }
+            $extra[$Parameter] = $null
+
+            [pscustomobject] (@{ Path = $folder; Account = 'S-1-1-0'; AccessRights = 'ReadData' } + $extra) |
+                & $Cmdlet -ErrorVariable pipedErrors -ErrorAction SilentlyContinue
+
+            if ($Parameter -eq 'AppliesTo') {
+                # AppliesTo is Mandatory: a piped $null is the pre-existing binder validation error, unrelated to this fix.
+                $pipedErrors | Should -HaveCount 1
+                $pipedErrors[0].FullyQualifiedErrorId | Should -BeLike 'ParameterArgumentValidationErrorNullNotAllowed,*'
+            }
+            else {
+                $pipedErrors | Should -BeNullOrEmpty
+            }
+        }
+    }
+
     Context 'Choosing the parameter set with a mix of string and enum values' {
         It 'Should reject a piped object whose AppliesTo is <AppliesToShape> combined with InheritanceFlags as <FlagShape> with an AmbiguousParameterSet error' -ForEach $auditAmbiguousValueCases {
             $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'AmbiguousPipedMix' -Directory