diff --git a/.memory-bank/activeContext.md b/.memory-bank/activeContext.md index 4a30140..cc03cd4 100644 --- a/.memory-bank/activeContext.md +++ b/.memory-bank/activeContext.md @@ -1,177 +1,131 @@ --- status: current -last-verified: 2026-10-06 +last-verified: 2026-10-08 owner: software-engineer -source: repository, git reflog, and public GitHub API evidence +source: repository, GitHub API, PowerShell Gallery, and domain-lab evidence --- # Active context ## Current task -The 2026-10-06 handoff sequence has run as far as it can without the user, -who delegated its decisions overnight; every agent decision is in the -handoff ledger for review. Handoff 07 is Blocked: this machine runs Hyper-V -and AutomatedLab but holds no lab VMs and no lab store, so live acceptance of -the combined candidate waits for the lab host and should precede the next -stable release. Handoff 08 is prepared and stopped before its first remote -action. Its pull request body follows `.github/PULL_REQUEST_TEMPLATE.md`, -which the prompt wrongly said did not exist, and its report lists every -remaining remote step with the exact command. GitVersion 5.12.0 on a -simulated merge reports `0.3.0-preview0002`. On `023afe9` both full gates -pass: Core 1,931 passed, 0 failed, 2 skipped at 83.11 percent asserted -coverage; Desktop 1,886 passed, 0 failed, 2 skipped at 80.95 percent. -`ai/test-gap-audit` no longer exists on the remote. Next, the user approves -or declines each remote step of 08, works through the 06 settings checklist, -and runs 07 on the lab host. No git remote was mutated. - -Handoff 06 restored the `## [0.2.0] - 2026-09-06` changelog section that the -pipeline never merged, with every later entry still under `[Unreleased]` and -a multiset proof that no line moved, and made a stable release end with an -error when its changelog pull request is missing, because Sampler's -`Create_ChangeLog_GitHub_PR` only logs a refused creation. The likeliest -cause of the v0.2.0 gap is a `GitHubToken` secret without pull-request scope; -the step log of run `34058863590` would settle it and needs an authenticated -download by the user. FIND-002 is closed. - -Handoff 05 pinned `actions/checkout` 7.0.1, `actions/upload-artifact` 7.0.1, -and `actions/download-artifact` 8.0.1 by the SHA each tag resolves to, which -ends the Node.js 20 deprecation warning. Every release note from v4 to each -target was checked against this workflow and no input changed meaning, so the -commit is seven pins; `download-artifact` v8 now fails a job on a digest -mismatch instead of warning, which is kept. A `tests/Unit/Build` test holds -every `uses:` reference to a SHA pin with a version comment and to a release -line that declares Node.js 24. The three Dependabot pull requests are -superseded: the seven changed lines are byte-identical to their proposals. - -Handoff 04 settled the NTFSSecurity positioning that the user confirmed: -`WindowsAccessControl` is the successor of NTFSSecurity, which is deprecated -and will be archived. `docs/migration-from-ntfssecurity.md` maps all 37 -commands that the 4.2.6 and 5.0.0-rc4 manifests export, confirmed against the -built module, with the parameter and output differences and the decision -behind each missing feature. The documentation index, the wiki Home page, the -README, and three See also lists point at the guide, and the README no longer -calls private keys read-only. The issue #4 reply is a draft in the handoff -`reports` folder; its wiki link 404s until `main` has the guide. - -Handoffs 01 to 03 made the lab runner refuse to delete what it did not -create, fixed the Task Scheduler, SMB, and AD review defects, and ported and -re-measured the performance refactor; the Desktop gate passes narrowly at -80.95 percent asserted coverage, and handoff 07 owns the first live and -installed-package acceptance of the refactor. On 2026-10-06 the user reviewed -all 14 recorded agent decisions of handoffs 01 to 03: twelve are accepted as -recorded, and the other two were overtaken by doing the work (the decision log -moved out of `systemPatterns.md`, and specs 0005 and 0006 name the descriptor -benchmark). WinRM stays enabled here. Later handoffs ask their own questions -with fresh evidence. The 2026-10-06 triage below remains the record of the -open work. `main` is `40e364a`, in sync with `origin/main`; -`ai/performance-refactor` stays at `5f06c03` until handoff 08 deletes it with -approval. +Handoff 07 and its follow-ups are done. Asked on 2026-10-07 whether every live +test was done, the user chose to give a test to each of the 17 commands in the +declared domain-lab-only files that nothing executed, including a source change +to make four of them unit-testable. The +[2026-10-08 acceptance](../docs/lab-acceptance-2026-10-08.md) accepted the +branch candidate `bad1302`, built as `0.3.0-record` with GitVersion 5.12.0: + +- `4096d02` routes `Get-WindowsADRootDse` and + `Get-WindowsADEffectiveAccessRecord` through `Send-WindowsADSearchRequest`, + so six unit cases reach their four guards. A deleted object during + `Get-ADObjectCallerEffectiveAccess` now reports `ItemNotFoundException` + (changelog `Changed`). +- `938bdff` and `bad1302` add eight live cases: five SMB share guards with the + confirmation prompt, two injected enrichment failures, and a deletion inside + the effective-access read. Each was red against a build with its guard + removed. +- DSC 5 of 5; four passes of 109 cases with eight ready cleanup entries; local + gates Core 1,937 at 91.26 percent and Desktop 1,892 at 90.53 percent + asserted; domain-lab-only coverage 100 percent, 154 of 154. + +The user then had the harness defect of that round corrected and its +AutomatedLab lesson moved into the shared Skill (2026-10-08): + +- `a572d3d` runs the remote body of `Enter-WindowsAccessControlMemberCoverage` + in a child scope, so arming coverage no longer leaves a member session + stopping on every error. A unit case failed with `Stop` before the change, + a live probe showed both behaviors, a built pass ran 109 of 109 in both + editions with line-identical lab coverage, and the local gates passed: + Core 1,938 and Desktop 1,893. The record's Harness correction section holds + the evidence. +- The CopilotAtelier `automatedlab-deployment` Skill now warns, on a local + branch there, that `Remove-LabVMSnapshot` deletes the named checkpoint's + subtree, with a per-VM recipe that keeps newer checkpoints. Merging and + deploying it is the user's step; deploying rewrites the installed copy that + running sessions use. + +The [2026-10-07 record](../docs/lab-acceptance-2026-10-07.md) holds the +acceptance of `2ebb3a5` as `0.3.0-preview0002` and the first follow-up. Every +commit since `ce22f00` sits on the local branch +`ai/record-post-release-publication` and is not pushed, because any push to +`main` publishes another preview. The branch returns to the development machine +as a bundle in the handoff folder, so these commits reach `main` together +(user, 2026-10-07). ## Handoff sequence -Eight sequential handoff prompts now drive the open work. They live outside -the repository in the user's desktop folder -`WindowsAccessControl-handoffs-2026-10-06`, with a README, a ledger, and a -`reports` folder; trust that ledger and git history for progress. All prompts -commit to the local integration branch `ai/post-release-fixes`, created from -`ai/post-release-triage`. Execution order, which supersedes the triage order -below: 01 lab-runner ownership guard, 02 Task Scheduler, SMB, and AD -defects, 03 performance-refactor port, 04 NTFSSecurity guide and issue #4 -draft, 05 Actions bumps, 06 changelog section, silent changelog pull request -step, and FIND-002, 07 live lab acceptance, 08 push, pull request, release, -and cleanup. Only 08 touches the remote, with approval per action. +Eight prompts in the user's desktop folder +`WindowsAccessControl-handoffs-2026-10-06` drove the post-release work; its +ledger and reports hold every ruling and the validation evidence. All eight +are done. The user accepted every agent decision of 01 to 08, answered the +07 housekeeping questions explicitly, and ran each remote step of 08 from +commands the agent handed over, because the house rules block agent pushes. + +## Lab state + +- All thirteen VMs run with one checkpoint, `wac07-after-bad1302-9d9776ca`, + taken after the 2026-10-08 acceptance. Removing `wac07-pre-livegaps-83cd16fa` + with `Remove-LabVMSnapshot` also deleted its child + `wac07-pre-938bdff-098b3392` on ten VMs, so the user had the new checkpoint + taken and the remaining old ones merged away with `Remove-VMSnapshot`. +- The unmarked `C:\WacRepo` and `C:\WacLive` folders on `F1ADC1` were removed + at the user's request after the run proved them unchanged, so the runner's + default `-RemoteRepositoryPath` works again. +- The unmarked `0.0.1` and `0.2.0` installations on `F1ADC1` and the `0.0.1` + installation on `F1DC1` stay (user). An installed-package pass of a `0.0.1` + fallback build is therefore still refused on `F1ADC1`; build acceptance + candidates with the GitVersion version. +- The lab host deletes per-session TEMP directories at logoff, and the private + evidence of 2026-09-07 is no longer there. The 2026-10-07 evidence sits in + the profile's base TEMP directory. ## Release state -- `v0.3.0-preview0001` shipped from `40e364a` on 2026-09-07: Build run - `34158650074` passed, and the GitHub release and PowerShell Gallery package - were published at 20:32 UTC. The earlier "release awaits authorization" - note was stale. -- `v0.2.0` is the current stable release (2026-09-06). -- The [reacceptance record](../docs/lab-reacceptance-2026-09-07.md) and the - [security review](../docs/security-review-2026-09-07.md) remain the evidence - for the shipped candidate. +- `v0.3.0-preview0002` shipped from `2ebb3a5` on 2026-10-07 and is now + accepted in the domain lab, on the installed package as well. +- `v0.2.0` remains the stable release (2026-09-06). +- A stable release still needs explicit authorization to merge, tag, push, + or publish. ## Open work, in recommended order -1. **Performance refactor, ported by handoff 03.** `ai/performance-refactor` - (five commits on `5991673`, 2026-09-10, never pushed) is now on - `ai/post-release-fixes` as `fbced95`, `d13a517`, and `b860448`, re-measured - against the current baseline. Live-lab and installed-package acceptance of - it never ran; handoff 07 owns them. The source branch stays at `5f06c03` - until handoff 08 deletes it with approval. -2. **Repository-wide review findings (2026-09-10).** They were recorded only - on that branch. Rechecked by reading `main` on 2026-10-06, not by running - anything: - - Blocker (lab harness), fixed by handoff 01 on `ai/post-release-fixes`: - `tests/Lab/Invoke-WindowsAccessControlLabAcceptance.ps1` recursively - deleted `-RemoteRepositoryPath` (only `ValidateNotNullOrEmpty`) on the - domain controller and replaced any same-version module under - `%ProgramFiles%\WindowsPowerShell\Modules\WindowsAccessControl`. It now - refuses unmarked directories. The existing `C:\WacRepo` and the `0.0.1` - installation on `F1ADC1` predate the marker, so 07 must decide what to do - with them before its first run. - - Post-release follow-ups (user, 2026-10-06): the SMB, certificate - private-key, Task Scheduler, and foreign-principal suites recursively - delete `C:\WindowsAccessControlLab\ModuleUnderTest` on the member server - without an ownership check; it is a fixed child of the harness's marked - member root. And `-SkipPayloadDeployment -ModuleSource Installed` with no - payload root creates an unmarked root that a later full deployment - refuses; it fails safe and can be removed by hand. - - Major, fixed by handoff 02: `Test-WindowsTaskSchedulerSystemAce` - refused a write that restores SYSTEM to a DACL without a SYSTEM ACE. A - missing or null current DACL is still refused. - - Major, fixed by handoff 02: `Set-WindowsSmbShareSecurityDescriptor` - restored the description captured at target resolution, overwriting a - concurrent edit. - - Major: the opt-in ModuleFast path in the vendored `Resolve-Dependency.ps1` - downloads and runs `bit.ly/modulefast` unverified. Disabled by default. - - Major (CNG final read) is FIND-001, resolved on `main` by `f5731f1`. - - Minor, fixed by handoff 02: `Get-ADObjectCallerEffectiveAccess` requested - `nTSecurityDescriptor` it never used, and a caller without read-control - access got a null-index error. Live evidence is owed by handoff 07. - - Follow-ups: the batch dispatcher `CommandName`/`ObjectFamily` guard is - tested since handoff 03, and handoff 04 corrected the README private-key - statement against specification 0015. -3. **Dependabot pull requests #1-#3** are superseded by handoff 05, which - applied all three bumps on `ai/post-release-fixes` in one commit: - `actions/checkout` 7.0.1, `actions/upload-artifact` 7.0.1, and - `actions/download-artifact` 8.0.1, each pinned by the SHA its tag resolves - to. The seven changed lines are byte-identical to the three proposals. - Reading every release note from v4 to each target found no input that - changed meaning for this workflow; `download-artifact` v8 now fails a job - on a digest mismatch instead of warning, which is kept. Their red test jobs - were inherited anyway: all three branch from `4806726` (2026-08-15, 42 - commits behind `main`), whose own push build failed the same four NTFS path - tests that `0502254` fixed the next morning. Handoff 08 closes #1-#3 with - approval, after the pull-request CI proves the bumps. -4. **Issue #4** (2026-09-07) asks whether this module replaces NTFSSecurity. - Handoff 04 answered it in `docs/migration-from-ntfssecurity.md` and left - the reply as a draft in the handoff `reports` folder. Handoff 08 posts it - only with approval and only after the guide reaches `main`, because the - linked URL returns 404 until then; re-check the NTFSSecurity Gallery state - first. Observation for a later decision: `Remove-NTFSAccessRule` and - `Remove-NTFSAuditRule` in their default `Exact` mode remove nothing and - report nothing when no identical entry exists, which a migrated - NTFSSecurity call that subtracts rights hits. The guide documents it; no - behavior changed. -5. **Changelog, closed by handoff 06.** `CHANGELOG.md` carries the 0.2.0 - section, and the publish job now ends a release whose changelog pull - request is missing. Branch `updateChangelogAfterv0.2.0` (`38cdabd`) is - superseded and must not be merged; handoff 08 deletes it with approval. - Left for the user: confirm that the `GitHubToken` secret grants - pull-request write access, and download the step log of run `34058863590` - if the cause needs proof rather than the best-supported hypothesis. -6. **FIND-002, closed by handoff 06**: both test files end with CRLF, and - `docs/security-review-2026-09-07.md` records the disposition. -7. **Housekeeping**: remote `ai/test-gap-audit` is merged. The thirteen lab - VMs, their checkpoints, and the administrator `%TEMP%` evidence are not - visible from this session's account; confirm before keeping or removing - them. - +1. **Changelog pull request token.** `GitHubToken` is a fine-grained token, + and the user is granting it "Pull requests: Read and write". The Actions + setting "Allow GitHub Actions to create and approve pull requests" stays + off, because it governs only `GITHUB_TOKEN`, which the changelog step does + not use. The guard from handoff 06 fails the next stable release if the + pull request is still refused. +2. **Lab-runner follow-ups** (user, 2026-10-06): the SMB, certificate + private-key, Task Scheduler, and foreign-principal suites delete + `C:\WindowsAccessControlLab\ModuleUnderTest` on the member server without + an ownership check, and `-SkipPayloadDeployment -ModuleSource Installed` + with no payload root creates an unmarked root that a later full deployment + refuses. +3. **Observation, 2026-10-07:** `Invoke-WindowsAccessControl` writes one + `$null` when its script block returns nothing, so `@()` around it counts + one item. Emitting nothing instead would be a behavior change and needs a + specification update first. +4. **Observation, 2026-10-08:** a prototype that stopped a remote job blocked + on the share-write confirmation and then wrote again left the `WacLab$` + description empty with its DACL unchanged. Four controlled replays did not + reproduce it; the cause is not established. +5. **Review finding, unscheduled:** the opt-in ModuleFast path in the vendored + `Resolve-Dependency.ps1` downloads and runs `bit.ly/modulefast` unverified; + it is disabled by default. +6. **Observation:** `Remove-NTFSAccessRule` and `Remove-NTFSAuditRule` in their + default `Exact` mode remove nothing and report nothing when no identical + entry exists, which a migrated NTFSSecurity call that subtracts rights + hits. The migration guide documents it; a warning would be a behavior + change and needs a specification update first. +7. **Watch:** CI has no lab coverage, so its Desktop asserted coverage stays + near the 80.95 percent of 2026-10-06 against the 80 percent threshold; + with lab evidence merged it is 90.53 percent (2026-10-08). GitHub moves + `ubuntu-latest`, which runs the publish job, to Ubuntu 26 from 2026-10-19. ## Limits -Remote deletions, pull request comments, issue replies, pushes, and releases -need explicit user authorization. The review findings above were confirmed by -reading code, not by fault injection or live runs. +Agents may not push or otherwise mutate the remote, even with approval; the +user runs those commands. Pushes, merges, releases, remote deletions, and +pull request or issue comments need explicit user authorization. The +2026-09-10 review findings were confirmed by reading code, not by fault +injection or live runs. diff --git a/.memory-bank/debugging-insights.md b/.memory-bank/debugging-insights.md index 81fc900..7b4298d 100644 --- a/.memory-bank/debugging-insights.md +++ b/.memory-bank/debugging-insights.md @@ -1,6 +1,6 @@ --- status: current -last-verified: 2026-10-06 +last-verified: 2026-10-08 owner: software-engineer source: implementation and test evidence --- @@ -1036,3 +1036,119 @@ outcome in an `Exit-Build` block so a red suite still reports it. The trigger that fired on 2026-08-11 was never named; the engine's own cache drop above 1024 entries reproduces it, but a real gate peaked at 528 entries. Removing the precondition closes every trigger at once, so do not spend the day naming one. + +## Lab-host evidence and completion checks can fail quietly + +The lab host deletes per-session TEMP directories at logoff: +`DeleteTempDirsOnExit` and `PerSessionTempDir` are 1 under +`HKLM\SYSTEM\CurrentControlSet\Control\Terminal Server`, and `$env:TEMP` in a +remote-desktop session is `...\Temp\`. The private evidence +directories of 2026-09-07 were gone a month later. Keep acceptance evidence in +the profile's base TEMP directory, `$env:LOCALAPPDATA\Temp`. + +On 2026-10-07 five agent-side checks reported a wrong result, for four +reasons unrelated to the module: + +- InvokeBuild colors its summary line, so `^Build succeeded\.` misses the ANSI + prefix in a redirected log, and a run with warnings prints + `Build succeeded with warnings.` Judge a child build by its exit code, and + strip `\x1b\[[0-9;]*[A-Za-z]` before matching any text. +- `Sort-Object` orders strings that contain hyphens and underscores + differently in Windows PowerShell 5.1 and PowerShell 7, so a joined, sorted + file list fingerprinted an identical tree differently on guest and host. + Sort with `[StringComparer]::Ordinal` on both sides, or compare sets. +- `Get-ChildItem -Recurse -ErrorAction SilentlyContinue` silently skips a + directory whose DACL denies the administrator, so the tree reads as empty + and its size as zero. Old ACL probes left such directories in TEMP. List + with backup semantics, `robocopy /L /E /B`, before calling a + tree empty, and confirm a deletion with `Test-Path`, not with the absence of + an error message. +- The same deny ACE made `Remove-Item` fail non-terminating inside a script + that then printed its success line. Report removal only after verifying it. + +## Live tests that reach a guard need the real trigger, measured first + +The 2026-10-07 follow-up gave four unit-only paths live cases. Each trigger +had to be measured before it could be trusted: + +- Task Scheduler reapplies inherited ACEs. Removing the inherited Local System + ACEs from a subfolder or task changes nothing, so a DACL without Local + System must be protected, with the remaining ACEs made explicit. + `GenericAce.AceFlags` is byte-backed: `-bnot` yields a negative `Int32` that + Windows PowerShell 5.1 cannot cast back, so clear `INHERITED_ACE` with byte + arithmetic. +- A DACL-only `SE_LMSHARE` write clears the share description in the same step + that publishes the DACL; a native recorder polling every 0.25 ms never saw + one without the other. `NetShareGetInfo` supports levels 0, 1, 2, 501, 502, + 503, and 1005 only; level 1501 is for `NetShareSetInfo`. +- A race watcher must fire on the exact change under test, such as the + grant's SID appearing in the descriptor, not on any byte difference, and it + must pace its polling. One tight, high-priority loop ran beside a share + write that stalled for a minute and then failed; paced polling at 1 ms won + every race in 40 stress writes. Give a race case retries and put the + per-attempt data in `-Because`, so a failure explains itself. +- A guard behind two identical reads, such as the SMB setter's description + read after target resolution, cannot be reached by any live condition. + Shadow the command in the module's script scope, fail only the call whose + caller is the guard, assert the exact call sequence, and remove the shadow + only when `Get-Command` shows it is the module's own. +- `-ErrorAction Stop` wraps the original error in an + `ActionPreferenceStopException` whose `ErrorRecord`, not `InnerException`, + holds it, so `GetBaseException()` stops at the wrapper. + `Invoke-WindowsAccessControl` writes one `$null` when its script block + returns nothing, so filter `$null` before counting its output. +- Prove each new live case red against a build of the parent commit: a + detached `git worktree` plus a copied `output\RequiredModules` builds in + about 15 seconds. The `96d6671` build reproduced the accepted `f5731f1` + module hash, so builds are byte-reproducible across worktrees. + +## Remote prompts, coverage sessions, and error preferences in live suites + +The 2026-10-08 round gave a test to every unexecuted lab-only command. Its +prototypes and one stopped pass established these facts: + +- A remote prompt is answered by the host of the runspace that opened the + PSSession, not the one that runs or receives the command: `Receive-Job` in a + runspace without a user interface still asked the opening console. Open the + session from a `[powershell]::Create()` runspace and every prompt fails with + a `HostException` that names the operation and the target. +- Such a session starts with remote debugging off, so `Set-PSBreakpoint` in it + fails with "remote debugging is not supported by the current host", and no + coverage can be armed. `$session.Runspace.Debugger.SetDebugMode` with + `LocalScript, RemoteScript` enables it; a custom host whose `DebuggerEnabled` + is true does not. Focused runs without coverage cannot show this, so run one + focused Desktop pass with member coverage armed before a full pass. +- `Invoke-Command -Session` runs its script block at the session's top level, + so `$ErrorActionPreference = 'Stop'` inside it persists. + `Enter-WindowsAccessControlMemberCoverage` leaked it into member sessions, in + the coverage pass only, until `a572d3d` ran the body as + `& { param(...) ... } $ModulePath ...`: the child scope ends the preference + with the call, and `$global:` assignments still persist. A unit test + reproduces a session's top level with + `[powershell]::Create().AddScript($ScriptBlock.ToString())` plus + `AddArgument`, because `AddScript` runs a script in the runspace's global + scope; the test failed with `Stop` before the change. +- A `pwsh -File` script that sets `$ErrorActionPreference = 'Stop'` at its top + level sets the global value, which module functions inherit, so a module's + non-terminating `Write-Error` stops the caller. A case that asserts a + degraded, non-terminating report states `-ErrorAction Continue`. +- Windows PowerShell 5.1 misparses `@{ A = if (...) { ... }` when another key + follows, because it looks ahead for `else`; wrap the value in `$()`. A remote + script block is parsed by the 5.1 endpoint even from PowerShell 7. +- To call the original of a mocked module function from the mock body, capture + `& $module { ${function:Name} }` before `Mock` and invoke that script block; + a `-ParameterFilter` leaves every other call on the original. +- Red proof against a guard: copy the built `.psm1`, apply exact replacements + anchored on function-specific text, because guard messages repeat across + object families, require exactly one match each, and parse the result in + Windows PowerShell 5.1 before staging it. +- Sampler keeps only the first hyphen-separated part of a prerelease label, so + GitVersion's `0.3.0-record-post-rele0001` builds `0.3.0-record`. +- AutomatedLab's `Remove-LabVMSnapshot` pipes the named checkpoint to + `Remove-VMSnapshot -IncludeAllChildSnapshots`, so it also deletes every + checkpoint below the named one in the VM's checkpoint tree, which in a + linear chain is every newer one. To keep a newer + checkpoint, remove the older one with Hyper-V's `Remove-VMSnapshot`, without + that switch, one checkpoint and one merge at a time. A checkpoint that + survives on only some domain controllers is unsafe to apply, because it + rewinds their directory against the rest of the forest. diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index 2fb91dc..2ec50a8 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -1,27 +1,76 @@ --- status: current -last-verified: 2026-10-06 +last-verified: 2026-10-08 owner: software-engineer -source: repository evidence +source: repository and domain-lab evidence --- # Progress ## Current status -`v0.3.0-preview0001` shipped from `40e364a` on 2026-09-07 with the accepted -FIND-001 correction; `v0.2.0` remains the stable release. A 2026-10-06 triage -found unmerged and untracked work and split it into eight handoffs on local -`ai/post-release-fixes`. Handoffs 01 to 06 fixed the lab-runner Blocker, the -Task Scheduler, SMB, and AD review defects, ported and re-measured the -performance refactor, wrote the NTFSSecurity migration guide, moved the build -workflow to the Node 24 releases of its three pinned actions, restored the -0.2.0 changelog section, and closed FIND-002. Handoff 07 is Blocked for want -of the lab host, and handoff 08 waits for the user's approval of each remote -action. `activeContext.md` holds the evidence and the order. +`v0.3.0-preview0002` shipped from `2ebb3a5` on 2026-10-07 and passed live +domain-lab acceptance the same day; `v0.2.0` remains the stable release. Pull +request #5 carried the post-release work that a 2026-10-06 triage split into +eight handoffs: the lab-runner Blocker, the Task Scheduler, SMB, and AD review +defects, the performance refactor, the NTFSSecurity migration guide, the +Node 24 action releases, the restored 0.2.0 changelog section with a release +guard, and FIND-002. All eight handoffs are done. `activeContext.md` holds +the open work. ## Recent milestones +- 2026-10-08: `a572d3d` stops arming member coverage from leaving a member + session stopping on every error: `Invoke-Command -Session` runs a script + block at the session's top level, so the preference the harness set there + outlasted the call, and it now sets it in a child scope. A unit case + failed with `Stop` before the change, a live probe showed both behaviors + in each edition, and a built pass ran 109 of 109 in both editions with + line-identical lab coverage; the local gates passed with Core 1,938 and + Desktop 1,893 tests. The CopilotAtelier `automatedlab-deployment` Skill + gained the `Remove-LabVMSnapshot` warning on a local branch there. + +- 2026-10-08: Every command in the declared domain-lab-only files now has a + test. `4096d02` routes the RootDSE and effective-access reads through + `Send-WindowsADSearchRequest`, so six unit cases reach their four guards; + `938bdff` and `bad1302` add eight live cases for the SMB share guards, the + confirmation prompt, the enrichment catches, and a deletion inside the + effective-access read. Each was red against a build with its guard + removed. `bad1302` passed the DSC gate, four passes of 109 cases, and both + local gates; domain-lab-only coverage is 100 percent + (`docs/lab-acceptance-2026-10-08.md`). The first built pass of `938bdff` + stopped because a session opened without a user interface cannot arm + coverage breakpoints until its remote debugging is enabled. Removing the + older checkpoint afterwards with `Remove-LabVMSnapshot` also deleted the + newer one on ten VMs; the lab now holds one checkpoint, + `wac07-after-bad1302-9d9776ca`, taken after the acceptance. + +- 2026-10-07: A follow-up gave the four handoff 02 paths that only unit tests + covered live cases: Task Scheduler repair of a protected DACL without + Local System, and the SMB description that the native write clears, edits + concurrently, or cannot read before the write. Each case failed against + builds of `96d6671` and `5531824` and passed against the candidate; all + four passes ran 101 cases each, and both local gates passed again. The + SMB usage page and specification 0009 now name both windows in which a + concurrent description edit can be replaced. + +- 2026-10-07: Handoff 07 accepted `2ebb3a5` in the domain lab, recorded in + `docs/lab-acceptance-2026-10-07.md`. The release-equivalent build matches + the Gallery package byte-for-byte; four full passes of 97 cases, the DSC + engine gate, and both local gates with fresh lab coverage passed; the two + lab-runner refusals were proven against decoys. At the user's request the + two older lab checkpoints, `C:\WacRepo`, and `C:\WacLive` were removed + afterwards; checkpoint `wac07-pre-2ebb3a5-b4de5d73` remains. + +- 2026-10-07: Published `v0.3.0-preview0002`. The user answered every pending + question and ran each remote step from commands the agent handed over, + because the house rules block agent pushes; the agent verified each + result read-only. Pull request #5 passed CI in both editions without an + annotation and merged as `2ebb3a5`; the GitHub release, the Gallery entry, + and the wiki Home page carry the version GitVersion had predicted. + Dependabot #1-#3 are closed as superseded, issue #4 has its reply, and + the stale branches are deleted. + - 2026-10-06: With the user's overnight delegation, handoff 07 was recorded as Blocked (no lab VMs on this machine) and handoff 08 was prepared up to its first remote action: a pull request body on the repository template, @@ -79,93 +128,16 @@ action. `activeContext.md` holds the evidence and the order. that, only its two WinRM-bound DSC-engine tests failed. One independent review approved; its Minor and three Nits were addressed. -- 2026-10-06: Handoff 02 fixed three review defects test-first on - `ai/post-release-fixes`, one commit each. A Task Scheduler write may - repair a DACL without a Local System ACE; a missing or null current DACL - is still refused. An SMB DACL write reads the description just before - the native call, keeps concurrent edits, and warns instead of failing - after a committed write. `Get-ADObjectCallerEffectiveAccess` no longer - requests `nTSecurityDescriptor`; an absent descriptor elsewhere raises a - typed read-control error. The Core gate passes 1,905 tests with two - environmental skips at 83.2 percent asserted coverage. The Desktop gate - fails only the two WinRM-bound DSC-engine tests; its scoped coverage, - computed from its document, is 81.03 percent. One review approved with - minor findings, most fixed. No live lab run; 07 owns two new AD cases. - -- 2026-10-06: Handoff 01 made the domain-lab acceptance runner refuse to - delete what it did not create, test-first on `ai/post-release-fixes`. It - marks the payload root, `package`, and installed module version directory - it creates; replaces only absent or marked directories with no junction or - symbolic link inside; validates the payload root on the management - controller in every mode; and stops when a remote step does not confirm - its directory, because AutomatedLab does not stop on a remote throw. 79 - lab-runner tests pass in Core 7.6.6 and Desktop 5.1; the full gate passes - 1,873 tests with two environmental skips at 83.0 percent asserted coverage. - One independent review approved with Minor findings; seven fixed, three - recorded. No live lab run; handoff 07 owns it. - -- 2026-10-06: Wrote eight sequential handoff prompts for the triaged work, - outside the repository, with a ledger and per-prompt reports. They commit - to local `ai/post-release-fixes`; only the final prompt touches the - remote, with approval per action. - -- 2026-10-06: Triaged open work. Verified from git, the public GitHub API, - and the PowerShell Gallery that `v0.3.0-preview0001` shipped on - 2026-09-07, so the pending-authorization note was stale. Found the - 2026-09-10 performance refactor and repository-wide review only on a - deleted, never-pushed local branch; restored `ai/performance-refactor` at - `5f06c03` before reflog expiry. Rechecked its findings against `main`: - the lab-runner deletion, Task Scheduler SYSTEM refusal, SMB description - overwrite, and AD descriptor overfetch remain; its CNG Major is FIND-001. - Dependabot failures are inherited from their parent `4806726`, fixed on - `main` by `0502254`. No code change and no remote mutation. - -- 2026-09-07: Completed fresh reacceptance of `f5731f1` without replacing the - lab. All four built/installed passes have 95 passed tests, zero failures or - skips, and ready cleanup. Five DSC-engine tests and both fresh local gates - pass; Core has 91.15 percent asserted coverage and Desktop 90.41 percent. - Verified original installation bytes/ACLs, fixture restoration, and all 38 - retained guest evidence files before staging removal. Thirteen VMs and - checkpoints remain. Final completion is 18:52:19 UTC; no git remote mutation. - -- 2026-09-07: Resolved FIND-001 test-first. A uniquely named persisted CNG key - reproduced the third-read exception after its requested DACL was already - stored, then passed with exactly two helper reads after the one-line source - correction. Core and Desktop each pass all 52 affected CNG tests; a focused - Desktop live mutation restores the fixture descriptor byte-for-byte and - removes its binding and staging. Full local runs pass 1,806 and 1,761 tests - at 83.00 and 80.81 percent asserted coverage without prior lab evidence. - PSScriptAnalyzer is clean on both changed files, and independent review - approved with no finding. The 22-task pack is clean and package SHA-256 is - `D93B2644B31A38B37F9FAC08DBD1D28C85AF8AD452D81E1428E0A3054530588C`. - Prior accepted artifacts remain preserved. - -- 2026-09-07: Completed one independent review of `3321350..358651e` plus the - CNG persistence path. Verified one Minor post-write read defect and two - missing-newline locations under one Nit; no Blocker or Major findings. - Reconciled raw-report tally and topology errors against the retained - evidence. No runtime or test code changed and no live tests were repeated. - -- 2026-09-07: Checkpointed all thirteen existing lab VMs and verified signed, - sealed Kerberos LDAP, WSMan, test dependencies, and the renewable template. - Rebuilt and byte-verified the candidate; closed the five-test Desktop DSC - gate on the reserved member. Fixed the stale cleanup-list entry exposed by - live acceptance, retained the failed evidence, and verified exact-identity - recovery. All four repaired passes finish with 95 tests each and clean - fixtures. Original installation bytes and ACLs are restored; final lab - services, LDAP, and checkpoint checks pass. Local Core passes 1,805 tests - and 91.16 percent asserted coverage; Desktop passes 1,760 tests and 90.41 - percent. Both imported the successful lab coverage byte-for-byte, with the - 80 percent threshold unchanged. Validation completed at 12:08 UTC and - monitoring stopped. The dated lab acceptance report retains candidate - hashes and the failure, recovery, and successful-run evidence. - -- 2026-09-03 to 2026-09-07: OI-31, the 105-command audit, the audit-gap +- 2026-09-03 to 2026-10-06: OI-31, the 105-command audit, the audit-gap closure, the `6f7ba15` close-out, the `34026468199` wiki-publication hang (DscResource.DocGenerator#111), the `34021812398` completer fix, the `ai/access-rights-completion` review, the script-inventory audit, the - wiki-publisher investigation that run `34055979655` closed, and the - `ai/test-gap-audit` handoff push are in + wiki-publisher investigation that run `34055979655` closed, the + `ai/test-gap-audit` handoff push, the 2026-09-07 lab checkpoints, review, + FIND-001 resolution, `f5731f1` reacceptance, the 2026-10-06 triage, the + eight handoff prompts, and handoffs 01 and 02 are in + `git show a572d3d:.memory-bank/progress.md`, + `git show 2ebb3a5:.memory-bank/progress.md`, `git show 023afe9:.memory-bank/progress.md`, `git show 4461b90:.memory-bank/progress.md`, `git show 8b67058:.memory-bank/progress.md`, and, for the oldest, diff --git a/.memory-bank/systemPatterns.md b/.memory-bank/systemPatterns.md index 4cbbd0c..88a00c5 100644 --- a/.memory-bank/systemPatterns.md +++ b/.memory-bank/systemPatterns.md @@ -1,6 +1,6 @@ --- status: current -last-verified: 2026-10-06 +last-verified: 2026-10-08 owner: software-engineer source: repository evidence --- @@ -35,6 +35,13 @@ letting structural identity treat it as current evidence. Fresh acceptance means candidate-bound payloads and evidence, not a rebuilt lab. Reuse healthy marked fixtures under a checkpoint; verify installation bytes/ACL restoration and evidence hashes before removing run-owned staging. +Directory reads reach LDAP only through `Send-WindowsADSearchRequest`, the seam +unit tests replace. A live guard that no lab identity can reach is injected +with a module-scope mock after the real read, and a prompt test opens its own +session from a runspace without a user interface, with remote debugging on so +coverage can be armed there. A script block sent to a persistent session runs +at its top level, so harness code that sets a preference there sets it in a +child scope. Process wrappers must drain redirected output while a child runs; waiting for exit before reading can deadlock on a large Git commit summary. Treat a publish workflow as non-atomic and inspect every external destination before rerunning diff --git a/CHANGELOG.md b/CHANGELOG.md index a3c372a..8d9af84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 a path longer than 260 characters needs PowerShell 7. The documentation index had pointed NTFSSecurity users at the `NTFSPermission` rename map, which never mentions NTFSSecurity +- Add live domain-lab cases for two safeguards that only unit tests covered. + A Task Scheduler folder and task whose protected DACL has no Local System + ACE accept an unrelated ACE, still refuse a Local System deny, and can have + Local System restored. Around an SMB share DACL write, the description + that the native write clears is restored and reported, a concurrent edit is + kept with a warning, and a description that cannot be read before the + write stops the command before anything is written +- Add live domain-lab cases for the SMB share and Active Directory paths that + no test executed. An SDDL without a DACL is refused, `-PassThru` returns + the stored descriptor, a rule copy that was not read from the share and a + rule bound to another server are refused before anything is removed, and a + share DACL write asks for confirmation and writes nothing when the caller + cannot answer. A failed inheritance-source or schema-name lookup leaves the + rules in the report with a non-terminating error, and an object deleted + after target resolution is reported as not found ### Changed @@ -47,9 +62,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 entry, so the deny-removal warning raised before a directory write keeps its exact-match guarantee. Private-key DACL comparison keeps an ordered desired-state check separate from unordered post-write verification +- Read the RootDSE and the caller's effective access through the same + directory search function as every other directory read, so unit tests + cover their response checks without a domain controller. An object deleted + between target resolution and the effective-access read of + `Get-ADObjectCallerEffectiveAccess` now fails with an + `ItemNotFoundException` naming it, like the family's other reads, instead + of a `MethodInvocationException` from the LDAP call + ([specification](specs/0018-active-directory-caller-effective-access.md#output-contract)) ### Fixed +- Stop the domain-lab coverage pass from changing how member-server suites + handle errors: arming coverage in a member session set + `$ErrorActionPreference` to `Stop` for the rest of that session, so the + SMB share, certificate private-key, Task Scheduler, and foreign-principal + suites stopped at a non-terminating error only in the pass that collects + coverage + ([verification and traceability](specs/0005-verification-and-traceability.md#code-coverage-measurement)) +- Describe both windows in which an SMB share DACL write can still lose a + concurrent description edit. The usage page and the specification named + only an edit that the native write clears; an edit that lands between the + write's read-back and its restoration is overwritten by that restoration + too, as the new live case observed + ([specification](specs/0009-smb-share-and-active-directory-dacl-management.md#smb-share-contract)) - End a release that published without opening its changelog pull request. Sampler's `Create_ChangeLog_GitHub_PR` reports a refused pull request only as a line in the build log, so the v0.2.0 run pushed diff --git a/build.yaml b/build.yaml index e223e6c..ac81b5a 100644 --- a/build.yaml +++ b/build.yaml @@ -152,8 +152,6 @@ CodeCoverage: CodeCoverageMergedOutputFile: JaCoCo_merged.xml DomainLabCodeCoverageFile: tests/Lab/coverage/JaCoCo_coverage_DomainLab.xml DomainLabOnlySourcePath: - - Private/Get-WindowsADEffectiveAccessRecord.ps1 - - Private/Get-WindowsADRootDse.ps1 - Private/Get-WindowsADRuleEnrichment.ps1 - Private/New-WindowsADConnection.ps1 - Private/Send-WindowsADSearchRequest.ps1 diff --git a/docs/README.md b/docs/README.md index 4cc7592..5c28baa 100644 --- a/docs/README.md +++ b/docs/README.md @@ -25,6 +25,8 @@ what the module does, and comment-based help defines every parameter. | [research.md](research.md) | Source review, platform API semantics, and the comparison that informed the specifications | Contributors and reviewers | | [domain-lab-inventory.md](domain-lab-inventory.md) | Machines, forests, and roles of the disposable acceptance lab | Contributors running the lab suites | | [lab-reacceptance-2026-09-07.md](lab-reacceptance-2026-09-07.md) | Corrected-candidate lab, package, DSC, local coverage, and cleanup evidence | Maintainers and reviewers | +| [lab-acceptance-2026-10-07.md](lab-acceptance-2026-10-07.md) | Post-release candidate (`v0.3.0-preview0002`) lab, package, DSC, runner ownership, local coverage, and cleanup evidence, plus the live cases for the paths only unit tests covered | Maintainers and reviewers | +| [lab-acceptance-2026-10-08.md](lab-acceptance-2026-10-08.md) | Branch candidate `bad1302` lab, package, DSC, local coverage, and cleanup evidence, with a test for every command that no lab or unit case executed, and the harness correction `a572d3d` | Maintainers and reviewers | | [performance-refactor-2026-09-10.md](performance-refactor-2026-09-10.md) | Performance changes, repeatable measurements, validation evidence, and remaining gates | Contributors and reviewers | ## The usage guide diff --git a/docs/lab-acceptance-2026-10-07.md b/docs/lab-acceptance-2026-10-07.md new file mode 100644 index 0000000..117678c --- /dev/null +++ b/docs/lab-acceptance-2026-10-07.md @@ -0,0 +1,265 @@ +# Lab acceptance: 2026-10-07 + +Live domain-lab acceptance of the post-release candidate, `main` at `2ebb3a5`, +which the release workflow published as `v0.3.0-preview0002`. The run followed +the [acceptance checklist](../tests/Lab/acceptance-checklist.md). All four +full lab passes, the isolated Desktop DSC-engine gate, and both local test and +coverage gates passed. The lab-runner ownership refusals were also proven live. +A follow-up the same day gave the changed paths that only unit tests covered +their own live cases, proved each case red against the code before its change, +and repeated all four passes and both local gates with them. This record is +not release approval. + +## Candidate and environment + +- Commit: `2ebb3a54cb7b8411aeee5582b0daee5cb0ed112b` on `main`, the merge of + pull request #5, with a clean worktree. Source, tests, and build inputs did + not change during acceptance. +- Version: GitVersion 5.12.0, the official standalone Windows release run + read-only, computed `0.3.0-preview0002`. The build received it through + `$env:ModuleVersion`, as the [contributing guide](../CONTRIBUTING.md) + describes, so the package is release-equivalent rather than the `0.0.1` + development fallback. +- One detached `pack` built the module and the package: 22 tasks, zero errors, + zero warnings. All 25 module files of the build, of the package, and of the + `0.3.0-preview0002` package on the PowerShell Gallery are byte-identical, so + these passes tested the bytes that users install. +- Lab: the existing isolated `WindowsAccessControlLab`, all thirteen VMs + reused. No VM removal, redeployment, checkpoint restoration, or topology + change was needed. AutomatedLab 5.61.704; lab Windows PowerShell + 5.1.26100.32684 and PowerShell 7.6.3; Pester 5.7.1 in both module roots; + RSAT on every machine. +- Checkpoint `wac07-pre-2ebb3a5-b4de5d73` covers all thirteen VMs. +- Readiness was proven at the protocol level, not only by VM state, before + the checkpoint, after it, and after each lab pass: WinRM on all thirteen + machines; signed and sealed Kerberos LDAP rootDSE queries against both + writable fixture controllers, each reporting `isSynchronized`; a + ticket-granting ticket and `ldap` service tickets; ten marked baseline + objects on both controllers with no leftover target; the member share, Task + Scheduler folder, and lab key; and the published renewable CNG template. + +## Payload and module ownership + +The unmarked `C:\WacRepo` payload on `F1ADC1` predated the runner's ownership +marker, so the runner would have refused it. It was the payload of the +2026-09-07 `5991673` acceptance: 541 files, no junction or symbolic link, +built module SHA-256 `A70F3C1E…F019D`. An older unmarked `C:\WacLive` staging +directory from 2026-08-11 was also present. `F1ADC1` holds unmarked `0.0.1` +and `0.2.0` installations, and `F1DC1` an unmarked `0.0.1` installation. During +acceptance none of them was removed, marked, or changed: every pass used a +fresh, absent payload root, which the runner created and marked, and the +release-equivalent package version `0.3.0` did not collide with an existing +installation. + +The two refusals from the lab-runner ownership change were proven against +run-owned decoys, so no pre-existing directory was put at risk: + +- An unmarked decoy payload root was refused at the reset step with the + documented message and resolution. +- An unmarked decoy `0.3.0` installation was refused at the install step, + after the runner had created and marked a fresh payload root and its + package staging directory. + +Both decoys were byte-identical afterwards and carried no marker. The decoys +and the demonstration's own payload root were then removed. + +## Lab results + +| Gate | Passed | Failed | Skipped | Cleanup | Completed UTC | +| --- | ---: | ---: | ---: | --- | --- | +| Desktop DSC engine | 5 | 0 | 0 | Temporary installation removed | 14:26:09 | +| Built module, Desktop with coverage | 97 | 0 | 0 | Eight ready entries | 14:40:57 | +| Built module, Core | 97 | 0 | 0 | Eight ready entries | 14:49:06 | +| Installed package, Desktop | 97 | 0 | 0 | Eight ready entries | 15:03:49 | +| Installed package, Core | 97 | 0 | 0 | Eight ready entries | 15:11:58 | + +These are four executions of the same 97 live cases, not 388 unique tests: +the 95 cases of 2026-09-07 plus the two Active Directory cases added after +them. Every full pass reported `Passed`, eight suites, zero skipped tests, +explicit native exit zero, and both domain and member readiness in all eight +cleanup entries. Per suite: domain lab 6, certificate private key 8, Task +Scheduler 8, SMB share 7, Active Directory object 16, schema default and +object type 35, foreign principal 5, and replication 12. + +The DSC gate ran in an isolated Windows PowerShell 5.1 process on the reserved +member `F1AFile2`, staged with the exact built module. It includes both actual +`Invoke-DscResource` cases. It ran while the built pass used the shared +fixtures, which it does not touch, and it finished before the installed-package +pass. The installed-package pass installed the package at the `0.3.0` version +directory of the machine module path on `F1ADC1`; its 25 module files matched +the candidate and its marker was present. + +## Changed behavior with live evidence + +- Lab-runner ownership: both refusals above, plus marked creation of every + payload root, package staging directory, and installed version directory. +- Active Directory: `Should evaluate a caller who cannot read the object + security descriptor` and `Should report a missing directory object as not + found` passed in all four passes, as did the earlier escaped-name and + reused-GUID regressions. +- SMB share description: the earlier SMB case asserts that the description is + byte-identical after both the add and the exact remove. The follow-up below + proves the three description paths one by one. +- Task Scheduler: the follow-up below proves the repair of a DACL that has no + Local System ACE. +- Shared batching: the Active Directory, SMB share, and Task Scheduler + commands that the live suites call dispatch through the refactored batching + layer, so all four passes ran it. The certificate private-key commands do + not use that layer. + +Every changed behavior now has live evidence. The two recorded lab-runner +follow-ups, member-server suites that delete `ModuleUnderTest` without an +ownership check and the unmarked root that `-SkipPayloadDeployment +-ModuleSource Installed` creates, are known runner defects rather than +untested behavior, and they stay scheduled after the release. + +## Follow-up live cases + +Four cases were added after the first passes. Each reaches its path with real +objects, except the last, whose failure is injected: + +| Case | How it reaches the path | +| --- | --- | +| `Should accept and repair a protected folder and task DACL that has no Local System ACE` | A disposable folder and task in the marked folder get protected DACLs without Local System, because an inherited Local System ACE would otherwise return. An unrelated ACE is accepted, a Local System deny is refused without a write, and Local System is restored to both. | +| `Should restore a description that the native DACL write cleared` | A raw DACL-only `SE_LMSHARE` write first shows that the native write clears the description; the delegated add then restores it and reports the restoration. | +| `Should keep a description edited during the DACL write and warn with the earlier value` | A native watcher edits the description as soon as the add's grant appears, before the setter reads the description back. A lost race is retried, up to five writes. | +| `Should stop before writing when the description cannot be read before the write` | Target resolution reads the share twice just before the setter does, so a module-scope shadow of `Get-SmbShare` fails only the setter's own read. The DACL stays unchanged. | + +Each case failed for its expected reason against modules built from the +commits before the changes, and passed against the candidate: + +| Module | Task Scheduler | Restoration | Concurrent edit | Unreadable description | +| --- | --- | --- | --- | --- | +| `96d6671`, before both changes | Refused, because the DACL had no SYSTEM ACE | Not reported | Stale value restored over the edit in 5 of 5 writes | DACL written first | +| `5531824`, Task Scheduler change only | Passed | Not reported | Stale value restored over the edit in 5 of 5 writes | DACL written first | +| Candidate | Passed | Passed | Passed | Passed | + +All four full passes were then repeated with the new cases: + +| Gate | Passed | Failed | Skipped | Cleanup | Completed UTC | +| --- | ---: | ---: | ---: | --- | --- | +| Built module, Desktop with coverage | 101 | 0 | 0 | Eight ready entries | 18:12:56 | +| Built module, Core | 101 | 0 | 0 | Eight ready entries | 18:20:53 | +| Installed package, Desktop | 101 | 0 | 0 | Eight ready entries | 18:32:17 | +| Installed package, Core | 101 | 0 | 0 | Eight ready entries | 18:40:22 | + +The Task Scheduler suite now runs 9 cases and the SMB share suite 10; every +other suite is unchanged. Every repeated pass reported `Passed`, eight suites, +zero skipped tests, explicit native exit zero, and both readiness flags in all +eight cleanup entries. + +Three observations came out of the follow-up: + +- The native write publishes the DACL and clears the description in one step. + A concurrent edit is also lost when it lands between the setter's read-back + and its restoration, a window that the specification and the usage page did + not name; both now do. Behavior is unchanged. +- `Invoke-WindowsAccessControl` writes one `$null` to the pipeline when its + script block returns nothing, so `@()` around it counts one item. The new + case filters it out; the command is unchanged. +- Once, against the `5531824` module, a share DACL write stalled for about a + minute while the watcher polled without pause, and then failed with + `ERROR_INVALID_PARAMETER`. It did not recur in 40 stress writes against the + candidate, and the watcher now polls once per millisecond. + +## Artifact identity + +| Artifact | SHA-256 | +| --- | --- | +| Tested root module | `8DCEC9CBFCF71359D253E536A7E30AF90DCE1B7D154E589DF571F58AA6B3F371` | +| Tested module manifest | `581F7E70ABEA58B3FB0B8D0A2C45FB79267207493CF9B215FAB285A3295F3785` | +| Tested package | `148C5558F205B11502FC0545C26BB4D44E64C06693FC6044AE05EF2D33B2860F` | +| PowerShell Gallery package | `3E122CC9BEA88C6C5D4B94492B1E91BB31F5BF445F35B61976B8472D0488CC03` | +| Fresh lab coverage | `B3EE43139B05614012F110A623EBE7FF48ECAB8F8441A349D0BA8726AE436FD6` | + +The two package files differ only in their packaging metadata; their module +files are identical. The lab coverage from the follow-up measures 8,506 +commands, of which the domain-lab profile exercised 3,874 (45.54 +percent); the harness reports 1,707 of them as reached only in the +member-server runspace. It is not the local or merged coverage percentage. It +replaced the first passes' document, `BC34FE2B…BA23`, which had 3,868 +exercised commands. The coverage document that preceded both was preserved +before the first run. Each new document reached the configured coverage path +from its instrumented Desktop pass and matches the guest original +byte-for-byte. + +## Local coverage gates + +| Edition | Passed | Failed | Skipped | Asserted coverage | Whole-module coverage | Completed UTC | +| --- | ---: | ---: | ---: | ---: | ---: | --- | +| Core 7.6.6 | 1,931 | 0 | 2 | 91.22% | 91.22% | 18:30:37 | +| Desktop 5.1.26100.33438 | 1,886 | 0 | 2 | 90.49% | 90.50% | 18:53:09 | + +These are the final gates, run after the follow-up's lab coverage was +collected. The first gates, against the first lab document, passed with the +same test counts: Core at 91.22 percent and Desktop at 90.49 percent +asserted coverage. Both editions imported the current lab document each time +and reported `Domain-lab evidence merged: yes`; the 80 percent threshold and +the asserted source scope were unchanged. Each ten-task workflow completed +with zero errors and two warnings from deliberately mocked evidence-copy +failures in the runner's unit tests, not from actual acceptance collection. +Each edition skipped the mounted-volume case and the audit-rule read that +needs an unavailable privilege. The local gates ran on the host after the lab +coverage was collected, while the remaining lab work continued in the VMs; +they touch neither the lab nor its fixtures. + +## Independent cleanup checks + +Before anything was removed, all evidence files on `F1ADC1` and `F1AFile2` +matched their host copies, both controllers answered signed and sealed +Kerberos LDAP, ten marked baseline objects remained with no leftover target, +the member fixtures were ready, no HTTP.sys binding or suite backup file +remained, and every directory, KDC, DNS, ADWS, WinRM, and CA service ran. +Only directories that still carried the runner's marker, this run's console +logs, and the DSC staging were then removed. + +A byte and ACL inventory taken before the run and again after cleanup shows no +difference in any path, length, hash, security descriptor, reparse flag, or +file time across the installations on `F1ADC1` and `F1DC1`, the module paths +of both file servers, `C:\WacRepo`, and `C:\WacLive`. All thirteen VMs were +running and all three checkpoint sets remained. + +After these checks, at the maintainer's request, the two 2026-09-07 +checkpoints were removed, as were `C:\WacRepo` and `C:\WacLive`, each first +re-verified file by file against the pre-run inventory. All thirteen VMs kept +running with checkpoint `wac07-pre-2ebb3a5-b4de5d73`, and the unmarked +installations on `F1ADC1` and `F1DC1` were kept. Without `C:\WacRepo`, the +runner's default payload root works again. + +The follow-up took checkpoint `wac07-pre-livegaps-83cd16fa` on all thirteen +VMs first, and did its probing on its own disposable shares, task folders, +and local accounts, all removed afterwards. After the repeated passes, the +same health checks passed again; the two follow-up payload roots, the marked +`0.3.0` installation, and the four verified console logs were removed; and a +new inventory of every module path showed no difference from the state before +the follow-up. At the maintainer's request, `wac07-pre-2ebb3a5-b4de5d73` was +then removed too, so `wac07-pre-livegaps-83cd16fa` is the lab's only +checkpoint. + +## Retention and remaining gates + +Private evidence is retained under administrator `%TEMP%` in +`wac07-accept-2ebb3a5-b4de5d737dca43c6aabcf2b7100bd5b7` and, for the +follow-up, `wac07-livegaps-83cd16fa7a4645cf95066e8e92330c0f`, in the +profile's base TEMP directory, because this host deletes per-session TEMP +directories at logoff. It holds the pinned candidate files, the Gallery +package, the GitVersion output, inspections, inventories, checkpoint +identities, native exit markers, raw console logs, per-pass reports, the +coverage documents, the red and green focused results, and the local test +results. Raw evidence is not sanitized for publication. The +private evidence directories named in the 2026-09-07 records are no longer on +this host. Two agent-side completion markers reported failure although the +build and the Core gate succeeded, because their patterns missed a colored +summary line and a `succeeded with warnings` summary; the authoritative +summaries and exit codes are recorded. + +All requested acceptance gates for this candidate are closed. A stable release +still requires explicit authorization to merge, tag, push, or publish. Broader +native fault injection, interrupted rollback, cancellation, soak tests, +unusual-ACE persistence, and untested topology profiles remain documented +limits. + +The [2026-10-08 acceptance](lab-acceptance-2026-10-08.md) tested a later +candidate on the same branch, which routes two directory reads through the +shared search function and gives a test to every command that no lab or unit +case executed. diff --git a/docs/lab-acceptance-2026-10-08.md b/docs/lab-acceptance-2026-10-08.md new file mode 100644 index 0000000..6878a95 --- /dev/null +++ b/docs/lab-acceptance-2026-10-08.md @@ -0,0 +1,359 @@ +# Lab acceptance: 2026-10-08 + +Live domain-lab acceptance of the candidate `bad1302` on the local branch +`ai/record-post-release-publication`. It routes two Active Directory reads +through the directory search function that every other directory read already +uses, and it gives a test to each of the 17 commands in the declared +domain-lab-only source files that no test had executed. The run followed the +[acceptance checklist](../tests/Lab/acceptance-checklist.md). The isolated +Desktop DSC-engine gate, all four full lab passes, and both local test and +coverage gates passed, and every one of the 17 commands now executes: the 13 +that only the lab reaches in the fresh lab coverage, and the 4 that the source +change made unit-testable in the merged local coverage. A first built pass of +the previous commit, `938bdff`, stopped on the new confirmation case; `bad1302` +corrected the case, and every pass ran again. This record is not release +approval, and the candidate is not pushed. + +A harness correction followed the same day. `a572d3d` stops arming member +coverage from changing the error preference of the member session; its built +pass and both local gates passed, as recorded under +[Harness correction](#harness-correction). It is not pushed either. + +## Candidate and environment + +- Commit: `bad130221f65b8064d8eba6d03c8325b4858ad18` with a clean worktree. + It adds three commits to the records of the + [2026-10-07 acceptance](lab-acceptance-2026-10-07.md): `4096d02` changes the + module and its unit tests, `938bdff` adds the live cases, and `bad1302` + corrects the confirmation case after the first built pass of `938bdff` + stopped on it. +- Version: GitVersion 5.12.0, the same official standalone Windows release as + on 2026-10-07 with a matching size and SHA-256, run read-only, computed + `0.3.0-record-post-release-publication.1`, NuGet `0.3.0-record-post-rele0001`. + The build received that value through `$env:ModuleVersion`, and Sampler + built `0.3.0-record`, because it keeps only the first hyphen-separated part + of a prerelease label. It is not a published version. +- A detached `pack` of `bad1302` built the module and the package: 22 tasks, + zero errors, zero warnings. Its root module is byte-identical to the build + of `938bdff` and to the development build that ran the focused green and + red runs below; the manifests differ only in their version or in the build + date of their release notes. +- Lab: the same isolated `WindowsAccessControlLab`, all thirteen VMs reused, + with checkpoint `wac07-pre-938bdff-098b3392` taken before the first pass. + After that pass stopped, the fixtures were verified intact and no checkpoint + was restored, so the same checkpoint precedes the passes of `bad1302`. + Readiness was proven at the protocol level before and after the checkpoint, + before the passes of `bad1302`, and after the last pass: WinRM on all + thirteen machines; signed and sealed Kerberos LDAP against both writable + fixture controllers, each reporting `isSynchronized`; a ticket-granting + ticket and `ldap` service tickets; ten marked baseline objects on both + controllers with no leftover target; the member share with its marker, the + Task Scheduler folder, and the lab key; and the published renewable CNG + template. + +## The commands no test executed + +The 13 source files that `build.yaml` declares domain-lab-only held 17 +commands that neither the unit profile nor the lab profile executed. Each now +has a test: + +| Source file | Unexecuted commands | Now covered by | +| --- | --- | --- | +| `Set-SmbShareSecurityDescriptor` | The refusal of an SDDL without a DACL; the `-PassThru` read | Live: `Should refuse an SDDL without a DACL before writing`, `Should return the stored descriptor with PassThru` | +| `Remove-SmbShareAccessRule` | The refusal of an object that is not a rule from the share; the refusal of a rule whose canonical target differs | Live: `Should refuse a rule copy that was not read from the share before removing anything`, `Should refuse a rule whose canonical target names another server` | +| `Invoke-WindowsSmbShareCommandBatch` | The throttle of one that a confirmation requires | Live: `Should ask before writing and write nothing when the caller cannot answer` | +| `Get-WindowsADRuleEnrichment` | Both catch blocks, eight commands | Live: `Should report rules without an inheritance source when the source lookup fails`, `Should report object GUIDs without names when the schema-name lookup fails` | +| `Get-WindowsADRootDse` | The refusal of a response without one entry; the report of an absent root domain naming context | Unit, after the source change below | +| `Get-WindowsADEffectiveAccessRecord` | The refusal of a response without one entry; the zero for an absent `sDRightsEffective` | Unit, after the source change below; live: `Should report an object deleted after target resolution as not found` | + +The SMB cases reach their paths with real objects: a rule restored from +serialized data, a rule reshaped by `Select-Object`, a rule whose canonical +target was edited, and a session that cannot answer a prompt. Three directory +conditions cannot be produced on demand, so they are injected. The two +enrichment lookups fail only on conditions no lab identity can create, because +the ancestor walk absorbs its own read failures and an identity that cannot +read the schema or configuration partition cannot read the rules either; a +module-scope Pester mock fails each lookup after the real descriptor read. +Nothing runs between target resolution and the effective-access read, so the +deletion case deletes a disposable organizational unit inside that read and +then sends the real request to the controller. + +## Source change + +`Get-WindowsADRootDse` and `Get-WindowsADEffectiveAccessRecord` sent their +LDAP requests straight to the connection, so no unit test could reach their +response checks, and both files were declared domain-lab-only. Both now call +`Send-WindowsADSearchRequest`, like every other directory read, and both +files left the domain-lab-only list, so the local gate asserts them. Six unit +cases replace the request at that function: a RootDSE response with zero or +two entries is refused, a RootDSE without `rootDomainNamingContext` reports +none, an effective-access response with zero or two entries is refused, and +an absent `sDRightsEffective` reports zero. They failed against the previous +code with "The LDAP server is unavailable", because that code bypassed the +function, and passed in both editions after the change. + +One behavior changes. An object deleted between target resolution and the +effective-access read now fails with an `ItemNotFoundException` naming it. +A probe of the previous call on `F1ADC1` showed what it raised instead: a +`MethodInvocationException` wrapping a `DirectoryOperationException` with +result code `NoSuchObject`. The changelog and the +[specification](../specs/0018-active-directory-caller-effective-access.md#output-contract) +record it. + +## Red evidence for the live cases + +A copy of the development build with one change per guard ran the eight live +cases in both editions. Every case failed for the reason its guard names, and +the unchanged build passed all eight in both editions: + +| Change to the module | Case | Result against the changed module | +| --- | --- | --- | +| The refusal of an SDDL without a DACL removed | SDDL without a DACL | The native helper refused it with its own message, which names `SetNamedSecurityDescriptor` | +| The `-PassThru` read removed | PassThru | No output | +| The non-rule refusal removed | Rule copy | The deserialized copy failed parameter conversion, and the reshaped copy removed the rule | +| The canonical-target refusal removed | Another server | The edited rule removed the grant | +| `ConfirmImpact` of `Set-SmbShareSecurityDescriptor` lowered to `Medium` | Confirmation | The write happened without a prompt | +| The inheritance-source catch made to rethrow | Inheritance source | The injected failure replaced the degraded report | +| The schema-name catch made to rethrow | Schema names | The injected failure replaced the degraded report | +| The effective-access read restored to the previous direct call | Deletion | No error, because the read never reached the search function | + +The throttle of one that `Invoke-WindowsSmbShareCommandBatch` applies before a +confirmation has no observable effect for a single share, so the confirmation +case covers that line without testing it; the change that case is red against +is the one it exists for. These runs used the cases as `938bdff` wrote them; +`bad1302` changed only how the confirmation case opens its session and which +error preference it states, not what it asserts. + +## Lab results + +| Gate | Passed | Failed | Skipped | Cleanup | Completed UTC | +| --- | ---: | ---: | ---: | --- | --- | +| Desktop DSC engine | 5 | 0 | 0 | Temporary installation removed | 00:08:36 | +| Built module, Desktop with coverage | 109 | 0 | 0 | Eight ready entries | 00:39:05 | +| Built module, Core | 109 | 0 | 0 | Eight ready entries | 00:48:10 | +| Installed package, Desktop | 109 | 0 | 0 | Eight ready entries | 01:02:04 | +| Installed package, Core | 109 | 0 | 0 | Eight ready entries | 01:10:54 | + +Every row is a run of `bad1302`. The four passes are four executions of the +same 109 live cases: the 101 of the 2026-10-07 follow-up plus the eight new +ones. Per suite: domain lab 6, certificate private key 8, Task Scheduler 9, +SMB share 15, Active Directory object 19, schema default and object type 35, +foreign principal 5, and replication 12. Every pass reported `Passed`, eight +suites, zero skipped tests, explicit native exit zero, and both readiness +flags in all eight cleanup entries. The DSC gate ran in an isolated Windows +PowerShell 5.1 process on the reserved member `F1AFile2`, staged with the +exact built module, and includes both actual `Invoke-DscResource` cases. The +installed-package pass installed the package at the `0.3.0` version directory +of the machine module path on `F1ADC1`, where its 25 module files matched the +candidate build and the runner's marker was present. + +The first built pass, of `938bdff` on 2026-10-07, stopped in its Desktop +coverage edition. The confirmation case could not arm member coverage in its +own session, so the SMB share suite reported 14 of 15, and the runner ended +that edition after four of the eight suites, with 37 of 38 cases passing. Its +Core edition, which arms no coverage, passed all 109 cases, and a DSC-engine +gate of `938bdff` had passed 5 of 5 before it. The evidence of that pass is +retained. Before the rerun, the fixtures, the share's marker, the directory +baseline, and the service state were verified intact. + +## Artifact identity + +| Artifact | SHA-256 | +| --- | --- | +| Tested root module | `A9671FD086456978E4CE0CB5E01270B201C56FE7C1305BF7B77B9B9FB0E921CA` | +| Tested module manifest | `3D2C5E517ED6CD8DDDBCBA79A8F8C83786EA5B769F000AC0670A126B146BA768` | +| Tested package, `WindowsAccessControl.0.3.0-record.nupkg`, 276,055 bytes | `1E109787ECAB1A78D8118EC5A20CBEA20D9A7122AF6C202ADB17BA7207D4995E` | +| Fresh lab coverage | `E020309798847A35FF9AFBD7E2ABEA23FA2531CB0A9166BA945BC35CDEE397CA` | + +The lab coverage measures 8,504 commands, of which the domain-lab profile +exercised 3,885 (45.68 percent); the harness reports 1,712 of them as reached +only in a member-server session. It is not the local or merged coverage +percentage. Each of the 13 commands that only the lab reaches is executed in +it, with no missed command on its line: the refusal of an SDDL without a DACL +and the `-PassThru` read in `Set-SmbShareSecurityDescriptor`, both refusals in +`Remove-SmbShareAccessRule`, the confirmation throttle in +`Invoke-WindowsSmbShareCommandBatch`, and the eight commands of the two +catches in `Get-WindowsADRuleEnrichment`. The confirmation case's own session +returned its hits as `SmbSharePermissions.Live.Tests.Confirmation.ps1`. The +document replaced the 2026-10-07 document `B3EE4313…6FD6`, reached the +configured coverage path from the instrumented Desktop pass, and matches the +guest original byte-for-byte. + +## Local coverage gates + +| Edition | Passed | Failed | Skipped | Asserted coverage | Whole-module coverage | Completed UTC | +| --- | ---: | ---: | ---: | ---: | ---: | --- | +| Core 7.6.6 | 1,937 | 0 | 2 | 91.26% | 91.42% | 00:59:04 | +| Desktop 5.1.26100.33438 | 1,892 | 0 | 2 | 90.53% | 90.70% | 01:21:28 | + +Both editions ran a fresh detached `build.ps1 -Tasks test` on `bad1302` with +the candidate's version, imported the fresh lab document, and reported +`Domain-lab evidence merged: yes`. They report the domain-lab-only coverage +as 100 percent, 154 of 154 commands over the 11 files that remain declared. +The two files that left the list are asserted now, and the four commands in +them that no test had reached are executed in the merged coverage by the six +new unit cases, which are also the six cases each edition gained over the +2026-10-07 gates. The 80 percent threshold was unchanged. Each ten-task +workflow completed with zero errors and two warnings from deliberately mocked +evidence-copy failures in the runner's unit tests, and each edition skipped +the mounted-volume case and the audit-rule read that needs an unavailable +privilege. The gates ran on the host while the installed-package pass used +the lab; they touch neither the lab nor its fixtures, and the pass used a +pinned copy of the package outside the build output. + +## Observations + +- A remote prompt goes to the host of the runspace that opened the session, + not to the runspace that runs or receives the command. A job blocked on the + prompt and received in a runspace without a user interface still asked the + host of the console that opened the session, so an interactive run would + have waited for a person. The confirmation case therefore opens its own + session from a runspace without a user interface, and arms and returns + member coverage for it under its own name. +- Such a session starts with remote debugging off, so no coverage breakpoint + can be set in it. The first built pass stopped on exactly that in its + Desktop coverage edition, which the focused runs, without coverage, could + not show. The case now enables remote debugging on the session before + arming coverage, and a focused Desktop run with member coverage armed proved + that the confirmation's throttle line is recorded. +- Arming member coverage left the member session stopping on every error: + `Enter-WindowsAccessControlMemberCoverage` set `$ErrorActionPreference` to + `Stop` in a script block that a session runs at its top level, so the value + persisted. Member suites therefore stopped on every error in the Desktop + coverage pass and continued in every other pass. `a572d3d` corrects the + harness, as recorded under [Harness correction](#harness-correction). The + confirmation case still states `Continue`, so it does not depend on the + preference of the session it runs in. +- A runner started with `-File` that sets `$ErrorActionPreference = 'Stop'` + at its top level sets the global preference, which module functions + inherit. The degraded enrichment error then stops the command, as a caller + who stops on every error asked; with the default `Continue` the rules are + returned with the error. The enrichment cases state `Continue`. +- The first confirmation prototype, which stopped a job that was blocked on + the prompt and then wrote again in the same session, left the share's + description empty with its DACL unchanged; its own error handling hid the + failing step. The marker was restored. Four controlled replays, stopping the + blocked job with and without a following write in each edition, wrote + nothing until the following write, kept the description and the session's + state, and released the target lock. The cause is not established, and the + shipped case never stops a pending write. + +## Cleanup and restoration + +Before anything was removed, all evidence files on `F1ADC1` and `F1AFile2` +matched their host copies, both controllers answered signed and sealed +Kerberos LDAP, ten marked baseline objects remained with no leftover target, +the member fixtures were ready with the share's marker, and every directory, +KDC, DNS, ADWS, WinRM, and CA service ran. The runner's marker then decided +what was removed: the three payload roots, the marked `0.3.0` installation, +the six verified console logs, and both DSC stagings with their copied +evidence. The coverage plans of the two focused coverage runs were removed +after their contents were confirmed. A byte and ACL inventory of every module +path on `F1ADC1`, `F1DC1`, `F1AFile1`, and `F1AFile2` shows no difference in +any path, length, hash, security descriptor, reparse flag, or file time from +the state the 2026-10-07 follow-up left. + +The prototypes of this round staged their module in their own `wac07` +directories, removed afterwards, and restored the share after each run. The +one described under Observations left the share's description empty; the +marker was restored before any pass ran. + +At the maintainer's request, checkpoint `wac07-pre-livegaps-83cd16fa` was then +to be removed and `wac07-pre-938bdff-098b3392` kept. AutomatedLab's +`Remove-LabVMSnapshot` removes a checkpoint together with its children, and +the newer checkpoint was a child of the older one, so on ten VMs both were +removed; on the other three neither was. With the maintainer's agreement, a +new checkpoint, `wac07-after-bad1302-9d9776ca`, was taken on all thirteen +VMs, and the old checkpoints on the three were removed one at a time with +Hyper-V's own removal, which merges each into its child. All thirteen VMs keep +running with that one checkpoint, and the protocol-level readiness checks +passed after each step. The new checkpoint captures the lab after this round, +not before it. + +## Harness correction + +At the maintainer's request, the harness defect under Observations was then +corrected test-first in `a572d3d`, on the same branch. The remote block of +`Enter-WindowsAccessControlMemberCoverage` now runs its body in a child scope +that receives the values as parameters, so the `Stop` preference it sets ends +with the call, while the global list of armed breakpoints still outlives it. +The module itself did not change. + +- A unit case runs the block at the top level of a separate runspace, as a + member session does, and asserts one armed location and an unchanged + `Continue` preference. It failed with `Stop` against the previous harness + in both editions and passes after the change. The lab unit folder passes + 97 of 97 in each edition, PSScriptAnalyzer 1.25.0 reports nothing in the + three changed scripts, and the QA specification and suite-identity tests + pass. +- A live probe opened a member session from `F1ADC1` to `F1AFile1` and armed + all 8,504 locations in each edition. With the previous harness the + session's preference was `Stop` afterwards, and a non-terminating error + ended the next call; with `a572d3d` it stayed `Continue`, and the call + reached its next statement. The probe's staging was removed. +- GitVersion 5.12.0 computed the same `0.3.0-record-post-rele0001`, and a + detached `pack` built 22 tasks with zero errors and zero warnings. The root + module is byte-identical to the build of `bad1302`; the manifest differs + only in the new release note. + +| Artifact | SHA-256 | +| --- | --- | +| Root module, unchanged | `A9671FD086456978E4CE0CB5E01270B201C56FE7C1305BF7B77B9B9FB0E921CA` | +| Module manifest | `CF62149AB6151C8E8F51C023E0AFCA2CC9D20CB38AEF2A6EFFE41E6D6CA5D1FD` | +| Package, `WindowsAccessControl.0.3.0-record.nupkg`, 276,413 bytes | `89D45C0545A3EA9B2332C3469825191D828EE0C07797356F929B8E89B0B8B5A8` | +| Lab coverage | `E67DAAC331BFF1B8E80E2975FFCD284A61BB976385E2B9A6F1C5785F5F33554C` | + +| Gate | Passed | Failed | Skipped | Cleanup or coverage | Completed UTC | +| --- | ---: | ---: | ---: | --- | --- | +| Built module, Desktop with coverage | 109 | 0 | 0 | Eight ready entries | 07:50:57 | +| Built module, Core | 109 | 0 | 0 | Eight ready entries | 07:59:19 | +| Local Core 7.6.6 | 1,938 | 0 | 2 | 91.26% asserted | 08:17:52 | +| Local Desktop 5.1.26100.33438 | 1,893 | 0 | 2 | 90.53% asserted | 08:40:51 | + +The built pass used the payload root `C:\wac07-built5-a572d3d`. Its Desktop +edition is the only edition of a pass that runs the corrected code, because +member coverage is armed only where coverage is collected. The four member +suites that arm it, certificate private key, Task Scheduler, SMB share, and +foreign principal, passed there under the same error preference as in every +other pass. The coverage document measures the same 3,885 of 8,504 commands, +1,712 of them reached only in a member session, with the same missed and +covered counts on each of its 7,687 lines; it differs from `E0203097…97CA` +only in its two timestamps. The confirmation session again returned its hits +under its own name. Both local gates imported that document, reported +`Domain-lab evidence merged: yes`, and each gained exactly the new unit case +over the `bad1302` gates; domain-lab-only coverage stays at 100 percent, 154 +of 154. + +Readiness was proven at the protocol level before and after the pass, as for +`bad1302`. Before anything was removed, the guest evidence matched its host +copies, the fixtures and the directory baseline were ready, and both +installations on `F1ADC1` matched their state before the pass in root-module +hash, file count, write time, and ACL. The runner's marker then decided what +was removed: the payload root and its two console logs. After cleanup, the +inventory of every module path shows no difference in any path, length, +hash, or security descriptor from the state after this round's earlier +cleanup, and all thirteen VMs keep running with the one checkpoint +`wac07-after-bad1302-9d9776ca`. + +The installed-package pass and the DSC-engine gate were not repeated, as an +agent decision for the maintainer's review: the installed pass collects no +coverage, so the corrected code does not run in it, and the DSC gate does +not load the lab harness, while the module bytes both test are unchanged. No +new checkpoint was taken, because the pass left the fixtures, the baseline, +and every module path as they were before it. + +## Retention and remaining gates + +Private evidence is retained under administrator `%TEMP%` in +`wac07-livegaps-83cd16fa7a4645cf95066e8e92330c0f`, in the profile's base TEMP +directory. It holds the pinned candidate files of `938bdff`, `bad1302`, and +`a572d3d`, the GitVersion download and its outputs, the readiness +inspections, inventories, checkpoint identity, native exit markers, raw +console logs, per-pass reports, the coverage documents, the focused green and +red results, the changed module the red runs used and the script that made +it, the prototypes and harness probes with their evidence, and the local test +results. Raw evidence is not sanitized for publication. + +All requested acceptance gates for `bad1302` and for the harness correction +`a572d3d` are closed. A stable release still requires explicit authorization +to merge, tag, push, or publish. diff --git a/docs/usage/smb-shares.md b/docs/usage/smb-shares.md index a9f37ea..967061b 100644 --- a/docs/usage/smb-shares.md +++ b/docs/usage/smb-shares.md @@ -42,8 +42,9 @@ Set-SmbShareSecurityDescriptor -Name 'Data$' ` Share DACL writes preserve the share description. A description the native write clears is restored. A description someone else changes during the write -is kept and reported with a warning, unless the write then clears it, in which -case the value from before the write returns. When the description cannot be +is kept and reported with a warning, unless the edit lands just before the +native write, which clears it, or just after the check, before the restoration; +then the value from before the write returns. When the description cannot be checked or restored after the DACL was written, a warning names the earlier value instead of failing a change that is already live. diff --git a/source/Private/Get-WindowsADEffectiveAccessRecord.ps1 b/source/Private/Get-WindowsADEffectiveAccessRecord.ps1 index fb2d99f..03d0300 100644 --- a/source/Private/Get-WindowsADEffectiveAccessRecord.ps1 +++ b/source/Private/Get-WindowsADEffectiveAccessRecord.ps1 @@ -22,9 +22,7 @@ function Get-WindowsADEffectiveAccessRecord { 'sDRightsEffective' ) ) - $response = [System.DirectoryServices.Protocols.SearchResponse]( - $Connection.SendRequest($request) - ) + $response = Send-WindowsADSearchRequest -Connection $Connection -Request $request if ($response.Entries.Count -ne 1) { throw "Active Directory object did not resolve uniquely: '$DistinguishedName'." } diff --git a/source/Private/Get-WindowsADRootDse.ps1 b/source/Private/Get-WindowsADRootDse.ps1 index d93ff60..8f14c16 100644 --- a/source/Private/Get-WindowsADRootDse.ps1 +++ b/source/Private/Get-WindowsADRootDse.ps1 @@ -17,9 +17,7 @@ function Get-WindowsADRootDse { 'rootDomainNamingContext' ) ) - $response = [System.DirectoryServices.Protocols.SearchResponse]( - $Connection.SendRequest($request) - ) + $response = Send-WindowsADSearchRequest -Connection $Connection -Request $request if ($response.Entries.Count -ne 1) { throw 'The selected domain controller did not return one RootDSE entry.' } diff --git a/source/Private/Send-WindowsADSearchRequest.ps1 b/source/Private/Send-WindowsADSearchRequest.ps1 index 3347695..b7ea337 100644 --- a/source/Private/Send-WindowsADSearchRequest.ps1 +++ b/source/Private/Send-WindowsADSearchRequest.ps1 @@ -2,9 +2,10 @@ function Send-WindowsADSearchRequest { <# Sends one LDAP search over a bound connection and returns its response. A request for an object that does not exist fails with - ItemNotFoundException. Get-WindowsADObjectRecord reaches the directory - only through this function, so unit tests can supply a response without - a domain controller. + ItemNotFoundException. Get-WindowsADObjectRecord, Get-WindowsADRootDse, + and Get-WindowsADEffectiveAccessRecord reach the directory only through + this function, so unit tests can supply a response without a domain + controller. #> [CmdletBinding()] [OutputType([System.DirectoryServices.Protocols.SearchResponse])] diff --git a/specs/0005-verification-and-traceability.md b/specs/0005-verification-and-traceability.md index 8e2bc33..faf4b96 100644 --- a/specs/0005-verification-and-traceability.md +++ b/specs/0005-verification-and-traceability.md @@ -42,9 +42,9 @@ is not reported as a successful live write. | FR-15 | Public command tests and pipeline cases across Integration tests | | FR-16 | Path/LiteralPath help QA and filesystem-object pipeline tests | | FR-17 | `MutatorSafety.Tests.ps1` | -| FR-18 | SMB command-contract Unit tests plus disposable share DACL round-trip, add, exact-remove, `WhatIf`, unrelated-ACE preservation, and rollback tests | -| FR-19 | AD command-contract and LDAP-adapter Unit tests plus disposable-OU signed/sealed read, delegated add, object-ACE exact-remove, `WhatIf`, GUID revalidation, and rollback tests | -| FR-20 | Task Scheduler command-contract and COM-boundary Unit tests plus disposable folder/task DACL reads, `WhatIf`, containment rejection, round trip, task-definition preservation, rollback, and cleanup tests; typed access-rule contract, rights-model mask, `AppliesTo` flag-conversion, inherited-rule rejection, canonical-identity revalidation, and live add/remove tests | +| FR-18 | SMB command-contract Unit tests plus disposable share DACL round-trip, add, exact-remove, `WhatIf`, unrelated-ACE preservation, and rollback tests; live description restoration after the native write, concurrent-edit warning, and stop on a description that cannot be read before the write; live refusal of an SDDL without a DACL, of a rule copy not read from the share, and of a rule bound to another server, the descriptor `-PassThru` returns, and the confirmation a share DACL write asks for | +| FR-19 | AD command-contract and LDAP-adapter Unit tests plus disposable-OU signed/sealed read, delegated add, object-ACE exact-remove, `WhatIf`, GUID revalidation, and rollback tests; live degraded rule reports when the inheritance-source or schema-name lookup fails, and a not-found report for an object deleted after target resolution | +| FR-20 | Task Scheduler command-contract and COM-boundary Unit tests plus disposable folder/task DACL reads, `WhatIf`, containment rejection, round trip, task-definition preservation, rollback, and cleanup tests; typed access-rule contract, rights-model mask, `AppliesTo` flag-conversion, inherited-rule rejection, canonical-identity revalidation, and live add/remove tests; live repair of a protected folder and task DACL without Local System and refusal of a Local System deny there | | FR-21 | SMB effective-access contract/behavior Unit tests plus delegated local share Authz, canonical deduplication, context-label, and backing-NTFS exclusion live evidence | | FR-22 | `Edit-NTFSItemSecurityDescriptor.Tests.ps1` Unit contract plus bounded live add, pass-through, callback-failure, and non-persistence tests; `NtfsDescriptorMutators.Tests.ps1` and `RegistryDescriptorMutators.Tests.ps1` descriptor parameter-set contract, in-memory staging, unloaded-section rejection, and concurrency-token tests; live `NtfsInMemoryDescriptorEditing` and `RegistryInMemoryDescriptorEditing` round-trip, bounded-scope, `RequireUnchanged` stale-rejection, and target-unchanged evidence | | FR-23 | Certificate-private-key command/identity Unit tests plus cross-edition read-only non-exportable CNG fixture acceptance | @@ -386,8 +386,11 @@ for every family it exercises and writes one JaCoCo document. measured where that code actually runs. The runner publishes the measurable locations of the module under test, the suite arms those locations in the member runspace, and the hit counts are returned in publication order and - added to the harness-side counts. A member module file whose content differs - from the measured one is refused rather than measured. + added to the harness-side counts. A suite that drives a second member + session arms and returns that session under its own name. Arming changes no + preference variable of the session, so a suite handles errors the same way + whether or not coverage is collected. A member module file whose content + differs from the measured one is refused rather than measured. - The document is rendered from the harness-side locations, so its package, class, and source-file names are relative to the built module directory and are identical to the names the repository build produces for the same module diff --git a/specs/0009-smb-share-and-active-directory-dacl-management.md b/specs/0009-smb-share-and-active-directory-dacl-management.md index eae1775..2977bf3 100644 --- a/specs/0009-smb-share-and-active-directory-dacl-management.md +++ b/specs/0009-smb-share-and-active-directory-dacl-management.md @@ -58,9 +58,11 @@ native write and afterwards restores it only when the write cleared it, reporting the restoration in the verbose stream. A description that changed to any other value reflects a concurrent edit: it is left in place, and a warning names the value from before the write. The window is narrowed, not eliminated: -an edit that lands during the write and is then cleared by it is replaced with -the value from before the write. A description that cannot be read before the -write stops the command before anything is written. After a committed write, a +an edit that lands after that read but before the native write is cleared by +the write, and an edit that lands between the read-back and the restoration is +overwritten by it, so either edit is replaced with the value from before the +write. A description that cannot be read before the write stops the command +before anything is written. After a committed write, a description that cannot be checked or restored is reported as a warning that names the earlier value, because a step after the write must not report failure for a change that is already live; a failed restoration after a failed @@ -214,12 +216,27 @@ multi-controller behavior this contract left open, and closes open issue OI-18. - Unit tests cover target rejection, rights typing, object GUID preservation, signed/sealed connection construction, protected-target rejection, `WhatIf`, idempotent add, exact removal, distinguished-name parent parsing, - ancestor provenance matching, and pinned domain-controller discovery. + ancestor provenance matching, pinned domain-controller discovery, and the + RootDSE read through the directory search seam, which refuses a response + without exactly one entry. - Disposable live tests prove SMB DACL round trip and unrelated-ACE - preservation on the member server. + preservation on the member server. They also prove that a DACL-only native + `SE_LMSHARE` write clears the description, that the command restores it and + reports the restoration, that a description a native watcher edits as soon + as the write lands is kept with the warning, and that a description that + cannot be read before the write stops the command with nothing written. + Target resolution reads the share just before the setter does, so that last + failure is injected by shadowing `Get-SmbShare` inside the module. Further + cases prove that an SDDL without a DACL is refused, that `-PassThru` returns + the written descriptor, that a rule object not read from the share or bound + to another target is refused before anything is written, and that a write + that needs confirmation fails closed in a session that cannot prompt. - Disposable live tests prove AD DACL round trip, object-specific ACE preservation, delegated mutation, GUID revalidation, and rollback in the - test OU. + test OU. A failed inheritance-source or schema-name lookup degrades the rule + report with a non-terminating error instead of discarding the descriptor + read; no directory condition the lab identity can create fails only those + lookups, so the live case injects the failure with a Pester mock. - Disposable live tests prove that set replaces only the matching object scope, that rights removal subtracts without dropping a still-granted ACE, that an account purge leaves unrelated rules intact, that a clear which would make the diff --git a/specs/0010-task-scheduler-dacl-management.md b/specs/0010-task-scheduler-dacl-management.md index 4ba5ee2..da282a4 100644 --- a/specs/0010-task-scheduler-dacl-management.md +++ b/specs/0010-task-scheduler-dacl-management.md @@ -145,7 +145,10 @@ folder, proves typed and deduplicated reads, `WhatIf`, unsafe-boundary rejection folder and task DACL round trips, typed rule add and exact removal with unrelated-ACE preservation, Local System preservation, task-definition preservation, rollback, COM release, and task deletion in Windows PowerShell -5.1 and PowerShell 7.6.3. The complete lab must remain ready afterward. +5.1 and PowerShell 7.6.3. A second disposable folder and task, whose protected +DACLs have no Local System ACE, prove that such a DACL accepts an unrelated +ACE, still refuses a Local System deny without writing, and lets Local System +be restored to both. The complete lab must remain ready afterward. ## Later work diff --git a/specs/0018-active-directory-caller-effective-access.md b/specs/0018-active-directory-caller-effective-access.md index 8241c8d..5ff678f 100644 --- a/specs/0018-active-directory-caller-effective-access.md +++ b/specs/0018-active-directory-caller-effective-access.md @@ -101,7 +101,10 @@ reuses the directory family's existing target resolution, so the naming-context, partition, and unique-resolution refusals apply unchanged. That resolution does not request `nTSecurityDescriptor`, because no output property depends on it, so a caller without read-control access to the object's security descriptor still -receives a result. +receives a result. The three-attribute request goes through the same directory +search seam as the family's other reads: an object deleted after target +resolution fails with `ItemNotFoundException` naming it, and a response without +exactly one entry fails as an object that did not resolve uniquely. ## Verification @@ -113,9 +116,13 @@ receives a result. that neither the prevalidation of every target nor the per-target resolution requests `nTSecurityDescriptor` or the security-descriptor flag control, and that a caller whose entry carries no descriptor still receives a result. -- The three-attribute base-scope request itself has no unit-testable seam, - because it needs a bound `LdapConnection`. Its source file is declared - domain-lab-only for coverage, and live domain-lab evidence proves it. +- Unit tests replace the three-attribute base-scope request at the directory + search seam and prove that it is sent there, that a response without exactly + one entry fails, and that an absent `sDRightsEffective` is reported as zero. + Live domain-lab evidence proves the request against a controller, and deletes + a disposable organizational unit after target resolution, just before the + request reaches the controller, to prove that the command reports the + object as not found. - Live domain-lab evidence reads a real organizational unit as a domain administrator and proves a nonzero section mask, a nonempty writable-attribute list, and a nonempty creatable-child-class list against the same object diff --git a/tests/Lab/ADObjectPermissions.Live.Tests.ps1 b/tests/Lab/ADObjectPermissions.Live.Tests.ps1 index 9b9ddbd..c0ef962 100644 --- a/tests/Lab/ADObjectPermissions.Live.Tests.ps1 +++ b/tests/Lab/ADObjectPermissions.Live.Tests.ps1 @@ -284,6 +284,59 @@ Describe 'Active Directory object DACL commands' ` -ExpectedMessage '*was not found*' } + It 'Should report an object deleted after target resolution as not found' { + $deleted = New-ADOrganizationalUnit ` + -Name ('WacDeleted{0}' -f [guid]::NewGuid().ToString('N')) ` + -Path $script:targetOu ` + -Server $script:server ` + -ProtectedFromAccidentalDeletion $false ` + -PassThru ` + -ErrorAction Stop + try { + # Nothing runs between target resolution and the effective-access + # read, so the deletion happens inside that read: the mock removes + # the object and then sends the real request to the controller. + $script:deletedObjectGuid = $deleted.ObjectGuid + $script:searchSeam = & $script:module { ${function:Send-WindowsADSearchRequest} } + Mock -ModuleName WindowsAccessControl -CommandName Send-WindowsADSearchRequest ` + -ParameterFilter { @($Request.Attributes) -contains 'sDRightsEffective' } ` + -MockWith { + Remove-ADOrganizationalUnit ` + -Identity $script:deletedObjectGuid ` + -Server $script:server ` + -Confirm:$false ` + -ErrorAction Stop + & $script:searchSeam -Connection $Connection -Request $Request + } + + { + Get-ADObjectCallerEffectiveAccess ` + -Server $script:server ` + -DistinguishedName $deleted.DistinguishedName ` + -ThrottleLimit 1 ` + -ErrorAction Stop + } | Should -Throw -ExceptionType ([System.Management.Automation.ItemNotFoundException]) ` + -ExpectedMessage ("Active Directory object was not found: '{0}'." -f $deleted.DistinguishedName) + Should -Invoke -CommandName Send-WindowsADSearchRequest -ModuleName WindowsAccessControl ` + -ParameterFilter { @($Request.Attributes) -contains 'sDRightsEffective' } ` + -Times 1 -Exactly + } + finally { + try { + Remove-ADOrganizationalUnit ` + -Identity $deleted.ObjectGuid ` + -Server $script:server ` + -Confirm:$false ` + -ErrorAction Stop + } + catch [Microsoft.ActiveDirectory.Management.ADIdentityNotFoundException] { + # The case itself deleted the object. + $null = $_ + } + Remove-Variable -Name deletedObjectGuid, searchSeam -Scope Script -ErrorAction SilentlyContinue + } + } + It 'Should reject configuration and schema partition reads' { foreach ($distinguishedName in @( $script:configurationDn @@ -880,3 +933,97 @@ Describe 'Active Directory object DACL commands' ` } } } + +# FR-19: a lookup that fails after the descriptor read degrades the rule report +# instead of discarding the rules already read. +Describe 'Active Directory rule enrichment failures' ` + -Tag 'DomainLab', 'WindowsOnly', 'RequiresElevation' { + # The ancestor walk absorbs its own read failures, and an identity that + # cannot read the schema or configuration partition cannot read the rules + # either, so no directory condition fails only these lookups. A module-scope + # mock injects that one failure after the real descriptor read. The runner + # stops on every error, so the calls state the default Continue preference + # under which the degraded report is returned. + It 'Should report rules without an inheritance source when the source lookup fails' { + Mock -ModuleName WindowsAccessControl -CommandName Get-WindowsADObjectInheritanceSource -MockWith { + throw [UnauthorizedAccessException]::new( + 'WindowsAccessControl lab injected failure: the inheritance sources could not be read.' + ) + } + + $stream = @( + Get-ADObjectAccessRule ` + -Server $script:server ` + -DistinguishedName $script:targetOu ` + -ExcludeExplicit ` + -ThrottleLimit 1 ` + -ErrorAction Continue 2>&1 + ) + $failures = @($stream | Where-Object { $_ -is [Management.Automation.ErrorRecord] }) + $rules = @($stream | Where-Object { $_ -isnot [Management.Automation.ErrorRecord] }) + $expectedMessage = ( + "Cannot resolve inheritance sources for '{0}': " + + 'WindowsAccessControl lab injected failure: the inheritance sources could not be read. ' + + 'Access rules are reported without an inheritance source.' + ) -f $script:targetOu + + Should -Invoke -CommandName Get-WindowsADObjectInheritanceSource ` + -ModuleName WindowsAccessControl -Times 1 -Exactly + $failures | Should -HaveCount 1 + $failures[0].Exception.Message | Should -BeExactly $expectedMessage + $failures[0].CategoryInfo.Category | + Should -Be ([Management.Automation.ErrorCategory]::ReadError) + $failures[0].TargetObject | Should -BeExactly $script:targetOu + $rules | Should -Not -BeNullOrEmpty + @($rules | Where-Object { -not $_.IsInherited }) | Should -BeNullOrEmpty + @($rules | Where-Object InheritedFrom) | Should -BeNullOrEmpty + # The schema-name lookup does not depend on the failed one. + $objectRules = @($rules | Where-Object { $_.ObjectTypeGuid -ne [guid]::Empty }) + $objectRules | Should -Not -BeNullOrEmpty + @($objectRules | Where-Object { -not $_.ObjectTypeName }) | Should -BeNullOrEmpty + } + + It 'Should report object GUIDs without names when the schema-name lookup fails' { + Mock -ModuleName WindowsAccessControl -CommandName Resolve-WindowsADSchemaGuidName -MockWith { + throw [UnauthorizedAccessException]::new( + 'WindowsAccessControl lab injected failure: the schema names could not be read.' + ) + } + + $stream = @( + Get-ADObjectAccessRule ` + -Server $script:server ` + -DistinguishedName $script:targetOu ` + -ExcludeExplicit ` + -ThrottleLimit 1 ` + -ErrorAction Continue 2>&1 + ) + $failures = @($stream | Where-Object { $_ -is [Management.Automation.ErrorRecord] }) + $rules = @($stream | Where-Object { $_ -isnot [Management.Automation.ErrorRecord] }) + $expectedMessage = ( + "Cannot resolve directory schema names for '{0}': " + + 'WindowsAccessControl lab injected failure: the schema names could not be read. ' + + 'Access rules are reported with object GUIDs only.' + ) -f $script:targetOu + + Should -Invoke -CommandName Resolve-WindowsADSchemaGuidName ` + -ModuleName WindowsAccessControl -Times 1 -Exactly + $failures | Should -HaveCount 1 + $failures[0].Exception.Message | Should -BeExactly $expectedMessage + $failures[0].CategoryInfo.Category | + Should -Be ([Management.Automation.ErrorCategory]::ReadError) + $failures[0].TargetObject | Should -BeExactly $script:targetOu + $objectRules = @( + $rules | Where-Object { + $_.ObjectTypeGuid -ne [guid]::Empty -or + $_.InheritedObjectTypeGuid -ne [guid]::Empty + } + ) + $objectRules | Should -Not -BeNullOrEmpty + @( + $objectRules | Where-Object { $_.ObjectTypeName -or $_.InheritedObjectTypeName } + ) | Should -BeNullOrEmpty + # The inheritance-source lookup does not depend on the failed one. + @($rules | Where-Object { -not $_.InheritedFrom }) | Should -BeNullOrEmpty + } +} diff --git a/tests/Lab/README.md b/tests/Lab/README.md index d0a8c3f..15dd524 100644 --- a/tests/Lab/README.md +++ b/tests/Lab/README.md @@ -232,9 +232,10 @@ For a junction or symbolic link, remove the link itself rather than what it points to, and run again. Directories left by runs before the marker existed carry no marker. The -`C:\WacRepo` payload on `F1ADC1` is one of them, so the first run refuses it. -The installed-package pass likewise refuses an unmarked installation of the -package's version, such as the `0.0.1` installation the +`C:\WacRepo` payload that `F1ADC1` held until 2026-10-07 was one of them; it +was removed after that day's acceptance, so the default payload root works +again. The installed-package pass likewise refuses an unmarked installation of +the package's version, such as the `0.0.1` installation the [2026-09-07 reacceptance](../../docs/lab-reacceptance-2026-09-07.md) found and restored on `F1ADC1`; a local build without GitVersion also has version `0.0.1`. Decide before the run whether to keep such a directory and choose a diff --git a/tests/Lab/SmbSharePermissions.Live.Tests.ps1 b/tests/Lab/SmbSharePermissions.Live.Tests.ps1 index d21d5b9..bd893d2 100644 --- a/tests/Lab/SmbSharePermissions.Live.Tests.ps1 +++ b/tests/Lab/SmbSharePermissions.Live.Tests.ps1 @@ -492,3 +492,752 @@ Describe 'SMB share DACL commands' -Tag 'DomainLab', 'WindowsOnly', 'RequiresEle $result.FinalSddl | Should -BeExactly $result.BeforeSddl } } + +# FR-18, NFR-11: a DACL write keeps the share description. The native write clears it and the +# command restores it, a concurrent edit is kept with a warning, and a description that cannot +# be read before the write stops the command before anything is written. +Describe 'SMB share description around a DACL write' -Tag 'DomainLab', 'WindowsOnly', 'RequiresElevation' { + BeforeAll { + # Native share calls the module does not make: a DACL-only SE_LMSHARE + # write, and a watcher that edits the description as soon as the DACL + # changes, before the setter reads the description back. + $shareNativeSource = @' +using System; +using System.ComponentModel; +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Threading; + +namespace WindowsAccessControlLab +{ + public static class SmbShareNative + { + [StructLayout(LayoutKind.Sequential)] + private struct ShareInfo502 + { + public IntPtr NetName; + public int Type; + public IntPtr Remark; + public int Permissions; + public int MaxUses; + public int CurrentUses; + public IntPtr Path; + public IntPtr Password; + public int Reserved; + public IntPtr SecurityDescriptor; + } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] + private struct ShareInfo1004 + { + [MarshalAs(UnmanagedType.LPWStr)] + public string Remark; + } + + [DllImport("netapi32.dll", CharSet = CharSet.Unicode)] + private static extern int NetShareGetInfo(string serverName, string netName, int level, out IntPtr buffer); + + [DllImport("netapi32.dll", CharSet = CharSet.Unicode)] + private static extern int NetShareSetInfo(string serverName, string netName, int level, ref ShareInfo1004 buffer, out int parameterError); + + [DllImport("netapi32.dll")] + private static extern int NetApiBufferFree(IntPtr buffer); + + [DllImport("advapi32.dll")] + private static extern int GetSecurityDescriptorLength(IntPtr securityDescriptor); + + [DllImport("advapi32.dll", CharSet = CharSet.Unicode, EntryPoint = "SetNamedSecurityInfoW")] + private static extern int SetNamedSecurityInfo(string objectName, int objectType, int securityInformation, IntPtr owner, IntPtr group, byte[] dacl, IntPtr sacl); + + public static byte[] ReadSecurityDescriptor(string shareName) + { + IntPtr buffer; + int status = NetShareGetInfo(null, shareName, 502, out buffer); + if (status != 0) + { + throw new Win32Exception(status); + } + try + { + ShareInfo502 info = (ShareInfo502)Marshal.PtrToStructure(buffer, typeof(ShareInfo502)); + if (info.SecurityDescriptor == IntPtr.Zero) + { + return new byte[0]; + } + int length = GetSecurityDescriptorLength(info.SecurityDescriptor); + byte[] bytes = new byte[length]; + Marshal.Copy(info.SecurityDescriptor, bytes, 0, length); + return bytes; + } + finally + { + NetApiBufferFree(buffer); + } + } + + public static void SetRemark(string shareName, string remark) + { + ShareInfo1004 info = new ShareInfo1004(); + info.Remark = remark; + int parameterError; + int status = NetShareSetInfo(null, shareName, 1004, ref info, out parameterError); + if (status != 0) + { + throw new Win32Exception(status); + } + } + + // SE_LMSHARE (5) with DACL_SECURITY_INFORMATION (4) only. + public static void WriteDacl(string shareName, byte[] dacl) + { + int status = SetNamedSecurityInfo(shareName, 5, 4, IntPtr.Zero, IntPtr.Zero, dacl, IntPtr.Zero); + if (status != 0) + { + throw new Win32Exception(status); + } + } + + public static long SetRemarkWhenGranted(string shareName, byte[] sid, string remark, int timeoutMilliseconds, ManualResetEventSlim ready, CancellationToken cancellation) + { + Stopwatch watch = Stopwatch.StartNew(); + long nextPoll = 0; + try + { + while (watch.ElapsedMilliseconds < timeoutMilliseconds && !cancellation.IsCancellationRequested) + { + // One read per millisecond keeps the watcher well inside the + // read-back window without flooding the server service. + while (watch.ElapsedTicks < nextPoll) + { + Thread.SpinWait(20); + } + nextPoll = watch.ElapsedTicks + Stopwatch.Frequency / 1000; + byte[] current = ReadSecurityDescriptor(shareName); + ready.Set(); + if (Contains(current, sid)) + { + SetRemark(shareName, remark); + return watch.ElapsedMilliseconds; + } + } + return -1; + } + finally + { + ready.Set(); + } + } + + private static bool Contains(byte[] data, byte[] pattern) + { + for (int start = 0; start + pattern.Length <= data.Length; start++) + { + int index = 0; + while (index < pattern.Length && data[start + index] == pattern[index]) + { + index++; + } + if (index == pattern.Length) + { + return true; + } + } + return false; + } + } +} +'@ + Invoke-Command ` + -Session $script:session ` + -ArgumentList $shareNativeSource ` + -ScriptBlock { + param($Source) + + if (-not ('WindowsAccessControlLab.SmbShareNative' -as [type])) { + Add-Type -TypeDefinition $Source -ErrorAction Stop + } + } + } + + AfterEach { + Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:delegatedDescriptor.Sddl, $script:originalDescription ` + -ScriptBlock { + param($ShareName, $Sddl, $Description) + + Set-SmbShareSecurityDescriptor ` + -Name $ShareName ` + -Sddl $Sddl ` + -Confirm:$false + Set-SmbShare ` + -Name $ShareName ` + -Description $Description ` + -Confirm:$false + } + } + + It 'Should restore a description that the native DACL write cleared' { + $result = Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:testSid, $script:originalDescription ` + -ScriptBlock { + param($ShareName, $TestSid, $Description) + + # Without the module, writing the unchanged DACL natively is + # enough to clear the description. + $descriptor = [Security.AccessControl.RawSecurityDescriptor]::new( + [WindowsAccessControlLab.SmbShareNative]::ReadSecurityDescriptor($ShareName), + 0 + ) + $dacl = [byte[]]::new($descriptor.DiscretionaryAcl.BinaryLength) + $descriptor.DiscretionaryAcl.GetBinaryForm($dacl, 0) + [WindowsAccessControlLab.SmbShareNative]::WriteDacl($ShareName, $dacl) + $nativeDescription = (Get-SmbShare -Name $ShareName -ErrorAction Stop).Description + Set-SmbShare ` + -Name $ShareName ` + -Description $Description ` + -Confirm:$false ` + -ErrorAction Stop + + $outcome = Invoke-WindowsAccessControl ` + -Credential $script:WacSmbCredential ` + -ScriptBlock { + param($Name, $Sid) + + $stream = @(Add-SmbShareAccessRule ` + -Name $Name ` + -Account $Sid ` + -AccessRights Read ` + -PassThru ` + -Confirm:$false ` + -Verbose 4>&1) + [pscustomobject]@{ + AddedSid = @($stream | Where-Object { + $_ -isnot [Management.Automation.VerboseRecord] + })[0].SID + Restorations = @($stream | Where-Object { + $_ -is [Management.Automation.VerboseRecord] -and + $_.Message -like 'Restored the description*' + } | ForEach-Object { $_.Message }) + Description = (Get-SmbShare -Name $Name -ErrorAction Stop).Description + } + } ` + -ArgumentList $ShareName, $TestSid + [pscustomobject]@{ + NativeDescription = $nativeDescription + AddedSid = $outcome.AddedSid + Restorations = $outcome.Restorations + Description = $outcome.Description + } + } + + $result.NativeDescription | Should -BeNullOrEmpty + $result.AddedSid | Should -BeExactly $script:testSid + @($result.Restorations) | Should -Be @( + "Restored the description of SMB share '{0}' to '{1}' after the DACL write cleared it." -f + $script:shareName, $script:originalDescription + ) + $result.Description | Should -BeExactly $script:originalDescription + } + + It 'Should keep a description edited during the DACL write and warn with the earlier value' { + $attempts = @(Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:testSid, $script:originalDescription, $script:delegatedDescriptor.Sddl ` + -ScriptBlock { + param($ShareName, $TestSid, $Description, $DelegatedSddl) + + $sid = [Security.Principal.SecurityIdentifier]::new($TestSid) + $sidBytes = [byte[]]::new($sid.BinaryLength) + $sid.GetBinaryForm($sidBytes, 0) + # The native write publishes the grant and clears the + # description in one step, and the setter reads the description + # back some milliseconds later through CIM. A watcher that fires + # on that grant edits the description inside this window. When + # it is descheduled past the setter's read-back, the restore + # replaces its edit and no warning is due, so the write is + # retried, up to five writes. + foreach ($attempt in 1..5) { + Set-SmbShare ` + -Name $ShareName ` + -Description $Description ` + -Confirm:$false ` + -ErrorAction Stop + if (@(Get-SmbShareAccessRule -Name $ShareName | Where-Object { $_.SID -eq $TestSid }).Count) { + throw 'The share already grants the test identity, so the watcher cannot detect the write.' + } + $editedDescription = 'WindowsAccessControl lab concurrent edit {0}' -f $attempt + $ready = [Threading.ManualResetEventSlim]::new($false) + $cancellation = [Threading.CancellationTokenSource]::new() + $watcher = [powershell]::Create() + $warnings = @() + $watcherElapsed = -1 + try { + # The watcher fires on the grant this add writes, not on + # any byte change, so it cannot edit the description + # before the setter reads it. + $null = $watcher.AddScript({ + param($Name, $Sid, $Remark, $Ready, $Token) + + [WindowsAccessControlLab.SmbShareNative]::SetRemarkWhenGranted( + $Name, + $Sid, + $Remark, + 60000, + $Ready, + $Token + ) + }).AddArgument($ShareName). + AddArgument($sidBytes). + AddArgument($editedDescription). + AddArgument($ready). + AddArgument($cancellation.Token) + $pending = $watcher.BeginInvoke() + if (-not $ready.Wait(30000)) { + throw 'The description watcher did not start.' + } + # Invoke-WindowsAccessControl returns $null when its + # script block returns nothing. + $warnings = @(@(Invoke-WindowsAccessControl ` + -Credential $script:WacSmbCredential ` + -ScriptBlock { + param($Name, $Sid) + + Add-SmbShareAccessRule ` + -Name $Name ` + -Account $Sid ` + -AccessRights Read ` + -Confirm:$false 3>&1 | + Where-Object { $_ -is [Management.Automation.WarningRecord] } | + ForEach-Object { $_.Message } + } ` + -ArgumentList $ShareName, $TestSid) | Where-Object { $null -ne $_ }) + $watcherElapsed = @($watcher.EndInvoke($pending))[0] + } + finally { + $cancellation.Cancel() + $watcher.Dispose() + $cancellation.Dispose() + $ready.Dispose() + } + $finalDescription = (Get-SmbShare -Name $ShareName -ErrorAction Stop).Description + # An unchanged DACL would make the next add a no-op. + Set-SmbShareSecurityDescriptor ` + -Name $ShareName ` + -Sddl $DelegatedSddl ` + -Confirm:$false + [pscustomobject]@{ + Attempt = $attempt + EditedDescription = $editedDescription + WatcherElapsedMs = $watcherElapsed + Warnings = $warnings + FinalDescription = $finalDescription + } + if ($warnings.Count -gt 0) { + break + } + } + }) + + $last = $attempts[-1] + $because = 'the attempts were {0}' -f ( + $attempts | + Select-Object -Property Attempt, WatcherElapsedMs, FinalDescription, Warnings | + ConvertTo-Json -Depth 3 -Compress + ) + @($last.Warnings) | Should -Be @( + ("The description of SMB share '{0}' changed while its DACL was written and was left as it is. " + + "The description before the write was '{1}'.") -f + $script:shareName, $script:originalDescription + ) -Because $because + $last.FinalDescription | Should -BeExactly $last.EditedDescription -Because $because + @($attempts | Where-Object { $_.WatcherElapsedMs -lt 0 }) | Should -BeNullOrEmpty -Because $because + } + + It 'Should stop before writing when the description cannot be read before the write' { + $result = Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:testSid ` + -ScriptBlock { + param($ShareName, $TestSid) + + # Target resolution reads the share twice just before the setter + # does, so no live condition fails only the setter's own read. A + # module-scope shadow of Get-SmbShare injects that one failure + # and passes every other call to the real command. + $module = Get-Module -Name WindowsAccessControl + $sddlBefore = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl + $descriptionBefore = (Get-SmbShare -Name $ShareName -ErrorAction Stop).Description + $failure = $null + $reads = @() + try { + & $module { + $script:WacLabShareReads = [Collections.Generic.List[string]]::new() + ${function:script:Get-SmbShare} = { + [CmdletBinding()] + param( + [Parameter(Position = 0)] + [string[]]$Name + ) + + $caller = (Get-PSCallStack)[1].Command + $script:WacLabShareReads.Add($caller) + if ($caller -eq 'Set-WindowsSmbShareSecurityDescriptor' -and + @($script:WacLabShareReads | Where-Object { $_ -eq $caller }).Count -eq 1) { + throw [UnauthorizedAccessException]::new( + 'WindowsAccessControl lab injected failure: the share description could not be read.' + ) + } + SmbShare\Get-SmbShare @PSBoundParameters + } + } + try { + Invoke-WindowsAccessControl ` + -Credential $script:WacSmbCredential ` + -ScriptBlock { + param($Name, $Sid) + + Add-SmbShareAccessRule ` + -Name $Name ` + -Account $Sid ` + -AccessRights Read ` + -Confirm:$false ` + -ErrorAction Stop + } ` + -ArgumentList $ShareName, $TestSid + } + catch { + # -ErrorAction Stop turns the error into an + # ActionPreferenceStopException whose ErrorRecord, not + # its InnerException, holds the original exception. + $exception = $_.Exception + for ($depth = 0; $depth -lt 10 -and $exception; $depth++) { + if ($exception -is [Management.Automation.ActionPreferenceStopException] -and + $exception.ErrorRecord -and + -not [object]::ReferenceEquals($exception.ErrorRecord.Exception, $exception)) { + $exception = $exception.ErrorRecord.Exception + } + elseif ($exception.InnerException) { + $exception = $exception.InnerException + } + else { + break + } + } + $failure = '{0}: {1}' -f $exception.GetType().FullName, $exception.Message + } + $reads = & $module { $script:WacLabShareReads.ToArray() } + } + finally { + & $module { + $shadow = Get-Command -Name Get-SmbShare -CommandType Function + if ($shadow.ModuleName -eq 'WindowsAccessControl') { + Remove-Item -LiteralPath 'Function:\Get-SmbShare' + } + Remove-Variable -Name WacLabShareReads -Scope Script -ErrorAction SilentlyContinue + } + } + [pscustomobject]@{ + Failure = $failure + Reads = $reads + ShadowRemoved = [string](& $module { (Get-Command -Name Get-SmbShare).ModuleName }) -eq 'SmbShare' + DaclUnchanged = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl -ceq $sddlBefore + GrantPresent = [bool]@( + Get-SmbShareAccessRule -Name $ShareName | Where-Object { $_.SID -eq $TestSid } + ).Count + DescriptionUnchanged = (Get-SmbShare -Name $ShareName -ErrorAction Stop).Description -ceq + $descriptionBefore + } + } + + $result.Failure | Should -BeExactly ( + 'System.UnauthorizedAccessException: ' + + 'WindowsAccessControl lab injected failure: the share description could not be read.' + ) + @($result.Reads) | Should -Be @( + 'Resolve-WindowsSmbShareTarget' + 'Resolve-WindowsSmbShareTarget' + 'Set-WindowsSmbShareSecurityDescriptor' + ) + $result.DaclUnchanged | Should -BeTrue + $result.GrantPresent | Should -BeFalse + $result.DescriptionUnchanged | Should -BeTrue + $result.ShadowRemoved | Should -BeTrue + } +} + +# FR-18, NFR-11: refusals and the confirmation that keep an unintended share +# write from happening, and the descriptor a write reports. +Describe 'SMB share DACL command guards' -Tag 'DomainLab', 'WindowsOnly', 'RequiresElevation' { + AfterEach { + Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:delegatedDescriptor.Sddl, $script:originalDescription ` + -ScriptBlock { + param($ShareName, $Sddl, $Description) + + Set-SmbShareSecurityDescriptor ` + -Name $ShareName ` + -Sddl $Sddl ` + -Confirm:$false + Set-SmbShare ` + -Name $ShareName ` + -Description $Description ` + -Confirm:$false + } + } + + It 'Should refuse an SDDL without a DACL before writing' { + $result = Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName ` + -ScriptBlock { + param($ShareName) + + $sddlBefore = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl + # No DACL section at all, and a DACL section that is present + # but null, which would grant everyone full access. + $failures = foreach ($sddl in 'O:BA', 'D:NO_ACCESS_CONTROL') { + try { + Set-SmbShareSecurityDescriptor ` + -Name $ShareName ` + -Sddl $sddl ` + -Confirm:$false ` + -ErrorAction Stop + 'no failure' + } + catch { + $_.Exception.Message + } + } + [pscustomobject]@{ + Failures = @($failures) + Unchanged = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl -ceq $sddlBefore + } + } + + @($result.Failures) | Should -Be @( + 'The supplied SDDL does not contain a non-null DACL.' + 'The supplied SDDL does not contain a non-null DACL.' + ) + $result.Unchanged | Should -BeTrue + } + + It 'Should return the stored descriptor with PassThru' { + $result = Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:testSid ` + -ScriptBlock { + param($ShareName, $TestSid) + + $requested = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl + + "(A;;0x1200a9;;;$TestSid)" + $output = @( + Set-SmbShareSecurityDescriptor ` + -Name $ShareName ` + -Sddl $requested ` + -PassThru ` + -Confirm:$false + ) + [pscustomobject]@{ + Requested = $requested + Output = $output + Stored = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl + Description = (Get-SmbShare -Name $ShareName -ErrorAction Stop).Description + } + } + + @($result.Output) | Should -HaveCount 1 + $result.Output[0].PSObject.TypeNames | + Should -Contain 'Deserialized.WindowsAccessControl.SmbShareSecurityDescriptor' + $result.Output[0].ShareName | Should -BeExactly $script:shareName + $result.Output[0].Sections.ToString() | Should -Be 'Access' + $result.Output[0].Sddl | Should -BeExactly $result.Stored + $result.Stored | Should -BeExactly $result.Requested + $result.Description | Should -BeExactly $script:originalDescription + } + + It 'Should refuse a rule copy that was not read from the share before removing anything' { + $result = Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:testSid ` + -ScriptBlock { + param($ShareName, $TestSid) + + $added = Add-SmbShareAccessRule ` + -Name $ShareName ` + -Account $TestSid ` + -AccessRights Read ` + -PassThru ` + -Confirm:$false + $sddlBefore = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl + # A rule restored from serialized data and a reshaped copy keep + # the rule's values but are no longer the object the share + # returned. + $copies = @( + [Management.Automation.PSSerializer]::Deserialize( + [Management.Automation.PSSerializer]::Serialize($added) + ) + $added | Select-Object -Property * + ) + $failures = foreach ($copy in $copies) { + try { + $copy | Remove-SmbShareAccessRule -Confirm:$false -ErrorAction Stop + 'no failure' + } + catch { + $_.Exception.Message + } + } + [pscustomobject]@{ + Failures = @($failures) + Unchanged = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl -ceq $sddlBefore + StillGranted = [bool]@( + Get-SmbShareAccessRule -Name $ShareName | Where-Object { $_.SID -eq $TestSid } + ).Count + } + } + + @($result.Failures) | Should -Be @( + 'InputObject must be a path-bound rule from Get-SmbShareAccessRule.' + 'InputObject must be a path-bound rule from Get-SmbShareAccessRule.' + ) + $result.Unchanged | Should -BeTrue + $result.StillGranted | Should -BeTrue + } + + It 'Should refuse a rule whose canonical target names another server' { + $result = Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:shareName, $script:testSid ` + -ScriptBlock { + param($ShareName, $TestSid) + + $added = Add-SmbShareAccessRule ` + -Name $ShareName ` + -Account $TestSid ` + -AccessRights Read ` + -PassThru ` + -Confirm:$false + $sddlBefore = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl + $readTarget = $added.CanonicalTarget + $added.CanonicalTarget = 'SmbShare:WACLABOTHER:{0}' -f $ShareName.ToUpperInvariant() + $failure = try { + $added | Remove-SmbShareAccessRule -Confirm:$false -ErrorAction Stop + 'no failure' + } + catch { + $_.Exception.Message + } + [pscustomobject]@{ + ReadTarget = $readTarget + Server = [Environment]::MachineName.ToUpperInvariant() + Failure = $failure + Unchanged = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl -ceq $sddlBefore + StillGranted = [bool]@( + Get-SmbShareAccessRule -Name $ShareName | Where-Object { $_.SID -eq $TestSid } + ).Count + } + } + + $result.ReadTarget | Should -BeExactly ( + 'SmbShare:{0}:{1}' -f $result.Server, $script:shareName.ToUpperInvariant() + ) + $result.Failure | Should -BeExactly 'The SMB share rule target no longer matches its canonical identity.' + $result.Unchanged | Should -BeTrue + $result.StillGranted | Should -BeTrue + } + + It 'Should ask before writing and write nothing when the caller cannot answer' { + # Without -Confirm:$false the high-impact write asks the caller first. + # A remote prompt goes to the host of the runspace that opened the + # session, so a session opened from a runspace without a user interface + # answers it with an error in every host, and this case never waits on + # a person. Such a session also starts with remote debugging off, which + # the coverage breakpoints armed in it need. + $opener = [powershell]::Create() + $promptless = $null + $coverageArmed = $false + try { + $null = $opener.AddCommand('New-PSSession'). + AddParameter('ComputerName', $env:WAC_DOMAIN_LAB_MEMBER). + AddParameter('Authentication', 'Kerberos'). + AddParameter('ErrorAction', 'Stop') + $promptless = @($opener.Invoke())[0] + $promptless.Runspace.Debugger.SetDebugMode( + [System.Management.Automation.DebugModes]'LocalScript, RemoteScript' + ) + Invoke-Command ` + -Session $promptless ` + -ArgumentList $script:remoteManifest ` + -ScriptBlock { + param($Manifest) + + Import-Module $Manifest -Force -ErrorAction Stop + } + $null = Enter-WindowsAccessControlMemberCoverage ` + -Session $promptless ` + -ModulePath (Join-Path $script:remoteModulePath 'WindowsAccessControl.psm1') + $coverageArmed = $true + $result = Invoke-Command ` + -Session $promptless ` + -ArgumentList $script:shareName, $script:testSid ` + -ScriptBlock { + param($ShareName, $TestSid) + + $sddlBefore = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl + $descriptionBefore = (Get-SmbShare -Name $ShareName -ErrorAction Stop).Description + # The call states the default Continue preference under + # which the refusal is reported, so the case does not + # depend on the preference of the session it runs in. + $stream = @( + Set-SmbShareSecurityDescriptor ` + -Name $ShareName ` + -Sddl ($sddlBefore + "(A;;0x1200a9;;;$TestSid)") ` + -ErrorAction Continue 2>&1 + ) + $failures = @($stream | Where-Object { $_ -is [Management.Automation.ErrorRecord] }) + [pscustomobject]@{ + ConfirmPreference = [string]$ConfirmPreference + OutputCount = $stream.Count - $failures.Count + Failures = @( + $failures | ForEach-Object { + $exception = $_.Exception + while ($exception.InnerException) { + $exception = $exception.InnerException + } + '{0}: {1}' -f $exception.GetType().FullName, $exception.Message + } + ) + Target = 'SmbShare:{0}:{1}' -f + [Environment]::MachineName.ToUpperInvariant(), $ShareName.ToUpperInvariant() + DaclUnchanged = (Get-SmbShareSecurityDescriptor -Name $ShareName).Sddl -ceq $sddlBefore + DescriptionUnchanged = (Get-SmbShare -Name $ShareName -ErrorAction Stop).Description -ceq + $descriptionBefore + } + } + } + finally { + if ($promptless) { + if ($coverageArmed) { + $null = Exit-WindowsAccessControlMemberCoverage ` + -Session $promptless ` + -Name 'SmbSharePermissions.Live.Tests.Confirmation.ps1' + } + Remove-PSSession -Session $promptless + } + $opener.Dispose() + } + + $result.ConfirmPreference | Should -BeExactly 'High' + $result.OutputCount | Should -Be 0 + @($result.Failures) | Should -HaveCount 1 + $result.Failures[0] | Should -BeLike 'System.Management.Automation.Host.HostException: *' + $result.Failures[0] | Should -Match ([regex]::Escape( + 'Performing the operation "Set SMB share DACL" on target "{0}".' -f $result.Target + )) + $result.DaclUnchanged | Should -BeTrue + $result.DescriptionUnchanged | Should -BeTrue + } +} diff --git a/tests/Lab/TaskSchedulerPermissions.Live.Tests.ps1 b/tests/Lab/TaskSchedulerPermissions.Live.Tests.ps1 index b2f0b31..284e37c 100644 --- a/tests/Lab/TaskSchedulerPermissions.Live.Tests.ps1 +++ b/tests/Lab/TaskSchedulerPermissions.Live.Tests.ps1 @@ -552,6 +552,215 @@ Describe 'Task Scheduler typed access-rule commands' -Tag 'DomainLab', 'WindowsO } } +# FR-20, NFR-13: a DACL without Local System can be repaired, and a Local System deny is still refused. +Describe 'Task Scheduler DACL without Local System' -Tag 'DomainLab', 'WindowsOnly', 'RequiresElevation' { + It 'Should accept and repair a protected folder and task DACL that has no Local System ACE' { + $result = Invoke-Command ` + -Session $script:session ` + -ArgumentList $script:taskPath ` + -ScriptBlock { + param($TaskPath) + + # The marked folder would pass Local System down again, so the + # disposable DACLs are protected and every remaining ACE is + # explicit. + $removeLocalSystem = { + param([string]$Sddl) + + $descriptor = [Security.AccessControl.RawSecurityDescriptor]::new($Sddl) + for ($index = $descriptor.DiscretionaryAcl.Count - 1; $index -ge 0; $index--) { + $ace = $descriptor.DiscretionaryAcl[$index] + $knownAce = $ace -as [Security.AccessControl.KnownAce] + if ($knownAce -and $knownAce.SecurityIdentifier.Value -eq 'S-1-5-18') { + $descriptor.DiscretionaryAcl.RemoveAce($index) + continue + } + # AceFlags is byte-backed, so clear INHERITED_ACE (0x10) + # with byte arithmetic. + $ace.AceFlags = [Security.AccessControl.AceFlags]( + [byte]$ace.AceFlags -band [byte]0xEF + ) + } + $descriptor.SetFlags( + $descriptor.ControlFlags -bor + [Security.AccessControl.ControlFlags]::DiscretionaryAclProtected + ) + $descriptor.GetSddlForm([Security.AccessControl.AccessControlSections]::Access) + } + $getLocalSystemAce = { + param([string]$Sddl) + + @( + [Security.AccessControl.RawSecurityDescriptor]::new($Sddl).DiscretionaryAcl | + Where-Object { + ($_ -as [Security.AccessControl.KnownAce]) -and + $_.SecurityIdentifier.Value -eq 'S-1-5-18' + } + ) + } + + $folderName = 'WacNoSystem' + [guid]::NewGuid().ToString('N').Substring(0, 8) + $folderPath = '{0}\{1}' -f $TaskPath, $folderName + $taskName = 'WacNoSystemTask' + $outcome = [ordered]@{} + $service = $null + $parentFolder = $null + $folder = $null + $definition = $null + $action = $null + $task = $null + try { + $service = New-Object -ComObject 'Schedule.Service' + $service.Connect() + $parentFolder = $service.GetFolder($TaskPath) + $folder = $parentFolder.CreateFolder($folderName, $null) + $definition = $service.NewTask(0) + $definition.RegistrationInfo.Description = + 'WindowsAccessControl disposable Local System repair test task.' + $definition.Settings.Enabled = $false + $definition.Principal.UserId = 'SYSTEM' + $definition.Principal.LogonType = 5 + $action = $definition.Actions.Create(0) + $action.Path = "$env:SystemRoot\System32\cmd.exe" + $action.Arguments = '/c exit 0' + $task = $folder.RegisterTaskDefinition( + $taskName, + $definition, + 6, + $null, + $null, + 5, + $null + ) + + $folder.SetSecurityDescriptor( + (& $removeLocalSystem (Get-TaskFolderSecurityDescriptor -Path $folderPath).Sddl), + 0 + ) + $task.SetSecurityDescriptor( + (& $removeLocalSystem (Get-ScheduledTaskSecurityDescriptor ` + -TaskPath $folderPath -TaskName $taskName).Sddl), + 0x10 + ) + $folderWithoutSystem = (Get-TaskFolderSecurityDescriptor -Path $folderPath).Sddl + $outcome.FolderSystemBefore = @(& $getLocalSystemAce $folderWithoutSystem).Count + $outcome.TaskSystemBefore = @(& $getLocalSystemAce (Get-ScheduledTaskSecurityDescriptor ` + -TaskPath $folderPath -TaskName $taskName).Sddl).Count + + $outcome.DenyRejected = $null + try { + Add-TaskFolderAccessRule ` + -Path $folderPath ` + -AllowedRootPath $TaskPath ` + -Account 'S-1-5-18' ` + -AccessRights FullControl ` + -AccessControlType Deny ` + -Confirm:$false ` + -ErrorAction Stop + } + catch { + $outcome.DenyRejected = $_.Exception.Message + } + $outcome.DenyLeftDaclUnchanged = (Get-TaskFolderSecurityDescriptor ` + -Path $folderPath).Sddl -ceq $folderWithoutSystem + + $unrelated = @(Add-TaskFolderAccessRule ` + -Path $folderPath ` + -AllowedRootPath $TaskPath ` + -Account 'S-1-1-0' ` + -AccessRights ReadAndTraverse ` + -PassThru ` + -Confirm:$false ` + -ErrorAction Stop) + $outcome.UnrelatedSid = $unrelated[0].SID + $outcome.SystemAfterUnrelated = @(& $getLocalSystemAce (Get-TaskFolderSecurityDescriptor ` + -Path $folderPath).Sddl).Count + + $folderRepair = @(Add-TaskFolderAccessRule ` + -Path $folderPath ` + -AllowedRootPath $TaskPath ` + -Account 'S-1-5-18' ` + -AccessRights FullControl ` + -PassThru ` + -Confirm:$false ` + -ErrorAction Stop) + $taskRepair = @(Add-ScheduledTaskAccessRule ` + -TaskPath $folderPath ` + -TaskName $taskName ` + -AllowedRootPath $TaskPath ` + -Account 'S-1-5-18' ` + -AccessRights FullControl ` + -PassThru ` + -Confirm:$false ` + -ErrorAction Stop) + $outcome.FolderRepairRights = [string]$folderRepair[0].AccessRights + $outcome.TaskRepairRights = [string]$taskRepair[0].AccessRights + $systemAfterRepair = @( + & $getLocalSystemAce (Get-TaskFolderSecurityDescriptor -Path $folderPath).Sddl + & $getLocalSystemAce (Get-ScheduledTaskSecurityDescriptor ` + -TaskPath $folderPath -TaskName $taskName).Sddl + ) + $outcome.ExplicitSystemAllowAfterRepair = @( + $systemAfterRepair | Where-Object { + -not $_.IsInherited -and + ($_ -as [Security.AccessControl.QualifiedAce]).AceQualifier -eq + [Security.AccessControl.AceQualifier]::AccessAllowed + } + ).Count + $outcome.SystemAceCountAfterRepair = $systemAfterRepair.Count + } + finally { + try { + if ($folder -and $task) { + $folder.DeleteTask($taskName, 0) + } + if ($parentFolder -and $folder) { + $parentFolder.DeleteFolder($folderName, 0) + } + } + catch { + Write-Warning $_.Exception.Message + } + $outcome.FolderRemoved = $true + try { + $remaining = $service.GetFolder($folderPath) + $outcome.FolderRemoved = $false + [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($remaining) + } + catch { + $null = $_ + } + foreach ($comObject in @( + $task, + $action, + $definition, + $folder, + $parentFolder, + $service + )) { + if ($null -ne $comObject -and + [Runtime.InteropServices.Marshal]::IsComObject($comObject)) { + [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($comObject) + } + } + } + [pscustomobject]$outcome + } + + $result.FolderSystemBefore | Should -Be 0 + $result.TaskSystemBefore | Should -Be 0 + $result.DenyRejected | Should -BeLike '*must not add an explicit SYSTEM deny ACE*' + $result.DenyLeftDaclUnchanged | Should -BeTrue + $result.UnrelatedSid | Should -BeExactly 'S-1-1-0' + $result.SystemAfterUnrelated | Should -Be 0 + $result.FolderRepairRights | Should -BeExactly 'FullControl' + $result.TaskRepairRights | Should -BeExactly 'FullControl' + $result.SystemAceCountAfterRepair | Should -Be 2 + $result.ExplicitSystemAllowAfterRepair | Should -Be 2 + $result.FolderRemoved | Should -BeTrue + } +} + Describe 'Task Scheduler portability and desired state' -Tag 'DomainLab', 'WindowsOnly', 'RequiresElevation' { It 'Should round trip a schema-version-2 task backup on its own computer' { $result = Invoke-Command ` diff --git a/tests/Lab/WindowsAccessControl.DomainLab.psm1 b/tests/Lab/WindowsAccessControl.DomainLab.psm1 index bb75d3e..456f52b 100644 --- a/tests/Lab/WindowsAccessControl.DomainLab.psm1 +++ b/tests/Lab/WindowsAccessControl.DomainLab.psm1 @@ -1264,39 +1264,46 @@ function Enter-WindowsAccessControlMemberCoverage { -ScriptBlock { param($ModulePath, $Lines, $Columns, $ModuleHash) - $ErrorActionPreference = 'Stop' - $algorithm = [Security.Cryptography.SHA256]::Create() - try { - $stream = [IO.File]::OpenRead($ModulePath) + # The session runs this block at its top level, so a preference set + # here would outlast the call and make the rest of the suite stop on + # its first non-terminating error. The child scope keeps it here. + & { + param($ModulePath, $Lines, $Columns, $ModuleHash) + + $ErrorActionPreference = 'Stop' + $algorithm = [Security.Cryptography.SHA256]::Create() try { - $actualHash = [BitConverter]::ToString( - $algorithm.ComputeHash($stream) - ).Replace('-', '') + $stream = [IO.File]::OpenRead($ModulePath) + try { + $actualHash = [BitConverter]::ToString( + $algorithm.ComputeHash($stream) + ).Replace('-', '') + } + finally { + $stream.Dispose() + } } finally { - $stream.Dispose() + $algorithm.Dispose() } - } - finally { - $algorithm.Dispose() - } - if ($actualHash -cne $ModuleHash) { - throw [InvalidOperationException]::new( - 'The member module under test does not match the measured module.' - ) - } - - $action = { $null = Remove-PSBreakpoint -Id $_.Id } - $global:WindowsAccessControlCoverageBreakpoints = @( - for ($index = 0; $index -lt $Lines.Count; $index++) { - Set-PSBreakpoint ` - -Script $ModulePath ` - -Line $Lines[$index] ` - -Column $Columns[$index] ` - -Action $action + if ($actualHash -cne $ModuleHash) { + throw [InvalidOperationException]::new( + 'The member module under test does not match the measured module.' + ) } - ) - @($global:WindowsAccessControlCoverageBreakpoints).Count + + $action = { $null = Remove-PSBreakpoint -Id $_.Id } + $global:WindowsAccessControlCoverageBreakpoints = @( + for ($index = 0; $index -lt $Lines.Count; $index++) { + Set-PSBreakpoint ` + -Script $ModulePath ` + -Line $Lines[$index] ` + -Column $Columns[$index] ` + -Action $action + } + ) + @($global:WindowsAccessControlCoverageBreakpoints).Count + } $ModulePath $Lines $Columns $ModuleHash }) } diff --git a/tests/Lab/acceptance-checklist.md b/tests/Lab/acceptance-checklist.md index 570fbdb..d83207f 100644 --- a/tests/Lab/acceptance-checklist.md +++ b/tests/Lab/acceptance-checklist.md @@ -30,13 +30,14 @@ test-fixture repair found during that run. A later candidate needs fresh proof. - Do not use `-SkipPayloadDeployment` for this first run: the old lab payload does not contain the new tests or fixes. - Decide first what happens to directories that predate the runner's - ownership marker, because the runner refuses to replace them. The existing - `C:\WacRepo` payload on `F1ADC1` has no marker, and neither has the `0.0.1` - installation that the 2026-09-07 reacceptance restored there; a local build - without GitVersion has that same version. Either keep such a directory and - add a new, absent `RemoteRepositoryPath` to `$parameters` or install a - package with another version, or inspect it and remove it yourself. The - runner has no backup and restore. See + ownership marker, because the runner refuses to replace them. The unmarked + `C:\WacRepo` payload on `F1ADC1` was removed after the 2026-10-07 + acceptance, so the default payload root works again. The unmarked `0.0.1` + and `0.2.0` installations there were kept; a local build without GitVersion + has that same `0.0.1` version. Either keep such a directory and add a new, + absent `RemoteRepositoryPath` to `$parameters` or install a package with + another version, or inspect it and remove it yourself. The runner has no + backup and restore. See [payload and module ownership](README.md#payload-and-module-ownership). Inspect the candidate locally: @@ -146,16 +147,34 @@ because it instruments the built module, not the installed copy. It marks the module version directory it installs and refuses to replace an unmarked installation of the same version. -The new live cases are: - -- `Should reject an escaped name outside the allowed OU without changing its - DACL` in [ADObjectPermissions.Live.Tests.ps1](ADObjectPermissions.Live.Tests.ps1). -- `Should reject an old expected GUID after a distinguished name is reused` in - [ADObjectReplication.Live.Tests.ps1](ADObjectReplication.Live.Tests.ps1). - -Both require unchanged DACL evidence and exact-identity cleanup. The existing -replication suite still intentionally stops and restores the partner directory -service; check that the partner answers LDAP after the suite. +The most recently added live cases are: + +- The `SMB share DACL command guards` block in + [SmbSharePermissions.Live.Tests.ps1](SmbSharePermissions.Live.Tests.ps1): + an SDDL without a DACL is refused, `-PassThru` returns the stored + descriptor, a rule copy that was not read from the share and a rule bound to + another server are refused, and a share DACL write without + `-Confirm:$false` asks for confirmation. The last case opens its own member + session from a runspace without a user interface, so the prompt fails in + every host instead of waiting for a person. It enables remote debugging on + that session, which such a session starts without, and then arms and + returns member coverage for it under the name + `SmbSharePermissions.Live.Tests.Confirmation.ps1`. +- The `Active Directory rule enrichment failures` block and `Should report an + object deleted after target resolution as not found` in + [ADObjectPermissions.Live.Tests.ps1](ADObjectPermissions.Live.Tests.ps1). + No directory condition fails only the enrichment lookups, so a module-scope + Pester mock injects each failure after the real descriptor read. The + deletion case creates a disposable organizational unit and deletes it + inside the effective-access request, before the real request reaches the + controller. + +Every earlier case still runs, including the Task Scheduler repair of a DACL +without Local System, the SMB description cases, and the escaped-name and +reused-GUID regressions, which require unchanged DACL evidence and +exact-identity cleanup. The existing replication suite still intentionally +stops and restores the partner directory service; check that the partner +answers LDAP after the suite. ## Evidence and stop conditions diff --git a/tests/Unit/Lab/WindowsAccessControl.DomainLab.Tests.ps1 b/tests/Unit/Lab/WindowsAccessControl.DomainLab.Tests.ps1 index fb49cbf..6d85ecc 100644 --- a/tests/Unit/Lab/WindowsAccessControl.DomainLab.Tests.ps1 +++ b/tests/Unit/Lab/WindowsAccessControl.DomainLab.Tests.ps1 @@ -512,3 +512,67 @@ Describe 'WindowsAccessControl domain lab plan' -Tag 'Unit', 'WindowsOnly' { $outputPath | Should -Not -Exist } } + +Describe 'Enter-WindowsAccessControlMemberCoverage' -Tag 'Unit', 'WindowsOnly' { + BeforeEach { + $script:previousCoverageDirectory = $env:WAC_DOMAIN_LAB_COVERAGE + } + + AfterEach { + if ($null -eq $script:previousCoverageDirectory) { + Remove-Item -Path 'Env:WAC_DOMAIN_LAB_COVERAGE' -ErrorAction SilentlyContinue + } + else { + $env:WAC_DOMAIN_LAB_COVERAGE = $script:previousCoverageDirectory + } + } + + It 'Should arm the measured locations without changing the error preference of the member session' { + $modulePath = Join-Path -Path $TestDrive -ChildPath 'Measured.psm1' + Set-Content -LiteralPath $modulePath -Value @('function Get-Measured {', ' 1', '}') + [ordered]@{ + SchemaVersion = 1 + ModuleHash = (Get-FileHash -LiteralPath $modulePath -Algorithm SHA256).Hash + Lines = @(2) + Columns = @(5) + } | + ConvertTo-Json | + Set-Content -LiteralPath (Join-Path -Path $TestDrive -ChildPath 'breakpoints.json') + $env:WAC_DOMAIN_LAB_COVERAGE = $TestDrive + $probe = @' +[pscustomobject]@{ + Preference = [string]$ErrorActionPreference + Armed = @($global:WindowsAccessControlCoverageBreakpoints).Count +} +'@ + # A member session runs every script block it receives at its top + # level, so whatever the block assigns there outlives the call. A + # runspace that runs the same text through AddScript does the same. + $memberRunspace = [powershell]::Create() + try { + Mock -ModuleName 'WindowsAccessControl.DomainLab' Invoke-Command { + $null = $memberRunspace.AddScript($ScriptBlock.ToString()) + foreach ($argument in $ArgumentList) { + $null = $memberRunspace.AddArgument($argument) + } + $memberRunspace.Invoke() + if ($memberRunspace.HadErrors) { + throw $memberRunspace.Streams.Error[0].Exception + } + $memberRunspace.Commands.Clear() + } + + $armed = Enter-WindowsAccessControlMemberCoverage ` + -Session (New-MockObject -Type ([System.Management.Automation.Runspaces.PSSession])) ` + -ModulePath $modulePath + $state = @($memberRunspace.AddScript($probe).Invoke())[0] + } + finally { + $memberRunspace.Dispose() + } + + $armed | Should -Be 1 + $state.Armed | Should -Be 1 + $state.Preference | Should -BeExactly 'Continue' + } +} diff --git a/tests/Unit/Private/WindowsADAccessControl.Tests.ps1 b/tests/Unit/Private/WindowsADAccessControl.Tests.ps1 index d7cc5d8..e3b2c28 100644 --- a/tests/Unit/Private/WindowsADAccessControl.Tests.ps1 +++ b/tests/Unit/Private/WindowsADAccessControl.Tests.ps1 @@ -534,6 +534,156 @@ Describe 'Active Directory access-control internals' -Tag 'Unit', 'WindowsOnly' } } + It 'Should read the RootDSE through the search seam and refuse entries' -ForEach @( + @{ Count = 0 } + @{ Count = 2 } + ) { + InModuleScope WindowsAccessControl -Parameters @{ EntryCount = $Count } { + $script:testEntryCount = $EntryCount + Mock Send-WindowsADSearchRequest { + [pscustomobject]@{ + Entries = @( + for ($index = 0; $index -lt $script:testEntryCount; $index++) { + [pscustomobject]@{ + DistinguishedName = '' + Attributes = @{ + defaultNamingContext = @('DC=example,DC=test') + } + } + } + ) + } + } + $connection = [System.DirectoryServices.Protocols.LdapConnection]::new( + [System.DirectoryServices.Protocols.LdapDirectoryIdentifier]::new( + 'dc01.example.test', 389, $true, $false + ) + ) + try { + { Get-WindowsADRootDse -Connection $connection } | + Should -Throw -ExpectedMessage 'The selected domain controller did not return one RootDSE entry.' + } + finally { + $connection.Dispose() + } + + Should -Invoke Send-WindowsADSearchRequest -Times 1 -Exactly -ParameterFilter { + $Request.DistinguishedName -eq '' -and + $Request.Scope -eq [System.DirectoryServices.Protocols.SearchScope]::Base + } + } + } + + It 'Should report no root domain naming context when the RootDSE omits it' { + InModuleScope WindowsAccessControl { + Mock Send-WindowsADSearchRequest { + [pscustomobject]@{ + Entries = @( + [pscustomobject]@{ + DistinguishedName = '' + Attributes = @{ + defaultNamingContext = @('DC=example,DC=test') + configurationNamingContext = @('CN=Configuration,DC=example,DC=test') + schemaNamingContext = @('CN=Schema,CN=Configuration,DC=example,DC=test') + } + } + ) + } + } + $connection = [System.DirectoryServices.Protocols.LdapConnection]::new( + [System.DirectoryServices.Protocols.LdapDirectoryIdentifier]::new( + 'dc01.example.test', 389, $true, $false + ) + ) + try { + $rootDse = Get-WindowsADRootDse -Connection $connection + } + finally { + $connection.Dispose() + } + + $rootDse.DefaultNamingContext | Should -BeExactly 'DC=example,DC=test' + $rootDse.SchemaNamingContext | + Should -BeExactly 'CN=Schema,CN=Configuration,DC=example,DC=test' + $rootDse.RootDomainNamingContext | Should -BeNullOrEmpty + } + } + + It 'Should read effective access through the search seam and refuse entries' -ForEach @( + @{ Count = 0 } + @{ Count = 2 } + ) { + InModuleScope WindowsAccessControl -Parameters @{ EntryCount = $Count } { + $script:testEntryCount = $EntryCount + Mock Send-WindowsADSearchRequest { + [pscustomobject]@{ + Entries = @( + for ($index = 0; $index -lt $script:testEntryCount; $index++) { + [pscustomobject]@{ + DistinguishedName = $Request.DistinguishedName + Attributes = @{ sDRightsEffective = @('15') } + } + } + ) + } + } + $connection = [System.DirectoryServices.Protocols.LdapConnection]::new( + [System.DirectoryServices.Protocols.LdapDirectoryIdentifier]::new( + 'dc01.example.test', 389, $true, $false + ) + ) + try { + { + Get-WindowsADEffectiveAccessRecord ` + -Connection $connection ` + -DistinguishedName 'OU=Lab,DC=example,DC=test' + } | Should -Throw -ExpectedMessage "Active Directory object did not resolve uniquely: 'OU=Lab,DC=example,DC=test'." + } + finally { + $connection.Dispose() + } + + Should -Invoke Send-WindowsADSearchRequest -Times 1 -Exactly -ParameterFilter { + $Request.DistinguishedName -eq 'OU=Lab,DC=example,DC=test' -and + @($Request.Attributes) -contains 'sDRightsEffective' + } + } + } + + It 'Should report zero descriptor rights when the controller omits sDRightsEffective' { + InModuleScope WindowsAccessControl { + Mock Send-WindowsADSearchRequest { + [pscustomobject]@{ + Entries = @( + [pscustomobject]@{ + DistinguishedName = $Request.DistinguishedName + Attributes = @{ + allowedAttributesEffective = @('description', 'displayName') + } + } + ) + } + } + $connection = [System.DirectoryServices.Protocols.LdapConnection]::new( + [System.DirectoryServices.Protocols.LdapDirectoryIdentifier]::new( + 'dc01.example.test', 389, $true, $false + ) + ) + try { + $record = Get-WindowsADEffectiveAccessRecord ` + -Connection $connection ` + -DistinguishedName 'OU=Lab,DC=example,DC=test' + } + finally { + $connection.Dispose() + } + + $record.SDRightsEffective | Should -Be 0 + @($record.WritableAttribute) | Should -Be @('description', 'displayName') + @($record.CreatableChildClass) | Should -HaveCount 0 + } + } + It 'Should split a distinguished name at the first unescaped comma only' { & $script:module { Get-WindowsADParentDistinguishedName `