diff --git a/scripts/desktop-update/runtime.ps1 b/scripts/desktop-update/runtime.ps1 index 283b02b0db..46bf8eec9d 100644 --- a/scripts/desktop-update/runtime.ps1 +++ b/scripts/desktop-update/runtime.ps1 @@ -9,7 +9,20 @@ function Get-HermesRuntimeCommand { foreach ($name in @('hermes.exe', 'hermes.cmd')) { $launcher = Join-Path $InstallRoot ".hermes\bin\$name" if (Test-Path -LiteralPath $launcher -PathType Leaf) { - $json = & $launcher --print-runtime-command --module $Module + # A machine boundary that names executables under the profile. + # The JSON is ASCII-escaped today (json.dumps default), but every + # other consumer of this boundary (hermes_cli.windows_ssh_runtime, + # the Electron updater, the Rust bootstrap) decodes it as UTF-8; + # scope the capture to the same contract so the boundary stays + # byte-exact if the emitter ever stops escaping, and so a + # non-ASCII path cannot arrive OEM-decoded (#124526). + $previousNativeOutputEncoding = [Console]::OutputEncoding + try { + [Console]::OutputEncoding = New-Object System.Text.UTF8Encoding($false) + $json = & $launcher --print-runtime-command --module $Module + } finally { + [Console]::OutputEncoding = $previousNativeOutputEncoding + } if ($LASTEXITCODE) { throw "Installation launcher failed (exit $LASTEXITCODE): $launcher" } $command = @((($json -join "`n") | ConvertFrom-Json)) if ($command.Count -lt 2 -or @($command | Where-Object { $_ -isnot [string] -or -not $_ }).Count) { @@ -31,7 +44,18 @@ function Get-HermesRuntimeCommand { foreach ($name in @('hermes.exe', 'hermes.cmd')) { $legacy = Join-Path $directory $name if (-not (Test-Path -LiteralPath $legacy -PathType Leaf)) { continue } - $version = (& $legacy --version 2>$null) -join "`n" + # The "Install directory:" line is UTF-8 (hermes_bootstrap + # reconfigures the CLI's stdio on import); PS 5.1 decodes + # captured native stdout with the console OEM code page, so a + # profile path like C:\Users\Balázs arrives mojibaked and the + # GetFullPath comparison below never matches (#124526). + $previousNativeOutputEncoding = [Console]::OutputEncoding + try { + [Console]::OutputEncoding = New-Object System.Text.UTF8Encoding($false) + $version = (& $legacy --version 2>$null) -join "`n" + } finally { + [Console]::OutputEncoding = $previousNativeOutputEncoding + } if ($LASTEXITCODE -or $version -notmatch '(?m)^Install directory: (.+)\r?$') { continue } $reported = [IO.Path]::GetFullPath($Matches[1].Trim()).TrimEnd('\', '/') $expected = [IO.Path]::GetFullPath($InstallRoot).TrimEnd('\', '/') diff --git a/scripts/install.ps1 b/scripts/install.ps1 index c389d57f24..ba1ea26a6c 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -330,6 +330,26 @@ function Initialize-ResolvedPaths { } else { Join-Path $resolvedHome 'hermes-agent' } + # A HermesHome equal to or inside InstallDir puts the pm tool store + # (\tools) inside the checkout: the repository stage's + # occupied-directory preflight then refuses every retry after the first + # run populated it, and `git stash --include-untracked` would sweep the + # toolchain into the stash. Refuse before anything downloads (#124526) + # unless HERMES_RUNTIME_DIR parks the store outside the checkout. + $cmpHome = "$resolvedHome".TrimEnd('\', '/') + $cmpDir = "$resolvedDir".TrimEnd('\', '/') + $dirPrefix = $cmpDir + [IO.Path]::DirectorySeparatorChar + $storeInside = $cmpHome -eq $cmpDir -or + $cmpHome.StartsWith($dirPrefix, [StringComparison]::OrdinalIgnoreCase) + if (-not $storeInside -and $env:HERMES_RUNTIME_DIR) { + # The override owns the store; only it decides where that lands. + $cmpStore = "$env:HERMES_RUNTIME_DIR".TrimEnd('\', '/') + $storeInside = $cmpStore -eq $cmpDir -or + $cmpStore.StartsWith($dirPrefix, [StringComparison]::OrdinalIgnoreCase) + } + if ($storeInside) { + Fail "HermesHome ($resolvedHome) cannot be the install directory or live inside it ($resolvedDir): the tool store would land inside the checkout. Use a separate -HermesHome, or point HERMES_RUNTIME_DIR outside -InstallDir." + } # The param() variables live in the CALLER's scope, which is the script # scope only under -File. Under the documented # `& ([scriptblock]::Create((irm ...)))` install they live in the diff --git a/scripts/tests/test-install-ps1-store-guard.ps1 b/scripts/tests/test-install-ps1-store-guard.ps1 new file mode 100644 index 0000000000..fdfa2e596a --- /dev/null +++ b/scripts/tests/test-install-ps1-store-guard.ps1 @@ -0,0 +1,158 @@ +# Tests for install.ps1's HermesHome/InstallDir separation guard (#124526). +# +# Run from a PowerShell prompt: +# +# pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/tests/test-install-ps1-store-guard.ps1 +# +# Background: passing -HermesHome and -InstallDir as the same path (or a +# HermesHome nested inside -InstallDir) makes the pm tool store land at +# \tools, INSIDE the checkout. Two failures follow: the +# repository stage's occupied-directory preflight refuses every retry after +# the first run populated the folder ("exists and is not a Hermes git +# checkout"), and `git stash --include-untracked` would sweep the toolchain +# into the stash. The installer now refuses the combination up front, before +# any download, unless HERMES_RUNTIME_DIR parks the store elsewhere. +# +# HOW THIS RUNS THE CODE: by executing install.ps1 as a real subprocess with +# crafted -HermesHome / -InstallDir arguments. -ShowResolvedPaths is a +# side-effect-free early exit that runs AFTER Initialize-ResolvedPaths, so +# the guard executes exactly as during an install. The refusal is a `throw`, +# which under -File exits the child non-zero before the JSON is printed; the +# accepted configurations print the resolved-path JSON with the requested +# paths intact. Nothing here parses install.ps1's source (AGENTS.md bans +# source-reading tests: they pass on broken code and fail on correct +# refactors). + +$ErrorActionPreference = "Stop" +$repoRoot = Split-Path -Parent (Split-Path -Parent (Split-Path -Parent $MyInvocation.MyCommand.Path)) +$installScript = Join-Path $repoRoot "scripts/install.ps1" + +if (-not (Test-Path $installScript)) { + throw "Could not locate install.ps1 at $installScript" +} + +$failures = 0 + +function Assert-True { + param([bool]$Condition, [Parameter(Mandatory = $true)][string]$Label) + if (-not $Condition) { + Write-Host "FAIL: $Label" -ForegroundColor Red + $script:failures++ + } else { + Write-Host "OK: $Label" -ForegroundColor Green + } +} + +# Run install.ps1 -ShowResolvedPaths with explicit -HermesHome/-InstallDir and +# capture (exit code, stdout, stderr). The call operator, not Start-Process: +# `&` inherits this process's environment on every host, and stderr is merged +# so Windows PowerShell 5.1's NativeCommandError records cannot fail the lane. +function Invoke-ResolvedPaths { + param([string[]]$ExtraArgs = @()) + + $psExe = (Get-Process -Id $PID).Path + $outFile = [System.IO.Path]::GetTempFileName() + $savedRuntimeDir = [Environment]::GetEnvironmentVariable('HERMES_RUNTIME_DIR') + $savedHermesHome = [Environment]::GetEnvironmentVariable('HERMES_HOME') + try { + if ($null -eq $savedRuntimeDir) { + Remove-Item Env:HERMES_RUNTIME_DIR -ErrorAction SilentlyContinue + } else { + $env:HERMES_RUNTIME_DIR = $savedRuntimeDir + } + if ($null -eq $savedHermesHome) { + Remove-Item Env:HERMES_HOME -ErrorAction SilentlyContinue + } else { + $env:HERMES_HOME = $savedHermesHome + } + $callArgs = @('-NoProfile', '-ExecutionPolicy', 'Bypass', '-File', $installScript) + $ExtraArgs + @('-ShowResolvedPaths') + $prevEAP = $ErrorActionPreference + $ErrorActionPreference = 'Continue' + $global:LASTEXITCODE = 0 + try { + & $psExe @callArgs *> $outFile + } finally { + $ErrorActionPreference = $prevEAP + } + $exitCode = $LASTEXITCODE + $raw = @(Get-Content -LiteralPath $outFile -ErrorAction SilentlyContinue) + $stdout = ($raw | Where-Object { $_ -notlike '`[hermes`]*' }) -join "`n" + $stderr = ($raw | Where-Object { $_ -like '`[hermes`]*' -or $_ -match 'HermesHome|install directory|tool store' }) -join "`n" + return @{ ExitCode = $exitCode; Stdout = $stdout; All = ($raw -join "`n") } + } finally { + if ($null -eq $savedRuntimeDir) { + Remove-Item Env:HERMES_RUNTIME_DIR -ErrorAction SilentlyContinue + } else { + $env:HERMES_RUNTIME_DIR = $savedRuntimeDir + } + if ($null -eq $savedHermesHome) { + Remove-Item Env:HERMES_HOME -ErrorAction SilentlyContinue + } else { + $env:HERMES_HOME = $savedHermesHome + } + Remove-Item -LiteralPath $outFile -Force -ErrorAction SilentlyContinue + } +} + +$base = Join-Path ([System.IO.Path]::GetTempPath()) ("hermes-store-guard-" + [guid]::NewGuid()) +$home1 = Join-Path $base 'home1' +$dir1 = Join-Path $base 'checkout1' +$insideHome = Join-Path $base 'checkout2' # HermesHome nested inside InstallDir +$insideDir = Join-Path $base 'checkout2' # the InstallDir that contains it + +try { + # 1. Same path for both: refused, before any download. + $r = Invoke-ResolvedPaths @('-HermesHome', $home1, '-InstallDir', $home1) + Assert-True ($r.ExitCode -ne 0) 'same HermesHome and InstallDir is refused (non-zero exit)' + Assert-True ($r.All -match 'tool store would land inside the checkout') 'refusal names the tool-store cause' + + # 2. HermesHome nested inside InstallDir: refused for the same reason. + $r = Invoke-ResolvedPaths @('-HermesHome', $insideHome, '-InstallDir', $insideDir) + Assert-True ($r.ExitCode -ne 0) 'HermesHome inside InstallDir is refused (non-zero exit)' + + # 3. Distinct paths: accepted, and the caller's paths survive verbatim. + $r = Invoke-ResolvedPaths @('-HermesHome', $home1, '-InstallDir', $dir1) + Assert-True ($r.ExitCode -eq 0) "distinct paths are accepted (exit $($r.ExitCode))" + $paths = $null + try { $paths = $r.Stdout | ConvertFrom-Json } catch { $paths = $null } + Assert-True ($null -ne $paths) 'accepted run prints the resolved-path JSON' + if ($paths) { + Assert-True ("$($paths.hermes_home)" -eq $home1) 'hermes_home echoes the requested -HermesHome' + Assert-True ("$($paths.install_dir)" -eq $dir1) 'install_dir echoes the requested -InstallDir' + } + + # 4. HERMES_RUNTIME_DIR outside the checkout lifts the refusal: the store + # no longer lands inside InstallDir, so the configuration is legitimate. + $storeElsewhere = Join-Path $base 'store' + $saved = $env:HERMES_RUNTIME_DIR + try { + $env:HERMES_RUNTIME_DIR = $storeElsewhere + $r = Invoke-ResolvedPaths @('-HermesHome', $home1, '-InstallDir', $home1) + Assert-True ($r.ExitCode -eq 0) "HERMES_RUNTIME_DIR outside lifts the refusal (exit $($r.ExitCode))" + } finally { + if ($null -eq $saved) { + Remove-Item Env:HERMES_RUNTIME_DIR -ErrorAction SilentlyContinue + } else { + $env:HERMES_RUNTIME_DIR = $saved + } + } + + # 5. Default derivation never trips the guard: InstallDir defaults to + # \hermes-agent, which is a child, not a parent. + $r = Invoke-ResolvedPaths @('-HermesHome', $home1) + Assert-True ($r.ExitCode -eq 0) "default -InstallDir under -HermesHome stays accepted (exit $($r.ExitCode))" + $paths = $null + try { $paths = $r.Stdout | ConvertFrom-Json } catch { $paths = $null } + if ($paths) { + Assert-True ("$($paths.install_dir)" -eq (Join-Path $home1 'hermes-agent')) 'default install_dir derives under hermes_home' + } +} finally { + Remove-Item -LiteralPath $base -Recurse -Force -ErrorAction SilentlyContinue +} + +if ($failures -gt 0) { + Write-Host "$failures assertion(s) failed." -ForegroundColor Red + exit 1 +} +Write-Host 'HermesHome/InstallDir separation guard tests passed.' -ForegroundColor Green +exit 0 diff --git a/tests/scripts/desktop_update/test_desktop_update_runtime_unicode.py b/tests/scripts/desktop_update/test_desktop_update_runtime_unicode.py new file mode 100644 index 0000000000..92b8ba20ef --- /dev/null +++ b/tests/scripts/desktop_update/test_desktop_update_runtime_unicode.py @@ -0,0 +1,131 @@ +"""runtime.ps1 must decode the launcher's UTF-8 path output on Windows (#124526). + +``scripts/desktop-update/runtime.ps1`` captures two machine-readable outputs from +Hermes launchers: ``--print-runtime-command`` (a JSON array whose first element +is the interpreter path) and ``--version`` (whose ``Install directory:`` line +names the source root). Both can sit under a non-ASCII profile +(``C:\\Users\\Balázs``). The launchers print UTF-8 — the real CLI's +``hermes_bootstrap`` reconfigures stdio on import — while Windows PowerShell 5.1 +decodes captured native stdout with the console OEM code page, mojibaking the +path (``á`` → ``├í``, ``ł`` → ``┼é``) so the returned command cannot run and the +legacy identity check never matches. + +Both call sites now scope ``[Console]::OutputEncoding`` to UTF-8 around the +capture and restore it. These tests drive the REAL script against a REAL +published launcher under non-ASCII paths, across three console code pages, and +assert the caller's encoding is restored. They never read the script's text. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +import subprocess +import sys + +import pytest + +from tests.installation_launcher_fixture import publish_fixture_launcher + +ROOT = Path(__file__).resolve().parents[3] +HELPER = str(ROOT / 'scripts/desktop-update/runtime.ps1').replace("'", "''") + +# Mirrors the production launchers: hermes_bootstrap reconfigures stdio to +# UTF-8 on import before the entry module runs, so both outputs carry raw +# UTF-8 bytes on a pipe. sys.reconfigure replicates that for the fixture's +# plain interpreter (the embedded bootstrap imports the real one). +CLI = """ +import io, json, os, sys +for _stream in (sys.stdout, sys.stderr): + if isinstance(_stream, io.TextIOWrapper): + try: _stream.reconfigure(encoding='utf-8', errors='replace') + except (OSError, ValueError): pass +from pathlib import Path +def main(): + if '--version' in sys.argv: + print('Install directory: ' + str(Path(__file__).resolve().parents[1])); return 0 + print(json.dumps([sys.executable] + sys.argv[2:])) + return 0 +if __name__ == '__main__': + sys.exit(main()) +""" + +# From #124526 / #128326: profile names that die as OEM mojibake. +PROFILE = 'Balázs Paweł Ñuñez' + + +def _run_helper(script: str, home: Path, env_extra: dict[str, str]) -> subprocess.CompletedProcess: + return subprocess.run(['powershell', '-NoProfile', '-Command', script], + env={**os.environ, 'HERMES_HOME': str(home), **env_extra}, + capture_output=True, text=True, timeout=120) + + +@pytest.mark.platforms('windows') +@pytest.mark.parametrize('code_page', [936, 437, 65001]) +def test_runtime_command_survives_a_non_ascii_profile_path(tmp_path: Path, code_page: int) -> None: + """Get-HermesRuntimeCommand returns the launcher's exact interpreter path. + + The launcher JSON is ASCII-escaped today, so this is the round-trip + contract for the boundary every other consumer (hermes_cli.windows_ssh_ + runtime, the Electron updater, the Rust bootstrap) already decodes as + UTF-8: the scoped capture must return the path byte-exact and restore + the caller's console encoding, whatever code page the console carries. + """ + profile = tmp_path / PROFILE + install = profile / 'hermes-agent' + # PM's store under the same non-ASCII profile, with a facts.json naming a + # store interpreter there — the layout a real install resolves and prints. + store = profile / 'tools' + python_dir = store / 'cpython-3.14-windows-x86_64-none' + python_dir.mkdir(parents=True) + python = python_dir / 'python.exe' + python.write_bytes(Path(sys.executable).read_bytes()) + (store / 'facts.json').write_text( + json.dumps({'packages': {'python': {'entry': python_dir.name}}}), encoding='utf-8') + publish_fixture_launcher(install, CLI) + home = tmp_path / 'home' + home.mkdir() + target = str(install).replace("'", "''") + prefix = f"[Console]::OutputEncoding = [Text.Encoding]::GetEncoding({code_page}); " + script = (prefix + f". '{HELPER}'; " + f"$c = @(Get-HermesRuntimeCommand -InstallRoot '{target}'); " + f"$c[0]; [Console]::OutputEncoding.CodePage") + result = _run_helper(script, home, {'HERMES_RUNTIME_DIR': str(store)}) + assert result.returncode == 0, result.stdout + result.stderr + lines = result.stdout.strip().splitlines() + assert lines[0] == str(python), ( + f"interpreter path was mojibaked under code page {code_page}: {lines[0]!r}") + assert int(lines[1]) == code_page, 'the capture must restore the caller console encoding' + + +@pytest.mark.platforms('windows') +@pytest.mark.parametrize('code_page', [936, 437, 65001]) +def test_legacy_version_identity_check_matches_a_non_ascii_install(tmp_path: Path, code_page: int) -> None: + """The user-bin fallback resolves by reported install directory, not PATH order. + + Red on base: the fixture CLI prints ``Install directory:`` as UTF-8 (the + production CLI reconfigures stdio the same way), and Windows PowerShell + 5.1 decodes that capture with the console OEM code page — so under CP437 + a profile like ``Paweł Łącki`` reports mojibaked and the identity check's + GetFullPath comparison never matches (#124526). + """ + install = tmp_path / PROFILE / 'hermes-agent' + launcher = publish_fixture_launcher(install, CLI) + # Earlier PM installers published only to user-bin: move (not copy) the + # launcher out of .hermes\bin so Get-HermesRuntimeCommand takes the + # legacy branch. Its body still reports the checkout it was minted from. + home = tmp_path / 'profile' + userbin = home / 'bin' + userbin.mkdir(parents=True) + external = userbin / launcher.name + launcher.rename(external) + target = str(install).replace("'", "''") + prefix = f"[Console]::OutputEncoding = [Text.Encoding]::GetEncoding({code_page}); " + script = (prefix + f". '{HELPER}'; " + f"try {{ @(Get-HermesRuntimeCommand -InstallRoot '{target}') | ConvertTo-Json -Compress }} " + f"catch {{ exit 1 }}") + result = _run_helper(script, home, {}) + assert result.returncode == 0, result.stdout + result.stderr + assert json.loads(result.stdout)[0] == str(external), ( + f"legacy identity check lost the non-ASCII path under code page {code_page}")