From f31c1c24652471e6014fccb6fa6e9b1dff009847 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Wed, 21 Oct 2026 10:37:22 +0000 Subject: [PATCH] docs(memory-bank): record Decision 27, the scope of rc9 before 5.0.0 A probe on the published rc8 found that a piped optional enum parameter silently ignores a value that PowerShell cannot convert: a typo for Deny in AccessType creates an Allow entry, AuditFlags, the flags, and AppliesTo fall back to their defaults, and Get-FileHash2 runs SHA256. Named calls fail loudly. The maintainer decided on 2026-10-21 that rc9 fixes the family in the four access and audit cmdlets, changes the items 3 to 7 of the sheet, and gets an independent review. Decision 27 holds the probes, the options, the designs that five read-only research threads proposed, and the recommended defaults. The lab suite run of the new tests (24 runs on six machine classes, 0 failed) is recorded as the agent's flagged assumption. activeContext and progress follow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- .memory-bank/activeContext.md | 37 +++-- .../decisions/0027-open-items-before-5.0.0.md | 143 ++++++++++++++++++ .memory-bank/progress.md | 29 ++-- 3 files changed, 175 insertions(+), 34 deletions(-) create mode 100644 .memory-bank/decisions/0027-open-items-before-5.0.0.md diff --git a/.memory-bank/activeContext.md b/.memory-bank/activeContext.md index 9b9d9e0..574a9a8 100644 --- a/.memory-bank/activeContext.md +++ b/.memory-bank/activeContext.md @@ -23,14 +23,14 @@ of the new error, a computer name of 32,718 characters, holds on Server 2019, 2022, 2025, and Windows 11 26H1. He merged the record, #128, on 2026-10-21 at 05:48Z, the tests-only round for the plain gaps (#129, 08:24Z; Next step 6), and this Memory Bank (#130, 08:47Z); `master` is `d0cb57c`. The new tests gave -the same counts on the runner in three CI runs, equal to the local run in each -test file. Open before the stable pull request: the maintainer's decision -on #34 on 2026-11-02 (no reply to his follow-up of 2026-10-19), his check of -`PSGALLERY_API_KEY`, and his decision on the open items (Next steps 3 and 6). -The stable build is new bytes that no lab ran; the procedure accepts the last -prerelease, so the stable change touches only the label, the CHANGELOG, and -`$publishedVersions`. Any other product change needs rc9 and the same -acceptance (about 70 minutes with the script). +the same counts on the runner (three CI runs) and on six machine classes of the +labs (24 runs, 0 failed). A probe found that a typo in a piped `AccessType` +silently creates an Allow entry. He decided (Decision 27): rc9 fixes that +family and the items 3 to 7 of its sheet, with a review. Open before the +stable pull request: rc9 (designs, tests, review, acceptance of about 70 +minutes), his decision on #34 on 2026-11-02, and his check of +`PSGALLERY_API_KEY`. The stable change then touches only the label, the +CHANGELOG, and `$publishedVersions`. State at 2026-10-10: the maintainer integrated the release-gate stack into `master` (#116, #120, #118, #119, #121; `fa0701b`, CI green; GitHub closed #117 @@ -164,11 +164,10 @@ Decisions 22 and 24 on 2026-10-19 and decides 23 on 2026-11-02. `[Unreleased]` and the links, and `5.0.0-rc8` in `$publishedVersions` (the list ends at rc7 now). The maintainer checks the expiry of `PSGALLERY_API_KEY` first, merges, and tags. Every release step is his. -3. He decides the other open items of the paths report: `FileSecurity` - conversions, `RemoveAll` account filters, lazy path overloads, abandoned - `PrivilegeEnabler`, dot patterns of `Get-ChildItem2 -Filter`, the 17 - owner-restore handlers without the later-command check, unused classes - (Decisions 21/22). Decisions 22 and 24 are accepted (2026-10-19). +3. Decision 27 (2026-10-21): rc9 changes the library-only items, the dot + patterns of `Get-ChildItem2 -Filter`, the 17 owner-restore handlers, the + errors of the descriptor sets, and three inconsistencies; designs are being + researched. Decisions 22 and 24 are accepted (2026-10-19). 4. #34 waits for a tester until 2026-11-02 (Decision 23, option 3; the follow-up went up on 2026-10-19, no reply yet; checklist: `Tests/Lab/Non-Windows-File-Server-Test.md`). On that date the maintainer @@ -180,8 +179,8 @@ Decisions 22 and 24 on 2026-10-19 and decides 23 on 2026-11-02. only with a result. He still decides the other: the ACL of the stage folders under `C:\` in the lab kit (they inherit Authenticated Users: Modify; protecting them is a design change of the controller and needs a new - acceptance). The help could also say that a local standard user who asks - about a domain account still gets "Access is denied". + acceptance). rc9 adds to the help that a local standard user who asks about + a domain account still gets "Access is denied" (Decision 27). 6. Handoff 5 is done and merged (Evidence). Decided on 2026-10-19 (Decision 21): the hard-link names under another root stay a known limit; the pages and the CHANGELOG say so since rc8. The tests-only round for the plain items @@ -191,8 +190,8 @@ Decisions 22 and 24 on 2026-10-19 and decides 23 on 2026-11-02. with a leftover (16 Limit, 7 MaintainerDecision, 3 would-pin), no defect; report `Tests/Coverage/Quality-Gate-Test-Round-2026-10-20.md`. The runner's counts equal the local run in three CI runs. No plain gap is left. A defect - that a test finds needs his decision and rc9. Still his: the 7 - MaintainerDecision and 3 would-pin rows, the silent default scope for a - piped `AppliesTo` that PowerShell cannot convert (a design change), the 8 - getters of pipeline parameters, and the audit paths. + that a test finds needs his decision and rc9. Decision 27 settles the rest: + rc9 fixes the 10 rows and the piped optional parameters (designs to + follow); the audit paths over SMB are accepted (#34); the tests for the 8 + getters follow rc9. 7. Do not release stable 5.0.0 or equate a percentage with gate closure. diff --git a/.memory-bank/decisions/0027-open-items-before-5.0.0.md b/.memory-bank/decisions/0027-open-items-before-5.0.0.md new file mode 100644 index 0000000..8e94c6d --- /dev/null +++ b/.memory-bank/decisions/0027-open-items-before-5.0.0.md @@ -0,0 +1,143 @@ +--- +status: accepted +date: 2026-10-21 +last-verified: 2026-10-21 +owner: shared +source: maintainer answers of 2026-10-21 (fix scope B, items 3 to 7 change, review on); probes of 2026-10-21 on the published rc8 +--- + +# Decision 27: The open items before 5.0.0, and the piped optional parameters + +- Context: the tests-only round left no plain test gap (#129). On 2026-10-21 + the maintainer chose to fix the silent default scope of a piped `AppliesTo` + before 5.0.0 (rc9) and to go through the other open items one by one. rc9 + is the last prerelease: a change after its acceptance needs rc10, and after + 5.0.0 the repository is archived (Decision 18). So this is the last moment + to change any of the items below, and unchanged means accepted. +- Status: accepted at the level of scope (see "The maintainer's choices"). The + designs of the items 3 to 7 are still to be proposed and chosen. Every + recommendation in the tables is the agent's; the last column of the table of + the other items stays empty, because the choices are listed below. + +## New finding: the flaw is a family + +PowerShell skips a property of a piped object whose value it cannot convert to +the type of an optional parameter, and the parameter keeps its default. A named +call fails loudly; the same value on a piped object does not. Probe +`p-unconvertible-optional-params.ps1`, the published rc8, both editions (the +only difference is how `ReadData` prints): + +| Piped property | What rc8 does | +| --- | --- | +| `AccessType = 'Deny'` | A Deny entry, as intended | +| `AccessType = 'Denyy'` (typo), or a blank CSV cell | **An Allow entry**, no message | +| `AuditFlags = 'Failurre'` | Success and Failure are audited (the default), no message | +| `InheritanceFlags` or `PropagationFlags = 'Typo'` | Ignored; the default flags (`ContainerInherit`, `ObjectInherit`) | +| `AppliesTo = 'Typo'`, a blank cell, or `99` | Ignored; the broader default scope | +| `Remove-NTFSAccess` with `AccessType = 'Denyy'` | Looks for an Allow entry: the Deny entry stays, no message | +| `Get-FileHash2` with `Algorithm = 'SHA999'` | SHA256, no message | +| `-AccessType Denyy` as a named parameter | A binding error, as it should be | + +The decision of 2026-10-21 covers `AppliesTo` only. A typo for `Deny` that +creates an Allow entry is at least as serious, and it has the same cause. + +## Options for the fix + +Prototypes (`p-proto-appliesto-shapes.ps1`, `p-proto-object3.ps1`), both +editions, same attribute shape as `Add-NTFSAccess`: + +| Shape | Result | +| --- | --- | +| One parameter set with optional parameters | Does not help: the binder skips the unconvertible value there too | +| `AppliesTo` as `object` alone | Loud for a bad scope, but a CSV row with `AppliesTo` and `InheritanceFlags` is no longer `AmbiguousParameterSet`: the scope silently wins | +| `AppliesTo`, `InheritanceFlags`, `PropagationFlags`, and `AccessType` or `AuditFlags` as `object`, converted in the cmdlet with `LanguagePrimitives.ConvertTo` | Loud for every bad value (a non-terminating error for that object, the next object goes on, nothing carries over); `AppliesTo` with flags is `AmbiguousParameterSet` for every mix of strings and enums; valid values unchanged | + +| Option | What it is | Cost | +| --- | --- | --- | +| A | `AppliesTo` only (the decision as asked) | Leaves the Deny-to-Allow flip | +| B | The family in the four cmdlets (recommended): `AppliesTo`, `InheritanceFlags`, `PropagationFlags`, `AccessType` (access), `AuditFlags` (audit) | See below | +| C | B and `Get-FileHash2 -Algorithm` | B plus one cmdlet of low severity | +| D | No code change: pages and CHANGELOG say that piped values are not checked | Weak for a permissions tool | + +Consequences of B: the help shows `` for these parameters (the pages +list the values; an argument completer keeps tab completion); a named call +with a bad value is an error record of the cmdlet instead of a terminating +binding error; a blank cell for any of them is an error, not the default; a +`$null` `AccessType` stays the default (Allow); the pipeline conflict check is +now uniform. The work: tests first (the pins of the silent behavior change), +four cmdlets, four pages, the help file, the CHANGELOG, rc9, and the +acceptance (about 70 minutes with `Chain-Published.ps1`). An independent +review is recommended: public behavior and the permission scope. + +## The other open items + +| # | Item | What a user can notice | If changed | Recommended | Choice | +| --- | --- | --- | --- | --- | --- | +| 2 | The help says that a local standard user who asks about a domain account still gets "Access is denied" | Nothing changes in behavior | Docs only; rides with rc9 | Do it in rc9 | | +| 3 | Library-only items of the DLL that no cmdlet reaches: the `FileSecurity` conversions always throw; the `RemoveAll` filters ignore the account list (#113); the lazy path overloads; a `PrivilegeEnabler` that is not disposed stays enabled; the unused registry model and helper classes; `PipelineControl.IsEnd`; one unvisited catch in `Enable-Privileges` | Only .NET callers of the DLL | Code change and rc9 | Accept; list as known limits of the .NET API | | +| 4 | `Get-ChildItem2 -Filter` dot patterns differ from `Get-ChildItem` (`Report.*` misses `Report`; `Rep*.` returns nothing) | Documented, pinned by tests | New matching code and rc9 | Accept as documented | | +| 5 | The `-SecurityDescriptor` sets of the four add and remove cmdlets end with a terminating error when the change throws; the `-Path` sets write an error per item and go on (7 test rows stay unpinned) | A failing change to a descriptor in memory stops the cmdlet | Per-item handlers and rc9 | Accept | | +| 6 | Three small inconsistencies: the error category of `Get-NTFSAccess` after a failed ownership retry (`WriteError` there, `OpenError` elsewhere); `Get-NTFSOrphanedAccess` warns where its siblings write an error; `Set-NTFSSecurityDescriptor` takes `-Path` at position 2, its siblings at 1 | Scripts that check a category or pass a path by position | A breaking change for such scripts, and rc9 | Accept | | +| 7 | Seventeen owner-restore handlers do not pass on what a later command raises, in the rare case that it throws only for the `RestoreOwnerError` record | Rare | One check per handler, an elevated-only test, rc9 | Accept | | +| 8 | The ownership retry and the read denial of an audit write over SMB have no trigger on a Windows file server | None known | A seam (a design change), or the answer of a non-Windows server (#34) | Accept; #34 decides on 2026-11-02 | | +| 9 | Eight getters of piped parameters that no test binds by property name | None (tests only) | Tests only, no rc9 | Do it after the code of rc9 is final; the fix tests cover `AccessType`, `AuditFlags`, and `Algorithm` | | +| 10 | Lab and process only: the ACL of the lab stage folders under `C:\`, rollback checkpoints that report Standard, a publication recovery that never ran against the Gallery, test helpers that accept a link, the cleanup list (AppVeyor, Read the Docs, wiki, `test/transfer`, old checkpoints) | Not shipped | n/a | Accept; the cleanup before the archive is the maintainer's | | +| 11 | The deferred reviews of rc3 to rc7 (`progress.md`, open work 3) | Not new | n/a | Accept | | + +Decided before: the hard-link names under another root stay a documented known +limit (Decision 21); Decisions 22 and 24 are accepted; #34 is decided on +2026-11-02 (Decision 23). + +## The maintainer's choices, 2026-10-21 + +| Question | Answer | +| --- | --- | +| Scope of the fix of the piped optional parameters | B: the family in the four cmdlets (`AppliesTo`, `InheritanceFlags`, `PropagationFlags`, `AccessType`, `AuditFlags`) | +| Items to change in rc9 | 3 (library-only items), 4 (`Get-ChildItem2 -Filter` dot patterns), 5 (descriptor sets report per descriptor), 6 (three inconsistencies), 7 (17 owner-restore handlers) | +| Items accepted as recommended | 8 (audit retry and read denial over SMB; #34 decides on 2026-11-02), 10, 11. Items 2 and 9 need no decision: the help sentence goes into rc9, and the tests for the eight getters follow when the code of rc9 is final | +| Independent review of the rc9 change | Yes (`review: on`) | + +What follows: the agent proposes the design of each of the items 3 to 7 +(options, risk, tests, a recommended default); the maintainer chooses; the work +goes test-first into one branch, one commit per item, so that any item can be +dropped. A defect that the review finds is fixed before rc9 is tagged; a defect +that the acceptance finds needs rc10. One acceptance (about 70 minutes with +`Chain-Published.ps1`) covers rc9 as a whole. + +## Designs proposed, 2026-10-21 + +Five read-only research threads read the code, the tests, and the docs. The +sheet below is their result, verified in part by the probes above. Fix B is +decided; the rest is to be chosen. Recommended defaults are the agent's. + +| # | Design | Recommended | Alternatives and risks | +| --- | --- | --- | --- | +| B | `AppliesTo`, `InheritanceFlags`, `PropagationFlags`, `AccessType` (access cmdlets), `AuditFlags` (audit cmdlets) become `object` parameters; ProcessRecord converts them with `LanguagePrimitives.ConvertTo` into locals. One shared error: id `InvalidPipedParameterValue`, category `InvalidData`, target the piped object, message "The value '{0}' for parameter '-{1}' could not be converted to '{2}'. The piped object was skipped." (blank: "The value for parameter '-{0}' is blank. The piped object was skipped."). A null `AccessType` or `AuditFlags` stays the default. Argument completers (`IArgumentCompleter`, available in the referenced Windows PowerShell 5.0 assembly) keep tab completion. Docs: `Type: Object`, help from `New-ExternalHelp`, the CHANGELOG paragraph at lines 247 to 258 | As described | `Get-FileHash2 -Algorithm` has the same flaw (option C, not chosen). Named calls change from a terminating binding error to the error record of the cmdlet | +| 5 | In the four `-SecurityDescriptor` branches, a `try/catch (Exception)` around the change of each descriptor writes `AddAceError` or `RemoveAceError`, category `WriteError`, target the descriptor, message of the exception, and goes on; the later-command check of item 7 goes into the same catch. Lines: `AddAccess.cs` 176 to 184, `RemoveAccess.cs` 177 to 185, `AddAudit.cs` 175 to 185, `RemoveAudit.cs` 176 to 186 | As described | Tests: a failing descriptor followed by a valid one; the 7 unpinned rows | +| 7 | One `IsFromLaterCommand(ex)` check, rethrowing, in each of the 17 outer handlers (the pattern of `SetSecurityDescriptor.cs` 72 to 101): `AddAccess`, `RemoveAccess`, `ClearAccess`, `GetAccess`, `GetOrphanedAccess`, `GetEffectiveAccess`, `AddAudit`, `RemoveAudit`, `ClearAudit`, `GetFileHash2`, the six inheritance cmdlets, and `SetSecurityDescriptor` | As described | Tests: one parameterized elevated-only test (a later command that throws on the `RestoreOwnerError` record), like the existing `restoreOwnerCases` | +| 3.1 | `FileSystemSecurity2` conversions from `FileSecurity` and `DirectorySecurity` keep the given descriptor | Fix | Remove the operators: breaking public API. Medium risk | +| 3.2 | `RemoveFileSystemAccessRuleAll` and `RemoveFileSystemAuditRuleAll` honor the account list | Fix | High risk for external callers (they remove entries); the cmdlets pass no list. Tests for both rule types | +| 3.3 | The path and several accounts overloads of the two add methods write at once, like their siblings | Fix | Medium risk (the moment of an exception changes) | +| 3.4 | `PrivilegeEnabler` no longer keeps an undisposed instance reachable | Defer unless the maintainer insists | High risk: token state in every cmdlet; a finalizer test is not deterministic; every cmdlet disposes its enabler today | +| 3.6 | The registry model (about 500 lines), `FileSystemFlags`, `FileSystemEffectivePermissionEntry`, and two raw descriptor readers | Keep and document (Decisions 21 and 22) | Removal breaks unknown .NET callers of the public DLL | +| 3.9 | `PipelineControl.IsEnd` | Keep as defense in depth | A change touches the pipeline semantics of all cmdlets for no known defect | +| 3.10 | The malformed `EnablePrivileges` setting: one validation in the base class, the dead catch removed | Fix | Medium risk (base class) | +| 4 | `Get-ChildItem2 -Filter`. A: pass the pattern unchanged to AlphaFS (the Windows PowerShell 5.1 rules; brings back the 8.3 short-name matches that a past fix removed). B: one cmdlet-side matcher with the Win32 dot rules on the real names and a broad enumeration (about 40 lines; costs a larger enumeration over SMB). D: keep as documented | B | Even `Get-ChildItem` differs between the editions here, so "aligned" needs a stated policy: the Windows PowerShell 5.1 dot rules on real names, in both editions | +| 6a | `Get-NTFSAccess`, the category of `ReadSecurityError` after a failed ownership retry: `WriteError` in `Get-NTFSAudit`, `Get-NTFSEffectiveAccess`, `Get-NTFSInheritance`, and `Get-NTFSSecurityDescriptor` too, so it is the majority convention | Keep, and pin it with a test | `OpenError` would break scripts that check the category, and make the module inconsistent unless six cmdlets change | +| 6b | `Get-NTFSOrphanedAccess` warns for a read failure other than access denied | An error: `ReadSecurityError`, `OpenError`, target the path | Breaks `-WarningAction` users; `-ErrorAction Stop` newly stops | +| 6c | `Set-NTFSSecurityDescriptor` has `-SecurityDescriptor` at position 2 and no other positional parameter | Position 1 | Cosmetic: a lone positional parameter binds first whatever its number | + +## Assumption flagged for the maintainer's review + +- The lab suite: he answered "not sure, you pick". The agent took the + recommendation: run the full suite (1,615 cases per configuration) on the + lab machine classes now, against the published rc8, so that an OS-specific + finding can still go into rc9 instead of forcing rc10. It changes nothing in + the repository or the product. Evidence: `suite-newtests` in the session + folder. Result (2026-10-21, 09:10 to 09:33 UTC): 24 runs on six machine + classes (Windows Server 2019, 2022, and 2025, Windows 11 22H2 and 26H1, and + the host), 0 failures. Every run gained exactly 121 cases against the rc8 + acceptance, and the four new files give the counts of the CI runner (elevated + Desktop: Access 74/0, Audit 55/2, Inheritance 72/1, Items 90/0, executed and + skipped). No OS-specific finding. The machines are saved again, and + `OSWin11E` is off. diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index 3baabd1..aac6b74 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -33,16 +33,14 @@ Gallery version: 4.2.6. After 5.0.0, archive in favor of WindowsAccessControl - 2026-10-08: #114 merged (`fcb370e`), rc5 published and live-tested. Decision 21 established the quality gate. Corrected four-run coverage: rc5 58.1% sequence points/38.0% branches, not the initial one-run result. -- 2026-10-08: rc6 Phase 2 added basic-user CI, parameter-set/error tests, - expanded domain/SMB cases, and fixes for privilege cleanup, path - resolution, ownership retries, item conflicts, output equality, and - inherited flags. Suite: 677; coverage 68.14% sequence/44.32% branches. - Lab acceptance of `7b0781f` passed; two independent review passes. -- 2026-10-08: rc7 implemented proposed Decision 22, including breaking - link binding/error changes, SimpleAccess traversal, cross-volume folder - preservation, and named effective-access warnings. Suite: 712, no - failures or test skipped everywhere. One review: no Blocker/Major. - Lab candidate `dc6e9f5`: 326 passed, 2 skipped, cleanup verified. +- 2026-10-08: rc6 Phase 2 (basic-user CI, parameter-set and error tests, + expanded domain/SMB cases, fixes for privilege cleanup, path resolution, + ownership retries, item conflicts, output equality, inherited flags; suite + 677, 68.14% sequence/44.32% branches; lab acceptance of `7b0781f` passed; + two review passes) and rc7 (Decision 22: breaking link binding and error + changes, SimpleAccess traversal, cross-volume folder preservation, named + effective-access warnings; suite 712; one review without Blocker or Major; + lab candidate `dc6e9f5`: 326 passed, 2 skipped, cleanup verified). - 2026-10-08: #115 merged (`b51d970`) and tagged rc6. Release run `37839669028` failed with HTTP 409 after Gallery publication. The log does not establish the previously assumed initial timeout/retry cause. @@ -135,8 +133,10 @@ Gallery version: 4.2.6. After 5.0.0, archive in favor of WindowsAccessControl of 64 sets closed (7 before), 26 with a leftover (16 Limit, 7 MaintainerDecision, 3 would-pin); no defect. Report: `Tests/Coverage/Quality-Gate-Test-Round-2026-10-20.md`. On 2026-10-21 he - merged #129 (08:24Z) and #130 (08:47Z); the runner gave the local counts in - three CI runs (`39551726767` and two more). + merged #129 (08:24Z) and #130 (08:47Z); the new tests passed in three CI + runs and in 24 lab runs on six machine classes (0 failed). A probe then + found that piped optional parameters ignore a bad value (a typo for Deny + creates an Allow entry); Decision 27 sets the scope of rc9. ## Stable capabilities @@ -151,9 +151,8 @@ Gallery version: 4.2.6. After 5.0.0, archive in favor of WindowsAccessControl 1. Decision 21 gate: the published rc7 and rc8 are accepted in the matrix and the first lab (records); stable follows rc8 (Decision 21, "Closing the - gate"). Do not release 5.0.0 until the other gates close: the decision on - #34 on 2026-11-02, the key check, and the decision on the open items (no - plain test gap is left). Release steps: + gate"). Do not release 5.0.0 until the other gates close: rc9 (Decision + 27), the decision on #34 on 2026-11-02, and the key check. Release steps: `Docs/Contributing/05-Releasing.md`; remove the prerelease label, date `[5.0.0]`, add the last prerelease (rc8) to `$publishedVersions` (never the version of the manifest), tag through CI.