fix: validate resolved npm path ends with .js before returning

[rkfshakti]

What this PR does

Validates that the resolved npm path from fs.realpathSync ends with .js before returning it. Some Node version managers (mise, asdf, proto) replace bin/npm with a non-JS wrapper script (e.g. a bash shim). fs.realpathSync succeeds on these (the file exists), but spawning node /path/to/bash-wrapper fails with a Syntax Error.

Why it's needed

When a user installs Node via mise, asdf, or proto, which npm resolves to a bash wrapper instead of the actual npm-cli.js. The current code calls fs.realpathSync on that path and returns it unconditionally, which causes child_process.spawn("node", [bashWrapper]) to fail. By validating the resolved path ends with .js, we fall back to the conventional path (/usr/local/lib/node_modules/npm/bin/npm-cli.js) which is always the real JS file.

Reviewer Test Plan

How to verify

1. Install mise (curl https://mise.run | sh), then mise use node@22
2. Run which npm — it points to a mise shim (e.g. ~/.local/share/mise/shims/npm)
3. Before the fix: node /path/to/mise-shim fails with a Syntax Error
4. After the fix: the update check succeeds because getNpmCliPath() falls through to the conventional path

Evidence (Before & After)

N/A — this is a non-UI change. The fix is validated by the logic: fs.realpathSync on a bash shim returns the shim path (no .js suffix), so the .js check rejects it and the fallback path is used.

Tested on

OS	Status
macOS	✅
Windows	⚠️
Linux	⚠️

Environment

Local macOS with mise-installed Node 22.

Risk & Scope

• Main risk or tradeoff: The .js check is a heuristic — a legitimate npm path that doesn't end in .js would also be rejected. In practice, npm-cli.js is always a .js file, so this is safe.
• Not validated / out of scope: Windows npm path resolution (different binary naming conventions). The fallback path is Unix-specific.
• Breaking changes / migration notes: None. Existing behavior for standard npm installations is unchanged — fs.realpathSync on a real .js file returns a path ending in .js, so the check passes.

Linked Issues

Fixes #7543

中文说明

这个 PR 做了什么

验证 fs.realpathSync 解析出的 npm 路径是否以 .js 结尾。某些 Node 版本管理器（mise、asdf、proto）会将 bin/npm 替换为非 JS 的包装脚本（例如 bash shim）。fs.realpathSync 在这些文件上会成功（文件存在），但执行 node /path/to/bash-wrapper 会因语法错误而失败。

为什么需要这个修复

当用户通过 mise、asdf 或 proto 安装 Node 时，which npm 解析到的是 bash 包装脚本而非真正的 npm-cli.js。当前代码无条件地对路径调用 fs.realpathSync 并返回，导致 child_process.spawn("node", [bashWrapper]) 失败。通过验证解析后的路径以 .js 结尾，我们可以回退到传统路径（/usr/local/lib/node_modules/npm/bin/npm-cli.js），该路径始终是真正的 JS 文件。

审查者测试计划

1. 安装 mise（curl https://mise.run | sh），然后 mise use node@22
2. 运行 which npm — 指向 mise shim（例如 ~/.local/share/mise/shims/npm）
3. 修复前：node /path/to/mise-shim 报语法错误
4. 修复后：更新检查成功，因为 getNpmCliPath() 回退到传统路径

风险与范围

• 主要风险：.js 检查是一种启发式方法 — 不以 .js 结尾的合法 npm 路径也会被拒绝。实际上 npm-cli.js 始终是 .js 文件，因此这是安全的。
• 未验证/超出范围：Windows npm 路径解析（不同的二进制命名约定）。回退路径是 Unix 特有的。
• 破坏性变更：无。标准 npm 安装的行为不变。

关联 Issue

Fixes #7543

[qwen-code-ci-bot]

Hi @rkfshakti — thanks for the fix! The change itself looks well-scoped, but the PR body doesn't follow the PR template. Could you update it to include the required sections?

• What this PR does / Why it's needed — the description is there in prose, just needs the headings
• Reviewer Test Plan — how to verify (e.g. install mise, mise use node, run the update check before/after), evidence, and which OS you tested on
• Risk & Scope — main risk/tradeoff, what's out of scope
• Linked Issues — Fixes #7543 (already in the body, just needs the heading)
• 中文说明 — a Chinese translation of the above

Once the template is filled in, re-run with @qwen-code /triage and we'll pick it right back up. 🙏

— Qwen Code · qwen3.8-max-preview

[rkfshakti]

@qwen-code /triage

[qwen-code-ci-bot]

Reviewed — no blockers. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

[Suggestion] getNpmCliPath has zero direct unit tests — the .endsWith('.js') guard (the core fix) is untested for both its success and rejection branches. The linked issue #7543's triage explicitly requested a test for this case.

Failure scenario: A future refactor simplifies the function back to return fs.realpathSync(adjacentNpm) (the pre-PR one-liner), or tightens the extension check. With no test asserting the non-.js fallback, the mise/asdf/proto SyntaxError regresses silently.

describe('getNpmCliPath', () => {
  it('returns resolved path when it ends with .js', () => { /* ... */ });
  it('falls back to conventional path for non-.js shim', () => { /* ... */ });
  it('falls back when realpathSync throws', () => { /* ... */ });
});

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Agent 2: Security — Review Result

Verdict: No findings.

Examined

• The diff in packages/cli/src/utils/installationInfo.ts: the .endsWith(".js") validation on fs.realpathSync(adjacentNpm) and the restructured catch/fall-through.
• The full post-change getNpmCliPath function (lines 25–65).
• Downstream consumers in managed-npm-update.ts (lines 57, 202) and updateCheck.ts (line 149), where the return value is passed to execFileSync(process.execPath, [...]).

Analysis

• No injection risk: nodePath defaults to process.execPath (trusted). Path construction uses path.join/path.dirname only. execFileSync does not invoke a shell.
• No new attack surface: the .endsWith(".js") check narrows what gets returned. Cannot be bypassed by case tricks on real npm installations (npm-cli.js).
• No sensitive data exposure, no path traversal introduced, no hardcoded secrets.

The change is security-positive: it adds a validation layer that prevents non-JS wrapper scripts from being returned as if they were the npm CLI entry point.

[qwen-code-ci-bot]

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

[Suggestion] When the .endsWith('.js') guard rejects a resolved path (non-JS shim from mise/asdf/proto), the function silently falls through to the conventional path with no diagnostic output. A user encountering an auto-update failure would see a generic downstream spawn error with no indication that npm path resolution fell through.

The module already has a debugLogger at line 68 — adding a debug log here would aid diagnosis:

if (resolved.endsWith('.js')) return resolved;
debugLogger(`Resolved npm path does not end with .js, falling back: ${resolved}`);

— qwen3.7-max via Qwen Code /review

[gwinthis]

Review: APPROVE (C=0)

Clean fix (+18/-10) for Node version managers (mise, asdf, proto) that replace bin/npm with a non-JS wrapper script. fs.realpathSync succeeds on these bash shims, but spawning node /path/to/bash-wrapper fails with SyntaxError.

Fix: Validate the resolved path ends with .js before returning it; otherwise fall through to the conventional npm-cli.js path. The fallback is the same as before — just restructured from catch-only to a general fallback.

Key details:

• Comment explains the why clearly (version manager shims are non-JS)
• The .endsWith('.js') check is the right granularity — all real npm entry points are .js
• Fallback path unchanged: ../lib/node_modules/npm/bin/npm-cli.js

Pattern: When resolving paths through symlinks, validate the resolved target's type, not just its existence. A symlink that resolves successfully can still point to the wrong kind of file.

中文说明

评审：APPROVE (C=0)

干净修复（+18/-10）：Node 版本管理器（mise/asdf/proto）用非 JS wrapper 脚本替换 bin/npm。realpathSync 成功解析但 node 执行 bash shim 会 SyntaxError。

修复： 校验解析路径以 .js 结尾，否则回退到标准 npm-cli.js 路径。

模式： 通过符号链接解析路径时，验证目标的类型而非仅验证存在性。

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Reviewed — no blockers. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

— qwen3.7-max via Qwen Code /review

[yiliang114]

Thanks for the fix! The fall-through structure is clean and the comment explaining the version-manager shim behavior is helpful. Really appreciate you taking the time to dig into this and test it locally on macOS with mise.

We merged #7545 which landed a bit earlier with the same approach plus unit tests for all branches, but your contribution is equally valid and the security review you triggered was a nice bonus. Looking forward to your next PR! 🙏

[rkfshakti]

@yiliang114 ok, will look forward for the next PR. Incase you have something to be fixed please assign it to me and will take it from there :).