Browse Source

fix(access): report a failing change of a descriptor as an error per descriptor

In the four -SecurityDescriptor parameter sets of Add-NTFSAccess,
Remove-NTFSAccess, Add-NTFSAudit, and Remove-NTFSAudit, a change that
throws, such as a Deny entry without rights, ended the cmdlet with a
terminating error, while the -Path parameter sets already wrote an
AddAceError or RemoveAceError for the failing item and went on to the
next one. The two kinds of parameter sets now behave the same way.

Each descriptor's change is wrapped in its own try/catch: a failure
writes a non-terminating AddAceError or RemoveAceError, category
WriteError, that targets the descriptor, and the cmdlet continues
with the next descriptor in the pipeline instead of stopping. -PassThru
stays outside the try, so what a later command raises when the
cmdlet writes its output is never caught there. The existing
TestAuditSection check of the audit cmdlets, and everything else
about a successful change, is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
pull/131/head
Raimund Andree 2 weeks ago
parent
commit
62699eeb7b
  1. 10
      NTFSSecurity/AccessCmdlets/AddAccess.cs
  2. 10
      NTFSSecurity/AccessCmdlets/RemoveAccess.cs
  3. 10
      NTFSSecurity/AuditCmdlets/AddAudit.cs
  4. 10
      NTFSSecurity/AuditCmdlets/RemoveAudit.cs
  5. 104
      Tests/ParameterSets.Access.Tests.ps1
  6. 81
      Tests/ParameterSets.Audit.Tests.ps1

10
NTFSSecurity/AccessCmdlets/AddAccess.cs

