Browse Source

fix: address the second review of 5.0.0-rc6

- The conversions of a FileSystemSecurity2 to FileSecurity and
  DirectorySecurity returned fields that were never set, so they gave
  null; they return the descriptor, and the dead fields are gone
  (finding 1).
- Equals of the entries and descriptors accepted the .NET type as well,
  which doesn't know the wrapper, so equality depended on the direction;
  only an object of the module can now be equal (finding 2).
- Invoke-TestsAsBasicUser.ps1 refuses a title with a line break, also a
  final one, which $ let through (finding 6).
- The InheritedFrom test of the access entries checks a known parent
  folder with two explicit entries in front; it fails on acfe3af
  (finding 7).

Checked and kept: a callback ACE before the inherited entries doesn't
shift InheritedFrom, because .NET returns it as a rule (finding 3).

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 4 days ago
parent
commit
7b0781ff8b
  1. 9
      .github/scripts/Invoke-TestsAsBasicUser.ps1
  2. 3
      CHANGELOG.md
  3. 7
      Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs
  4. 7
      Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.cs
  5. 17
      Security2/FileSystem/FileSystemSecurity2.cs
  6. 28
      Tests/Access.Tests.ps1
  7. 17
      Tests/Repository.Tests.ps1
  8. 20
      Tests/SecurityDescriptor.Tests.ps1

9
.github/scripts/Invoke-TestsAsBasicUser.ps1

@ -14,8 +14,8 @@
Specifies the path of the result file in the NUnit format. Specifies the path of the result file in the NUnit format.
.PARAMETER Title .PARAMETER Title
Specifies the heading of the test results in the job summary. It can't contain a double quote or a percent sign, Specifies the heading of the test results in the job summary. It can't contain a double quote, a percent sign, or a
which would change the command line of cmd.exe. line break, which would change the command line of cmd.exe.
.EXAMPLE .EXAMPLE
.\.github\scripts\Invoke-TestsAsBasicUser.ps1 -ResultPath TestResults\WindowsPowerShell-BasicUser.xml -Title 'Windows PowerShell 5.1 as a basic user' .\.github\scripts\Invoke-TestsAsBasicUser.ps1 -ResultPath TestResults\WindowsPowerShell-BasicUser.xml -Title 'Windows PowerShell 5.1 as a basic user'
@ -31,8 +31,9 @@ param (
[Parameter(Mandatory)] [Parameter(Mandatory)]
[ValidateNotNullOrEmpty()] [ValidateNotNullOrEmpty()]
# cmd.exe gets the title in a quoted argument and expands environment variables also inside quotes. # cmd.exe gets the title in a quoted argument, expands environment variables also inside quotes, and ends the
[ValidatePattern('^[^"%]+$')] # command at a line break. \z, unlike $, doesn't match before a final line feed.
[ValidatePattern('\A[^"%\r\n]+\z')]
[string] [string]
$Title $Title
) )

3
CHANGELOG.md

@ -321,7 +321,8 @@ The format is based on
`Select-Object -Unique` and `Compare-Object`, stopped with an `Select-Object -Unique` and `Compare-Object`, stopped with an
`InvalidCastException`, and a security descriptor as the key of a `InvalidCastException`, and a security descriptor as the key of a
hashtable with a `NullReferenceException`. Two objects are now equal when hashtable with a `NullReferenceException`. Two objects are now equal when
they hold the same entry or descriptor, as in .NET they hold the same entry or descriptor, as in .NET. Converting a security
descriptor to `FileSecurity` or `DirectorySecurity` returned `$null`
- Fix `InheritedFrom` of `Get-NTFSAccess` and `Get-NTFSAudit`: with - Fix `InheritedFrom` of `Get-NTFSAccess` and `Get-NTFSAudit`: with
`-ExcludeExplicit`, each inherited entry showed the source of another `-ExcludeExplicit`, each inherited entry showed the source of another
entry, and `Get-NTFSAccess -SecurityDescriptor` stopped with an entry, and `Get-NTFSAccess -SecurityDescriptor` stopped with an

7
Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.cs

