diff --git a/CHANGELOG.md b/CHANGELOG.md index b4146ee..75c19be 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Docs/Cmdlets/Disable-NTFSAccessInheritance.md b/Docs/Cmdlets/Disable-NTFSAccessInheritance.md index 4166f56..fc19225 100644 --- a/Docs/Cmdlets/Disable-NTFSAccessInheritance.md +++ b/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) diff --git a/Docs/Cmdlets/Disable-NTFSAuditInheritance.md b/Docs/Cmdlets/Disable-NTFSAuditInheritance.md index 3e08779..18ad002 100644 --- a/Docs/Cmdlets/Disable-NTFSAuditInheritance.md +++ b/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) diff --git a/Docs/Cmdlets/Enable-NTFSAccessInheritance.md b/Docs/Cmdlets/Enable-NTFSAccessInheritance.md index dc029fc..0a8b6da 100644 --- a/Docs/Cmdlets/Enable-NTFSAccessInheritance.md +++ b/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) diff --git a/Docs/Cmdlets/Enable-NTFSAuditInheritance.md b/Docs/Cmdlets/Enable-NTFSAuditInheritance.md index f7c7704..4765a1b 100644 --- a/Docs/Cmdlets/Enable-NTFSAuditInheritance.md +++ b/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) diff --git a/Docs/Cmdlets/Set-NTFSInheritance.md b/Docs/Cmdlets/Set-NTFSInheritance.md index f9986a8..34c141f 100644 --- a/Docs/Cmdlets/Set-NTFSInheritance.md +++ b/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) diff --git a/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs b/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs index d196fb7..c5f301c 100644 --- a/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs +++ b/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)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs b/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs index ac603cf..f8dbcef 100644 --- a/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs +++ b/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)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs b/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs index 672b98f..de3b55a 100644 --- a/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs +++ b/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)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs b/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs index 3f29546..943234b 100644 --- a/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs +++ b/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)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs index 3877e43..1a07fa7 100644 --- a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs +++ b/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)); } } } diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 1958dd4..a5b974e 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -2413,6 +2413,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd 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. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. 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. @@ -2658,6 +2659,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd 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. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. 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. @@ -3028,6 +3030,7 @@ PS C:\> Get-Privileges | Where-Object { $_.Privilege -in 'Backup', 'Restore', 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. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. 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. @@ -3273,6 +3276,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd 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. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. 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. @@ -9518,6 +9522,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` could fail with the error "Nullable object must have a value". 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. diff --git a/Tests/Inheritance.Tests.ps1 b/Tests/Inheritance.Tests.ps1 index ddcde8e..e809462 100644 --- a/Tests/Inheritance.Tests.ps1 +++ b/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 {