mirror of
https://github.com/GoodOlClint/PSProxmoxVE.git
synced 2026-09-06 12:09:01 +00:00
fix: store the active PVE session per runspace instead of in a static (#226)
A binary module assembly loads once per process, so the internal static ActiveSession slot was shared by every runspace in the host. Under ForEach-Object -Parallel, a multi-runspace host or a JEA endpoint, one runspace's Connect-PveServer retargeted another runspace's next cmdlet, and a credential established in one runspace was reachable from all of them. The session now lives in the module's own session state, which PowerShell creates once per runspace, read and written through ModuleState.GetActiveSession/SetActiveSession behind the existing single read point PveCmdletBase.GetSession(). The name is scope-qualified so a nested scope cannot write a copy that dies with it, nor read through to a same-named global. A module imported from the assembly rather than the manifest has no session state; that path falls back to the runspace's global scope, which is still per-runspace. No IModuleAssemblyCleanup: no mutable static remains to clear. Closes #150 Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
This commit is contained in:
goodolclint-claude[bot]
committed by
GitHub
GitHub
parent
832aa6e4df
commit
27145ee53e
@@ -137,7 +137,7 @@ namespace PSProxmoxVE.Cmdlets.Connection
|
||||
return;
|
||||
}
|
||||
|
||||
ModuleState.ActiveSession = session;
|
||||
ModuleState.SetActiveSession(this, session);
|
||||
|
||||
if (SkipCertificateCheck.IsPresent)
|
||||
WriteWarning("TLS certificate validation is disabled for this session. Connections are susceptible to man-in-the-middle attacks. Use only in trusted networks or test environments.");
|
||||
|
||||
@@ -19,7 +19,8 @@ namespace PSProxmoxVE.Cmdlets.Connection
|
||||
protected override void ProcessPveRecord()
|
||||
{
|
||||
bool explicitSessionSupplied = MyInvocation.BoundParameters.ContainsKey(nameof(Session));
|
||||
var sessionToDisconnect = explicitSessionSupplied ? Session : ModuleState.ActiveSession;
|
||||
var moduleSession = ModuleState.GetActiveSession(this);
|
||||
var sessionToDisconnect = explicitSessionSupplied ? Session : moduleSession;
|
||||
|
||||
if (sessionToDisconnect is null)
|
||||
{
|
||||
@@ -27,7 +28,7 @@ namespace PSProxmoxVE.Cmdlets.Connection
|
||||
return;
|
||||
}
|
||||
|
||||
if (!ReferenceEquals(sessionToDisconnect, ModuleState.ActiveSession))
|
||||
if (!ReferenceEquals(sessionToDisconnect, moduleSession))
|
||||
{
|
||||
var lifecycle = sessionToDisconnect.AuthMode == PveAuthMode.ApiToken
|
||||
? "API tokens do not expire; revoke it with Remove-PveApiToken if it is no longer needed."
|
||||
@@ -39,7 +40,7 @@ namespace PSProxmoxVE.Cmdlets.Connection
|
||||
if (!ShouldProcess($"{sessionToDisconnect.Hostname}:{sessionToDisconnect.Port}", "Disconnect"))
|
||||
return;
|
||||
|
||||
ModuleState.ActiveSession = null;
|
||||
ModuleState.SetActiveSession(this, null);
|
||||
WriteVerbose($"Disconnected from {sessionToDisconnect.Hostname}:{sessionToDisconnect.Port}.");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ namespace PSProxmoxVE.Cmdlets.Connection
|
||||
|
||||
protected override void ProcessRecord()
|
||||
{
|
||||
var session = ModuleState.ActiveSession;
|
||||
var session = ModuleState.GetActiveSession(this);
|
||||
var isConnected = session is not null && !session.IsExpired;
|
||||
|
||||
if (Detailed.IsPresent)
|
||||
|
||||
@@ -80,13 +80,13 @@ namespace PSProxmoxVE.Cmdlets
|
||||
|
||||
/// <summary>
|
||||
/// Returns the session to use for this cmdlet.
|
||||
/// Resolution order: -Session parameter → ModuleState.ActiveSession.
|
||||
/// Resolution order: -Session parameter → the runspace's module session.
|
||||
/// Throws <see cref="PveNotConnectedException"/> if no session is available,
|
||||
/// or <see cref="PveSessionExpiredException"/> if the session ticket has expired.
|
||||
/// </summary>
|
||||
protected PveSession GetSession()
|
||||
{
|
||||
var session = Session ?? ModuleState.ActiveSession;
|
||||
var session = Session ?? ModuleState.GetActiveSession(this);
|
||||
|
||||
if (session is null)
|
||||
throw new PveNotConnectedException();
|
||||
|
||||
@@ -1,10 +1,46 @@
|
||||
using System.Management.Automation;
|
||||
using PSProxmoxVE.Core.Authentication;
|
||||
|
||||
namespace PSProxmoxVE
|
||||
{
|
||||
/// <summary>Module-scoped state container for the active PVE session</summary>
|
||||
/// <summary>
|
||||
/// Per-runspace storage for the session established by Connect-PveServer.
|
||||
/// The value lives in the module's own session state, which PowerShell creates
|
||||
/// once per runspace, so runspaces cannot see or overwrite each other's session.
|
||||
/// </summary>
|
||||
internal static class ModuleState
|
||||
{
|
||||
internal static PveSession? ActiveSession { get; set; }
|
||||
internal const string ActiveSessionVariable = "PSProxmoxVE.ActiveSession";
|
||||
|
||||
// Qualified so a nested scope in the module's session state (InModuleScope, a
|
||||
// module-scoped scriptblock) neither writes a copy that dies with the scope nor reads
|
||||
// through to a same-named global.
|
||||
private const string ModuleActiveSessionVariable = "script:" + ActiveSessionVariable;
|
||||
|
||||
// Importing the assembly directly rather than through the manifest yields a module
|
||||
// with no session state; the global scope is then the only per-runspace slot left.
|
||||
private const string GlobalActiveSessionVariable = "global:" + ActiveSessionVariable;
|
||||
|
||||
internal static PveSession? GetActiveSession(PSCmdlet cmdlet)
|
||||
{
|
||||
var moduleState = ModuleSessionState(cmdlet);
|
||||
return moduleState is null
|
||||
? cmdlet.SessionState.PSVariable.GetValue(GlobalActiveSessionVariable) as PveSession
|
||||
: moduleState.PSVariable.GetValue(ModuleActiveSessionVariable) as PveSession;
|
||||
}
|
||||
|
||||
internal static void SetActiveSession(PSCmdlet cmdlet, PveSession? session)
|
||||
{
|
||||
var moduleState = ModuleSessionState(cmdlet);
|
||||
if (moduleState is null)
|
||||
cmdlet.SessionState.PSVariable.Set(GlobalActiveSessionVariable, session);
|
||||
else
|
||||
moduleState.PSVariable.Set(ModuleActiveSessionVariable, session);
|
||||
}
|
||||
|
||||
// PSCmdlet.SessionState resolves in the caller's scope, so it would store the session
|
||||
// wherever the cmdlet happened to be invoked from.
|
||||
private static SessionState? ModuleSessionState(PSCmdlet cmdlet) =>
|
||||
cmdlet.MyInvocation?.MyCommand?.Module?.SessionState;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -44,19 +44,18 @@ Describe 'Disconnect-PveServer' {
|
||||
|
||||
Context 'Active session lifecycle' {
|
||||
BeforeAll {
|
||||
$script:PveSessionType = [PSProxmoxVE.Core.Authentication.PveSession]
|
||||
$script:ModuleStateType = [System.AppDomain]::CurrentDomain.GetAssemblies() |
|
||||
Where-Object { $_.GetName().Name -eq 'PSProxmoxVE' } |
|
||||
ForEach-Object { $_.GetType('PSProxmoxVE.ModuleState') } |
|
||||
Select-Object -First 1
|
||||
$script:ModuleStateType | Should -Not -BeNullOrEmpty
|
||||
$script:Module = Get-Module PSProxmoxVE
|
||||
$script:Module | Should -Not -BeNullOrEmpty
|
||||
|
||||
$script:ActiveSessionProperty = $script:ModuleStateType.GetProperty(
|
||||
'ActiveSession',
|
||||
[System.Reflection.BindingFlags]'NonPublic, Static')
|
||||
$script:ActiveSessionProperty | Should -Not -BeNullOrEmpty
|
||||
$script:SetActiveSession = {
|
||||
param($Value)
|
||||
$script:Module.SessionState.PSVariable.Set('PSProxmoxVE.ActiveSession', $Value)
|
||||
}
|
||||
$script:GetActiveSession = {
|
||||
$script:Module.SessionState.PSVariable.GetValue('PSProxmoxVE.ActiveSession')
|
||||
}
|
||||
|
||||
$script:SessionCtor = $script:PveSessionType.GetConstructor(
|
||||
$script:SessionCtor = [PSProxmoxVE.Core.Authentication.PveSession].GetConstructor(
|
||||
[System.Reflection.BindingFlags]'NonPublic, Instance',
|
||||
$null,
|
||||
[type[]]@([string], [int], [bool], [string]),
|
||||
@@ -65,7 +64,7 @@ Describe 'Disconnect-PveServer' {
|
||||
}
|
||||
|
||||
AfterEach {
|
||||
$script:ActiveSessionProperty.SetValue($null, $null)
|
||||
& $script:SetActiveSession $null
|
||||
}
|
||||
|
||||
It 'Should report "no session" when nothing is active' {
|
||||
@@ -75,11 +74,11 @@ Describe 'Disconnect-PveServer' {
|
||||
|
||||
It 'Should clear the active session and warn on a second disconnect' {
|
||||
$activeSession = $script:SessionCtor.Invoke(@('active.example', 8006, $false, 'activetoken'))
|
||||
$script:ActiveSessionProperty.SetValue($null, $activeSession)
|
||||
& $script:SetActiveSession $activeSession
|
||||
|
||||
Disconnect-PveServer -Confirm:$false -WarningVariable w -WarningAction SilentlyContinue
|
||||
$w | Should -BeNullOrEmpty
|
||||
$script:ActiveSessionProperty.GetValue($null) | Should -BeNullOrEmpty
|
||||
& $script:GetActiveSession | Should -BeNullOrEmpty
|
||||
|
||||
Disconnect-PveServer -Confirm:$false -WarningVariable w2 -WarningAction SilentlyContinue
|
||||
$w2[0] | Should -Match 'No active Proxmox VE session'
|
||||
@@ -87,33 +86,41 @@ Describe 'Disconnect-PveServer' {
|
||||
|
||||
It 'Should warn when disconnecting an explicit non-active session, and leave the active session untouched' {
|
||||
$activeSession = $script:SessionCtor.Invoke(@('active.example', 8006, $false, 'activetoken'))
|
||||
$script:ActiveSessionProperty.SetValue($null, $activeSession)
|
||||
& $script:SetActiveSession $activeSession
|
||||
$mismatchedSession = $script:SessionCtor.Invoke(@('other.example', 8006, $false, 'othertoken'))
|
||||
|
||||
Disconnect-PveServer -Session $mismatchedSession -Confirm:$false -WarningVariable w -WarningAction SilentlyContinue
|
||||
|
||||
$w[0] | Should -Match 'not the module-level session'
|
||||
$w[0] | Should -Match 'Remove-PveApiToken'
|
||||
[object]::ReferenceEquals($script:ActiveSessionProperty.GetValue($null), $activeSession) | Should -BeTrue
|
||||
[object]::ReferenceEquals((& $script:GetActiveSession), $activeSession) | Should -BeTrue
|
||||
}
|
||||
|
||||
It 'Should disconnect when -Session matches the active session' {
|
||||
$activeSession = $script:SessionCtor.Invoke(@('active.example', 8006, $false, 'activetoken'))
|
||||
$script:ActiveSessionProperty.SetValue($null, $activeSession)
|
||||
& $script:SetActiveSession $activeSession
|
||||
|
||||
Disconnect-PveServer -Session $activeSession -Confirm:$false -WarningVariable w -WarningAction SilentlyContinue
|
||||
|
||||
$w | Should -BeNullOrEmpty
|
||||
$script:ActiveSessionProperty.GetValue($null) | Should -BeNullOrEmpty
|
||||
& $script:GetActiveSession | Should -BeNullOrEmpty
|
||||
}
|
||||
|
||||
It 'Should not clear the active session under -WhatIf' {
|
||||
$activeSession = $script:SessionCtor.Invoke(@('active.example', 8006, $false, 'activetoken'))
|
||||
$script:ActiveSessionProperty.SetValue($null, $activeSession)
|
||||
& $script:SetActiveSession $activeSession
|
||||
|
||||
Disconnect-PveServer -WhatIf -ErrorAction Stop
|
||||
|
||||
[object]::ReferenceEquals($script:ActiveSessionProperty.GetValue($null), $activeSession) | Should -BeTrue
|
||||
[object]::ReferenceEquals((& $script:GetActiveSession), $activeSession) | Should -BeTrue
|
||||
}
|
||||
|
||||
It 'Should see the session Test-PveConnection sees' {
|
||||
$activeSession = $script:SessionCtor.Invoke(@('active.example', 8006, $false, 'activetoken'))
|
||||
& $script:SetActiveSession $activeSession
|
||||
|
||||
Test-PveConnection | Should -BeTrue
|
||||
[object]::ReferenceEquals((Test-PveConnection -Detailed), $activeSession) | Should -BeTrue
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
#Requires -Module Pester
|
||||
<#
|
||||
.SYNOPSIS
|
||||
Pester 5 tests proving the active PVE session is per-runspace state, not a
|
||||
process-wide static shared by every runspace in the host.
|
||||
Fully offline — no live Proxmox VE target is required.
|
||||
#>
|
||||
|
||||
BeforeAll {
|
||||
. $PSScriptRoot/../_TestHelper.ps1
|
||||
|
||||
# A dll import produces a module with no session state, so the runspaces import the
|
||||
# manifest that sits beside it.
|
||||
$script:ModulePath = Join-Path (Split-Path (Get-Module PSProxmoxVE).Path) 'PSProxmoxVE.psd1'
|
||||
|
||||
function New-PveTestRunspace {
|
||||
param([string]$Path)
|
||||
|
||||
$ps = [powershell]::Create()
|
||||
$null = $ps.AddScript({
|
||||
param($p)
|
||||
Import-Module $p -Force -ErrorAction Stop
|
||||
}).AddArgument($Path)
|
||||
$null = $ps.Invoke()
|
||||
if ($ps.Streams.Error.Count -gt 0) {
|
||||
$err = $ps.Streams.Error[0]
|
||||
$ps.Dispose()
|
||||
throw "Runspace module import failed: $err"
|
||||
}
|
||||
$ps.Commands.Clear()
|
||||
$ps.Streams.Error.Clear()
|
||||
$ps
|
||||
}
|
||||
|
||||
function Invoke-InRunspace {
|
||||
param([System.Management.Automation.PowerShell]$Runspace, [scriptblock]$Script, $Argument)
|
||||
|
||||
$Runspace.Commands.Clear()
|
||||
$Runspace.Streams.Error.Clear()
|
||||
$null = $Runspace.AddScript($Script)
|
||||
if ($PSBoundParameters.ContainsKey('Argument')) { $null = $Runspace.AddArgument($Argument) }
|
||||
$out = $Runspace.Invoke()
|
||||
if ($Runspace.Streams.Error.Count -gt 0) {
|
||||
throw "Runspace script failed: $($Runspace.Streams.Error[0])"
|
||||
}
|
||||
, $out
|
||||
}
|
||||
}
|
||||
|
||||
Describe 'Active session isolation between runspaces' {
|
||||
BeforeAll {
|
||||
$script:SetSession = {
|
||||
param($hostname)
|
||||
$ctor = [PSProxmoxVE.Core.Authentication.PveSession].GetConstructor(
|
||||
[System.Reflection.BindingFlags]'NonPublic, Instance',
|
||||
$null,
|
||||
[type[]]@([string], [int], [bool], [string]),
|
||||
$null)
|
||||
$session = $ctor.Invoke(@($hostname, 8006, $false, 'token'))
|
||||
(Get-Module PSProxmoxVE).SessionState.PSVariable.Set('PSProxmoxVE.ActiveSession', $session)
|
||||
}
|
||||
|
||||
$script:ClearSession = {
|
||||
(Get-Module PSProxmoxVE).SessionState.PSVariable.Set('PSProxmoxVE.ActiveSession', $null)
|
||||
}
|
||||
|
||||
$script:GetHostname = {
|
||||
$detailed = Test-PveConnection -Detailed
|
||||
if ($null -eq $detailed) { '<none>' } else { $detailed.Hostname }
|
||||
}
|
||||
|
||||
$script:RunspaceA = New-PveTestRunspace -Path $script:ModulePath
|
||||
$script:RunspaceB = New-PveTestRunspace -Path $script:ModulePath
|
||||
}
|
||||
|
||||
AfterAll {
|
||||
if ($script:RunspaceA) { $script:RunspaceA.Dispose() }
|
||||
if ($script:RunspaceB) { $script:RunspaceB.Dispose() }
|
||||
}
|
||||
|
||||
It 'Should not expose one runspace''s session to another' {
|
||||
Invoke-InRunspace -Runspace $script:RunspaceB -Script $script:ClearSession
|
||||
Invoke-InRunspace -Runspace $script:RunspaceA -Script $script:SetSession -Argument 'runspace-a.example'
|
||||
|
||||
$connectedInB = Invoke-InRunspace -Runspace $script:RunspaceB -Script { Test-PveConnection }
|
||||
$connectedInB[0] | Should -BeFalse
|
||||
}
|
||||
|
||||
It 'Should keep each runspace on its own server after both connect' {
|
||||
Invoke-InRunspace -Runspace $script:RunspaceA -Script $script:SetSession -Argument 'runspace-a.example'
|
||||
Invoke-InRunspace -Runspace $script:RunspaceB -Script $script:SetSession -Argument 'runspace-b.example'
|
||||
|
||||
$inA = Invoke-InRunspace -Runspace $script:RunspaceA -Script $script:GetHostname
|
||||
$inB = Invoke-InRunspace -Runspace $script:RunspaceB -Script $script:GetHostname
|
||||
|
||||
$inA[0] | Should -Be 'runspace-a.example'
|
||||
$inB[0] | Should -Be 'runspace-b.example'
|
||||
}
|
||||
|
||||
It 'Should not let one runspace''s disconnect clear another''s session' {
|
||||
Invoke-InRunspace -Runspace $script:RunspaceA -Script $script:SetSession -Argument 'runspace-a.example'
|
||||
Invoke-InRunspace -Runspace $script:RunspaceB -Script $script:SetSession -Argument 'runspace-b.example'
|
||||
|
||||
Invoke-InRunspace -Runspace $script:RunspaceB -Script { Disconnect-PveServer -Confirm:$false }
|
||||
|
||||
$inA = Invoke-InRunspace -Runspace $script:RunspaceA -Script $script:GetHostname
|
||||
$inB = Invoke-InRunspace -Runspace $script:RunspaceB -Script $script:GetHostname
|
||||
$inA[0] | Should -Be 'runspace-a.example'
|
||||
$inB[0] | Should -Be '<none>'
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user