@ -48,13 +48,12 @@ namespace Security2
return new FileSystemAccessRule2(ace); return new FileSystemAccessRule2(ace);
} }
// Like the entries of .NET, two objects are equal when they wrap the same entry. A cast instead of "as" // Like the entries of .NET, two objects are equal when they wrap the same entry; an entry of .NET is never equal,
// threw an InvalidCastException for any other type, also for -eq in PowerShell. // so that equality is the same in both directions. A cast instead of "as" threw an InvalidCastException.
public override bool Equals(object obj) public override bool Equals(object obj)
{ {
var other = obj as FileSystemAccessRule2; var other = obj as FileSystemAccessRule2;
var rule = other != null ? other.fileSystemAccessRule : obj as FileSystemAccessRule; return other != null && ReferenceEquals(fileSystemAccessRule, other.fileSystemAccessRule);
return rule != null && ReferenceEquals(fileSystemAccessRule, rule);
} }
public override int GetHashCode() public override int GetHashCode()
{ {

7
Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.cs

@ -48,13 +48,12 @@ namespace Security2
{ {
return new FileSystemAuditRule2(ace); return new FileSystemAuditRule2(ace);
} }
// Like the entries of .NET, two objects are equal when they wrap the same entry. A cast instead of "as" // Like the entries of .NET, two objects are equal when they wrap the same entry; an entry of .NET is never equal,
// threw an InvalidCastException for any other type, also for -eq in PowerShell. // so that equality is the same in both directions. A cast instead of "as" threw an InvalidCastException.
public override bool Equals(object obj) public override bool Equals(object obj)
{ {
var other = obj as FileSystemAuditRule2; var other = obj as FileSystemAuditRule2;
var rule = other != null ? other.fileSystemAuditRule : obj as FileSystemAuditRule; return other != null && ReferenceEquals(fileSystemAuditRule, other.fileSystemAuditRule);
return rule != null && ReferenceEquals(fileSystemAuditRule, rule);
} }
public override int GetHashCode() public override int GetHashCode()
{ {

17
Security2/FileSystem/FileSystemSecurity2.cs

@ -7,8 +7,6 @@ namespace Security2
{ {
public class FileSystemSecurity2 public class FileSystemSecurity2
{ {
protected FileSecurity fileSecurityDescriptor;
protected DirectorySecurity directorySecurityDescriptor;
protected FileSystemInfo item; protected FileSystemInfo item;
protected FileSystemSecurity sd; protected FileSystemSecurity sd;
protected AccessControlSections sections; protected AccessControlSections sections;
@ -236,9 +234,11 @@ namespace Security2
} }
#region Conversion #region Conversion
// The descriptor is in sd. Before 5.0.0-rc6, these conversions returned fields that were never set, so they
// gave null, and the hash code threw a NullReferenceException.
public static implicit operator FileSecurity(FileSystemSecurity2 fs2) public static implicit operator FileSecurity(FileSystemSecurity2 fs2)
{ {
return fs2.fileSecurityDescriptor; return fs2.sd as FileSecurity;
} }
public static implicit operator FileSystemSecurity2(FileSecurity fs) public static implicit operator FileSystemSecurity2(FileSecurity fs)
{ {
@ -247,21 +247,20 @@ namespace Security2
public static implicit operator DirectorySecurity(FileSystemSecurity2 fs2) public static implicit operator DirectorySecurity(FileSystemSecurity2 fs2)
{ {
return fs2.directorySecurityDescriptor; return fs2.sd as DirectorySecurity;
} }
public static implicit operator FileSystemSecurity2(DirectorySecurity fs) public static implicit operator FileSystemSecurity2(DirectorySecurity fs)
{ {
return new FileSystemSecurity2(new DirectoryInfo("")); return new FileSystemSecurity2(new DirectoryInfo(""));
} }
// Like the descriptors of .NET, two objects are equal when they wrap the same descriptor. The descriptor is in // Like the descriptors of .NET, two objects are equal when they wrap the same descriptor; a descriptor of .NET
// sd; fileSecurityDescriptor is never set, so its hash code threw a NullReferenceException, and the cast an // is never equal, so that equality is the same in both directions. A cast instead of "as" threw an
// InvalidCastException for any other type. // InvalidCastException.
public override bool Equals(object obj) public override bool Equals(object obj)
{ {
var other = obj as FileSystemSecurity2; var other = obj as FileSystemSecurity2;
var descriptor = other != null ? other.sd : obj as FileSystemSecurity; return other != null && ReferenceEquals(sd, other.sd);
return descriptor != null && ReferenceEquals(sd, descriptor);
} }
public override int GetHashCode() public override int GetHashCode()
{ {

28
Tests/Access.Tests.ps1

@ -809,6 +809,15 @@ Describe 'Comparing access entries' {
@(@($entries) + $entries[0] | Select-Object -Unique) | Should -HaveCount $entries.Count @(@($entries) + $entries[0] | Select-Object -Unique) | Should -HaveCount $entries.Count
Compare-Object -ReferenceObject $entries -DifferenceObject $entries | Should -BeNullOrEmpty Compare-Object -ReferenceObject $entries -DifferenceObject $entries | Should -BeNullOrEmpty
} }
# The entry of .NET doesn't know the object of the module, so equality in one direction only would make a hashtable
# lookup depend on which of the two is the key.
It 'Should be equal only to an entry of the module, in both directions' {
$raw = [System.Security.AccessControl.FileSystemAccessRule] $entries[0]
$entries[0].Equals($raw) | Should -BeFalse
$raw.Equals($entries[0]) | Should -BeFalse
}
} }
# Before 5.0.0-rc6, -ExcludeExplicit gave each inherited entry the source of another entry, and Get-NTFSAccess stopped # Before 5.0.0-rc6, -ExcludeExplicit gave each inherited entry the source of another entry, and Get-NTFSAccess stopped
@ -840,6 +849,25 @@ Describe 'InheritedFrom of access entries' {
} }
} }
# Two explicit entries before the inherited ones; before 5.0.0-rc6, -ExcludeExplicit shifted the sources by two.
It 'Should name the folder of an inheritable entry, also with -ExcludeExplicit' {
$parent = New-TestSandboxItem -Sandbox $sandbox -Name 'InheritedFromParent' -Directory
Add-NTFSAccess -Path $parent -Account 'S-1-5-32-546' -AccessRights ReadData -AppliesTo ThisFolderSubfoldersAndFiles
$child = Join-Path -Path $parent -ChildPath 'Child.txt'
Assert-TestSandboxPath -Sandbox $sandbox -Path $child
Set-Content -LiteralPath $child -Value 'Child'
Add-NTFSAccess -Path $child -Account 'S-1-1-0' -AccessRights ReadData
Add-NTFSAccess -Path $child -Account 'S-1-5-32-545' -AccessRights ReadData
$all = @(Get-NTFSAccess -Path $child | Where-Object -FilterScript { $_.IsInherited -and "$($_.Account.Sid)" -eq 'S-1-5-32-546' })
$inherited = @(Get-NTFSAccess -Path $child -ExcludeExplicit | Where-Object -FilterScript { "$($_.Account.Sid)" -eq 'S-1-5-32-546' })
$all | Should -HaveCount 1
$all[0].InheritedFrom | Should -Be $parent
$inherited | Should -HaveCount 1
$inherited[0].InheritedFrom | Should -Be $parent
}
It 'Should read a security descriptor with audit entries and name the same folders' -Skip:(-not $holdsSecurityPrivilege) { It 'Should read a security descriptor with audit entries and name the same folders' -Skip:(-not $holdsSecurityPrivilege) {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'InheritedFromAudit' $file = New-TestSandboxItem -Sandbox $sandbox -Name 'InheritedFromAudit'
Add-NTFSAccess -Path $file -Account 'S-1-1-0' -AccessRights ReadData Add-NTFSAccess -Path $file -Account 'S-1-1-0' -AccessRights ReadData

17
Tests/Repository.Tests.ps1

@ -117,9 +117,20 @@ Describe 'Invoke-TestsAsBasicUser.ps1' {
$bindParameters = [scriptblock]::Create($ast.ParamBlock.Extent.Text) $bindParameters = [scriptblock]::Create($ast.ParamBlock.Extent.Text)
} }
# The title goes into a quoted argument of cmd.exe, which expands environment variables also inside quotes. # The title goes into a quoted argument of cmd.exe, which expands environment variables also inside quotes and ends
It 'Should refuse a title with the character <_>, which would change the command line of cmd.exe' -ForEach @('%', '"') { # the command at a line break.
{ & $bindParameters -ResultPath 'TestResults\Refused.xml' -Title "CI run $_ 1" } | It 'Should refuse a title with the character <Name>, which would change the command line of cmd.exe' -ForEach @(
@{ Name = '%'; Character = '%' }
@{ Name = 'double quote'; Character = '"' }
@{ Name = 'line feed'; Character = "`n" }
@{ Name = 'carriage return'; Character = "`r" }
) {
{ & $bindParameters -ResultPath 'TestResults\Refused.xml' -Title "CI run $Character 1" } |
Should -Throw -ExpectedMessage "*'Title'*"
}
It 'Should refuse a title that ends with a line break' {
{ & $bindParameters -ResultPath 'TestResults\Refused.xml' -Title "CI run`n" } |
Should -Throw -ExpectedMessage "*'Title'*" Should -Throw -ExpectedMessage "*'Title'*"
} }

20
Tests/SecurityDescriptor.Tests.ps1

@ -311,4 +311,24 @@ Describe 'Comparing security descriptors' {
$table[$sd] | Should -Be 'first' $table[$sd] | Should -Be 'first'
$table.Count | Should -Be 2 $table.Count | Should -Be 2
} }
# Before 5.0.0-rc6, the conversion returned a field that was never set, so it gave $null.
It 'Should convert to the security object of .NET that it holds' {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'ConvertFile'
$folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ConvertFolder' -Directory
$fileSd = Get-NTFSSecurityDescriptor -Path $file
$folderSd = Get-NTFSSecurityDescriptor -Path $folder
[object]::ReferenceEquals([System.Security.AccessControl.FileSecurity] $fileSd, $fileSd.SecurityDescriptor) | Should -BeTrue
[object]::ReferenceEquals([System.Security.AccessControl.DirectorySecurity] $folderSd, $folderSd.SecurityDescriptor) | Should -BeTrue
}
It 'Should be equal only to a descriptor of the module, in both directions' {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'Symmetric'
$sd = Get-NTFSSecurityDescriptor -Path $file
$raw = $sd.SecurityDescriptor
$sd.Equals($raw) | Should -BeFalse
$raw.Equals($sd) | Should -BeFalse
}
} }

Loading…
Cancel
Save