From 987334e415cfd50c55e264a8c9a5334169bbecda Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Wed, 21 Oct 2026 11:40:43 +0000 Subject: [PATCH] fix(security2): keep the descriptor in the conversions from FileSecurity and DirectorySecurity Converting a FileSecurity or DirectorySecurity object to FileSystemSecurity2 (for example `[Security2.FileSystemSecurity2] $fileSecurity`) always threw, because the conversion ignored the descriptor it was given and built the result from an empty path, which AlphaFS rejects. The conversion now wraps the descriptor that was passed: SecurityDescriptor is that same instance, IsFile reflects whether it came from a FileSecurity or a DirectorySecurity, the round trip back to FileSecurity/DirectorySecurity returns the same instance, and Equals/GetHashCode keep comparing descriptors by reference. The resulting object has no item, since it was never read from a file or folder: FullName and Name are now null instead of throwing a NullReferenceException, and Write() throws a clear InvalidOperationException instead of a NullReferenceException deep in SetSecurity. A caller can still write such an object by setting its Item property first, or by calling Write(FileSystemInfo) or Write(string) directly, neither of which depends on an item being set. No cmdlet reaches this conversion; only a .NET caller of Security2.dll that converts a FileSecurity/DirectorySecurity it built itself is affected. Public signatures are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- Security2/FileSystem/FileSystemSecurity2.cs | 46 ++++++++-- Tests/ObjectApis.Tests.ps1 | 93 +++++++++++++++++++++ 2 files changed, 134 insertions(+), 5 deletions(-) diff --git a/Security2/FileSystem/FileSystemSecurity2.cs b/Security2/FileSystem/FileSystemSecurity2.cs index cf905ba..9e29088 100644 --- a/Security2/FileSystem/FileSystemSecurity2.cs +++ b/Security2/FileSystem/FileSystemSecurity2.cs @@ -22,9 +22,12 @@ namespace Security2 set { item = value; } } - public string FullName { get { return item.FullName; } } + // An object that wraps a descriptor directly, such as the conversion from FileSecurity or DirectorySecurity, + // has no item and so no path to report; null reads more naturally here than an empty string, which would look + // like the path of an item whose name happens to be empty. + public string FullName { get { return item != null ? item.FullName : null; } } - public string Name { get { return item.Name; } } + public string Name { get { return item != null ? item.Name : null; } } public bool IsFile { get { return isFile; } } @@ -39,6 +42,18 @@ namespace Security2 RememberSections(); } + // Wraps a descriptor that a caller created directly, such as the conversions from FileSecurity and + // DirectorySecurity do; the object has no item. The descriptor is treated as if every section had been read, + // since there was no partial read: WriteChanges compares against it as a whole. + private FileSystemSecurity2(FileSystemSecurity sd) + { + this.sd = sd; + isFile = sd is FileSecurity; + sections = AccessControlSections.All; + + RememberSections(); + } + public FileSystemSecurity2(FileSystemInfo item) { this.item = item; @@ -195,6 +210,8 @@ namespace Security2 // that is read alone, and writing them back fails for an owner that the user cannot assign (#34). public void Write() { + RequireItem(); + SetSecurity(item, sd, sections); RememberSections(); @@ -204,6 +221,8 @@ namespace Security2 // unchanged owner that the user cannot assign isn't written back (#34). Without a change, it writes nothing. internal void WriteChanges() { + RequireItem(); + var changedSections = ChangedSections; if (changedSections == AccessControlSections.None) { @@ -215,6 +234,20 @@ namespace Security2 RememberSections(); } + // Write() and WriteChanges() write to the item that the descriptor was read from; an object that a conversion + // from FileSecurity or DirectorySecurity created has none. Without this check they would fail with a + // NullReferenceException deep in SetSecurity instead of saying what is missing. + private void RequireItem() + { + if (item == null) + { + throw new InvalidOperationException( + "This security descriptor has no item to write to: it was created directly from a FileSecurity or " + + "DirectorySecurity object, not read from a file or folder. Set the Item property first, or call " + + "Write(FileSystemInfo) or Write(string) to choose where to write it."); + } + } + // Writes the sections that the descriptor was read with to another item, like Write(). public void Write(FileSystemInfo item) { @@ -243,14 +276,17 @@ namespace Security2 #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. + // gave null, and the hash code threw a NullReferenceException. Before 5.0.0, the conversions the other way + // (from FileSecurity/DirectorySecurity) ignored the descriptor that was passed and returned an object built + // from an empty path, which always threw; they now keep the descriptor that was passed, see the private + // constructor above. public static implicit operator FileSecurity(FileSystemSecurity2 fs2) { return fs2.sd as FileSecurity; } public static implicit operator FileSystemSecurity2(FileSecurity fs) { - return new FileSystemSecurity2(new FileInfo("")); + return new FileSystemSecurity2((FileSystemSecurity)fs); } public static implicit operator DirectorySecurity(FileSystemSecurity2 fs2) @@ -259,7 +295,7 @@ namespace Security2 } public static implicit operator FileSystemSecurity2(DirectorySecurity fs) { - return new FileSystemSecurity2(new DirectoryInfo("")); + return new FileSystemSecurity2((FileSystemSecurity)fs); } // Like the descriptors of .NET, two objects are equal when they wrap the same descriptor; a descriptor of .NET diff --git a/Tests/ObjectApis.Tests.ps1 b/Tests/ObjectApis.Tests.ps1 index 575e34c..eafb0e1 100644 --- a/Tests/ObjectApis.Tests.ps1 +++ b/Tests/ObjectApis.Tests.ps1 @@ -797,6 +797,99 @@ Describe 'Owner and descriptor objects' { } } +# Before 5.0.0, both conversions ignored the descriptor that was passed and returned a FileSystemSecurity2 that wrapped +# a FileInfo or a DirectoryInfo built from an empty path, which AlphaFS rejects, so the conversion always threw. The +# object that the fixed conversion returns has no item: FullName and Name, which have no path to report, are null, +# which reads most naturally as "nothing to name". +Describe 'The FileSystemSecurity2 conversions from FileSecurity and DirectorySecurity' { + BeforeAll { + function New-RawFileSystemSecurity { + [Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Builds an in-memory descriptor only.' + )] + param ([string] $Kind) + + $raw = New-Object -TypeName "System.Security.AccessControl.$Kind" + $rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( + $sid, [System.Security.AccessControl.FileSystemRights]::ReadData, + [System.Security.AccessControl.AccessControlType]::Allow + ) + $raw.AddAccessRule($rule) + $raw + } + } + + It 'Should wrap the descriptor that was passed, report IsFile as , and round-trip the same instance' -ForEach @( + @{ Kind = 'FileSecurity'; IsFile = $true } + @{ Kind = 'DirectorySecurity'; IsFile = $false } + ) { + $raw = New-RawFileSystemSecurity -Kind $Kind + $targetType = [type] "System.Security.AccessControl.$Kind" + + $wrapped = [Security2.FileSystemSecurity2] $raw + + [object]::ReferenceEquals($wrapped.SecurityDescriptor, $raw) | Should -BeTrue + $wrapped.IsFile | Should -Be $IsFile + [object]::ReferenceEquals(($wrapped -as $targetType), $raw) | Should -BeTrue + $wrapped.FullName | Should -Be $null + $wrapped.Name | Should -Be $null + } + + It 'Should compare two conversions of the same descriptor as equal, and of different descriptors as unequal' -ForEach @( + @{ Kind = 'FileSecurity' } + @{ Kind = 'DirectorySecurity' } + ) { + $raw = New-RawFileSystemSecurity -Kind $Kind + + $first = [Security2.FileSystemSecurity2] $raw + $second = [Security2.FileSystemSecurity2] $raw + $other = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + + $first.Equals($second) | Should -BeTrue + $first.GetHashCode() | Should -Be $second.GetHashCode() + $first.Equals($other) | Should -BeFalse + } + + It 'Should throw an InvalidOperationException, not a NullReferenceException, when a without an item is written' -ForEach @( + @{ Kind = 'FileSecurity' } + @{ Kind = 'DirectorySecurity' } + ) { + $wrapped = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + + $failure = { $wrapped.Write() } | Should -Throw -PassThru + + $failure.Exception.GetBaseException() | Should -BeOfType [System.InvalidOperationException] + $failure.Exception.GetBaseException().Message | Should -BeLike '*item*' + } + + It 'Should write a without an item once the caller sets Item' -ForEach @( + @{ Kind = 'FileSecurity'; Directory = $false } + @{ Kind = 'DirectorySecurity'; Directory = $true } + ) { + $path = New-TestSandboxItem -Sandbox $sandbox -Name 'ConvertedWriteItem' -Directory:$Directory + $wrapped = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + $wrapped.Item = Get-Item2 -Path $path + + $wrapped.Write() + + @((Get-Acl -LiteralPath $path).GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | + Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' }) | Should -HaveCount 1 + } + + It 'Should write a without an item directly to a path' -ForEach @( + @{ Kind = 'FileSecurity'; Directory = $false } + @{ Kind = 'DirectorySecurity'; Directory = $true } + ) { + $path = New-TestSandboxItem -Sandbox $sandbox -Name 'ConvertedWritePath' -Directory:$Directory + $wrapped = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + + $wrapped.Write([string] $path) + + @((Get-Acl -LiteralPath $path).GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | + Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' }) | Should -HaveCount 1 + } +} + Describe 'Generic access rights and identity errors' { It 'Should map the generic mask to the file system rights ' -ForEach @( @{ Mask = '80000000'; Expected = '00120089' }