From a0362c359c57ce1fc8a576fa292ef4a9d28c7110 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Wed, 21 Oct 2026 11:51:53 +0000 Subject: [PATCH] fix(security2): make the path overloads of AddFileSystemAccessRule and AddFileSystemAuditRule write at once AddFileSystemAccessRule(string, List, ...) and AddFileSystemAuditRule(string, List, ...) were iterator methods (yield return), so nothing happened until the caller enumerated the result: called as a statement, or assigned to a variable that the caller never reads, they changed nothing. Their siblings, the overloads for FileSystemInfo and FileSystemSecurity2 with a list, and the single-account overloads, already built a list and wrote at once. Both overloads now build the list and add the entries before returning, like their siblings; a call that is never enumerated now adds the entries just the same. An exception that adding an entry raises changes only in timing: it is now thrown by the call itself, not by the later enumeration of its result. The returned collection still holds one rule per account, in the order of the account list, for both a file path and a folder path. Public signatures are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- ...temAccessRule2.AddFileSystemAccessRules.cs | 16 ++++++++++-- ...ystemAuditRule2.AddFileSystemAuditRules.cs | 16 ++++++++++-- Tests/ObjectApis.Tests.ps1 | 25 ++++++++++++++----- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.AddFileSystemAccessRules.cs b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.AddFileSystemAccessRules.cs index cd8f067..d5ba957 100644 --- a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.AddFileSystemAccessRules.cs +++ b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.AddFileSystemAccessRules.cs @@ -87,17 +87,27 @@ namespace Security2 return ace; } + /// + /// Adds one access entry per account to the item at the path, and returns them in the order of + /// . Before 5.0.0, this overload was an iterator (a yield return in the loop + /// below), so nothing happened until the caller enumerated the result; called as a statement, or assigned to + /// a variable that is never read, it changed nothing. It now builds the list and writes the entries before + /// returning, like its FileSystemInfo and FileSystemSecurity2 siblings; an exception that adding an entry + /// raises is now thrown by this call, not by the later enumeration. + /// public static IEnumerable AddFileSystemAccessRule(string path, List accounts, FileSystemRights2 rights, AccessControlType type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags) { if (type == AccessControlType.Allow) rights = rights | FileSystemRights2.Synchronize; + var aces = new List(); + if (File.Exists(path)) { var item = new FileInfo(path); foreach (var account in accounts) { - yield return AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags); + aces.Add(AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags)); } } else @@ -105,9 +115,11 @@ namespace Security2 var item = new DirectoryInfo(path); foreach (var account in accounts) { - yield return AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags); + aces.Add(AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags)); } } + + return aces; } public static void AddFileSystemAccessRule(FileSystemAccessRule2 rule) diff --git a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.AddFileSystemAuditRules.cs b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.AddFileSystemAuditRules.cs index 59e1b9b..5ed1c8a 100644 --- a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.AddFileSystemAuditRules.cs +++ b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.AddFileSystemAuditRules.cs @@ -78,14 +78,24 @@ namespace Security2 return aces; } + /// + /// Adds one audit entry per account to the item at the path, and returns them in the order of + /// . Before 5.0.0, this overload was an iterator (a yield return in the loop + /// below), so nothing happened until the caller enumerated the result; called as a statement, or assigned to + /// a variable that is never read, it changed nothing. It now builds the list and writes the entries before + /// returning, like its FileSystemInfo and FileSystemSecurity2 siblings; an exception that adding an entry + /// raises is now thrown by this call, not by the later enumeration. + /// public static IEnumerable AddFileSystemAuditRule(string path, List accounts, FileSystemRights2 rights, AuditFlags type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags) { + var aces = new List(); + if (File.Exists(path)) { var item = new FileInfo(path); foreach (var account in accounts) { - yield return AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags); + aces.Add(AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags)); } } else @@ -93,9 +103,11 @@ namespace Security2 var item = new DirectoryInfo(path); foreach (var account in accounts) { - yield return AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags); + aces.Add(AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags)); } } + + return aces; } public static void AddFileSystemAuditRule(FileSystemAuditRule2 rule) diff --git a/Tests/ObjectApis.Tests.ps1 b/Tests/ObjectApis.Tests.ps1 index e2fc353..76f65ce 100644 --- a/Tests/ObjectApis.Tests.ps1 +++ b/Tests/ObjectApis.Tests.ps1 @@ -395,9 +395,11 @@ Describe 'Access rule helpers that take a path' { $entries[0].FileSystemRights | Should -Be ([System.Security.AccessControl.FileSystemRights] 'ReadData, Synchronize') } - # The overload that takes a path returns an iterator, so the caller must enumerate the result to write the entries. - # The overloads that take an item write them at once; this test doesn't pin the difference. - It 'Should add the entries of several accounts to a by its path when the result is enumerated' -ForEach @( + # Before 5.0.0, the overload that takes a path was an iterator (yield return) that changed nothing until the + # caller enumerated the result, so a call as a statement, or an assignment that the caller never reads, wrote + # nothing; the overloads that take an item or a FileSystemSecurity2 were never lazy. It now writes at once, like + # its siblings, so the entries exist before $pending is ever touched. + It 'Should add the entries of several accounts to a by its path at once, before the result is ever enumerated' -ForEach @( @{ Kind = 'file'; Directory = $false } @{ Kind = 'folder'; Directory = $true } ) { @@ -408,9 +410,13 @@ Describe 'Access rule helpers that take a path' { $path, $accounts, [Security2.FileSystemRights2]::ReadData, $allow, $noInheritance, $noPropagation ) - @($pending) | Should -HaveCount 2 @(Get-ExplicitEntries -Path $path) | Should -HaveCount 1 @(Get-ExplicitEntries -Path $path -Account 'S-1-5-32-545') | Should -HaveCount 1 + + $added = @($pending) + $added | Should -HaveCount 2 + $added[0].Account.Sid | Should -BeExactly $identity.Sid + $added[1].Account.Sid | Should -BeExactly $users.Sid } It 'Should add the deny entries of several accounts without Synchronize when the result is enumerated' { @@ -589,7 +595,10 @@ Describe 'Audit rule helpers that take a path' -Skip:(-not $holdsSecurityPrivile $found[0].FullName | Should -BeExactly $path } - It 'Should add the entries of several accounts to a by its path when the result is enumerated, and remove them again' -ForEach @( + # Before 5.0.0, the overload that takes a path was an iterator (yield return) that changed nothing until the + # caller enumerated the result; it now writes at once, like its siblings, so the entries exist before $pending is + # ever touched. This overload needs the Security privilege like its neighbors, so the whole Describe skips without it. + It 'Should add the entries of several accounts to a by its path at once, before the result is ever enumerated, and remove them again' -ForEach @( @{ Kind = 'file'; Directory = $false } @{ Kind = 'folder'; Directory = $true } ) { @@ -602,9 +611,13 @@ Describe 'Audit rule helpers that take a path' -Skip:(-not $holdsSecurityPrivile $path, $accounts, [Security2.FileSystemRights2]::Delete, $success, $noInheritance, $noPropagation ) - @($pending) | Should -HaveCount 2 @(Get-AuditEntries -Path $path) | Should -HaveCount 1 @(Get-AuditEntries -Path $path -Account 'S-1-5-32-545') | Should -HaveCount 1 + $added = @($pending) + $added | Should -HaveCount 2 + $added[0].Account.Sid | Should -BeExactly $identity.Sid + $added[1].Account.Sid | Should -BeExactly $users.Sid + [Security2.FileSystemAuditRule2]::RemoveFileSystemAuditRule( $path, $identity, [Security2.FileSystemRights2]::Delete, $success, $noInheritance, $noPropagation, $true )