Browse Source

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 <ai@example.com>
pull/104/head
Raimund Andree 7 days ago
parent
commit
e2ea2e414d
  1. 2
      .memory-bank/progress.md
  2. 15
      CHANGELOG.md
  3. 2
      Docs/Cmdlets/Set-NTFSInheritance.md
  4. 5
      NTFSSecurity/MiscCmdlets/GetFileHash2.cs
  5. 2
      NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml
  6. 2
      Tests/Access.Tests.ps1
  7. 13
      Tests/FileHash.Tests.ps1
  8. 3
      Tests/Inheritance.Tests.ps1
  9. 2
      Tests/OutputTypes.Tests.ps1

2
.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.

15
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

2
Docs/Cmdlets/Set-NTFSInheritance.md

@ -29,7 +29,7 @@ Set-NTFSInheritance [-SecurityDescriptor] <FileSystemSecurity2[]> [-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.

5
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)
{

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

@ -9334,7 +9334,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
</command:details>
<maml:description>
<maml:para>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.</maml:para>
<maml:para>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.</maml:para>
<maml:para>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.</maml:para>
<maml:para>Omit `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` to leave that section unchanged. Changing the audit section requires the Security privilege and therefore an elevated session.</maml:para>
<maml:para>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.</maml:para>
</maml:description>

2
Tests/Access.Tests.ps1

@ -301,4 +301,4 @@ Describe 'Clear-NTFSAccess' {
$acl.Access | Should -BeNullOrEmpty
}
}
}
}

13
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*'
}

3
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
}
}
}

2
Tests/OutputTypes.Tests.ps1

@ -99,4 +99,4 @@ Describe 'Cmdlet classes' {
(Get-Command -Name 'Remove-Item2').ImplementingType.GetField('filter', $flags) | Should -BeNullOrEmpty
}
}
}

Loading…
Cancel
Save