From e2ea2e414ded3e25e3c3d0d656881f81b179fd8e Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 02:52:20 +0200 Subject: [PATCH] fix: address the review of the group E decisions Mark the Set-NTFSInheritance change as breaking and warn that scripts that used it to drop the inherited access entries now leave broader access in place. Report any failure to create the hash algorithm as HashAlgorithmNotAvailable, assert that error ID, check that the MACTripleDES warning appears once, and guard the descriptor test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- .memory-bank/progress.md | 2 ++ CHANGELOG.md | 15 +++++++++------ Docs/Cmdlets/Set-NTFSInheritance.md | 2 +- NTFSSecurity/MiscCmdlets/GetFileHash2.cs | 5 +++++ NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 2 +- Tests/Access.Tests.ps1 | 2 +- Tests/FileHash.Tests.ps1 | 13 ++++++++----- Tests/Inheritance.Tests.ps1 | 3 ++- Tests/OutputTypes.Tests.ps1 | 2 +- 9 files changed, 30 insertions(+), 16 deletions(-) diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index dc0b932..5c15c67 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -177,3 +177,5 @@ Numbered as agreed with the maintainer; each is documented on its page. - D5: `Get-FileHash2` works in PowerShell 7; `RIPEMD160` and `MACTripleDES` stop it there with `HashAlgorithmNotAvailable`. `MACTripleDES` uses a random key (verified), so it is deprecated. +- Review of group E: the changelog now marks the `Set-NTFSInheritance` + change as breaking and warns that it leaves broader access in place. diff --git a/CHANGELOG.md b/CHANGELOG.md index ba8dd57..3f4c0b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,12 +40,15 @@ The format is based on `-RemoveInheritedAuditRules` and `-RemoveExplicitAccessRules` of `Enable-NTFSAuditInheritance` to `-RemoveExplicitAuditRules`, because they act on audit entries; the old names still work as aliases -- `Set-NTFSInheritance` keeps entries like the dedicated cmdlets: - `-AccessInheritanceEnabled $false` now copies the inherited access entries - into the DACL instead of removing them, and - `-AuditInheritanceEnabled $true` now keeps the explicit audit entries. To - remove them, use `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` - or `Enable-NTFSAuditInheritance -RemoveExplicitAuditRules` +- **Breaking:** `Set-NTFSInheritance` keeps entries like the dedicated + cmdlets: `-AccessInheritanceEnabled $false` now copies the inherited access + entries into the DACL instead of removing them, and + `-AuditInheritanceEnabled $true` now keeps the explicit audit entries. A + script that used `-AccessInheritanceEnabled $false` to drop the inherited + access entries now leaves them in place, which grants broader access than + before. To remove the entries, use + `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` or + `Enable-NTFSAuditInheritance -RemoveExplicitAuditRules` ### Deprecated diff --git a/Docs/Cmdlets/Set-NTFSInheritance.md b/Docs/Cmdlets/Set-NTFSInheritance.md index 855d9c2..e3667c6 100644 --- a/Docs/Cmdlets/Set-NTFSInheritance.md +++ b/Docs/Cmdlets/Set-NTFSInheritance.md @@ -29,7 +29,7 @@ Set-NTFSInheritance [-SecurityDescriptor] [-AccessInheri The `Set-NTFSInheritance` cmdlet turns the inheritance of access rules and audit rules on or off in a single call. It reads the current state of the item first and changes a section only when the requested value differs from the current one, which makes the cmdlet suitable for repeatedly applying a desired state to a folder tree. -The cmdlet performs the same operations as `Enable-NTFSAccessInheritance`, `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, and `Disable-NTFSAuditInheritance`, but it does not expose their switches; it uses their defaults instead. `-AccessInheritanceEnabled $false` copies the inherited access rules into the item's own DACL, `-AccessInheritanceEnabled $true` keeps the explicit access rules, `-AuditInheritanceEnabled $false` copies the inherited audit rules into the item's own SACL, and `-AuditInheritanceEnabled $true` keeps the explicit audit rules. To remove the rules instead, use `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` or `Enable-NTFSAuditInheritance -RemoveExplicitAuditRules`. Before 5.0.0, `-AccessInheritanceEnabled $false` discarded the inherited access rules, and `-AuditInheritanceEnabled $true` removed the explicit audit rules. +The cmdlet performs the same operations as `Enable-NTFSAccessInheritance`, `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, and `Disable-NTFSAuditInheritance`, but it does not expose their switches; it uses their defaults instead. `-AccessInheritanceEnabled $false` copies the inherited access rules into the item's own DACL, `-AccessInheritanceEnabled $true` keeps the explicit access rules, `-AuditInheritanceEnabled $false` copies the inherited audit rules into the item's own SACL, and `-AuditInheritanceEnabled $true` keeps the explicit audit rules. To remove the rules instead, use `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` or `Enable-NTFSAuditInheritance -RemoveExplicitAuditRules`. Before 5.0.0, `-AccessInheritanceEnabled $false` discarded the inherited access rules, and `-AuditInheritanceEnabled $true` removed the explicit audit rules. Review scripts that used `-AccessInheritanceEnabled $false` to drop the inherited access rules: they now keep them, which leaves broader access in place; `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` gives the old result. Omit `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` to leave that section unchanged. Changing the audit section requires the Security privilege and therefore an elevated session. diff --git a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs index ac3b7a7..113d8f0 100644 --- a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs +++ b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs @@ -48,6 +48,11 @@ namespace NTFSSecurity { ThrowTerminatingError(new ErrorRecord(ex, "HashAlgorithmNotAvailable", ErrorCategory.NotImplemented, algorithm)); } + catch (Exception ex) + { + // For example, an algorithm that a FIPS policy doesn't allow. + ThrowTerminatingError(new ErrorRecord(ex, "HashAlgorithmNotAvailable", ErrorCategory.NotImplemented, algorithm)); + } if (algorithm == HashAlgorithms.MACTripleDES && !deprecationWarningWritten) { diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 781722d..fd81345 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -9334,7 +9334,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd The `Set-NTFSInheritance` cmdlet turns the inheritance of access rules and audit rules on or off in a single call. It reads the current state of the item first and changes a section only when the requested value differs from the current one, which makes the cmdlet suitable for repeatedly applying a desired state to a folder tree. - The cmdlet performs the same operations as `Enable-NTFSAccessInheritance`, `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, and `Disable-NTFSAuditInheritance`, but it does not expose their switches; it uses their defaults instead. `-AccessInheritanceEnabled $false` copies the inherited access rules into the item's own DACL, `-AccessInheritanceEnabled $true` keeps the explicit access rules, `-AuditInheritanceEnabled $false` copies the inherited audit rules into the item's own SACL, and `-AuditInheritanceEnabled $true` keeps the explicit audit rules. To remove the rules instead, use `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` or `Enable-NTFSAuditInheritance -RemoveExplicitAuditRules`. Before 5.0.0, `-AccessInheritanceEnabled $false` discarded the inherited access rules, and `-AuditInheritanceEnabled $true` removed the explicit audit rules. + The cmdlet performs the same operations as `Enable-NTFSAccessInheritance`, `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, and `Disable-NTFSAuditInheritance`, but it does not expose their switches; it uses their defaults instead. `-AccessInheritanceEnabled $false` copies the inherited access rules into the item's own DACL, `-AccessInheritanceEnabled $true` keeps the explicit access rules, `-AuditInheritanceEnabled $false` copies the inherited audit rules into the item's own SACL, and `-AuditInheritanceEnabled $true` keeps the explicit audit rules. To remove the rules instead, use `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` or `Enable-NTFSAuditInheritance -RemoveExplicitAuditRules`. Before 5.0.0, `-AccessInheritanceEnabled $false` discarded the inherited access rules, and `-AuditInheritanceEnabled $true` removed the explicit audit rules. Review scripts that used `-AccessInheritanceEnabled $false` to drop the inherited access rules: they now keep them, which leaves broader access in place; `Disable-NTFSAccessInheritance -RemoveInheritedAccessRules` gives the old result. Omit `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` to leave that section unchanged. Changing the audit section requires the Security privilege and therefore an elevated session. In the `Path` parameter set the cmdlet writes each changed section back to disk immediately. In the `SecurityDescriptor` parameter set it changes the `Security2.FileSystemSecurity2` object in memory only; nothing reaches the file system until you pass that object to `Set-NTFSSecurityDescriptor`. `-Path`, `-AccessInheritanceEnabled`, and `-AuditInheritanceEnabled` all accept pipeline input by property name, so a `Security2.FileSystemInheritanceInfo` object from `Get-NTFSInheritance` binds to all three at once. diff --git a/Tests/Access.Tests.ps1 b/Tests/Access.Tests.ps1 index b423e0f..5582a81 100644 --- a/Tests/Access.Tests.ps1 +++ b/Tests/Access.Tests.ps1 @@ -301,4 +301,4 @@ Describe 'Clear-NTFSAccess' { $acl.Access | Should -BeNullOrEmpty } } -} \ No newline at end of file +} diff --git a/Tests/FileHash.Tests.ps1 b/Tests/FileHash.Tests.ps1 index 271eb71..221c7b8 100644 --- a/Tests/FileHash.Tests.ps1 +++ b/Tests/FileHash.Tests.ps1 @@ -55,14 +55,17 @@ Describe 'Get-FileHash2' { It 'Should stop with an error that names <_> in PowerShell 7' -Skip:(-not $isCore) -ForEach @('RIPEMD160', 'MACTripleDES') { $algorithm = $_ - { Get-FileHash2 -Path $first -Algorithm $algorithm -ErrorAction Stop } | - Should -Throw -ExpectedMessage "*'$algorithm'*Windows PowerShell 5.1*" + $hashError = { Get-FileHash2 -Path $first -Algorithm $algorithm -ErrorAction Stop } | + Should -Throw -ExpectedMessage "*'$algorithm'*Windows PowerShell 5.1*" -PassThru + + $hashError.FullyQualifiedErrorId | Should -BeLike 'HashAlgorithmNotAvailable,*' } - It 'Should warn that MACTripleDES is deprecated' -Skip:$isCore { - $result = Get-FileHash2 -Path $first -Algorithm MACTripleDES -WarningVariable hashWarnings -WarningAction SilentlyContinue + It 'Should warn once that MACTripleDES is deprecated' -Skip:$isCore { + $results = @(Get-FileHash2 -Path $first, $second -Algorithm MACTripleDES -WarningVariable hashWarnings -WarningAction SilentlyContinue) - $result.Hash | Should -Not -BeNullOrEmpty + $results | Should -HaveCount 2 + $results[0].Hash | Should -Not -BeNullOrEmpty $hashWarnings | Should -HaveCount 1 $hashWarnings[0].Message | Should -BeLike '*MACTripleDES*random key*deprecated*' } diff --git a/Tests/Inheritance.Tests.ps1 b/Tests/Inheritance.Tests.ps1 index b0fbc7b..19afd2c 100644 --- a/Tests/Inheritance.Tests.ps1 +++ b/Tests/Inheritance.Tests.ps1 @@ -126,6 +126,7 @@ Describe 'Set-NTFSInheritance' { # In memory, the kept entries stay marked as inherited; Windows stores them as explicit ones on write. It 'Should keep the inherited access entries of a security descriptor' { $file = New-TestSandboxItem -Sandbox $sandbox -Name 'KeepDescriptor' + Assert-TestSandboxPath -Sandbox $sandbox -Path $file $sd = Get-NTFSSecurityDescriptor -Path $file $sidType = [System.Security.Principal.SecurityIdentifier] $inheritedCount = @($sd.SecurityDescriptor.GetAccessRules($false, $true, $sidType)).Count @@ -214,4 +215,4 @@ Describe 'Audit inheritance switches' { { & $Command @parameters -ErrorAction SilentlyContinue } | Should -Not -Throw } -} \ No newline at end of file +} diff --git a/Tests/OutputTypes.Tests.ps1 b/Tests/OutputTypes.Tests.ps1 index c767c41..7708b67 100644 --- a/Tests/OutputTypes.Tests.ps1 +++ b/Tests/OutputTypes.Tests.ps1 @@ -99,4 +99,4 @@ Describe 'Cmdlet classes' { (Get-Command -Name 'Remove-Item2').ImplementingType.GetField('filter', $flags) | Should -BeNullOrEmpty } -} \ No newline at end of file +}