fix(ssh): probe npm via prepended PATH, not colocated with node (#9165) (#9255)

* fix(ssh): probe npm via prepended PATH, not colocated with node (#9165)

The remote Node/npm toolchain gate invoked npm by its absolute path
<nodeBinDir>/npm (POSIX) / npm.cmd (Windows, behind a Test-Path
colocation check). But deploy (commandWithNodePath) runs bare `npm`
with nodeBinDir merely prepended to PATH, so npm can resolve from
anywhere on PATH.

A host whose only resolvable node has npm elsewhere on PATH (e.g. node
symlinked into a dir without npm) deployed fine on v1.4.144, but after
upgrade the candidate is rejected with no fallback → SSH/relay
connection fails to establish.

Make the probe resolve npm exactly the way deploy does — bare
`npm --version` under the same prepended PATH — so it still confirms npm
is runnable (the #8450 concern) without requiring colocation. Windows
now prepends the backslash-form dir (matching deploy) so bare-command
PATH lookup resolves reliably.

* test(ssh): cover split Node npm PATH resolution
This commit is contained in:
Brennan Benson 2026-07-17 18:51:10 -07:00 committed by GitHub
parent c436df0551
commit 3e276d78ba
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 55 additions and 18 deletions

View File

@ -51,12 +51,46 @@ describe('resolveRemoteNodePath', () => {
'/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[1]![1]).toContain("PATH='/usr/bin':$PATH npm --version")
expect(execCommandMock.mock.calls[2]![1]).toContain(
"'/home/u/.nvm/versions/node/v22.22.0/bin/npm' --version"
"PATH='/home/u/.nvm/versions/node/v22.22.0/bin':$PATH npm --version"
)
})
it.runIf(process.platform !== 'win32')(
'accepts npm elsewhere on PATH without probing another Node candidate',
async () => {
const root = mkdtempSync(path.join(os.tmpdir(), 'orca-split-node-npm-'))
try {
const nodePath = path.join(root, 'selected node', 'bin', 'node')
const npmBinDir = path.join(root, 'npm elsewhere', 'bin')
mkdirSync(path.dirname(nodePath), { recursive: true })
mkdirSync(npmBinDir, { recursive: true })
writeFileSync(nodePath, '#!/bin/sh\nprintf "v22.22.0\\n"\n')
writeFileSync(path.join(npmBinDir, 'npm'), '#!/bin/sh\nprintf "11.13.0\\n"\n')
chmodSync(nodePath, 0o755)
chmodSync(path.join(npmBinDir, 'npm'), 0o755)
execCommandMock
.mockResolvedValueOnce(`${nodePath}\n${path.join(root, 'fallback', 'bin', 'node')}\n`)
.mockImplementationOnce((_conn: SshConnection, command: string) =>
Promise.resolve(
execFileSync('/bin/sh', ['-c', command], {
encoding: 'utf8',
env: { HOME: root, PATH: npmBinDir }
})
)
)
await expect(resolveRemoteNodePath(conn)).resolves.toBe(nodePath)
// One inventory exec plus one candidate probe keeps SSH startup work bounded.
expect(execCommandMock).toHaveBeenCalledTimes(2)
} finally {
rmSync(root, { recursive: true, force: true })
}
}
)
it('probes mise install directories', async () => {
execCommandMock
.mockResolvedValueOnce('/home/u/.local/share/mise/installs/node/20/bin/node\n')

View File

@ -174,9 +174,8 @@ async function tryResolveViaLoginShell(
return null
}
// 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.
// Validates the same PATH-prepend + bare npm contract used during deployment.
// This rejects missing npm (#8450) without requiring colocation (#9165).
// Caches nothing — this runs at most a few times per resolution (one per
// candidate), and the exec round-trip dominates.
async function nodeToolchainMeetsRequirements(

View File

@ -6,20 +6,22 @@ import {
} 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', () => {
it('probes bare npm with the selected POSIX Node directory prepended to PATH', () => {
// Deploy runs bare `npm` under the same prepended PATH, so accept npm from
// anywhere on PATH rather than requiring it colocated with node (#9165).
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"
"printf '%s\\n' '__ORCA_NPM_VERSION__' && PATH='/home/u/My Node/bin':$PATH npm --version"
)
})
it('probes npm.cmd beside the selected Windows Node', () => {
it('probes bare npm with the selected Windows Node directory prepended to PATH', () => {
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")
expect(probe).not.toContain('Test-Path')
expect(probe).toContain("$env:PATH = 'C:\\Program Files\\nodejs' + ';' + $env:PATH")
expect(probe).toContain("& 'C:/Program Files/nodejs/node.exe' --version")
expect(probe).toContain('& npm --version')
})
it('requires marked, parseable Node and npm versions', () => {

View File

@ -8,26 +8,28 @@ 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`
// Why: deploy prepends nodeBinDir before running bare npm; requiring a
// colocated executable rejects valid split layouts (#9165).
`PATH=${shellEscape(nodeBinDir)}:$PATH npm --version`
].join(' && ')
}
export function buildWindowsNodeToolchainProbe(nodePath: string): string {
const nodeBinDir = posixPath.dirname(nodePath)
const npmPath = posixPath.join(nodeBinDir, 'npm.cmd')
const windowsNodeBinDir = nodeBinDir.replace(/\//g, '\\')
return [
`if (!(Test-Path -LiteralPath ${powerShellLiteral(npmPath)} -PathType Leaf)) { exit 1 }`,
`$env:PATH = ${powerShellLiteral(nodeBinDir)} + ';' + $env:PATH`,
// Why: mirror deploy's PATH-prepend + bare npm resolution so split
// Node/npm layouts are not rejected solely for lacking npm.cmd (#9165).
`$env:PATH = ${powerShellLiteral(windowsNodeBinDir)} + ';' + $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`,
'& npm --version',
'exit $LASTEXITCODE'
].join('; ')
}