🧹 chore: code quality audit — fix critical, high, medium, and low bugs

[SamErde]

Summary

Full code-quality audit of the repository focused on invalid code, missed logic, and dead code. 31 findings were identified and resolved across 4 severity levels. Security findings (F01) were intentionally excluded from this pass.

---

Changes by Severity

🔴 Critical (9 fixes)

Finding	File	Fix
F04	General/Purge-InstalledModules.ps1	Added missing comma in module name array
F06	Active Directory/Get-ADUserTransitiveGroupMembership.ps1	Added missing -Value to Set-Variable
F08	Active Directory/Export-AllADUserTransitiveGroupMemberships.ps1	Added missing -Value to Set-Variable
F10	Active Directory/Export-AllGroupMemberships.ps1	Fixed single-quoted AD filter preventing $true expansion
F12	Active Directory/Get-ADAttributeUniqueValues.ps1	Fixed default value not in ValidateSet
F13/F15	Active Directory/AD Groups/Archive-ObsoleteGroups.ps1	Made $Domain mandatory; fixed 12-hour/single-digit date format; fixed comment block syntax
F21	Active Directory/Domain Services/Get-FSMORoleDetails.ps1	Replaced multi-property -ExpandProperty (unsupported) with direct property access
F23	DDI/Update-DnsServerList.ps1	Fixed $._Index typo → $($netadapter.Index)
F26	Windows/Push-DNSClientServerAddresses.ps1	Replaced interactive Enter-PSSession/Exit-PSSession with Invoke-Command -Session + Remove-PSSession in finally block

🟠 High (9 fixes)

Finding	File	Fix
F07	Active Directory/Get-ADUserTransitiveGroupMembership.ps1	Moved pipeline emit from end to process block
F09	Active Directory/Export-AllADUserTransitiveGroupMemberships.ps1	Fixed undefined $UserCount → $Users.Count
F16	Active Directory/AD Users/Get-InactiveADUser.ps1	Fixed operator precedence bug with -not and comparison
F17	Active Directory/AD Users/Get-InactiveADUser.ps1	$ExportCSV value now used as export path when provided
F19	Active Directory/AD Users/Get-LockedOutLocation.ps1	continue → return in catch block outside any loop
F22	Active Directory/Get-ADObjectFromPipeline.ps1	Moved pipeline identity resolution from begin to process block
F24	General/Update-ModuleVersion.ps1	Made $InputVersion mandatory to prevent null member access
F27	Windows/Push-DNSClientServerAddresses.ps1	Replaced empty placeholder strings with mandatory param() block
F29	Exchange/Parse-TransportLogs.ps1	Removed hardcoded .DOMAINNAME.org placeholder from ConnectionUri

🟡 Medium (9 fixes)

Finding	File	Fix
F02	Windows/Activate and Get License.ps1	FullControl → ReadKey in RegistryAccessRule (comment said "read permissions")
F05	DDI/Get Hostnames from CSV IP Addresses.ps1	$error[0] → $_ in catch block
F15	Active Directory/AD Groups/Archive-ObsoleteGroups.ps1	Confirmed fixed in critical pass
F18	Active Directory/AD Users/Get-InactiveUsers.ps1	break 1 → return in two catch blocks outside any loop
F20	Active Directory/AD Users/Get-LockedOutLocation.ps1	Added BadPasswordTime to Get-ADUser -Properties list
F25	General/Update-ModuleVersion.ps1	Fixed undefined $PrereleaseVersion → "$NewVersion$PrereleaseTag"
F28	Exchange/Parse-TransportLogs.ps1	Removed Set-ExecutionPolicy RemoteSigned side effect
F30	Exchange/Parse-TransportLogs.ps1	Added bounds check before split('<')[1]
F31	Exchange/Parse-TransportLogs.ps1	Replaced $ErrorActionPreference global mutation with try/catch

🔵 Low (1 fix)

Finding	File	Fix
F03	Windows/Activate and Get License.ps1	Removed dead code re-fetch of $ProductKey at EOF

---

Not Addressed

• F01 (security) — intentionally excluded from this pass
• F11 — already correct in current file state
• F14 — environment-specific AD placeholder values; TODO comments added

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[Copilot]

Pull request overview

This PR performs a broad PowerShell code-quality audit across the repository, addressing a set of correctness and reliability issues (with security finding F01 intentionally deferred).

Changes:

• Fixes multiple PowerShell runtime issues (e.g., invalid cmdlet usage/typos, incorrect filters, invalid control flow like break/continue outside loops).
• Reduces operational side effects (e.g., removes interactive remoting patterns; removes execution policy mutation; adds safer parsing bounds checks).
• Improves parameter validation/default correctness and message clarity in several scripts.

Reviewed changes

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

Show a summary per file

