Skip to content

docs(plan): remove §17 + §18 steps 9-10 (no lab DC available)#17

Merged
chaospheremk merged 8 commits into
mainfrom
dev
May 8, 2026
Merged

docs(plan): remove §17 + §18 steps 9-10 (no lab DC available)#17
chaospheremk merged 8 commits into
mainfrom
dev

Conversation

@chaospheremk

Copy link
Copy Markdown
Owner

Scope reduction: plan §17 (Validation / Smoke Tests) and §18 steps 9-10
(lab smoke run + 30k-object perf pass) all required a live lab DC that
this project does not have and has no path to. Removing them rather
than carrying them as permanent "deferred" debt that misleads readers
about coverage.

§17 rewritten to enumerate what the per-phase Pester suites cover at
the helper-function level, and to call out explicitly what
real-directory behaviour is uncovered:

  • Schema-default ACE handling on a fresh user
  • DACL_PROTECTED + Convert anomaly absence
  • Transitive expansion correctness on real nested groups
  • Property-set name resolution against a real PropertySetMembersMap
  • Throughput on a 30k-object domain

The script behaviour on those cases is asserted by unit tests against
fabricated SDs only. Operators running this against a production
directory should treat the first run as exploratory and review the
JSONL log for BatchError / PARSE_ERROR / OrphanSid /
InheritedAceOnProtectedDacl events before acting on the CSV output.

Step 8 (Phase 6 pivot CSV writer) is now the final implementation
step.

Files

  • docs/AD-Permissions-Analyzer-Plan.md — §17 + §18 rewritten
  • CHANGELOG.md[Unreleased] / Changed entry recording the scope
    reduction
  • Tests/Phase6-Output.Tests.ps1 — reconciliation test re-justified
    without the §17 reference; test logic unchanged

Vault-side

  • decisions.md — ADR-025 with rationale, uncovered callouts, and
    reversibility note
  • issues.md — Step 9/10 struck through with ADR-025 ref; "Next
    session" rewritten as "implementation complete"

Verification

Invoke-Build Build clean: 144 passed, 1 skipped, 0 lint findings.

dougjohnson-me and others added 8 commits May 7, 2026 09:36
…an §18.6)

- New-AceIndex: composite-key dictionary over EXPLICIT rows only; skips
  inherited, Synthetic.Owner (-1), and PARSE_ERROR (-2). DN keys normalised
  to ToUpperInvariant for OI tuple equality.
- Get-ParentDistinguishedName: char-by-char tokenizer respecting LDAP
  escapes (\, \\ \HH); returns $null at NC root.
- Test-IsContainerClass: small heuristic over structural classes.
- Test-InheritanceFlagsPropagateTo: pure rule over AceFlagsRaw byte +
  InheritedObjectTypeName + descendant class + IsDirectChild. Encodes
  ContainerInherit/ObjectInherit container-vs-leaf gating, NoPropagateInherit
  level-1-only, InheritOnly transparent for descendants, InheritedObjectType
  class filter via OrdinalIgnoreCase string equality.
- Resolve-InheritanceSource: mutates $aceRecords in place — adds
  InheritanceSourceDN/Note columns on every row uniformly (ADR-013).
  DACL_PROTECTED short-circuits to InconsistentProtectedDacl + emits
  anomaly into -ProtectedDaclAnomalies sink. Otherwise walks parent
  chain, direct-lookup at each ancestor, first matching candidate wins;
  falls back to SchemaDefaultOrUnresolved. Stops at NC root.

Wiring: Invoke-ADPermissionAnalysis.ps1 runs Phase 5 single-threaded
after Phase 4 PhaseEnd. Anomalies forwarded to Write-LogEvent at WARN
with BatchError event name AND added to $ErrorBag for exit-code-2.
PhaseEnd data: indexed/inheritedTotal/resolved/unresolved/protectedDacl.

Tests: 24 cases covering New-AceIndex, Get-ParentDistinguishedName,
Test-InheritanceFlagsPropagateTo (all 5 rule branches), and
Resolve-InheritanceSource end-to-end (direct parent, level-2 walk,
DACL_PROTECTED short-circuit, SchemaDefaultOrUnresolved, explicit
rows un-mutated, Synthetic.Owner / PARSE_ERROR rows still get columns).
Invoke-Build Build clean: 112 passed, 1 skipped, 0 lint findings.

Smoke-test scenario covered: §17 #3.

ADRs added: 013 (in-place mutation), 014 (DN parser escape handling),
015 (AceFlagsRaw not InheritanceFlags string), 016 (anomaly sink +
WARN log + ErrorBag), 017 (DN normalization for composite key).
… §18.7)

