Add Active Directory service account cleanup capabilities #32

PrzemyslawKlys · 2025-08-07T21:24:24Z

Summary

add helper functions to filter and action AD service accounts
introduce Invoke-ADServiceAccountsCleanup entry point
cover new service account logic with Pester tests

Testing

pwsh -NoLogo -NoProfile -Command "Invoke-Pester -Path Tests"
pwsh -NoLogo -NoProfile -Command "Invoke-ModuleBuild" (fails: The term 'Invoke-ModuleBuild' is not recognized)

https://chatgpt.com/codex/tasks/task_e_689512658dfc832ea137c46cb44c1562

Copilot

Pull request overview

This PR adds Active Directory service account cleanup capabilities to the CleanupMonster module, enabling administrators to identify, disable, and delete stale or inactive managed service accounts based on various criteria like last logon date, password age, and account creation date.

Key changes:

Introduces Invoke-ADServiceAccountsCleanup as the main entry point for service account cleanup operations
Implements filtering and action logic through helper functions for retrieving, processing, disabling, and deleting service accounts
Adds comprehensive HTML reporting capabilities with detailed account status tracking

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`Public/Invoke-ADServiceAccountsCleanup.ps1`	Main entry point function with parameters for account filtering, disable/delete actions, and reporting options
`Private/Get-InitialADServiceAccounts.ps1`	Retrieves service accounts from AD domains and applies initial include/exclude filtering
`Private/Get-ADServiceAccountsToProcess.ps1`	Filters accounts based on age/inactivity criteria to determine which should be disabled or deleted
`Private/Request-ADServiceAccountsDisable.ps1`	Executes disable operations on service accounts with WhatIf support
`Private/Request-ADServiceAccountsDelete.ps1`	Executes delete operations on service accounts with WhatIf support
`Private/New-HTMLProcessedServiceAccounts.ps1`	Generates HTML reports showing processed accounts with statistics and filtering conditions
`Tests/Invoke-ADServiceAccountsCleanup.Tests.ps1`	Pester tests covering function export, filtering logic, WhatIf support, and HTML report generation
`CleanupMonster.psd1`	Adds new function to module's exported functions list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-23T16:52:01Z

Private/Request-ADServiceAccountsDisable.ps1

+        }
+        $Account.ActionDate = $Today
+        if ($WhatIfDisable.IsPresent) {
+            $Account.ActionStatus = 'WhatIf'
+        } else {
+            $Account.ActionStatus = $Success
+        }
+        $Account


[nitpick] When an account is skipped due to not matching the Action type (line 11), the function continues without outputting the account. However, if ReportOnly is false and ShouldProcess returns false (user responds "No" to the confirmation prompt), the account is not output at all. This means there's no record of accounts that were candidates but weren't processed due to user declining the action. Consider outputting the account with an appropriate ActionStatus in this case.

Suggested change

}

$Account.ActionDate = $Today

if ($WhatIfDisable.IsPresent) {

$Account.ActionStatus = 'WhatIf'

} else {

$Account.ActionStatus = $Success

}

$Account

$Account.ActionDate = $Today

if ($WhatIfDisable.IsPresent) {

$Account.ActionStatus = 'WhatIf'

} else {

$Account.ActionStatus = $Success

}

$Account

} else {

$Account.ActionDate = $Today

$Account.ActionStatus = 'SkippedByUser'

$Account

}

Copilot · 2025-11-23T16:52:02Z

Private/Request-ADServiceAccountsDelete.ps1

+        }
+        $Account.ActionDate = $Today
+        if ($WhatIfDelete.IsPresent) {
+            $Account.ActionStatus = 'WhatIf'
+        } else {
+            $Account.ActionStatus = $Success
+        }
+        $Account


[nitpick] When an account is skipped due to not matching the Action type (line 11), the function continues without outputting the account. However, if ReportOnly is false and ShouldProcess returns false (user responds "No" to the confirmation prompt), the account is not output at all. This means there's no record of accounts that were candidates but weren't processed due to user declining the action. Consider outputting the account with an appropriate ActionStatus in this case.

Suggested change

}

$Account.ActionDate = $Today

if ($WhatIfDelete.IsPresent) {

$Account.ActionStatus = 'WhatIf'

} else {

$Account.ActionStatus = $Success

}

$Account

$Account.ActionDate = $Today

