🐛 fix(scripts): third audit pass — string interpolation, obsolete API, and scope bugs (#17)

* 🐛 fix(scripts): correct string interpolation, obsolete API, and ErrorActionPreference 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>

* 🐛 fix(scripts): address prior PR Copilot review follow-ups

- 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>

* 🐛 fix(scripts): address PR 17 Copilot review comments

- 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>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: SamErde

Active Directory/AD Users/Set User EmailAddress from PrimaryAddress.ps1

@@ -32,7 +32,7 @@ Write-Progress -Id 1 -Activity $MainScriptActivity -Status $MainScriptStatus -Pe
 foreach ($user in $users)
 {
     $i++
-    Write-Progress -Id 2 -Activity $UserLoopActivity -Status "$i of $userCount - $user.Name" -PercentComplete ($i / $userCount * 100) -ParentId 1
+    Write-Progress -Id 2 -Activity $UserLoopActivity -Status "$i of $userCount - $($user.Name)" -PercentComplete ($i / $userCount * 100) -ParentId 1
 
     $addresses = $user | Select-Object -ExpandProperty ProxyAddresses
     If ($addresses)

Active Directory/Domain Services/DNSZonesRemote.ps1

@@ -48,7 +48,7 @@ foreach ($srv in $servers) {
             }
         }
     } Catch {
-        Write-Host -ForegroundColor DarkYellow "Failed to enter the PSSession for $server. Skipping."
+        Write-Host -ForegroundColor DarkYellow "Failed to create or use the PSSession for $server. Skipping."
         Continue
     } Finally {
         if ($session) {

Active Directory/Set DNS Server Zone Settings via Registry.ps1

@@ -55,7 +55,7 @@ foreach ($srv in $servers) {
             }
         }
     } Catch {
-        Write-Host "Failed to enter the PSSession for $server. Skipping." -ForegroundColor DarkYellow
+        Write-Host "Failed to create or use the PSSession for $server. Skipping." -ForegroundColor DarkYellow
         Continue
     } Finally {
         if ($session) {

DDI/DNS Reverse Lookup from CSV and Add Column with Hostname.ps1

@@ -2,6 +2,6 @@ $file = ""
 $csv = Import-Csv $file
 foreach ($row in $csv) {
     $IP = $row.SourceIP
-    $row.SourceName = ([System.Net.DNS]::GetHostbyAddress($IP)).Hostname
+    $row.SourceName = ([System.Net.DNS]::GetHostEntry($IP)).Hostname
 }
 $csv | Export-Csv "results.csv" -NoTypeInformation

DDI/Resolve-IPs.ps1

@@ -14,11 +14,14 @@ $ResultList = @()
 
 
 foreach ($IP in $ListOfIPs) {
-    $ErrorActionPreference = 'silentlycontinue'
     $Result = $null
 
     Write-Host "Resolving $IP" -ForegroundColor Green
-    $result = [System.Net.Dns]::gethostentry($IP)
+    try {
+        $Result = [System.Net.Dns]::GetHostEntry($IP)
+    } catch {
+        $Result = $null
+    }
 
     If ($Result) {
         $ResultList += "$IP," + [string]$Result.HostName

Profile and Prompt/PSProfileBase.ps1

@@ -170,8 +170,10 @@ if ($IsPowerShellISE -or (-not (Get-Command -Name 'oh-my-posh' -ErrorAction Sile
     # Custom prompt function that shows admin status, command ID, command duration, and path.
     function Prompt {
         $LastCommand = Get-History -Count 1 -ErrorAction SilentlyContinue
+        $CommandId = if ($LastCommand) { $LastCommand.Id + 1 } else { 1 }
+        $DurationMilliseconds = if ($LastCommand) { [math]::Ceiling($LastCommand.Duration.TotalMilliseconds) } else { 0 }
         Write-Host "$AdminStatus " -ForegroundColor "$AdminStatusColor" -NoNewline
-        Write-Host "[$($LastCommand.Id +1)] $([math]::Ceiling($LastCommand.Duration.TotalMilliseconds))ms " -NoNewline -ForegroundColor White
+        Write-Host "[$CommandId] ${DurationMilliseconds}ms " -NoNewline -ForegroundColor White
         Write-Host "$FolderGlyph $($PWD.ToString() -ireplace [regex]::Escape($HOME),'~')" -ForegroundColor Yellow
         Write-Host '>' -NoNewline -ForegroundColor White
         return ' '
@@ -190,8 +192,10 @@ if ($IsPowerShellISE -or (-not (Get-Command -Name 'oh-my-posh' -ErrorAction Sile
 
     function Prompt {
         $LastCommand = Get-History -Count 1 -ErrorAction SilentlyContinue
+        $CommandId = if ($LastCommand) { $LastCommand.Id + 1 } else { 1 }
+        $DurationMilliseconds = if ($LastCommand) { [math]::Ceiling($LastCommand.Duration.TotalMilliseconds) } else { 0 }
         Write-Host "$AdminStatus " -ForegroundColor "$AdminStatusColor" -NoNewline
-        Write-Host "[$($LastCommand.Id +1)] $([math]::Ceiling($LastCommand.Duration.TotalMilliseconds))ms " -NoNewline -ForegroundColor White
+        Write-Host "[$CommandId] ${DurationMilliseconds}ms " -NoNewline -ForegroundColor White
         Write-Host "$FolderGlyph $($PWD.ToString() -ireplace [regex]::Escape($HOME),'~')" -ForegroundColor Yellow
         Write-Host '>' -NoNewline -ForegroundColor White
         return ' '

Snippets/RemoteServerSessionLoop2.ps1

@@ -15,24 +15,26 @@ if ($session) { Remove-PSSession $session }
 
 #Loop through each server in the list, open a PowerShell remoting session, then show the name and status of the session. Skip (continue) to the next server if a connection fails.
 foreach ($server in $servers) {
-    $session = New-PSSession -ComputerName $server -Name $server -Credential $Creds
+    $session = $null
 
     Try {
         Write-Information "Connecting to $server... " -InformationAction Continue
-        Enter-PSSession $session
+        $session = New-PSSession -ComputerName $server -Name $server -Credential $Creds -ErrorAction Stop
+        # Enter-PSSession is interactive-only; use Invoke-Command to run code in the remote session.
+        Invoke-Command -Session $session -ErrorAction Stop -ScriptBlock {
+            <#
+                Code to be run on each remote server goes here.
+            #>
+            Write-Information 'Inner code.' -InformationAction Continue
+        }
     } Catch {
-        Write-Warning "Failed to enter the PSSession for $server. Skipping." -WarningAction Continue
+        Write-Warning "Failed to create or use the PSSession for $server. Skipping." -WarningAction Continue
         Continue
+    } Finally {
+        if ($session) {
+            # Cleanup and then show the current PSSession state.
+            Remove-PSSession $session -ErrorAction SilentlyContinue
+            Write-Information "$($session.ComputerName) $($session.State) `n`n" -InformationAction Continue
+        }
     }
-    Write-Output $session.State
-
-    <#
-        Code to be run on each remote server go here.
-    #>
-    Write-Information 'Inner code.' -InformationAction Continue
-
-    #Cleanup and then show the current PSSession state.
-    Exit-PSSession
-    Remove-PSSession $session
-    Write-Information "$session.ComputerName $session.State `n`n" -InformationAction Continue
 }

Windows/Disable-Poshv2.ps1

@@ -11,7 +11,7 @@ $exclusions = @('', '', '')
 #Filtering out production servers as well as any SQL, Exchange, SharePoint (SPS), Telephony, rinf* servers, and any servers in the "Non Windows" OU.
 $servers = (Get-ADComputer -Filter 'OperatingSystem -like "Windows Server*" -and Enabled -eq "True" -and Name -notlike "*sps*"' `
         -SearchBase 'DC=DOMAIN,DC=COM' -SearchScope Subtree).Name | Where-Object { $_ -notin $Exclusions } | Sort-Object
-Write-Information "$servers.Count servers found."
+Write-Information "$($servers.Count) servers found."
 
 foreach ($server in $servers) {
     if (Test-WSMan -ComputerName $server -ErrorAction Ignore) {

Windows/New-gMSA.ps1

@@ -37,6 +37,7 @@ Function New-gMSA {
     .OUTPUTS
         None
     #>
+    [CmdletBinding(SupportsShouldProcess = $true, ConfirmImpact = 'Medium')]
     param (
         [Parameter(Mandatory=$true)]
         [ValidateLength(1,15)]
@@ -65,7 +66,10 @@ Function New-gMSA {
     Import-Module ActiveDirectory
     $Group = "MSA " + $gMSA
     $DNS = "$gMSA.$DomainDnsName"
-    New-ADGroup -Name $Group -GroupScope Global -DisplayName $Group -Description "Permission group for $gMSA" -Path $GroupPath -Credential $Credential
-    Add-ADGroupMember -Identity $Group -Members ($Servers | ForEach-Object {Get-ADComputer -Identity $_ -Credential $Credential}) -Credential $Credential
-    New-ADServiceAccount -Name $gMSA -DNSHostName $DNS -PrincipalsAllowedToRetrieveManagedPassword $Group -Path $ServiceAccountPath -Credential $Credential
+
+    if ($PSCmdlet.ShouldProcess($gMSA, 'Create gMSA, retrieval group, and group membership')) {
+        New-ADGroup -Name $Group -GroupScope Global -DisplayName $Group -Description "Permission group for $gMSA" -Path $GroupPath -Credential $Credential
+        Add-ADGroupMember -Identity $Group -Members ($Servers | ForEach-Object {Get-ADComputer -Identity $_ -Credential $Credential}) -Credential $Credential
+        New-ADServiceAccount -Name $gMSA -DNSHostName $DNS -PrincipalsAllowedToRetrieveManagedPassword $Group -Path $ServiceAccountPath -Credential $Credential
+    }
 }