@ -210,7 +210,15 @@ namespace NTFSSecurity
{ {
foreach (var sd in securityDescriptors) foreach (var sd in securityDescriptors)
{ {
FileSystemAccessRule2.AddFileSystemAccessRule(sd, account.ToList(), accessRights, accessTypeValue, inheritance, propagation); try
{
FileSystemAccessRule2.AddFileSystemAccessRule(sd, account.ToList(), accessRights, accessTypeValue, inheritance, propagation);
}
catch (Exception ex)
{
WriteError(new ErrorRecord(ex, "AddAceError", ErrorCategory.WriteError, sd));
continue;
}
if (passThru == true) if (passThru == true)
{ {

10
NTFSSecurity/AccessCmdlets/RemoveAccess.cs

@ -221,7 +221,15 @@ namespace NTFSSecurity
{ {
foreach (var sd in securityDescriptors) foreach (var sd in securityDescriptors)
{ {
FileSystemAccessRule2.RemoveFileSystemAccessRule(sd, account.ToList(), accessRights, accessTypeValue, inheritance, propagation, removeSpecific); try
{
FileSystemAccessRule2.RemoveFileSystemAccessRule(sd, account.ToList(), accessRights, accessTypeValue, inheritance, propagation, removeSpecific);
}
catch (Exception ex)
{
WriteError(new ErrorRecord(ex, "RemoveAceError", ErrorCategory.WriteError, sd));
continue;
}
if (passThru == true) if (passThru == true)
{ {

10
NTFSSecurity/AuditCmdlets/AddAudit.cs

@ -214,7 +214,15 @@ namespace NTFSSecurity
continue; continue;
} }
FileSystemAuditRule2.AddFileSystemAuditRule(sd, account.ToList(), accessRights, auditFlagsValue, inheritance, propagation); try
{
FileSystemAuditRule2.AddFileSystemAuditRule(sd, account.ToList(), accessRights, auditFlagsValue, inheritance, propagation);
}
catch (Exception ex)
{
WriteError(new ErrorRecord(ex, "AddAceError", ErrorCategory.WriteError, sd));
continue;
}
if (passThru == true) if (passThru == true)
{ {

10
NTFSSecurity/AuditCmdlets/RemoveAudit.cs

@ -225,7 +225,15 @@ namespace NTFSSecurity
continue; continue;
} }
FileSystemAuditRule2.RemoveFileSystemAuditRule(sd, account.ToList(), accessRights, auditFlagsValue, inheritance, propagation, removeSpecific); try
{
FileSystemAuditRule2.RemoveFileSystemAuditRule(sd, account.ToList(), accessRights, auditFlagsValue, inheritance, propagation, removeSpecific);
}
catch (Exception ex)
{
WriteError(new ErrorRecord(ex, "RemoveAceError", ErrorCategory.WriteError, sd));
continue;
}
if (passThru == true) if (passThru == true)
{ {

104
Tests/ParameterSets.Access.Tests.ps1

@ -418,6 +418,57 @@ Describe 'Add-NTFSAccess' {
(Get-Acl -LiteralPath $second).Sddl | Should -BeExactly $before[1] (Get-Acl -LiteralPath $second).Sddl | Should -BeExactly $before[1]
} }
} }
# Item 5 of Decision 27: a failing change to one of several descriptors (a Deny entry without rights, which .NET
# refuses to build, the same as Access.Tests.ps1:774 pins for -Path) reports an error for that descriptor and goes
# on with the next one, instead of ending the cmdlet with a terminating error.
Context 'A failing change to one of several descriptors' {
It 'Should write an AddAceError for the descriptor whose change fails in the SDSimple set, continue with the next, and return -PassThru only for the one that succeeded' {
$first = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorSimpleFirst'
$second = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorSimpleSecond'
$descriptors = @(Get-NTFSSecurityDescriptor -Path $first, $second -ErrorAction Stop)
$before = $descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access')
$rows = @(
[pscustomobject]@{ SecurityDescriptor = $descriptors[0]; Account = 'S-1-1-0'; AccessRights = 'None'; AccessType = 'Deny'; AppliesTo = 'ThisFolderOnly' }
[pscustomobject]@{ SecurityDescriptor = $descriptors[1]; Account = 'S-1-1-0'; AccessRights = 'ReadData'; AccessType = 'Deny'; AppliesTo = 'ThisFolderOnly' }
)
$result = @($rows | Add-NTFSAccess -PassThru -ErrorVariable sdErrors -ErrorAction SilentlyContinue)
$sdErrors | Should -HaveCount 1
$sdErrors[0].FullyQualifiedErrorId | Should -BeLike 'AddAceError,*'
$sdErrors[0].CategoryInfo.Category | Should -Be 'WriteError'
$sdErrors[0].TargetObject | Should -Be $descriptors[0]
$descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access') | Should -BeExactly $before
$result | Should -Not -BeNullOrEmpty
$result | ForEach-Object -Process { $_.FullName | Should -Be $second }
@($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-1-0' -and $_.AccessControlType -eq 'Deny' }) | Should -HaveCount 1
}
It 'Should write an AddAceError for the descriptor whose change fails in the SDComplex set, continue with the next, and return -PassThru only for the one that succeeded' {
$first = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexFirst'
$second = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexSecond'
$descriptors = @(Get-NTFSSecurityDescriptor -Path $first, $second -ErrorAction Stop)
$before = $descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access')
$rows = @(
[pscustomobject]@{ SecurityDescriptor = $descriptors[0]; Account = 'S-1-1-0'; AccessRights = 'None'; AccessType = 'Deny' }
[pscustomobject]@{ SecurityDescriptor = $descriptors[1]; Account = 'S-1-1-0'; AccessRights = 'ReadData'; AccessType = 'Deny' }
)
$result = @($rows | Add-NTFSAccess -PassThru -ErrorVariable sdErrors -ErrorAction SilentlyContinue)
$sdErrors | Should -HaveCount 1
$sdErrors[0].FullyQualifiedErrorId | Should -BeLike 'AddAceError,*'
$sdErrors[0].CategoryInfo.Category | Should -Be 'WriteError'
$sdErrors[0].TargetObject | Should -Be $descriptors[0]
$descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access') | Should -BeExactly $before
$result | Should -Not -BeNullOrEmpty
$result | ForEach-Object -Process { $_.FullName | Should -Be $second }
@($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-1-0' -and $_.AccessControlType -eq 'Deny' }) | Should -HaveCount 1
}
}
} }
Describe 'Remove-NTFSAccess' { Describe 'Remove-NTFSAccess' {
@ -736,6 +787,59 @@ Describe 'Remove-NTFSAccess' {
} }
} }
} }
# Item 5 of Decision 27: a failing change to one of several descriptors (a Deny entry without rights, which .NET
# refuses to build, the same as Access.Tests.ps1:571 pins for -Path) reports an error for that descriptor and goes
# on with the next one, instead of ending the cmdlet with a terminating error.
Context 'A failing change to one of several descriptors' {
It 'Should write a RemoveAceError for the descriptor whose change fails in the SDSimple set, continue with the next, and return -PassThru only for the one that succeeded' {
$first = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorSimpleFirst'
$second = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorSimpleSecond'
Add-NTFSAccess -Path $first, $second -Account 'S-1-1-0' -AccessRights ReadData -AccessType Deny -AppliesTo ThisFolderOnly -ErrorAction Stop
$descriptors = @(Get-NTFSSecurityDescriptor -Path $first, $second -ErrorAction Stop)
$before = $descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access')
$rows = @(
[pscustomobject]@{ SecurityDescriptor = $descriptors[0]; Account = 'S-1-1-0'; AccessRights = 'None'; AccessType = 'Deny'; AppliesTo = 'ThisFolderOnly' }
[pscustomobject]@{ SecurityDescriptor = $descriptors[1]; Account = 'S-1-1-0'; AccessRights = 'ReadData'; AccessType = 'Deny'; AppliesTo = 'ThisFolderOnly' }
)
$result = @($rows | Remove-NTFSAccess -PassThru -ErrorVariable sdErrors -ErrorAction SilentlyContinue)
$sdErrors | Should -HaveCount 1
$sdErrors[0].FullyQualifiedErrorId | Should -BeLike 'RemoveAceError,*'
$sdErrors[0].CategoryInfo.Category | Should -Be 'WriteError'
$sdErrors[0].TargetObject | Should -Be $descriptors[0]
$descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access') | Should -BeExactly $before
$result | Should -Not -BeNullOrEmpty
$result | ForEach-Object -Process { $_.FullName | Should -Be $second }
@($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-1-0' }) | Should -BeNullOrEmpty
}
It 'Should write a RemoveAceError for the descriptor whose change fails in the SDComplex set, continue with the next, and return -PassThru only for the one that succeeded' {
$first = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexFirst'
$second = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexSecond'
Add-NTFSAccess -Path $first, $second -Account 'S-1-1-0' -AccessRights ReadData -AccessType Deny -ErrorAction Stop
$descriptors = @(Get-NTFSSecurityDescriptor -Path $first, $second -ErrorAction Stop)
$before = $descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access')
$rows = @(
[pscustomobject]@{ SecurityDescriptor = $descriptors[0]; Account = 'S-1-1-0'; AccessRights = 'None'; AccessType = 'Deny' }
[pscustomobject]@{ SecurityDescriptor = $descriptors[1]; Account = 'S-1-1-0'; AccessRights = 'ReadData'; AccessType = 'Deny' }
)
$result = @($rows | Remove-NTFSAccess -PassThru -ErrorVariable sdErrors -ErrorAction SilentlyContinue)
$sdErrors | Should -HaveCount 1
$sdErrors[0].FullyQualifiedErrorId | Should -BeLike 'RemoveAceError,*'
$sdErrors[0].CategoryInfo.Category | Should -Be 'WriteError'
$sdErrors[0].TargetObject | Should -Be $descriptors[0]
$descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Access') | Should -BeExactly $before
$result | Should -Not -BeNullOrEmpty
$result | ForEach-Object -Process { $_.FullName | Should -Be $second }
@($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-1-0' }) | Should -BeNullOrEmpty
}
}
} }
Describe 'Clear-NTFSAccess' { Describe 'Clear-NTFSAccess' {

81
Tests/ParameterSets.Audit.Tests.ps1

@ -357,6 +357,55 @@ Describe 'Add-NTFSAudit' {
(Get-TestAccountAuditRule -Acl $sd.SecurityDescriptor -Account $probe) | Should -HaveCount 1 (Get-TestAccountAuditRule -Acl $sd.SecurityDescriptor -Account $probe) | Should -HaveCount 1
} }
} }
# Item 5 of Decision 27: a failing change to one of several descriptors (an audit entry without rights, which
# .NET refuses to build, the same as Audit.Tests.ps1:141 pins for -Path) reports an error for that descriptor and
# goes on with the next one, instead of ending the cmdlet with a terminating error.
Context 'A failing change to one of several descriptors' {
It 'Should write an AddAceError for the descriptor whose change fails in the SDSimple set, continue with the next, and return -PassThru only for the one that succeeded' -Skip:(-not $canReadAudit) {
$first = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorSimpleFirst'
$second = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorSimpleSecond'
$descriptors = @(Get-NTFSSecurityDescriptor -Path $first, $second -ErrorAction Stop)
$before = $descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit')
$rows = @(
[pscustomobject]@{ SecurityDescriptor = $descriptors[0]; Account = $probe; AccessRights = 'None'; AuditFlags = 'Success'; AppliesTo = 'ThisFolderOnly' }
[pscustomobject]@{ SecurityDescriptor = $descriptors[1]; Account = $probe; AccessRights = 'ReadData'; AuditFlags = 'Success'; AppliesTo = 'ThisFolderOnly' }
)
$result = @($rows | Add-NTFSAudit -PassThru -ErrorVariable sdErrors -ErrorAction SilentlyContinue)
$sdErrors | Should -HaveCount 1
$sdErrors[0].FullyQualifiedErrorId | Should -BeLike 'AddAceError,*'
$sdErrors[0].CategoryInfo.Category | Should -Be 'WriteError'
$sdErrors[0].TargetObject | Should -Be $descriptors[0]
$descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit') | Should -BeExactly $before
$result | Should -Not -BeNullOrEmpty
$result | ForEach-Object -Process { $_.FullName | Should -Be $second }
}
It 'Should write an AddAceError for the descriptor whose change fails in the SDComplex set, continue with the next, and return -PassThru only for the one that succeeded' -Skip:(-not $canReadAudit) {
$first = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexFirst'
$second = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexSecond'
$descriptors = @(Get-NTFSSecurityDescriptor -Path $first, $second -ErrorAction Stop)
$before = $descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit')
$rows = @(
[pscustomobject]@{ SecurityDescriptor = $descriptors[0]; Account = $probe; AccessRights = 'None'; AuditFlags = 'Success' }
[pscustomobject]@{ SecurityDescriptor = $descriptors[1]; Account = $probe; AccessRights = 'ReadData'; AuditFlags = 'Success' }
)
$result = @($rows | Add-NTFSAudit -PassThru -ErrorVariable sdErrors -ErrorAction SilentlyContinue)
$sdErrors | Should -HaveCount 1
$sdErrors[0].FullyQualifiedErrorId | Should -BeLike 'AddAceError,*'
$sdErrors[0].CategoryInfo.Category | Should -Be 'WriteError'
$sdErrors[0].TargetObject | Should -Be $descriptors[0]
$descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit') | Should -BeExactly $before
$result | Should -Not -BeNullOrEmpty
$result | ForEach-Object -Process { $_.FullName | Should -Be $second }
}
}
} }
Describe 'Remove-NTFSAudit' { Describe 'Remove-NTFSAudit' {
@ -605,6 +654,38 @@ Describe 'Remove-NTFSAudit' {
Get-TestDaclSddl -Path $file | Should -BeExactly $daclBefore Get-TestDaclSddl -Path $file | Should -BeExactly $daclBefore
} }
} }
# Item 5 of Decision 27: a failing change to one of several descriptors (an audit entry without rights, which
# .NET refuses to build, the same as Audit.Tests.ps1:141 pins for -Path) reports an error for that descriptor and
# goes on with the next one, instead of ending the cmdlet with a terminating error. Only the SDComplex set is
# listed as open in Quality-Gate-Test-Round-2026-10-20-ParameterSets.csv (row 26); SDSimple is already closed.
Context 'A failing change to one of several descriptors' {
It 'Should write a RemoveAceError for the descriptor whose change fails in the SDComplex set, continue with the next, and return -PassThru only for the one that succeeded' -Skip:(-not $canReadAudit) {
$first = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexFirst'
$second = New-TestSandboxItem -Sandbox $sandbox -Name 'SdErrorComplexSecond'
# A keeper entry for a different account on both items: audit entries are not inherited by default, so
# without one, -PassThru of the descriptor that succeeds would show nothing once its only entry is gone.
Add-NTFSAudit -Path $first, $second -Account $probe -AccessRights ReadData -AuditFlags Success -ErrorAction Stop
Add-NTFSAudit -Path $first, $second -Account $keeper -AccessRights Delete -AuditFlags Success -ErrorAction Stop
$descriptors = @(Get-NTFSSecurityDescriptor -Path $first, $second -ErrorAction Stop)
$before = $descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit')
$rows = @(
[pscustomobject]@{ SecurityDescriptor = $descriptors[0]; Account = $probe; AccessRights = 'None'; AuditFlags = 'Success' }
[pscustomobject]@{ SecurityDescriptor = $descriptors[1]; Account = $probe; AccessRights = 'ReadData'; AuditFlags = 'Success' }
)
$result = @($rows | Remove-NTFSAudit -PassThru -ErrorVariable sdErrors -ErrorAction SilentlyContinue)
$sdErrors | Should -HaveCount 1
$sdErrors[0].FullyQualifiedErrorId | Should -BeLike 'RemoveAceError,*'
$sdErrors[0].CategoryInfo.Category | Should -Be 'WriteError'
$sdErrors[0].TargetObject | Should -Be $descriptors[0]
$descriptors[0].SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit') | Should -BeExactly $before
$result | Should -Not -BeNullOrEmpty
$result | ForEach-Object -Process { $_.FullName | Should -Be $second }
}
}
} }
Describe 'Clear-NTFSAudit' { Describe 'Clear-NTFSAudit' {

Loading…
Cancel
Save