From db04ef27601e255973df7eb644e7ef51a91fdd37 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Sat, 10 Oct 2026 02:47:53 +0000 Subject: [PATCH] test(lab): harden the kit after the independent review The review of the matrix kit found no Blocker or Major issue and these Minor ones, all fixed here: - Add-OsMatrixMachine.ps1 assigned the path of the AutomatedLab disk deployment lock before it checked that the lock exists, so a refusal because another deployment held the lock made the finally block delete that foreign lock. The path is kept until this script has created the lock. - Repair-OsMatrixBoot.ps1 tested the switches of the machine with an array -ne, which is false for a machine without an adapter, so the guard that is meant to refuse a machine outside the lab let it through and the script turned it off. The guard counts the switches now. - Deploy-OsMatrixLab.ps1 took the installation and domain administrator password of the lab from Get-Random, which isn't a cryptographic generator. It uses RandomNumberGenerator without a remainder bias, as the controller does. - Run-MatrixLocalSuite.ps1 removed its scheduled tasks, which store the password of the account that runs them, only after a successful poll. The finally block of the machine removes the tasks of the run now. - Probe-EffectiveAccess.ps1 cleaned up the domain controller before the machine without a try block, so a failure there skipped the cleanup of the machine. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- Tests/Lab/Acceptance/Add-OsMatrixMachine.ps1 | 8 +++++--- Tests/Lab/Acceptance/Deploy-OsMatrixLab.ps1 | 13 +++++++++++-- .../Lab/Acceptance/Probe-EffectiveAccess.ps1 | 19 +++++++++++++------ Tests/Lab/Acceptance/Repair-OsMatrixBoot.ps1 | 4 +++- Tests/Lab/Acceptance/Run-MatrixLocalSuite.ps1 | 15 ++++++++++++++- 5 files changed, 46 insertions(+), 13 deletions(-) diff --git a/Tests/Lab/Acceptance/Add-OsMatrixMachine.ps1 b/Tests/Lab/Acceptance/Add-OsMatrixMachine.ps1 index 3ee1c50..de4de2c 100644 --- a/Tests/Lab/Acceptance/Add-OsMatrixMachine.ps1 +++ b/Tests/Lab/Acceptance/Add-OsMatrixMachine.ps1 @@ -71,9 +71,11 @@ try { if ($difference.Count -gt 0) { throw "The exported lab doesn't hold exactly the old machines plus $Name. Restore the metadata from $backup." } Write-Step 'definition extended and exported' - $lockPath = Get-LabConfigurationItem -Name DiskDeploymentInProgressPath - if (Test-Path -LiteralPath $lockPath) { throw "Another lab disk deployment seems to be in progress ($lockPath)." } - $null = New-Item -Path $lockPath -ItemType File -Value $LabName + $lockCandidate = Get-LabConfigurationItem -Name DiskDeploymentInProgressPath + if (Test-Path -LiteralPath $lockCandidate) { throw "Another lab disk deployment seems to be in progress ($lockCandidate)." } + $null = New-Item -Path $lockCandidate -ItemType File -Value $LabName + # Only a lock that this script created is removed in the finally block below. + $lockPath = $lockCandidate New-LabBaseImages Write-Step 'base images ready' diff --git a/Tests/Lab/Acceptance/Deploy-OsMatrixLab.ps1 b/Tests/Lab/Acceptance/Deploy-OsMatrixLab.ps1 index 5d9a1c2..3d7c159 100644 --- a/Tests/Lab/Acceptance/Deploy-OsMatrixLab.ps1 +++ b/Tests/Lab/Acceptance/Deploy-OsMatrixLab.ps1 @@ -51,8 +51,17 @@ try { Write-Step ('preflight ok; existing labs: {0}; existing machine names: {1}' -f ($labs -join ', '), $existingNames.Count) $characters = ([char[]](48..57) + [char[]](65..90) + [char[]](97..122) + '!', '#', '%', '+', '-', '=') - $password = -join (1..24 | ForEach-Object { $characters | Get-Random }) - $password = 'Aa1!' + $password + # A cryptographic generator, without the bias of a remainder: this is the installation and domain administrator password of the lab. + $generator = [Security.Cryptography.RandomNumberGenerator]::Create() + $limit = 256 - (256 % $characters.Count) + $buffer = New-Object -TypeName 'byte[]' -ArgumentList 1 + $chosen = New-Object -TypeName 'System.Text.StringBuilder' + while ($chosen.Length -lt 24) { + $generator.GetBytes($buffer) + if ($buffer[0] -lt $limit) { $null = $chosen.Append($characters[$buffer[0] % $characters.Count]) } + } + + $password = 'Aa1!' + $chosen.ToString() New-LabDefinition -Name $LabName -DefaultVirtualizationEngine HyperV -VmPath $VmPath Add-LabVirtualNetworkDefinition -Name $LabName -AddressSpace $AddressSpace diff --git a/Tests/Lab/Acceptance/Probe-EffectiveAccess.ps1 b/Tests/Lab/Acceptance/Probe-EffectiveAccess.ps1 index 93c3bf1..399a1d6 100644 --- a/Tests/Lab/Acceptance/Probe-EffectiveAccess.ps1 +++ b/Tests/Lab/Acceptance/Probe-EffectiveAccess.ps1 @@ -204,13 +204,20 @@ try { finally { # The domain account goes first: its member entry on the machine is then an orphaned SID, which the cleanup of the machine removes. if ($dcSession) { - $dcLeftOver = Invoke-Command -Session $dcSession -ArgumentList $domainUser -ScriptBlock { - param ($Name) - Import-Module -Name ActiveDirectory - if (Get-ADUser -Filter "SamAccountName -eq '$Name'") { Remove-ADUser -Identity $Name -Confirm:$false } - if (Get-ADUser -Filter "SamAccountName -eq '$Name'") { "domain user $Name still exists" } + # A failure here must not skip the cleanup of the machine below. + try { + $dcLeftOver = Invoke-Command -Session $dcSession -ArgumentList $domainUser -ScriptBlock { + param ($Name) + Import-Module -Name ActiveDirectory + if (Get-ADUser -Filter "SamAccountName -eq '$Name'") { Remove-ADUser -Identity $Name -Confirm:$false } + if (Get-ADUser -Filter "SamAccountName -eq '$Name'") { "domain user $Name still exists" } + } + Write-Step ('cleanup of the domain controller: ' + $(if (@($dcLeftOver).Count -eq 0) { 'nothing left' } else { @($dcLeftOver) -join '; ' })) } - Write-Step ('cleanup of the domain controller: ' + $(if (@($dcLeftOver).Count -eq 0) { 'nothing left' } else { @($dcLeftOver) -join '; ' })) + catch { + Write-Step ("cleanup of the domain controller FAILED, remove the domain user $domainUser by hand: " + $_.Exception.Message) + } + Remove-PSSession -Session $dcSession -ErrorAction SilentlyContinue } diff --git a/Tests/Lab/Acceptance/Repair-OsMatrixBoot.ps1 b/Tests/Lab/Acceptance/Repair-OsMatrixBoot.ps1 index 90936c2..42549e4 100644 --- a/Tests/Lab/Acceptance/Repair-OsMatrixBoot.ps1 +++ b/Tests/Lab/Acceptance/Repair-OsMatrixBoot.ps1 @@ -26,7 +26,9 @@ try { $vm = Get-VM -Name $VmName if ($vm.Generation -ne 2) { throw "$VmName isn't a generation 2 machine." } $switches = @(Get-VMNetworkAdapter -VMName $VmName | ForEach-Object -Process { $_.SwitchName }) - if ($switches -ne $LabName) { throw "$VmName isn't connected only to the switch '$LabName' (switches: $($switches -join ', ')). Refusing." } + # An array comparison with -ne returns the elements that differ, and an empty result is false: a machine without an adapter, or with an + # adapter that has no switch, would pass, so the guard counts. + if ($switches.Count -eq 0 -or @($switches | Where-Object -FilterScript { $_ -ne $LabName }).Count -gt 0) { throw "$VmName isn't connected only to the switch '$LabName' (switches: $($switches -join ', ')). Refusing." } if ($vm.State -ne 'Off') { Stop-VM -Name $VmName -TurnOff -Force Write-Step "$VmName turned off" diff --git a/Tests/Lab/Acceptance/Run-MatrixLocalSuite.ps1 b/Tests/Lab/Acceptance/Run-MatrixLocalSuite.ps1 index 66da46b..1b9d7eb 100644 --- a/Tests/Lab/Acceptance/Run-MatrixLocalSuite.ps1 +++ b/Tests/Lab/Acceptance/Run-MatrixLocalSuite.ps1 @@ -225,7 +225,20 @@ foreach ($name in $targets) { Write-Sequence "machine $name FAILED: $_" } finally { - if ($session) { Remove-PSSession -Session $session -ErrorAction SilentlyContinue } + if ($session) { + # The tasks of this run store the password of the account that runs them. A run that stops early must not leave them on the machine. + try { + Invoke-Command -Session $session -ArgumentList ('NtfsMatrixLocal-{0}-*' -f $Label.ToLowerInvariant()) -ScriptBlock { + param ($Pattern) + Get-ScheduledTask -TaskName $Pattern -ErrorAction SilentlyContinue | ForEach-Object -Process { Unregister-ScheduledTask -TaskName $_.TaskName -Confirm:$false -ErrorAction SilentlyContinue } + } + } + catch { + Write-Sequence "machine ${name}: the scheduled tasks of this run could not be removed: $($_.Exception.Message)" + } + + Remove-PSSession -Session $session -ErrorAction SilentlyContinue + } } Write-Sequence "machine $name END"