fix(hooks): detect the Vercel CLI on Windows - #163
Open
ohad6k wants to merge 1 commit into
Open
Conversation
Fixes vercel#140. On Windows the SessionStart hook reports the Vercel CLI as not installed even when it is, so the model is told a capability is unavailable and avoids `vercel env pull`, `vercel deploy` and `vercel logs`. Two independent failures, either of which alone breaks detection. Candidate ordering. npm installs three shims for a global CLI on Windows: an extensionless POSIX sh script for Git Bash, a .cmd and a .ps1. `getBinaryPathCandidates` returned the bare name first, and `accessSync(X_OK)` cannot rule it out because Node treats X_OK as F_OK on win32, so any existing file passes. Resolution therefore always returned the one shim Windows cannot execute. Extensions now come first and the bare name last. Executing the shim. Since CVE-2024-27980 Node refuses to spawn a .cmd or .bat through execFile, so the version check failed even when resolution was correct. Both call sites now go through one `execBinarySync` helper. The helper uses spawnSync with `windowsVerbatimArguments`, not `shell: true`. `shell: true` alongside an args array emits DEP0190, and invoking ComSpec without verbatim arguments does not work either: Node applies its Windows argument escaping, cmd.exe receives \"C:\path\to\vercel.CMD\" and reports the whole quoted path as an unrecognised command. `windowsVerbatimArguments` is typed on the spawn options and not on execFileSync's, which is why the helper uses spawnSync. The npm call site had the same bug and is fixed with it, so a stale-version check no longer fails silently on Windows. Verified on Windows 11 (26200), Node v24.12.0, against a real npm-installed global CLI: before resolved ...\npm\claude execFileSync -> ENOENT after resolved ...\npm\claude.CMD execBinarySync -> OK Adds the regression test the issue asks for: on win32 an executable extension must sort ahead of the bare name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #140.
On Windows the SessionStart hook reports the Vercel CLI as not installed even
when it is, so the model is told a capability is unavailable and avoids
vercel env pull,vercel deployandvercel logs.Two independent failures, either of which alone breaks detection.
Candidate ordering. npm installs three shims for a global CLI on Windows: an
extensionless POSIX sh script for Git Bash, a .cmd and a .ps1.
getBinaryPathCandidatesreturned the bare name first, andaccessSync(X_OK)cannot rule it out because Node treats X_OK as F_OK on win32, so any existing
file passes. Resolution therefore always returned the one shim Windows cannot
execute. Extensions now come first and the bare name last.
Executing the shim. Since CVE-2024-27980 Node refuses to spawn a .cmd or .bat
through execFile, so the version check failed even when resolution was correct.
Both call sites now go through one
execBinarySynchelper.The helper uses spawnSync with
windowsVerbatimArguments, notshell: true.shell: truealongside an args array emits DEP0190, and invoking ComSpecwithout verbatim arguments does not work either: Node applies its Windows
argument escaping, cmd.exe receives "C:\path\to\vercel.CMD" and reports the
whole quoted path as an unrecognised command.
windowsVerbatimArgumentsistyped on the spawn options and not on execFileSync's, which is why the helper
uses spawnSync.
The npm call site had the same bug and is fixed with it, so a stale-version
check no longer fails silently on Windows.
Verified on Windows 11 (26200), Node v24.12.0, against a real npm-installed
global CLI:
before resolved ...\npm\claude execFileSync -> ENOENT
after resolved ...\npm\claude.CMD execBinarySync -> OK
Adds the regression test the issue asks for: on win32 an executable extension
must sort ahead of the bare name.