Before the move, I audited Find-UnknownSID’s documentation against the script and found most of it describing a tool that didn’t exist. That fixed the docs for the script. Then the script became a module the same day, and the documentation had to follow it again.

It took four more passes. The first was a rename. The other three each found code that didn’t do what the guide said, and in most of those cases the code was wrong, not the guide.

When I was writing the docs, I was trying to be as documented as possible for a security team who didn’t bat an eyelash if I said I had to run ad hoc fixes, but a weekly automation suddenly drew the highest level of scrutiny. My end goal was a smug ‘RTFM’.

Renaming the guides

The conversion commit’s documentation pass touched 22 files. It swapped the script’s name for the module’s, pointed recipes at Import-Module instead of dot-sourcing private files, renamed three guides that had Find-UnknownSID in their file names, and deleted the sections about the class-hash layer that no longer existed.

The rename was a fair amount of work, and it left most of the guides wrong. They read like a module and still described the script underneath. Before tagging 1.0.0, a second pass checked every claim against the code. Plenty of the script was still in there:

  • -Remove and -Restore switches, now separate commands
  • exit codes, which a module doesn’t return
  • a ShouldContinue prompt with a “Refusing to … without confirmation” message, where the commands use ConfirmImpact = 'High'
  • no pipeline output, from a script that only wrote CSV
  • a backup folder under the script’s folder
  • private helpers called as if they were exported

The diagnostic snippets in the troubleshooting guides were the most broken. Each one called private functions at the prompt. Once those functions live inside a module, you can’t reach them that way. The snippets now run in module scope:

1

& (Get-Module OrphanedSid) { Get-SIDValidationCacheStat }

Ranges that didn’t agree

Checking the documented parameter ranges against the code found the first real defect. -MaxMemoryUsageMB on the public commands accepts 100 to 16384, and the configuration file accepts a MemoryCheckInterval of 1 to 1000. Further down, Initialize-MemoryManager had its own idea:

1
2
3
4
5

[ValidateRange(256, 16384)]
[int]$MaxMemoryMB,
# ...
[ValidateRange(5, 100)]
[int]$CheckInterval,

If you passed -MaxMemoryUsageMB 200, it got through the command’s validation, passed the configuration check, and then failed when the memory manager started. So did a configured interval of 500. The configuration loader capped the interval at 100 as well. All three now take the ranges the docs list, and the memory manager says why:

1
2
3
4
5

# The same ranges the commands and the configuration file accept. This function once took
# 256-16384 and 5-100, so -MaxMemoryUsageMB 100-255 and a configured interval outside 5-100
# passed every earlier check and then failed initialization.
[ValidateRange(100, 16384)]
[int]$MaxMemoryMB,

The same pass caught one more -WhatIf leak, after the ones the live run found. Invoke-MainProcessingLogic ran a cleanup every 100 objects without pinning -WhatIf:$false, so a large rehearsal printed a What if line for memory cleanup. It’s pinned like the rest now.

One more live run before the tag

Those fixes went in, the unit tests passed, and the pass was ready to merge. Before tagging 1.0.0 I wanted the merged main run against the lab DC once more, installed from the same staging folder the publish script builds. Discovery found both seeded orphans, the -WhatIf removal previewed one entry each and created no backup folder, the real removal wrote two backups, discovery found 0, restore put both back, and discovery found 2 again.

I hate tagging something and then immediately finding new improvements. I try to make sure it’s squeaky clean at that time, and a live run helps with that.

That run predates the three range and -WhatIf fixes, so only unit tests stand behind those. I knew that going in.

Describe what the code does, then file the gap

The pass after 1.0.1 started with the same goal and ended up with a rule. Every claim in the README, the command help, about_OrphanedSid, Troubleshooting/ and Documentation/ was checked against the code. Snippets were run as written. Anything that depended on Active Directory was tested on the lab DC.

When a guide described what the module was supposed to do, and the module did something else, the guide changed to describe what it does, and the difference became an issue. That pass filed #39 through #55. The first two are the worst bugs in this series so far:

  • #39: the deleted-objects lookup also returns live objects, so a live principal whose normal lookup failed could be reported orphaned and removed.
  • #40: removing two orphaned SIDs from one object wrote two backups, and a restore brought back only one.

