From f544820552e2625624e7c2bdff9bfa08597fd4af Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:27:31 -0700 Subject: [PATCH] fix(ssh): require a coherent colocated Node/npm toolchain for the relay (#9165) --- .../ssh/ssh-remote-node-resolution.test.ts | 16 +++++ src/main/ssh/ssh-remote-node-resolution.ts | 51 +++++++-------- .../ssh-remote-node-toolchain-probe.test.ts | 47 ++++++++++++++ .../ssh/ssh-remote-node-toolchain-probe.ts | 64 +++++++++++++++++++ 4 files changed, 150 insertions(+), 28 deletions(-) create mode 100644 src/main/ssh/ssh-remote-node-toolchain-probe.test.ts create mode 100644 src/main/ssh/ssh-remote-node-toolchain-probe.ts diff --git a/src/main/ssh/ssh-remote-node-resolution.test.ts b/src/main/ssh/ssh-remote-node-resolution.test.ts index 109725e54..55a3877ff 100644 --- a/src/main/ssh/ssh-remote-node-resolution.test.ts +++ b/src/main/ssh/ssh-remote-node-resolution.test.ts @@ -41,6 +41,22 @@ describe('resolveRemoteNodePath', () => { await expect(resolveRemoteNodePath(conn)).resolves.toBe('/usr/local/bin/node') }) + it('skips an incomplete system Node and selects a complete NVM toolchain', async () => { + execCommandMock + .mockResolvedValueOnce('/usr/bin/node\n/home/u/.nvm/versions/node/v22.22.0/bin/node\n') + .mockRejectedValueOnce(new Error('/usr/bin/npm: not found')) + .mockResolvedValueOnce('__ORCA_NODE_VERSION__\nv22.22.0\n__ORCA_NPM_VERSION__\n11.13.0\n') + + await expect(resolveRemoteNodePath(conn)).resolves.toBe( + '/home/u/.nvm/versions/node/v22.22.0/bin/node' + ) + + expect(execCommandMock.mock.calls[1]![1]).toContain("'/usr/bin/npm' --version") + expect(execCommandMock.mock.calls[2]![1]).toContain( + "'/home/u/.nvm/versions/node/v22.22.0/bin/npm' --version" + ) + }) + it('probes mise install directories', async () => { execCommandMock .mockResolvedValueOnce('/home/u/.local/share/mise/installs/node/20/bin/node\n') diff --git a/src/main/ssh/ssh-remote-node-resolution.ts b/src/main/ssh/ssh-remote-node-resolution.ts index a5041ed15..2dd89e4a2 100644 --- a/src/main/ssh/ssh-remote-node-resolution.ts +++ b/src/main/ssh/ssh-remote-node-resolution.ts @@ -1,21 +1,21 @@ import type { SshConnection } from './ssh-connection' -import { createSshOperationAbortError, shellEscape } from './ssh-connection-utils' +import { createSshOperationAbortError } from './ssh-connection-utils' import type { RemoteHostPlatform } from './ssh-remote-platform' import { isWindowsRemoteHost, normalizeWindowsRemotePath } from './ssh-remote-platform' -import { powerShellCommand, powerShellLiteral } from './ssh-remote-powershell' +import { powerShellCommand } from './ssh-remote-powershell' import { buildPosixNodeInstallGuidance, type RemoteNodeResolutionOptions } from './ssh-remote-node-install-guidance' import { execCommand } from './ssh-relay-deploy-helpers' +import { + buildPosixNodeToolchainProbe, + buildWindowsNodeToolchainProbe, + nodeToolchainVersionsMeetRequirements +} from './ssh-remote-node-toolchain-probe' import { isSshSessionLimitError } from './ssh-session-limit-error' import { buildSshLoginShellCommand } from './ssh-login-shell-command' -// Why: the relay requires Node.js 18+. Version managers like nvm keep every -// installed version on disk, so a naive "highest version" glob can hand back -// Node 8/10/12 and crash the relay on launch. Gate every candidate on this. -const MIN_NODE_MAJOR = 18 - // Why: the login-shell fallback catches custom PATH setups in ~/.profile that // the path probes don't cover. Interactive configs (conda prompts, etc.) can // hang a login shell, so keep this short. @@ -53,7 +53,7 @@ export async function resolveRemoteNodePath( // Probe the on-disk install directories of every common Node version manager // plus system package-manager locations. Every probe runs unconditionally so // a missing directory prints nothing rather than short-circuiting later -// probes. Returns the first candidate that meets the minimum version. +// probes. Returns the first candidate with a complete Node/npm toolchain. async function tryResolveViaKnownPaths( conn: SshConnection, options?: RemoteNodeResolutionOptions @@ -113,7 +113,7 @@ true continue } seen.add(candidate) - if (await nodeMeetsVersionRequirement(conn, candidate, options)) { + if (await nodeToolchainMeetsRequirements(conn, candidate, options)) { console.log(`[ssh-relay] Found node via path probe: ${candidate}`) return candidate } @@ -160,7 +160,7 @@ async function tryResolveViaLoginShell( return null } - if (await nodeMeetsVersionRequirement(conn, candidate, options)) { + if (await nodeToolchainMeetsRequirements(conn, candidate, options)) { console.log(`[ssh-relay] Found node via login shell (${shell}): ${candidate}`) return candidate } @@ -174,10 +174,12 @@ async function tryResolveViaLoginShell( return null } -// Returns true if `nodePath` runs and reports Node >= MIN_NODE_MAJOR. +// Returns true if `nodePath` runs, reports Node >= 18, and has a runnable npm +// beside it. Deployment prepends this same directory before invoking npm, so +// accepting a looser pairing would recreate #8450. // Caches nothing — this runs at most a few times per resolution (one per // candidate), and the exec round-trip dominates. -async function nodeMeetsVersionRequirement( +async function nodeToolchainMeetsRequirements( conn: SshConnection, nodePath: string, options?: RemoteNodeResolutionOptions @@ -185,10 +187,12 @@ async function nodeMeetsVersionRequirement( try { const versionOutput = await execCommand( conn, - `${shellEscape(nodePath)} --version`, - commandOptions({ wrapCommand: false }, options) + buildPosixNodeToolchainProbe(nodePath), + // Why: the paired probe uses POSIX PATH assignment syntax, which fish + // and csh cannot parse when sshd delegates directly to the login shell. + commandOptions({ wrapCommand: true }, options) ) - return nodeVersionMeetsRequirement(versionOutput) + return nodeToolchainVersionsMeetRequirements(versionOutput) } catch (err) { if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err @@ -234,7 +238,7 @@ async function resolveRemoteWindowsNodePath( continue } const normalized = normalizeWindowsRemotePath(nodePath) - if (await windowsNodeMeetsVersionRequirement(conn, normalized, options)) { + if (await windowsNodeToolchainMeetsRequirements(conn, normalized, options)) { console.log(`[ssh-relay] Found Windows node at: ${normalized}`) return normalized } @@ -250,7 +254,7 @@ async function resolveRemoteWindowsNodePath( throwWindowsNodeNotFound(options) } -async function windowsNodeMeetsVersionRequirement( +async function windowsNodeToolchainMeetsRequirements( conn: SshConnection, nodePath: string, options?: RemoteNodeResolutionOptions @@ -258,10 +262,10 @@ async function windowsNodeMeetsVersionRequirement( try { const versionOutput = await execCommand( conn, - powerShellCommand(`& ${powerShellLiteral(nodePath)} --version`), + powerShellCommand(buildWindowsNodeToolchainProbe(nodePath)), commandOptions({ wrapCommand: false }, options) ) - return nodeVersionMeetsRequirement(versionOutput) + return nodeToolchainVersionsMeetRequirements(versionOutput) } catch (err) { if (options?.rethrowSessionLimitErrors && isSshSessionLimitError(err)) { throw err @@ -271,15 +275,6 @@ async function windowsNodeMeetsVersionRequirement( } } -function nodeVersionMeetsRequirement(versionOutput: string): boolean { - const match = versionOutput.trim().match(/^v?(\d+)/) - if (!match) { - return false - } - const major = Number.parseInt(match[1]!, 10) - return major >= MIN_NODE_MAJOR -} - async function throwNodeNotFound( conn: SshConnection, options?: RemoteNodeResolutionOptions diff --git a/src/main/ssh/ssh-remote-node-toolchain-probe.test.ts b/src/main/ssh/ssh-remote-node-toolchain-probe.test.ts new file mode 100644 index 000000000..4fb3dda56 --- /dev/null +++ b/src/main/ssh/ssh-remote-node-toolchain-probe.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import { + buildPosixNodeToolchainProbe, + buildWindowsNodeToolchainProbe, + nodeToolchainVersionsMeetRequirements +} from './ssh-remote-node-toolchain-probe' + +describe('remote Node/npm toolchain probe', () => { + it('probes npm beside the selected POSIX Node with that directory on PATH', () => { + expect(buildPosixNodeToolchainProbe('/home/u/My Node/bin/node')).toBe( + "printf '%s\\n' '__ORCA_NODE_VERSION__' && '/home/u/My Node/bin/node' --version && " + + "printf '%s\\n' '__ORCA_NPM_VERSION__' && PATH='/home/u/My Node/bin':$PATH " + + "'/home/u/My Node/bin/npm' --version" + ) + }) + + it('probes npm.cmd beside the selected Windows Node', () => { + const probe = buildWindowsNodeToolchainProbe('C:/Program Files/nodejs/node.exe') + + expect(probe).toContain("Test-Path -LiteralPath 'C:/Program Files/nodejs/npm.cmd'") + expect(probe).toContain("$env:PATH = 'C:/Program Files/nodejs' + ';' + $env:PATH") + expect(probe).toContain("& 'C:/Program Files/nodejs/npm.cmd' --version") + }) + + it('requires marked, parseable Node and npm versions', () => { + expect( + nodeToolchainVersionsMeetRequirements( + 'banner\n__ORCA_NODE_VERSION__\nv22.22.0\n__ORCA_NPM_VERSION__\n11.13.0\n' + ) + ).toBe(true) + expect( + nodeToolchainVersionsMeetRequirements( + '__ORCA_NODE_VERSION__\nv22.22.0\n__ORCA_NPM_VERSION__\nshim did nothing\n' + ) + ).toBe(false) + expect( + nodeToolchainVersionsMeetRequirements( + '__ORCA_NODE_VERSION__\nv16.20.2\n__ORCA_NPM_VERSION__\n10.8.2\n' + ) + ).toBe(false) + }) + + it('accepts legacy Node-only output from existing proxy integrations', () => { + expect(nodeToolchainVersionsMeetRequirements('v18.0.0\n')).toBe(true) + expect(nodeToolchainVersionsMeetRequirements('v16.20.2\n')).toBe(false) + }) +}) diff --git a/src/main/ssh/ssh-remote-node-toolchain-probe.ts b/src/main/ssh/ssh-remote-node-toolchain-probe.ts new file mode 100644 index 000000000..addf9b575 --- /dev/null +++ b/src/main/ssh/ssh-remote-node-toolchain-probe.ts @@ -0,0 +1,64 @@ +import { posix as posixPath } from 'node:path' +import { shellEscape } from './ssh-connection-utils' +import { powerShellLiteral } from './ssh-remote-powershell' + +const MIN_NODE_MAJOR = 18 +const NODE_VERSION_MARKER = '__ORCA_NODE_VERSION__' +const NPM_VERSION_MARKER = '__ORCA_NPM_VERSION__' + +export function buildPosixNodeToolchainProbe(nodePath: string): string { + const nodeBinDir = posixPath.dirname(nodePath) + const npmPath = posixPath.join(nodeBinDir, 'npm') + return [ + `printf '%s\\n' '${NODE_VERSION_MARKER}'`, + `${shellEscape(nodePath)} --version`, + `printf '%s\\n' '${NPM_VERSION_MARKER}'`, + `PATH=${shellEscape(nodeBinDir)}:$PATH ${shellEscape(npmPath)} --version` + ].join(' && ') +} + +export function buildWindowsNodeToolchainProbe(nodePath: string): string { + const nodeBinDir = posixPath.dirname(nodePath) + const npmPath = posixPath.join(nodeBinDir, 'npm.cmd') + return [ + `if (!(Test-Path -LiteralPath ${powerShellLiteral(npmPath)} -PathType Leaf)) { exit 1 }`, + `$env:PATH = ${powerShellLiteral(nodeBinDir)} + ';' + $env:PATH`, + `Write-Output ${powerShellLiteral(NODE_VERSION_MARKER)}`, + `& ${powerShellLiteral(nodePath)} --version`, + 'if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }', + `Write-Output ${powerShellLiteral(NPM_VERSION_MARKER)}`, + `& ${powerShellLiteral(npmPath)} --version`, + 'exit $LASTEXITCODE' + ].join('; ') +} + +export function nodeToolchainVersionsMeetRequirements(versionOutput: string): boolean { + const nodeMajor = markedVersionMajor(versionOutput, NODE_VERSION_MARKER) + const npmMajor = markedVersionMajor(versionOutput, NPM_VERSION_MARKER) + if (versionOutput.includes(NODE_VERSION_MARKER)) { + return nodeMajor !== null && nodeMajor >= MIN_NODE_MAJOR && npmMajor !== null + } + + // Why: existing proxy integrations and tests may return only the Node + // version even though production probes now emit both version markers. + const legacyMatch = versionOutput.trim().match(/^v?(\d+)/) + return legacyMatch ? Number.parseInt(legacyMatch[1]!, 10) >= MIN_NODE_MAJOR : false +} + +function markedVersionMajor(output: string, marker: string): number | null { + const lines = output.split(/\r?\n/) + const markerIndex = lines.indexOf(marker) + if (markerIndex < 0) { + return null + } + for (const line of lines.slice(markerIndex + 1)) { + if (line.startsWith('__ORCA_')) { + return null + } + const match = line.trim().match(/^v?(\d+)(?:\.\d+){1,2}(?:[-+].*)?$/) + if (match) { + return Number.parseInt(match[1]!, 10) + } + } + return null +}