Enable support for WinRM over HTTPS

[Copilot]

CIS-hardened environments disable WinRM over HTTP (port 5985), breaking all PSRemoting-based diagnostics. This adds first-class -UseSSL and -Port support throughout the PSRemoting call chain so diagnostics work with HTTPS (port 5986).

Description

Summary of changes:

• New-PSRemotingSession: Added -UseSSL switch and -Port parameter. Defaults to port 5986 when SSL is enabled, 5985 otherwise. Falls back to $Global:SdnDiagnostics.Config.UseSSL/Port when not explicitly provided, allowing module-wide configuration without per-call parameter threading. Skips WinRM TrustedHosts management for IP addresses when using SSL (certificate validates identity). Made New-PSSessionOption platform-aware to allow Linux test compatibility.
• Invoke-PSRemoteCommand, Copy-FileFromRemoteComputerWinRM, Copy-FileToRemoteComputerWinRM, Copy-FileFromRemoteComputer, Copy-FileToRemoteComputer: Added -UseSSL and -Port parameters propagated to New-PSRemotingSession.
• $Global:SdnDiagnostics.Config: Added UseSSL = $false and Port = 0 defaults for module-wide transport configuration.
• Start-SdnDataCollection: Added -UseSSL and -Port parameters; sets global config so all downstream calls inherit the transport settings. Fixed the WinRM pre-flight connectivity check (was hardcoded to 5985) to derive port from global config.
• Tests: Added 5 Pester tests covering port auto-selection (SSL→5986, HTTP→5985), explicit port override, and global config inheritance.

# Module-wide HTTPS config — all subsequent PSRemoting calls use port 5986
$Global:SdnDiagnostics.Config.UseSSL = $true

# Or per-invocation
Start-SdnDataCollection -NcUri $nc -UseSSL

# Or fine-grained
Invoke-PSRemoteCommand -ComputerName $node -UseSSL -Port 5986 -ScriptBlock { ... }

Change type

• New Feature (non-breaking change that adds new functionality without impacting existing)

Checklist:

• My code follows the style and contribution guidelines of this project.
• I have tested and validated my code changes.

• Fixes Enable support for WinRM over HTTPS #423

[Copilot]

Copilot wasn't able to review any files in this pull request.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/modules/SdnDiag.Utilities.psm1:2093

• HTTPS authenticates the server certificate, but it does not make Kerberos support an IP-address target. WinRM connections addressed by IP still require explicit credentials; otherwise this branch proceeds with default credentials and the session fails authentication. Keep the credential requirement independent of UseSSL.

                    if ($isIpAddress -and -NOT $UseSSL -and $Credential -eq [System.Management.Automation.PSCredential]::Empty) {

src/modules/SdnDiag.Utilities.psm1:1954

• An existing SdnDiag-* session is reused solely by computer name, so a prior HTTP/5985 session can be returned even when this new option requests HTTPS or a different port. That makes transport changes ineffective until sessions are manually removed or -Force is used. Match reusable sessions on the requested URI scheme and port (or create a new session when either differs).

        [Parameter(Mandatory = $false)]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false)]
        [System.Int32]$Port

src/SdnDiagnostics.psm1:955

• ForEach-Object -Parallel invokes this mutation concurrently, but ArrayList is not thread-safe. Simultaneous failures can race while adding entries, potentially omitting unreachable nodes or corrupting the collection. Use a thread-safe collection for the shared results.

                    [void]($using:nodesToRemove).Add($_)

src/SdnDiagnostics.psm1:770

• The exported cmdlet adds UseSSL and Port, but its comment-based help does not include .PARAMETER entries for either option. Add descriptions covering HTTPS selection, automatic ports, and the custom-port override so Get-Help Start-SdnDataCollection -Full documents the new public API.

        [Switch]$UseSSL,

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [System.Int32]$Port

src/SdnDiagnostics.psm1:790