Neither was a documentation problem. The guides described the behavior the module should’ve had, and checking them on a real domain controller showed it didn’t. Both got fixed for 1.1.0.

Some of what the DC turned up was smaller. The scheduled task example registered the task like this:

1

Register-ScheduledTask -TaskName 'OrphanedSid Audit' -Action $action -Trigger $trigger -User 'CONTOSO\svc-adsecurity' -RunLevel Highest

With -User and no -Password, Task Scheduler registers an interactive logon type, and the task only runs while that account is logged in. For a nightly audit, that’s never. The example uses a group managed service account now:

1
2
3

# A group managed service account runs logged off without a stored password
$principal = New-ScheduledTaskPrincipal -UserId 'CONTOSO\svc-adsecurity$' -LogonType Password -RunLevel Highest
Register-ScheduledTask -TaskName 'OrphanedSid Audit' -Action $action -Trigger $trigger -Principal $principal

Advice that couldn’t work

The pass before 1.1.0 went over everything again, against the code on main. Most of it was right by then. The interesting finds were pieces of advice that read fine and couldn’t work.

Memory-Security.md told you how to unblock a protected SID that was orphaned:

1
2
3

2. If a SID in the default `ProtectedSIDs` list is genuinely orphaned in this forest, supply a
   configuration file with a `ProtectedSIDs` array that omits it, and keep that file under change
   control.

All six default protected SIDs are also built-in well-known SIDs, and the well-known check runs whatever the configuration file says. If you leave one out of the file, nothing changes. The guide says so now.

The SIEM section of the integration guide told you to log straight into the Splunk forwarder’s folder:

1

$logPath = 'C:\Program Files\SplunkUniversalForwarder\var\log\splunk\OrphanedSid.log'

The module refuses any log path containing Program Files, Windows or system32, so if you followed that example, you got no log at all. It points at D:\SIEM\OrphanedSid\ now, with a sentence about the refused folders.

If you size a jump host from the guides, their memory and result-size figures were estimates, and they’re measurements from the lab DC now. A Windows PowerShell 5.1 session with the ActiveDirectory module and OrphanedSid loaded had been “commonly 150 to 300 MB”, and measured about 105. A streamed result had been “a few hundred bytes”, and measured about 1.5 KB of JSON.

That pass also found #90, where a configuration file’s LogFilePath overrode -LogPath partway through a run and split one run’s log across two files. It went in as a bug and got fixed.

Help that pointed nowhere

The last documentation fix before 1.1.0 was in the private functions’ comment-based help. Most of them ended with troubleshooting pointers like these, from Invoke-MemoryMonitoring.ps1:

1
2

- For monitoring issues: .\Troubleshooting\Performance\Memory-Monitoring.md
- For threshold problems: .\Troubleshooting\Performance\Memory-Thresholds.md

Neither guide exists. Across the module there were 116 references to 72 guides that were never written. The help had been filled in from a template, and the template assumed a guide per topic.

Each reference now names a guide that exists. To keep it that way, the contract test from the live-run post gained a check that resolves every reference in the shipping files:

1
2
3
4
5
6
7
8
9

$missing = foreach ($match in ($shippingFiles | Select-String -Pattern '\.\\Troubleshooting\\[^\s''"`)]+\.md' -AllMatches)) {
    foreach ($reference in $match.Matches.Value) {
        # The references are written with backslashes; the suite also runs on Linux
        $relative = $reference.Substring(2).Replace('\', [System.IO.Path]::DirectorySeparatorChar)
        if (-not (Test-Path -LiteralPath (Join-Path $script:ModuleRoot $relative) -PathType Leaf)) {
            "$($match.Filename):$($match.LineNumber) $reference"
        }
    }
}

With the old references put back, it fails and lists each dangling one by file and line.

Documentation/Archive/ was left out of every pass. It records how the project got here, so its documents describe earlier versions. Its README now says so, and that’s the only change it got.