Browse Source

fix: address the review of 5.0.0-rc3

- Set-NTFSSecurityDescriptor -Verbose names the sections that it writes,
  or says that it writes nothing for an unchanged descriptor; its page
  says "since it was read or last written" (review F-02).
- The pages of Enable-NTFSAccessInheritance, Disable-NTFSAccessInheritance,
  and Set-NTFSInheritance get the #34 note, like the other fixed cmdlets
  (review F-06).
- Set-TestOwner throws its own error when icacls fails, also when the
  caller uses -ErrorAction Stop in Windows PowerShell (review F-07).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
pull/112/head
Raimund Andree 5 days ago
parent
commit
0947683372
  1. 7
      CHANGELOG.md
  2. 2
      Docs/Cmdlets/Disable-NTFSAccessInheritance.md
  3. 2
      Docs/Cmdlets/Enable-NTFSAccessInheritance.md
  4. 2
      Docs/Cmdlets/Set-NTFSInheritance.md
  5. 2
      Docs/Cmdlets/Set-NTFSSecurityDescriptor.md
  6. 11
      NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs
  7. 5
      NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml
  8. 23
      Tests/SecurityDescriptor.Tests.ps1
  9. 9
      Tests/TestHelpers.Tests.ps1
  10. 3
      Tests/TestHelpers.psm1

7
CHANGELOG.md

@ -68,9 +68,10 @@ The format is based on
- Write only the sections of a security descriptor that changed since it
was read in `Set-NTFSSecurityDescriptor`, such as the DACL after
`Add-NTFSAccess -SecurityDescriptor`; a descriptor without changes writes
nothing. The cmdlet wrote every section that `Get-NTFSSecurityDescriptor`
had read, also an unchanged owner, which failed with error 1307 where the
account may not assign that owner
nothing, and `-Verbose` names the sections that the cmdlet writes. The
cmdlet wrote every section that `Get-NTFSSecurityDescriptor` had read,
also an unchanged owner, which failed with error 1307 where the account
may not assign that owner
([#34](https://github.com/raandree/NTFSSecurity/issues/34))
### Deprecated

2
Docs/Cmdlets/Disable-NTFSAccessInheritance.md

@ -172,6 +172,8 @@ Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
In the `Path` parameter set, the cmdlet writes only the DACL of the item and leaves its owner, its group, and its SACL as they are. Before 5.0.0, it could also write the owner back, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.
## RELATED LINKS
[Enable-NTFSAccessInheritance](Enable-NTFSAccessInheritance.md)

2
Docs/Cmdlets/Enable-NTFSAccessInheritance.md

@ -171,6 +171,8 @@ Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
In the `Path` parameter set, the cmdlet writes only the DACL of the item and leaves its owner, its group, and its SACL as they are. Before 5.0.0, it could also write the owner back, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.
## RELATED LINKS
[Disable-NTFSAccessInheritance](Disable-NTFSAccessInheritance.md)

2
Docs/Cmdlets/Set-NTFSInheritance.md

@ -198,6 +198,8 @@ Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was
Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.
In the `Path` parameter set, the cmdlet writes only the section that it changes, the DACL or the SACL, and leaves the owner and the group of the item as they are. Before 5.0.0, a change of the access inheritance could also write the owner back, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.
## RELATED LINKS
[Get-NTFSInheritance](Get-NTFSInheritance.md)

2
Docs/Cmdlets/Set-NTFSSecurityDescriptor.md

@ -21,7 +21,7 @@ Set-NTFSSecurityDescriptor [-SecurityDescriptor] <FileSystemSecurity2[]> [-PassT
The `Set-NTFSSecurityDescriptor` cmdlet writes a `Security2.FileSystemSecurity2` object to the file system. It is the final step of the security descriptor workflow: `Get-NTFSSecurityDescriptor` reads a descriptor into memory, cmdlets such as `Add-NTFSAccess`, `Remove-NTFSAccess`, `Set-NTFSOwner`, and `Disable-NTFSAccessInheritance` change that copy through their `-SecurityDescriptor` parameter, and this cmdlet applies all of those changes in a single write.
Each descriptor remembers the item it was read from, and the cmdlet writes it back to exactly that item. There is no parameter that redirects the write to a different path. The cmdlet writes only the sections of the descriptor that changed since it was read, such as the DACL after `Add-NTFSAccess`, and leaves the other sections of the item as they are, so a descriptor that you did not change writes nothing. Before 5.0.0, the cmdlet wrote every section that it had read, also an unchanged owner, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.
Each descriptor remembers the item it was read from, and the cmdlet writes it back to exactly that item. There is no parameter that redirects the write to a different path. The cmdlet writes only the sections of the descriptor that changed since it was read or last written, such as the DACL after `Add-NTFSAccess`, and leaves the other sections of the item as they are, so a descriptor that you did not change writes nothing. With `-Verbose`, the cmdlet names the sections that it writes, or says that it writes nothing. Before 5.0.0, the cmdlet wrote every section that it had read, also an unchanged owner, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.
The cmdlet produces no output unless you use `-PassThru`, which reads the item again after the write and returns a new `FileSystemSecurity2` object that reflects what is now stored on disk. Descriptors can be passed as an array or through the pipeline, and each one is processed on its own.

11
NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs

@ -1,6 +1,7 @@
using Security2;
using System;
using System.Management.Automation;
using System.Security.AccessControl;
namespace NTFSSecurity
{
@ -40,6 +41,16 @@ namespace NTFSSecurity
try
{
// Only the changed sections, so that an unchanged owner, for example, isn't written back (#34)
var changedSections = sd.ChangedSections;
if (changedSections == AccessControlSections.None)
{
WriteVerbose(string.Format("No section of the security descriptor of '{0}' changed since it was read or last written; nothing is written", sd.FullName));
}
else
{
WriteVerbose(string.Format("Writing the changed sections of the security descriptor of '{0}': {1}", sd.FullName, changedSections));
}
sd.WriteChanges();
if (passThru)

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

@ -2427,6 +2427,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:para>A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths.</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
<maml:para>In the `Path` parameter set, the cmdlet writes only the DACL of the item and leaves its owner, its group, and its SACL as they are. Before 5.0.0, it could also write the owner back, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>
@ -3044,6 +3045,7 @@ PS C:\&gt; Get-Privileges | Where-Object { $_.Privilege -in 'Backup', 'Restore',
<maml:para>A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths.</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
<maml:para>In the `Path` parameter set, the cmdlet writes only the DACL of the item and leaves its owner, its group, and its SACL as they are. Before 5.0.0, it could also write the owner back, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>
@ -9557,6 +9559,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:para>Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` could fail with the error "Nullable object must have a value".</maml:para>
<maml:para>Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled.</maml:para>
<maml:para>Before 5.0.0, `-PassThru` returned the unchanged state of an item also when the change failed, and stopped the command when the item could not be read.</maml:para>
<maml:para>In the `Path` parameter set, the cmdlet writes only the section that it changes, the DACL or the SACL, and leaves the owner and the group of the item as they are. Before 5.0.0, a change of the access inheritance could also write the owner back, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>
@ -9875,7 +9878,7 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
</command:details>
<maml:description>
<maml:para>The `Set-NTFSSecurityDescriptor` cmdlet writes a `Security2.FileSystemSecurity2` object to the file system. It is the final step of the security descriptor workflow: `Get-NTFSSecurityDescriptor` reads a descriptor into memory, cmdlets such as `Add-NTFSAccess`, `Remove-NTFSAccess`, `Set-NTFSOwner`, and `Disable-NTFSAccessInheritance` change that copy through their `-SecurityDescriptor` parameter, and this cmdlet applies all of those changes in a single write.</maml:para>
<maml:para>Each descriptor remembers the item it was read from, and the cmdlet writes it back to exactly that item. There is no parameter that redirects the write to a different path. The cmdlet writes only the sections of the descriptor that changed since it was read, such as the DACL after `Add-NTFSAccess`, and leaves the other sections of the item as they are, so a descriptor that you did not change writes nothing. Before 5.0.0, the cmdlet wrote every section that it had read, also an unchanged owner, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.</maml:para>
<maml:para>Each descriptor remembers the item it was read from, and the cmdlet writes it back to exactly that item. There is no parameter that redirects the write to a different path. The cmdlet writes only the sections of the descriptor that changed since it was read or last written, such as the DACL after `Add-NTFSAccess`, and leaves the other sections of the item as they are, so a descriptor that you did not change writes nothing. With `-Verbose`, the cmdlet names the sections that it writes, or says that it writes nothing. Before 5.0.0, the cmdlet wrote every section that it had read, also an unchanged owner, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.</maml:para>
<maml:para>The cmdlet produces no output unless you use `-PassThru`, which reads the item again after the write and returns a new `FileSystemSecurity2` object that reflects what is now stored on disk. Descriptors can be passed as an array or through the pipeline, and each one is processed on its own.</maml:para>
<maml:para>When the write fails because access is denied, the cmdlet takes ownership of the item with the account of the current session, writes the descriptor, and restores the previous owner. If that fails as well, it writes a non-terminating error and continues with the next descriptor. Windows checks each section separately: changing the access control list requires the Change Permissions right on the item, changing the owner requires the Take Ownership right or the Take Ownership privilege, assigning ownership to another account requires the Restore privilege, and writing audit entries requires the Security privilege.</maml:para>
</maml:description>

23
Tests/SecurityDescriptor.Tests.ps1

@ -157,4 +157,27 @@ Describe 'Set-NTFSSecurityDescriptor' {
(Get-Acl -LiteralPath $file).GetOwner($sidType).Value | Should -Be $trustedInstaller
}
}
Context 'With -Verbose' {
It 'Should name the sections that it writes' {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'VerboseChanged'
Assert-TestSandboxPath -Sandbox $sandbox -Path $file
$sd = Get-NTFSSecurityDescriptor -Path $file
Add-NTFSAccess -SecurityDescriptor $sd -Account 'Everyone' -AccessRights ReadData
$messages = Set-NTFSSecurityDescriptor -SecurityDescriptor $sd -Verbose 4>&1
$messages.Message | Should -Contain "Writing the changed sections of the security descriptor of '$($sd.FullName)': Access"
}
It 'Should say that it writes nothing for an unchanged descriptor' {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'VerboseUnchanged'
$sd = Get-NTFSSecurityDescriptor -Path $file
$messages = Set-NTFSSecurityDescriptor -SecurityDescriptor $sd -Verbose 4>&1
$messages.Message |
Should -Contain "No section of the security descriptor of '$($sd.FullName)' changed since it was read or last written; nothing is written"
}
}
}

9
Tests/TestHelpers.Tests.ps1

@ -159,6 +159,15 @@ Describe 'Test helpers' {
{ Set-TestOwner -Sandbox $sandbox -Path "$sandbox-Other\File.txt" -Sid $trustedInstaller } |
Should -Throw -ExpectedMessage 'Refusing to change*'
}
# icacls reports a failure on stderr, which Windows PowerShell turns into a terminating error of its own when
# the caller uses -ErrorAction Stop.
It 'Should throw its own error when icacls fails, also with -ErrorAction Stop' {
$missing = Join-Path -Path $sandbox -ChildPath 'Missing.txt'
{ Set-TestOwner -Sandbox $sandbox -Path $missing -Sid $trustedInstaller -ErrorAction Stop } |
Should -Throw -ExpectedMessage 'icacls could not make*'
}
}
Context 'Test-IsElevated and Test-PrivilegeHeld' {

3
Tests/TestHelpers.psm1

@ -293,6 +293,9 @@ function Set-TestOwner {
)
Assert-TestSandboxPath -Sandbox $Sandbox -Path $Path
# icacls reports a failure on stderr, which Windows PowerShell turns into a terminating error when the caller uses
# ErrorAction Stop; the exit code decides instead.
$ErrorActionPreference = 'Continue'
# icacls resolves a relative path against the working folder of the process, not the location of PowerShell.
$location = (Get-Location -PSProvider FileSystem).ProviderPath
$fullName = [IO.Path]::GetFullPath([IO.Path]::Combine($location, $Path))

Loading…
Cancel
Save