From 88e53f7f84bbbf7405aac351dd58735c463d5aaa Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 12 Nov 2026 11:24:00 -0500 Subject: [PATCH] fix(install): Install-ScriptWorkerIsolation.ps1 works on PowerShell 7 and 5.1, -Uninstall included (#883) First real run of the #780 installer (windev, #800 gate) failed on both hosts and its rollback failed too: - PowerShell 7: DirectoryInfo.GetAccessControl/SetAccessControl do not exist on .NET. Use Get-Acl / Set-Acl. - Windows PowerShell 5.1: an if-expression unrolled the wanted ACE to a scalar ($null on -Remove), so .Count threw under StrictMode. Wrap it in @(...). - Found on the re-run: an apply from 5.1 was not idempotent. 5.1 passes the literal quotes in sc.exe binPath= with legacy quoting, so the anchor path was stored unquoted and AnchorService reported 'changed' on every run. sc create/config now runs through Invoke-OtOpcUaSc with an explicit command line, identical on both hosts. Adds scripts/install/tests/Test-ScriptWorkerIsolation.Windows.ps1: a Windows-executed dry run -> apply -> re-apply -> uninstall -> uninstall test (scratch names, stand-in host service) for either host. The plan-only harness cannot see these. Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi --- scripts/install/ScriptWorkerIsolation.psm1 | 44 ++++-- .../Test-ScriptWorkerIsolation.Windows.ps1 | 128 ++++++++++++++++++ 2 files changed, 160 insertions(+), 12 deletions(-) create mode 100644 scripts/install/tests/Test-ScriptWorkerIsolation.Windows.ps1 diff --git a/scripts/install/ScriptWorkerIsolation.psm1 b/scripts/install/ScriptWorkerIsolation.psm1 index 2ff6622de..3bc0567ed 100644 --- a/scripts/install/ScriptWorkerIsolation.psm1 +++ b/scripts/install/ScriptWorkerIsolation.psm1 @@ -188,16 +188,17 @@ function Set-OtOpcUaExplicitAce { if ($Remove) { return 'unchanged' } throw "$Path does not exist" } - $item = Get-Item -LiteralPath $Path - $acl = $item.GetAccessControl('Access') + $acl = Get-Acl -LiteralPath $Path # #800: DirectoryInfo.GetAccessControl does not exist on PowerShell 7 (.NET) $identity = [System.Security.Principal.SecurityIdentifier]::new($Sid) $explicit = @($acl.GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | Where-Object { $_.IdentityReference -eq $identity }) $inherit = [System.Security.AccessControl.InheritanceFlags]'ContainerInherit, ObjectInherit' - $wanted = if ($Remove) { @() } else { - , [System.Security.AccessControl.FileSystemAccessRule]::new($identity, [System.Security.AccessControl.FileSystemRights]$Mask, + # #800: @() keeps $wanted an array; the if-expression unrolled it to a scalar ($null on -Remove), and .Count on that + # throws under Set-StrictMode -Version Latest on Windows PowerShell 5.1. + $wanted = @(if (-not $Remove) { + [System.Security.AccessControl.FileSystemAccessRule]::new($identity, [System.Security.AccessControl.FileSystemRights]$Mask, $inherit, [System.Security.AccessControl.PropagationFlags]::None, [System.Security.AccessControl.AccessControlType]$Type) - } + }) $same = ($explicit.Count -eq $wanted.Count) -and ( $wanted.Count -eq 0 -or ( [int]$explicit[0].FileSystemRights -eq $Mask -and $explicit[0].AccessControlType -eq $wanted[0].AccessControlType -and @@ -205,10 +206,27 @@ function Set-OtOpcUaExplicitAce { if ($same) { return 'unchanged' } $acl.PurgeAccessRules($identity) foreach ($rule in $wanted) { $acl.AddAccessRule($rule) } - $item.SetAccessControl($acl) + Set-Acl -LiteralPath $Path -AclObject $acl 'changed' } +function Invoke-OtOpcUaSc { + # Runs sc.exe with an explicitly quoted command line (CommandLineToArgvW rules), identical on PowerShell 5.1 and 7. + # Returns the exit code. Used where an argument contains literal double quotes. + param([string[]]$Arguments) + $quoted = foreach ($a in $Arguments) { '"' + ($a -replace '(\\*)"', '$1$1\"' -replace '(\\+)$', '$1$1') + '"' } + $psi = [System.Diagnostics.ProcessStartInfo]::new((Join-Path ([Environment]::SystemDirectory) 'sc.exe'), ($quoted -join ' ')) + $psi.UseShellExecute = $false + $psi.CreateNoWindow = $true + $psi.RedirectStandardOutput = $true + $psi.RedirectStandardError = $true + $proc = [System.Diagnostics.Process]::Start($psi) + $null = $proc.StandardOutput.ReadToEnd() + $null = $proc.StandardError.ReadToEnd() + $proc.WaitForExit() + $proc.ExitCode +} + function Set-OtOpcUaStepAce { param([hashtable]$Data) if ($Data.ContainsKey('Remove')) { return (Set-OtOpcUaExplicitAce -Path $Data.Path -Sid $Data.Sid -Remove) } @@ -237,9 +255,12 @@ function Invoke-ScriptWorkerIsolationStep { if ($cfg -and $cfg.StartMode -eq 'Disabled' -and $cfg.StartName -ieq $d.Identity -and $cfg.PathName -eq "`"$($d.BinPath)`"") { return 'unchanged' } - & sc.exe $verb $d.Name binPath= "`"$($d.BinPath)`"" start= disabled obj= $d.Identity ` - DisplayName= 'OtOpcUa ScriptWorker identity (anchor, never started)' | Out-Null - if ($LASTEXITCODE -ne 0) { throw "sc.exe $verb $($d.Name) failed ($LASTEXITCODE)" } + # #883: the binPath value carries literal quotes. Windows PowerShell 5.1 passes native arguments with legacy quoting + # (embedded quotes are dropped or mangled, so the stored path was unquoted and every re-run reported 'changed'), + # PowerShell 7 escapes them. Build the command line explicitly so both hosts store the same value. + $scExit = Invoke-OtOpcUaSc @($verb, $d.Name, 'binPath=', "`"$($d.BinPath)`"", 'start=', 'disabled', 'obj=', $d.Identity, + 'DisplayName=', 'OtOpcUa ScriptWorker identity (anchor, never started)') + if ($scExit -ne 0) { throw "sc.exe $verb $($d.Name) failed ($scExit)" } & sc.exe sidtype $d.Name unrestricted | Out-Null & sc.exe description $d.Name 'Anchors the NT SERVICE virtual account OtOpcUa ScriptWorker processes are restricted to (lmxopcua#780). Never started: the OtOpcUaHost launches workers itself under a restricted token.' | Out-Null return 'changed' @@ -252,12 +273,11 @@ function Invoke-ScriptWorkerIsolationStep { } $changed = $false if (-not (Test-Path -LiteralPath $d.Path)) { New-Item -ItemType Directory -Path $d.Path -Force | Out-Null; $changed = $true } - $item = Get-Item -LiteralPath $d.Path - $current = $item.GetAccessControl('Access, Owner').GetSecurityDescriptorSddlForm('Access, Owner') + $current = (Get-Acl -LiteralPath $d.Path).GetSecurityDescriptorSddlForm('Access, Owner') $desired = [System.Security.AccessControl.DirectorySecurity]::new() $desired.SetSecurityDescriptorSddlForm($d.Sddl, 'Access, Owner') if ($current -ne $desired.GetSecurityDescriptorSddlForm('Access, Owner')) { - $item.SetAccessControl($desired) # propagates to the key file, pe\ and workers\ (inherited ACEs) + Set-Acl -LiteralPath $d.Path -AclObject $desired # propagates to the key file, pe\ and workers\ (inherited ACEs) $changed = $true } return $(if ($changed) { 'changed' } else { 'unchanged' }) diff --git a/scripts/install/tests/Test-ScriptWorkerIsolation.Windows.ps1 b/scripts/install/tests/Test-ScriptWorkerIsolation.Windows.ps1 new file mode 100644 index 000000000..ecc118559 --- /dev/null +++ b/scripts/install/tests/Test-ScriptWorkerIsolation.Windows.ps1 @@ -0,0 +1,128 @@ +<# +.SYNOPSIS + Windows-executed test of Install-ScriptWorkerIsolation.ps1 (lmxopcua#883): apply -> re-apply -> verify -> uninstall -> uninstall. + The dry-run harness (Test-ScriptWorkerIsolation.ps1) only asserts plan data; it cannot see host-specific failures such as + DirectoryInfo.GetAccessControl missing on PowerShell 7, or StrictMode .Count on a scalar on Windows PowerShell 5.1. + Run it once under EACH host: + + pwsh -NoProfile -File scripts\install\tests\Test-ScriptWorkerIsolation.Windows.ps1 + powershell -NoProfile -ExecutionPolicy Bypass -File scripts\install\tests\Test-ScriptWorkerIsolation.Windows.ps1 + +.DESCRIPTION + Needs an elevated Windows session. It works in a scratch folder under $env:TEMP with a scratch worker service name and a + scratch stand-in host service (a disabled service that is never started), and uses -HostServiceAccount 'NT AUTHORITY\SYSTEM'. + It does NOT touch OtOpcUaHost. The firewall rule name is fixed by the installer, so the test refuses to run (exit 2) when + that rule already exists, i.e. on a node that has the real isolation installed. Everything it creates is removed in + `finally`, including on failure. +#> +[CmdletBinding()] +param([string]$WorkerServiceName = 'OtOpcUaIsoTestWorker', [string]$HostServiceName = 'OtOpcUaIsoTestHost') + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +if ($PSVersionTable.PSEdition -eq 'Core' -and -not $IsWindows) { Write-Host 'SKIP: Windows only'; exit 0 } +$principal = [Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent() +if (-not $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)) { Write-Host 'run elevated'; exit 2 } + +$installer = Join-Path (Split-Path -Parent $PSScriptRoot) 'Install-ScriptWorkerIsolation.ps1' +Import-Module (Join-Path (Split-Path -Parent $PSScriptRoot) 'ScriptWorkerIsolation.psm1') -Force +$ruleName = 'OtOpcUa-ScriptWorker-BlockOutbound' +if (Get-NetFirewallRule -Name $ruleName -ErrorAction SilentlyContinue) { Write-Host "refusing: firewall rule $ruleName already exists (real isolation installed on this node)"; exit 2 } +foreach ($n in $WorkerServiceName, $HostServiceName) { + if (Get-Service -Name $n -ErrorAction SilentlyContinue) { Write-Host "refusing: service $n already exists"; exit 2 } +} + +$script:failures = 0; $script:passes = 0 +function Assert([bool]$Condition, [string]$Message) { + if ($Condition) { $script:passes++ } else { $script:failures++; Write-Host " FAIL: $Message" -ForegroundColor Red } +} +function Invoke-Installer([string[]]$Extra) { + # Same PowerShell host as this test; the installer's output lines are ' [outcome ] StepId target'. + $exe = if ($PSVersionTable.PSEdition -eq 'Core') { (Get-Process -Id $PID).Path } else { 'powershell.exe' } + $out = & $exe -NoProfile -ExecutionPolicy Bypass -File $installer -InstallRoot $root -HostServiceAccount 'NT AUTHORITY\SYSTEM' ` + -PeCacheDir $cache -WorkerServiceName $WorkerServiceName -HostServiceName $HostServiceName -ConfigureHostService @Extra 2>&1 | Out-String + $steps = @([regex]::Matches($out, '\[(?[a-z]+)\s*\]\s+(?[A-Za-z]+)\s') | ForEach-Object { [pscustomobject]@{ Outcome = $_.Groups['o'].Value; Id = $_.Groups['id'].Value } }) + [pscustomobject]@{ Exit = $LASTEXITCODE; Output = $out; Steps = $steps } +} +function Get-Explicit([string]$Path, [string]$Sid) { + $id = [System.Security.Principal.SecurityIdentifier]::new($Sid) + @((Get-Acl -LiteralPath $Path).GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | Where-Object { $_.IdentityReference -eq $id }) +} + +$scratch = Join-Path ([IO.Path]::GetTempPath()) ("otopcua-iso-test-" + [guid]::NewGuid().ToString('N').Substring(0, 8)) +$root = Join-Path $scratch 'install' +$cache = Join-Path $scratch 'pe-cache' +$workerExe = Join-Path $root 'script-worker\ZB.MOM.WW.OtOpcUa.ScriptWorker.exe' +$workerSid = Get-OtOpcUaServiceSid -Name $WorkerServiceName +$hostKey = "HKLM:\SYSTEM\CurrentControlSet\Services\$HostServiceName" +try { + New-Item -ItemType Directory -Path (Split-Path $workerExe) -Force | Out-Null + Set-Content -LiteralPath $workerExe -Value 'stub' + & sc.exe create $HostServiceName binPath= "$env:SystemRoot\System32\cmd.exe" start= disabled | Out-Null + if ($LASTEXITCODE -ne 0) { throw "sc.exe create $HostServiceName failed" } + Set-ItemProperty -Path $hostKey -Name Environment -Type MultiString -Value ([string[]]@('Keep__Me=1')) + Write-Host "Host: $($PSVersionTable.PSEdition) $($PSVersionTable.PSVersion); scratch $scratch" + + Write-Host 'Dry run' + $dry = Invoke-Installer @('-WhatIf') + Assert ($dry.Exit -eq 0) "-WhatIf exits 0 ($($dry.Output))" + Assert (-not (Get-Service $WorkerServiceName -ErrorAction SilentlyContinue) -and -not (Test-Path $cache) -and + -not (Get-NetFirewallRule -Name $ruleName -ErrorAction SilentlyContinue)) '-WhatIf changed nothing' + + Write-Host 'Apply' + $a = Invoke-Installer @() + Assert ($a.Exit -eq 0) "apply exits 0 ($($a.Output))" + Assert ($a.Steps.Count -eq 7 -and @($a.Steps | Where-Object Outcome -ne 'changed').Count -eq 0) "apply: 7 steps, all changed ($(($a.Steps | ForEach-Object { "$($_.Id)=$($_.Outcome)" }) -join ','))" + Assert ((Get-CimInstance Win32_Service -Filter "Name='$WorkerServiceName'").StartMode -eq 'Disabled') 'anchor service is Disabled' + Assert ((& sc.exe showsid $WorkerServiceName | Out-String) -match [regex]::Escape($workerSid)) 'anchor SID matches Get-OtOpcUaServiceSid' + Assert ((Get-CimInstance Win32_Service -Filter "Name='$WorkerServiceName'").PathName -eq "`"$workerExe`"") 'anchor binPath is stored quoted (Windows PowerShell 5.1 used to drop the quotes)' + $pe = @(Get-Explicit (Join-Path $cache 'pe') $workerSid) + Assert ($pe.Count -eq 1 -and $pe[0].AccessControlType -eq 'Allow') 'worker has one Allow ACE on \pe' + Assert (@(Get-Explicit $cache $workerSid).Count -eq 0) 'worker has no ACE on the PeCacheDir root' + Assert ((Get-Acl -LiteralPath $cache).AreAccessRulesProtected) 'PeCacheDir DACL is protected' + $deny = @(Get-Explicit $root $workerSid) + Assert ($deny.Count -eq 1 -and $deny[0].AccessControlType -eq 'Deny') 'worker has one Deny ACE on the install root' + Assert (@(Get-Explicit (Join-Path $root 'script-worker') $workerSid).Count -eq 1) 'worker has one ACE on script-worker\' + $fw = Get-NetFirewallRule -Name $ruleName -ErrorAction SilentlyContinue + Assert ($null -ne $fw -and "$($fw.Direction)" -eq 'Outbound' -and "$($fw.Action)" -eq 'Block' -and (($fw | Get-NetFirewallApplicationFilter).Program -ieq $workerExe)) 'firewall rule is Outbound/Block on the worker exe' + $env1 = @((Get-ItemProperty -Path $hostKey -Name Environment).Environment) + Assert (($env1 -contains 'Keep__Me=1') -and (@($env1 | Where-Object { $_ -like 'Scripting__Worker__Identity=*' }).Count -eq 1) -and + (@($env1 | Where-Object { $_ -like 'Scripting__Worker__PeCacheDir=*' }).Count -eq 1)) 'host env has both Scripting__Worker__* entries and keeps the others' + + Write-Host 'Re-apply (idempotent)' + $b = Invoke-Installer @() + Assert ($b.Exit -eq 0) "re-apply exits 0 ($($b.Output))" + Assert ($b.Steps.Count -eq 7 -and @($b.Steps | Where-Object Outcome -ne 'unchanged').Count -eq 0) "re-apply: 7 steps, all unchanged ($(($b.Steps | ForEach-Object { "$($_.Id)=$($_.Outcome)" }) -join ','))" + + Write-Host 'Uninstall, twice' + $u1 = Invoke-Installer @('-Uninstall') + Assert ($u1.Exit -eq 0) "uninstall exits 0 ($($u1.Output))" + Assert ($u1.Steps.Count -eq 6 -and @($u1.Steps | Where-Object Outcome -ne 'changed').Count -eq 0) "uninstall: 6 steps, all changed ($(($u1.Steps | ForEach-Object { "$($_.Id)=$($_.Outcome)" }) -join ','))" + Assert (-not (Get-NetFirewallRule -Name $ruleName -ErrorAction SilentlyContinue)) 'firewall rule removed' + Assert (@(Get-Explicit $root $workerSid).Count -eq 0 -and @(Get-Explicit (Join-Path $root 'script-worker') $workerSid).Count -eq 0 -and + @(Get-Explicit (Join-Path $cache 'pe') $workerSid).Count -eq 0) 'no explicit worker ACEs remain' + Assert (Test-Path $cache) 'the PE cache is kept without -RemovePeCache' + $env2 = @((Get-ItemProperty -Path $hostKey -Name Environment).Environment) + Assert (($env2 -join '|') -eq 'Keep__Me=1') "host env restored ($($env2 -join '|'))" + Start-Sleep -Milliseconds 500 + $u2 = Invoke-Installer @('-Uninstall') + Assert ($u2.Exit -eq 0) "second uninstall exits 0 ($($u2.Output))" + Assert ($u2.Steps.Count -eq 6 -and @($u2.Steps | Where-Object Outcome -ne 'unchanged').Count -eq 0) "second uninstall: 6 steps, all unchanged ($(($u2.Steps | ForEach-Object { "$($_.Id)=$($_.Outcome)" }) -join ','))" +} +catch { $script:failures++; Write-Host " FAIL: unexpected error: $($_ | Out-String)" -ForegroundColor Red } +finally { + # Best-effort cleanup that does not depend on the code under test. + Remove-NetFirewallRule -Name $ruleName -ErrorAction SilentlyContinue + foreach ($n in $WorkerServiceName, $HostServiceName) { + if (Get-Service -Name $n -ErrorAction SilentlyContinue) { & sc.exe delete $n | Out-Null } + } + if (Test-Path -LiteralPath $scratch) { + & icacls $scratch /reset /T /C /Q 2>&1 | Out-Null + Remove-Item -LiteralPath $scratch -Recurse -Force -ErrorAction SilentlyContinue + } +} + +Write-Host '' +Write-Host "$($script:passes) passed, $($script:failures) failed" +if ($script:failures -gt 0) { exit 1 }