• The exported Start-SdnDataCollection path now mutates module-wide transport settings and selects the preflight port, but the added tests only exercise New-PSRemotingSession. Please add offline Pester coverage verifying that -UseSSL/-Port update the configuration and that the preflight uses 5986 or the explicit port.

    if ($PSBoundParameters.ContainsKey('UseSSL')) {
        $Global:SdnDiagnostics.Config.UseSSL = $UseSSL.IsPresent
    }

    if ($PSBoundParameters.ContainsKey('Port')) {

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (7)

Previously missed (2) — in code that hasn't changed since the last review.

src/modules/SdnDiag.Utilities.psm1:504

• The exported Copy-SdnFileFromComputer wrapper (SdnDiag.Utilities.psm1:2753-2797) does not declare these new parameters, so callers cannot request HTTPS or a custom port per copy operation even though the private implementation supports them. Add UseSSL and Port to the exported wrapper and its help, then forward them through @PSBoundParameters.

This issue also appears on line 776 of the same file.

        [Parameter(Mandatory = $false)]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false)]
        [System.Int32]$Port

src/modules/SdnDiag.Utilities.psm1:1651

• These per-call options are only added to the private helper. The exported Invoke-SdnCommand wrapper (SdnDiag.Utilities.psm1:3007-3039) does not accept them, so the documented fine-grained HTTPS invocation is unavailable through the module's public API. Add and document both parameters on Invoke-SdnCommand; its existing @PSBoundParameters forwarding will then carry them through.

        [Parameter(Mandatory = $false)]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false)]
        [System.Int32]$Port

src/SdnDiagnostics.psm1:770

• This exported cmdlet's signature and WinRM preflight behavior changed, but there is no Start-SdnDataCollection test under tests/offline/; the added tests exercise only New-PSRemotingSession. Please add regression coverage proving that UseSSL/Port update configuration and that preflight selects the configured port, as required for modified exported functions.

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [System.Int32]$Port

src/SdnDiagnostics.psm1:770

• Start-SdnDataCollection -UseSSL still calls the exported Install-SdnDiagnostics, whose version probe uses Invoke-Command -ComputerName directly (SdnDiag.Utilities.psm1:2961-2970) without UseSSL or Port. In an HTTPS-only environment that probe therefore attempts HTTP, catches the connection failure, and skips module installation; later imported remoting sessions can then fail when the module is absent or outdated. Route this probe through the configured remoting path or add the transport parameters to Install-SdnDiagnostics.

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [System.Int32]$Port

src/modules/SdnDiag.Utilities.psm1:780

• The exported Copy-SdnFileToComputer wrapper (SdnDiag.Utilities.psm1:2800-2843) does not expose these transport parameters. Consequently users can configure this private helper per invocation only from inside the module, not through the public copy cmdlet. Add and document UseSSL and Port on the wrapper so @PSBoundParameters forwards them.

        [Parameter(Mandatory = $false)]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false)]
        [System.Int32]$Port

src/modules/SdnDiag.Utilities.psm1:1954

• Session reuse is still keyed only by ComputerName at lines 2004-2012. If an HTTP session to a host already exists, requesting -UseSSL or a different Port returns that cached session before these new settings reach New-PSSession, silently defeating the requested transport. Match cached sessions on scheme and port as well, or force creation when the existing endpoint differs.

        [Parameter(Mandatory = $false)]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false)]
        [System.Int32]$Port

src/SdnDiagnostics.psm1:770

• The exported Start-SdnDataCollection help block ends after ConvertETW and does not document either new public parameter. Add .PARAMETER UseSSL and .PARAMETER Port entries, including the automatic 5985/5986 behavior and custom-port semantics, so Get-Help accurately describes the API.

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [System.Int32]$Port

[Copilot]

Pull request overview

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

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

tests/offline/SdnDiagnostics.Tests.ps1:115