File	Description
Windows/Push-DNSClientServerAddresses.ps1	Adds mandatory parameters and replaces interactive remoting with Invoke-Command + session cleanup.
Windows/Activate and Get License.ps1	Narrows registry ACL permissions and removes dead code.
General/Update-ModuleVersion.ps1	Makes input version mandatory; fixes prerelease validation message content.
General/Purge-InstalledModules.ps1	Fixes array syntax (missing comma).
Exchange/Parse-TransportLogs.ps1	Removes execution policy side effect; hardens parsing; replaces global error preference mutation with try/catch.
DDI/Update-DnsServerList.ps1	Fixes CIM filter typo so adapter lookup uses the correct index.
DDI/Get Hostnames from CSV IP Addresses.ps1	Improves error handling in hostname resolution loop.
Active Directory/Get-ADUserTransitiveGroupMembership.ps1	Adjusts preference variable setting and fixes pipeline emission behavior.
Active Directory/Get-ADObjectFromPipeline.ps1	Moves pipeline identity resolution into process where pipeline input is available.
Active Directory/Get-ADAttributeUniqueValues.ps1	Fixes default value to match the ValidateSet.
Active Directory/Export-AllGroupMemberships.ps1	Fixes AD filter so $true expands correctly.
Active Directory/Export-AllADUserTransitiveGroupMemberships.ps1	Fixes undefined count reference and updates preference variable setting.
Active Directory/Domain Services/Get-FSMORoleDetails.ps1	Replaces unsupported multi-property -ExpandProperty usage with direct property access.
Active Directory/AD Users/Get-LockedOutLocation.ps1	Adds required AD property and fixes invalid continue usage outside loops.
Active Directory/AD Users/Get-InactiveUsers.ps1	Fixes invalid break usage outside loops.
Active Directory/AD Users/Get-InactiveADUser.ps1	Fixes operator precedence bug; uses ExportCSV value as the export path when provided.
Active Directory/AD Groups/Archive-ObsoleteGroups.ps1	Makes $Domain mandatory, fixes date formatting, and corrects a malformed comment terminator.

Comments suppressed due to low confidence (3)

Active Directory/Get-ADObjectFromPipeline.ps1:59

• This function is declared as ValueFromPipeline, but it only emits $User/$Computer in the end block. With multiple pipeline inputs, this will return only the last processed object (and $User/$Computer can also leak across iterations). Emit results from within the process block (or collect all results in a list and output the list at end) so each pipeline input produces an output.

    process {
        # Resolve identity type in process block where pipeline input ($Identity) is available.
        if ($Identity -is [Microsoft.ActiveDirectory.Management.ADUser]) {
            # We have an ADUser object
            # Might want to normalize the type to an ADObject IF we can get sidHistory from an ADObject
        }
        if ($Identity -is [Microsoft.ActiveDirectory.Management.ADComputer]) {
            # We have an ADComputer object
            # Might want to normalize the type to an ADObject IF we can get sidHistory from an ADObject
        }
        if ($Identity -is [string]) {
            # Find an AD object and determine its type
            $Identity = Get-ADObject -Filter "Name -eq `"$Identity`""
        }
        $IdentityType = $Identity.ObjectClass
        switch ($IdentityType) {
            'user' {
                # Not Complete
                $User = Get-ADUser -Identity $Identity -Properties PrimaryGroup,SidHistory
            }
            'computer' {
                # Not Complete
                $Computer = Get-ADComputer -Identity $Identity -Properties PrimaryGroup,SidHistory
            }
            Default {
                Write-Error "Identity type not supported."
            }
        }
    }

    end {
        # Do something and/or return the resulting object to the pipeline.
        if ($User) {
            $User
        }
        if ($Computer) {
            $Computer
        }
    }

Active Directory/Get-ADUserTransitiveGroupMembership.ps1:68

• ProgressPreference is set at global scope to SilentlyContinue, but it is only restored after the connection checks. If the function throws while checking connectivity, the global preference stays modified for the caller’s session. Wrap the temporary preference change in a try/finally (or avoid -Scope Global) to guarantee restoration on all exit paths.

        $CurrentProgressPreference = Get-Variable -Name ProgressPreference -ValueOnly
        Set-Variable -Name ProgressPreference -Value 'SilentlyContinue' -Force -Scope Global -ErrorAction SilentlyContinue
        # Check if the global catalog server is available on the specified port.
        if (-not (Test-NetConnection -ComputerName $Server -Port $Port -InformationLevel Quiet -ErrorAction SilentlyContinue)) {
            if (-not (Test-NetConnection -ComputerName $Server -Port $AltPort -InformationLevel Quiet -ErrorAction SilentlyContinue)) {
                throw "Unable to connect to the global catalog server '$Server' on port '$Port' or '$AltPort.'"
            }
        }
        Set-Variable -Name ProgressPreference -Value $CurrentProgressPreference -Force -Scope Global -ErrorAction SilentlyContinue
    }

Active Directory/Export-AllADUserTransitiveGroupMemberships.ps1:158

• This function temporarily sets ProgressPreference at global scope, but it’s only restored after the port checks. If it throws while validating connectivity, the caller’s session can be left with ProgressPreference = SilentlyContinue. Use try/finally around the global preference mutation (or avoid global scope) so it’s always restored.

            $CurrentProgressPreference = Get-Variable -Name ProgressPreference -ValueOnly
            Set-Variable -Name ProgressPreference -Value 'SilentlyContinue' -Scope Global -Force -ErrorAction SilentlyContinue
            # Check if the global catalog server is available on the specified port.
            if (-not (Test-NetConnection -ComputerName $Server -Port $Port -InformationLevel Quiet -ErrorAction SilentlyContinue)) {
                if (-not (Test-NetConnection -ComputerName $Server -Port $AltPort -InformationLevel Quiet -ErrorAction SilentlyContinue)) {
                    throw "Unable to connect to the global catalog server '$Server' on port '$Port' or '$AltPort.'"
                }
            }
            Set-Variable -Name ProgressPreference -Value $CurrentProgressPreference -Scope Global -Force -ErrorAction SilentlyContinue