Conversation
β¦ActionPreference scope bugs - Snippets\RemoteServerSessionLoop2.ps1: replace Enter-PSSession (interactive-only) with Invoke-Command -Session; fix string interpolation for \.ComputerName and \.State using \ subexpression syntax - DDI\DNS Reverse Lookup from CSV.ps1: replace obsolete GetHostbyAddress() with GetHostEntry() - Windows\Disable-Poshv2.ps1: fix \.Count not expanding in double-quoted string - AD Users\Set User EmailAddress from PrimaryAddress.ps1: fix \.Name not expanding in Write-Progress -Status string - DDI\Resolve-IPs.ps1: remove \Continue global assignment inside foreach loop; wrap DNS lookup in try/catch to suppress per-call errors without leaking preference scope Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Up to standards βπ’ Issues
|
There was a problem hiding this comment.
Pull request overview
Third audit pass addressing several PowerShell runtime issues across the repo, primarily fixing broken string interpolation, replacing an obsolete DNS API, and correcting remoting/error-scope behavior.
Changes:
- Fix PowerShell string interpolation in multiple scripts to correctly expand properties/counts.
- Replace obsolete
[System.Net.DNS]::GetHostbyAddress()with[System.Net.DNS]::GetHostEntry(). - Adjust remoting and error-handling patterns (notably removing
$ErrorActionPreferenceleakage inDDI/Resolve-IPs.ps1and switching an interactive remoting pattern toInvoke-Command).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Windows/Disable-Poshv2.ps1 | Fixes count interpolation so the βservers foundβ message shows the actual count. |
| Snippets/RemoteServerSessionLoop2.ps1 | Replaces interactive Enter-PSSession usage with Invoke-Command and fixes property interpolation when logging session details. |
| DDI/Resolve-IPs.ps1 | Removes $ErrorActionPreference leakage by switching to try/catch around DNS lookups. |
| DDI/DNS Reverse Lookup from CSV and Add Column with Hostname.ps1 | Replaces an obsolete .NET DNS reverse-lookup API with the supported equivalent. |
| Active Directory/AD Users/Set User EmailAddress from PrimaryAddress.ps1 | Fixes Write-Progress status interpolation for user name display. |
- Make profile Prompt implementations null-safe when command history is empty - Add ShouldProcess support around New-gMSA Active Directory changes - Update stale remoting catch messages after replacing Enter-PSSession usage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move PSSession creation into try/catch with terminating errors - Ensure PSSession cleanup runs from finally when remote execution fails - Use consistent PascalCase Result variable in Resolve-IPs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Third audit pass over the repository. Zero syntax errors found (verified via AST parse of all 233 files). Five confirmed runtime bugs fixed across 5 files.
Changes
Snippets/RemoteServerSessionLoop2.ps1β 2 bugsEnter-PSSessionin a loop (Critical):Enter-PSSessionis interactive-only β it does not route subsequent script lines through the remote session. Replaced withInvoke-Command -Session $session -ScriptBlock { ... }. Also removed the now-unnecessaryExit-PSSession."$session.ComputerName $session.State"was appending the property names as literal text. Fixed to"$($session.ComputerName) $($session.State)".DDI/DNS Reverse Lookup from CSV and Add Column with Hostname.ps1β 1 bug[System.Net.DNS]::GetHostbyAddress()is obsolete. Replaced with[System.Net.DNS]::GetHostEntry(), which has the same return type (IPHostEntry) and the same.HostNameproperty.Windows/Disable-Poshv2.ps1β 1 bug"$servers.Count servers found."was outputting the array object's.ToString()followed by the literal text.Count servers found.. Fixed to"$($servers.Count) servers found.".Active Directory/AD Users/Set User EmailAddress from PrimaryAddress.ps1β 1 bug"$i of $userCount - $user.Name"inWrite-Progress -Statuswas appending.Nameas a literal string. Fixed to"$i of $userCount - $($user.Name)".DDI/Resolve-IPs.ps1β 1 bug$ErrorActionPreferenceglobal scope leak (Medium):$ErrorActionPreference = 'silentlycontinue'was set inside theforeachloop on every iteration, permanently overriding the preference for the entire script (and the caller's scope). Removed the global assignment and wrapped the .NET DNS lookup in atry/catchblock to suppress per-call errors without leaking the preference change.Verification
[System.Management.Automation.Language.Parser]::ParseFile()