Browse Source

fix: address the review of 5.0.0-rc6

- Copy-Item2 and Move-Item2 name a missing destination folder in a verbose
  message with -WhatIf, like an existing destination (#108); the operation
  itself fails with an error that names the folder (finding 1).
- Invoke-TestsAsBasicUser.ps1 refuses a title with a double quote or a
  percent sign, which cmd.exe interprets inside the quoted argument, and
  fails when waiting for the test process fails (findings 4 and 5).
- The page of Get-NTFSSimpleAccess says that ReadData is the right to list
  a folder (ListDirectory), which the cmdlet reports as Read (finding 10).

Checked and kept: a UNC destination on a share that doesn't exist names
the share, which a new test pins (finding 3); the lab script refuses
machines outside the lab before it changes anything (finding 6).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
ai/release-5.0.0-rc6
Raimund Andree 3 days ago
parent
commit
acfe3af4cf
  1. 14
      .github/scripts/Invoke-TestsAsBasicUser.ps1
  2. 5
      CHANGELOG.md
  3. 2
      Docs/Cmdlets/Copy-Item2.md
  4. 2
      Docs/Cmdlets/Get-NTFSSimpleAccess.md
  5. 2
      Docs/Cmdlets/Move-Item2.md
  6. 20
      NTFSSecurity/BaseCmdlets.cs
  7. 8
      NTFSSecurity/ItemCmdlets/CopyItem2.cs
  8. 8
      NTFSSecurity/ItemCmdlets/MoveItem2.cs
  9. 6
      NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml
  10. 31
      Tests/ItemCmdlets.Tests.ps1
  11. 20
      Tests/Repository.Tests.ps1

14
.github/scripts/Invoke-TestsAsBasicUser.ps1

@ -14,7 +14,8 @@
Specifies the path of the result file in the NUnit format.
.PARAMETER Title
Specifies the heading of the test results in the job summary.
Specifies the heading of the test results in the job summary. It can't contain a double quote or a percent sign,
which would change the command line of cmd.exe.
.EXAMPLE
.\.github\scripts\Invoke-TestsAsBasicUser.ps1 -ResultPath TestResults\WindowsPowerShell-BasicUser.xml -Title 'Windows PowerShell 5.1 as a basic user'
@ -30,6 +31,8 @@ param (
[Parameter(Mandatory)]
[ValidateNotNullOrEmpty()]
# cmd.exe gets the title in a quoted argument and expands environment variables also inside quotes.
[ValidatePattern('^[^"%]+$')]
[string]
$Title
)
@ -48,6 +51,7 @@ public static class NTFSSecurityBasicUserProcess
private const uint SaferLevelOpen = 1;
private const uint CreateNoWindow = 0x08000000;
private const uint Infinite = 0xFFFFFFFF;
private const uint WaitObject0 = 0;
[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
private struct StartupInfo
@ -129,7 +133,11 @@ public static class NTFSSecurityBasicUserProcess
try
{
WaitForSingleObject(processInformation.hProcess, Infinite);
if (WaitForSingleObject(processInformation.hProcess, Infinite) != WaitObject0)
{
throw new Win32Exception(Marshal.GetLastWin32Error());
}
uint exitCode;
if (!GetExitCodeProcess(processInformation.hProcess, out exitCode))
{
@ -175,7 +183,7 @@ $executable = (Get-Process -Id $PID).Path
$testScript = Join-Path -Path $PSScriptRoot -ChildPath 'Invoke-Tests.ps1'
# cmd redirects the output of the tests, which have no console of their own.
$commandLine = 'cmd.exe /d /s /c ""{0}" -NoProfile -NonInteractive -ExecutionPolicy Bypass -File "{1}" -ResultPath "{2}" -Title "{3}" > "{4}" 2>&1"' -f
$executable, $testScript, $runResultPath, $Title.Replace('"', "'"), $logPath
$executable, $testScript, $runResultPath, $Title, $logPath
$stepSummary = $env:GITHUB_STEP_SUMMARY
$env:GITHUB_STEP_SUMMARY = $summaryPath

5
CHANGELOG.md

@ -297,8 +297,9 @@ The format is based on
an error that named the source item
([#21](https://github.com/raandree/NTFSSecurity/issues/21)); they now
write `DestinationFileAlreadyExists` and an error that names the missing
folder. `Copy-Item2` no longer creates the missing folders of the
destination when it copies a folder, which the prereleases of 5.0.0 did
folder, and with `-WhatIf` a verbose message that names it. `Copy-Item2`
no longer creates the missing folders of the destination when it copies a
folder, which the prereleases of 5.0.0 did
- Fix `Get-NTFSHardLink`, which stopped for all remaining paths at a folder
and at a file on a network share, where Windows can't list the names of
a file, and `New-NTFSHardLink -PassThru`, which stopped with a

2
Docs/Cmdlets/Copy-Item2.md

@ -24,7 +24,7 @@ The `Copy-Item2` cmdlet copies the items in `-Path` to the location in `-Destina
How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and copies it into that folder. In every other case the value is the full path of the new item, which lets you copy and rename in one step. `-Destination` is resolved against the current location once, when the cmdlet starts.
Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it or merging into it. With `-WhatIf`, it names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, an existing file is replaced, and a folder is copied into an existing folder of the same name, replacing the files that exist in both. The folder that is to contain the new item must exist; otherwise the cmdlet writes an error that names that folder. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported.
Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it or merging into it. With `-WhatIf`, it names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, an existing file is replaced, and a folder is copied into an existing folder of the same name, replacing the files that exist in both. The folder that is to contain the new item must exist; otherwise the cmdlet writes an error that names that folder, or, with `-WhatIf`, a verbose message. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported.
The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`.

2
Docs/Cmdlets/Get-NTFSSimpleAccess.md

@ -27,7 +27,7 @@ Get-NTFSSimpleAccess [-IncludeRootFolder] [-SecurityDescriptor] <FileSystemSecur
## DESCRIPTION
Reads the access control entries of folders and writes them as `Security2.SimpleFileSystemAccessRule` objects whose rights are reduced to the three values `Read`, `Write`, and `Delete`. Reading rights such as `ReadAttributes` or `Traverse` become `Read`, changing rights such as `CreateFiles`, `WriteAttributes`, `ChangePermissions`, or `TakeOwnership` become `Write`, and `Delete` and `DeleteSubdirectoriesAndFiles` become `Delete`; `FullControl` becomes all three. The result answers who may read, change, or delete in a folder without the detail of the full ACL.
Reads the access control entries of folders and writes them as `Security2.SimpleFileSystemAccessRule` objects whose rights are reduced to the three values `Read`, `Write`, and `Delete`. Reading rights such as `ReadData`, which on a folder is the right to list it (`ListDirectory`), `ReadAttributes`, or `Traverse` become `Read`, changing rights such as `CreateFiles`, `WriteAttributes`, `ChangePermissions`, or `TakeOwnership` become `Write`, and `Delete` and `DeleteSubdirectoriesAndFiles` become `Delete`; `FullControl` becomes all three. The result answers who may read, change, or delete in a folder without the detail of the full ACL.
The second simplification is that repetitions are left out. The first folder the cmdlet processes is reported with all of its entries, and for every folder that follows only the entries are reported that its parent folder does not already cover. An entry is covered when the parent has an entry for the same account and access type that includes at least the same simple rights. This makes a recursive listing show where permissions actually change instead of repeating the inherited ones on every level, and it requires the parent folder to be processed before its children, which `Get-ChildItem`, `Get-ChildItem2`, and `Get-Item2` do by default.

2
Docs/Cmdlets/Move-Item2.md

@ -24,7 +24,7 @@ The `Move-Item2` cmdlet moves the items in `-Path` to the location in `-Destinat
How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and moves it into that folder. In every other case the value is the full path of the new item, which lets you move and rename in one step, or rename an item in place. `-Destination` is resolved against the current location once, when the cmdlet starts.
Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-WhatIf`, the cmdlet names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. The folder that is to contain the moved item must exist; otherwise the cmdlet writes an error that names that folder.
Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-WhatIf`, the cmdlet names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. The folder that is to contain the moved item must exist; otherwise the cmdlet writes an error that names that folder, or, with `-WhatIf`, a verbose message.
The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`.

20
NTFSSecurity/BaseCmdlets.cs

@ -150,6 +150,22 @@ namespace NTFSSecurity
#endregion
#region WriteMissingDestinationFolderError
/// <summary>
/// Returns the folder of a destination path when that folder doesn't exist.
/// </summary>
/// <param name="destinationPath">The full path of the item that the operation would create.</param>
/// <returns>The missing folder, or null when the folder exists or the path has none, such as a share root.</returns>
protected string GetMissingDestinationFolder(string destinationPath)
{
var folder = Alphaleonis.Win32.Filesystem.Path.GetDirectoryName(destinationPath.TrimEnd('\\'));
if (string.IsNullOrEmpty(folder) || Alphaleonis.Win32.Filesystem.Directory.Exists(folder))
{
return null;
}
return folder;
}
/// <summary>
/// Writes an error that names the folder of a destination path when that folder doesn't exist. Before
/// 5.0.0-rc6, AlphaFS reported such a destination as the source path that could not be found (#21), and
@ -160,8 +176,8 @@ namespace NTFSSecurity
/// <returns>Whether the folder is missing and the error was written.</returns>
protected bool WriteMissingDestinationFolderError(string destinationPath, string errorId)
{
var folder = Alphaleonis.Win32.Filesystem.Path.GetDirectoryName(destinationPath.TrimEnd('\\'));
if (string.IsNullOrEmpty(folder) || Alphaleonis.Win32.Filesystem.Directory.Exists(folder))
var folder = GetMissingDestinationFolder(destinationPath);
if (folder == null)
{
return false;
}

8
NTFSSecurity/ItemCmdlets/CopyItem2.cs

@ -98,6 +98,14 @@ namespace NTFSSecurity
{
WriteVerbose(string.Format("The destination '{0}' already exists; without -Force, the copy would fail", actualDestination));
}
else
{
var missingFolder = GetMissingDestinationFolder(actualDestination);
if (missingFolder != null)
{
WriteVerbose(string.Format("The destination folder '{0}' does not exist; the copy would fail", missingFolder));
}
}
continue;
}

8
NTFSSecurity/ItemCmdlets/MoveItem2.cs

@ -98,6 +98,14 @@ namespace NTFSSecurity
{
WriteVerbose(string.Format("The destination '{0}' already exists; without -Force, the move would fail", actualDestination));
}
else
{
var missingFolder = GetMissingDestinationFolder(actualDestination);
if (missingFolder != null)
{
WriteVerbose(string.Format("The destination folder '{0}' does not exist; the move would fail", missingFolder));
}
}
continue;
}

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

@ -1997,7 +1997,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:description>
<maml:para>The `Copy-Item2` cmdlet copies the items in `-Path` to the location in `-Destination`. It is the long-path counterpart of the built-in `Copy-Item` cmdlet: it works through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), so source and destination may be longer than the 260-character `MAX_PATH` limit.</maml:para>
<maml:para>How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and copies it into that folder. In every other case the value is the full path of the new item, which lets you copy and rename in one step. `-Destination` is resolved against the current location once, when the cmdlet starts.</maml:para>
<maml:para>Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it or merging into it. With `-WhatIf`, it names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, an existing file is replaced, and a folder is copied into an existing folder of the same name, replacing the files that exist in both. The folder that is to contain the new item must exist; otherwise the cmdlet writes an error that names that folder. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported.</maml:para>
<maml:para>Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it or merging into it. With `-WhatIf`, it names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, an existing file is replaced, and a folder is copied into an existing folder of the same name, replacing the files that exist in both. The folder that is to contain the new item must exist; otherwise the cmdlet writes an error that names that folder, or, with `-WhatIf`, a verbose message. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported.</maml:para>
<maml:para>The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`.</maml:para>
</maml:description>
<command:syntax>
@ -6382,7 +6382,7 @@ PS C:\Data&gt; Get-NTFSSecurityDescriptor</dev:code>
</maml:description>
</command:details>
<maml:description>
<maml:para>Reads the access control entries of folders and writes them as `Security2.SimpleFileSystemAccessRule` objects whose rights are reduced to the three values `Read`, `Write`, and `Delete`. Reading rights such as `ReadAttributes` or `Traverse` become `Read`, changing rights such as `CreateFiles`, `WriteAttributes`, `ChangePermissions`, or `TakeOwnership` become `Write`, and `Delete` and `DeleteSubdirectoriesAndFiles` become `Delete`; `FullControl` becomes all three. The result answers who may read, change, or delete in a folder without the detail of the full ACL.</maml:para>
<maml:para>Reads the access control entries of folders and writes them as `Security2.SimpleFileSystemAccessRule` objects whose rights are reduced to the three values `Read`, `Write`, and `Delete`. Reading rights such as `ReadData`, which on a folder is the right to list it (`ListDirectory`), `ReadAttributes`, or `Traverse` become `Read`, changing rights such as `CreateFiles`, `WriteAttributes`, `ChangePermissions`, or `TakeOwnership` become `Write`, and `Delete` and `DeleteSubdirectoriesAndFiles` become `Delete`; `FullControl` becomes all three. The result answers who may read, change, or delete in a folder without the detail of the full ACL.</maml:para>
<maml:para>The second simplification is that repetitions are left out. The first folder the cmdlet processes is reported with all of its entries, and for every folder that follows only the entries are reported that its parent folder does not already cover. An entry is covered when the parent has an entry for the same account and access type that includes at least the same simple rights. This makes a recursive listing show where permissions actually change instead of repeating the inherited ones on every level, and it requires the parent folder to be processed before its children, which `Get-ChildItem`, `Get-ChildItem2`, and `Get-Item2` do by default.</maml:para>
<maml:para>`-IncludeRootFolder` is on by default and adds the parent folder of the first path as the baseline for the comparison, which is why the first result usually belongs to the folder above the one that was asked for. Use `-IncludeRootFolder:$false` to start the comparison at the first path itself.</maml:para>
<maml:para>The cmdlet only processes folders; a path that points to a file is skipped silently, while the security descriptor of a file is reported. Relative paths are resolved against the current location, and the current location is used when `-Path` is omitted. `-ExcludeInherited`, `-ExcludeExplicit`, and `-Account` work as in `Get-NTFSAccess`. With `-SecurityDescriptor`, the cmdlet reports the entries of a `Security2.FileSystemSecurity2` object that `Get-NTFSSecurityDescriptor` returned, without comparing them with a parent folder.</maml:para>
@ -6785,7 +6785,7 @@ PS C:\Data&gt; Get-NTFSSecurityDescriptor</dev:code>
<maml:description>
<maml:para>The `Move-Item2` cmdlet moves the items in `-Path` to the location in `-Destination`. It is the long-path counterpart of the built-in `Move-Item` cmdlet: it works through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), so source and destination may be longer than the 260-character `MAX_PATH` limit. Files and folders can both be moved, and a folder is moved with everything it contains.</maml:para>
<maml:para>How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and moves it into that folder. In every other case the value is the full path of the new item, which lets you move and rename in one step, or rename an item in place. `-Destination` is resolved against the current location once, when the cmdlet starts.</maml:para>
<maml:para>Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-WhatIf`, the cmdlet names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. The folder that is to contain the moved item must exist; otherwise the cmdlet writes an error that names that folder.</maml:para>
<maml:para>Without `-Force`, the cmdlet checks whether a file or folder already exists at the destination and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-WhatIf`, the cmdlet names an existing destination in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. The folder that is to contain the moved item must exist; otherwise the cmdlet writes an error that names that folder, or, with `-WhatIf`, a verbose message.</maml:para>
<maml:para>The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`.</maml:para>
</maml:description>
<command:syntax>

31
Tests/ItemCmdlets.Tests.ps1

@ -316,6 +316,37 @@ Describe 'Copy-Item2, Move-Item2, and Remove-Item2 with several paths' {
@($messages | Where-Object -FilterScript { "$_" -like "*'$existing' already exists*" }) | Should -HaveCount 1
}
# Before 5.0.0-rc6, -WhatIf didn't tell that the operation would fail because the folder of the destination is
# missing.
It '<_> should name the missing folder of the destination in a verbose message with -WhatIf, and write no error' -ForEach @('Copy-Item2', 'Move-Item2') {
$missingFolder = Join-Path -Path $folder -ChildPath 'MissingFolder'
$target = Join-Path -Path $missingFolder -ChildPath 'Item'
Assert-TestSandboxPath -Sandbox $sandbox -Path $missingFolder, $target
$messages = & $_ -Path $first -Destination $target -WhatIf -Verbose -ErrorVariable itemErrors -ErrorAction SilentlyContinue 4>&1
$itemErrors | Should -BeNullOrEmpty
@($messages | Where-Object -FilterScript { "$_" -like "*'$missingFolder' does not exist*" }) | Should -HaveCount 1
$first | Should -Exist
$missingFolder | Should -Not -Exist
}
# The folder of a destination on a share that doesn't exist is the share itself, which the error names.
It '<Command> should name the missing share of a UNC destination' -ForEach @(
@{ Command = 'Copy-Item2'; ErrorId = 'CopyError' }
@{ Command = 'Move-Item2'; ErrorId = 'MoveError' }
) {
$missingShare = '\\localhost\NTFSSecurityMissing-{0}' -f [guid]::NewGuid().ToString('N').Substring(0, 8)
$target = Join-Path -Path $missingShare -ChildPath 'Item.txt'
& $Command -Path $first -Destination $target -ErrorVariable itemErrors -ErrorAction SilentlyContinue
$itemErrors | Should -HaveCount 1
$itemErrors[0].FullyQualifiedErrorId | Should -BeLike "$ErrorId,*"
$itemErrors[0].Exception.Message | Should -BeLike "*'$missingShare'*"
$first | Should -Exist
}
}
Describe 'Copy-Item2' {

20
Tests/Repository.Tests.ps1

@ -107,3 +107,23 @@ Describe 'Release metadata' {
Should -Not -Match '\d+\.\d+\.\d+-[A-Za-z]'
}
}
Describe 'Invoke-TestsAsBasicUser.ps1' {
BeforeAll {
# Only the parameters of the script, so that a test binds them without running the tests as a basic user
$path = Join-Path -Path $PSScriptRoot -ChildPath '..\.github\scripts\Invoke-TestsAsBasicUser.ps1'
$tokens = $parseErrors = $null
$ast = [System.Management.Automation.Language.Parser]::ParseFile($path, [ref] $tokens, [ref] $parseErrors)
$bindParameters = [scriptblock]::Create($ast.ParamBlock.Extent.Text)
}
# The title goes into a quoted argument of cmd.exe, which expands environment variables also inside quotes.
It 'Should refuse a title with the character <_>, which would change the command line of cmd.exe' -ForEach @('%', '"') {
{ & $bindParameters -ResultPath 'TestResults\Refused.xml' -Title "CI run $_ 1" } |
Should -Throw -ExpectedMessage "*'Title'*"
}
It 'Should accept the title <_> of the CI workflow' -ForEach @('Windows PowerShell 5.1 as a basic user', 'PowerShell 7 as a basic user') {
{ & $bindParameters -ResultPath 'TestResults\Accepted.xml' -Title $_ } | Should -Not -Throw
}
}

Loading…
Cancel
Save