Browse Source

fix: write -PassThru of the inheritance cmdlets only after a change

Defect 20 (#74). Enable-NTFSAccessInheritance,
Disable-NTFSAccessInheritance, Enable-NTFSAuditInheritance,
Disable-NTFSAuditInheritance, and Set-NTFSInheritance wrote the
-PassThru object in a finally block. After a failed change, such as an
audit change without the Security privilege, they returned the unchanged
state, which made the inheritance look disabled; when the item could not
be read at all, reading the state in the finally block threw and stopped
the command. The object is now written only after a successful change.

Tests/Inheritance.Tests.ps1: 5 tests. The audit tests need the missing
privilege and skip in CI; the read-deny tests skip where the Backup
privilege may bypass the deny entry (CI).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
pull/102/head
Raimund Andree 1 week ago
parent
commit
46d71bb2e3
  1. 6
      CHANGELOG.md
  2. 2
      Docs/Cmdlets/Disable-NTFSAccessInheritance.md
  3. 2
      Docs/Cmdlets/Disable-NTFSAuditInheritance.md
  4. 2
      Docs/Cmdlets/Enable-NTFSAccessInheritance.md
  5. 2
      Docs/Cmdlets/Enable-NTFSAuditInheritance.md
  6. 2
      Docs/Cmdlets/Set-NTFSInheritance.md
  7. 9
      NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs
  8. 9
      NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs
  9. 9
      NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs
  10. 9
      NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs
  11. 9
      NTFSSecurity/InheritanceCmdlets/SetInheritance.cs
  12. 5
      NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml
  13. 40
      Tests/Inheritance.Tests.ps1

6
CHANGELOG.md

@ -118,5 +118,11 @@ The format is based on
- Fix `Remove-NTFSAccess` and `Remove-NTFSAudit`, which went on with a path
that didn't exist, wrote a second, misleading `RemoveAceError`, and with
`-PassThru` stopped with a `NullReferenceException`
- Fix `-PassThru` of `Enable-NTFSAccessInheritance`,
`Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`,
`Disable-NTFSAuditInheritance`, and `Set-NTFSInheritance`, which returned
the unchanged state of an item also when the change failed, so that the
inheritance looked disabled
([#74](https://github.com/raandree/NTFSSecurity/issues/74))
[Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD

2
Docs/Cmdlets/Disable-NTFSAccessInheritance.md

@ -170,6 +170,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti
Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
## RELATED LINKS
[Enable-NTFSAccessInheritance](Enable-NTFSAccessInheritance.md)

2
Docs/Cmdlets/Disable-NTFSAuditInheritance.md

@ -172,6 +172,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti
Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
## RELATED LINKS
[Enable-NTFSAuditInheritance](Enable-NTFSAuditInheritance.md)

2
Docs/Cmdlets/Enable-NTFSAccessInheritance.md

@ -169,6 +169,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti
Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
## RELATED LINKS
[Disable-NTFSAccessInheritance](Disable-NTFSAccessInheritance.md)

2
Docs/Cmdlets/Enable-NTFSAuditInheritance.md

@ -171,6 +171,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti
Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
## RELATED LINKS
[Disable-NTFSAuditInheritance](Disable-NTFSAuditInheritance.md)

2
Docs/Cmdlets/Set-NTFSInheritance.md

@ -196,6 +196,8 @@ Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled`
Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
## RELATED LINKS
[Get-NTFSInheritance](Get-NTFSInheritance.md)

9
NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs

@ -101,12 +101,11 @@ namespace NTFSSecurity
WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path));
continue;
}
finally
// Only after a successful change, so that a failure doesn't report the unchanged state
if (passThru)
{
if (passThru)
{
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
}
}

9
NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs

@ -102,12 +102,11 @@ namespace NTFSSecurity
WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path));
continue;
}
finally
// Only after a successful change, so that a failure doesn't report the unchanged state
if (passThru)
{
if (passThru)
{
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
}
}

9
NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs

@ -101,12 +101,11 @@ namespace NTFSSecurity
WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path));
continue;
}
finally
// Only after a successful change, so that a failure doesn't report the unchanged state
if (passThru)
{
if (passThru)
{
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
}
}

9
NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs

@ -101,12 +101,11 @@ namespace NTFSSecurity
WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path));
continue;
}
finally
// Only after a successful change, so that a failure doesn't report the unchanged state
if (passThru)
{
if (passThru)
{
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
}
}

9
NTFSSecurity/InheritanceCmdlets/SetInheritance.cs

@ -109,12 +109,11 @@ namespace NTFSSecurity
WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path));
continue;
}
finally
// Only after a successful change, so that a failure doesn't report the unchanged state
if (passThru)
{
if (passThru)
{
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item));
}
}
}

5
NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml

@ -2413,6 +2413,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:para>Blocking access inheritance requires permission to change the DACL of the item, which the owner of an item always has. If the descriptor cannot be opened, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; otherwise the cmdlet writes an error and continues with the next item.</maml:para>
<maml:para>A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths.</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>
@ -2658,6 +2659,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:para>If the descriptor cannot be opened because the account has no permission to the item, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; a missing Security privilege is not an access problem and is not repaired by it.</maml:para>
<maml:para>A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths.</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>
@ -3028,6 +3030,7 @@ PS C:\&gt; Get-Privileges | Where-Object { $_.Privilege -in 'Backup', 'Restore',
<maml:para>Restoring access inheritance requires permission to change the DACL of the item, which the owner of an item always has. If the descriptor cannot be opened, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; otherwise the cmdlet writes an error and continues with the next item.</maml:para>
<maml:para>A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths.</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>
@ -3273,6 +3276,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:para>If the descriptor cannot be opened because the account has no permission to the item, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; a missing Security privilege is not an access problem and is not repaired by it.</maml:para>
<maml:para>A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths.</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>
@ -9518,6 +9522,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:para>A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths.</maml:para>
<maml:para>Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` could fail with the error "Nullable object must have a value".</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>

40
Tests/Inheritance.Tests.ps1

@ -53,6 +53,46 @@ Describe 'Get-NTFSInheritance' {
}
}
Describe 'Inheritance cmdlets with -PassThru' {
BeforeDiscovery {
# With the Backup privilege, Windows may grant reading the security descriptor despite a deny entry.
$canBypassDeny = Test-PrivilegeHeld -Name 'SeBackupPrivilege'
}
# Before 5.0.0, the cmdlets wrote the -PassThru object in a finally block, also after a failure (#74).
It '<_> should return nothing when the audit change fails' -Skip:$canChangeAudit -ForEach @(
'Enable-NTFSAuditInheritance', 'Disable-NTFSAuditInheritance'
) {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'PassThru'
$result = @(& $_ -Path $file -PassThru -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue)
$inheritanceErrors | Should -Not -BeNullOrEmpty
$result | Should -BeNullOrEmpty
}
It 'Set-NTFSInheritance should return nothing when the audit change fails' -Skip:$canChangeAudit {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'PassThru'
$result = @(Set-NTFSInheritance -Path $file -AuditInheritanceEnabled $false -PassThru -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue)
$inheritanceErrors | Should -Not -BeNullOrEmpty
$result | Should -BeNullOrEmpty
}
It '<_> should write an error and return nothing when the item cannot be read' -Skip:$canBypassDeny -ForEach @(
'Enable-NTFSAccessInheritance', 'Disable-NTFSAccessInheritance'
) {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'Denied'
Block-TestReadPermission -Sandbox $sandbox -Path $file
$result = @(& $_ -Path $file -PassThru -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue)
$inheritanceErrors | Should -Not -BeNullOrEmpty
$result | Should -BeNullOrEmpty
}
}
Describe 'Set-NTFSInheritance' {
Context 'When -AccessInheritanceEnabled or -AuditInheritanceEnabled is omitted' {
BeforeEach {

Loading…
Cancel
Save