diff --git a/CHANGELOG.md b/CHANGELOG.md index f77d40e..e4a4a2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -131,8 +131,17 @@ The format is based on when a later command ended the pipeline: a `break` or `continue`, `Select-Object -First`, or a `throw` was handled as a failure of the item, so that `Remove-Item2 -PassThru | Select-Object -First 1` removed every - item, and the caller never saw the `throw`. They now stop and write no + item, and the caller never saw the `throw`. The same held when the later + command took a stream instead of the objects: the verbose messages of + `Get-FileHash2` and `Set-NTFSSecurityDescriptor`, the debug messages of + `Set-NTFSOwner`, and the errors of `Get-ChildItem2` for a folder that it + cannot read, for example with `4>&1` or `2>&1`. They now stop and write no error, and the error of the later command reaches the caller +- Fix the cmdlets that enable the privileges, which left a privilege enabled + in the session and hid the exception of a later command when that command + took the debug message after the enabling, for example with + `5>&1 | Select-Object -First 2`; they now disable the privilege and pass + the exception on - Fix `Get-ChildItem2 -Filter`, which read a bracket as the start of a character class, so that it did not return a file with brackets in its name, such as `Report[1].txt`, for that name; only `*` and `?` are wildcards. A diff --git a/Docs/Cmdlets/Get-ChildItem2.md b/Docs/Cmdlets/Get-ChildItem2.md index 3783233..6226b4e 100644 --- a/Docs/Cmdlets/Get-ChildItem2.md +++ b/Docs/Cmdlets/Get-ChildItem2.md @@ -319,7 +319,9 @@ Before 5.0.0, a `-Path` value that points to a file stopped the cmdlet with an ` Before 5.0.0, `-Filter` read a bracket as the start of a character class, so a file with brackets in its name, such as `Report[1].txt`, was not returned for its name, and a pattern of an asterisk, a dot, and an asterisk dropped the items without a dot in their names, most folders among them. -Before 5.0.0, a `break` or `continue` in a later command of the pipeline did not end the cmdlet for an item below the first folder. +The enumeration of the AlphaFS library decides which names match, and its rules for a dot differ from those of `Get-ChildItem`: a pattern such as `Report.*` does not return the file `Report`, which has no dot, a pattern that ends in a dot returns nothing, and an empty value returns nothing. Only the pattern of an asterisk, a dot, and an asterisk is treated as a single asterisk. + +Before 5.0.0, a `break`, a `continue`, or a `throw` in a later command of the pipeline did not end the cmdlet for an item below the first folder, also when the later command took the error of a folder that the cmdlet cannot read, for example with `2>&1`. ## RELATED LINKS diff --git a/NTFSSecurity/BaseCmdlets.cs b/NTFSSecurity/BaseCmdlets.cs index cb3da6e..0d319c7 100644 --- a/NTFSSecurity/BaseCmdlets.cs +++ b/NTFSSecurity/BaseCmdlets.cs @@ -11,9 +11,11 @@ namespace NTFSSecurity /// /// Recognizes what a later command in the pipeline raises to end the pipeline or the loop around it: the end of the /// pipeline, for example for Select-Object -First, and a break or continue in a script block. These exceptions pass - /// through a cmdlet while it writes an object. A catch for the failures of an item must pass them on: reported as - /// the error of that item, they would end nothing, and the cmdlet would go on with the next item. Anything else that - /// a Write method raises is recognized by BaseCmdlet.IsFromLaterCommand. + /// through a cmdlet while it writes to a stream. A catch-all for the failures of an item must pass them on: reported + /// as the error of that item, they would end nothing, and the cmdlet would go on with the next item. BaseCmdlet + /// notes the exception that each of its Write methods raises, which includes everything that a later command can + /// throw; this check by type is a second line of defense for the other calls into PowerShell, which also raise the + /// end of the pipeline. See BaseCmdlet.IsFromLaterCommand. /// internal static class PipelineControl { @@ -47,9 +49,10 @@ namespace NTFSSecurity // The exception that a Write method of this cmdlet raised last. A Write method runs the later commands of the // pipeline and so raises what they raise: a throw in a script block, an error with -ErrorAction Stop, the end of the - // pipeline, a break or a continue. None of it is a failure of the item that the cmdlet processes. A catch for those - // failures must pass it on (IsFromLaterCommand), or the cmdlet reports it as the error of that item, goes on with - // the next one, and the caller never sees the exception. + // pipeline, a break or a continue. None of it is a failure of the item that the cmdlet processes. A catch-all for + // those failures must pass it on (IsFromLaterCommand), or the cmdlet reports it as the error of that item, goes on + // with the next one, and the caller never sees the exception. WriteWarning is the one Write method that isn't + // noted, because no catch-all of the module encloses it. private Exception laterCommandException; /// Writes the object to the pipeline and notes what a later command raises, see IsFromLaterCommand. @@ -80,7 +83,21 @@ namespace NTFSSecurity } } - // The streams that a later command can take, for example Select-Object -First with 4>&1. + // The error, verbose, and debug streams, which a later command can take too, for example Select-Object -First with 2>&1. + /// Writes the error and notes what a later command raises, see IsFromLaterCommand. + public new void WriteError(ErrorRecord errorRecord) + { + try + { + base.WriteError(errorRecord); + } + catch (Exception ex) + { + laterCommandException = ex; + throw; + } + } + /// Writes a verbose message and notes what a later command raises, see IsFromLaterCommand. public new void WriteVerbose(string text) { @@ -424,9 +441,10 @@ namespace NTFSSecurity WriteDebug(string.Format("The privilege {0} is disabled...", privilege)); //activate it privControl.EnablePrivilege(privilege); - WriteDebug(string.Format("..enabled")); - //remember the privilege so that we can automatically disable it after the cmdlet finished processing + //remember the privilege so that we can automatically disable it after the cmdlet finished processing; before + //the next message, which a later command can answer with an exception: Dispose disables only what is noted enabledPrivileges.Add(privilege.ToString()); + WriteDebug(string.Format("..enabled")); privileges = privControl.GetPrivileges(); } @@ -450,6 +468,12 @@ namespace NTFSSecurity } catch(Exception ex) { + // Not a failure to enable the privilege: a later command that took one of the debug messages raised it. + if (IsFromLaterCommand(ex)) + { + throw; + } + WriteDebug(string.Format("Could not enable privilege {0}. The error was: {1}", privilege, ex.Message)); return false; } diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index b45b6de..3fc7a58 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -3867,7 +3867,8 @@ PS C:\> Disable-Privileges 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`, `-Attributes` returned only the items that had all the listed attributes, and an empty `-Attributes` value returned every item, also the hidden ones. Earlier builds, including the 5.0.0 prereleases, could also omit the first hidden item with `-Hidden` unless `-Force` was explicitly supplied. Before 5.0.0, `-Filter` read a bracket as the start of a character class, so a file with brackets in its name, such as `Report[1].txt`, was not returned for its name, and a pattern of an asterisk, a dot, and an asterisk dropped the items without a dot in their names, most folders among them. - Before 5.0.0, a `break` or `continue` in a later command of the pipeline did not end the cmdlet for an item below the first folder. + The enumeration of the AlphaFS library decides which names match, and its rules for a dot differ from those of `Get-ChildItem`: a pattern such as `Report.*` does not return the file `Report`, which has no dot, a pattern that ends in a dot returns nothing, and an empty value returns nothing. Only the pattern of an asterisk, a dot, and an asterisk is treated as a single asterisk. + Before 5.0.0, a `break`, a `continue`, or a `throw` in a later command of the pipeline did not end the cmdlet for an item below the first folder, also when the later command took the error of a folder that the cmdlet cannot read, for example with `2>&1`. diff --git a/Tests/PipelineControl.Tests.ps1 b/Tests/PipelineControl.Tests.ps1 index 903c545..79a6f4e 100644 --- a/Tests/PipelineControl.Tests.ps1 +++ b/Tests/PipelineControl.Tests.ps1 @@ -1,10 +1,10 @@ <# Tests how the cmdlets of the module built in NTFSSecurity\bin\Release behave when a later command in the pipeline ends it: a break or continue in a script block, Select-Object -First, or a terminating error such as a throw. The - exception that carries it passes through the cmdlet while it writes an object, a verbose message, or a debug message. - A catch for the failures of an item must not report it as an error of that item and go on with the next one: a - cmdlet that removes, copies, moves, or changes items would change them all, although the caller ended the pipeline, - and the caller would never see the exception. Every test works on files and folders in a sandbox. + exception that carries it passes through the cmdlet while it writes an object, an error, a verbose message, or a debug + message. A catch-all for the failures of an item must not report it as an error of that item and go on with the next + one: a cmdlet that removes, copies, moves, or changes items would change them all, although the caller ended the + pipeline, and the caller would never see the exception. Every test works on files and folders in a sandbox. #> [Diagnostics.CodeAnalysis.SuppressMessageAttribute( 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' @@ -473,8 +473,135 @@ Describe 'A later command that ends the pipeline' { } } +# The errors that Get-ChildItem2 writes for a folder that it cannot read reach a later command too, for example with 2>&1. +# The folders that cannot be read come first in the order of the file system, so that the folder with the file is reached +# only if the listing goes on after the first error. +Describe 'A later command and the error of a folder that Get-ChildItem2 cannot read' { + BeforeAll { + $errorTree = New-TestSandboxItem -Sandbox $sandbox -Name 'ErrorTree' -Directory + $unreadable = foreach ($name in 'A', 'B') { + $folder = Join-Path -Path $errorTree -ChildPath $name + Assert-TestSandboxPath -Sandbox $sandbox -Path $folder + New-Item -ItemType Directory -Path $folder | Out-Null + $folder + } + $readableFile = Join-Path -Path $errorTree -ChildPath 'C\Three.txt' + Assert-TestSandboxPath -Sandbox $sandbox -Path $readableFile + New-Item -ItemType Directory -Path (Split-Path -Path $readableFile -Parent) | Out-Null + Set-Content -LiteralPath $readableFile -Value 'Three' + foreach ($folder in $unreadable) { + Add-TestDenyRule -Sandbox $sandbox -Path $folder -Rights @{ 'S-1-1-0' = 'ReadData' } + } + } + + # Before 5.0.0-rc7, the recursion took what the later command threw for the error of a nested folder as a failure of + # the folder above it, wrote a verbose message, and left the loop over the folders: the listing ended early and the + # caller never saw the exception. + It 'Should pass on what a later command throws when it takes the error of a nested folder' { + $emitted = 0 + $caught = $null + try { + Get-ChildItem2 -Path $errorTree -Recurse -File 2>&1 | ForEach-Object -Process { + $emitted++ + throw 'Downstream failure' + } + } + catch { + $caught = $_ + } + + $caught.Exception.Message | Should -BeLike '*Downstream failure*' + $emitted | Should -Be 1 + } + + It 'Should leave the loop for a of a later command that takes the error of a nested folder' -ForEach @( + @{ Keyword = 'break' } + @{ Keyword = 'continue' } + ) { + $emitted = 0 + $reachedEnd = $false + foreach ($round in 1) { + Get-ChildItem2 -Path $errorTree -Recurse -File 2>&1 | ForEach-Object -Process { + $emitted++ + if ($Keyword -eq 'break') { break } else { continue } + } + $reachedEnd = $true + } + + $emitted | Should -Be 1 + $reachedEnd | Should -BeFalse + } + + It 'Should stop with the error of the first nested folder that it cannot read for -ErrorAction Stop' { + $listed = New-Object -TypeName 'System.Collections.Generic.List[object]' + $caught = $null + try { + Get-ChildItem2 -Path $errorTree -Recurse -File -ErrorAction Stop | ForEach-Object -Process { $listed.Add($_) } + } + catch { + $caught = $_ + } + + $caught | Should -Not -BeNullOrEmpty + $caught.FullyQualifiedErrorId | Should -BeLike 'DirUnauthorizedAccessError,*' + $caught.TargetObject | Should -BeIn $unreadable + $listed | Should -BeNullOrEmpty + } +} + +# The catches of Get-ChildItem2 for an UnauthorizedAccessException and of Remove-Item2 for an IOException don't ask where +# the exception comes from: they rely on PowerShell wrapping what a later command throws, so that an exception of these +# types never reaches them as it was thrown. A PowerShell version that hands it on as it is fails these tests. +Describe 'A later command that throws an exception of a type that a cmdlet handles' { + It 'Get-ChildItem2 should pass on a thrown UnauthorizedAccessException' { + $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ThrownDenied' -Directory + $files = 'One.txt', 'Two.txt' | ForEach-Object -Process { Join-Path -Path $folder -ChildPath $_ } + Assert-TestSandboxPath -Sandbox $sandbox -Path $files + Set-Content -LiteralPath $files -Value 'File' + $emitted = 0 + $caught = $null + $Error.Clear() + try { + Get-ChildItem2 -Path $folder -File -ErrorAction SilentlyContinue | ForEach-Object -Process { + $emitted++ + throw [System.UnauthorizedAccessException]::new('Downstream failure') + } + } + catch { + $caught = $_ + } + + $caught.Exception | Should -BeOfType [System.UnauthorizedAccessException] + $caught.Exception.Message | Should -BeExactly 'Downstream failure' + $emitted | Should -Be 1 + @($Error | Where-Object -FilterScript { $_.FullyQualifiedErrorId -like 'DirUnauthorizedAccessError,*' }) | Should -BeNullOrEmpty + } + + It 'Remove-Item2 should pass on a thrown IOException and leave the next item' { + $pair = New-Pair + $emitted = 0 + $caught = $null + $Error.Clear() + try { + Remove-Item2 -Path $pair.First, $pair.Second -PassThru -ErrorAction SilentlyContinue | ForEach-Object -Process { + $emitted++ + throw [System.IO.IOException]::new('Downstream failure') + } + } + catch { + $caught = $_ + } + + $caught.Exception | Should -BeOfType [System.IO.IOException] + $caught.Exception.Message | Should -BeExactly 'Downstream failure' + $emitted | Should -Be 1 + $pair.Second | Should -Exist + @($Error | Where-Object -FilterScript { $_.FullyQualifiedErrorId -like 'DeleteError,*' }) | Should -BeNullOrEmpty + } +} + # A cmdlet also meets the end of the pipeline where it did not write: another call of PowerShell can raise it too. The -# check that every catch makes recognizes the exceptions by their types. PowerShell keeps the exceptions of break and +# check that every catch-all makes recognizes the exceptions by their types. PowerShell keeps the exceptions of break and # continue internal, so they are recognized by the name of their base type, which a stand-in with that name shows. Describe 'Recognizing the end of a pipeline by the type of the exception' { BeforeAll { diff --git a/Tests/Privileges.Tests.ps1 b/Tests/Privileges.Tests.ps1 index 6c984d9..f07cf56 100644 --- a/Tests/Privileges.Tests.ps1 +++ b/Tests/Privileges.Tests.ps1 @@ -182,6 +182,101 @@ Describe 'Privileges when the pipeline stops early' { } } +# A cmdlet enables the privileges one after the other and writes a debug message before and after each one. A later command +# that takes the debug stream can end the pipeline or throw at the message after the enabling, before the cmdlet has noted +# that it enabled the privilege. +Describe 'Privileges when a later command takes the debug messages of the cmdlet' { + BeforeAll { + $privateData['EnablePrivileges'] = $true + $debugFile = New-TestSandboxItem -Sandbox $sandbox -Name 'DebugStopped' + } + + AfterAll { + $privateData['EnablePrivileges'] = $enablePrivileges + } + + BeforeEach { + Disable-Privileges -ErrorAction SilentlyContinue -WarningAction SilentlyContinue + } + + AfterEach { + Disable-Privileges -ErrorAction SilentlyContinue -WarningAction SilentlyContinue + } + + # Before 5.0.0-rc7, the privilege that the cmdlet had enabled at that moment stayed enabled in the session: nothing + # disabled it, because the cmdlet had not noted yet that it enabled it. + It 'Should disable the privilege when Select-Object -First ends the pipeline at the message after its enabling' -Skip:(-not $holdsPrivileges) { + $DebugPreference = 'Continue' + $messages = @(Get-NTFSOwner -Path $debugFile 5>&1 | ForEach-Object -Process { $_.Message }) + $enabledAt = $messages.IndexOf('..enabled') + 1 + $enabledAt | Should -BeGreaterThan 0 + Get-EnabledFileSystemPrivilege | Should -BeNullOrEmpty + + $result = @(Get-NTFSOwner -Path $debugFile 5>&1 | Select-Object -First $enabledAt) + + $result | Should -HaveCount $enabledAt + $result[-1].Message | Should -BeExactly '..enabled' + Get-EnabledFileSystemPrivilege | Should -BeNullOrEmpty + } + + # Before 5.0.0-rc7, the cmdlet took the exception for the failure to enable the privilege, went on with the next + # privilege, and the caller never saw it; all four privileges stayed enabled. + It 'Should pass on what a later command throws at the message after the enabling and disable the privileges' -Skip:(-not $holdsPrivileges) { + $DebugPreference = 'Continue' + $caught = $null + try { + Get-NTFSOwner -Path $debugFile 5>&1 | ForEach-Object -Process { + if ($_.Message -eq '..enabled') { throw 'Downstream failure' } + $_ + } | Out-Null + } + catch { + $caught = $_ + } + + $caught.Exception.Message | Should -BeLike '*Downstream failure*' + Get-EnabledFileSystemPrivilege | Should -BeNullOrEmpty + } +} + +# Enable-Privileges recognizes the script NTFSSecurity.Init.ps1, which a user adds to start the module, by its name: from +# that script, it enables the privileges only for the module setting EnablePrivileges, from any other script always. Each +# test runs the script in a child process, which starts without enabled privileges, so that the privileges of this +# process stay as they are. +Describe 'Enable-Privileges in the script NTFSSecurity.Init.ps1' { + BeforeAll { + function Invoke-StartScript { + param ([string] $ScriptName, [bool] $Setting) + + $folder = Join-Path -Path $sandbox -ChildPath ('Start-{0}' -f [guid]::NewGuid().ToString('N').Substring(0, 8)) + $script = Join-Path -Path $folder -ChildPath $ScriptName + Assert-TestSandboxPath -Sandbox $sandbox -Path $script + New-Item -ItemType Directory -Path $folder | Out-Null + Set-Content -LiteralPath $script -Value @' +param ($ModulePath, $Setting) +Import-Module -Name $ModulePath -ErrorAction Stop +(Get-Module -Name NTFSSecurity).PrivateData['EnablePrivileges'] = ($Setting -eq 'True') +Enable-Privileges +'BACKUP:{0}' -f (Get-Privileges | Where-Object -Property Privilege -EQ -Value 'Backup').PrivilegeState +'@ + $output = & (Get-Process -Id $PID).Path -NoProfile -NonInteractive -ExecutionPolicy Bypass -File $script -ModulePath ([IO.Path]::GetFullPath($modulePath)) -Setting $Setting + @($output | Where-Object -FilterScript { $_ -like 'BACKUP:*' }) -replace '^BACKUP:' + } + } + + It 'Should enable the privileges when the module setting EnablePrivileges is $true' -Skip:(-not $holdsPrivileges) { + Invoke-StartScript -ScriptName 'NTFSSecurity.Init.ps1' -Setting $true | Should -Be 'Enabled' + } + + It 'Should leave the privileges disabled when the module setting EnablePrivileges is $false' -Skip:(-not $holdsPrivileges) { + Invoke-StartScript -ScriptName 'NTFSSecurity.Init.ps1' -Setting $false | Should -Be 'Disabled' + } + + It 'Should enable the privileges in a script of another name also when the module setting EnablePrivileges is $false' -Skip:(-not $holdsPrivileges) { + Invoke-StartScript -ScriptName 'Other.ps1' -Setting $false | Should -Be 'Enabled' + } +} + Describe 'Privileges that another command in the pipeline changes' { BeforeAll { $privateData['EnablePrivileges'] = $true