diff --git a/.github/scripts/Invoke-TestsAsBasicUser.ps1 b/.github/scripts/Invoke-TestsAsBasicUser.ps1 new file mode 100644 index 0000000..60d33c6 --- /dev/null +++ b/.github/scripts/Invoke-TestsAsBasicUser.ps1 @@ -0,0 +1,215 @@ +<# +.SYNOPSIS + Runs the Pester tests as a basic user, without the privileges and the Administrators group of the current account. + +.DESCRIPTION + Some tests need a session without the Security, Restore, or Create Symbolic Link privilege, and skip in an elevated + session such as that of a GitHub runner. This script derives a token of the SAFER level Normal User from the token + of the current process, as runas /trustlevel:0x20000 does: the Administrators group is deny-only, and only the + privilege to bypass traverse checking stays. It starts Invoke-Tests.ps1 in the same PowerShell edition with that + token, waits for it, prints its output, and fails if the tests failed. The job summary of GitHub Actions gets the + results through a file of this account, which the restricted token can write. + +.PARAMETER ResultPath + Specifies the path of the result file in the NUnit format. + +.PARAMETER Title + Specifies the heading of the test results in the job summary. It can't contain a double quote, a percent sign, or a + line break, which would change the command line of cmd.exe. + +.EXAMPLE + .\.github\scripts\Invoke-TestsAsBasicUser.ps1 -ResultPath TestResults\WindowsPowerShell-BasicUser.xml -Title 'Windows PowerShell 5.1 as a basic user' + + Runs the tests in Windows PowerShell 5.1 as a basic user. +#> +[CmdletBinding()] +param ( + [Parameter(Mandatory)] + [ValidateNotNullOrEmpty()] + [string] + $ResultPath, + + [Parameter(Mandatory)] + [ValidateNotNullOrEmpty()] + # cmd.exe gets the title in a quoted argument, expands environment variables also inside quotes, and ends the + # command at a line break. \z, unlike $, doesn't match before a final line feed. + [ValidatePattern('\A[^"%\r\n]+\z')] + [string] + $Title +) + +$ErrorActionPreference = 'Stop' + +Add-Type -TypeDefinition @' +using System; +using System.ComponentModel; +using System.Runtime.InteropServices; + +public static class NTFSSecurityBasicUserProcess +{ + private const uint SaferScopeIdUser = 2; + private const uint SaferLevelIdNormalUser = 0x20000; + private const uint SaferLevelOpen = 1; + private const uint CreateNoWindow = 0x08000000; + private const uint Infinite = 0xFFFFFFFF; + private const uint WaitObject0 = 0; + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] + private struct StartupInfo + { + public int cb; + public string lpReserved; + public string lpDesktop; + public string lpTitle; + public int dwX; + public int dwY; + public int dwXSize; + public int dwYSize; + public int dwXCountChars; + public int dwYCountChars; + public int dwFillAttribute; + public int dwFlags; + public short wShowWindow; + public short cbReserved2; + public IntPtr lpReserved2; + public IntPtr hStdInput; + public IntPtr hStdOutput; + public IntPtr hStdError; + } + + [StructLayout(LayoutKind.Sequential)] + private struct ProcessInformation + { + public IntPtr hProcess; + public IntPtr hThread; + public int dwProcessId; + public int dwThreadId; + } + + [DllImport("advapi32.dll", SetLastError = true)] + private static extern bool SaferCreateLevel(uint scopeId, uint levelId, uint openFlags, out IntPtr levelHandle, IntPtr reserved); + + [DllImport("advapi32.dll", SetLastError = true)] + private static extern bool SaferComputeTokenFromLevel(IntPtr levelHandle, IntPtr inAccessToken, out IntPtr outAccessToken, uint flags, IntPtr reserved); + + [DllImport("advapi32.dll", SetLastError = true)] + private static extern bool SaferCloseLevel(IntPtr levelHandle); + + [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + private static extern bool CreateProcessAsUser(IntPtr token, string applicationName, string commandLine, IntPtr processAttributes, IntPtr threadAttributes, bool inheritHandles, uint creationFlags, IntPtr environment, string currentDirectory, ref StartupInfo startupInfo, out ProcessInformation processInformation); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern uint WaitForSingleObject(IntPtr handle, uint milliseconds); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool GetExitCodeProcess(IntPtr process, out uint exitCode); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool CloseHandle(IntPtr handle); + + // Starts the command line with a token of the SAFER level Normal User, waits for it, and returns its exit code. + public static int Run(string applicationName, string commandLine, string currentDirectory) + { + IntPtr level; + if (!SaferCreateLevel(SaferScopeIdUser, SaferLevelIdNormalUser, SaferLevelOpen, out level, IntPtr.Zero)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + IntPtr token = IntPtr.Zero; + try + { + if (!SaferComputeTokenFromLevel(level, IntPtr.Zero, out token, 0, IntPtr.Zero)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + var startupInfo = new StartupInfo(); + startupInfo.cb = Marshal.SizeOf(typeof(StartupInfo)); + ProcessInformation processInformation; + if (!CreateProcessAsUser(token, applicationName, commandLine, IntPtr.Zero, IntPtr.Zero, false, CreateNoWindow, IntPtr.Zero, currentDirectory, ref startupInfo, out processInformation)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + try + { + if (WaitForSingleObject(processInformation.hProcess, Infinite) != WaitObject0) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + uint exitCode; + if (!GetExitCodeProcess(processInformation.hProcess, out exitCode)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + return (int)exitCode; + } + finally + { + CloseHandle(processInformation.hThread); + CloseHandle(processInformation.hProcess); + } + } + finally + { + if (token != IntPtr.Zero) + { + CloseHandle(token); + } + + SaferCloseLevel(level); + } + } +} +'@ + +$repositoryPath = (Resolve-Path -LiteralPath (Join-Path -Path $PSScriptRoot -ChildPath '..\..')).ProviderPath +$resultFullPath = [IO.Path]::GetFullPath((Join-Path -Path $repositoryPath -ChildPath $ResultPath)) +$resultFolder = Split-Path -Path $resultFullPath -Parent +if (-not (Test-Path -LiteralPath $resultFolder)) { + New-Item -ItemType Directory -Path $resultFolder | Out-Null +} + +$runFolder = Join-Path -Path ([IO.Path]::GetTempPath()) -ChildPath ('NTFSSecurity.BasicUser-{0}' -f [guid]::NewGuid().ToString('N').Substring(0, 8)) +New-Item -ItemType Directory -Path $runFolder | Out-Null +$logPath = Join-Path -Path $runFolder -ChildPath 'Output.log' +$summaryPath = Join-Path -Path $runFolder -ChildPath 'Summary.md' +# In the temp folder of the account, which the restricted token can write, unlike maybe the folder of the repository +$runResultPath = Join-Path -Path $runFolder -ChildPath 'Result.xml' + +$executable = (Get-Process -Id $PID).Path +$testScript = Join-Path -Path $PSScriptRoot -ChildPath 'Invoke-Tests.ps1' +# cmd redirects the output of the tests, which have no console of their own. +$commandLine = 'cmd.exe /d /s /c ""{0}" -NoProfile -NonInteractive -ExecutionPolicy Bypass -File "{1}" -ResultPath "{2}" -Title "{3}" > "{4}" 2>&1"' -f + $executable, $testScript, $runResultPath, $Title, $logPath + +$stepSummary = $env:GITHUB_STEP_SUMMARY +$env:GITHUB_STEP_SUMMARY = $summaryPath +try { + "Running the tests as a basic user: $commandLine" + $exitCode = [NTFSSecurityBasicUserProcess]::Run((Join-Path -Path $env:SystemRoot -ChildPath 'System32\cmd.exe'), $commandLine, $repositoryPath) +} +finally { + $env:GITHUB_STEP_SUMMARY = $stepSummary +} + +if (Test-Path -LiteralPath $logPath) { + Get-Content -LiteralPath $logPath +} + +if ($stepSummary -and (Test-Path -LiteralPath $summaryPath)) { + $encoding = New-Object -TypeName 'System.Text.UTF8Encoding' -ArgumentList $false + [IO.File]::AppendAllText($stepSummary, [IO.File]::ReadAllText($summaryPath), $encoding) +} + +if (Test-Path -LiteralPath $runResultPath) { + Copy-Item -LiteralPath $runResultPath -Destination $resultFullPath -Force +} + +Remove-Item -LiteralPath $runFolder -Recurse -Force +if ($exitCode -ne 0) { + throw "The tests as a basic user failed with exit code $exitCode." +} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc25332..e2c63d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,6 +121,18 @@ jobs: shell: pwsh run: ./.github/scripts/Invoke-Tests.ps1 -ResultPath TestResults/PowerShell7.xml -Title 'PowerShell 7' + # Without the privileges and the Administrators group of the runner account, so the tests that an elevated + # session skips run as well + - name: Run the tests in Windows PowerShell 5.1 as a basic user + if: ${{ !cancelled() && steps.build.outcome == 'success' }} + shell: powershell + run: .\.github\scripts\Invoke-TestsAsBasicUser.ps1 -ResultPath TestResults\WindowsPowerShell-BasicUser.xml -Title 'Windows PowerShell 5.1 as a basic user' + + - name: Run the tests in PowerShell 7 as a basic user + if: ${{ !cancelled() && steps.build.outcome == 'success' }} + shell: pwsh + run: ./.github/scripts/Invoke-TestsAsBasicUser.ps1 -ResultPath TestResults/PowerShell7-BasicUser.xml -Title 'PowerShell 7 as a basic user' + - name: Upload the test results if: ${{ !cancelled() }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/.memory-bank/activeContext.md b/.memory-bank/activeContext.md index 935054f..c6136ce 100644 --- a/.memory-bank/activeContext.md +++ b/.memory-bank/activeContext.md @@ -1,6 +1,6 @@ --- status: current -last-verified: 2026-10-07 +last-verified: 2026-10-08 owner: active-agent source: current task evidence --- @@ -9,50 +9,70 @@ source: current task evidence ## Current focus -5.0.0-rc5, prepared on the branch `ai/release-5.0.0-rc5`: the live tests -in `Tests\Lab` (Decision 20) and the fix of -`Get-NTFSEffectiveAccess -ServerName` that they found. The maintainer -pushes the branch, opens and merges the pull request, and tags -`5.0.0-rc5`; CI publishes it (Decision 12). Then, with the tester feedback -in #34, he decides on 5.0.0. After 5.0.0 the repository is archived in -favor of WindowsAccessControl (Decision 18). +Phase 2 of the quality gate before 5.0.0 (Decision 21) is complete on the +local branch `ai/release-5.0.0-rc6`, candidate `7b0781f`. The maintainer +pushes the branch, merges the pull request after CI, and tags 5.0.0-rc6. +Then Phase 3 (live tests on more operating systems) and 5.0.0; after +5.0.0 the repository is archived in favor of WindowsAccessControl +(Decision 18). ## Evidence -- 2026-10-07: the live tests ran in the lab of WindowsAccessControl - (`F1ADC1`, `F1AFile2`, `F1AFile1` in `a.forest1.net`) against the Gallery - packages of 5.0.0-rc2 and 5.0.0-rc4 and against the branch build, in - Windows PowerShell 5.1 and PowerShell 7, with the same results in both: - - Case 1 (#34 over SMB): rc2 fails `Add-NTFSAccess`, `Clear-NTFSAccess`, - and `Set-NTFSSecurityDescriptor` with error 1307, on folders with and - without the auto-inherit flag; the other four cmdlets succeed in rc2 - too. rc4 passes all seven and keeps Administrators as the owner. - - Case 2: the administrators of the file server read, add, and remove - audit entries over SMB; the delegated account gets the errors that the - cmdlet pages describe, and the folders stay unchanged. Same in rc2. - - Case 3: with `-ServerName` of the file server, the result includes its - local group, without a warning; without it, only the domain groups - count. With a computer that can't be reached, rc2 and rc4 returned no - access, because error 1722 was swallowed; fixed on the branch. - - Case 4, long paths on the share, and #108 pass; #108 fails in rc2. - - The branch build passes every live test, and the 499 tests of `Tests` - in both editions. -- One `security-reviewer` pass approved the branch with minor findings; - the assertion of the fallback warning and the path guard of the live - tests were hardened. Deferred: the bare `catch` in - `Win32.GetEffectiveAccess`, which still swallows any other error of the - remote initialization (not reproducible: for a user who isn't an - administrator of the file server, the cmdlet writes "Access is denied"); - the unchecked `AUTHZ_ACCESS_REPLY.Error`; a fallback warning that names - the server and the error, which would change behavior (Decision 16). -- #34: no report from the tester by 16:00 UTC on 2026-10-07; he announced - results against two file servers, one of them IBM ESS, for that day. -- The lab keeps the accounts, the share, and the folders of the last run; - `Invoke-NTFSSecurityLabTest.ps1 -RemoveFixture` removes them. +- 2026-10-08, Phase 2 on `ai/release-5.0.0-rc6`, 31 commits on `fcb370e` + (5.0.0-rc5); the candidate is `7b0781f`: + - The suite has 684 tests. Elevated: 662 passed and 22 skipped in + Windows PowerShell 5.1, 632 and 52 in PowerShell 7. As a basic user + through `Invoke-TestsAsBasicUser.ps1`: 590 and 94, 560 and 124. No + failure, and no test is skipped in all four configurations; CI runs + all four since this branch. + - C# coverage of all four configurations (AltCover without `--save`, + `techContext.md`): 68.1% of the lines and 44.3% of the branches on + `1b9edbb`; rc5 58.1% and 38.0%. The earlier figures counted one of + the four runs. Of the 972 points that no test ran at `e2b6b24` + (without the classes that no cmdlet calls), 400 are code that nothing + calls, 107 defensive guards, 74 need a failure of Windows, 4 need the + lab, and 387 are reachable; 51 of those ran in runs that the first + measurement missed, and the top items of the rest are in + `progress.md`, open work 7. + - Fixed test-first: `Get-NTFSSimpleAccess` (`ReadData`, the parent of a + relative path); `Copy-Item2` and `Move-Item2` (a folder at the + destination, the missing destination folder of #21, also with + `-WhatIf`); the hard-link cmdlets on shares and at folders; + `Set-NTFSSecurityDescriptor -PassThru` (R5); the error ID of + `Get-NTFSOrphanedAccess`; relative paths that start with a dot, which + every cmdlet shortened by two characters (`Remove-Item2 .x` removed + another item); comparing entries and descriptors + (`InvalidCastException`, also `Compare-Object` in PowerShell 7) and + their conversions; `InheritedFrom` with `-ExcludeExplicit` and for a + descriptor with audit entries (`ArgumentOutOfRangeException`). + `Copy-Item2` no longer creates the missing folders of a destination + for a folder (assumption, flagged for the maintainer). + - Live tests: cases 4b to 9 and accounts of `b.forest1.net`, + `forest2.net`, and `forest3.net`. The lab acceptance passed for + `acfe3af`, `1b9edbb`, and `7b0781f` (326 tests each, none failed; + `Tests/Lab/Acceptance-2026-10-08-5.0.0-rc6.md`). Baseline: the + published 5.0.0-rc5 fails only the two hard-link tests of case 8. The + fixture was removed and the removal checked after each run; three + checkpoints `ntfs-rc6-*-before-acceptance` stay on six machines. + - `Get-NTFSEffectiveAccess -ServerName` needs administrators or + Access Control Assistance Operators on the named computer (lab probe, + documented). + - Two `security-reviewer` passes approved the branch with no Blocker or + Major finding: `fcb370e..e2b6b24` (findings 1, 4, 5, 10 fixed in + `acfe3af`) and `e2b6b24..1b9edbb` (findings 1, 2, 6, 7 fixed in + `7b0781f`). Not fixed: a failed privilege disable isn't tried again, + and a non-qualified ACE could shift `InheritedFrom` (neither + reproducible); `New-NTFSHardLink` stays terminating for a folder (a + behavior change for the maintainer). +- #34: no reply from the tester since 2026-10-06. ## Next step -The maintainer runs the push and pull request commands of the session of -2026-10-07, merges, and tags `5.0.0-rc5`. Then check the published package -with `Invoke-NTFSSecurityLabTest.ps1 -Version 5.0.0-rc5`, wait for the #34 -feedback, and release 5.0.0 as `progress.md` describes. +1. The maintainer pushes the branch, opens the pull request, merges it + after CI, and tags `5.0.0-rc6`; then the live tests run against the + published package (`-Version 5.0.0-rc6`). The first CI run is the first + run of `Invoke-TestsAsBasicUser.ps1` on a GitHub runner. +2. The maintainer decides the behavior changes in `progress.md`, open + work 4 (Decision 16), and the scope of Phase 3: the operating systems, + the code that nothing calls, and file servers that aren't Windows + (#34). diff --git a/.memory-bank/decisions/0021-quality-gate-before-5.0.0.md b/.memory-bank/decisions/0021-quality-gate-before-5.0.0.md new file mode 100644 index 0000000..5664903 --- /dev/null +++ b/.memory-bank/decisions/0021-quality-gate-before-5.0.0.md @@ -0,0 +1,37 @@ +--- +status: accepted +date: 2026-10-08 +last-verified: 2026-10-08 +owner: shared +source: maintainer decision of 2026-10-08 +--- + +# Decision 21: A quality gate before 5.0.0 + +- Choice: 5.0.0 ships only at the highest quality, with everything tested + (maintainer, 2026-10-08). The gate has three phases: + 1. Measure 5.0.0-rc5: done on 2026-10-08 (`progress.md`). + 2. Add tests until every code path is tested or explained, live tests + for the remaining cmdlets, and test-first fixes of the known defects; + release them as 5.0.0-rc6. The maintainer approved it on 2026-10-08. + 3. Run the live tests on more operating systems, such as a Windows 11 + client and Server 2019 and 2022 file servers, then release 5.0.0. +- Exit criteria for 5.0.0, as proposed on 2026-10-08: + - Every cmdlet and parameter set has behavior tests, error paths + included. + - No test is skipped in every configuration that runs. + - The C# coverage is measured, and every path that no test runs is + tested or explained. + - Every known defect is fixed, or accepted by the maintainer and listed + in the release notes. + - The published package passes the live tests on every operating system + of the matrix. +- Rationale: rc5 passed every test that ran, but the tests ran 55.9% of + the code lines and 37.4% of the branches; five cmdlets had no tests of + their own, and 19 cmdlets never ran over SMB. (That measurement counted + only one of the four test runs; with all four, rc5 runs 58.1% of the + lines and 38.0% of the branches, see `techContext.md`.) +- Open: behavior changes found on the way stay the maintainer's decision + (Decision 16); so do the 244 lines of classes that no cmdlet calls, the + operating systems of Phase 3, and how to cover file servers that aren't + Windows (#34). diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index e17a9d8..435d20f 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -1,6 +1,6 @@ --- status: current -last-verified: 2026-10-07 +last-verified: 2026-10-08 owner: active-agent source: repository evidence --- @@ -9,15 +9,15 @@ source: repository evidence ## Current status -5.0.0-rc4 is on the PowerShell Gallery and in the GitHub releases, -published by CI from the tag `5.0.0-rc4` on `master` (`01d9264`, the merge -of #113) on 2026-10-06 (Decision 12). It adds to 5.0.0-rc3 the fixes of the -issues #41, #108, #109, and #111 and of the leftovers of the rc3 review. -5.0.0-rc5 is prepared on the branch `ai/release-5.0.0-rc5`: the live tests -in a lab (Decision 20) and the fix of `Get-NTFSEffectiveAccess -ServerName` -that they found. The stable Gallery version is still 4.2.6. NTFSSecurity -will be archived soon; its users move to WindowsAccessControl -(Decision 18). +5.0.0-rc5 is on the PowerShell Gallery and in the GitHub releases, +published by CI on 2026-10-08 from the tag `5.0.0-rc5` on `master` +(`fcb370e`, the merge of #114; Decision 12). Phase 2 of the quality gate +(Decision 21) is complete on the local branch `ai/release-5.0.0-rc6`: the +candidate 5.0.0-rc6 (`acfe3af`) passed the suite in four configurations, +one `security-reviewer` pass, and the lab acceptance. It waits for the +maintainer to push, merge, and tag it; Phase 3 follows. The stable Gallery +version is still 4.2.6. NTFSSecurity will be archived soon; its users move +to WindowsAccessControl (Decision 18). ## Recent milestones @@ -68,6 +68,38 @@ will be archived soon; its users move to WindowsAccessControl to fix it test-first in 5.0.0-rc5; the branch build passes all live tests and the suite. One `security-reviewer` pass approved it with minor findings (`activeContext.md`). +- 2026-10-08: #114 merged (`fcb370e`); the tag `5.0.0-rc5` published it to + the Gallery and the GitHub releases, whose `NTFSSecurity.zip` holds the + same 11 files. Phase 1 of the quality gate (Decision 21) measured rc5: + the published package passes the live tests in both editions; the 11 + tests that need a session without the Security privilege pass as a basic + user, so every test runs in at least one configuration, but CI runs only + elevated; the suite runs 55.9% of the C# lines and 37.4% of the branches. + The maintainer approved Phase 2. +- 2026-10-08: Phase 2, step 1 on `ai/release-5.0.0-rc6` (local): tests for + `Set-NTFSOwner`, `Test-Path2`, `Get-DiskSpace`, and the link cmdlets + (suite: 555 tests). Fixed test-first: the privileges stayed enabled after + an early stop; `Test-Path2` stopped for invalid characters in Windows + PowerShell; and, from one `security-reviewer` pass, the privilege cleanup + decided on stale states, a defect since 4.2.6. The page of + `New-NTFSSymbolicLink` was corrected after a lab check of Developer Mode. +- 2026-10-08: Phase 2 finished on `ai/release-5.0.0-rc6` (local). Tests for + `Get-NTFSOrphanedAudit`, `Get-NTFSSimpleAccess`, the + `-SecurityDescriptor` parameter sets, the error contracts of all path + cmdlets, and #110. Fixed test-first: `Get-NTFSSimpleAccess` (`ReadData`, + relative paths), `Copy-Item2` and `Move-Item2` (folder conflicts, the + missing destination folder of #21), the hard-link cmdlets on shares, + `Set-NTFSSecurityDescriptor -PassThru` (R5), and the error ID of + `Get-NTFSOrphanedAccess`; from the coverage report, relative paths that + start with a dot (every cmdlet acted on the item without the first two + characters), comparing output objects (`InvalidCastException`), and + `InheritedFrom`. CI runs the suite as a basic user too; the live tests + cover all cmdlet groups and accounts of three more domains. Suite: 677 + tests, none failed, none skipped in every configuration; C# coverage + 68.1% of the lines and 44.3% of the branches (rc5: 58.1% and 38.0%, + measured again; the first measurements counted one of four runs). Two + `security-reviewer` passes; the lab acceptance of `7b0781f` passed + (`Tests/Lab/Acceptance-2026-10-08-5.0.0-rc6.md`). ## Stable capabilities @@ -82,15 +114,20 @@ will be archived soon; its users move to WindowsAccessControl ## Open work -1. Publish 5.0.0-rc5 (merge, then tag), check its package in the lab with - `Tests\Lab\Invoke-NTFSSecurityLabTest.ps1 -Version 5.0.0-rc5`, and then - release 5.0.0 through CI (Decision 12) with the tester feedback in #34: - remove the label, date `[Unreleased]` as `[5.0.0]`, add `5.0.0-rc5` to - `$publishedVersions`, and tag `5.0.0` (steps in - `Docs/Contributing/05-Releasing.md`). #34 stays open with Bug and Help - Wanted until a tester with a file server that refuses the owner - confirms the fix, or until 5.0.0 ships. -2. Issues: #110 (tests) is the open follow-up of the review findings; #68 +1. Quality gate before 5.0.0 (Decision 21): Phase 2 is done on the branch + `ai/release-5.0.0-rc6` (`activeContext.md`); the maintainer pushes it, + merges the pull request, and tags 5.0.0-rc6, and the live tests run + against the published package. Phase 3 runs the live tests on more + operating systems. Then release + 5.0.0 through CI (Decision 12): remove the label, date `[Unreleased]` as + `[5.0.0]`, add the last prerelease to `$publishedVersions`, and tag + `5.0.0` (steps in `Docs/Contributing/05-Releasing.md`). #34 stays open + with Bug and Help Wanted until a tester with a file server that refuses + the owner confirms the fix, or until 5.0.0 ships. +2. Issues: the rc6 branch addresses the seven items of #110 (tests); the + pull request names it without a closing keyword, so the maintainer + closes it after the merge. #21 (a misleading error of `Move-Item2`) got + a fix in rc6 that names the missing destination folder. #68 tracks `-WhatIf` and `-Confirm` for every cmdlet that changes security. The labels follow Decision 17; #16, #21, #45, and #89 wait for their reporters (Needs Info). Not planned for 5.0.0: the enhancements #22, @@ -104,11 +141,51 @@ will be archived soon; its users move to WindowsAccessControl `AUTHZ_ACCESS_REPLY.Error`, a fallback warning without the server name, and hardening of the lab controller (guards in the setup blocks, interpolated `-EncodedCommand` paths, CredSSP by IP address, the - password string in memory, disabling the role accounts after a run). -4. `pwsh` 7.6.1 crashed three times during test runs on the ARM64 + password string in memory, disabling the role accounts after a run); of + rc6, a privilege that fails to be disabled isn't tried again by + `Dispose` (finding 2, not reproducible). +4. Behavior changes found in Phase 2, for the maintainer (Decision 16): + `Get-NTFSOrphanedAudit` returns nothing without the Security privilege, + while `Get-NTFSAudit` writes `ReadSecurityError`; + `Get-NTFSSimpleAccess` skips a folder whose parent it didn't process; + `New-NTFSSymbolicLink` could create links without the privilege in + Developer Mode (flag `SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE`, with + a fallback before Windows 10 1703); `-WhatIf` names a conflict in a + verbose message instead of a warning (R7); the fallback warning of + `Get-NTFSEffectiveAccess` doesn't name the server; `Move-Item2` can't + move a folder to another volume; `New-NTFSHardLink` stops with a + terminating error for a folder, unlike `Get-NTFSHardLink` (rc6 review, + finding 11). Taken as an assumption in rc6, for review: `Copy-Item2` + no longer creates the missing folders of a destination for a folder. + Found by the coverage report of rc6: `-Target` of the link cmdlets is + optional and means the current location when it's omitted; equality of + entries and descriptors is the identity of the wrapped .NET object, so + two reads of the same entry differ for `Compare-Object` and + `Select-Object -Unique` (value equality would be a behavior change); 400 + points of code that nothing calls besides the 244 lines of unused + classes (Phase 3). +5. `pwsh` 7.6.1 crashed three times during test runs on the ARM64 workstation (x64 emulation), without module frames; none of the CI runs on native x64 on 2026-10-05 crashed. -5. Optional for the maintainer: delete the AppVeyor project and revoke its +6. Optional for the maintainer: delete the AppVeyor project and revoke its GitHub authorization, restrict wiki editing to collaborators, ask `Sup3rlativ3` to delete the Read the Docs project, and delete the branch - `test/transfer`. + `test/transfer`. In the lab, delete the checkpoints + `ntfs-rc6-*-before-acceptance` of the six machines when they are no + longer needed. +7. Reachable code that no test runs (coverage report of rc6, ranked by + impact; about 300 points): `Remove-Item2` on folders (`-Recurse`, + `-Force`, `DeleteError`); the owner restore after taking ownership + (`RestoreOwnerError`); the inheritance cmdlets on folders and + `Set-NTFSInheritance -AccessInheritanceEnabled $true`; the mapping of + all 13 `-AppliesTo` values and the flag parameters of + `Remove-NTFSAccess`, `Add-NTFSAudit`, and `Remove-NTFSAudit`; the + switches and errors of `Get-ChildItem2`; the table views and + `InheritedFrom` in them; `Move-Item2 -Force`; account input errors; + `-PassThru` after success of the audit and inheritance cmdlets; + `Set-NTFSSecurityDescriptor` failures; the audit cmdlets without the + Security privilege on a local item; `Get-NTFSEffectiveAccess` for an + unresolvable SID; failed ownership retries of `Clear-NTFSAccess` and + `Set-NTFSInheritance`. A display limit, not a defect: a conditional ACE + shows as an unconditional entry, because the .NET rules have no + condition. diff --git a/.memory-bank/systemPatterns.md b/.memory-bank/systemPatterns.md index c28e09b..5b9208e 100644 --- a/.memory-bank/systemPatterns.md +++ b/.memory-bank/systemPatterns.md @@ -1,6 +1,6 @@ --- status: current -last-verified: 2026-10-07 +last-verified: 2026-10-08 owner: active-agent source: repository evidence --- @@ -35,7 +35,15 @@ NTFSSecurity.dll ── cmdlets ──> Security2.dll (FileSystemAccessRule2, ownership and restores the previous owner on every exit path. - `BaseCmdletWithPrivControl` enables Backup, Restore, TakeOwnership, and Security in `BeginProcessing` when `PrivateData.EnablePrivileges` is - `$true`, and disables the ones it enabled in `EndProcessing`. + `$true`, and disables the ones it enabled in `EndProcessing` and, since + 5.0.0-rc6, in `Dispose`: PowerShell skips `EndProcessing` when a later + command, such as `Select-Object -First`, or a terminating error stops the + pipeline, but calls `Dispose`. `Enable-Privileges` keeps them + (`KeepEnabledPrivileges`). The cleanup reads the current state of each + privilege, because another command in the pipeline can have changed it, + and tries every privilege even when one fails: `EndProcessing` warns, + `Dispose` stays silent, because PowerShell ignores exceptions thrown + there and no stream is open anymore. - `PrivateData` switches: `EnablePrivileges`, `GetInheritedFrom`, `GetFileSystemModeProperty`, `IdentifyHardLinks`, `ShowAccountSid`. - Cmdlets accept `-Path` (alias `FullName`) or `-SecurityDescriptor`; the @@ -67,6 +75,7 @@ Each Decision record is a file in `decisions/`; read only the relevant ones. | 18 | [NTFSSecurity will be archived](decisions/0018-archive-for-windowsaccesscontrol.md) | | 19 | [Cmdlets write only the sections that they change](decisions/0019-write-only-changed-sections.md) | | 20 | [Live tests in a lab live in Tests\Lab](decisions/0020-live-tests-in-tests-lab.md) | +| 21 | [A quality gate before 5.0.0](decisions/0021-quality-gate-before-5.0.0.md) | ## Patterns @@ -90,9 +99,20 @@ Each Decision record is a file in `decisions/`; read only the relevant ones. - A test that changes files, links, or security descriptors uses `Tests\TestHelpers.psm1`: its own sandbox, `Assert-TestSandboxPath` before each change, `Remove-TestSandbox`. Cases that need a privilege - skip with `Test-PrivilegeHeld` and run in CI (elevated). `Block-Test*` - make a read or a write fail without elevation; `Set-TestOwner` with + skip with `Test-PrivilegeHeld`, cases that need its absence skip when + elevated; CI runs the suite elevated and as a basic user in both + editions, so each case runs somewhere. `Block-Test*` make a read or a + write fail without elevation; `Set-TestOwner` with `EnablePrivileges = $false` reproduces an owner the user can't assign. +- Fixtures write a DACL with `SetAccessControl`, never with `Set-Acl`: + `Set-Acl` compares `AreAuditRulesProtected` of the new descriptor with + `AreAccessRulesProtected` of the item (`FileSystemSecurity.cs` of + PowerShell), so for an item with a protected DACL it writes the audit + section too. Without the Security privilege that fails with + `PrivilegeNotHeldException`; with it, `Set-Acl` writes every section and + drops the audit entries. Windows PowerShell has + `FileInfo`/`DirectoryInfo.SetAccessControl`; PowerShell 7 has + `[System.IO.FileSystemAclExtensions]::SetAccessControl`. - `Get-Help -Online` tests run only in Windows PowerShell, which honors the hook `BypassOnlineHelpRetrieval`. `Manifest.Tests.ps1` and `Release.Tests.ps1` check the manifest, the version (Decision 10), the diff --git a/.memory-bank/techContext.md b/.memory-bank/techContext.md index 625f722..9b949e1 100644 --- a/.memory-bank/techContext.md +++ b/.memory-bank/techContext.md @@ -1,6 +1,6 @@ --- status: current -last-verified: 2026-10-07 +last-verified: 2026-10-08 owner: active-agent source: repository evidence --- @@ -72,20 +72,24 @@ source: repository evidence - The third workstation (`ExHost`, a Windows Server 2025 VM, x64, used since 2026-10-07) runs the agent session elevated and hosts the AutomatedLab lab `WindowsAccessControlLab` (Decision 20) with Hyper-V and AutomatedLab - 5.61.704. It has no NuGet cache, platyPS, or GitHub CLI: check each + 5.61.704. It has no NuGet cache or platyPS: check each nuget.org package against the SHA-512 `packageHash` of its catalog entry (`https://api.nuget.org/v3/registration5-semver1//.json`, then `catalogEntry`), and each Gallery package against `PackageHash` of `api/v2/Packages(Id='',Version='')`. Pester 5.7.1 is in - `V:\Git\WindowsAccessControl\output\RequiredModules`; read issues and pull - requests through the GitHub REST API. The lab domains `a.forest1.net` and - `b.forest1.net` had a maximum password age of 42 days, so the password of - `install` expired on 2026-09-15 and AutomatedLab got access denied; it - never expires since 2026-10-07, as in `forest1.net`. + `V:\Git\WindowsAccessControl\output\RequiredModules`. The GitHub CLI + 2.102.0 is in `C:\Program Files\GitHub CLI`, outside the PATH, and signed + in as `raandree` since 2026-10-08; `Block-RemoteMutation` denies its + mutating commands, so the agent uses it read-only. The lab domains + `a.forest1.net` and `b.forest1.net` had a maximum password age of 42 + days, so the password of `install` expired on 2026-09-15 and AutomatedLab + got access denied; it never expires since 2026-10-07, as in + `forest1.net`. ## Constraints -- `ModuleVersion` is `5.0.0` with the prerelease label `rc5`. +- `ModuleVersion` is `5.0.0` with the prerelease label `rc6` on the branch + `ai/release-5.0.0-rc6` (`rc5` on `master`). The latest stable tag and Gallery release is `4.2.6`. The manifest requires PowerShell 5.1 and .NET Framework 4.5.2, uses `RootModule`, and lists exactly 36 cmdlets; `Test-ModuleManifest` passes in Windows @@ -96,7 +100,8 @@ source: repository evidence - PowerShell Gallery versions (publish dates): 4.0.0 (2015-08-19), 4.2.2 (2016-05-18), 4.2.3 (2016-05-19), 4.2.4 (2018-08-13), 4.2.5 (2019-07-11), 4.2.6 (2019-07-12), none with release notes; 5.0.0-rc1 (2026-10-04), - 5.0.0-rc2 (2026-10-05), 5.0.0-rc3 and 5.0.0-rc4 (2026-10-06), published + 5.0.0-rc2 (2026-10-05), 5.0.0-rc3 and 5.0.0-rc4 (2026-10-06), 5.0.0-rc5 + (2026-10-08), published by CI. Older versions were released on CodePlex only, and their dates are lost. The git history starts on 2016-10-10, when the project moved from CodePlex. @@ -139,7 +144,9 @@ source: repository evidence then: 01 `Update-MarkdownHelp` and fail on `git diff -- Docs/Cmdlets`; 02 `Get-MarkdownLink -BrokenOnly`; 03 regenerate the help file and fail on `git status --porcelain -- NTFSSecurity/en-US`; 04 `Invoke-Tests.ps1` in - Windows PowerShell 5.1 and in PowerShell 7. Job `wiki` on `ubuntu-latest` + Windows PowerShell 5.1 and in PowerShell 7, then + `Invoke-TestsAsBasicUser.ps1` in both editions (since 5.0.0-rc6). Job + `wiki` on `ubuntu-latest` (read-only) clones the wiki (`gh auth setup-git` with the built-in token), runs `Export-WikiContent.ps1`, and lists the changed pages in the job summary; job `publish-wiki` (`contents: write`) repeats that and publishes, @@ -159,7 +166,8 @@ source: repository evidence ci.yml`, `gh pr checks `, and `gh run view --log-failed` (read-only). - Workflow lint: actionlint (download the release zip into `$env:TEMP` and - check its SHA-256 against the checksum file); PowerShell steps check + check its SHA-256 against the checksum file; 1.7.12 on 2026-10-08); + PowerShell steps check `$LASTEXITCODE` after every native command, because GitHub checks only the last one. - Run platyPS in Windows PowerShell 5.1 to avoid PowerShell 7.4+ @@ -171,16 +179,61 @@ source: repository evidence PowerShell 5.1: the launcher starts `pwsh`, and its payload runs `powershell.exe -NoProfile -EncodedCommand` with Pester imported by full path. A run without `bin\Release\en-US` must fail. -- Tests that run only without a privilege skip in CI and in an elevated - session. Run them as a basic user with `runas /trustlevel:0x20000`, and - give Windows PowerShell its own `PSModulePath`; that token holds one - privilege, so the `Enable-Privileges -PassThru` count test fails there. +- Tests that run only without a privilege skip in an elevated session. + `.github\scripts\Invoke-TestsAsBasicUser.ps1` runs the suite from an + elevated session with a token of the SAFER level Normal User, like + `runas /trustlevel:0x20000`, and CI runs it in both editions. For a + single file, `runas /trustlevel:0x20000` works too; give Windows + PowerShell its own `PSModulePath`, and note that `runas` returns at once, + so the script it starts writes its own log. Both tokens hold only the + privilege to bypass traverse checking. + Pester reports a skipped `-ForEach` test under its template name, such as + `<_> should ...`, and a test that ran under the expanded name: compare + runs by template. +- C# coverage (Decision 21): AltCover 9.0.145 (`tools\net472\AltCover.exe` + of the nuget.org package) instruments a copy of the local Release build, + which has the PDB files that the published package lacks: + `--reportFormat=OpenCover`, AlphaFS and `System.Management.Automation` + excluded with `--assemblyFilter`, and no `--save`: then every process + writes its hits into the report when it exits. With `--save`, each + process writes a recorder file, and `runner --collect` keeps only the + first one (verified 2026-10-08), so the numbers measured that way held + only the main process of the elevated Windows PowerShell run. Put the + instrumented module in `NTFSSecurity\bin\Release` of a `git worktree`, + run `.github\scripts\Invoke-Tests.ps1` elevated and + `Invoke-TestsAsBasicUser.ps1` in both editions, then + `AltCover.exe runner --collect --recorderDirectory=`, which recalculates the summary of the report from the hits. + All four configurations, 2026-10-08: the rc5 tree 58.1% of the lines + (2,020 of 3,476) and 38.0% of the branches (711 of 1,873), 62.5% without + 244 lines in classes that no cmdlet calls; the rc6 candidate (`1b9edbb`) + 68.1% of the lines (2,412 of 3,540) and 44.3% of the branches (850 of + 1,918), 73.2% without those classes, the `NTFSSecurity` assembly 78.1%. + The earlier figures, 55.9% for rc5 and 65.6% for rc6, used `--save`. - Live tests (Decision 20): in an elevated Windows PowerShell 5.1 session on the lab host, `Tests\Lab\Invoke-NTFSSecurityLabTest.ps1` with `-Version` for Gallery packages or `-ModulePath` for a build; it writes the results to `$env:TEMP\NTFSSecurityLab\Results`. A run of two versions in both editions takes about 30 minutes; `-RemoveFixture` removes its - accounts, share, and folders from the lab. + accounts, share, and folders from the lab. For a check on the client as + an account without administrator rights, use `NtfsLiveServerAdmin` + (Remote Management Users on the client, CredSSP by IP address like the + controller): reset its password on the PDC emulator to a random value + in memory; the next run of the controller sets a new one anyway. +- Lab acceptance of a candidate (modeled on the WindowsAccessControl + handoff 07): build once, package it with `New-ModulePackage.ps1`, and + record the SHA-256 of the packages and module files; check WinRM, LDAP + (RootDSE), Kerberos (`klist get`), the secure channel, and the clock of + the six VMs; take a Production checkpoint named + `ntfs-