From 2ae7e32a5c8fccb155e30a72947b8b65951aa2ce Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Sun, 4 Oct 2026 23:52:29 +0200 Subject: [PATCH 01/19] test: add sandbox, path guard, and elevation helpers for the tests Tests/TestHelpers.psm1 lets a test that changes files, links, ACLs, owners, audit entries, or inheritance work in its own sandbox below $env:TEMP\NTFSSecurity.Tests: - New-TestSandbox creates the folder; Assert-TestSandboxPath throws unless every target, relative ones resolved against the location, is inside it. - Remove-TestSandbox removes the links first without following them (Windows PowerShell 5.1 follows directory links when it removes a folder), resets ACL changes, and deletes the folder. - Test-IsElevated and Test-PrivilegeHeld decide which tests can run; CI runners are elevated, the workstation isn't. Tests/TestHelpers.Tests.ps1 (14 tests) covers the helpers. CI runs every *.Tests.ps1 file in Tests, so new test files need no workflow change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- Tests/TestHelpers.Tests.ps1 | 119 ++++++++++++++++++++++++++ Tests/TestHelpers.psm1 | 163 ++++++++++++++++++++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 Tests/TestHelpers.Tests.ps1 create mode 100644 Tests/TestHelpers.psm1 diff --git a/Tests/TestHelpers.Tests.ps1 b/Tests/TestHelpers.Tests.ps1 new file mode 100644 index 0000000..d850384 --- /dev/null +++ b/Tests/TestHelpers.Tests.ps1 @@ -0,0 +1,119 @@ +<# + Tests the shared helpers in TestHelpers.psm1, which keep the tests that change files, links, and security + descriptors inside their sandbox folders. CI runs every *.Tests.ps1 file of this folder. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force +} + +AfterAll { + Remove-Module -Name TestHelpers -Force -ErrorAction SilentlyContinue +} + +Describe 'Test helpers' { + Context 'New-TestSandbox' { + BeforeAll { + $sandbox = New-TestSandbox -Name 'Helpers' + } + + AfterAll { + Remove-TestSandbox -Sandbox $sandbox + } + + It 'Should create an empty folder below $env:TEMP\NTFSSecurity.Tests' { + $sandbox | Should -Exist + Get-ChildItem -LiteralPath $sandbox -Force | Should -BeNullOrEmpty + $expectedParent = [IO.Path]::GetFullPath((Join-Path -Path ([IO.Path]::GetTempPath()) -ChildPath 'NTFSSecurity.Tests')) + Split-Path -Path $sandbox -Parent | Should -Be $expectedParent + Split-Path -Path $sandbox -Leaf | Should -BeLike 'Helpers-*' + } + } + + Context 'Assert-TestSandboxPath' { + BeforeAll { + $sandbox = New-TestSandbox -Name 'Helpers' + Push-Location -LiteralPath $sandbox + } + + AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + } + + It 'Should accept a full path inside the sandbox' { + { Assert-TestSandboxPath -Sandbox $sandbox -Path (Join-Path -Path $sandbox -ChildPath 'Folder\File.txt') } | + Should -Not -Throw + } + + It 'Should resolve a relative path against the current location' { + { Assert-TestSandboxPath -Sandbox $sandbox -Path '.\File.txt', 'Folder\File.txt' } | Should -Not -Throw + } + + It 'Should reject <_>' -ForEach @('..', '..\Other', 'C:\Windows', '\Windows') { + { Assert-TestSandboxPath -Sandbox $sandbox -Path $_ } | Should -Throw -ExpectedMessage 'Refusing to change*' + } + + It 'Should reject a sibling folder whose name starts with the name of the sandbox' { + { Assert-TestSandboxPath -Sandbox $sandbox -Path "$sandbox-Other\File.txt" } | + Should -Throw -ExpectedMessage 'Refusing to change*' + } + + It 'Should reject a sandbox that New-TestSandbox did not create' { + { Assert-TestSandboxPath -Sandbox $env:TEMP -Path (Join-Path -Path $env:TEMP -ChildPath 'File.txt') } | + Should -Throw -ExpectedMessage '*is not a test sandbox*' + } + } + + Context 'Remove-TestSandbox' { + BeforeAll { + $sandbox = New-TestSandbox -Name 'Helpers' + $otherSandbox = New-TestSandbox -Name 'Helpers' + $target = Join-Path -Path $otherSandbox -ChildPath 'Target' + $link = Join-Path -Path $sandbox -ChildPath 'Link' + $locked = Join-Path -Path $sandbox -ChildPath 'Locked' + + Assert-TestSandboxPath -Sandbox $otherSandbox -Path $target + New-Item -ItemType Directory -Path $target | Out-Null + Set-Content -LiteralPath (Join-Path -Path $target -ChildPath 'Keep.txt') -Value 'Keep' + Assert-TestSandboxPath -Sandbox $sandbox -Path $link, $locked + New-Item -ItemType Junction -Path $link -Value $target | Out-Null + New-Item -ItemType Directory -Path $locked | Out-Null + Set-Content -LiteralPath (Join-Path -Path $locked -ChildPath 'File.txt') -Value 'Locked' + # An empty, protected DACL that grants nobody access, as some tests leave behind + & icacls.exe $locked /inheritance:r *> $null + + Remove-TestSandbox -Sandbox $sandbox + } + + AfterAll { + Remove-TestSandbox -Sandbox $otherSandbox + } + + It 'Should remove the sandbox, even with a folder that denies access' { + $sandbox | Should -Not -Exist + } + + It 'Should remove a junction without removing the files of its target' { + Join-Path -Path $target -ChildPath 'Keep.txt' | Should -Exist + } + } + + Context 'Test-IsElevated and Test-PrivilegeHeld' { + It 'Should tell whether the process is elevated' { + Test-IsElevated | Should -BeOfType [bool] + } + + It 'Should find SeChangeNotifyPrivilege, which every access token holds' { + Test-PrivilegeHeld -Name 'SeChangeNotifyPrivilege' | Should -BeTrue + } + + It 'Should not find a privilege that does not exist' { + Test-PrivilegeHeld -Name 'SeNoSuchPrivilege' | Should -BeFalse + } + } +} diff --git a/Tests/TestHelpers.psm1 b/Tests/TestHelpers.psm1 new file mode 100644 index 0000000..3ac1f64 --- /dev/null +++ b/Tests/TestHelpers.psm1 @@ -0,0 +1,163 @@ +<# + Shared helpers for the Pester tests. A test that changes files, links, ACLs, owners, audit entries, or + inheritance works in its own sandbox folder below $env:TEMP\NTFSSecurity.Tests: it creates the sandbox with + New-TestSandbox, sets the location to it, checks every target with Assert-TestSandboxPath before the change, + and removes the sandbox with Remove-TestSandbox. +#> + +$script:sandboxRoot = Join-Path -Path ([IO.Path]::GetTempPath()) -ChildPath 'NTFSSecurity.Tests' + +function New-TestSandbox { + <# + .SYNOPSIS + Creates an empty sandbox folder for one test file or block and returns its full path. + #> + [Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Test helper that only creates sandboxes.' + )] + [CmdletBinding()] + [OutputType([string])] + param ( + [ValidatePattern('^[\w-]+$')] + [string] + $Name = 'Sandbox' + ) + + $path = Join-Path -Path $script:sandboxRoot -ChildPath ('{0}-{1}' -f $Name, [guid]::NewGuid().ToString('N').Substring(0, 8)) + (New-Item -ItemType Directory -Path $path -Force).FullName +} + +function Assert-TestSandboxPath { + <# + .SYNOPSIS + Throws unless each path is inside the given sandbox, which itself must be a sandbox of New-TestSandbox. + Relative paths resolve against the current location. + #> + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [string] + $Sandbox, + + [Parameter(Mandatory, ValueFromPipeline)] + [string[]] + $Path + ) + + begin { + $sandboxFullName = [IO.Path]::GetFullPath($Sandbox).TrimEnd('\') + $rootFullName = [IO.Path]::GetFullPath($script:sandboxRoot).TrimEnd('\') + '\' + if (-not $sandboxFullName.StartsWith($rootFullName, [StringComparison]::OrdinalIgnoreCase) -or + $sandboxFullName.Length -le $rootFullName.Length) { + throw "'$Sandbox' is not a test sandbox below '$rootFullName'." + } + $sandboxPrefix = $sandboxFullName + '\' + } + + process { + foreach ($item in $Path) { + $location = (Get-Location -PSProvider FileSystem).ProviderPath + $fullName = [IO.Path]::GetFullPath([IO.Path]::Combine($location, $item)).TrimEnd('\') + if (-not ($fullName + '\').StartsWith($sandboxPrefix, [StringComparison]::OrdinalIgnoreCase)) { + throw "Refusing to change '$fullName', which is outside the test sandbox '$sandboxFullName'." + } + } + } +} + +function Remove-TestSandbox { + <# + .SYNOPSIS + Removes a sandbox of New-TestSandbox: first its links, without following them, then its ACL changes, then + the folder. + #> + [Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Test helper that only removes sandboxes.' + )] + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [string] + $Sandbox + ) + + Assert-TestSandboxPath -Sandbox $Sandbox -Path $Sandbox + if (-not (Test-Path -LiteralPath $Sandbox)) { + return + } + + # Windows PowerShell 5.1 follows directory links when it removes a folder recursively, so the links go first. + & icacls.exe $Sandbox /reset /T /C /Q *> $null + $pending = New-Object -TypeName 'System.Collections.Generic.Stack[string]' + $pending.Push($Sandbox) + while ($pending.Count -gt 0) { + foreach ($entry in [IO.Directory]::GetFileSystemEntries($pending.Pop())) { + $attributes = [IO.File]::GetAttributes($entry) + if ($attributes -band [IO.FileAttributes]::ReparsePoint) { + if ($attributes -band [IO.FileAttributes]::Directory) { + [IO.Directory]::Delete($entry, $false) + } + else { + [IO.File]::Delete($entry) + } + } + elseif ($attributes -band [IO.FileAttributes]::Directory) { + $pending.Push($entry) + } + } + } + & icacls.exe $Sandbox /reset /T /C /Q *> $null + Get-ChildItem -LiteralPath $Sandbox -Recurse -Force | ForEach-Object -Process { + $_.Attributes = [IO.FileAttributes]::Normal + } + Remove-Item -LiteralPath $Sandbox -Recurse -Force + try { + # Fails while another sandbox exists, also one of a test run in parallel + [IO.Directory]::Delete($script:sandboxRoot, $false) + } + catch { + Write-Verbose -Message "Keeping '$script:sandboxRoot': $($_.Exception.Message)" + } +} + +function Test-IsElevated { + <# + .SYNOPSIS + Returns $true when the process runs elevated as an administrator. + #> + [CmdletBinding()] + [OutputType([bool])] + param () + + $principal = New-Object -TypeName 'Security.Principal.WindowsPrincipal' -ArgumentList ( + [Security.Principal.WindowsIdentity]::GetCurrent() + ) + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) +} + +function Test-PrivilegeHeld { + <# + .SYNOPSIS + Returns $true when the access token of the process holds the privilege, enabled or not. + .PARAMETER Name + The privilege name, such as SeSecurityPrivilege or SeBackupPrivilege. + #> + [CmdletBinding()] + [OutputType([bool])] + param ( + [Parameter(Mandatory)] + [ValidatePattern('^Se\w+Privilege$')] + [string] + $Name + ) + + # The column names are localized; the privilege names in the first column are not. + $output = & whoami.exe /priv /fo csv 2>$null + if ($LASTEXITCODE -ne 0) { + return $false + } + [bool] ($output | Select-Object -Skip 1 | ConvertFrom-Csv -Header 'Name', 'Description', 'State' | + Where-Object -Property Name -EQ -Value $Name) +} + +Export-ModuleMember -Function New-TestSandbox, Assert-TestSandboxPath, Remove-TestSandbox, Test-IsElevated, Test-PrivilegeHeld From 3a61680a7886053e70dbf00fc650028dbda3ac80 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Sun, 4 Oct 2026 23:59:23 +0200 Subject: [PATCH 02/19] fix: leave a section unchanged when Set-NTFSInheritance omits its value Defect 1. Set-NTFSInheritance compared the current state with an unset Nullable and then read its value, so omitting -AccessInheritanceEnabled always failed with "Nullable object must have a value", and omitting -AuditInheritanceEnabled failed wherever the audit section is readable. In the SecurityDescriptor set the error was terminating. An omitted parameter now leaves its section unchanged. The item, retry, and security descriptor paths share one implementation instead of three copies. Tests/Inheritance.Tests.ps1 (new): 4 tests; the audit case needs the Security privilege and runs in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 3 + Docs/Cmdlets/Set-NTFSInheritance.md | 4 +- .../InheritanceCmdlets/SetInheritance.cs | 193 ++++++++---------- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 3 +- Tests/Inheritance.Tests.ps1 | 77 +++++++ 5 files changed, 173 insertions(+), 107 deletions(-) create mode 100644 Tests/Inheritance.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 085bcb2..e75f892 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,5 +52,8 @@ The format is based on - Remove `Show-NTFSSimpleAccess`, which no longer exists, and duplicate entries from the cmdlets that the module manifest exports and the PowerShell Gallery lists +- Fix `Set-NTFSInheritance`, which failed with "Nullable object must have a + value" when `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` was + omitted; an omitted parameter now leaves its section unchanged [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Set-NTFSInheritance.md b/Docs/Cmdlets/Set-NTFSInheritance.md index 4297cf6..6eb05e6 100644 --- a/Docs/Cmdlets/Set-NTFSInheritance.md +++ b/Docs/Cmdlets/Set-NTFSInheritance.md @@ -31,7 +31,7 @@ The `Set-NTFSInheritance` cmdlet turns the inheritance of access rules and audit The cmdlet performs the same operations as `Enable-NTFSAccessInheritance`, `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, and `Disable-NTFSAuditInheritance`, but it does not expose their switches and it does not use their defaults. `-AccessInheritanceEnabled $false` discards the inherited access rules instead of copying them 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` removes the explicit audit rules. Use the individual Enable and Disable cmdlets when you need the opposite behavior. -Specify both `-AccessInheritanceEnabled` and `-AuditInheritanceEnabled`. The cmdlet compares the current state against the parameter value even when the parameter was not supplied, and when such a comparison reports a difference it fails with the non-terminating error "Nullable object must have a value". Omitting `-AccessInheritanceEnabled` always triggers that error, and omitting `-AuditInheritanceEnabled` triggers it in a session that can read the audit section. Changing the audit section requires the Security privilege and therefore an elevated session. +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. @@ -192,6 +192,8 @@ If the descriptor cannot be opened because the account has no permission to the 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". + ## RELATED LINKS [Get-NTFSInheritance](Get-NTFSInheritance.md) diff --git a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs index 109a2e2..a8effe7 100644 --- a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs @@ -84,41 +84,7 @@ namespace NTFSSecurity try { - var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); - - if (currentState.AccessInheritanceEnabled != accessInheritanceEnabled) - { - WriteVerbose("AccessInheritanceEnabled not equal"); - if (accessInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAccessInheritance"); - FileSystemInheritanceInfo.EnableAccessInheritance(item, false); - } - else - { - WriteVerbose("Calling DisableAccessInheritance"); - FileSystemInheritanceInfo.DisableAccessInheritance(item, true); - } - } - else - WriteVerbose("AccessInheritanceEnabled is equal - no change was done"); - - if (currentState.AuditInheritanceEnabled != auditInheritanceEnabled) - { - WriteVerbose("AuditInheritanceEnabled not equal"); - if (auditInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAuditInheritance"); - FileSystemInheritanceInfo.EnableAuditInheritance(item, true); - } - else - { - WriteVerbose("Calling DisableAuditInheritance"); - FileSystemInheritanceInfo.DisableAuditInheritance(item, false); - } - } - else - WriteVerbose("AuditInheritanceEnabled is equal - no change was done"); + SetInheritanceState(item); } catch (UnauthorizedAccessException) { @@ -129,41 +95,7 @@ namespace NTFSSecurity FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); - - if (currentState.AccessInheritanceEnabled != accessInheritanceEnabled) - { - WriteVerbose("AccessInheritanceEnabled not equal"); - if (accessInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAccessInheritance"); - FileSystemInheritanceInfo.EnableAccessInheritance(item, false); - } - else - { - WriteVerbose("Calling DisableAccessInheritance"); - FileSystemInheritanceInfo.DisableAccessInheritance(item, true); - } - } - else - WriteVerbose("AccessInheritanceEnabled is equal - no change was done"); - - if (currentState.AuditInheritanceEnabled != auditInheritanceEnabled) - { - WriteVerbose("AuditInheritanceEnabled not equal"); - if (auditInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAuditInheritance"); - FileSystemInheritanceInfo.EnableAuditInheritance(item, true); - } - else - { - WriteVerbose("Calling DisableAuditInheritance"); - FileSystemInheritanceInfo.DisableAuditInheritance(item, false); - } - } - else - WriteVerbose("AuditInheritanceEnabled is equal - no change was done"); + SetInheritanceState(item); FileSystemOwner.SetOwner(item, previousOwner); } @@ -191,41 +123,7 @@ namespace NTFSSecurity { foreach (var sd in securityDescriptors) { - var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(sd); - - if (currentState.AccessInheritanceEnabled != accessInheritanceEnabled) - { - WriteVerbose("AccessInheritanceEnabled not equal"); - if (accessInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAccessInheritance"); - FileSystemInheritanceInfo.EnableAccessInheritance(sd, false); - } - else - { - WriteVerbose("Calling DisableAccessInheritance"); - FileSystemInheritanceInfo.DisableAccessInheritance(sd, true); - } - } - else - WriteVerbose("AccessInheritanceEnabled is equal - no change was done"); - - if (currentState.AuditInheritanceEnabled != auditInheritanceEnabled) - { - WriteVerbose("AuditInheritanceEnabled not equal"); - if (auditInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAuditInheritance"); - FileSystemInheritanceInfo.EnableAuditInheritance(sd, true); - } - else - { - WriteVerbose("Calling DisableAuditInheritance"); - FileSystemInheritanceInfo.DisableAuditInheritance(sd, false); - } - } - else - WriteVerbose("AuditInheritanceEnabled is equal - no change was done"); + SetInheritanceState(sd); if (passThru) { @@ -234,5 +132,90 @@ namespace NTFSSecurity } } } + + private void SetInheritanceState(FileSystemInfo item) + { + var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); + + if (IsChangeRequested("AccessInheritanceEnabled", accessInheritanceEnabled, currentState.AccessInheritanceEnabled)) + { + if (accessInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAccessInheritance"); + FileSystemInheritanceInfo.EnableAccessInheritance(item, false); + } + else + { + WriteVerbose("Calling DisableAccessInheritance"); + FileSystemInheritanceInfo.DisableAccessInheritance(item, true); + } + } + + if (IsChangeRequested("AuditInheritanceEnabled", auditInheritanceEnabled, currentState.AuditInheritanceEnabled)) + { + if (auditInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAuditInheritance"); + FileSystemInheritanceInfo.EnableAuditInheritance(item, true); + } + else + { + WriteVerbose("Calling DisableAuditInheritance"); + FileSystemInheritanceInfo.DisableAuditInheritance(item, false); + } + } + } + + private void SetInheritanceState(FileSystemSecurity2 sd) + { + var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(sd); + + if (IsChangeRequested("AccessInheritanceEnabled", accessInheritanceEnabled, currentState.AccessInheritanceEnabled)) + { + if (accessInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAccessInheritance"); + FileSystemInheritanceInfo.EnableAccessInheritance(sd, false); + } + else + { + WriteVerbose("Calling DisableAccessInheritance"); + FileSystemInheritanceInfo.DisableAccessInheritance(sd, true); + } + } + + if (IsChangeRequested("AuditInheritanceEnabled", auditInheritanceEnabled, currentState.AuditInheritanceEnabled)) + { + if (auditInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAuditInheritance"); + FileSystemInheritanceInfo.EnableAuditInheritance(sd, true); + } + else + { + WriteVerbose("Calling DisableAuditInheritance"); + FileSystemInheritanceInfo.DisableAuditInheritance(sd, false); + } + } + } + + // An omitted parameter leaves its section unchanged. + private bool IsChangeRequested(string parameterName, bool? requestedState, bool? currentState) + { + if (!requestedState.HasValue) + { + WriteVerbose(string.Format("{0} not specified - no change was done", parameterName)); + return false; + } + + if (currentState == requestedState) + { + WriteVerbose(string.Format("{0} is equal - no change was done", parameterName)); + return false; + } + + WriteVerbose(string.Format("{0} not equal", parameterName)); + return true; + } } } \ No newline at end of file diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 039bfe6..e59d3a7 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -9169,7 +9169,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 and it does not use their defaults. `-AccessInheritanceEnabled $false` discards the inherited access rules instead of copying them 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` removes the explicit audit rules. Use the individual Enable and Disable cmdlets when you need the opposite behavior. - Specify both `-AccessInheritanceEnabled` and `-AuditInheritanceEnabled`. The cmdlet compares the current state against the parameter value even when the parameter was not supplied, and when such a comparison reports a difference it fails with the non-terminating error "Nullable object must have a value". Omitting `-AccessInheritanceEnabled` always triggers that error, and omitting `-AuditInheritanceEnabled` triggers it in a session that can read the audit section. Changing the audit section requires the Security privilege and therefore an elevated session. + 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. @@ -9382,6 +9382,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd The audit section of a security descriptor can only be read and written with the Security privilege (`SeSecurityPrivilege`), which an account can only use in an elevated session. Without it, a requested change of `-AuditInheritanceEnabled` produces a non-terminating error that reports Windows error 1314, "A required privilege is not held by the client". The access section is processed first, so a change of `-AccessInheritanceEnabled` in the same command is applied even when the audit change fails. If the descriptor cannot be opened because the account has no permission to the item, the cmdlet takes ownership of the item, applies the changes, 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, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` could fail with the error "Nullable object must have a value". diff --git a/Tests/Inheritance.Tests.ps1 b/Tests/Inheritance.Tests.ps1 new file mode 100644 index 0000000..f21b891 --- /dev/null +++ b/Tests/Inheritance.Tests.ps1 @@ -0,0 +1,77 @@ +<# + Tests the inheritance cmdlets of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. + Tests that change the audit section need the Security privilege and skip without it; CI runs them elevated. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeDiscovery { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $canChangeAudit = Test-PrivilegeHeld -Name 'SeSecurityPrivilege' +} + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'Inheritance' + Push-Location -LiteralPath $sandbox + + function New-SandboxFile { + $path = Join-Path -Path $sandbox -ChildPath ('File-{0}.txt' -f [guid]::NewGuid().ToString('N').Substring(0, 8)) + Assert-TestSandboxPath -Sandbox $sandbox -Path $path + Set-Content -LiteralPath $path -Value 'Inheritance test' + $path + } +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Set-NTFSInheritance' { + Context 'When -AccessInheritanceEnabled or -AuditInheritanceEnabled is omitted' { + BeforeEach { + $file = New-SandboxFile + Assert-TestSandboxPath -Sandbox $sandbox -Path $file + } + + It 'Should change nothing and write no error when both are omitted' { + Set-NTFSInheritance -Path $file -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue + + $inheritanceErrors | Should -BeNullOrEmpty + (Get-NTFSInheritance -Path $file).AccessInheritanceEnabled | Should -BeTrue + } + + It 'Should leave a security descriptor unchanged when both are omitted' { + $sd = Get-NTFSSecurityDescriptor -Path $file + + { Set-NTFSInheritance -SecurityDescriptor $sd -ErrorAction Stop } | Should -Not -Throw + $sd.SecurityDescriptor.AreAccessRulesProtected | Should -BeFalse + } + + It 'Should change only the access inheritance when -AuditInheritanceEnabled is omitted' { + $before = Get-NTFSInheritance -Path $file + + Set-NTFSInheritance -Path $file -AccessInheritanceEnabled $false -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue + + $inheritanceErrors | Should -BeNullOrEmpty + $after = Get-NTFSInheritance -Path $file + $after.AccessInheritanceEnabled | Should -BeFalse + $after.AuditInheritanceEnabled | Should -Be $before.AuditInheritanceEnabled + } + + It 'Should change only the audit inheritance when -AccessInheritanceEnabled is omitted' -Skip:(-not $canChangeAudit) { + Set-NTFSInheritance -Path $file -AuditInheritanceEnabled $false -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue + + $inheritanceErrors | Should -BeNullOrEmpty + $after = Get-NTFSInheritance -Path $file + $after.AccessInheritanceEnabled | Should -BeTrue + $after.AuditInheritanceEnabled | Should -BeFalse + } + } +} From d2c3d04b072cc92047aa7f7c782278c37f863554 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:06:42 +0200 Subject: [PATCH 03/19] fix: return the file when Get-ChildItem2 gets a file path Defect 2. Get-ChildItem2 cast every -Path item to DirectoryInfo, so a file path stopped the cmdlet with an InvalidCastException, a terminating error that also skipped the remaining paths. Like Get-ChildItem, a file path now returns the file itself, filtered like the other items; with -Directory it returns nothing. Tests/ItemCmdlets.Tests.ps1 (new): 3 tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 2 + Docs/Cmdlets/Get-ChildItem2.md | 4 +- NTFSSecurity/ItemCmdlets/GetChildItem2.cs | 16 ++++-- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 5 +- Tests/ItemCmdlets.Tests.ps1 | 57 ++++++++++++++++++++ 5 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 Tests/ItemCmdlets.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index e75f892..850671d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,5 +55,7 @@ The format is based on - Fix `Set-NTFSInheritance`, which failed with "Nullable object must have a value" when `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` was omitted; an omitted parameter now leaves its section unchanged +- Fix `Get-ChildItem2`, which stopped with an `InvalidCastException` when + `-Path` pointed to a file; it now returns the file, like `Get-ChildItem` [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-ChildItem2.md b/Docs/Cmdlets/Get-ChildItem2.md index 477c3ae..5ae3191 100644 --- a/Docs/Cmdlets/Get-ChildItem2.md +++ b/Docs/Cmdlets/Get-ChildItem2.md @@ -182,7 +182,7 @@ Accept wildcard characters: False ### -Path -Specifies the folders whose content you want to list. Relative paths are resolved against the current location, and wildcard characters are not supported. If you omit this parameter, the cmdlet lists the current location. A value that points to a file instead of a folder produces an error. +Specifies the folders whose content you want to list. Relative paths are resolved against the current location, and wildcard characters are not supported. If you omit this parameter, the cmdlet lists the current location. A value that points to a file returns that file, like `Get-ChildItem`, unless you use `-Directory`. ```yaml Type: String[] @@ -305,6 +305,8 @@ The `PrivateData` section of the module manifest `NTFSSecurity.psd1` contains tw A folder that cannot be read produces a non-terminating error with the ID `DirUnauthorizedAccessError` for an access denial or `DirUnspecifiedError` for any other failure, and a path that does not exist produces the error `FileNotFound`. In each case the cmdlet continues with the next path. Failures that occur while `-Recurse` collects the subfolders of a folder are reported as verbose messages only, not as errors. +Before 5.0.0, a `-Path` value that points to a file stopped the cmdlet with an `InvalidCastException`. + ## RELATED LINKS [Get-Item2](Get-Item2.md) diff --git a/NTFSSecurity/ItemCmdlets/GetChildItem2.cs b/NTFSSecurity/ItemCmdlets/GetChildItem2.cs index 788c035..96f08f1 100644 --- a/NTFSSecurity/ItemCmdlets/GetChildItem2.cs +++ b/NTFSSecurity/ItemCmdlets/GetChildItem2.cs @@ -148,12 +148,11 @@ namespace NTFSSecurity { foreach (var path in paths) { - DirectoryInfo di = null; + FileSystemInfo item = null; try { - di = (DirectoryInfo)GetFileSystemInfo2(path); - + item = GetFileSystemInfo2(path); } catch (System.IO.FileNotFoundException ex) { @@ -161,6 +160,17 @@ namespace NTFSSecurity continue; } + var di = item as DirectoryInfo; + if (di == null) + { + // Like Get-ChildItem, a file path returns the file itself. + if (!directory) + { + WriteFileSystemInfoCollection(new FileSystemInfo[] { item }.GetEnumerator()); + } + continue; + } + try { WriteFileSystem(di, 0); diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index e59d3a7..bf7b04d 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -3482,7 +3482,7 @@ PS C:\> Disable-Privileges Path - Specifies the folders whose content you want to list. Relative paths are resolved against the current location, and wildcard characters are not supported. If you omit this parameter, the cmdlet lists the current location. A value that points to a file instead of a folder produces an error. + Specifies the folders whose content you want to list. Relative paths are resolved against the current location, and wildcard characters are not supported. If you omit this parameter, the cmdlet lists the current location. A value that points to a file returns that file, like `Get-ChildItem`, unless you use `-Directory`. String[] @@ -3734,7 +3734,7 @@ PS C:\> Disable-Privileges Path - Specifies the folders whose content you want to list. Relative paths are resolved against the current location, and wildcard characters are not supported. If you omit this parameter, the cmdlet lists the current location. A value that points to a file instead of a folder produces an error. + Specifies the folders whose content you want to list. Relative paths are resolved against the current location, and wildcard characters are not supported. If you omit this parameter, the cmdlet lists the current location. A value that points to a file returns that file, like `Get-ChildItem`, unless you use `-Directory`. String[] @@ -3838,6 +3838,7 @@ PS C:\> Disable-Privileges The module defines the alias `dir2` for this cmdlet. The `PrivateData` section of the module manifest `NTFSSecurity.psd1` contains two settings that this cmdlet reads when it starts. `GetFileSystemModeProperty` adds the calculated `Mode` property to every item. `IdentifyHardLinks` adds the `HardLinkCount` property to every file, which requires an extra call into the file system for each file and therefore slows down large listings noticeably. Set either value to `$false` in the manifest and import the module again if you prefer the faster enumeration over the additional properties. A folder that cannot be read produces a non-terminating error with the ID `DirUnauthorizedAccessError` for an access denial or `DirUnspecifiedError` for any other failure, and a path that does not exist produces the error `FileNotFound`. In each case the cmdlet continues with the next path. Failures that occur while `-Recurse` collects the subfolders of a folder are reported as verbose messages only, not as errors. + Before 5.0.0, a `-Path` value that points to a file stopped the cmdlet with an `InvalidCastException`. diff --git a/Tests/ItemCmdlets.Tests.ps1 b/Tests/ItemCmdlets.Tests.ps1 new file mode 100644 index 0000000..75af9ec --- /dev/null +++ b/Tests/ItemCmdlets.Tests.ps1 @@ -0,0 +1,57 @@ +<# + Tests the long-path item cmdlets of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'ItemCmdlets' + Push-Location -LiteralPath $sandbox +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Get-ChildItem2' { + Context 'When -Path is a file' { + BeforeAll { + $folder = Join-Path -Path $sandbox -ChildPath 'ChildItem' + $file = Join-Path -Path $folder -ChildPath 'File.txt' + $other = Join-Path -Path $folder -ChildPath 'Folder\Other.txt' + Assert-TestSandboxPath -Sandbox $sandbox -Path $folder, $file, $other + New-Item -ItemType Directory -Path (Split-Path -Path $other -Parent) -Force | Out-Null + Set-Content -LiteralPath $file -Value 'File' + Set-Content -LiteralPath $other -Value 'Other' + } + + It 'Should return the file itself, like Get-ChildItem' { + $items = @(Get-ChildItem2 -Path $file -ErrorVariable childItemErrors -ErrorAction SilentlyContinue) + + $childItemErrors | Should -BeNullOrEmpty + $items | Should -HaveCount 1 + $items[0] | Should -BeOfType [Alphaleonis.Win32.Filesystem.FileInfo] + $items[0].Name | Should -BeExactly 'File.txt' + } + + It 'Should continue with the next path after a file' { + $items = @(Get-ChildItem2 -Path $file, (Join-Path -Path $folder -ChildPath 'Folder') -ErrorAction SilentlyContinue) + + $items.Name | Should -Be @('File.txt', 'Other.txt') + } + + It 'Should return nothing for a file with -Directory' { + $items = @(Get-ChildItem2 -Path $file -Directory -ErrorVariable childItemErrors -ErrorAction SilentlyContinue) + + $childItemErrors | Should -BeNullOrEmpty + $items | Should -BeNullOrEmpty + } + } +} From 7f625a168b76a689f40d5789c3152c940122fa7d Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:10:15 +0200 Subject: [PATCH 04/19] fix: skip folders in Get-FileHash2 and hash the remaining paths Defect 3. Get-FileHash2 left ProcessRecord at the first folder in -Path, so the files that followed the folder in the same array were not hashed. It now skips the folder, like Get-FileHash, and continues. Tests/FileHash.Tests.ps1 (new): 1 test. It skips in PowerShell 7, where every Get-FileHash2 call fails until the RIPEMD160 reference goes (decision D5, later in this run). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 2 + Docs/Cmdlets/Get-FileHash2.md | 4 +- NTFSSecurity/MiscCmdlets/GetFileHash2.cs | 6 ++- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 3 +- Tests/FileHash.Tests.ps1 | 45 ++++++++++++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 Tests/FileHash.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 850671d..b9ceb6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,5 +57,7 @@ The format is based on omitted; an omitted parameter now leaves its section unchanged - Fix `Get-ChildItem2`, which stopped with an `InvalidCastException` when `-Path` pointed to a file; it now returns the file, like `Get-ChildItem` +- Fix `Get-FileHash2`, which stopped at a folder in `-Path` and didn't hash + the files that followed it; folders are now skipped [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-FileHash2.md b/Docs/Cmdlets/Get-FileHash2.md index 3eb6d05..3867548 100644 --- a/Docs/Cmdlets/Get-FileHash2.md +++ b/Docs/Cmdlets/Get-FileHash2.md @@ -23,7 +23,7 @@ The `Get-FileHash2` cmdlet calculates the hash value of each file that `-Path` p `-Algorithm` selects the hash algorithm and accepts `SHA1`, `SHA256`, `SHA384`, `SHA512`, `MACTripleDES`, `MD5`, and `RIPEMD160`. The default is `SHA256`. -The cmdlet hashes files only. A path that points to a folder is skipped, and because the cmdlet stops processing the current input when it meets one, a folder in the middle of a `-Path` array suppresses the results of the paths that follow it in the same array. Pass only file paths, or filter folders out before you pipe items into the cmdlet. A path that does not exist produces a non-terminating `ReadFileError`. +The cmdlet hashes files only and skips paths that point to folders. A path that does not exist produces a non-terminating `ReadFileError`. `-Path` accepts pipeline input by value and by property name through its `FullName` alias, so you can pipe the output of `Get-ChildItem2`, `Get-Item2`, or `Get-ChildItem` into the cmdlet; folders that arrive through the pipeline are skipped individually. Because the cmdlet reads files through the AlphaFS library, it also hashes files whose path exceeds the 260-character `MAX_PATH` limit. Relative paths are resolved against the current location. @@ -125,6 +125,8 @@ The hash is returned as an uppercase hexadecimal string without separators, whic `MACTripleDES` is a keyed message authentication code that is created with a key that is generated for each call, so its result is not reproducible across invocations and is not suitable for comparing files. +Before 5.0.0, a folder in a `-Path` array stopped the processing of that array, so the files that followed the folder were not hashed. + ## RELATED LINKS [Get-ChildItem2](Get-ChildItem2.md) diff --git a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs index 1555a4c..b717d2b 100644 --- a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs +++ b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs @@ -48,7 +48,11 @@ namespace NTFSSecurity { item = GetFileSystemInfo2(path) as FileInfo; if (item == null) - return; + { + // Like Get-FileHash, skip folders and continue with the next path. + WriteVerbose(string.Format("Skipping '{0}', which is a folder", path)); + continue; + } } catch (Exception ex) { diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index bf7b04d..7e44f86 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -4042,7 +4042,7 @@ PS C:\> Disable-Privileges The `Get-FileHash2` cmdlet calculates the hash value of each file that `-Path` points to and returns the file object with the result attached. The returned object is the file object of the file, extended with a `Hash` property that holds the hash as an uppercase hexadecimal string and an `Algorithm` property that names the algorithm that was used. The module's formatting data displays those objects as a table with the `Algorithm`, `Hash`, and `FullName` columns. `-Algorithm` selects the hash algorithm and accepts `SHA1`, `SHA256`, `SHA384`, `SHA512`, `MACTripleDES`, `MD5`, and `RIPEMD160`. The default is `SHA256`. - The cmdlet hashes files only. A path that points to a folder is skipped, and because the cmdlet stops processing the current input when it meets one, a folder in the middle of a `-Path` array suppresses the results of the paths that follow it in the same array. Pass only file paths, or filter folders out before you pipe items into the cmdlet. A path that does not exist produces a non-terminating `ReadFileError`. + The cmdlet hashes files only and skips paths that point to folders. A path that does not exist produces a non-terminating `ReadFileError`. `-Path` accepts pipeline input by value and by property name through its `FullName` alias, so you can pipe the output of `Get-ChildItem2`, `Get-Item2`, or `Get-ChildItem` into the cmdlet; folders that arrive through the pipeline are skipped individually. Because the cmdlet reads files through the AlphaFS library, it also hashes files whose path exceeds the 260-character `MAX_PATH` limit. Relative paths are resolved against the current location. @@ -4143,6 +4143,7 @@ PS C:\> Disable-Privileges If the file cannot be opened because access is denied, the cmdlet takes ownership of the file with the account that runs it, calculates the hash, and restores the previous owner afterward. That fallback fails with a `GetHashError` when the account is not allowed to change the owner of the file. The hash is returned as an uppercase hexadecimal string without separators, which differs from the lowercase output of some other hashing tools. Compare hash values case-insensitively. `MACTripleDES` is a keyed message authentication code that is created with a key that is generated for each call, so its result is not reproducible across invocations and is not suitable for comparing files. + Before 5.0.0, a folder in a `-Path` array stopped the processing of that array, so the files that followed the folder were not hashed. diff --git a/Tests/FileHash.Tests.ps1 b/Tests/FileHash.Tests.ps1 new file mode 100644 index 0000000..64d9cba --- /dev/null +++ b/Tests/FileHash.Tests.ps1 @@ -0,0 +1,45 @@ +<# + Tests Get-FileHash2 of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'FileHash' + Push-Location -LiteralPath $sandbox + + $folder = Join-Path -Path $sandbox -ChildPath 'Folder' + $first = Join-Path -Path $sandbox -ChildPath 'One.txt' + $second = Join-Path -Path $sandbox -ChildPath 'Two.txt' + Assert-TestSandboxPath -Sandbox $sandbox -Path $folder, $first, $second + New-Item -ItemType Directory -Path $folder | Out-Null + Set-Content -LiteralPath $first -Value 'One' + Set-Content -LiteralPath $second -Value 'Two' +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Get-FileHash2' { + Context 'When -Path contains a folder' { + It 'Should skip the folder and hash the files that follow it' { + if ($PSVersionTable.PSEdition -eq 'Core') { + Set-ItResult -Skipped -Because 'Get-FileHash2 fails in PowerShell 7 until it no longer references RIPEMD160' + } + + $results = @(Get-FileHash2 -Path $first, $folder, $second -ErrorVariable hashErrors -ErrorAction SilentlyContinue) + + $hashErrors | Should -BeNullOrEmpty + $results.Name | Should -Be @('One.txt', 'Two.txt') + $results[1].Hash | Should -BeExactly (Get-FileHash -LiteralPath $second -Algorithm SHA256).Hash + } + } +} From ac9e57e349d4db9e61737813e44d9bfa3fc86da8 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:15:00 +0200 Subject: [PATCH 05/19] fix: report unreadable audit entries in Get-NTFSAudit Defect 4, both parts: - Get-NTFSAudit kept the entries of the previous item and wrote them in a finally block, so a path whose security descriptor failed to read returned the previous item's entries again. Each item now starts empty, and entries are written only after a successful read. - Without the Security privilege, the cmdlet read the descriptor without its SACL and returned nothing, like an item without audit entries. It now reads the SACL alone, so a missing privilege is a ReadSecurityError ("A required privilege is not held by the client"). A descriptor from Get-NTFSSecurityDescriptor that was read without the SACL gets the same error; FileSystemSecurity2 now records which sections it read (internal, visible to NTFSSecurity). Tests/Audit.Tests.ps1 (new): 3 tests. The repeat test needs the Security privilege to add an audit entry and runs in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 3 + Docs/Cmdlets/Get-NTFSAudit.md | 6 +- NTFSSecurity/AuditCmdlets/GetAudit.cs | 50 ++++++---- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 5 +- Security2/FileSystem/FileSystemSecurity2.cs | 12 +++ Security2/Properties/AssemblyInfo.cs | 3 + Tests/Audit.Tests.ps1 | 100 +++++++++++++++++++ 7 files changed, 154 insertions(+), 25 deletions(-) create mode 100644 Tests/Audit.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index b9ceb6e..f69ca96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,5 +59,8 @@ The format is based on `-Path` pointed to a file; it now returns the file, like `Get-ChildItem` - Fix `Get-FileHash2`, which stopped at a folder in `-Path` and didn't hash the files that followed it; folders are now skipped +- Fix `Get-NTFSAudit`, which returned nothing without the Security privilege + instead of an error, and which returned the entries of the previous item + again after a path whose security descriptor it couldn't read [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-NTFSAudit.md b/Docs/Cmdlets/Get-NTFSAudit.md index 03ec754..b010e7f 100644 --- a/Docs/Cmdlets/Get-NTFSAudit.md +++ b/Docs/Cmdlets/Get-NTFSAudit.md @@ -173,16 +173,18 @@ You can pass an account name or a SID string to `-Account`, which the cmdlet con ### Security2.FileSystemAuditRule2 -The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, or when the SACL cannot be read, the cmdlet returns nothing for that item. +The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, the cmdlet returns nothing for that item; when its SACL cannot be read, the cmdlet writes an error. ## NOTES When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. -Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it the cmdlet falls back to reading the security descriptor without its SACL; it then returns no audit entries and reports no error, which looks the same as an item that is not audited at all. +Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well. If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. +Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. + ## RELATED LINKS [Add-NTFSAudit](Add-NTFSAudit.md) diff --git a/NTFSSecurity/AuditCmdlets/GetAudit.cs b/NTFSSecurity/AuditCmdlets/GetAudit.cs index 394417e..6531274 100644 --- a/NTFSSecurity/AuditCmdlets/GetAudit.cs +++ b/NTFSSecurity/AuditCmdlets/GetAudit.cs @@ -79,13 +79,13 @@ namespace NTFSSecurity protected override void ProcessRecord() { - IEnumerable acl = null; - FileSystemInfo item = null; - if (ParameterSetName == "Path") { foreach (var path in paths) { + FileSystemInfo item = null; + IEnumerable acl = null; + try { item = GetFileSystemInfo2(path); @@ -98,7 +98,7 @@ namespace NTFSSecurity try { - acl = FileSystemAuditRule2.GetFileSystemAuditRules(item, !excludeExplicit, !excludeInherited, getInheritedFrom); + acl = GetAuditRules(item); } catch (UnauthorizedAccessException) { @@ -108,7 +108,7 @@ namespace NTFSSecurity var previousOwner = ownerInfo.Owner; FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - acl = FileSystemAuditRule2.GetFileSystemAuditRules(item, !excludeExplicit, !excludeInherited, getInheritedFrom); + acl = GetAuditRules(item); FileSystemOwner.SetOwner(item, previousOwner); } catch (Exception ex2) @@ -122,34 +122,42 @@ namespace NTFSSecurity WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.OpenError, path)); continue; } - finally - { - if (acl != null) - { - if (account != null) - { - acl = acl.Where(ace => ace.Account == account); - } - acl.ForEach(ace => WriteObject(ace)); - } - } + WriteAuditRules(acl); } } else { foreach (var sd in securityDescriptors) { - acl = FileSystemAuditRule2.GetFileSystemAuditRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom); - - if (account != null) + if (!sd.HasAuditSection) { - acl = acl.Where(ace => ace.Account == account); + var ex = new InvalidOperationException(string.Format( + "The security descriptor of '{0}' doesn't contain the audit entries, because it was read without the Security privilege.", sd.FullName)); + WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.InvalidData, sd)); + continue; } - acl.ForEach(ace => WriteObject(ace)); + WriteAuditRules(FileSystemAuditRule2.GetFileSystemAuditRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom)); } } } + + private IEnumerable GetAuditRules(FileSystemInfo item) + { + // Reading only the SACL fails without the Security privilege, instead of returning no entries. + var sd = new FileSystemSecurity2(item, System.Security.AccessControl.AccessControlSections.Audit); + return FileSystemAuditRule2.GetFileSystemAuditRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom); + } + + private void WriteAuditRules(IEnumerable acl) + { + if (account != null) + { + acl = acl.Where(ace => ace.Account == account); + } + + acl.ForEach(ace => WriteObject(ace)); + } } } \ No newline at end of file diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 7e44f86..c93e5ab 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -4832,15 +4832,16 @@ PS C:\> Disable-Privileges Security2.FileSystemAuditRule2 - The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, or when the SACL cannot be read, the cmdlet returns nothing for that item. + The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, the cmdlet returns nothing for that item; when its SACL cannot be read, the cmdlet writes an error. When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. - Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it the cmdlet falls back to reading the security descriptor without its SACL; it then returns no audit entries and reports no error, which looks the same as an item that is not audited at all. + Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well. If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. + Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. diff --git a/Security2/FileSystem/FileSystemSecurity2.cs b/Security2/FileSystem/FileSystemSecurity2.cs index cd5eb08..fda0c72 100644 --- a/Security2/FileSystem/FileSystemSecurity2.cs +++ b/Security2/FileSystem/FileSystemSecurity2.cs @@ -53,16 +53,19 @@ namespace Security2 try { sd = ((FileInfo)this.item).GetAccessControl(AccessControlSections.All); + sections = AccessControlSections.All; } catch { try { sd = ((FileInfo)this.item).GetAccessControl(AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group); + sections = AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group; } catch { sd = ((FileInfo)this.item).GetAccessControl(AccessControlSections.Access); + sections = AccessControlSections.Access; } } @@ -74,21 +77,30 @@ namespace Security2 try { sd = ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.All); + sections = AccessControlSections.All; } catch { try { sd = ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group); + sections = AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group; } catch { sd = ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.Access); + sections = AccessControlSections.Access; } } } } + // Without the Security privilege, the security descriptor is read without its SACL. + internal bool HasAuditSection + { + get { return (sections & AccessControlSections.Audit) == AccessControlSections.Audit; } + } + public FileSystemSecurity SecurityDescriptor { get diff --git a/Security2/Properties/AssemblyInfo.cs b/Security2/Properties/AssemblyInfo.cs index 89a37d6..5f5bd6d 100644 --- a/Security2/Properties/AssemblyInfo.cs +++ b/Security2/Properties/AssemblyInfo.cs @@ -19,6 +19,9 @@ using System.Runtime.InteropServices; // COM, set the ComVisible attribute to true on that type. [assembly: ComVisible(false)] +// Lets the cmdlets tell whether a security descriptor was read with its SACL. +[assembly: InternalsVisibleTo("NTFSSecurity")] + // The following GUID is for the ID of the typelib if this project is exposed to COM [assembly: Guid("d89dc40a-9b43-4bce-972d-b995df8d2820")] diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 new file mode 100644 index 0000000..59293c2 --- /dev/null +++ b/Tests/Audit.Tests.ps1 @@ -0,0 +1,100 @@ +<# + Tests the audit cmdlets of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. Reading + and changing audit entries needs the Security privilege; tests that need it skip without it and run in CI, + whose runners are elevated. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeDiscovery { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $canReadAudit = Test-PrivilegeHeld -Name 'SeSecurityPrivilege' +} + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'Audit' + Push-Location -LiteralPath $sandbox + + function New-SandboxItem { + param ( + [string] $Name, + [switch] $Directory + ) + + $path = Join-Path -Path $sandbox -ChildPath ('{0}-{1}' -f $Name, [guid]::NewGuid().ToString('N').Substring(0, 8)) + Assert-TestSandboxPath -Sandbox $sandbox -Path $path + if ($Directory) { + New-Item -ItemType Directory -Path $path | Out-Null + } + else { + Set-Content -LiteralPath $path -Value 'Audit test' + } + $path + } + + # Denies the owner, the current account, to read the security descriptor of the item. + function Deny-ReadPermission { + param ([string] $Path) + + Assert-TestSandboxPath -Sandbox $sandbox -Path $Path + $acl = Get-Acl -LiteralPath $Path + $ownerRights = New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-3-4' + $rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( + $ownerRights, [System.Security.AccessControl.FileSystemRights]::ReadPermissions, [System.Security.AccessControl.AccessControlType]::Deny + ) + $acl.AddAccessRule($rule) + Set-Acl -LiteralPath $Path -AclObject $acl + } +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Get-NTFSAudit' { + Context 'When the audit entries cannot be read' { + It 'Should write an error without the Security privilege instead of returning nothing' -Skip:$canReadAudit { + $file = New-SandboxItem -Name 'NoPrivilege' + + $entries = @(Get-NTFSAudit -Path $file -ErrorVariable auditErrors -ErrorAction SilentlyContinue) + + $entries | Should -BeNullOrEmpty + $auditErrors | Should -HaveCount 1 + $auditErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadSecurityError,*' + } + + It 'Should write an error for a security descriptor that was read without the audit entries' { + $file = New-SandboxItem -Name 'AccessOnly' + $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList ( + (Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Access + ) + + $entries = @(Get-NTFSAudit -SecurityDescriptor $sd -ErrorVariable auditErrors -ErrorAction SilentlyContinue) + + $entries | Should -BeNullOrEmpty + $auditErrors | Should -HaveCount 1 + $auditErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadSecurityError,*' + } + } + + Context 'When a path fails after a path with audit entries' { + # Before 5.0.0, the cmdlet wrote the entries of the previous item again for the failing path. + It 'Should return the entries of the first item once' -Skip:(-not $canReadAudit) { + $folder = New-SandboxItem -Name 'Audited' -Directory + $denied = New-SandboxItem -Name 'Denied' + Add-NTFSAudit -Path $folder -Account 'Everyone' -AccessRights Delete -AuditFlags Success + Deny-ReadPermission -Path $denied + + $entries = @(Get-NTFSAudit -Path $folder, $denied -ExcludeInherited -ErrorAction SilentlyContinue) + + @($entries | Where-Object -Property FullName -EQ -Value $folder) | Should -HaveCount 1 + } + } +} From b329998de3bc4b66d7bc1b18e0f3a6205fd288ae Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:21:20 +0200 Subject: [PATCH 06/19] test: share the sandbox item and read-deny helpers - New-TestSandboxItem creates a uniquely named file or folder in a sandbox after the path guard, replacing per-file copies. - Block-TestReadPermission denies OWNER RIGHTS the right to read the security descriptor of a sandbox item, so that reading it fails without elevation. - Remove-TestSandbox no longer stops when icacls cannot reset such an item: Windows PowerShell turned the icacls message on stderr into a terminating error; the item is still deleted through its folder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- Tests/Audit.Tests.ps1 | 41 +++------------------ Tests/Inheritance.Tests.ps1 | 9 +---- Tests/TestHelpers.psm1 | 71 ++++++++++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 45 deletions(-) diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 index 59293c2..77df1b5 100644 --- a/Tests/Audit.Tests.ps1 +++ b/Tests/Audit.Tests.ps1 @@ -19,37 +19,6 @@ BeforeAll { Import-Module -Name $modulePath -Force -ErrorAction Stop $sandbox = New-TestSandbox -Name 'Audit' Push-Location -LiteralPath $sandbox - - function New-SandboxItem { - param ( - [string] $Name, - [switch] $Directory - ) - - $path = Join-Path -Path $sandbox -ChildPath ('{0}-{1}' -f $Name, [guid]::NewGuid().ToString('N').Substring(0, 8)) - Assert-TestSandboxPath -Sandbox $sandbox -Path $path - if ($Directory) { - New-Item -ItemType Directory -Path $path | Out-Null - } - else { - Set-Content -LiteralPath $path -Value 'Audit test' - } - $path - } - - # Denies the owner, the current account, to read the security descriptor of the item. - function Deny-ReadPermission { - param ([string] $Path) - - Assert-TestSandboxPath -Sandbox $sandbox -Path $Path - $acl = Get-Acl -LiteralPath $Path - $ownerRights = New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-3-4' - $rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( - $ownerRights, [System.Security.AccessControl.FileSystemRights]::ReadPermissions, [System.Security.AccessControl.AccessControlType]::Deny - ) - $acl.AddAccessRule($rule) - Set-Acl -LiteralPath $Path -AclObject $acl - } } AfterAll { @@ -61,7 +30,7 @@ AfterAll { Describe 'Get-NTFSAudit' { Context 'When the audit entries cannot be read' { It 'Should write an error without the Security privilege instead of returning nothing' -Skip:$canReadAudit { - $file = New-SandboxItem -Name 'NoPrivilege' + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'NoPrivilege' $entries = @(Get-NTFSAudit -Path $file -ErrorVariable auditErrors -ErrorAction SilentlyContinue) @@ -71,7 +40,7 @@ Describe 'Get-NTFSAudit' { } It 'Should write an error for a security descriptor that was read without the audit entries' { - $file = New-SandboxItem -Name 'AccessOnly' + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'AccessOnly' $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList ( (Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Access ) @@ -87,10 +56,10 @@ Describe 'Get-NTFSAudit' { Context 'When a path fails after a path with audit entries' { # Before 5.0.0, the cmdlet wrote the entries of the previous item again for the failing path. It 'Should return the entries of the first item once' -Skip:(-not $canReadAudit) { - $folder = New-SandboxItem -Name 'Audited' -Directory - $denied = New-SandboxItem -Name 'Denied' + $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'Audited' -Directory + $denied = New-TestSandboxItem -Sandbox $sandbox -Name 'Denied' Add-NTFSAudit -Path $folder -Account 'Everyone' -AccessRights Delete -AuditFlags Success - Deny-ReadPermission -Path $denied + Block-TestReadPermission -Sandbox $sandbox -Path $denied $entries = @(Get-NTFSAudit -Path $folder, $denied -ExcludeInherited -ErrorAction SilentlyContinue) diff --git a/Tests/Inheritance.Tests.ps1 b/Tests/Inheritance.Tests.ps1 index f21b891..8503a53 100644 --- a/Tests/Inheritance.Tests.ps1 +++ b/Tests/Inheritance.Tests.ps1 @@ -18,13 +18,6 @@ BeforeAll { Import-Module -Name $modulePath -Force -ErrorAction Stop $sandbox = New-TestSandbox -Name 'Inheritance' Push-Location -LiteralPath $sandbox - - function New-SandboxFile { - $path = Join-Path -Path $sandbox -ChildPath ('File-{0}.txt' -f [guid]::NewGuid().ToString('N').Substring(0, 8)) - Assert-TestSandboxPath -Sandbox $sandbox -Path $path - Set-Content -LiteralPath $path -Value 'Inheritance test' - $path - } } AfterAll { @@ -36,7 +29,7 @@ AfterAll { Describe 'Set-NTFSInheritance' { Context 'When -AccessInheritanceEnabled or -AuditInheritanceEnabled is omitted' { BeforeEach { - $file = New-SandboxFile + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'File' Assert-TestSandboxPath -Sandbox $sandbox -Path $file } diff --git a/Tests/TestHelpers.psm1 b/Tests/TestHelpers.psm1 index 3ac1f64..4d7656d 100644 --- a/Tests/TestHelpers.psm1 +++ b/Tests/TestHelpers.psm1 @@ -86,6 +86,10 @@ function Remove-TestSandbox { return } + # icacls reports items it cannot reset on stderr, which Windows PowerShell turns into a terminating error when + # the caller uses ErrorAction Stop. Such items can still be deleted through the rights on their folder. + $ErrorActionPreference = 'Continue' + # Windows PowerShell 5.1 follows directory links when it removes a folder recursively, so the links go first. & icacls.exe $Sandbox /reset /T /C /Q *> $null $pending = New-Object -TypeName 'System.Collections.Generic.Stack[string]' @@ -120,6 +124,70 @@ function Remove-TestSandbox { } } +function New-TestSandboxItem { + <# + .SYNOPSIS + Creates a file or folder with a unique name in the sandbox and returns its full path. + #> + [Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Test helper that only writes to sandboxes.' + )] + [CmdletBinding()] + [OutputType([string])] + param ( + [Parameter(Mandatory)] + [string] + $Sandbox, + + [ValidatePattern('^[\w-]+$')] + [string] + $Name = 'Item', + + [switch] + $Directory + ) + + $path = Join-Path -Path $Sandbox -ChildPath ('{0}-{1}' -f $Name, [guid]::NewGuid().ToString('N').Substring(0, 8)) + Assert-TestSandboxPath -Sandbox $Sandbox -Path $path + if ($Directory) { + New-Item -ItemType Directory -Path $path | Out-Null + } + else { + Set-Content -LiteralPath $path -Value $Name + } + $path +} + +function Block-TestReadPermission { + <# + .SYNOPSIS + Denies the owner of an item in the sandbox to read its security descriptor, so that reading it fails. + .DESCRIPTION + Adds a deny entry for OWNER RIGHTS (S-1-3-4) with ReadPermissions, which replaces the implicit right of the + owner to read the security descriptor. Remove-TestSandbox resets it. + #> + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [string] + $Sandbox, + + [Parameter(Mandatory)] + [string] + $Path + ) + + Assert-TestSandboxPath -Sandbox $Sandbox -Path $Path + $acl = Get-Acl -LiteralPath $Path + $ownerRights = New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-3-4' + $rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( + $ownerRights, [System.Security.AccessControl.FileSystemRights]::ReadPermissions, + [System.Security.AccessControl.AccessControlType]::Deny + ) + $acl.AddAccessRule($rule) + Set-Acl -LiteralPath $Path -AclObject $acl +} + function Test-IsElevated { <# .SYNOPSIS @@ -160,4 +228,5 @@ function Test-PrivilegeHeld { Where-Object -Property Name -EQ -Value $Name) } -Export-ModuleMember -Function New-TestSandbox, Assert-TestSandboxPath, Remove-TestSandbox, Test-IsElevated, Test-PrivilegeHeld +Export-ModuleMember -Function New-TestSandbox, Assert-TestSandboxPath, Remove-TestSandbox, New-TestSandboxItem, + Block-TestReadPermission, Test-IsElevated, Test-PrivilegeHeld From 3a13e49d429ec285e44f2df4dc2e6c90c43331b0 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:21:20 +0200 Subject: [PATCH 07/19] fix: don't repeat the previous item's entries in Get-NTFSAccess Found while fixing defect 4: Get-NTFSAccess has the same pattern as Get-NTFSAudit. It kept the entries of the previous item and wrote them in a finally block, so a path whose ACL failed to read returned the previous item's entries again, next to the error. Each item now starts empty, and entries are written only after a successful read. Tests/Access.Tests.ps1 (new): 1 test; it failed before the fix with 6 entries instead of 3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 2 + Docs/Cmdlets/Get-NTFSAccess.md | 2 + NTFSSecurity/AccessCmdlets/GetAccess.cs | 39 ++++++++------------ NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 1 + Tests/Access.Tests.ps1 | 37 +++++++++++++++++++ 5 files changed, 58 insertions(+), 23 deletions(-) create mode 100644 Tests/Access.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index f69ca96..2934782 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,5 +62,7 @@ The format is based on - Fix `Get-NTFSAudit`, which returned nothing without the Security privilege instead of an error, and which returned the entries of the previous item again after a path whose security descriptor it couldn't read +- Fix `Get-NTFSAccess`, which returned the entries of the previous item again + after a path whose ACL it couldn't read [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-NTFSAccess.md b/Docs/Cmdlets/Get-NTFSAccess.md index 8036d44..b765835 100644 --- a/Docs/Cmdlets/Get-NTFSAccess.md +++ b/Docs/Cmdlets/Get-NTFSAccess.md @@ -184,6 +184,8 @@ If the ACL of an item cannot be read because access is denied, the cmdlet tries Entries whose account cannot be translated into a name are returned with their SID. Use `Get-NTFSOrphanedAccess` to list only those entries. +Before 5.0.0, after a path whose ACL could not be read, the cmdlet returned the entries of the previous item again. + ## RELATED LINKS [Add-NTFSAccess](Add-NTFSAccess.md) diff --git a/NTFSSecurity/AccessCmdlets/GetAccess.cs b/NTFSSecurity/AccessCmdlets/GetAccess.cs index 7013487..6b9339a 100644 --- a/NTFSSecurity/AccessCmdlets/GetAccess.cs +++ b/NTFSSecurity/AccessCmdlets/GetAccess.cs @@ -79,13 +79,13 @@ namespace NTFSSecurity protected override void ProcessRecord() { - IEnumerable acl = null; - FileSystemInfo item = null; - if (ParameterSetName == "Path") { foreach (var path in paths) { + FileSystemInfo item = null; + IEnumerable acl = null; + try { item = GetFileSystemInfo2(path); @@ -122,34 +122,27 @@ namespace NTFSSecurity WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.OpenError, path)); continue; } - finally - { - if (acl != null) - { - if (account != null) - { - acl = acl.Where(ace => ace.Account == account); - } - acl.ForEach(ace => WriteObject(ace)); - } - } + WriteAccessRules(acl); } } else { foreach (var sd in securityDescriptors) { - acl = FileSystemAccessRule2.GetFileSystemAccessRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom); - - if (account != null) - { - acl = acl.Where(ace => ace.Account == account); - } - - acl.ForEach(ace => WriteObject(ace)); + WriteAccessRules(FileSystemAccessRule2.GetFileSystemAccessRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom)); } } } + + private void WriteAccessRules(IEnumerable acl) + { + if (account != null) + { + acl = acl.Where(ace => ace.Account == account); + } + + acl.ForEach(ace => WriteObject(ace)); + } } -} +} \ No newline at end of file diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index c93e5ab..b9dff19 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -4560,6 +4560,7 @@ PS C:\> Disable-Privileges When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. If the ACL of an item cannot be read because access is denied, the cmdlet tries once more after making the current account the owner of the item, and restores the previous owner afterwards. Changing the owner of an item requires the Take Ownership and Restore privileges, so this fallback only succeeds in an elevated session of an account that holds them. Entries whose account cannot be translated into a name are returned with their SID. Use `Get-NTFSOrphanedAccess` to list only those entries. + Before 5.0.0, after a path whose ACL could not be read, the cmdlet returned the entries of the previous item again. diff --git a/Tests/Access.Tests.ps1 b/Tests/Access.Tests.ps1 new file mode 100644 index 0000000..51f60e6 --- /dev/null +++ b/Tests/Access.Tests.ps1 @@ -0,0 +1,37 @@ +<# + Tests the access cmdlets of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'Access' + Push-Location -LiteralPath $sandbox +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Get-NTFSAccess' { + Context 'When a path fails after a readable path' { + # Before 5.0.0, the cmdlet wrote the entries of the previous item again for the failing path. + It 'Should return the entries of the first item once' { + $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'Readable' -Directory + $denied = New-TestSandboxItem -Sandbox $sandbox -Name 'Denied' + Block-TestReadPermission -Sandbox $sandbox -Path $denied + $expected = @(Get-NTFSAccess -Path $folder).Count + + $entries = @(Get-NTFSAccess -Path $folder, $denied -ErrorAction SilentlyContinue) + + @($entries | Where-Object -Property FullName -EQ -Value $folder) | Should -HaveCount $expected + } + } +} From ef1d4262e3422d9f556e8dc718d88051b6cfa8e5 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:29:37 +0200 Subject: [PATCH 08/19] fix: move Add-NTFSAudit -AccessRights to position 3 Defect 5 (#4). -Account and -AccessRights of Add-NTFSAudit were both declared at position 2, so a positional call failed with "Cannot bind positional parameters because no names were given". -AccessRights is now at position 3 in all four parameter sets, like in Remove-NTFSAudit. The page's parameter metadata says position 3 as well. platyPS takes the position from Get-Help, that is from the shipped help file, so Update-MarkdownHelp kept the old value; the page, the regenerated help file, and the build now agree. Tests/Audit.Tests.ps1: 5 tests (positions in each parameter set and a positional call against an in-memory security descriptor). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 4 + Docs/Cmdlets/Add-NTFSAudit.md | 4 +- NTFSSecurity/AuditCmdlets/AddAudit.cs | 2 +- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 108 +++++++++---------- Tests/Audit.Tests.ps1 | 27 +++++ 5 files changed, 88 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2934782..2c03f4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,5 +64,9 @@ The format is based on again after a path whose security descriptor it couldn't read - Fix `Get-NTFSAccess`, which returned the entries of the previous item again after a path whose ACL it couldn't read +- Fix `Add-NTFSAudit`, whose `-Account` and `-AccessRights` parameters were + both at position 2, so that positional calls failed; `-AccessRights` is + now at position 3, like in `Remove-NTFSAudit` + ([#4](https://github.com/raandree/NTFSSecurity/issues/4)) [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Add-NTFSAudit.md b/Docs/Cmdlets/Add-NTFSAudit.md index 6b24358..bc30563 100644 --- a/Docs/Cmdlets/Add-NTFSAudit.md +++ b/Docs/Cmdlets/Add-NTFSAudit.md @@ -101,7 +101,7 @@ Aliases: FileSystemRights Accepted values: None, ReadData, ListDirectory, WriteData, CreateFiles, AppendData, CreateDirectories, ReadExtendedAttributes, WriteExtendedAttributes, ExecuteFile, Traverse, DeleteSubdirectoriesAndFiles, ReadAttributes, WriteAttributes, Write, Delete, ReadPermissions, Read, ReadAndExecute, Modify, ChangePermissions, TakeOwnership, Synchronize, FullControl, GenericAll, GenericExecute, GenericWrite, GenericRead Required: True -Position: 2 +Position: 3 Default value: None Accept pipeline input: True (ByPropertyName) Accept wildcard characters: False @@ -290,7 +290,7 @@ Writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. -The syntax shows `-Path`, `-Account`, and `-AccessRights` as positional parameters, but `-Account` and `-AccessRights` are both declared at position 2. A command that passes them positionally therefore fails with the error that positional parameters cannot be bound because no names were given, and `Get-Command Add-NTFSAudit -Syntax` leaves `-Account` out for the same reason. Pass `-Account` and `-AccessRights` by name, as the examples above do. +`-Path` or `-SecurityDescriptor`, `-Account`, and `-AccessRights` are positional parameters at positions 1, 2, and 3, like in `Remove-NTFSAudit`. Before 5.0.0, `-Account` and `-AccessRights` were both declared at position 2, so a command that passed them by position failed. An audit entry alone does not create events. Windows writes the events to the security log only while the "Audit object access" policy, or the corresponding "Audit File System" advanced audit policy, is enabled for success, failure, or both. That policy is a Windows setting and is not managed by this module. diff --git a/NTFSSecurity/AuditCmdlets/AddAudit.cs b/NTFSSecurity/AuditCmdlets/AddAudit.cs index 1b1aa6d..8d97b54 100644 --- a/NTFSSecurity/AuditCmdlets/AddAudit.cs +++ b/NTFSSecurity/AuditCmdlets/AddAudit.cs @@ -54,7 +54,7 @@ namespace NTFSSecurity set { account = value; } } - [Parameter(Mandatory = true, Position = 2, ValueFromPipelineByPropertyName = true)] + [Parameter(Mandatory = true, Position = 3, ValueFromPipelineByPropertyName = true)] [Alias("FileSystemRights")] public FileSystemRights2 AccessRights { diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index b9dff19..942c0ec 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -814,7 +814,19 @@ None - + + Account + + Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. + + IdentityReference2[] + + IdentityReference2[] + + + None + + AccessRights Specifies the access rights to audit. The value accepts the basic rights such as `Read`, `Write`, `Modify`, and `FullControl` as well as the individual rights such as `Delete` or `WriteAttributes`, and it accepts a comma-separated list that combines them. For the meaning of each right, see Concepts (../Concepts.md). @@ -856,18 +868,6 @@ None - - Account - - Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. - - IdentityReference2[] - - IdentityReference2[] - - - None - AppliesTo @@ -938,7 +938,19 @@ None - + + Account + + Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. + + IdentityReference2[] + + IdentityReference2[] + + + None + + AccessRights Specifies the access rights to audit. The value accepts the basic rights such as `Read`, `Write`, `Modify`, and `FullControl` as well as the individual rights such as `Delete` or `WriteAttributes`, and it accepts a comma-separated list that combines them. For the meaning of each right, see Concepts (../Concepts.md). @@ -980,18 +992,6 @@ None - - Account - - Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. - - IdentityReference2[] - - IdentityReference2[] - - - None - AppliesTo @@ -1062,7 +1062,19 @@ None - + + Account + + Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. + + IdentityReference2[] + + IdentityReference2[] + + + None + + AccessRights Specifies the access rights to audit. The value accepts the basic rights such as `Read`, `Write`, `Modify`, and `FullControl` as well as the individual rights such as `Delete` or `WriteAttributes`, and it accepts a comma-separated list that combines them. For the meaning of each right, see Concepts (../Concepts.md). @@ -1104,18 +1116,6 @@ None - - Account - - Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. - - IdentityReference2[] - - IdentityReference2[] - - - None - AuditFlags @@ -1193,7 +1193,19 @@ None - + + Account + + Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. + + IdentityReference2[] + + IdentityReference2[] + + + None + + AccessRights Specifies the access rights to audit. The value accepts the basic rights such as `Read`, `Write`, `Modify`, and `FullControl` as well as the individual rights such as `Delete` or `WriteAttributes`, and it accepts a comma-separated list that combines them. For the meaning of each right, see Concepts (../Concepts.md). @@ -1235,18 +1247,6 @@ None - - Account - - Specifies the accounts whose access to the item is audited. The value is an account name such as `CONTOSO\JohnDoe`, `CONTOSO\Domain Users`, `BUILTIN\Users`, or `Everyone`, or a SID string such as `S-1-5-32-545`. When you pass several accounts, the cmdlet adds one audit entry per account. - - IdentityReference2[] - - IdentityReference2[] - - - None - AuditFlags @@ -1312,7 +1312,7 @@ - + AccessRights Specifies the access rights to audit. The value accepts the basic rights such as `Read`, `Write`, `Modify`, and `FullControl` as well as the individual rights such as `Delete` or `WriteAttributes`, and it accepts a comma-separated list that combines them. For the meaning of each right, see Concepts (../Concepts.md). @@ -1502,7 +1502,7 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. Writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `AddAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. - The syntax shows `-Path`, `-Account`, and `-AccessRights` as positional parameters, but `-Account` and `-AccessRights` are both declared at position 2. A command that passes them positionally therefore fails with the error that positional parameters cannot be bound because no names were given, and `Get-Command Add-NTFSAudit -Syntax` leaves `-Account` out for the same reason. Pass `-Account` and `-AccessRights` by name, as the examples above do. + `-Path` or `-SecurityDescriptor`, `-Account`, and `-AccessRights` are positional parameters at positions 1, 2, and 3, like in `Remove-NTFSAudit`. Before 5.0.0, `-Account` and `-AccessRights` were both declared at position 2, so a command that passed them by position failed. An audit entry alone does not create events. Windows writes the events to the security log only while the "Audit object access" policy, or the corresponding "Audit File System" advanced audit policy, is enabled for success, failure, or both. That policy is a Windows setting and is not managed by this module. diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 index 77df1b5..432cced 100644 --- a/Tests/Audit.Tests.ps1 +++ b/Tests/Audit.Tests.ps1 @@ -67,3 +67,30 @@ Describe 'Get-NTFSAudit' { } } } + +Describe 'Add-NTFSAudit' { + Context 'Positional parameters' { + It 'Should take -Account at position 2 and -AccessRights at position 3 in the <_> parameter set' -ForEach @( + 'PathSimple', 'PathComplex', 'SDSimple', 'SDComplex' + ) { + $parameterSet = (Get-Command -Name Add-NTFSAudit).ParameterSets | Where-Object -Property Name -EQ -Value $_ + $positions = @{} + $parameterSet.Parameters | Where-Object -Property Position -GE -Value 0 | ForEach-Object -Process { + $positions[$_.Name] = $_.Position + } + + $positions['Account'] | Should -Be 2 + $positions['AccessRights'] | Should -Be 3 + } + + It 'Should bind an account and access rights that are passed by position' { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Positional' + $sd = Get-NTFSSecurityDescriptor -Path $file + + Add-NTFSAudit -SecurityDescriptor $sd 'Everyone' 'ReadData' -InheritanceFlags None -PropagationFlags None -ErrorAction Stop + + $rules = $sd.SecurityDescriptor.GetAuditRules($true, $false, [System.Security.Principal.SecurityIdentifier]) + @($rules | Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' }) | Should -HaveCount 1 + } + } +} From 5a19974f59ec7f7bb8b5785099f83430eb3cbd1d Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:33:12 +0200 Subject: [PATCH 09/19] fix: return audit entries from -PassThru of the audit cmdlets Defect 6. With -PassThru, Add-NTFSAudit returned the access entries of a security descriptor in its SecurityDescriptor sets, and Remove-NTFSAudit returned the access entries of the item in its Path sets. Both now return the audit entries, as in their other parameter sets. Tests/Audit.Tests.ps1: 2 tests; the Remove-NTFSAudit test writes a SACL and runs in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 3 ++ Docs/Cmdlets/Add-NTFSAudit.md | 4 +-- Docs/Cmdlets/Remove-NTFSAudit.md | 4 +-- NTFSSecurity/AuditCmdlets/AddAudit.cs | 2 +- NTFSSecurity/AuditCmdlets/RemoveAudit.cs | 2 +- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 8 +++--- Tests/Audit.Tests.ps1 | 29 ++++++++++++++++++++ 7 files changed, 42 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c03f4c..fc2071a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,5 +68,8 @@ The format is based on both at position 2, so that positional calls failed; `-AccessRights` is now at position 3, like in `Remove-NTFSAudit` ([#4](https://github.com/raandree/NTFSSecurity/issues/4)) +- Fix `-PassThru` of `Add-NTFSAudit` with `-SecurityDescriptor` and of + `Remove-NTFSAudit` with `-Path`, which returned access entries; both now + return the audit entries [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Add-NTFSAudit.md b/Docs/Cmdlets/Add-NTFSAudit.md index bc30563..4ffb685 100644 --- a/Docs/Cmdlets/Add-NTFSAudit.md +++ b/Docs/Cmdlets/Add-NTFSAudit.md @@ -278,9 +278,9 @@ The value passed to `-AppliesTo` is converted to this type and binds by property ## OUTPUTS -### Security2.FileSystemAccessRule2 +### Security2.FileSystemAuditRule2 -Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the type depends on the parameter set: in the `Path` sets the cmdlet writes all audit entries of the item, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects, while in the `SecurityDescriptor` sets it writes the access entries of the descriptor as `Security2.FileSystemAccessRule2` objects. Use `Get-NTFSAudit` when you need the audit entries of a security descriptor. +Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `SecurityDescriptor` sets wrote the access entries of the descriptor instead. ## NOTES diff --git a/Docs/Cmdlets/Remove-NTFSAudit.md b/Docs/Cmdlets/Remove-NTFSAudit.md index 6738cff..c387e31 100644 --- a/Docs/Cmdlets/Remove-NTFSAudit.md +++ b/Docs/Cmdlets/Remove-NTFSAudit.md @@ -276,9 +276,9 @@ The value passed to `-AppliesTo` is converted to this type and binds by property ## OUTPUTS -### Security2.FileSystemAccessRule2 +### Security2.FileSystemAuditRule2 -Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the type depends on the parameter set: in the `Path` sets the cmdlet writes all access entries of the item, explicit and inherited ones, as `Security2.FileSystemAccessRule2` objects, while in the `SecurityDescriptor` sets it writes all audit entries of the descriptor as `Security2.FileSystemAuditRule2` objects. Use `Get-NTFSAudit` to check the audit entries of an item after the removal. +Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `Path` sets wrote the access entries of the item instead. ## NOTES diff --git a/NTFSSecurity/AuditCmdlets/AddAudit.cs b/NTFSSecurity/AuditCmdlets/AddAudit.cs index 8d97b54..9151a6f 100644 --- a/NTFSSecurity/AuditCmdlets/AddAudit.cs +++ b/NTFSSecurity/AuditCmdlets/AddAudit.cs @@ -169,7 +169,7 @@ namespace NTFSSecurity if (passThru == true) { - FileSystemAccessRule2.GetFileSystemAccessRules(sd, true, true).ForEach(ace => WriteObject(ace)); + FileSystemAuditRule2.GetFileSystemAuditRules(sd, true, true).ForEach(ace => WriteObject(ace)); } } } diff --git a/NTFSSecurity/AuditCmdlets/RemoveAudit.cs b/NTFSSecurity/AuditCmdlets/RemoveAudit.cs index 8ce78b0..6876297 100644 --- a/NTFSSecurity/AuditCmdlets/RemoveAudit.cs +++ b/NTFSSecurity/AuditCmdlets/RemoveAudit.cs @@ -162,7 +162,7 @@ namespace NTFSSecurity if (passThru == true) { - FileSystemAccessRule2.GetFileSystemAccessRules(item, true, true).ForEach(ace => WriteObject(ace)); + FileSystemAuditRule2.GetFileSystemAuditRules(item, true, true).ForEach(ace => WriteObject(ace)); } } } diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 942c0ec..a420e33 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -1490,10 +1490,10 @@ - Security2.FileSystemAccessRule2 + Security2.FileSystemAuditRule2 - Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the type depends on the parameter set: in the `Path` sets the cmdlet writes all audit entries of the item, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects, while in the `SecurityDescriptor` sets it writes the access entries of the descriptor as `Security2.FileSystemAccessRule2` objects. Use `Get-NTFSAudit` when you need the audit entries of a security descriptor. + Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `SecurityDescriptor` sets wrote the access entries of the descriptor instead. @@ -9083,10 +9083,10 @@ PS C:\Data> Get-NTFSSecurityDescriptor - Security2.FileSystemAccessRule2 + Security2.FileSystemAuditRule2 - Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the type depends on the parameter set: in the `Path` sets the cmdlet writes all access entries of the item, explicit and inherited ones, as `Security2.FileSystemAccessRule2` objects, while in the `SecurityDescriptor` sets it writes all audit entries of the descriptor as `Security2.FileSystemAuditRule2` objects. Use `Get-NTFSAudit` to check the audit entries of an item after the removal. + Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `Path` sets wrote the access entries of the item instead. diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 index 432cced..3ce0b6f 100644 --- a/Tests/Audit.Tests.ps1 +++ b/Tests/Audit.Tests.ps1 @@ -93,4 +93,33 @@ Describe 'Add-NTFSAudit' { @($rules | Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' }) | Should -HaveCount 1 } } + + Context 'With -PassThru' { + It 'Should return the audit entries of a security descriptor, not its access entries' { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'PassThru' + $sd = Get-NTFSSecurityDescriptor -Path $file + + $result = @(Add-NTFSAudit -SecurityDescriptor $sd -Account 'Everyone' -AccessRights ReadData -InheritanceFlags None -PropagationFlags None -PassThru) + + $result | Should -Not -BeNullOrEmpty + $result | ForEach-Object -Process { $_ | Should -BeOfType [Security2.FileSystemAuditRule2] } + @($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-1-0' }) | Should -HaveCount 1 + } + } +} + +Describe 'Remove-NTFSAudit' { + Context 'With -PassThru' { + It 'Should return the audit entries of the item, not its access entries' -Skip:(-not $canReadAudit) { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'PassThru' + Add-NTFSAudit -Path $file -Account 'Everyone' -AccessRights ReadData -InheritanceFlags None -PropagationFlags None + Add-NTFSAudit -Path $file -Account 'BUILTIN\Users' -AccessRights Delete -InheritanceFlags None -PropagationFlags None + + $result = @(Remove-NTFSAudit -Path $file -Account 'Everyone' -AccessRights ReadData -InheritanceFlags None -PropagationFlags None -PassThru) + + $result | Should -Not -BeNullOrEmpty + $result | ForEach-Object -Process { $_ | Should -BeOfType [Security2.FileSystemAuditRule2] } + @($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-5-32-545' }) | Should -HaveCount 1 + } + } } From 34250e53075672d3f41b54f66aefd1d750ff41a4 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:35:19 +0200 Subject: [PATCH 10/19] fix: take InheritanceEnabled of audit entries from the SACL Defect 7. FileSystemAuditRule2.GetFileSystemAuditRules set the InheritanceEnabled property of every audit entry from AreAccessRulesProtected, the protection of the DACL. It now uses AreAuditRulesProtected, so the property, and the "Inheritance enabled" header of the audit view, describe the audit entries. Tests/Audit.Tests.ps1: 1 test on an in-memory security descriptor whose SACL is protected and whose DACL is not. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 3 +++ Docs/Cmdlets/Get-NTFSAudit.md | 2 +- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 2 +- .../FileSystemAuditRule2.GetFileSystemAuditRules.cs | 2 +- Tests/Audit.Tests.ps1 | 12 ++++++++++++ 5 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc2071a..6c153da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,5 +71,8 @@ The format is based on - Fix `-PassThru` of `Add-NTFSAudit` with `-SecurityDescriptor` and of `Remove-NTFSAudit` with `-Path`, which returned access entries; both now return the audit entries +- Fix the `InheritanceEnabled` property of audit entries, which reported the + inheritance of the access entries; it now reports whether the audit + entries are inherited [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-NTFSAudit.md b/Docs/Cmdlets/Get-NTFSAudit.md index b010e7f..2b03198 100644 --- a/Docs/Cmdlets/Get-NTFSAudit.md +++ b/Docs/Cmdlets/Get-NTFSAudit.md @@ -183,7 +183,7 @@ Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. -Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. +Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. The `InheritanceEnabled` property of the entries also reported whether the access entries were inherited instead of the audit entries. ## RELATED LINKS diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index a420e33..993a3d0 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -4842,7 +4842,7 @@ PS C:\> Disable-Privileges When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well. If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. - Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. + Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. The `InheritanceEnabled` property of the entries also reported whether the access entries were inherited instead of the audit entries. diff --git a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs index 9e797b6..9fc525c 100644 --- a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs +++ b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.GetFileSystemAuditRules.cs @@ -31,7 +31,7 @@ namespace Security2 foreach (FileSystemAuditRule ace in acl) { - var ace2 = new FileSystemAuditRule2(ace) { FullName = sd.Item.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAccessRulesProtected }; + var ace2 = new FileSystemAuditRule2(ace) { FullName = sd.Item.FullName, InheritanceEnabled = !sd.SecurityDescriptor.AreAuditRulesProtected }; if (getInheritedFrom) { ace2.inheritedFrom = string.IsNullOrEmpty(inheritedFrom[aceCounter]) ? "" : inheritedFrom[aceCounter].Substring(0, inheritedFrom[aceCounter].Length - 1); diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 index 3ce0b6f..587ea5b 100644 --- a/Tests/Audit.Tests.ps1 +++ b/Tests/Audit.Tests.ps1 @@ -105,6 +105,18 @@ Describe 'Add-NTFSAudit' { $result | ForEach-Object -Process { $_ | Should -BeOfType [Security2.FileSystemAuditRule2] } @($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-1-0' }) | Should -HaveCount 1 } + + It 'Should report the inheritance of the audit entries in InheritanceEnabled, not that of the access entries' { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'AuditProtected' + $sd = Get-NTFSSecurityDescriptor -Path $file + $sd.SecurityDescriptor.SetAuditRuleProtection($true, $false) + + $result = @(Add-NTFSAudit -SecurityDescriptor $sd -Account 'Everyone' -AccessRights ReadData -InheritanceFlags None -PropagationFlags None -PassThru) + + $sd.SecurityDescriptor.AreAccessRulesProtected | Should -BeFalse + $result | Should -Not -BeNullOrEmpty + $result | ForEach-Object -Process { $_.InheritanceEnabled | Should -BeFalse } + } } } From 784beecdafa66e49946f5e1917b44203656d3747 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:37:23 +0200 Subject: [PATCH 11/19] fix: report unknown audit inheritance for a descriptor without SACL Defect 8. For a security descriptor that was read without its SACL (no Security privilege), Get-NTFSInheritance -SecurityDescriptor reported AuditInheritanceEnabled as $true, because the protection flag of a section that was never read is not set. It now reports $null, like the Path parameter set, using the sections that FileSystemSecurity2 records since defect 4. Set-NTFSInheritance -SecurityDescriptor reads the same state, so it no longer skips a requested audit change on such a descriptor as "equal". Tests/Inheritance.Tests.ps1: 2 tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 3 +++ Docs/Cmdlets/Get-NTFSInheritance.md | 4 ++- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 3 ++- .../FileSystem/FileSystemInheritanceInfo.cs | 9 ++++++- Tests/Inheritance.Tests.ps1 | 27 +++++++++++++++++++ 5 files changed, 43 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c153da..c7e42d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,5 +74,8 @@ The format is based on - Fix the `InheritanceEnabled` property of audit entries, which reported the inheritance of the access entries; it now reports whether the audit entries are inherited +- Fix `Get-NTFSInheritance -SecurityDescriptor`, which reported + `AuditInheritanceEnabled` as `$true` for a security descriptor that was + read without its audit section; it now reports `$null`, like `-Path` [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-NTFSInheritance.md b/Docs/Cmdlets/Get-NTFSInheritance.md index dcd0577..a9b0656 100644 --- a/Docs/Cmdlets/Get-NTFSInheritance.md +++ b/Docs/Cmdlets/Get-NTFSInheritance.md @@ -29,7 +29,7 @@ The `Get-NTFSInheritance` cmdlet reports whether a file or folder inherits acces `AccessInheritanceEnabled` is `$false` when the discretionary access control list (DACL) of the item is protected, which is the state that `Disable-NTFSAccessInheritance` produces. `AuditInheritanceEnabled` reports the same for the system access control list (SACL), which holds the audit rules. When the audit section cannot be read because the session does not hold the Security privilege, `AuditInheritanceEnabled` is `$null` and its column stays empty; the access value is still reported and no error is written. -In the `Path` parameter set the cmdlet reads the security descriptor of each item from disk. In the `SecurityDescriptor` parameter set it reads the state from the `Security2.FileSystemSecurity2` objects that `Get-NTFSSecurityDescriptor` returns, without touching the file system. Note that a descriptor that was retrieved without its audit section reports `AuditInheritanceEnabled` as `$true`, because the protection flag of a section that was never read is not set. +In the `Path` parameter set the cmdlet reads the security descriptor of each item from disk. In the `SecurityDescriptor` parameter set it reads the state from the `Security2.FileSystemSecurity2` objects that `Get-NTFSSecurityDescriptor` returns, without touching the file system. A descriptor that was read without its audit section, because the session doesn't hold the Security privilege, reports `AuditInheritanceEnabled` as `$null`, like the `Path` parameter set. `-Path` accepts pipeline input by value and by property name through its `FullName` alias, so the output of `Get-ChildItem`, `Get-ChildItem2`, and `Get-Item2` binds to it. Relative paths are resolved against the current location, and when no path is supplied at all, the cmdlet reports the current location. @@ -130,6 +130,8 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva Reading the audit section (SACL) of an item requires the Security privilege (`SeSecurityPrivilege`), which an account can only use in an elevated session. Without it, the cmdlet still reports the access state and sets `AuditInheritanceEnabled` to `$null` instead of writing an error. +Before 5.0.0, a security descriptor that was read without its audit section reported `AuditInheritanceEnabled` as `$true`. + If the security descriptor of an item cannot be opened because the account has no permission to it, the cmdlet takes ownership of the item, reads the state, 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. diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 993a3d0..57ec507 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -5329,7 +5329,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd The `Get-NTFSInheritance` cmdlet reports whether a file or folder inherits access rules from its parent folder and whether it inherits audit rules. For each item it writes one `Security2.FileSystemInheritanceInfo` object with the `Name`, `FullName`, `AccessInheritanceEnabled`, and `AuditInheritanceEnabled` properties, plus the underlying file system object in the `Item` property. The default table view shows `Name`, `AccessInheritanceEnabled`, and `AuditInheritanceEnabled`. `AccessInheritanceEnabled` is `$false` when the discretionary access control list (DACL) of the item is protected, which is the state that `Disable-NTFSAccessInheritance` produces. `AuditInheritanceEnabled` reports the same for the system access control list (SACL), which holds the audit rules. When the audit section cannot be read because the session does not hold the Security privilege, `AuditInheritanceEnabled` is `$null` and its column stays empty; the access value is still reported and no error is written. - In the `Path` parameter set the cmdlet reads the security descriptor of each item from disk. In the `SecurityDescriptor` parameter set it reads the state from the `Security2.FileSystemSecurity2` objects that `Get-NTFSSecurityDescriptor` returns, without touching the file system. Note that a descriptor that was retrieved without its audit section reports `AuditInheritanceEnabled` as `$true`, because the protection flag of a section that was never read is not set. + In the `Path` parameter set the cmdlet reads the security descriptor of each item from disk. In the `SecurityDescriptor` parameter set it reads the state from the `Security2.FileSystemSecurity2` objects that `Get-NTFSSecurityDescriptor` returns, without touching the file system. A descriptor that was read without its audit section, because the session doesn't hold the Security privilege, reports `AuditInheritanceEnabled` as `$null`, like the `Path` parameter set. `-Path` accepts pipeline input by value and by property name through its `FullName` alias, so the output of `Get-ChildItem`, `Get-ChildItem2`, and `Get-Item2` binds to it. Relative paths are resolved against the current location, and when no path is supplied at all, the cmdlet reports the current location. @@ -5426,6 +5426,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. Reading the audit section (SACL) of an item requires the Security privilege (`SeSecurityPrivilege`), which an account can only use in an elevated session. Without it, the cmdlet still reports the access state and sets `AuditInheritanceEnabled` to `$null` instead of writing an error. + Before 5.0.0, a security descriptor that was read without its audit section reported `AuditInheritanceEnabled` as `$true`. If the security descriptor of an item cannot be opened because the account has no permission to it, the cmdlet takes ownership of the item, reads the state, 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. diff --git a/Security2/FileSystem/FileSystemInheritanceInfo.cs b/Security2/FileSystem/FileSystemInheritanceInfo.cs index 1b822c9..81163db 100644 --- a/Security2/FileSystem/FileSystemInheritanceInfo.cs +++ b/Security2/FileSystem/FileSystemInheritanceInfo.cs @@ -94,7 +94,14 @@ namespace Security2 public static FileSystemInheritanceInfo GetFileSystemInheritanceInfo(FileSystemSecurity2 sd) { - return new FileSystemInheritanceInfo(sd.Item, !sd.SecurityDescriptor.AreAccessRulesProtected, !sd.SecurityDescriptor.AreAuditRulesProtected); + // Like for an item, the audit state is unknown when the SACL was not read. + bool? auditInheritanceEnabled = null; + if (sd.HasAuditSection) + { + auditInheritanceEnabled = !sd.SecurityDescriptor.AreAuditRulesProtected; + } + + return new FileSystemInheritanceInfo(sd.Item, !sd.SecurityDescriptor.AreAccessRulesProtected, auditInheritanceEnabled); } #endregion GetFileSystemInheritanceInfo diff --git a/Tests/Inheritance.Tests.ps1 b/Tests/Inheritance.Tests.ps1 index 8503a53..ddcde8e 100644 --- a/Tests/Inheritance.Tests.ps1 +++ b/Tests/Inheritance.Tests.ps1 @@ -26,6 +26,33 @@ AfterAll { Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue } +Describe 'Get-NTFSInheritance' { + Context 'With a security descriptor' { + BeforeEach { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Descriptor' + } + + It 'Should report the same state as for the path of the item' { + $byPath = Get-NTFSInheritance -Path $file + $bySecurityDescriptor = Get-NTFSInheritance -SecurityDescriptor (Get-NTFSSecurityDescriptor -Path $file) + + $bySecurityDescriptor.AccessInheritanceEnabled | Should -Be $byPath.AccessInheritanceEnabled + $bySecurityDescriptor.AuditInheritanceEnabled | Should -Be $byPath.AuditInheritanceEnabled + } + + It 'Should report the audit inheritance as $null for a security descriptor without the audit entries' { + $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList ( + (Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Access + ) + + $result = Get-NTFSInheritance -SecurityDescriptor $sd + + $result.AccessInheritanceEnabled | Should -BeTrue + $result.AuditInheritanceEnabled | Should -BeNullOrEmpty + } + } +} + Describe 'Set-NTFSInheritance' { Context 'When -AccessInheritanceEnabled or -AuditInheritanceEnabled is omitted' { BeforeEach { From c00bc6af881e58fe85b880da91b202eddba2c118 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:40:15 +0200 Subject: [PATCH 12/19] fix: let a stopped pipeline end Get-NTFSOwner without errors Defect 9, both parts: - Get-NTFSOwner wrote each owner inside a try block whose catch-all turned every exception into a ReadSecurityError. When a command such as Select-Object -First 1 stopped the pipeline, the cmdlet wrote "The pipeline has been stopped" as an error for every path, and the stop escaped into the caller's script. The owner is now written after the try block. - After access was denied, the retry called the same failing GetOwner again before taking ownership, so it always failed and reported a WriteError. Taking ownership would also replace the owner that the cmdlet reports. The cmdlet now writes a ReadSecurityError with the category PermissionDenied and continues. Tests/Owner.Tests.ps1 (new): 2 tests; the access-denied test skips 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 --- CHANGELOG.md | 4 ++ Docs/Cmdlets/Get-NTFSOwner.md | 4 ++ NTFSSecurity/OwnerCmdlets/GetOwner.cs | 31 ++++------- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 2 + Tests/Owner.Tests.ps1 | 54 ++++++++++++++++++++ 5 files changed, 75 insertions(+), 20 deletions(-) create mode 100644 Tests/Owner.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index c7e42d2..814b06c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,5 +77,9 @@ The format is based on - Fix `Get-NTFSInheritance -SecurityDescriptor`, which reported `AuditInheritanceEnabled` as `$true` for a security descriptor that was read without its audit section; it now reports `$null`, like `-Path` +- Fix `Get-NTFSOwner`, which wrote a "The pipeline has been stopped" error + for every path when a command such as `Select-Object -First 1` stopped the + pipeline, and which repeated a failed read instead of reporting the + denied access [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-NTFSOwner.md b/Docs/Cmdlets/Get-NTFSOwner.md index bd23beb..ccafc18 100644 --- a/Docs/Cmdlets/Get-NTFSOwner.md +++ b/Docs/Cmdlets/Get-NTFSOwner.md @@ -127,6 +127,10 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva The module also adds an `Owner` script property to `System.IO.FileInfo` and `System.IO.DirectoryInfo`, so `(Get-Item C:\Data).Owner` returns the owning account as a `Security2.IdentityReference2` object as well. +If the owner of an item cannot be read because access is denied, the cmdlet writes the non-terminating error `ReadSecurityError` with the category `PermissionDenied` and continues with the next path. It does not take ownership of the item, which would replace the owner that it reports. + +Before 5.0.0, a command that stopped the pipeline early, such as `Select-Object -First 1`, made the cmdlet write a `ReadSecurityError` with the message "The pipeline has been stopped" for every path. + ## RELATED LINKS [Set-NTFSOwner](Set-NTFSOwner.md) diff --git a/NTFSSecurity/OwnerCmdlets/GetOwner.cs b/NTFSSecurity/OwnerCmdlets/GetOwner.cs index b19e259..3c7b524 100644 --- a/NTFSSecurity/OwnerCmdlets/GetOwner.cs +++ b/NTFSSecurity/OwnerCmdlets/GetOwner.cs @@ -43,10 +43,11 @@ namespace NTFSSecurity.OwnerCmdlets { if (ParameterSetName == "Path") { - FileSystemInfo item = null; - foreach (var path in paths) { + FileSystemInfo item = null; + FileSystemOwner owner = null; + try { item = GetFileSystemInfo2(path); @@ -59,32 +60,22 @@ namespace NTFSSecurity.OwnerCmdlets try { - WriteObject(FileSystemOwner.GetOwner(item)); + owner = FileSystemOwner.GetOwner(item); } - catch (UnauthorizedAccessException) + catch (UnauthorizedAccessException ex) { - try - { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - WriteObject(FileSystemOwner.GetOwner(item)); - - FileSystemOwner.SetOwner(item, previousOwner); - } - catch (Exception ex2) - { - WriteError(new ErrorRecord(ex2, "ReadSecurityError", ErrorCategory.WriteError, path)); - continue; - } + // Taking ownership to read the owner would replace the owner that the cmdlet reports. + WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.PermissionDenied, path)); + continue; } catch (Exception ex) { WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.OpenError, path)); continue; } + + // Outside the try block, so that a stopped pipeline isn't reported as a read error. + WriteObject(owner); } } else diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 57ec507..c777abf 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -6146,6 +6146,8 @@ PS C:\> $orphaned | Select-Object FullName, Account, AccessRights, AuditFlags When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. The module also adds an `Owner` script property to `System.IO.FileInfo` and `System.IO.DirectoryInfo`, so `(Get-Item C:\Data).Owner` returns the owning account as a `Security2.IdentityReference2` object as well. + If the owner of an item cannot be read because access is denied, the cmdlet writes the non-terminating error `ReadSecurityError` with the category `PermissionDenied` and continues with the next path. It does not take ownership of the item, which would replace the owner that it reports. + Before 5.0.0, a command that stopped the pipeline early, such as `Select-Object -First 1`, made the cmdlet write a `ReadSecurityError` with the message "The pipeline has been stopped" for every path. diff --git a/Tests/Owner.Tests.ps1 b/Tests/Owner.Tests.ps1 new file mode 100644 index 0000000..9c5d993 --- /dev/null +++ b/Tests/Owner.Tests.ps1 @@ -0,0 +1,54 @@ +<# + Tests the owner cmdlets of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeDiscovery { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + # With the Backup privilege, Windows may grant reading the owner despite a deny entry. + $canBypassDeny = Test-PrivilegeHeld -Name 'SeBackupPrivilege' +} + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'Owner' + Push-Location -LiteralPath $sandbox +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Get-NTFSOwner' { + Context 'When a downstream command stops the pipeline' { + It 'Should stop without writing errors' { + $files = 1..3 | ForEach-Object -Process { New-TestSandboxItem -Sandbox $sandbox -Name "Owner$_" } + + $result = @(Get-NTFSOwner -Path $files -ErrorVariable ownerErrors -ErrorAction SilentlyContinue | Select-Object -First 1) + + $ownerErrors | Should -BeNullOrEmpty + $result | Should -HaveCount 1 + } + } + + Context 'When the owner cannot be read' { + It 'Should write one permission error and keep the owner' -Skip:$canBypassDeny { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Denied' + Block-TestReadPermission -Sandbox $sandbox -Path $file + + $result = @(Get-NTFSOwner -Path $file -ErrorVariable ownerErrors -ErrorAction SilentlyContinue) + + $result | Should -BeNullOrEmpty + $ownerErrors | Should -HaveCount 1 + $ownerErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadSecurityError,*' + $ownerErrors[0].CategoryInfo.Category | Should -Be 'PermissionDenied' + } + } +} From 03cc896bccbe5bae007e5154b761d6ec19fdd359 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:42:30 +0200 Subject: [PATCH 13/19] fix: create the destination folder before Copy-Item2 copies a folder Defect 10. AlphaFS 2.2 copies a folder only into an existing destination folder; otherwise DirectoryInfo.CopyTo fails with a DirectoryNotFoundException for the first file, so Copy-Item2 could not copy a folder that contained files. The cmdlet now creates the destination folder first; AlphaFS then copies the files and subfolders. Tests/ItemCmdlets.Tests.ps1: 1 test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 2 ++ Docs/Cmdlets/Copy-Item2.md | 2 +- NTFSSecurity/ItemCmdlets/CopyItem2.cs | 3 +++ NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 2 +- Tests/ItemCmdlets.Tests.ps1 | 25 ++++++++++++++++++++ 5 files changed, 32 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 814b06c..298c734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,5 +81,7 @@ The format is based on for every path when a command such as `Select-Object -First 1` stopped the pipeline, and which repeated a failed read instead of reporting the denied access +- Fix `Copy-Item2`, which failed with a `DirectoryNotFoundException` when it + copied a folder that contained files [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Copy-Item2.md b/Docs/Cmdlets/Copy-Item2.md index 4c93df3..025dd6d 100644 --- a/Docs/Cmdlets/Copy-Item2.md +++ b/Docs/Cmdlets/Copy-Item2.md @@ -184,7 +184,7 @@ By default this cmdlet returns nothing. With `-PassThru $true` it returns an `Al `Copy-Item2` copies through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), which is why it handles source and destination paths that exceed the 260-character `MAX_PATH` limit of the built-in `Copy-Item` cmdlet. -Copying a folder that contains files currently fails with a `CopyError` that reports a `DirectoryNotFoundException` for the first file in the folder. Copy files individually, for example by piping `Get-ChildItem2 -Recurse -File` into this cmdlet, and create the target folders beforehand. +Before 5.0.0, copying a folder that contained files failed with a `CopyError` that reported a `DirectoryNotFoundException` for the first file in the folder. If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. diff --git a/NTFSSecurity/ItemCmdlets/CopyItem2.cs b/NTFSSecurity/ItemCmdlets/CopyItem2.cs index 79d4067..8e27a1b 100644 --- a/NTFSSecurity/ItemCmdlets/CopyItem2.cs +++ b/NTFSSecurity/ItemCmdlets/CopyItem2.cs @@ -105,6 +105,9 @@ namespace NTFSSecurity { if (ShouldProcess(resolvedPath, "Copy Directory")) { + // AlphaFS 2.2 copies into an existing folder only and otherwise fails with a + // DirectoryNotFoundException for the first file. + Directory.CreateDirectory(actualDestination); ((DirectoryInfo)item).CopyTo(actualDestination, force ? CopyOptions.None : CopyOptions.FailIfExists, PathFormat.RelativePath); WriteVerbose(string.Format("Directory '{0}' copied to '{0}'", resolvedPath, destination)); } diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index c777abf..4380a03 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -2173,7 +2173,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd `Copy-Item2` copies through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), which is why it handles source and destination paths that exceed the 260-character `MAX_PATH` limit of the built-in `Copy-Item` cmdlet. - Copying a folder that contains files currently fails with a `CopyError` that reports a `DirectoryNotFoundException` for the first file in the folder. Copy files individually, for example by piping `Get-ChildItem2 -Recurse -File` into this cmdlet, and create the target folders beforehand. + Before 5.0.0, copying a folder that contained files failed with a `CopyError` that reported a `DirectoryNotFoundException` for the first file in the folder. If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. diff --git a/Tests/ItemCmdlets.Tests.ps1 b/Tests/ItemCmdlets.Tests.ps1 index 75af9ec..5534a0e 100644 --- a/Tests/ItemCmdlets.Tests.ps1 +++ b/Tests/ItemCmdlets.Tests.ps1 @@ -55,3 +55,28 @@ Describe 'Get-ChildItem2' { } } } + +Describe 'Copy-Item2' { + Context 'When -Path is a folder with files and subfolders' { + BeforeAll { + $source = New-TestSandboxItem -Sandbox $sandbox -Name 'Source' -Directory + $sourceFile = Join-Path -Path $source -ChildPath 'File.txt' + $sourceSubfolderFile = Join-Path -Path $source -ChildPath 'Subfolder\Other.txt' + Assert-TestSandboxPath -Sandbox $sandbox -Path $sourceFile, $sourceSubfolderFile + New-Item -ItemType Directory -Path (Split-Path -Path $sourceSubfolderFile -Parent) | Out-Null + Set-Content -LiteralPath $sourceFile -Value 'File' + Set-Content -LiteralPath $sourceSubfolderFile -Value 'Other' + } + + It 'Should copy the folder with its files and subfolders' { + $destination = Join-Path -Path $sandbox -ChildPath ('Copy-{0}' -f [guid]::NewGuid().ToString('N').Substring(0, 8)) + Assert-TestSandboxPath -Sandbox $sandbox -Path $destination + + Copy-Item2 -Path $source -Destination $destination -ErrorVariable copyErrors -ErrorAction SilentlyContinue + + $copyErrors | Should -BeNullOrEmpty + Join-Path -Path $destination -ChildPath 'File.txt' | Should -Exist + Join-Path -Path $destination -ChildPath 'Subfolder\Other.txt' | Should -Exist + } + } +} From 8a80f26717a58f12d87f2b480e68893d8931d344 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:43:06 +0200 Subject: [PATCH 14/19] docs: drop the advice to always supply both Set-NTFSInheritance values Completes the documentation of defect 1: the descriptions of -AccessInheritanceEnabled and -AuditInheritanceEnabled now say that an omitted value leaves its section unchanged, instead of asking to always supply the parameter to avoid "Nullable object must have a value". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- Docs/Cmdlets/Set-NTFSInheritance.md | 4 ++-- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Docs/Cmdlets/Set-NTFSInheritance.md b/Docs/Cmdlets/Set-NTFSInheritance.md index 6eb05e6..51c45c2 100644 --- a/Docs/Cmdlets/Set-NTFSInheritance.md +++ b/Docs/Cmdlets/Set-NTFSInheritance.md @@ -77,7 +77,7 @@ The first two commands read the security descriptor and change its inheritance i ### -AccessInheritanceEnabled -Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. Always supply this parameter, because the cmdlet fails with "Nullable object must have a value" when it has to compare against a value that was not provided. +Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the access section is left unchanged. ```yaml Type: Boolean @@ -93,7 +93,7 @@ Accept wildcard characters: False ### -AuditInheritanceEnabled -Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. Reading and writing the audit section requires the Security privilege and therefore an elevated session. +Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the audit section is left unchanged. Reading and writing the audit section requires the Security privilege and therefore an elevated session. ```yaml Type: Boolean diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 4380a03..daf7f72 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -9197,7 +9197,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd AccessInheritanceEnabled - Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. Always supply this parameter, because the cmdlet fails with "Nullable object must have a value" when it has to compare against a value that was not provided. + Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the access section is left unchanged. Boolean @@ -9209,7 +9209,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd AuditInheritanceEnabled - Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. Reading and writing the audit section requires the Security privilege and therefore an elevated session. + Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the audit section is left unchanged. Reading and writing the audit section requires the Security privilege and therefore an elevated session. Boolean @@ -9249,7 +9249,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd AccessInheritanceEnabled - Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. Always supply this parameter, because the cmdlet fails with "Nullable object must have a value" when it has to compare against a value that was not provided. + Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the access section is left unchanged. Boolean @@ -9261,7 +9261,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd AuditInheritanceEnabled - Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. Reading and writing the audit section requires the Security privilege and therefore an elevated session. + Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the audit section is left unchanged. Reading and writing the audit section requires the Security privilege and therefore an elevated session. Boolean @@ -9287,7 +9287,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd AccessInheritanceEnabled - Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. Always supply this parameter, because the cmdlet fails with "Nullable object must have a value" when it has to compare against a value that was not provided. + Specifies whether the item inherits access rules from its parent folder. `$true` removes the protection from the DACL and keeps the access rules that are stored directly on the item; `$false` protects the DACL and discards the rules the item currently inherits, which leaves only its explicit rules. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the access section is left unchanged. Boolean @@ -9299,7 +9299,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd AuditInheritanceEnabled - Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. Reading and writing the audit section requires the Security privilege and therefore an elevated session. + Specifies whether the item inherits audit rules from its parent folder. `$true` removes the protection from the SACL and removes the audit rules that are stored directly on the item; `$false` protects the SACL and copies the inherited audit rules into it. The section is left untouched when the requested value already matches the current state. When you omit the parameter, the audit section is left unchanged. Reading and writing the audit section requires the Security privilege and therefore an elevated session. Boolean From 804cfd292e0eb74adf20be860a35f92b038eb531 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:46:04 +0200 Subject: [PATCH 15/19] fix: let Disable-Privileges work when EnablePrivileges is $false Defect 11. DisableFileSystemPrivileges read the privileges of the token into a local variable that hid the field, and DisablePrivilege read the field. With the module setting EnablePrivileges = $false, BeginProcessing never filled the field, so every DisablePrivilege call hit a null reference, which TryDisablePrivilege turned into a warning, and the privileges stayed enabled. The method now refreshes the field. Tests/Privileges.Tests.ps1 (new): 1 test, CI-only, because it needs a token that holds the privileges. Without the fix it fails on the warnings and on the Backup privilege that stays enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 2 + Docs/Cmdlets/Disable-Privileges.md | 2 + NTFSSecurity/BaseCmdlets.cs | 3 +- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 1 + Tests/Privileges.Tests.ps1 | 63 ++++++++++++++++++++ 5 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 Tests/Privileges.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 298c734..400aaba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,5 +83,7 @@ The format is based on denied access - Fix `Copy-Item2`, which failed with a `DirectoryNotFoundException` when it copied a folder that contained files +- Fix `Disable-Privileges`, which couldn't disable the privileges when the + module setting `EnablePrivileges` was `$false` [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Disable-Privileges.md b/Docs/Cmdlets/Disable-Privileges.md index 97b1fcc..0d48b8f 100644 --- a/Docs/Cmdlets/Disable-Privileges.md +++ b/Docs/Cmdlets/Disable-Privileges.md @@ -99,6 +99,8 @@ With `-PassThru`, the cmdlet writes the privilege collection of the current proc When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), the file system cmdlets of the module try to enable the Backup, Restore, Take Ownership, and Security privileges while they run and disable the privileges they enabled when they finish. You therefore need `Disable-Privileges` only after an explicit `Enable-Privileges`. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. +Before 5.0.0, when `EnablePrivileges` was `$false`, the cmdlet wrote warnings that it could not disable the privileges and left them enabled. + ## RELATED LINKS [Enable-Privileges](Enable-Privileges.md) diff --git a/NTFSSecurity/BaseCmdlets.cs b/NTFSSecurity/BaseCmdlets.cs index 7480a2a..0a5eb49 100644 --- a/NTFSSecurity/BaseCmdlets.cs +++ b/NTFSSecurity/BaseCmdlets.cs @@ -278,7 +278,8 @@ namespace NTFSSecurity protected void DisableFileSystemPrivileges() { - var privileges = privControl.GetPrivileges(); + // Refreshes the field that DisablePrivilege reads; it is null when BeginProcessing enabled nothing. + privileges = privControl.GetPrivileges(); if (privileges.Where(p => p.Privilege == Privilege.TakeOwnership) != null) if (!TryDisablePrivilege(Privilege.TakeOwnership)) diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index daf7f72..5731ff5 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -2788,6 +2788,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), the file system cmdlets of the module try to enable the Backup, Restore, Take Ownership, and Security privileges while they run and disable the privileges they enabled when they finish. You therefore need `Disable-Privileges` only after an explicit `Enable-Privileges`. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. + Before 5.0.0, when `EnablePrivileges` was `$false`, the cmdlet wrote warnings that it could not disable the privileges and left them enabled. diff --git a/Tests/Privileges.Tests.ps1 b/Tests/Privileges.Tests.ps1 new file mode 100644 index 0000000..5070758 --- /dev/null +++ b/Tests/Privileges.Tests.ps1 @@ -0,0 +1,63 @@ +<# + Tests how the cmdlets of the module built in NTFSSecurity\bin\Release handle the Backup, Restore, Take Ownership, + and Security privileges. These tests need an access token that holds the privileges, so they skip without them + and run in CI, whose runners are elevated. They change only the privileges of the test process and restore the + module setting EnablePrivileges. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeDiscovery { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $missingPrivileges = @('SeBackupPrivilege', 'SeRestorePrivilege', 'SeTakeOwnershipPrivilege', 'SeSecurityPrivilege') | + Where-Object -FilterScript { -not (Test-PrivilegeHeld -Name $_) } + $holdsPrivileges = -not $missingPrivileges +} + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'Privileges' + Push-Location -LiteralPath $sandbox + + $privateData = (Get-Module -Name NTFSSecurity).PrivateData + $enablePrivileges = $privateData['EnablePrivileges'] + + function Get-BackupPrivilegeState { + (Get-Privileges | Where-Object -Property Privilege -EQ -Value 'Backup').PrivilegeState + } +} + +AfterAll { + $privateData['EnablePrivileges'] = $enablePrivileges + Disable-Privileges -ErrorAction SilentlyContinue -WarningAction SilentlyContinue + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Disable-Privileges' { + Context 'When the module setting EnablePrivileges is $false' { + BeforeAll { + $privateData['EnablePrivileges'] = $false + } + + AfterAll { + $privateData['EnablePrivileges'] = $enablePrivileges + } + + # Before 5.0.0, the cmdlet warned that it could not disable the privileges and left them enabled. + It 'Should disable the privileges that Enable-Privileges enabled' -Skip:(-not $holdsPrivileges) { + Enable-Privileges + Get-BackupPrivilegeState | Should -Be 'Enabled' + + Disable-Privileges -WarningVariable privilegeWarnings -WarningAction SilentlyContinue + + $privilegeWarnings | Should -BeNullOrEmpty + Get-BackupPrivilegeState | Should -Be 'Disabled' + } + } +} From 7627f8c0812df7b432d69e99f2625da8e53a5be0 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:48:17 +0200 Subject: [PATCH 16/19] fix: enable privileges in the inheritance cmdlets only when configured Defect 12. Besides the base class, which enables the privileges only when the module setting EnablePrivileges is $true and disables them again in EndProcessing, the six inheritance cmdlets called EnableFileSystemPrivileges in BeginProcessing unconditionally. With EnablePrivileges = $false they enabled the privileges anyway and, because EndProcessing disables them only when the setting is $true, left them enabled. The extra calls are gone; the inheritance cmdlets now behave like the other cmdlets. Tests/Privileges.Tests.ps1: 6 tests, one per cmdlet, CI-only, because they need a token that holds the privileges. Without the fix the Backup privilege is enabled after each call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 3 +++ Docs/Cmdlets/Disable-NTFSAccessInheritance.md | 2 ++ Docs/Cmdlets/Disable-NTFSAuditInheritance.md | 2 ++ Docs/Cmdlets/Enable-NTFSAccessInheritance.md | 2 ++ Docs/Cmdlets/Enable-NTFSAuditInheritance.md | 2 ++ Docs/Cmdlets/Get-NTFSInheritance.md | 2 ++ Docs/Cmdlets/Set-NTFSInheritance.md | 2 ++ Docs/Concepts.md | 6 ++--- .../DisableAccessInheritance.cs | 1 - .../DisableAuditInheritance.cs | 1 - .../EnableAccessInheritance.cs | 1 - .../EnableAuditInheritance.cs | 1 - .../InheritanceCmdlets/GetInheritance.cs | 1 - .../InheritanceCmdlets/SetInheritance.cs | 1 - NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 6 +++++ Tests/Privileges.Tests.ps1 | 25 +++++++++++++++++++ 16 files changed, 49 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 400aaba..16e8e40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,5 +85,8 @@ The format is based on copied a folder that contained files - Fix `Disable-Privileges`, which couldn't disable the privileges when the module setting `EnablePrivileges` was `$false` +- Fix the inheritance cmdlets, which enabled the Backup, Restore, Take + Ownership, and Security privileges even when the module setting + `EnablePrivileges` was `$false`, and left them enabled [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 6c9d98b..4166f56 100644 --- a/Docs/Cmdlets/Disable-NTFSAccessInheritance.md +++ b/Docs/Cmdlets/Disable-NTFSAccessInheritance.md @@ -168,6 +168,8 @@ Blocking access inheritance requires permission to change the DACL of the 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. + ## RELATED LINKS [Enable-NTFSAccessInheritance](Enable-NTFSAccessInheritance.md) diff --git a/Docs/Cmdlets/Disable-NTFSAuditInheritance.md b/Docs/Cmdlets/Disable-NTFSAuditInheritance.md index 59d9246..3e08779 100644 --- a/Docs/Cmdlets/Disable-NTFSAuditInheritance.md +++ b/Docs/Cmdlets/Disable-NTFSAuditInheritance.md @@ -170,6 +170,8 @@ If the descriptor cannot be opened because the account has no permission to the 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. + ## RELATED LINKS [Enable-NTFSAuditInheritance](Enable-NTFSAuditInheritance.md) diff --git a/Docs/Cmdlets/Enable-NTFSAccessInheritance.md b/Docs/Cmdlets/Enable-NTFSAccessInheritance.md index 2d8fb50..dc029fc 100644 --- a/Docs/Cmdlets/Enable-NTFSAccessInheritance.md +++ b/Docs/Cmdlets/Enable-NTFSAccessInheritance.md @@ -167,6 +167,8 @@ Restoring access inheritance requires permission to change the DACL of the 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. + ## RELATED LINKS [Disable-NTFSAccessInheritance](Disable-NTFSAccessInheritance.md) diff --git a/Docs/Cmdlets/Enable-NTFSAuditInheritance.md b/Docs/Cmdlets/Enable-NTFSAuditInheritance.md index fec40fe..f7c7704 100644 --- a/Docs/Cmdlets/Enable-NTFSAuditInheritance.md +++ b/Docs/Cmdlets/Enable-NTFSAuditInheritance.md @@ -169,6 +169,8 @@ If the descriptor cannot be opened because the account has no permission to the 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. + ## RELATED LINKS [Disable-NTFSAuditInheritance](Disable-NTFSAuditInheritance.md) diff --git a/Docs/Cmdlets/Get-NTFSInheritance.md b/Docs/Cmdlets/Get-NTFSInheritance.md index a9b0656..e2d67ea 100644 --- a/Docs/Cmdlets/Get-NTFSInheritance.md +++ b/Docs/Cmdlets/Get-NTFSInheritance.md @@ -136,6 +136,8 @@ If the security descriptor of an item cannot be opened because the account has n 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. + ## RELATED LINKS [Set-NTFSInheritance](Set-NTFSInheritance.md) diff --git a/Docs/Cmdlets/Set-NTFSInheritance.md b/Docs/Cmdlets/Set-NTFSInheritance.md index 51c45c2..f9986a8 100644 --- a/Docs/Cmdlets/Set-NTFSInheritance.md +++ b/Docs/Cmdlets/Set-NTFSInheritance.md @@ -194,6 +194,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti 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. + ## RELATED LINKS [Get-NTFSInheritance](Get-NTFSInheritance.md) diff --git a/Docs/Concepts.md b/Docs/Concepts.md index d51cb0d..b9237bf 100644 --- a/Docs/Concepts.md +++ b/Docs/Concepts.md @@ -188,9 +188,9 @@ The access, audit, inheritance, owner, and security descriptor cmdlets enable these privileges automatically while they run and disable the ones they enabled when they finish. If a privilege cannot be enabled, the cmdlet continues without it. You can turn this behavior off with the -`EnablePrivileges` module setting. The inheritance cmdlets are an exception: -they always try to enable the privileges, and when `EnablePrivileges` is -`$false`, they leave them enabled. +`EnablePrivileges` module setting. Before 5.0.0, the inheritance cmdlets were +an exception: they always tried to enable the privileges, and when +`EnablePrivileges` was `$false`, they left them enabled. `Enable-Privileges` enables the four privileges for the current PowerShell process until you run `Disable-Privileges` or close the session. diff --git a/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs b/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs index 235a49b..d196fb7 100644 --- a/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs @@ -53,7 +53,6 @@ namespace NTFSSecurity protected override void BeginProcessing() { base.BeginProcessing(); - EnableFileSystemPrivileges(true); } protected override void ProcessRecord() diff --git a/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs b/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs index e8c7769..ac603cf 100644 --- a/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs @@ -54,7 +54,6 @@ namespace NTFSSecurity protected override void BeginProcessing() { base.BeginProcessing(); - EnableFileSystemPrivileges(true); } protected override void ProcessRecord() diff --git a/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs b/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs index f7cb8b5..672b98f 100644 --- a/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs @@ -53,7 +53,6 @@ namespace NTFSSecurity protected override void BeginProcessing() { base.BeginProcessing(); - EnableFileSystemPrivileges(true); } protected override void ProcessRecord() diff --git a/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs b/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs index c25617b..3f29546 100644 --- a/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs @@ -53,7 +53,6 @@ namespace NTFSSecurity protected override void BeginProcessing() { base.BeginProcessing(); - EnableFileSystemPrivileges(true); } protected override void ProcessRecord() diff --git a/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs b/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs index 895bf79..df19f8d 100644 --- a/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs @@ -38,7 +38,6 @@ namespace NTFSSecurity protected override void BeginProcessing() { base.BeginProcessing(); - EnableFileSystemPrivileges(true); if (paths.Count == 0) { diff --git a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs index a8effe7..3877e43 100644 --- a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs @@ -61,7 +61,6 @@ namespace NTFSSecurity protected override void BeginProcessing() { base.BeginProcessing(); - EnableFileSystemPrivileges(true); } protected override void ProcessRecord() diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 5731ff5..0ba7b55 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -2412,6 +2412,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. 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. @@ -2656,6 +2657,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd The audit section of a security descriptor can only be read and written with the Security privilege (`SeSecurityPrivilege`), which an account can only use in an elevated session. Without it, the cmdlet writes a non-terminating error that reports Windows error 1314, "A required privilege is not held by the client", and the audit rules of the item stay unchanged. 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. @@ -3025,6 +3027,7 @@ PS C:\> Get-Privileges | Where-Object { $_.Privilege -in 'Backup', 'Restore', When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. 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. @@ -3269,6 +3272,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd The audit section of a security descriptor can only be read and written with the Security privilege (`SeSecurityPrivilege`), which an account can only use in an elevated session. Without it, the cmdlet writes a non-terminating error that reports Windows error 1314, "A required privilege is not held by the client", and the audit rules of the item stay unchanged. 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. @@ -5430,6 +5434,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd Before 5.0.0, a security descriptor that was read without its audit section reported `AuditInheritanceEnabled` as `$true`. If the security descriptor of an item cannot be opened because the account has no permission to it, the cmdlet takes ownership of the item, reads the state, 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. @@ -9391,6 +9396,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 changes, 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, 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. diff --git a/Tests/Privileges.Tests.ps1 b/Tests/Privileges.Tests.ps1 index 5070758..1ebfa5d 100644 --- a/Tests/Privileges.Tests.ps1 +++ b/Tests/Privileges.Tests.ps1 @@ -61,3 +61,28 @@ Describe 'Disable-Privileges' { } } } + +Describe 'Inheritance cmdlets' { + Context 'When the module setting EnablePrivileges is $false' { + BeforeAll { + $privateData['EnablePrivileges'] = $false + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Inheritance' + } + + AfterAll { + $privateData['EnablePrivileges'] = $enablePrivileges + } + + # Before 5.0.0, the inheritance cmdlets enabled the privileges anyway and left them enabled. + It '<_> should leave the privileges disabled' -Skip:(-not $holdsPrivileges) -ForEach @( + 'Get-NTFSInheritance', 'Set-NTFSInheritance', 'Enable-NTFSAccessInheritance', + 'Disable-NTFSAccessInheritance', 'Enable-NTFSAuditInheritance', 'Disable-NTFSAuditInheritance' + ) { + Get-BackupPrivilegeState | Should -Be 'Disabled' + + & $_ -Path $file -ErrorAction SilentlyContinue | Out-Null + + Get-BackupPrivilegeState | Should -Be 'Disabled' + } + } +} From f32f1923b3d536a995ccc2393d7e14b2abea49d8 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:50:43 +0200 Subject: [PATCH 17/19] fix: show the access inheritance of each item in the Inherits column Defect 13. The Inherits column of the Children2 view, which formats the Get-ChildItem2 output, negated the IsInheritanceBlocked property. The module defines that property for System.IO.FileInfo and DirectoryInfo only, not for the AlphaFS objects that Get-ChildItem2 returns, so the column showed !$null, that is True, for every item. The view now reads the protection of the DACL from the item. Tests/ItemCmdlets.Tests.ps1: 2 tests on the formatted output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 2 ++ Docs/Cmdlets/Get-ChildItem2.md | 2 ++ NTFSSecurity/NTFSSecurity.format.ps1xml | 2 +- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 1 + Tests/ItemCmdlets.Tests.ps1 | 24 ++++++++++++++++++++ 5 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16e8e40..76c05c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,5 +88,7 @@ The format is based on - Fix the inheritance cmdlets, which enabled the Backup, Restore, Take Ownership, and Security privileges even when the module setting `EnablePrivileges` was `$false`, and left them enabled +- Fix the `Inherits` column of the `Get-ChildItem2` output, which showed + `True` for every item, also for items whose inheritance is disabled [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-ChildItem2.md b/Docs/Cmdlets/Get-ChildItem2.md index 5ae3191..0bd8f8a 100644 --- a/Docs/Cmdlets/Get-ChildItem2.md +++ b/Docs/Cmdlets/Get-ChildItem2.md @@ -301,6 +301,8 @@ The cmdlet returns this object for every folder it finds. Depending on the modul The module defines the alias `dir2` for this cmdlet. +The default table view shows the `Mode`, `Inherits`, `LastWriteTime`, `Size(M)`, and `Name` columns. `Inherits` is `False` for an item whose access inheritance is disabled. Before 5.0.0, the column showed `True` for every item. + The `PrivateData` section of the module manifest `NTFSSecurity.psd1` contains two settings that this cmdlet reads when it starts. `GetFileSystemModeProperty` adds the calculated `Mode` property to every item. `IdentifyHardLinks` adds the `HardLinkCount` property to every file, which requires an extra call into the file system for each file and therefore slows down large listings noticeably. Set either value to `$false` in the manifest and import the module again if you prefer the faster enumeration over the additional properties. A folder that cannot be read produces a non-terminating error with the ID `DirUnauthorizedAccessError` for an access denial or `DirUnspecifiedError` for any other failure, and a path that does not exist produces the error `FileNotFound`. In each case the cmdlet continues with the next path. Failures that occur while `-Recurse` collects the subfolders of a folder are reported as verbose messages only, not as errors. diff --git a/NTFSSecurity/NTFSSecurity.format.ps1xml b/NTFSSecurity/NTFSSecurity.format.ps1xml index d931bff..455df7c 100644 --- a/NTFSSecurity/NTFSSecurity.format.ps1xml +++ b/NTFSSecurity/NTFSSecurity.format.ps1xml @@ -430,7 +430,7 @@ - !$_.IsInheritanceBlocked + -not $_.GetAccessControl([System.Security.AccessControl.AccessControlSections]::Access).AreAccessRulesProtected diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 0ba7b55..166bd71 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -3841,6 +3841,7 @@ PS C:\> Disable-Privileges `Get-ChildItem2` enumerates the file system through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), which is why it returns items whose path exceeds the 260-character `MAX_PATH` limit that the built-in `Get-ChildItem` cmdlet is bound to. The objects are AlphaFS objects, not `System.IO` objects, and the other NTFSSecurity cmdlets accept them directly because their `-Path` parameters have the alias `FullName`. The module defines the alias `dir2` for this cmdlet. + The default table view shows the `Mode`, `Inherits`, `LastWriteTime`, `Size(M)`, and `Name` columns. `Inherits` is `False` for an item whose access inheritance is disabled. Before 5.0.0, the column showed `True` for every item. The `PrivateData` section of the module manifest `NTFSSecurity.psd1` contains two settings that this cmdlet reads when it starts. `GetFileSystemModeProperty` adds the calculated `Mode` property to every item. `IdentifyHardLinks` adds the `HardLinkCount` property to every file, which requires an extra call into the file system for each file and therefore slows down large listings noticeably. Set either value to `$false` in the manifest and import the module again if you prefer the faster enumeration over the additional properties. A folder that cannot be read produces a non-terminating error with the ID `DirUnauthorizedAccessError` for an access denial or `DirUnspecifiedError` for any other failure, and a path that does not exist produces the error `FileNotFound`. In each case the cmdlet continues with the next path. Failures that occur while `-Recurse` collects the subfolders of a folder are reported as verbose messages only, not as errors. Before 5.0.0, a `-Path` value that points to a file stopped the cmdlet with an `InvalidCastException`. diff --git a/Tests/ItemCmdlets.Tests.ps1 b/Tests/ItemCmdlets.Tests.ps1 index 5534a0e..3a7ca2c 100644 --- a/Tests/ItemCmdlets.Tests.ps1 +++ b/Tests/ItemCmdlets.Tests.ps1 @@ -54,6 +54,30 @@ Describe 'Get-ChildItem2' { $items | Should -BeNullOrEmpty } } + + Context 'Default table view' { + BeforeAll { + $viewFolder = New-TestSandboxItem -Sandbox $sandbox -Name 'View' -Directory + $blocked = Join-Path -Path $viewFolder -ChildPath 'Blocked.txt' + $inheriting = Join-Path -Path $viewFolder -ChildPath 'Inheriting.txt' + Assert-TestSandboxPath -Sandbox $sandbox -Path $blocked, $inheriting + Set-Content -LiteralPath $blocked -Value 'Blocked' + Set-Content -LiteralPath $inheriting -Value 'Inheriting' + $acl = Get-Acl -LiteralPath $blocked + $acl.SetAccessRuleProtection($true, $true) + Set-Acl -LiteralPath $blocked -AclObject $acl + + $lines = Get-ChildItem2 -Path $viewFolder | Out-String -Stream -Width 200 + } + + It 'Should show False in the Inherits column for a file whose inheritance is disabled' { + ($lines | Where-Object -FilterScript { $_ -match 'Blocked\.txt\s*$' }) | Should -Match '\bFalse\b' + } + + It 'Should show True in the Inherits column for a file that inherits' { + ($lines | Where-Object -FilterScript { $_ -match 'Inheriting\.txt\s*$' }) | Should -Match '\bTrue\b' + } + } } Describe 'Copy-Item2' { From d282dfe32c087761d1f291fdc2f0e6beb240d6cf Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 00:54:26 +0200 Subject: [PATCH 18/19] chore(memory-bank): record group A and the shared test patterns - activeContext.md: the results of ai/defects-a and the next step. - progress.md: group A is fixed on ai/defects-a (not merged yet). - systemPatterns.md: the sandbox test helpers and how platyPS takes a parameter position from the shipped help file; older bullets condensed to stay within the line budget. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- .memory-bank/activeContext.md | 9 +++++- .memory-bank/progress.md | 2 +- .memory-bank/systemPatterns.md | 51 +++++++++++++++++----------------- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/.memory-bank/activeContext.md b/.memory-bank/activeContext.md index 90f556c..1cfd177 100644 --- a/.memory-bank/activeContext.md +++ b/.memory-bank/activeContext.md @@ -39,7 +39,14 @@ the PRs, and tags `5.0.0-rc2` after the merges. 261 passed, 7 skipped; PowerShell 7 232 passed, 36 skipped (268 tests). - `ai/maintenance` adds `Tests\Repository.Tests.ps1` (8 tests): Windows PowerShell 269 passed, 7 skipped; PowerShell 7 240 passed, 36 skipped. + Review: Dependabot PRs ran unreviewed actions in a job with + `contents: write`; the wiki preview is now read-only (`publish-wiki`). +- `ai/defects-a` fixes defects 1 to 13 and the same repeat bug in + `Get-NTFSAccess` (found with 4): Windows PowerShell 310 passed, 17 + skipped; PowerShell 7 280 passed, 47 skipped (327 tests). 10 tests need + privileges and run only in CI. ## Next step -Group A on `ai/defects-a`, starting with the shared test helpers. +Group B (14 to 17) on `ai/defects-b`, then C, D, the E decisions, and the +bugs from the issue triage (`ai/issue-fixes`). diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index 2f930e8..8d20ffb 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -93,7 +93,7 @@ authorization, restrict wiki editing to collaborators, and ask Numbered as agreed with the maintainer; each is documented on its page. -#### A: Crashes and wrong results +#### A: Crashes and wrong results (fixed on `ai/defects-a`, not merged) - (1) `Set-NTFSInheritance` reads an unset `Nullable` when `-AccessInheritanceEnabled` is omitted; omitted should mean unchanged. diff --git a/.memory-bank/systemPatterns.md b/.memory-bank/systemPatterns.md index aac475a..35d657d 100644 --- a/.memory-bank/systemPatterns.md +++ b/.memory-bank/systemPatterns.md @@ -66,32 +66,33 @@ Each Decision record is a file in `decisions/`; read only the relevant ones. - Run platyPS in Windows PowerShell 5.1 against a module build; a copy of `Docs/Cmdlets` must round-trip through `Update-MarkdownHelp` unchanged. platyPS rewrites non-ASCII punctuation, so keep cmdlet pages ASCII-only. -- GitHub renders the docs (Decision 9); CI publishes them to the wiki - (Decision 11). MarkdownLinkCheck: relative `Docs` links, no anchors; - `Tests\Wiki.Tests.ps1`: every wiki link and anchor (GitHub slug rules). - Neither covers the links in `README.md` and `CHANGELOG.md`. -- The wiki is generated from `Docs`; never edit the wiki. Pages are named - after their files, `Docs/README.md` becomes Home, and its cmdlet groups - form the sidebar; a cmdlet missing there fails `Wiki.Tests.ps1`. -- In cmdlet pages, end a sentence with a link: platyPS renders a link as - `text (url)` in the help file and drops the space after it. -- Verify examples in a `$env:TEMP` sandbox, never on real data; parse every - example and check its parameters against `Get-Command` metadata. + It takes a parameter's `Position` from `Get-Help`, that is from the shipped + help file: after a position change, edit the page YAML, run + `New-ExternalHelp`, rebuild, and check the round trip. +- Links: MarkdownLinkCheck checks relative `Docs` links (no anchors), + `Tests\Wiki.Tests.ps1` the wiki links and anchors; neither covers + `README.md` and `CHANGELOG.md`. The wiki is generated from `Docs` (never + edit it); `Docs/README.md` becomes Home, its cmdlet groups the sidebar. +- In cmdlet pages, end a sentence with a link (platyPS drops the space after + it). Verify examples in a `$env:TEMP` sandbox, never on real data. ### Testing the module - Pester 5 tests in `Tests/*.Tests.ps1` import - `NTFSSecurity\bin\Release\NTFSSecurity.psd1`; CI runs them in Windows - PowerShell 5.1 and in PowerShell 7 (Decision 11). -- `Get-Help -Online` tests use the internal hook `BypassOnlineHelpRetrieval` - (URI instead of a browser); it skips the help file in PowerShell 7, so - those 36 tests run only in Windows PowerShell. -- `.github/scripts/Invoke-Tests.ps1` runs Pester in CI: counts and failures - to the job summary, NUnit to `test-results`; failed test files fail too. -- `Tests\Manifest.Tests.ps1`: `Test-ModuleManifest` without errors or - warnings, exactly 36 cmdlets, one version in manifest and assemblies - (Decision 10). A new cmdlet updates `CmdletsToExport` and that count. -- `Tests\Release.Tests.ps1` checks that `CHANGELOG.md` has release notes - for the manifest version (dated section, or `[Unreleased]` for a - prerelease) and the packages: only `FileList` files, version with label, - command tags, and `NTFSSecurity.zip` with the module folder. + `NTFSSecurity\bin\Release\NTFSSecurity.psd1`; CI runs every file of + `Tests` in Windows PowerShell 5.1 and in PowerShell 7 (Decision 11) with + `.github/scripts/Invoke-Tests.ps1` (job summary, NUnit `test-results`). +- A test that changes files, links, or security descriptors uses + `Tests\TestHelpers.psm1`: its own sandbox below + `$env:TEMP\NTFSSecurity.Tests`, `Assert-TestSandboxPath` before each + change, `Remove-TestSandbox` (links first, then ACL reset). Cases that need + a privilege skip with `Test-PrivilegeHeld` and run in CI (elevated); + `Block-TestReadPermission` (OWNER RIGHTS deny) makes a read fail without + elevation. +- `Get-Help -Online` tests use the internal hook `BypassOnlineHelpRetrieval`, + which PowerShell 7 ignores for the help file: 36 tests run only in Windows + PowerShell. +- `Manifest.Tests.ps1`: `Test-ModuleManifest` clean, exactly 36 cmdlets, one + version in manifest and assemblies (Decision 10). `Release.Tests.ps1`: + release notes for the manifest version, and the packages (`FileList` + files, version with label, command tags, zip layout). From 95332b12ca4a7562ddd2bb2cc19d34ab96d98f6a Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 01:49:17 +0200 Subject: [PATCH 19/19] fix: resolve the review of the group A changes Security review of this branch, Major findings: - M4: Get-NTFSAudit took ownership of an item whose SACL it couldn't read. Ownership grants no access to the SACL, so the retry always failed, and it left the owner changed. The cmdlet now writes a ReadSecurityError with the category PermissionDenied, as Get-NTFSOwner does since defect 9; the page says so. - M1: Remove-TestSandbox reset the ACLs recursively before it removed the links. Measured: icacls /reset /T did not follow the junction (the target's explicit entry stayed), but the links now go first anyway; a folder that denies listing gets a reset without /T. New test: the ACL of a junction target stays unchanged. - m4: Assert-TestSandboxPath now rejects a path below a link, which can point outside the sandbox (new test, failed before). - M5: the Inherits column reads one ACL per displayed item; the Get-ChildItem2 page names the cost and how to avoid it. - M2: the comment of the CI-only Get-NTFSAudit repeat test states what it guards; Access.Tests.ps1 guards the same loop fix without elevation. - M3, the stale hash of Get-FileHash2, is fixed on ai/defects-c. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- .memory-bank/progress.md | 2 ++ CHANGELOG.md | 4 ++- Docs/Cmdlets/Get-ChildItem2.md | 2 +- Docs/Cmdlets/Get-NTFSAudit.md | 2 +- NTFSSecurity/AuditCmdlets/GetAudit.cs | 19 +++----------- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 4 +-- Tests/Audit.Tests.ps1 | 5 +++- Tests/TestHelpers.Tests.ps1 | 27 ++++++++++++++++++++ Tests/TestHelpers.psm1 | 24 ++++++++++++++--- 9 files changed, 65 insertions(+), 24 deletions(-) diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index 8d20ffb..fd8308b 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -122,6 +122,8 @@ Numbered as agreed with the maintainer; each is documented on its page. `BeginProcessing` and leave them enabled. - (13) Format view `Children2`: the `Inherits` column uses `IsInheritanceBlocked`, so it always shows `True` for `Get-ChildItem2`. +- Review of group A: five Major findings fixed in the last commit; the + Minor ones are listed in the PR description. #### B: Ignored parameters and parameter sets diff --git a/CHANGELOG.md b/CHANGELOG.md index 76c05c2..64ccd2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,7 +61,9 @@ The format is based on the files that followed it; folders are now skipped - Fix `Get-NTFSAudit`, which returned nothing without the Security privilege instead of an error, and which returned the entries of the previous item - again after a path whose security descriptor it couldn't read + again after a path whose security descriptor it couldn't read; it also no + longer takes ownership of an item whose audit entries it can't read, which + didn't help and could leave the owner changed - Fix `Get-NTFSAccess`, which returned the entries of the previous item again after a path whose ACL it couldn't read - Fix `Add-NTFSAudit`, whose `-Account` and `-AccessRights` parameters were diff --git a/Docs/Cmdlets/Get-ChildItem2.md b/Docs/Cmdlets/Get-ChildItem2.md index 0bd8f8a..5444ce2 100644 --- a/Docs/Cmdlets/Get-ChildItem2.md +++ b/Docs/Cmdlets/Get-ChildItem2.md @@ -301,7 +301,7 @@ The cmdlet returns this object for every folder it finds. Depending on the modul The module defines the alias `dir2` for this cmdlet. -The default table view shows the `Mode`, `Inherits`, `LastWriteTime`, `Size(M)`, and `Name` columns. `Inherits` is `False` for an item whose access inheritance is disabled. Before 5.0.0, the column showed `True` for every item. +The default table view shows the `Mode`, `Inherits`, `LastWriteTime`, `Size(M)`, and `Name` columns. `Inherits` is `False` for an item whose access inheritance is disabled. Reading that value costs one access to the ACL of each displayed item, which slows down the display of large listings; to avoid it, select the properties you need, for example with `Format-Table -Property Mode, LastWriteTime, Length, Name`. Objects that you pipe to another command are not affected. Before 5.0.0, the column showed `True` for every item. The `PrivateData` section of the module manifest `NTFSSecurity.psd1` contains two settings that this cmdlet reads when it starts. `GetFileSystemModeProperty` adds the calculated `Mode` property to every item. `IdentifyHardLinks` adds the `HardLinkCount` property to every file, which requires an extra call into the file system for each file and therefore slows down large listings noticeably. Set either value to `$false` in the manifest and import the module again if you prefer the faster enumeration over the additional properties. diff --git a/Docs/Cmdlets/Get-NTFSAudit.md b/Docs/Cmdlets/Get-NTFSAudit.md index 2b03198..9294d25 100644 --- a/Docs/Cmdlets/Get-NTFSAudit.md +++ b/Docs/Cmdlets/Get-NTFSAudit.md @@ -181,7 +181,7 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well. -If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. +If reading the audit entries is denied, the cmdlet writes a `ReadSecurityError` with the category `PermissionDenied`. It doesn't take ownership of the item, because ownership grants no access to the SACL. Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. The `InheritanceEnabled` property of the entries also reported whether the access entries were inherited instead of the audit entries. diff --git a/NTFSSecurity/AuditCmdlets/GetAudit.cs b/NTFSSecurity/AuditCmdlets/GetAudit.cs index 6531274..c569212 100644 --- a/NTFSSecurity/AuditCmdlets/GetAudit.cs +++ b/NTFSSecurity/AuditCmdlets/GetAudit.cs @@ -100,22 +100,11 @@ namespace NTFSSecurity { acl = GetAuditRules(item); } - catch (UnauthorizedAccessException) + catch (UnauthorizedAccessException ex) { - try - { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - acl = GetAuditRules(item); - FileSystemOwner.SetOwner(item, previousOwner); - } - catch (Exception ex2) - { - WriteError(new ErrorRecord(ex2, "ReadSecurityError", ErrorCategory.WriteError, path)); - continue; - } + // Taking ownership grants no access to the SACL, so it wouldn't help, and it would change the owner. + WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.PermissionDenied, path)); + continue; } catch (Exception ex) { diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 166bd71..56c1d24 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -3841,7 +3841,7 @@ PS C:\> Disable-Privileges `Get-ChildItem2` enumerates the file system through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), which is why it returns items whose path exceeds the 260-character `MAX_PATH` limit that the built-in `Get-ChildItem` cmdlet is bound to. The objects are AlphaFS objects, not `System.IO` objects, and the other NTFSSecurity cmdlets accept them directly because their `-Path` parameters have the alias `FullName`. The module defines the alias `dir2` for this cmdlet. - The default table view shows the `Mode`, `Inherits`, `LastWriteTime`, `Size(M)`, and `Name` columns. `Inherits` is `False` for an item whose access inheritance is disabled. Before 5.0.0, the column showed `True` for every item. + The default table view shows the `Mode`, `Inherits`, `LastWriteTime`, `Size(M)`, and `Name` columns. `Inherits` is `False` for an item whose access inheritance is disabled. Reading that value costs one access to the ACL of each displayed item, which slows down the display of large listings; to avoid it, select the properties you need, for example with `Format-Table -Property Mode, LastWriteTime, Length, Name`. Objects that you pipe to another command are not affected. Before 5.0.0, the column showed `True` for every item. The `PrivateData` section of the module manifest `NTFSSecurity.psd1` contains two settings that this cmdlet reads when it starts. `GetFileSystemModeProperty` adds the calculated `Mode` property to every item. `IdentifyHardLinks` adds the `HardLinkCount` property to every file, which requires an extra call into the file system for each file and therefore slows down large listings noticeably. Set either value to `$false` in the manifest and import the module again if you prefer the faster enumeration over the additional properties. A folder that cannot be read produces a non-terminating error with the ID `DirUnauthorizedAccessError` for an access denial or `DirUnspecifiedError` for any other failure, and a path that does not exist produces the error `FileNotFound`. In each case the cmdlet continues with the next path. Failures that occur while `-Recurse` collects the subfolders of a folder are reported as verbose messages only, not as errors. Before 5.0.0, a `-Path` value that points to a file stopped the cmdlet with an `InvalidCastException`. @@ -4847,7 +4847,7 @@ PS C:\> Disable-Privileges When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. Reading the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well. - If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. + If reading the audit entries is denied, the cmdlet writes a `ReadSecurityError` with the category `PermissionDenied`. It doesn't take ownership of the item, because ownership grants no access to the SACL. Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again. The `InheritanceEnabled` property of the entries also reported whether the access entries were inherited instead of the audit entries. diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 index 587ea5b..5043c43 100644 --- a/Tests/Audit.Tests.ps1 +++ b/Tests/Audit.Tests.ps1 @@ -54,7 +54,10 @@ Describe 'Get-NTFSAudit' { } Context 'When a path fails after a path with audit entries' { - # Before 5.0.0, the cmdlet wrote the entries of the previous item again for the failing path. + # Before 5.0.0, the cmdlet wrote the entries of the previous item again for the failing path. In CI, the deny + # entry made the original implementation, which also read the DACL, fail for the second path. The current one + # reads only the SACL, which the deny entry doesn't block; Access.Tests.ps1 guards the same loop fix in + # Get-NTFSAccess with a read that fails without elevation. It 'Should return the entries of the first item once' -Skip:(-not $canReadAudit) { $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'Audited' -Directory $denied = New-TestSandboxItem -Sandbox $sandbox -Name 'Denied' diff --git a/Tests/TestHelpers.Tests.ps1 b/Tests/TestHelpers.Tests.ps1 index d850384..c743ed4 100644 --- a/Tests/TestHelpers.Tests.ps1 +++ b/Tests/TestHelpers.Tests.ps1 @@ -67,6 +67,21 @@ Describe 'Test helpers' { { Assert-TestSandboxPath -Sandbox $env:TEMP -Path (Join-Path -Path $env:TEMP -ChildPath 'File.txt') } | Should -Throw -ExpectedMessage '*is not a test sandbox*' } + + It 'Should reject a path below a link, which can point outside the sandbox' { + $otherSandbox = New-TestSandbox -Name 'Helpers' + try { + $link = Join-Path -Path $sandbox -ChildPath 'Link' + Assert-TestSandboxPath -Sandbox $sandbox -Path $link + New-Item -ItemType Junction -Path $link -Value $otherSandbox | Out-Null + + { Assert-TestSandboxPath -Sandbox $sandbox -Path (Join-Path -Path $link -ChildPath 'File.txt') } | + Should -Throw -ExpectedMessage '*is a link*' + } + finally { + Remove-TestSandbox -Sandbox $otherSandbox + } + } } Context 'Remove-TestSandbox' { @@ -80,6 +95,14 @@ Describe 'Test helpers' { Assert-TestSandboxPath -Sandbox $otherSandbox -Path $target New-Item -ItemType Directory -Path $target | Out-Null Set-Content -LiteralPath (Join-Path -Path $target -ChildPath 'Keep.txt') -Value 'Keep' + # An explicit entry that a reset through the link would remove + $targetAcl = Get-Acl -LiteralPath $target + $targetAcl.AddAccessRule((New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( + (New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-1-0'), + [System.Security.AccessControl.FileSystemRights]::ReadData, [System.Security.AccessControl.AccessControlType]::Allow + ))) + Set-Acl -LiteralPath $target -AclObject $targetAcl + $targetSddl = (Get-Acl -LiteralPath $target).Sddl Assert-TestSandboxPath -Sandbox $sandbox -Path $link, $locked New-Item -ItemType Junction -Path $link -Value $target | Out-Null New-Item -ItemType Directory -Path $locked | Out-Null @@ -101,6 +124,10 @@ Describe 'Test helpers' { It 'Should remove a junction without removing the files of its target' { Join-Path -Path $target -ChildPath 'Keep.txt' | Should -Exist } + + It 'Should not change the ACL of the target of a junction' { + (Get-Acl -LiteralPath $target).Sddl | Should -BeExactly $targetSddl + } } Context 'Test-IsElevated and Test-PrivilegeHeld' { diff --git a/Tests/TestHelpers.psm1 b/Tests/TestHelpers.psm1 index 4d7656d..5f11a15 100644 --- a/Tests/TestHelpers.psm1 +++ b/Tests/TestHelpers.psm1 @@ -61,6 +61,16 @@ function Assert-TestSandboxPath { if (-not ($fullName + '\').StartsWith($sandboxPrefix, [StringComparison]::OrdinalIgnoreCase)) { throw "Refusing to change '$fullName', which is outside the test sandbox '$sandboxFullName'." } + + # A link inside the sandbox can point outside of it, so no folder of the path may be a link. + $parent = [IO.Path]::GetDirectoryName($fullName) + while ($parent -and $parent.Length -gt $sandboxFullName.Length) { + if ([IO.Directory]::Exists($parent) -and + ([IO.File]::GetAttributes($parent) -band [IO.FileAttributes]::ReparsePoint)) { + throw "Refusing to change '$fullName', because its folder '$parent' is a link." + } + $parent = [IO.Path]::GetDirectoryName($parent) + } } } } @@ -90,12 +100,20 @@ function Remove-TestSandbox { # the caller uses ErrorAction Stop. Such items can still be deleted through the rights on their folder. $ErrorActionPreference = 'Continue' - # Windows PowerShell 5.1 follows directory links when it removes a folder recursively, so the links go first. - & icacls.exe $Sandbox /reset /T /C /Q *> $null + # Windows PowerShell 5.1 and icacls /T follow directory links, so the links go first. A folder that denies + # listing its content gets its own ACL reset, without /T, before it is listed. $pending = New-Object -TypeName 'System.Collections.Generic.Stack[string]' $pending.Push($Sandbox) while ($pending.Count -gt 0) { - foreach ($entry in [IO.Directory]::GetFileSystemEntries($pending.Pop())) { + $folder = $pending.Pop() + try { + $entries = [IO.Directory]::GetFileSystemEntries($folder) + } + catch { + & icacls.exe $folder /reset /C /Q *> $null + $entries = [IO.Directory]::GetFileSystemEntries($folder) + } + foreach ($entry in $entries) { $attributes = [IO.File]::GetAttributes($entry) if ($attributes -band [IO.FileAttributes]::ReparsePoint) { if ($attributes -band [IO.FileAttributes]::Directory) {