From 2452efa9211d09e0acdf8cf9bfbf82a96f5a3522 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Thu, 22 Oct 2026 01:03:44 -0700 Subject: [PATCH] fix(ci): make Windows PowerShell observers cheap and cold-start tolerant (#6279) The private-runtime ACL observer spent most of its time in PowerShell machinery: Get-Acl module autoload, an Update-TypeData guard that updates the session type table, and a Where-Object/ConvertTo-Json pipeline. On starved hosted runners this pushed it past the 15s deadline, and one run never reached the first script statement within 15s. Read the descriptor through DirectorySecurity with SID-typed GetOwner and GetAccessRules, check every compared identity is a SecurityIdentifier, keep Get-Acl/Access/NTAccount/Translate out of the observer with a static check, and build the JSON by string. Route the snapshot digest, readonly-children and sharing-reader observers through the same cmdlet-free helper. Record process uptime at script start, and give observers a 60s deadline that bounds a hung child while covering cold powershell.exe start. Asserted facts are unchanged. Fixes #6275 Co-authored-by: Claude Opus 5.5 --- docs/testing.md | 19 ++- scripts/verify-windows-native.mjs | 206 ++++++++++++++++-------------- 2 files changed, 124 insertions(+), 101 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 0deb82f19c..ef6e98da55 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -707,13 +707,18 @@ miss. Native artifacts, dependency installs, package archives, and runtime verification remain host-local. Keep platform-dependent outputs out of this shared cache. -The Windows native verifier's private-runtime ACL observer reads SID rules once -for both trustee validation and counting. It deliberately rejects the convenience -`Access` property, which resolves account names. Its `private-runtime-acl` stderr -markers report completed phases and elapsed time; inspect these on a timeout -before attributing it to startup, ACL reads, or identity lookup. The original -15-second deadline and all ownership, DACL, and readonly assertions remain in -force. A passing retry alone does not explain a previous timeout. +The Windows native verifier's private-runtime ACL observer reads the owner and +access rules only in SID form, checks that every identity it compares is a +`SecurityIdentifier`, and never uses `Get-Acl` or the convenience `Access` +property, which resolves account names. Its Windows PowerShell observers call +.NET members directly and build their JSON without cmdlets, pipelines, module +autoloading, or type-table updates. The `private-runtime-acl` stderr markers +report the powershell.exe process uptime and each completed phase; inspect them +on a timeout before attributing it to startup, ACL reads, or identity lookup. +The 60-second observer deadline bounds a hung observer while covering a cold +powershell.exe start on a starved hosted runner. All ownership, DACL, and +readonly assertions remain in force. A passing retry alone does not explain a +previous timeout. Record cold and warm preparation separately from fresh test execution when benchmarking the optional `npm run check:pr` gate. Compare multiple runs on the diff --git a/scripts/verify-windows-native.mjs b/scripts/verify-windows-native.mjs index 4077639889..8492cc5535 100644 --- a/scripts/verify-windows-native.mjs +++ b/scripts/verify-windows-native.mjs @@ -56,6 +56,81 @@ const windowsPowerShellEnvironment = Object.fromEntries( ([name]) => name.toLowerCase() !== "psmodulepath", ), ); +// Windows PowerShell observers call .NET members directly: no cmdlets, +// pipelines, module autoloading, type-table updates or JSON serializer, which +// made up most of the earlier ACL observer's 1.2-2.9s in-script time on hosted +// runners. The deadline bounds a hung observer rather than the observation. It +// must also cover a cold powershell.exe start on a starved runner: one run +// printed nothing within 15s, and another ran the same phases ~20x slower. +const windowsPowerShellObserverTimeoutMs = 60_000; +const windowsPowerShellArguments = (script) => [ + "-NoProfile", + "-NonInteractive", + "-Command", + script, +]; +// Shared prelude: fail on the first error and emit JSON booleans without +// loading ConvertTo-Json. +const windowsPowerShellObserverPrelude = String.raw` + $ErrorActionPreference = 'Stop' + function Format-JsonBoolean([bool]$value) { if ($value) { 'true' } else { 'false' } } +`; +const observeWithWindowsPowerShell = (script, observerEnvironment) => + execFileSync( + "powershell.exe", + windowsPowerShellArguments(windowsPowerShellObserverPrelude + script), + { + encoding: "utf8", + timeout: windowsPowerShellObserverTimeoutMs, + windowsHide: true, + env: { ...windowsPowerShellEnvironment, ...observerEnvironment }, + }, + ); +// Independent private-runtime DACL observer. Every identity it reads is +// requested and checked in SID form; it never resolves account names. Get-Acl +// and the ETS Access property are avoided: Access requests NTAccount rules. +const privateRuntimeAclObserver = String.raw` + $clock = [Diagnostics.Stopwatch]::StartNew() + function Trace-AclPhase([string]$phase) { + [Console]::Error.WriteLine('private-runtime-acl: ' + $phase + ' ' + $clock.ElapsedMilliseconds + 'ms') + } + Trace-AclPhase 'started' + # Separates powershell.exe startup from the observation itself. + [Console]::Error.WriteLine('private-runtime-acl: process-uptime ' + [int]([DateTime]::Now - [Diagnostics.Process]::GetCurrentProcess().StartTime).TotalMilliseconds + 'ms') + $p = $env:REA_TEST_PRIVATE_RUNTIME_PATH + $sid = [Security.Principal.SecurityIdentifier] + $a = [Security.AccessControl.DirectorySecurity]::new($p, [Security.AccessControl.AccessControlSections]'Access, Owner') + Trace-AclPhase 'acl-read' + $u = [Security.Principal.WindowsIdentity]::GetCurrent().User + $owner = $a.GetOwner($sid) + if ($owner -isnot $sid) { throw 'ACL owner observation must stay in SID form' } + Trace-AclPhase 'owner-sid-read' + $rules = $a.GetAccessRules($true, $true, $sid) + $trusteesOnlyUserAndSystem = $true + foreach ($rule in $rules) { + $trustee = $rule.IdentityReference + if ($trustee -isnot $sid) { throw 'ACL rule observation must stay in SID form' } + if ($trustee.Value -ne $u.Value -and $trustee.Value -ne 'S-1-5-18') { $trusteesOnlyUserAndSystem = $false } + } + Trace-AclPhase 'rule-sids-read' + [IO.File]::SetAttributes($p, [IO.FileAttributes]([int][IO.File]::GetAttributes($p) -bor 1)) + Trace-AclPhase 'readonly-set' + [Console]::Out.Write( + '{"protected":' + (Format-JsonBoolean $a.AreAccessRulesProtected) + + ',"ownerMatchesUser":' + (Format-JsonBoolean ($owner.Value -eq $u.Value)) + + ',"trusteesOnlyUserAndSystem":' + (Format-JsonBoolean $trusteesOnlyUserAndSystem) + + ',"count":' + $rules.Count + + ',"rootReadonly":' + (Format-JsonBoolean (([int][IO.File]::GetAttributes($p) -band 1) -ne 0)) + '}') + Trace-AclPhase 'complete' +`; +// Keep account-name resolution out of the observer statically. The former +// Update-TypeData guard reloaded PowerShell's type data at runtime; its phase +// took 0.3-9.5s on hosted runners. +assert.doesNotMatch( + privateRuntimeAclObserver, + /Get-Acl|\.Access\b|NTAccount|\.Translate\b/u, + "The private-runtime ACL observer must read only SID forms.", +); const environment = Object.entries(process.env) .filter(([, value]) => value !== undefined) .map(([key, value]) => `${key}=${value}`); @@ -70,25 +145,23 @@ const exists = (path) => const startDeleteSharingDeniedReader = async (path) => { const child = spawn( "powershell.exe", - [ - "-NoProfile", - "-NonInteractive", - "-Command", + windowsPowerShellArguments( "$ErrorActionPreference = 'Stop'; " + "$stream = [IO.File]::Open($env:REA_CLEANUP_HELD_PATH, [IO.FileMode]::Open, [IO.FileAccess]::Read, [IO.FileShare]::ReadWrite); " + - "try { [Console]::WriteLine('ready'); [Console]::In.ReadLine() | Out-Null } " + + "try { [Console]::WriteLine('ready'); [void][Console]::In.ReadLine() } " + "finally { $stream.Dispose() }", - ], + ), { - env: { ...process.env, REA_CLEANUP_HELD_PATH: path }, + env: { ...windowsPowerShellEnvironment, REA_CLEANUP_HELD_PATH: path }, stdio: ["pipe", "pipe", "pipe"], + windowsHide: true, }, ); const closed = once(child, "close"); void closed.catch(() => undefined); try { const [ready] = await once(child.stdout, "data", { - signal: AbortSignal.timeout(15_000), + signal: AbortSignal.timeout(windowsPowerShellObserverTimeoutMs), }); assert.equal(String(ready).trim(), "ready"); } catch (cause) { @@ -363,54 +436,9 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe const runtimeVerificationErrors = []; try { const acl = JSON.parse( - execFileSync( - "powershell.exe", - [ - "-NoProfile", - "-NonInteractive", - "-Command", - String.raw` - $ErrorActionPreference = 'Stop' - $clock = [Diagnostics.Stopwatch]::StartNew() - function Trace-AclPhase([string]$phase) { - [Console]::Error.WriteLine('private-runtime-acl: ' + $phase + ' ' + $clock.ElapsedMilliseconds + 'ms') - } - Trace-AclPhase 'started' - $p = $env:REA_TEST_PRIVATE_RUNTIME_PATH - $a = Get-Acl -LiteralPath $p - Trace-AclPhase 'acl-read' - $u = [Security.Principal.WindowsIdentity]::GetCurrent().User - $owner = $a.GetOwner([Security.Principal.SecurityIdentifier]) - Trace-AclPhase 'owner-sid-read' - # Guard the independent observer against accidental account-name resolution. - # Access is a PowerShell convenience property that requests NTAccount rules. - Update-TypeData -TypeName ($a.GetType().FullName) -MemberType ScriptProperty -MemberName Access -Value { throw 'ACL observation must use SID rules' } -Force - if ($a.PSObject.Properties['Access'].MemberType -ne [Management.Automation.PSMemberTypes]::ScriptProperty) { - throw 'ACL observation Access guard was not installed' - } - $rules = @($a.GetAccessRules($true, $true, [Security.Principal.SecurityIdentifier])) - Trace-AclPhase 'rule-sids-read' - [IO.File]::SetAttributes($p, [IO.FileAttributes]([int][IO.File]::GetAttributes($p) -bor 1)) - Trace-AclPhase 'readonly-set' - @{ - protected = $a.AreAccessRulesProtected - ownerMatchesUser = ($owner.Value -eq $u.Value) - trusteesOnlyUserAndSystem = (@($rules | Where-Object { $_.IdentityReference.Value -ne $u.Value -and $_.IdentityReference.Value -ne 'S-1-5-18' }).Count -eq 0) - count = $rules.Count - rootReadonly = (([int][IO.File]::GetAttributes($p) -band 1) -ne 0) - } | ConvertTo-Json -Compress - Trace-AclPhase 'complete' - `, - ], - { - encoding: "utf8", - timeout: 15_000, - env: { - ...windowsPowerShellEnvironment, - REA_TEST_PRIVATE_RUNTIME_PATH: runtime.path, - }, - }, - ), + observeWithWindowsPowerShell(privateRuntimeAclObserver, { + REA_TEST_PRIVATE_RUNTIME_PATH: runtime.path, + }), ); assert.deepEqual(acl, { protected: true, @@ -548,27 +576,15 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe ); // Java RandomAccessFile uses read/write sharing without delete sharing. // Exercise that independent Win32 reader contract without requiring Java. - const readerDigest = execFileSync( - "powershell.exe", - [ - "-NoProfile", - "-NonInteractive", - "-Command", - "$ErrorActionPreference = 'Stop'; " + - "$stream = [IO.File]::Open($env:REA_SNAPSHOT_READER_PATH, [IO.FileMode]::Open, [IO.FileAccess]::Read, [IO.FileShare]::ReadWrite); " + - "$hash = [Security.Cryptography.SHA256]::Create(); " + - "try { [BitConverter]::ToString($hash.ComputeHash($stream)).Replace('-', '').ToLowerInvariant() } " + - "finally { $hash.Dispose(); $stream.Dispose() }", - ], - { - encoding: "utf8", - timeout: 15_000, - env: { - ...windowsPowerShellEnvironment, - REA_SNAPSHOT_READER_PATH: snapshotPath, - }, - }, - ).trim(); + const readerDigest = observeWithWindowsPowerShell( + String.raw` + $stream = [IO.File]::Open($env:REA_SNAPSHOT_READER_PATH, [IO.FileMode]::Open, [IO.FileAccess]::Read, [IO.FileShare]::ReadWrite) + $hash = [Security.Cryptography.SHA256]::Create() + try { [Console]::Out.Write([BitConverter]::ToString($hash.ComputeHash($stream)).Replace('-', '').ToLowerInvariant()) } + finally { $hash.Dispose(); $stream.Dispose() } + `, + { REA_SNAPSHOT_READER_PATH: snapshotPath }, + ); assert.equal(readerDigest, readbackSnapshot.sha256); await assert.rejects(writeFile(snapshotPath, "replace")); await assert.rejects(rename(snapshotPath, `${snapshotPath}.renamed`)); @@ -701,25 +717,27 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe asynchronousSnapshot: true, snapshotCancellation: true, }; + // Paths are newline-separated: Windows file names cannot contain control + // characters, and the observer then needs no JSON parser. const readonlyChildren = JSON.parse( - execFileSync( - "powershell.exe", - [ - "-NoProfile", - "-NonInteractive", - "-Command", - "$ErrorActionPreference='Stop'; $items=ConvertFrom-Json -InputObject $env:REA_TEST_READONLY_CHILDREN; @($items | ForEach-Object { $current=[int][IO.File]::GetAttributes($_); $next=if(($current -band 16) -ne 0){$current -bor 1}else{1}; [IO.File]::SetAttributes($_,[IO.FileAttributes]$next); $a=[int][IO.File]::GetAttributes($_); @{readonly=(($a -band 1) -ne 0); directory=(($a -band 16) -ne 0); onlyReadonly=($a -eq 1)} }) | ConvertTo-Json -Compress", - ], + observeWithWindowsPowerShell( + String.raw` + $observations = foreach ($item in $env:REA_TEST_READONLY_CHILDREN.Split([char]10)) { + $current = [int][IO.File]::GetAttributes($item) + $next = if (($current -band 16) -ne 0) { $current -bor 1 } else { 1 } + [IO.File]::SetAttributes($item, [IO.FileAttributes]$next) + $a = [int][IO.File]::GetAttributes($item) + '{"readonly":' + (Format-JsonBoolean (($a -band 1) -ne 0)) + + ',"directory":' + (Format-JsonBoolean (($a -band 16) -ne 0)) + + ',"onlyReadonly":' + (Format-JsonBoolean ($a -eq 1)) + '}' + } + [Console]::Out.Write('[' + ($observations -join ',') + ']') + `, { - encoding: "utf8", - timeout: 15_000, - env: { - ...windowsPowerShellEnvironment, - REA_TEST_READONLY_CHILDREN: JSON.stringify([ - join(runtime.path, "inputs"), - join(runtime.path, "inputs", "target.bin"), - ]), - }, + REA_TEST_READONLY_CHILDREN: [ + join(runtime.path, "inputs"), + join(runtime.path, "inputs", "target.bin"), + ].join("\n"), }, ), );