• On PowerShell 7, the production code invokes Test-NetConnection inside a ForEach-Object -Parallel child runspace. This Pester mock exists only in the current runspace, so it cannot intercept or record that call; the tests contact the fictional host and the subsequent Should -Invoke assertions see no invocation. These tests therefore break under pwsh and only exercise the serial PowerShell 5.1 branch. Refactor the connectivity probe behind a mockable helper or otherwise provide a test seam for the parallel branch.

                # Mock Test-NetConnection to return $true so preflight passes;
                # the test asserts it was called with the correct Port argument.
                Mock Test-NetConnection { return $true }

tests/offline/Utilities.Tests.ps1:474

• This filter inspects the filter scriptblock's $PSBoundParameters, not the parameters captured by Pester for the mocked invocation. It is therefore empty here and the predicate passes even if UseSSL or Port was forwarded, making the unbound regression test ineffective. Use $PesterBoundParameters (as the test already does at line 439) to inspect the actual mock call.

            Should -Invoke New-PSRemotingSession -Times 1 -ParameterFilter {
                -not $PSBoundParameters.ContainsKey('UseSSL') -and -not $PSBoundParameters.ContainsKey('Port')
            }

tests/offline/Utilities.Tests.ps1:502

• This filter checks $PSBoundParameters for the filter scriptblock rather than Pester's captured mock-call parameters, so it remains true even if the implementation incorrectly forwards UseSSL or Port. Check $PesterBoundParameters to make this test validate the intended unbound behavior.

            Should -Invoke New-PSRemotingSession -Times 1 -ParameterFilter {
                -not $PSBoundParameters.ContainsKey('UseSSL') -and -not $PSBoundParameters.ContainsKey('Port')
            }

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

tests/offline/Utilities.Tests.ps1:404

• This mock adds a UseSSL property that real WSManConnectionInfo objects do not have, which masks the production cache-key bug. Model the actual Scheme property so this regression test exercises the same API shape returned by Get-PSSession.

                                    Port   = 5986
                                    UseSSL = $true

src/modules/SdnDiag.NetworkController.SF.psm1:2491