Phase6-Output.ps1: New-CsvFieldEscaper (RFC-4180), Write-CsvHeader,
ConvertTo-DetailRow (pure 30-field transform per plan §11),
Get-EffectiveTrusteeRecord (ACE × effective-trustee fan-out),
Resolve-NamingContextLabel (longest-suffix DN match, memoised),
Update-PivotStat (per-row counter increments), Write-DetailCsv
(StreamWriter UTF-8 no-BOM, AutoFlush off, ProgressCallback).

Wired into Invoke-ADPermissionAnalysis.ps1: PhaseStart, Phase6Progress
every 50 000 rows via callback → Write-LogEvent, PhaseEnd with
detailRowCount + distinctTrustees + elapsedMs. $script:PivotStats
persists for Step 8.

Tests: 16 cases — escaper triggers, ConvertTo-DetailRow column
order/Synthetic.Owner/PARSE_ERROR/inherited, Get-EffectiveTrusteeRecord
direct/group-fan-out/terminal-skip/cache-miss, Write-DetailCsv
header+body Import-Csv round-trip with embedded comma + double-quote,
PivotStats counters across direct+group-expanded fixture.

Build: 128 passed, 1 skipped, 0 lint findings.

ADR-018..021: StreamWriter buffering, escape boundary, pivot shape,
fan-out tuple shape + progress accounting.
Adds Format-RightsSummary, Format-NamingContextsTouched,
Format-ObjectClassesTouched, ConvertTo-PivotRow, and Write-PivotCsv to
scripts/lib/Phase6-Output.ps1 alongside the existing detail-CSV writer.
$script:Phase6PivotColumns is the single 16-column source of truth for
the header and ConvertTo-PivotRow body (mirrors Phase6DetailColumns from
Step 7).

Sort order:
  - RightsSummary / ObjectClassesTouched: count desc, name asc tiebreak
  - NamingContextsTouched: ordinal-ignore-case ascending, ; joined
  - Pivot rows: TotalAceCount desc → trustee name asc → SID asc

Write-PivotCsv opens its own StreamWriter (UTF-8 no-BOM, AutoFlush off)
and runs after Write-DetailCsv closes — the accumulator is fully
populated by the time the detail writer returns, so concurrent writers
buy nothing (ADR-023).

Format-NamingContextsTouched parameter typed [object] not
[HashSet[string]] — PowerShell binder unrolls typed HashSet[string]
through IEnumerable, rejecting empty sets and rebinding singletons as
bare strings (ADR-024). Dictionary parameters unaffected.

Wired into Invoke-ADPermissionAnalysis.ps1 with a fresh PivotStart /
PivotEnd JSONL pair so the log distinguishes detail-write from
pivot-write.

Tests: Phase6-Output.Tests.ps1 extended 16 → 31 cases. Format-* helpers
(empty, sort, tiebreak), ConvertTo-PivotRow (16 columns, scalar counts +
three formatted summaries), Write-PivotCsv end-to-end (Import-Csv
round-trip with embedded , and ; in RightsSummary), reconciliation
(plan §17 scenario 4: sum(pivot.TotalAceCount) == Write-DetailCsv row
count).

Invoke-Build Build clean: 144 passed, 1 skipped, 0 lint findings.

Plumbing decisions: ADR-022 (sort order), ADR-023 (separate StreamWriter,
sequential after detail close), ADR-024 (HashSet[string] parameter
binder workaround).
Plan §17 (Validation / Smoke Tests) and §18 steps 9-10 (lab smoke run +
30k-object perf pass) all required a live lab DC that this project does
not have and has no path to. Removing them rather than carrying them as
permanent "deferred" debt that misleads readers about coverage.

§17 rewritten to enumerate what the per-phase Pester suites cover at the
helper-function level, and to call out explicitly what real-directory
behaviour is uncovered (schema-default ACE handling, DACL_PROTECTED +
Convert anomaly absence, transitive expansion on real groups, property-set
name resolution against a real PropertySetMembersMap, throughput on 30k
objects). The script's behaviour on those cases is asserted by unit tests
against fabricated SDs only.

Step 8 (Phase 6 pivot CSV writer) is now the final implementation step.

- docs/AD-Permissions-Analyzer-Plan.md: §17 + §18 rewritten
- CHANGELOG.md: [Unreleased] / Changed entry recording the scope change
- Tests/Phase6-Output.Tests.ps1: reconciliation test re-justified without
  the §17 reference; the test itself is unchanged

Decision: ADR-025 in vault (../decisions.md).
@chaospheremk
chaospheremk merged commit 79a6b38 into main May 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants