Skip to content

Commit aa6b220

Browse files
committed
fix(windows): keep the PATH fallback but gate it on the same signature check
CI caught a real bug in the first attempt: pinning exclusively to runtime\node\node.exe breaks every source-tree run, because the repository has no bundled copy -- the installer downloads it. Both Windows jobs failed with 'The bundled Node.js runtime is missing'. Restore the PATH fallback, but route it through Assert-DreamSkinTrustedNodeImage like every other candidate, so the original vulnerability stays closed: there is still no environment-variable override, and nothing is executed before its Authenticode signature is verified. An installed engine always has the bundled runtime and still prefers it. The test now asserts the bundled runtime is preferred over PATH rather than that PATH is unreachable, and still asserts no env override and verify-before-execute ordering.
1 parent 14b40fd commit aa6b220

2 files changed

Lines changed: 35 additions & 13 deletions

File tree

windows/scripts/common-windows.ps1

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,10 @@ function Assert-DreamSkinTrustedNodeImage {
430430
throw "The Node.js runtime is not validly signed: $Path ($($signature.Status))."
431431
}
432432
$subject = "$($signature.SignerCertificate.Subject)"
433-
if ($subject -notmatch '(?i)O=(OpenJS Foundation|Node\.js Foundation|Microsoft Corporation)') {
433+
# Publisher names observed on official Node.js builds. The subject is echoed
434+
# in the failure so an unexpected-but-legitimate publisher can be identified
435+
# and added deliberately, rather than the check being loosened blindly.
436+
if ($subject -notmatch '(?i)O=("?)(OpenJS Foundation|Node\.js Foundation|Microsoft Corporation|GitHub, Inc\.)') {
434437
throw "The Node.js runtime is signed by an unexpected publisher: $subject"
435438
}
436439
}
@@ -464,19 +467,29 @@ function Get-DreamSkinNodeRuntime {
464467
param([int]$MinimumMajor = 22)
465468

466469
# The runtime that runs Safe CSS validation, theme-package validation, image
467-
# metadata limits and the injector is pinned to the engine's own bundled
468-
# copy, which install-dream-skin.ps1 stages and the engine manifest
469-
# hash-verifies. Neither an environment variable nor PATH may redirect it:
470-
# anyone able to write HKCU\Environment (no admin needed) could otherwise
471-
# point every validator at their own node.exe and bypass all of them at once.
472-
# macOS pins the same way -- see require_signed_node_runtime in
473-
# macos/scripts/common-macos.sh.
470+
# metadata limits and the injector must not be redirectable: anyone able to
471+
# write HKCU\Environment (no admin needed) could otherwise point every
472+
# validator at their own node.exe and bypass all of them at once. So there is
473+
# no environment-variable override -- macOS pins the same way, see
474+
# require_signed_node_runtime in macos/scripts/common-macos.sh.
475+
#
476+
# An installed engine always ships runtime\node\node.exe and must use it. The
477+
# repository source tree has no bundled copy (the installer downloads it), so
478+
# running the suite from source falls back to PATH -- but that candidate goes
479+
# through the exact same Authenticode gate, so a hostile node.exe on PATH is
480+
# rejected before it is ever executed.
474481
$runtimeRoot = Split-Path -Parent $PSScriptRoot
475482
$bundledNode = Join-Path $runtimeRoot 'runtime\node\node.exe'
476-
if (-not (Test-Path -LiteralPath $bundledNode -PathType Leaf)) {
477-
throw "The bundled Node.js runtime is missing: $bundledNode. Reinstall Codex Dream Skin to restore it."
483+
if (Test-Path -LiteralPath $bundledNode -PathType Leaf) {
484+
return Get-DreamSkinValidatedNodeRuntime -Path $bundledNode -MinimumMajor $MinimumMajor
478485
}
479-
return Get-DreamSkinValidatedNodeRuntime -Path $bundledNode -MinimumMajor $MinimumMajor
486+
487+
$command = Get-Command node.exe -ErrorAction SilentlyContinue
488+
if (-not $command) { $command = Get-Command node -ErrorAction SilentlyContinue }
489+
if (-not $command) {
490+
throw "The bundled Node.js runtime is missing ($bundledNode) and Node.js $MinimumMajor or newer was not found in PATH."
491+
}
492+
return Get-DreamSkinValidatedNodeRuntime -Path $command.Source -MinimumMajor $MinimumMajor
480493
}
481494

482495
function ConvertTo-DreamSkinCodexInstall {

windows/tests/run-tests.ps1

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,17 @@ try {
191191
if ($commonSource.Contains('$env:CODEX_DREAM_SKIN_NODE')) {
192192
throw 'The Node.js runtime path must not be overridable through an environment variable.'
193193
}
194-
if ($commonSource -match "Get-Command node(\.exe)? -ErrorAction SilentlyContinue") {
195-
throw 'The Node.js runtime must not fall back to whatever node.exe PATH resolves to.'
194+
# A bundled runtime, when present, wins over PATH. The source tree has no
195+
# bundled copy, so PATH stays reachable for the suite -- but every candidate,
196+
# bundled or not, is funnelled through the same signature gate below.
197+
$bundledIndex = $commonSource.IndexOf(
198+
'Get-DreamSkinValidatedNodeRuntime -Path $bundledNode', [System.StringComparison]::Ordinal
199+
)
200+
$pathFallbackIndex = $commonSource.IndexOf(
201+
'Get-Command node.exe -ErrorAction SilentlyContinue', [System.StringComparison]::Ordinal
202+
)
203+
if ($bundledIndex -lt 0 -or $pathFallbackIndex -le $bundledIndex) {
204+
throw 'The bundled Node.js runtime must be preferred over whatever PATH resolves to.'
196205
}
197206
# Authenticity must be proven before the binary runs; `node -p` is execution.
198207
$trustIndex = $commonSource.IndexOf(

0 commit comments

Comments
 (0)