• This fixes the HTTPS transport for the exported Invoke-SdnServiceFabricCommand, but no offline test invokes this function or verifies that the remote Invoke-Command receives the configured UseSSL and Port values (including default-port selection). Please add a regression test for this newly fixed path.

        $params.Add('Port', $port)
        if ($useSSL) {
            $params.Add('UseSSL', $true)

src/modules/SdnDiag.Utilities.psm1:2933

• The public help says these options affect only the version probe, but they are now also forwarded to the WinRM copy fallback at line 3061. Document both operations so callers understand the transport setting applies throughout installation.

    $path = "SdnDiag.{0}.Config.psd1" -f $Role
    $moduleConfig = Get-Item -Path (Join-Path -Path $PSScriptRoot -ChildPath $path) -ErrorAction SilentlyContinue
    if ($moduleConfig) {
        $moduleConfigData = Import-PowerShellDataFile -Path $moduleConfig.FullName

[Copilot]

Fixed in commit just pushed. The session cache match now compares $_.Runspace.ConnectionInfo.Scheme -ieq 'https' against -UseSSL, and the corresponding test mocks in tests/offline/Utilities.Tests.ps1 now use Scheme instead of a nonexistent UseSSL property to match the real WSManConnectionInfo shape.

[Copilot]

The Scheme-based cache match was applied in 4e12b45 — New-PSRemotingSession now compares ($_.Runspace.ConnectionInfo.Scheme -ieq 'https') -eq $UseSSL (src/modules/SdnDiag.Utilities.psm1:2053), and the test mocks model Scheme instead of a non-existent UseSSL property. The remaining suppressed items from this review are also covered: the Invoke-SdnServiceFabricCommand UseSSL/Port regression tests exist in tests/offline/NetworkController.SF.Tests.ps1, and the Install-SdnDiagnostics help for -UseSSL/-Port now documents that the settings apply to both the version probe and the WinRM module copy (7777534).

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/modules/SdnDiag.Utilities.psm1:2053

• WSManConnectionInfo does not expose a UseSSL property, so this expression casts $null to $false. Consequently, real HTTPS sessions never match and every HTTPS operation creates another cached session instead of reusing the available one. Compare ConnectionInfo.Scheme with https/http (and update the cache tests to model Scheme, rather than the synthetic UseSSL property).

                [bool]$_.Runspace.ConnectionInfo.UseSSL -eq $UseSSL

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

tests/offline/SdnDiagnostics.Tests.ps1:98

• The shared setup runs in SdnDiagnostics, where Pester cannot resolve the private [TraceLevel] type on Trace-Output; this prevents all four preflight tests from running. Remove the Level type when creating this mock.

                Mock Trace-Output {}

tests/offline/SdnDiagnostics.Tests.ps1:39

• This Trace-Output mock is generated outside SdnDiag.Utilities, so Pester cannot resolve the command's private [TraceLevel] parameter type and the test fails during setup. Remove the Level type from the mock metadata.

                Mock Trace-Output {}

tests/offline/SdnDiagnostics.Tests.ps1:68

• This Trace-Output mock is generated outside SdnDiag.Utilities, so Pester cannot resolve the command's private [TraceLevel] parameter type and the test fails during setup. Remove the Level type from the mock metadata.

                Mock Trace-Output {}

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/SdnDiagnostics.psm1:770

• Enabling HTTPS here still leaves the collection targets normalized to ComputerNameNetBIOS below (lines 908 and 938), even when the caller supplied an FQDN. WinRM HTTPS certificates normally identify the server by FQDN, so Start-SdnDataCollection -ComputerName host.contoso.com -UseSSL subsequently connects to host and can fail certificate name validation. Preserve/use ComputerNameFQDN as the remoting target when SSL is enabled (while retaining the short name separately if it is needed for display or role matching).

        [Parameter(Mandatory = $false, ParameterSetName = 'Role')]
        [Parameter(Mandatory = $false, ParameterSetName = 'Computer')]
        [Switch]$UseSSL,

src/SdnDiagnostics.psm1:977

• The new regression tests do not exercise this PowerShell 7 branch: CI runs RunTests.ps1 with Windows PowerShell (shell: powershell), so every assertion covers only lines 980–985. Moreover, a Pester Test-NetConnection mock in the parent runspace is not available inside ForEach-Object -Parallel. Add a PS7-safe test seam (for example, extract the probe and mock/test it independently) and run this path under pwsh, so the changed parallel removal/port behavior is covered.

            $nodesToRemove = @($dataCollectionNodes | ForEach-Object -ThrottleLimit 10 -Parallel {
                $tncResult = Test-NetConnection -ComputerName $_.Name -Port $using:tncPort -InformationLevel Quiet
                if (-NOT ($tncResult)) {
                    $_
                }
            })

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/modules/SdnDiag.Utilities.psm1:3138

• The PR description’s fine-grained example still tells consumers to call Invoke-PSRemoteCommand, but that helper is not exported; after a normal module import, only this Invoke-SdnCommand wrapper is available. Update the example to use Invoke-SdnCommand -UseSSL -Port ... so the documented command actually works for module consumers.

        [Parameter(Mandatory = $false)]
        [Switch]$UseSSL,

        [Parameter(Mandatory = $false)]
        [ValidateRange(1, 65535)]
        [System.Int32]$Port

[Copilot]

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tests/offline/SdnDiagnostics.Tests.ps1:109

• These preflight tests leave the Service Fabric manifest lookup live. ClusterConfigType defaults to ServiceFabric, so Start-SdnDataCollection calls Get-SdnServiceFabricClusterManifest before reaching Test-NetConnection; that function can attempt real remoting to DVLAB-NC01, making this offline test slow and environment-dependent. Mock the manifest lookup (or set and restore a non-Service-Fabric cluster type) in BeforeEach.

                Mock Get-SdnInfrastructureInfo {
                    return @{
                        Server = @('DVLAB-S1-N01')
                    }
                }