if ($WhatIfDelete.IsPresent) {

$Account.ActionStatus = 'WhatIf'

} else {

$Account.ActionStatus = $Success

}

$Account

} else {

$Account.ActionDate = $Today

$Account.ActionStatus = 'Declined'

$Account

}

Copilot · 2025-11-23T16:52:02Z

Private/Get-ADServiceAccountsToProcess.ps1

+    $Today = Get-Date
+    [Array] $Output = foreach ($Account in $Accounts) {
+        foreach ($Exclude in $Exclusions) {
+            if ($Account.DistinguishedName -like $Exclude -or $Account.Name -like $Exclude) {


The exclusion logic checks if the account's DistinguishedName or Name matches the exclusion pattern, but the Name property may not exist on the account objects. Based on the account retrieval in Get-InitialADServiceAccounts.ps1, the objects have SamAccountName property, not Name. This should likely be $Account.SamAccountName instead of $Account.Name for consistency with the filtering logic elsewhere.

Suggested change

if ($Account.DistinguishedName -like $Exclude -or $Account.Name -like $Exclude) {

if ($Account.DistinguishedName -like $Exclude -or $Account.SamAccountName -like $Exclude) {

Copilot · 2025-11-23T16:52:02Z

Private/New-HTMLProcessedServiceAccounts.ps1

+            if ($Disable) {
+                New-HTMLText -Text "Service accounts will be disabled only if: " -FontWeight bold
+                New-HTMLList {
+                    foreach ($Key in $DisableOnlyIf.Keys) {
+                        New-HTMLListItem -Text @(
+                            if ($null -eq $DisableOnlyIf[$Key]) {
+                                $($Key), " is ", 'Not Set'
+                                $ColorInUse = 'Cinnabar'
+                            } else {
+                                $($Key), " is ", $($DisableOnlyIf[$Key])
+                                $ColorInUse = 'Apple'
+                            }
+                        ) -FontWeight bold, normal, bold -Color $ColorInUse, None, CornflowerBlue
+                    }
+                }
+            } else {
+                New-HTMLText -Text "Service accounts will not be disabled, as the disable functionality was not enabled." -FontWeight bold
+            }
+        }
+
+        New-HTMLPanel {
+            if ($Delete) {
+                New-HTMLText -Text "Service accounts will be deleted only if: " -FontWeight bold
+                New-HTMLList {
+                    foreach ($Key in $DeleteOnlyIf.Keys) {
+                        New-HTMLListItem -Text @(
+                            if ($null -eq $DeleteOnlyIf[$Key]) {
+                                $($Key), " is ", 'Not Set'
+                                $ColorInUse = 'Cinnabar'
+                            } else {
+                                $($Key), " is ", $($DeleteOnlyIf[$Key])
+                                $ColorInUse = 'Apple'
+                            }
+                        ) -FontWeight bold, normal, bold -Color $ColorInUse, None, CornflowerBlue
+                    }
+                }


The function accesses $DisableOnlyIf.Keys and $DeleteOnlyIf.Keys (lines 93, 114) but doesn't validate that these parameters are not null. If these hashtables are not provided (which can happen based on the bug identified in the main function), this will cause a null reference error. Add null checks before iterating over the Keys.

Copilot · 2025-11-23T16:52:02Z

Private/Get-InitialADServiceAccounts.ps1

+        if ($ExcludeAccounts) {
+            $ExcludePattern = [string]::Join('|', ($ExcludeAccounts | ForEach-Object { [regex]::Escape($_).Replace('\*','.*') }))
+            $Accounts = $Accounts | Where-Object { $_.SamAccountName -notmatch "^($ExcludePattern)$" }
+        }


[nitpick] Exclusion filtering is applied twice: first in Get-InitialADServiceAccounts (lines 18-21) which filters by account name patterns, and again in Get-ADServiceAccountsToProcess (lines 20-24) which filters by DistinguishedName or Name patterns. This redundant filtering could lead to confusion. Consider removing the exclusion logic from one of these functions, or clearly document why exclusions need to be applied at both stages.

Suggested change

if ($ExcludeAccounts) {

$ExcludePattern = [string]::Join('|', ($ExcludeAccounts | ForEach-Object { [regex]::Escape($_).Replace('\*','.*') }))

$Accounts = $Accounts | Where-Object { $_.SamAccountName -notmatch "^($ExcludePattern)$" }

}

Copilot · 2025-11-23T16:52:03Z

Public/Invoke-ADServiceAccountsCleanup.ps1

+    Write-Color -Text "[i] ", "Executed by: ", $Env:USERNAME, ' from domain ', $Env:USERDNSDOMAIN -Color Yellow, White, Green, White
+
+    try {
+        $ForestInformation = Get-WinADForestDetails -PreferWritable -Forest $Forest -IncludeDomains $IncludeDomains -ExcludeDomains $ExcludeDomains


The -PreferWritable parameter is passed to Get-WinADForestDetails but is not defined in the function's parameter block. This will cause an error when the function is executed. Either add -PreferWritable as a parameter to this function, or remove it from this call if it's not needed.

Suggested change

$ForestInformation = Get-WinADForestDetails -PreferWritable -Forest $Forest -IncludeDomains $IncludeDomains -ExcludeDomains $ExcludeDomains

$ForestInformation = Get-WinADForestDetails -Forest $Forest -IncludeDomains $IncludeDomains -ExcludeDomains $ExcludeDomains

Copilot · 2025-11-23T16:52:03Z

Public/Invoke-ADServiceAccountsCleanup.ps1

+            DisableOnlyIf     = $DisableOnlyIf
+            DeleteOnlyIf      = $DeleteOnlyIf


Variables $DisableOnlyIf and $DeleteOnlyIf are only defined within the conditional blocks (lines 133 and 139) but are referenced outside their scope on lines 167-168 when generating the HTML report. If neither -Disable nor -Delete is specified, or if only one is specified, the undefined variable will be passed to the report generation function, potentially causing errors. Consider defining these variables before the foreach loop with default empty hashtables.

Copilot · 2025-11-23T16:52:03Z

Private/Get-ADServiceAccountsToProcess.ps1

+        if ($ActionIf.LastLogonDateMoreThan) {
+            if ($Account.LastLogonDate) {
+                $LastLogonDays = (New-TimeSpan -Start $Account.LastLogonDate -End $Today).Days
+                $Account | Add-Member -NotePropertyName LastLogonDays -NotePropertyValue $LastLogonDays -Force
+                if ($LastLogonDays -le $ActionIf.LastLogonDateMoreThan) { $Include = $false }
+            }


The filtering logic has a potential issue: when a condition is set (e.g., LastLogonDateMoreThan) but the corresponding account property is null (e.g., $Account.LastLogonDate is null), the account will still pass the filter. This means accounts that have never logged on would be included even when specifying LastLogonDateMoreThan. Consider whether accounts with null values should be explicitly handled - either included (as currently) or excluded, and document this behavior clearly.

Add service account cleanup support

0f242e5

PrzemyslawKlys added the codex label Aug 7, 2025 — with ChatGPT Codex Connector

PrzemyslawKlys added 3 commits August 8, 2025 22:16

enhance service account cleanup reporting

afae39a

Refactor service account retrieval

fa568f5

Merge branch 'master' into add-ad-service-account-cleanup-functions

e6795b7

PrzemyslawKlys requested a review from Copilot November 23, 2025 16:48

Copilot started reviewing on behalf of PrzemyslawKlys November 23, 2025 16:48 View session

Copilot finished reviewing on behalf of PrzemyslawKlys November 23, 2025 16:50

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add Active Directory service account cleanup capabilities #32

Add Active Directory service account cleanup capabilities #32

Uh oh!

PrzemyslawKlys commented Aug 7, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if ($Account.DistinguishedName -like $Exclude -or $Account.Name -like $Exclude) {
	if ($Account.DistinguishedName -like $Exclude -or $Account.SamAccountName -like $Exclude) {

	$ForestInformation = Get-WinADForestDetails -PreferWritable -Forest $Forest -IncludeDomains $IncludeDomains -ExcludeDomains $ExcludeDomains
	$ForestInformation = Get-WinADForestDetails -Forest $Forest -IncludeDomains $IncludeDomains -ExcludeDomains $ExcludeDomains

Uh oh!

Add Active Directory service account cleanup capabilities #32

Are you sure you want to change the base?

Add Active Directory service account cleanup capabilities #32

Uh oh!

Conversation

PrzemyslawKlys commented Aug 7, 2025

Summary

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants