diff --git a/.memory-bank/activeContext.md b/.memory-bank/activeContext.md index dfb9647..915ec7e 100644 --- a/.memory-bank/activeContext.md +++ b/.memory-bank/activeContext.md @@ -48,8 +48,10 @@ the PRs, and tags `5.0.0-rc2` after the merges. - `ai/defects-b` fixes defects 14 to 17 (`-AppliesTo` is mandatory in the `Simple` sets; `-RemoveSpecific` is back): Windows PowerShell 331 passed, 19 skipped; PowerShell 7 301 passed, 49 skipped (350 tests). +- `ai/defects-c` fixes defects 18 to 21, the same missing `continue` in + `Remove-NTFSAudit`, and a stale hash in `Get-FileHash2`. ## Next step -Group C (18 to 21) on `ai/defects-c`, then D, the E decisions, and the -bugs from the issue triage (`ai/issue-fixes`). +Group D (22 to 24) on `ai/defects-d`, then the E decisions and the bugs +from the issue triage (`ai/issue-fixes`). diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index 9621387..bba0bb4 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -141,7 +141,7 @@ Numbered as agreed with the maintainer; each is documented on its page. - Review of group B: no Blocker or Major; three Minor findings fixed, the rest are listed in the PR description. -#### C: Error handling +#### C: Error handling (fixed on `ai/defects-c`, not merged) - (18) `Copy-Item2`, `Move-Item2`, `Remove-Item2`: one failing path skips the rest of `-Path` (`return` instead of `continue`). @@ -150,6 +150,9 @@ Numbered as agreed with the maintainer; each is documented on its page. - (20) The inheritance cmdlets write `-PassThru` output in `finally`, even after a failure. - (21) `New-NTFSHardLink` says the target path exists when it doesn't. +- Review of group C: the add and remove cmdlets also wrote `-PassThru` + after a failed change, and a failed retry after taking ownership left the + owner changed; `BaseCmdlet.InvokeAsOwner` now restores it on every path. #### D: Metadata and cosmetics diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ee92b2..2c73265 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -112,5 +112,27 @@ The format is based on - Restore the `-RemoveSpecific` switch of `Remove-NTFSAccess`, which version 4.1 introduced but later versions lacked, and add it to `Remove-NTFSAudit`: with it, the cmdlets remove only an entry that matches exactly +- Fix `Copy-Item2`, `Move-Item2`, and `Remove-Item2`, which skipped the + remaining paths of `-Path` after a path that didn't exist or, for copy and + move, a file that already existed at the destination +- Fix `Remove-NTFSAccess` and `Remove-NTFSAudit`, which went on with a path + that didn't exist, wrote a second, misleading `RemoveAceError`, and with + `-PassThru` stopped with a `NullReferenceException` +- Fix `-PassThru` of `Enable-NTFSAccessInheritance`, + `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, + `Disable-NTFSAuditInheritance`, and `Set-NTFSInheritance`, which returned + the unchanged state of an item also when the change failed, so that the + inheritance looked disabled + ([#74](https://github.com/raandree/NTFSSecurity/issues/74)) +- Fix the error of `New-NTFSHardLink` for a missing `-Target`, which said + that the target path existed +- Fix `Get-FileHash2`, which wrote a result for a file that it couldn't + read, with the hash of the previous file +- Fix `-PassThru` of `Add-NTFSAccess`, `Add-NTFSAudit`, `Remove-NTFSAccess`, + and `Remove-NTFSAudit`, which returned the unchanged entries of an item + also when the change failed +- Fix the cmdlets that take ownership of an item to repeat an operation that + was denied: when the second attempt failed as well, the account that ran + the cmdlet stayed the owner of the item; now the previous owner is restored [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Add-NTFSAccess.md b/Docs/Cmdlets/Add-NTFSAccess.md index 078f24e..b39a443 100644 --- a/Docs/Cmdlets/Add-NTFSAccess.md +++ b/Docs/Cmdlets/Add-NTFSAccess.md @@ -282,7 +282,7 @@ The scope of the ACE, piped by the property `AppliesTo` in the `Simple` paramete ### Security2.FileSystemAccessRule2 -With `-PassThru`, the cmdlet writes all access control entries of every processed item, explicit and inherited. Without `-PassThru` it writes nothing. +With `-PassThru`, the cmdlet writes all access control entries, explicit and inherited, of every item it changed; an item it could not change produces only an error. Before 5.0.0, `-PassThru` also wrote the unchanged entries of such an item. Without `-PassThru` it writes nothing. ## NOTES diff --git a/Docs/Cmdlets/Add-NTFSAudit.md b/Docs/Cmdlets/Add-NTFSAudit.md index 838800e..ad280e6 100644 --- a/Docs/Cmdlets/Add-NTFSAudit.md +++ b/Docs/Cmdlets/Add-NTFSAudit.md @@ -280,7 +280,7 @@ The value passed to `-AppliesTo` is converted to this type and binds by property ### Security2.FileSystemAuditRule2 -Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `SecurityDescriptor` sets wrote the access entries of the descriptor instead. +Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `SecurityDescriptor` sets wrote the access entries of the descriptor instead. An item whose audit entries could not be changed produces only an error; before 5.0.0, `-PassThru` also wrote its unchanged entries. ## NOTES @@ -288,7 +288,7 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva Writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `AddAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. -If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. +If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. `-Path` or `-SecurityDescriptor`, `-Account`, and `-AccessRights` are positional parameters at positions 1, 2, and 3, like in `Remove-NTFSAudit`. Before 5.0.0, `-Account` and `-AccessRights` were both declared at position 2, so a command that passed them by position failed. diff --git a/Docs/Cmdlets/Clear-NTFSAudit.md b/Docs/Cmdlets/Clear-NTFSAudit.md index 7acbcf7..7ec4adf 100644 --- a/Docs/Cmdlets/Clear-NTFSAudit.md +++ b/Docs/Cmdlets/Clear-NTFSAudit.md @@ -142,7 +142,7 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva Reading and writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without that privilege the cmdlet reads the security descriptor without its SACL, finds no audit entries to remove, and finishes without an error although nothing was changed. `-DisableInheritance` fails in that situation with a `ClearAclError` whose message states that a required privilege is not held by the client. -If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. +If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. ## RELATED LINKS diff --git a/Docs/Cmdlets/Copy-Item2.md b/Docs/Cmdlets/Copy-Item2.md index 025dd6d..9bd30c6 100644 --- a/Docs/Cmdlets/Copy-Item2.md +++ b/Docs/Cmdlets/Copy-Item2.md @@ -186,7 +186,7 @@ By default this cmdlet returns nothing. With `-PassThru $true` it returns an `Al Before 5.0.0, copying a folder that contained files failed with a `CopyError` that reported a `DirectoryNotFoundException` for the first file in the folder. -If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. +If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and continues with the next path. Before 5.0.0, it skipped the remaining paths that were passed in the same call. ## RELATED LINKS diff --git a/Docs/Cmdlets/Disable-NTFSAccessInheritance.md b/Docs/Cmdlets/Disable-NTFSAccessInheritance.md index 4166f56..fc19225 100644 --- a/Docs/Cmdlets/Disable-NTFSAccessInheritance.md +++ b/Docs/Cmdlets/Disable-NTFSAccessInheritance.md @@ -170,6 +170,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. +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. + ## RELATED LINKS [Enable-NTFSAccessInheritance](Enable-NTFSAccessInheritance.md) diff --git a/Docs/Cmdlets/Disable-NTFSAuditInheritance.md b/Docs/Cmdlets/Disable-NTFSAuditInheritance.md index 3e08779..18ad002 100644 --- a/Docs/Cmdlets/Disable-NTFSAuditInheritance.md +++ b/Docs/Cmdlets/Disable-NTFSAuditInheritance.md @@ -172,6 +172,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. +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. + ## RELATED LINKS [Enable-NTFSAuditInheritance](Enable-NTFSAuditInheritance.md) diff --git a/Docs/Cmdlets/Enable-NTFSAccessInheritance.md b/Docs/Cmdlets/Enable-NTFSAccessInheritance.md index dc029fc..0a8b6da 100644 --- a/Docs/Cmdlets/Enable-NTFSAccessInheritance.md +++ b/Docs/Cmdlets/Enable-NTFSAccessInheritance.md @@ -169,6 +169,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. +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. + ## RELATED LINKS [Disable-NTFSAccessInheritance](Disable-NTFSAccessInheritance.md) diff --git a/Docs/Cmdlets/Enable-NTFSAuditInheritance.md b/Docs/Cmdlets/Enable-NTFSAuditInheritance.md index f7c7704..4765a1b 100644 --- a/Docs/Cmdlets/Enable-NTFSAuditInheritance.md +++ b/Docs/Cmdlets/Enable-NTFSAuditInheritance.md @@ -171,6 +171,8 @@ A path that does not exist produces a non-terminating error and the cmdlet conti Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. +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. + ## RELATED LINKS [Disable-NTFSAuditInheritance](Disable-NTFSAuditInheritance.md) diff --git a/Docs/Cmdlets/Get-FileHash2.md b/Docs/Cmdlets/Get-FileHash2.md index 3867548..6dd281e 100644 --- a/Docs/Cmdlets/Get-FileHash2.md +++ b/Docs/Cmdlets/Get-FileHash2.md @@ -119,13 +119,13 @@ The cmdlet does not return access rules. For every hashed file it writes the fil The cmdlet works only in Windows PowerShell. In PowerShell 7, it fails for every algorithm with the error `Could not load type 'System.Security.Cryptography.RIPEMD160'`, because .NET no longer includes the RIPEMD-160 implementation that the cmdlet references. In PowerShell 7, use the built-in `Get-FileHash` cmdlet instead. -If the file cannot be opened because access is denied, the cmdlet takes ownership of the file with the account that runs it, calculates the hash, and restores the previous owner afterward. That fallback fails with a `GetHashError` when the account is not allowed to change the owner of the file. +If the file cannot be opened because access is denied, the cmdlet takes ownership of the file with the account that runs it, calculates the hash, and restores the previous owner afterward. That fallback fails with a `GetHashError` when the account is not allowed to change the owner of the file. A file that cannot be read produces a `GetHashError` and no result. The hash is returned as an uppercase hexadecimal string without separators, which differs from the lowercase output of some other hashing tools. Compare hash values case-insensitively. `MACTripleDES` is a keyed message authentication code that is created with a key that is generated for each call, so its result is not reproducible across invocations and is not suitable for comparing files. -Before 5.0.0, a folder in a `-Path` array stopped the processing of that array, so the files that followed the folder were not hashed. +Before 5.0.0, a folder in a `-Path` array stopped the processing of that array, so the files that followed the folder were not hashed, and a file that could not be read got a result with the hash of the previous file. ## RELATED LINKS diff --git a/Docs/Cmdlets/Move-Item2.md b/Docs/Cmdlets/Move-Item2.md index addffd2..b4f3eff 100644 --- a/Docs/Cmdlets/Move-Item2.md +++ b/Docs/Cmdlets/Move-Item2.md @@ -186,7 +186,7 @@ By default this cmdlet returns nothing. With `-PassThru $true` it returns an `Al The cmdlet chooses between two mutually exclusive move options. Without `-Force` it moves with `CopyAllowed`, which permits a file to cross volume boundaries because Windows then copies and deletes it. With `-Force` it moves with `ReplaceExisting`, which overwrites the destination but does not request `CopyAllowed`, so a move across volumes can fail when `-Force` is specified. -If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. +If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and continues with the next path. Before 5.0.0, it skipped the remaining paths that were passed in the same call. ## RELATED LINKS diff --git a/Docs/Cmdlets/New-NTFSHardLink.md b/Docs/Cmdlets/New-NTFSHardLink.md index 16c4fdd..184f5a7 100644 --- a/Docs/Cmdlets/New-NTFSHardLink.md +++ b/Docs/Cmdlets/New-NTFSHardLink.md @@ -138,6 +138,8 @@ The cmdlet does not overwrite anything. If `-Path` already exists, or if `-Targe Because all names of a file share the same data, the number of hard links is a property of the file, not of an individual name. Use `Get-NTFSHardLink` to list them, and delete a link with `Remove-Item2` or `Remove-Item`, which removes only that name as long as other names remain. +Before 5.0.0, the error for a missing `-Target` said "The target path exist", the opposite of the cause. + ## RELATED LINKS [Get-NTFSHardLink](Get-NTFSHardLink.md) diff --git a/Docs/Cmdlets/Remove-Item2.md b/Docs/Cmdlets/Remove-Item2.md index 40d1bad..0669f84 100644 --- a/Docs/Cmdlets/Remove-Item2.md +++ b/Docs/Cmdlets/Remove-Item2.md @@ -179,7 +179,7 @@ By default this cmdlet returns nothing. With `-PassThru` it returns an `Alphaleo The module defines the aliases `rm2` and `del2` for this cmdlet. -A path that does not exist causes the error `FileNotFound`, and a deletion that the file system rejects causes a `DeleteError`. In both cases the cmdlet skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. +A path that does not exist causes the error `FileNotFound`, and a deletion that the file system rejects causes a `DeleteError`. In both cases the cmdlet continues with the next path. Before 5.0.0, a path that did not exist made the cmdlet skip the remaining paths that were passed in the same call. ## RELATED LINKS diff --git a/Docs/Cmdlets/Remove-NTFSAccess.md b/Docs/Cmdlets/Remove-NTFSAccess.md index 9eda4d0..e2b4c18 100644 --- a/Docs/Cmdlets/Remove-NTFSAccess.md +++ b/Docs/Cmdlets/Remove-NTFSAccess.md @@ -294,7 +294,7 @@ The scope of the entry, piped by the property `AppliesTo` in the `Simple` parame ### Security2.FileSystemAccessRule2 -With `-PassThru`, the cmdlet writes all access control entries of every processed item, explicit and inherited. Without `-PassThru` it writes nothing. +With `-PassThru`, the cmdlet writes all access control entries, explicit and inherited, of every item it changed; an item it could not change produces only an error. Before 5.0.0, `-PassThru` also wrote the unchanged entries of such an item. Without `-PassThru` it writes nothing. ## NOTES @@ -306,6 +306,8 @@ Removing rights from an entry that does not exist is not an error; the cmdlet le Before 5.0.0, the `-RemoveSpecific` switch was missing, although version 4.1 had introduced it. +A path that does not exist produces the non-terminating error `ReadFileError`, and the cmdlet continues with the next path. Before 5.0.0, the cmdlet also wrote a misleading `RemoveAceError` for that path, and with `-PassThru` it stopped with a `NullReferenceException`. + ## RELATED LINKS [Get-NTFSAccess](Get-NTFSAccess.md) diff --git a/Docs/Cmdlets/Remove-NTFSAudit.md b/Docs/Cmdlets/Remove-NTFSAudit.md index 846edeb..c8c7835 100644 --- a/Docs/Cmdlets/Remove-NTFSAudit.md +++ b/Docs/Cmdlets/Remove-NTFSAudit.md @@ -294,7 +294,7 @@ The value passed to `-AppliesTo` is converted to this type and binds by property ### Security2.FileSystemAuditRule2 -Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `Path` sets wrote the access entries of the item instead. +Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `Path` sets wrote the access entries of the item instead. An item whose audit entries could not be changed produces only an error; before 5.0.0, `-PassThru` also wrote its unchanged entries. ## NOTES @@ -302,12 +302,14 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva Reading and writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `RemoveAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. -If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. +If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. The cmdlet reports no error when no entry matches the supplied values. Compare the result with `Get-NTFSAudit` to confirm that the entry is gone. Before 5.0.0, the cmdlet had no `-RemoveSpecific` switch. +A path that does not exist produces the non-terminating error `ReadFileError`, and the cmdlet continues with the next path. Before 5.0.0, the cmdlet also wrote a misleading `RemoveAceError` for that path, and with `-PassThru` it stopped with a `NullReferenceException`. + ## RELATED LINKS [Get-NTFSAudit](Get-NTFSAudit.md) diff --git a/Docs/Cmdlets/Set-NTFSInheritance.md b/Docs/Cmdlets/Set-NTFSInheritance.md index f9986a8..34c141f 100644 --- a/Docs/Cmdlets/Set-NTFSInheritance.md +++ b/Docs/Cmdlets/Set-NTFSInheritance.md @@ -196,6 +196,8 @@ Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. +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. + ## RELATED LINKS [Get-NTFSInheritance](Get-NTFSInheritance.md) diff --git a/Docs/Concepts.md b/Docs/Concepts.md index aaa2a15..18f347c 100644 --- a/Docs/Concepts.md +++ b/Docs/Concepts.md @@ -199,7 +199,8 @@ state. When reading or changing an item fails with an access-denied error, most of these cmdlets make the current user the owner of the item, retry, and then -restore the previous owner. This requires the privileges above. +restore the previous owner, also when the retry fails. Before 5.0.0, a failed +retry left the current user as the owner. This requires the privileges above. Reading or changing audit entries always requires the Security privilege. Without it, the audit cmdlets fail, and `Get-NTFSEffectiveAccess` warns that diff --git a/NTFSSecurity/AccessCmdlets/AddAccess.cs b/NTFSSecurity/AccessCmdlets/AddAccess.cs index c22e0f5..4770aa9 100644 --- a/NTFSSecurity/AccessCmdlets/AddAccess.cs +++ b/NTFSSecurity/AccessCmdlets/AddAccess.cs @@ -138,23 +138,21 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemAccessRule2.AddFileSystemAccessRule(item, account.ToList(), accessRights, accessType, inheritanceFlags, propagationFlags); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemAccessRule2.AddFileSystemAccessRule(item, account.ToList(), accessRights, accessType, inheritanceFlags, propagationFlags); + }); } catch (Exception ex2) { WriteError(new ErrorRecord(ex2, "AddAceError", ErrorCategory.WriteError, path)); + continue; } } catch (Exception ex) { WriteError(new ErrorRecord(ex, "AddAceError", ErrorCategory.WriteError, path)); + continue; } if (passThru == true) diff --git a/NTFSSecurity/AccessCmdlets/ClearAccess.cs b/NTFSSecurity/AccessCmdlets/ClearAccess.cs index 20b1943..9e96a39 100644 --- a/NTFSSecurity/AccessCmdlets/ClearAccess.cs +++ b/NTFSSecurity/AccessCmdlets/ClearAccess.cs @@ -76,16 +76,12 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemAccessRule2.RemoveFileSystemAccessRuleAll(item); - if (disableInheritance) - FileSystemInheritanceInfo.DisableAccessInheritance(item, true); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemAccessRule2.RemoveFileSystemAccessRuleAll(item); + if (disableInheritance) + FileSystemInheritanceInfo.DisableAccessInheritance(item, true); + }); } catch (Exception ex2) { diff --git a/NTFSSecurity/AccessCmdlets/GetAccess.cs b/NTFSSecurity/AccessCmdlets/GetAccess.cs index 6b9339a..f010e18 100644 --- a/NTFSSecurity/AccessCmdlets/GetAccess.cs +++ b/NTFSSecurity/AccessCmdlets/GetAccess.cs @@ -104,12 +104,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - acl = FileSystemAccessRule2.GetFileSystemAccessRules(item, !excludeExplicit, !excludeInherited, getInheritedFrom); - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + acl = FileSystemAccessRule2.GetFileSystemAccessRules(item, !excludeExplicit, !excludeInherited, getInheritedFrom); + }); } catch (Exception ex2) { diff --git a/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs b/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs index 4fb3c23..d929bbf 100644 --- a/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs +++ b/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs @@ -133,14 +133,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - result = EffectiveAccess.GetEffectiveAccess(item, account, serverName); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + result = EffectiveAccess.GetEffectiveAccess(item, account, serverName); + }); } catch (Exception ex2) { diff --git a/NTFSSecurity/AccessCmdlets/GetOrphanedAccess.cs b/NTFSSecurity/AccessCmdlets/GetOrphanedAccess.cs index fbe88ce..94ad30c 100644 --- a/NTFSSecurity/AccessCmdlets/GetOrphanedAccess.cs +++ b/NTFSSecurity/AccessCmdlets/GetOrphanedAccess.cs @@ -48,14 +48,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - acl = FileSystemAccessRule2.GetFileSystemAccessRules(item, !ExcludeExplicit, !ExcludeInherited, getInheritedFrom); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + acl = FileSystemAccessRule2.GetFileSystemAccessRules(item, !ExcludeExplicit, !ExcludeInherited, getInheritedFrom); + }); } catch (Exception ex2) { diff --git a/NTFSSecurity/AccessCmdlets/RemoveAccess.cs b/NTFSSecurity/AccessCmdlets/RemoveAccess.cs index 4ed939a..bd4c04d 100644 --- a/NTFSSecurity/AccessCmdlets/RemoveAccess.cs +++ b/NTFSSecurity/AccessCmdlets/RemoveAccess.cs @@ -138,6 +138,7 @@ namespace NTFSSecurity catch (Exception ex) { WriteError(new ErrorRecord(ex, "ReadFileError", ErrorCategory.OpenError, path)); + continue; } if (ParameterSetName == "PathSimple") @@ -153,23 +154,21 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemAccessRule2.RemoveFileSystemAccessRule(item, account.ToList(), accessRights, accessType, inheritanceFlags, propagationFlags, removeSpecific); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemAccessRule2.RemoveFileSystemAccessRule(item, account.ToList(), accessRights, accessType, inheritanceFlags, propagationFlags, removeSpecific); + }); } catch (Exception ex2) { WriteError(new ErrorRecord(ex2, "RemoveAceError", ErrorCategory.WriteError, path)); + continue; } } catch (Exception ex) { WriteError(new ErrorRecord(ex, "RemoveAceError", ErrorCategory.WriteError, path)); + continue; } if (passThru == true) diff --git a/NTFSSecurity/AuditCmdlets/AddAudit.cs b/NTFSSecurity/AuditCmdlets/AddAudit.cs index 7470de6..4ce3edb 100644 --- a/NTFSSecurity/AuditCmdlets/AddAudit.cs +++ b/NTFSSecurity/AuditCmdlets/AddAudit.cs @@ -137,23 +137,21 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemAuditRule2.AddFileSystemAuditRule(item, account.ToList(), accessRights, auditFlags, inheritanceFlags, propagationFlags); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemAuditRule2.AddFileSystemAuditRule(item, account.ToList(), accessRights, auditFlags, inheritanceFlags, propagationFlags); + }); } catch (Exception ex2) { WriteError(new ErrorRecord(ex2, "AddAceError", ErrorCategory.WriteError, path)); + continue; } } catch (Exception ex) { WriteError(new ErrorRecord(ex, "AddAceError", ErrorCategory.WriteError, path)); + continue; } if (passThru == true) diff --git a/NTFSSecurity/AuditCmdlets/ClearAudit.cs b/NTFSSecurity/AuditCmdlets/ClearAudit.cs index c34bb29..622e5d4 100644 --- a/NTFSSecurity/AuditCmdlets/ClearAudit.cs +++ b/NTFSSecurity/AuditCmdlets/ClearAudit.cs @@ -76,16 +76,12 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemAuditRule2.RemoveFileSystemAuditRuleAll(item); - if (disableInheritance) - FileSystemInheritanceInfo.DisableAuditInheritance(item, true); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemAuditRule2.RemoveFileSystemAuditRuleAll(item); + if (disableInheritance) + FileSystemInheritanceInfo.DisableAuditInheritance(item, true); + }); } catch (Exception ex2) { diff --git a/NTFSSecurity/AuditCmdlets/RemoveAudit.cs b/NTFSSecurity/AuditCmdlets/RemoveAudit.cs index d4a738d..f075b30 100644 --- a/NTFSSecurity/AuditCmdlets/RemoveAudit.cs +++ b/NTFSSecurity/AuditCmdlets/RemoveAudit.cs @@ -137,6 +137,7 @@ namespace NTFSSecurity catch (Exception ex) { WriteError(new ErrorRecord(ex, "ReadFileError", ErrorCategory.OpenError, path)); + continue; } if (ParameterSetName == "PathSimple") @@ -152,23 +153,21 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemAuditRule2.RemoveFileSystemAuditRule(item, account.ToList(), accessRights, auditFlags, inheritanceFlags, propagationFlags, removeSpecific); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemAuditRule2.RemoveFileSystemAuditRule(item, account.ToList(), accessRights, auditFlags, inheritanceFlags, propagationFlags, removeSpecific); + }); } catch (Exception ex2) { WriteError(new ErrorRecord(ex2, "RemoveAceError", ErrorCategory.WriteError, path)); + continue; } } catch (Exception ex) { WriteError(new ErrorRecord(ex, "RemoveAceError", ErrorCategory.WriteError, path)); + continue; } if (passThru == true) diff --git a/NTFSSecurity/BaseCmdlets.cs b/NTFSSecurity/BaseCmdlets.cs index 0a5eb49..8cdb84d 100644 --- a/NTFSSecurity/BaseCmdlets.cs +++ b/NTFSSecurity/BaseCmdlets.cs @@ -148,6 +148,38 @@ namespace NTFSSecurity return path; } #endregion + + #region InvokeAsOwner + /// + /// Takes ownership of the item, runs the action, and restores the previous owner on every exit path. + /// A failure to restore the owner is written as a RestoreOwnerError and doesn't hide an error of the action. + /// + /// The file or folder to take ownership of. + /// The path the user specified, used as the error target. + /// The operation to run while the current user owns the item. + protected void InvokeAsOwner(Alphaleonis.Win32.Filesystem.FileSystemInfo item, string path, Action action) + { + var previousOwner = FileSystemOwner.GetOwner(item).Owner; + + FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); + + try + { + action(); + } + finally + { + try + { + FileSystemOwner.SetOwner(item, previousOwner); + } + catch (Exception ex) + { + WriteError(new ErrorRecord(ex, "RestoreOwnerError", ErrorCategory.WriteError, path)); + } + } + } + #endregion } public class BaseCmdletWithPrivControl : BaseCmdlet diff --git a/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs b/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs index d196fb7..f459e61 100644 --- a/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/DisableAccessInheritance.cs @@ -81,14 +81,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemInheritanceInfo.DisableAccessInheritance(item, removeInheritedAccessRules); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemInheritanceInfo.DisableAccessInheritance(item, removeInheritedAccessRules); + }); } catch (Exception ex2) { @@ -101,12 +97,11 @@ namespace NTFSSecurity WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path)); continue; } - finally + + // Only after a successful change, so that a failure doesn't report the unchanged state + if (passThru) { - if (passThru) - { - WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); - } + WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs b/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs index ac603cf..bf51018 100644 --- a/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/DisableAuditInheritance.cs @@ -82,14 +82,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemInheritanceInfo.DisableAuditInheritance(item, removeInheritedAccessRules); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemInheritanceInfo.DisableAuditInheritance(item, removeInheritedAccessRules); + }); } catch (Exception ex2) { @@ -102,12 +98,11 @@ namespace NTFSSecurity WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path)); continue; } - finally + + // Only after a successful change, so that a failure doesn't report the unchanged state + if (passThru) { - if (passThru) - { - WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); - } + WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs b/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs index 672b98f..1821e49 100644 --- a/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/EnableAccessInheritance.cs @@ -81,14 +81,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemInheritanceInfo.EnableAccessInheritance(item, removeExplicitAccessRules); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemInheritanceInfo.EnableAccessInheritance(item, removeExplicitAccessRules); + }); } catch (Exception ex2) { @@ -101,12 +97,11 @@ namespace NTFSSecurity WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path)); continue; } - finally + + // Only after a successful change, so that a failure doesn't report the unchanged state + if (passThru) { - if (passThru) - { - WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); - } + WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs b/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs index 3f29546..d3c753c 100644 --- a/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/EnableAuditInheritance.cs @@ -81,14 +81,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - FileSystemInheritanceInfo.EnableAuditInheritance(item, removeExplicitAccessRules); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + FileSystemInheritanceInfo.EnableAuditInheritance(item, removeExplicitAccessRules); + }); } catch (Exception ex2) { @@ -101,12 +97,11 @@ namespace NTFSSecurity WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path)); continue; } - finally + + // Only after a successful change, so that a failure doesn't report the unchanged state + if (passThru) { - if (passThru) - { - WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); - } + WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); } } } diff --git a/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs b/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs index df19f8d..07d510c 100644 --- a/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/GetInheritance.cs @@ -72,14 +72,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - inheritanceInfo = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + inheritanceInfo = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); + }); } catch (Exception ex2) { diff --git a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs index 3877e43..3dcc8b3 100644 --- a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs @@ -89,14 +89,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - SetInheritanceState(item); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + SetInheritanceState(item); + }); } catch (Exception ex2) { @@ -109,12 +105,11 @@ namespace NTFSSecurity WriteError(new ErrorRecord(ex, "ModifySdError", ErrorCategory.WriteError, path)); continue; } - finally + + // Only after a successful change, so that a failure doesn't report the unchanged state + if (passThru) { - if (passThru) - { - WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); - } + WriteObject(FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item)); } } } diff --git a/NTFSSecurity/ItemCmdlets/CopyItem2.cs b/NTFSSecurity/ItemCmdlets/CopyItem2.cs index 8e27a1b..3e391d9 100644 --- a/NTFSSecurity/ItemCmdlets/CopyItem2.cs +++ b/NTFSSecurity/ItemCmdlets/CopyItem2.cs @@ -71,7 +71,7 @@ namespace NTFSSecurity catch (System.IO.FileNotFoundException ex) { WriteError(new ErrorRecord(ex, "FileNotFound", ErrorCategory.ObjectNotFound, resolvedPath)); - return; + continue; } //destination is a directory @@ -88,7 +88,7 @@ namespace NTFSSecurity if (!force & File.Exists(actualDestination)) { WriteError(new ErrorRecord(new AlreadyExistsException(), "DestinationFileAlreadyExists", ErrorCategory.ResourceExists, actualDestination)); - return; + continue; } try diff --git a/NTFSSecurity/ItemCmdlets/MoveItem2.cs b/NTFSSecurity/ItemCmdlets/MoveItem2.cs index 4e93b51..2a360e1 100644 --- a/NTFSSecurity/ItemCmdlets/MoveItem2.cs +++ b/NTFSSecurity/ItemCmdlets/MoveItem2.cs @@ -71,7 +71,7 @@ namespace NTFSSecurity catch (System.IO.FileNotFoundException ex) { WriteError(new ErrorRecord(ex, "FileNotFound", ErrorCategory.ObjectNotFound, resolvedPath)); - return; + continue; } //destination is a directory @@ -88,7 +88,7 @@ namespace NTFSSecurity if (!force & File.Exists(actualDestination)) { WriteError(new ErrorRecord(new AlreadyExistsException(), "DestinationFileAlreadyExists", ErrorCategory.ResourceExists, actualDestination)); - return; + continue; } try diff --git a/NTFSSecurity/ItemCmdlets/RemoveItem2.cs b/NTFSSecurity/ItemCmdlets/RemoveItem2.cs index 0d3a135..de34ca0 100644 --- a/NTFSSecurity/ItemCmdlets/RemoveItem2.cs +++ b/NTFSSecurity/ItemCmdlets/RemoveItem2.cs @@ -66,7 +66,7 @@ namespace NTFSSecurity catch (System.IO.FileNotFoundException ex) { WriteError(new ErrorRecord(ex, "FileNotFound", ErrorCategory.ObjectNotFound, path)); - return; + continue; } try diff --git a/NTFSSecurity/LinkCmdlets/NewHardLink.cs b/NTFSSecurity/LinkCmdlets/NewHardLink.cs index 7f9f47d..cec9d5b 100644 --- a/NTFSSecurity/LinkCmdlets/NewHardLink.cs +++ b/NTFSSecurity/LinkCmdlets/NewHardLink.cs @@ -63,7 +63,7 @@ namespace NTFSSecurity throw new ArgumentException(string.Format("The file '{0}' does already exist, cannot create the link", path)); if (!TryGetFileSystemInfo2(target, out temp)) - throw new ArgumentException("The target path exist, cannot create the link"); + throw new ArgumentException(string.Format("The target '{0}' does not exist, cannot create the link", target)); else if (temp is DirectoryInfo) throw new ArgumentException("The target is not a file, cannot create the link"); diff --git a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs index b717d2b..2e46f78 100644 --- a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs +++ b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs @@ -39,11 +39,11 @@ namespace NTFSSecurity protected override void ProcessRecord() { - string hash = string.Empty; - FileSystemInfo item = null; - foreach (var path in paths) { + string hash = null; + FileSystemInfo item = null; + try { item = GetFileSystemInfo2(path) as FileInfo; @@ -68,23 +68,21 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - hash = ((FileInfo)item).GetHash(algorithm); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + hash = ((FileInfo)item).GetHash(algorithm); + }); } catch (Exception ex2) { WriteError(new ErrorRecord(ex2, "GetHashError", ErrorCategory.WriteError, path)); + continue; } } catch (Exception ex) { WriteError(new ErrorRecord(ex, "GetHashError", ErrorCategory.WriteError, path)); + continue; } var result = new PSObject(item); diff --git a/NTFSSecurity/SecurityDescriptorCmdlets/GetSecurityDescriptor.cs b/NTFSSecurity/SecurityDescriptorCmdlets/GetSecurityDescriptor.cs index 0b9dc93..4d5e3f4 100644 --- a/NTFSSecurity/SecurityDescriptorCmdlets/GetSecurityDescriptor.cs +++ b/NTFSSecurity/SecurityDescriptorCmdlets/GetSecurityDescriptor.cs @@ -56,14 +56,10 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - WriteObject(new FileSystemSecurity2(item)); - - FileSystemOwner.SetOwner(item, previousOwner); + InvokeAsOwner(item, path, () => + { + WriteObject(new FileSystemSecurity2(item)); + }); } catch (Exception ex2) { diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 826e823..9425ccf 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -712,7 +712,7 @@ Security2.FileSystemAccessRule2 - With `-PassThru`, the cmdlet writes all access control entries of every processed item, explicit and inherited. Without `-PassThru` it writes nothing. + With `-PassThru`, the cmdlet writes all access control entries, explicit and inherited, of every item it changed; an item it could not change produces only an error. Before 5.0.0, `-PassThru` also wrote the unchanged entries of such an item. Without `-PassThru` it writes nothing. @@ -1493,7 +1493,7 @@ Security2.FileSystemAuditRule2 - Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `SecurityDescriptor` sets wrote the access entries of the descriptor instead. + Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `SecurityDescriptor` sets wrote the access entries of the descriptor instead. An item whose audit entries could not be changed produces only an error; before 5.0.0, `-PassThru` also wrote its unchanged entries. @@ -1501,7 +1501,7 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. Writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `AddAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. - If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. + If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. `-Path` or `-SecurityDescriptor`, `-Account`, and `-AccessRights` are positional parameters at positions 1, 2, and 3, like in `Remove-NTFSAudit`. Before 5.0.0, `-Account` and `-AccessRights` were both declared at position 2, so a command that passed them by position failed. An audit entry alone does not create events. Windows writes the events to the security log only while the "Audit object access" policy, or the corresponding "Audit File System" advanced audit policy, is enabled for success, failure, or both. That policy is a Windows setting and is not managed by this module. @@ -1913,7 +1913,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. Reading and writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without that privilege the cmdlet reads the security descriptor without its SACL, finds no audit entries to remove, and finishes without an error although nothing was changed. `-DisableInheritance` fails in that situation with a `ClearAclError` whose message states that a required privilege is not held by the client. - If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. + If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. @@ -2174,7 +2174,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd `Copy-Item2` copies through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), which is why it handles source and destination paths that exceed the 260-character `MAX_PATH` limit of the built-in `Copy-Item` cmdlet. Before 5.0.0, copying a folder that contained files failed with a `CopyError` that reported a `DirectoryNotFoundException` for the first file in the folder. - If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. + If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and continues with the next path. Before 5.0.0, it skipped the remaining paths that were passed in the same call. @@ -2413,6 +2413,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd Blocking access inheritance requires permission to change the DACL of the item, which the owner of an item always has. If the descriptor cannot be opened, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; otherwise the cmdlet writes an error and continues with the next item. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. + 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. @@ -2658,6 +2659,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd If the descriptor cannot be opened because the account has no permission to the item, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; a missing Security privilege is not an access problem and is not repaired by it. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. + 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. @@ -3028,6 +3030,7 @@ PS C:\> Get-Privileges | Where-Object { $_.Privilege -in 'Backup', 'Restore', Restoring access inheritance requires permission to change the DACL of the item, which the owner of an item always has. If the descriptor cannot be opened, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; otherwise the cmdlet writes an error and continues with the next item. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. + 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. @@ -3273,6 +3276,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd If the descriptor cannot be opened because the account has no permission to the item, the cmdlet takes ownership of the item, applies the change, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; a missing Security privilege is not an access problem and is not repaired by it. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. + 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. @@ -4146,10 +4150,10 @@ PS C:\> Disable-Privileges The cmdlet works only in Windows PowerShell. In PowerShell 7, it fails for every algorithm with the error `Could not load type 'System.Security.Cryptography.RIPEMD160'`, because .NET no longer includes the RIPEMD-160 implementation that the cmdlet references. In PowerShell 7, use the built-in `Get-FileHash` cmdlet instead. - If the file cannot be opened because access is denied, the cmdlet takes ownership of the file with the account that runs it, calculates the hash, and restores the previous owner afterward. That fallback fails with a `GetHashError` when the account is not allowed to change the owner of the file. + If the file cannot be opened because access is denied, the cmdlet takes ownership of the file with the account that runs it, calculates the hash, and restores the previous owner afterward. That fallback fails with a `GetHashError` when the account is not allowed to change the owner of the file. A file that cannot be read produces a `GetHashError` and no result. The hash is returned as an uppercase hexadecimal string without separators, which differs from the lowercase output of some other hashing tools. Compare hash values case-insensitively. `MACTripleDES` is a keyed message authentication code that is created with a key that is generated for each call, so its result is not reproducible across invocations and is not suitable for comparing files. - Before 5.0.0, a folder in a `-Path` array stopped the processing of that array, so the files that followed the folder were not hashed. + Before 5.0.0, a folder in a `-Path` array stopped the processing of that array, so the files that followed the folder were not hashed, and a file that could not be read got a result with the hash of the previous file. @@ -6945,7 +6949,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor `Move-Item2` moves through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), which is why it handles source and destination paths that exceed the 260-character `MAX_PATH` limit of the built-in `Move-Item` cmdlet. The cmdlet chooses between two mutually exclusive move options. Without `-Force` it moves with `CopyAllowed`, which permits a file to cross volume boundaries because Windows then copies and deletes it. With `-Force` it moves with `ReplaceExisting`, which overwrites the destination but does not request `CopyAllowed`, so a move across volumes can fail when `-Force` is specified. - If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. + If a path in `-Path` does not exist or the destination file exists and `-Force` is missing, the cmdlet writes a non-terminating error and continues with the next path. Before 5.0.0, it skipped the remaining paths that were passed in the same call. @@ -7131,6 +7135,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor Windows supports hard links only for files on the same NTFS volume. A link that points to a file on another volume, or a target on a file system that does not implement hard links, cannot be created. The cmdlet does not overwrite anything. If `-Path` already exists, or if `-Target` is missing or is a folder, the cmdlet reports an error and leaves the file system unchanged. Because all names of a file share the same data, the number of hard links is a property of the file, not of an individual name. Use `Get-NTFSHardLink` to list them, and delete a link with `Remove-Item2` or `Remove-Item`, which removes only that name as long as other names remain. + Before 5.0.0, the error for a missing `-Target` said "The target path exist", the opposite of the cause. @@ -7551,7 +7556,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor `Remove-Item2` deletes through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), which is why it reaches items whose path exceeds the 260-character `MAX_PATH` limit of the built-in `Remove-Item` cmdlet. Deletion is permanent; the cmdlet does not use the Recycle Bin. The module defines the aliases `rm2` and `del2` for this cmdlet. - A path that does not exist causes the error `FileNotFound`, and a deletion that the file system rejects causes a `DeleteError`. In both cases the cmdlet skips the remaining paths that were passed in the same call. Items that arrive one by one through the pipeline are not affected, because each of them is processed separately. + A path that does not exist causes the error `FileNotFound`, and a deletion that the file system rejects causes a `DeleteError`. In both cases the cmdlet continues with the next path. Before 5.0.0, a path that did not exist made the cmdlet skip the remaining paths that were passed in the same call. @@ -8375,7 +8380,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor Security2.FileSystemAccessRule2 - With `-PassThru`, the cmdlet writes all access control entries of every processed item, explicit and inherited. Without `-PassThru` it writes nothing. + With `-PassThru`, the cmdlet writes all access control entries, explicit and inherited, of every item it changed; an item it could not change produces only an error. Before 5.0.0, `-PassThru` also wrote the unchanged entries of such an item. Without `-PassThru` it writes nothing. @@ -8385,6 +8390,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor If the ACL of an item cannot be written because access is denied, the cmdlet tries once more after making the current account the owner of the item, and restores the previous owner afterwards. Changing the owner of an item requires the Take Ownership and Restore privileges, so this fallback only succeeds in an elevated session of an account that holds them. Removing rights from an entry that does not exist is not an error; the cmdlet leaves the ACL unchanged. Before 5.0.0, the `-RemoveSpecific` switch was missing, although version 4.1 had introduced it. + A path that does not exist produces the non-terminating error `ReadFileError`, and the cmdlet continues with the next path. Before 5.0.0, the cmdlet also wrote a misleading `RemoveAceError` for that path, and with `-PassThru` it stopped with a `NullReferenceException`. @@ -9213,7 +9219,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor Security2.FileSystemAuditRule2 - Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `Path` sets wrote the access entries of the item instead. + Without `-PassThru` the cmdlet writes nothing. With `-PassThru` the cmdlet writes all audit entries of the item or the security descriptor, explicit and inherited ones, as `Security2.FileSystemAuditRule2` objects. Before 5.0.0, the `Path` sets wrote the access entries of the item instead. An item whose audit entries could not be changed produces only an error; before 5.0.0, `-PassThru` also wrote its unchanged entries. @@ -9221,9 +9227,10 @@ PS C:\Data> Get-NTFSSecurityDescriptor When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. Reading and writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `RemoveAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. - If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back. + If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. The cmdlet reports no error when no entry matches the supplied values. Compare the result with `Get-NTFSAudit` to confirm that the entry is gone. Before 5.0.0, the cmdlet had no `-RemoveSpecific` switch. + A path that does not exist produces the non-terminating error `ReadFileError`, and the cmdlet continues with the next path. Before 5.0.0, the cmdlet also wrote a misleading `RemoveAceError` for that path, and with `-PassThru` it stopped with a `NullReferenceException`. @@ -9516,6 +9523,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` could fail with the error "Nullable object must have a value". Before 5.0.0, the cmdlet enabled the privileges even when `EnablePrivileges` was `$false`, and left them enabled. + 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. diff --git a/Tests/Access.Tests.ps1 b/Tests/Access.Tests.ps1 index 6ebe074..bede5be 100644 --- a/Tests/Access.Tests.ps1 +++ b/Tests/Access.Tests.ps1 @@ -6,6 +6,12 @@ )] param () +BeforeDiscovery { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + # With the Restore privilege, Windows may grant writing the DACL despite a deny entry. + $canBypassWriteDeny = Test-PrivilegeHeld -Name 'SeRestorePrivilege' +} + BeforeAll { Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' @@ -149,6 +155,41 @@ Describe 'Get-NTFSSimpleAccess' { } Describe 'Remove-NTFSAccess' { + Context 'When a path does not exist' { + BeforeAll { + $missing = Join-Path -Path $sandbox -ChildPath 'Missing.txt' + } + + # Before 5.0.0, the cmdlet went on with the missing item and wrote a second, misleading RemoveAceError. + It 'Should write only the read error' { + Remove-NTFSAccess -Path $missing -Account 'Everyone' -AccessRights ReadData -ErrorVariable removeErrors -ErrorAction SilentlyContinue + + $removeErrors | Should -HaveCount 1 + $removeErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadFileError,*' + } + + It 'Should not stop with -PassThru' { + { Remove-NTFSAccess -Path $missing -Account 'Everyone' -AccessRights ReadData -PassThru -ErrorAction SilentlyContinue } | + Should -Not -Throw + } + } + + Context 'When the entries cannot be changed' { + # Before 5.0.0, -PassThru returned the unchanged entries of the item after the error. + It 'Should write an error and return nothing with -PassThru' -Skip:$canBypassWriteDeny { + $protected = New-TestSandboxItem -Sandbox $sandbox -Name 'RemoveProtected' + Assert-TestSandboxPath -Sandbox $sandbox -Path $protected + Add-NTFSAccess -Path $protected -Account 'Everyone' -AccessRights ReadData + Block-TestWritePermission -Sandbox $sandbox -Path $protected + + $result = @(Remove-NTFSAccess -Path $protected -Account 'Everyone' -AccessRights ReadData -PassThru -ErrorVariable removeErrors -ErrorAction SilentlyContinue) + + $removeErrors | Should -HaveCount 1 + $removeErrors[0].FullyQualifiedErrorId | Should -BeLike 'RemoveAceError,*' + $result | Should -BeNullOrEmpty + } + } + Context 'With -RemoveSpecific' { BeforeEach { $removeFolder = New-TestSandboxItem -Sandbox $sandbox -Name 'RemoveSpecific' -Directory @@ -183,6 +224,23 @@ Describe 'Remove-NTFSAccess' { } } +Describe 'Add-NTFSAccess' { + Context 'When the entries cannot be changed' { + # Before 5.0.0, -PassThru returned the unchanged entries of the item after the error. + It 'Should write an error and return nothing with -PassThru' -Skip:$canBypassWriteDeny { + $protected = New-TestSandboxItem -Sandbox $sandbox -Name 'AddProtected' + Assert-TestSandboxPath -Sandbox $sandbox -Path $protected + Block-TestWritePermission -Sandbox $sandbox -Path $protected + + $result = @(Add-NTFSAccess -Path $protected -Account 'Everyone' -AccessRights ReadData -PassThru -ErrorVariable addErrors -ErrorAction SilentlyContinue) + + $addErrors | Should -HaveCount 1 + $addErrors[0].FullyQualifiedErrorId | Should -BeLike 'AddAceError,*' + $result | Should -BeNullOrEmpty + } + } +} + Describe 'Security descriptor parameter sets' { BeforeAll { $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ParameterSets' -Directory diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 index 8797303..7345e13 100644 --- a/Tests/Audit.Tests.ps1 +++ b/Tests/Audit.Tests.ps1 @@ -148,6 +148,25 @@ Describe 'Get-NTFSOrphanedAudit' { } Describe 'Remove-NTFSAudit' { + Context 'When a path does not exist' { + BeforeAll { + $missing = Join-Path -Path $sandbox -ChildPath 'Missing.txt' + } + + # Before 5.0.0, the cmdlet went on with the missing item and wrote a second, misleading RemoveAceError. + It 'Should write only the read error' { + Remove-NTFSAudit -Path $missing -Account 'Everyone' -AccessRights ReadData -ErrorVariable removeErrors -ErrorAction SilentlyContinue + + $removeErrors | Should -HaveCount 1 + $removeErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadFileError,*' + } + + It 'Should not stop with -PassThru' { + { Remove-NTFSAudit -Path $missing -Account 'Everyone' -AccessRights ReadData -PassThru -ErrorAction SilentlyContinue } | + Should -Not -Throw + } + } + Context 'With -RemoveSpecific' { BeforeEach { $removeFolder = New-TestSandboxItem -Sandbox $sandbox -Name 'RemoveSpecific' -Directory diff --git a/Tests/FileHash.Tests.ps1 b/Tests/FileHash.Tests.ps1 index 64d9cba..91eeb2c 100644 --- a/Tests/FileHash.Tests.ps1 +++ b/Tests/FileHash.Tests.ps1 @@ -6,6 +6,11 @@ )] param () +BeforeDiscovery { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $isElevated = Test-IsElevated +} + BeforeAll { Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' @@ -42,4 +47,48 @@ Describe 'Get-FileHash2' { $results[1].Hash | Should -BeExactly (Get-FileHash -LiteralPath $second -Algorithm SHA256).Hash } } + + Context 'When a file cannot be read' { + # Before 5.0.0, the cmdlet wrote a result for the file anyway, with the hash of the previous file. + It 'Should write an error and no result for the file' { + if ($PSVersionTable.PSEdition -eq 'Core') { + Set-ItResult -Skipped -Because 'Get-FileHash2 fails in PowerShell 7 until it no longer references RIPEMD160' + } + $locked = New-TestSandboxItem -Sandbox $sandbox -Name 'Locked' + $stream = [IO.File]::Open($locked, [IO.FileMode]::Open, [IO.FileAccess]::Read, [IO.FileShare]::None) + try { + $results = @(Get-FileHash2 -Path $first, $locked -ErrorVariable hashErrors -ErrorAction SilentlyContinue) + } + finally { + $stream.Dispose() + } + + $hashErrors | Should -HaveCount 1 + $results.Name | Should -Be @('One.txt') + } + } + + Context 'When the file cannot be read after taking ownership' { + # Before 5.0.0, the account that ran the cmdlet stayed the owner when the second attempt failed. Only an + # elevated process can make another account the owner first, so the test runs in CI. + It 'Should restore the previous owner' -Skip:(-not $isElevated) { + if ($PSVersionTable.PSEdition -eq 'Core') { + Set-ItResult -Skipped -Because 'Get-FileHash2 fails in PowerShell 7 until it no longer references RIPEMD160' + } + $denied = New-TestSandboxItem -Sandbox $sandbox -Name 'Denied' + Assert-TestSandboxPath -Sandbox $sandbox -Path $denied + Set-NTFSOwner -Path $denied -Account 'S-1-5-32-544' + Add-NTFSAccess -Path $denied -Account 'S-1-1-0' -AccessRights ReadData -AccessType Deny + + $results = @(Get-FileHash2 -Path $denied -ErrorVariable hashErrors -ErrorAction SilentlyContinue) + if (-not $hashErrors) { + Set-ItResult -Inconclusive -Because 'the elevated process could read the file despite the deny entry' + } + + $hashErrors | Should -HaveCount 1 + $hashErrors[0].FullyQualifiedErrorId | Should -BeLike 'GetHashError,*' + $results | Should -BeNullOrEmpty + (Get-NTFSOwner -Path $denied).Owner.Sid | Should -Be 'S-1-5-32-544' + } + } } diff --git a/Tests/Inheritance.Tests.ps1 b/Tests/Inheritance.Tests.ps1 index ddcde8e..80d5fc3 100644 --- a/Tests/Inheritance.Tests.ps1 +++ b/Tests/Inheritance.Tests.ps1 @@ -53,6 +53,60 @@ Describe 'Get-NTFSInheritance' { } } +Describe 'Inheritance cmdlets with -PassThru' { + BeforeDiscovery { + # With the Backup privilege, Windows may grant reading the security descriptor despite a deny entry. + $canBypassDeny = Test-PrivilegeHeld -Name 'SeBackupPrivilege' + } + + # Before 5.0.0, the cmdlets wrote the -PassThru object in a finally block, also after a failure (#74). + It '<_> should return nothing when the audit change fails' -Skip:$canChangeAudit -ForEach @( + 'Enable-NTFSAuditInheritance', 'Disable-NTFSAuditInheritance' + ) { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'PassThru' + + $result = @(& $_ -Path $file -PassThru -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue) + + $inheritanceErrors | Should -Not -BeNullOrEmpty + $result | Should -BeNullOrEmpty + } + + It 'Set-NTFSInheritance should return nothing when the audit change fails' -Skip:$canChangeAudit { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'PassThru' + + $result = @(Set-NTFSInheritance -Path $file -AuditInheritanceEnabled $false -PassThru -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue) + + $inheritanceErrors | Should -Not -BeNullOrEmpty + $result | Should -BeNullOrEmpty + } + + It '<_> should write an error and return nothing when the security descriptor cannot be read' -Skip:$canBypassDeny -ForEach @( + 'Enable-NTFSAccessInheritance', 'Disable-NTFSAccessInheritance' + ) { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Denied' + Block-TestReadPermission -Sandbox $sandbox -Path $file + + $result = @(& $_ -Path $file -PassThru -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue) + + $inheritanceErrors | Should -HaveCount 1 + $inheritanceErrors[0].FullyQualifiedErrorId | Should -BeLike 'ModifySdError,*' + $result | Should -BeNullOrEmpty + } + + It '<_> should write a read error and return nothing for a path that does not exist' -ForEach @( + 'Enable-NTFSAccessInheritance', 'Disable-NTFSAccessInheritance', + 'Enable-NTFSAuditInheritance', 'Disable-NTFSAuditInheritance' + ) { + $missing = Join-Path -Path $sandbox -ChildPath 'Missing.txt' + + $result = @(& $_ -Path $missing -PassThru -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue) + + $inheritanceErrors | Should -HaveCount 1 + $inheritanceErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadFileError,*' + $result | Should -BeNullOrEmpty + } +} + Describe 'Set-NTFSInheritance' { Context 'When -AccessInheritanceEnabled or -AuditInheritanceEnabled is omitted' { BeforeEach { diff --git a/Tests/ItemCmdlets.Tests.ps1 b/Tests/ItemCmdlets.Tests.ps1 index 3a7ca2c..31a5f48 100644 --- a/Tests/ItemCmdlets.Tests.ps1 +++ b/Tests/ItemCmdlets.Tests.ps1 @@ -80,6 +80,44 @@ Describe 'Get-ChildItem2' { } } +Describe 'Copy-Item2, Move-Item2, and Remove-Item2 with several paths' { + BeforeEach { + $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'Several' -Directory + $missing = Join-Path -Path $folder -ChildPath 'Missing.txt' + $first = Join-Path -Path $folder -ChildPath 'First.txt' + $second = Join-Path -Path $folder -ChildPath 'Second.txt' + $destination = Join-Path -Path $folder -ChildPath 'Destination' + Assert-TestSandboxPath -Sandbox $sandbox -Path $missing, $first, $second, $destination + Set-Content -LiteralPath $first -Value 'First' + Set-Content -LiteralPath $second -Value 'Second' + New-Item -ItemType Directory -Path $destination | Out-Null + } + + # Before 5.0.0, the cmdlets stopped processing -Path at the first failing path. + It 'Remove-Item2 should continue after a path that does not exist' { + Remove-Item2 -Path $missing, $first -ErrorVariable itemErrors -ErrorAction SilentlyContinue + + $itemErrors | Should -HaveCount 1 + $first | Should -Not -Exist + } + + It '<_> should continue after a path that does not exist' -ForEach @('Copy-Item2', 'Move-Item2') { + & $_ -Path $missing, $first -Destination $destination -ErrorVariable itemErrors -ErrorAction SilentlyContinue + + $itemErrors | Should -HaveCount 1 + Join-Path -Path $destination -ChildPath 'First.txt' | Should -Exist + } + + It '<_> should continue after a file that exists at the destination' -ForEach @('Copy-Item2', 'Move-Item2') { + Set-Content -LiteralPath (Join-Path -Path $destination -ChildPath 'First.txt') -Value 'Existing' + + & $_ -Path $first, $second -Destination $destination -ErrorVariable itemErrors -ErrorAction SilentlyContinue + + $itemErrors | Should -HaveCount 1 + Join-Path -Path $destination -ChildPath 'Second.txt' | Should -Exist + } +} + Describe 'Copy-Item2' { Context 'When -Path is a folder with files and subfolders' { BeforeAll { diff --git a/Tests/Links.Tests.ps1 b/Tests/Links.Tests.ps1 new file mode 100644 index 0000000..f75509a --- /dev/null +++ b/Tests/Links.Tests.ps1 @@ -0,0 +1,44 @@ +<# + Tests the link cmdlets of the module built in NTFSSecurity\bin\Release in a sandbox folder. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'Links' + Push-Location -LiteralPath $sandbox +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'New-NTFSHardLink' { + It 'Should create a hard link to a file' { + $target = New-TestSandboxItem -Sandbox $sandbox -Name 'Target' + $link = Join-Path -Path $sandbox -ChildPath 'Link.txt' + Assert-TestSandboxPath -Sandbox $sandbox -Path $link + + New-NTFSHardLink -Path $link -Target $target -ErrorAction Stop + + $link | Should -Exist + Get-Content -LiteralPath $link | Should -Be (Get-Content -LiteralPath $target) + } + + # Before 5.0.0, the error said "The target path exist" for a target that did not exist. + It 'Should report that a missing target does not exist' { + $missing = Join-Path -Path $sandbox -ChildPath 'Missing.txt' + $link = Join-Path -Path $sandbox -ChildPath 'MissingLink.txt' + Assert-TestSandboxPath -Sandbox $sandbox -Path $missing, $link + + { New-NTFSHardLink -Path $link -Target $missing -ErrorAction Stop } | Should -Throw -ExpectedMessage '*does not exist*' + $link | Should -Not -Exist + } +} diff --git a/Tests/TestHelpers.psm1 b/Tests/TestHelpers.psm1 index 5f11a15..b47faaf 100644 --- a/Tests/TestHelpers.psm1 +++ b/Tests/TestHelpers.psm1 @@ -195,14 +195,70 @@ function Block-TestReadPermission { $Path ) + Add-TestDenyRule -Sandbox $Sandbox -Path $Path -Rights @{ 'S-1-3-4' = 'ReadPermissions' } +} + +function Block-TestWritePermission { + <# + .SYNOPSIS + Denies the owner of an item in the sandbox to change its permissions, so that writing its DACL fails. + .DESCRIPTION + Adds a deny entry for OWNER RIGHTS (S-1-3-4) with ChangePermissions, which replaces the implicit right of the + owner to write the DACL. Taking ownership drops that entry, so the current user is also denied + TakeOwnership; without a privilege, an attempt of the module to take ownership and try again fails as well. + Reading the item and its security descriptor still works, and Remove-TestSandbox deletes the item through + the rights on its folder. + #> + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [string] + $Sandbox, + + [Parameter(Mandatory)] + [string] + $Path + ) + + $currentUser = [System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value + Add-TestDenyRule -Sandbox $Sandbox -Path $Path -Rights @{ + 'S-1-3-4' = 'ChangePermissions' + $currentUser = 'TakeOwnership' + } +} + +function Add-TestDenyRule { + <# + .SYNOPSIS + Adds deny entries to an item in the sandbox in one write, so that an entry cannot block writing the next. + .PARAMETER Rights + The rights to deny, keyed by the SID of the account. + #> + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [string] + $Sandbox, + + [Parameter(Mandatory)] + [string] + $Path, + + [Parameter(Mandatory)] + [System.Collections.IDictionary] + $Rights + ) + Assert-TestSandboxPath -Sandbox $Sandbox -Path $Path $acl = Get-Acl -LiteralPath $Path - $ownerRights = New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-3-4' - $rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( - $ownerRights, [System.Security.AccessControl.FileSystemRights]::ReadPermissions, - [System.Security.AccessControl.AccessControlType]::Deny - ) - $acl.AddAccessRule($rule) + foreach ($sid in $Rights.Keys) { + $identity = New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList $sid + $rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( + $identity, [System.Security.AccessControl.FileSystemRights] $Rights[$sid], + [System.Security.AccessControl.AccessControlType]::Deny + ) + $acl.AddAccessRule($rule) + } Set-Acl -LiteralPath $Path -AclObject $acl } @@ -247,4 +303,4 @@ function Test-PrivilegeHeld { } Export-ModuleMember -Function New-TestSandbox, Assert-TestSandboxPath, Remove-TestSandbox, New-TestSandboxItem, - Block-TestReadPermission, Test-IsElevated, Test-PrivilegeHeld + Block-TestReadPermission, Block-TestWritePermission, Test-IsElevated, Test-PrivilegeHeld