feat(runtime-host): discover managed Host updates - #3667
Conversation
3797d6d to
5bc7713
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Review at 5bc77138 (still a draft, so this is context rather than a gate). One [P2], filed inline on runtime-host-update-discovery.ts.
The headline boundary holds: this really is discovery only. I checked the whole dispatch — npm view, service status, local manifest reads, nothing that installs or executes a downloaded artifact. Version comparison also matches scripts/release-version.mjs (build metadata rejected, prereleases compared numerically).
The P2 is about what the unattended verdict is grounded in, and it matters more once the second commit's "discovery is authoritative" intent is realised downstream.
Not approving: draft, test has not gone terminal on this head, and the P2 is open.
中文
在 5bc77138 上审（仍是 draft，所以这条是背景而非门禁结论）。一条 [P2]，行内提在 runtime-host-update-discovery.ts。
"只发现不升级"这条边界是守住的：整条命令只有 npm view、服务状态和本地 manifest 读取，没有任何安装或执行下载物的路径。版本比较也和 scripts/release-version.mjs 一致（拒 build metadata，预发布号按数字比）。P2 说的是 unattended 结论建立在什么之上——等第二个 commit 那句"让 discovery 成为权威"在下游落地，这一点会更要紧。
不 approve：draft、test 在本 head 未终态、且 P2 未闭。
5bc7713 to
f369bd8
Compare
Resolve latest, next, or exact package selectors to immutable registry metadata without mutating the managed service. Admit unattended updates only for newer packages carrying the same explicit compatibility epoch; all missing, mismatched, or downgrade cases remain manual. Publish the compatibility epoch in CLI artifacts and expose a bounded management frame for future Desktop reconciliation. Generated-by: Codex
Verify the installed service deployment before trusting its package metadata, and require a canonical SHA-512 registry integrity value. Represent update eligibility as one closed outcome so machine and human output cannot disagree and manual paths retain their reason. Generated-by: Codex
Resolve update metadata only from the official npm registry outside project npm configuration. Share canonical package evidence validation between the CLI and framed operator contract, and document the artifact revalidation required before mutation. Generated-by: Codex
Make the service summary the sole installed-version fact in update discovery. The framed contract now requires a canonical installed service and derives outcome consistency from that version, eliminating a contradictory wire state. Generated-by: Codex
f369bd8 to
16968be
Compare
There was a problem hiding this comment.
Review — feat(runtime-host): discover managed Host updates
Coverage (start): FULL for the discovery authority and its wire contract — I read runtime-host-update-discovery.ts (364L) end to end, operator/update-package-evidence.ts (80L) end to end, the full operator/service-management-frame.ts check_update schema and superRefine, the CLI selector parsing (runtime-host-cli.ts), the dispatch (cli-core.ts), and the update-discovery unit tests. I ran git merge-tree --write-tree origin/main <head> (clean, no conflict) and a full-repo grep for consumers of unattended_update/check_update/updateCheck and for any spawn('npm', … install …). Sampled: I did not exhaustively re-review the pre-existing, unchanged runtime-host-service-manager.ts install path (only confirmed it is untouched by this PR and not wired to the discovery result), nor every changed doc/README/test fixture. The gate test (12m39s) and audit (34s) are green on this exact head 16968be6.
The three required answers:
1. Is the prior P2 closed on this head? — NO, it is open (unchanged on this dimension). unattended_update still depends solely on candidate.compatibility === currentCompatibility (inline @ :164); dist.integrity is sha512-validated but never enters the decision. spawn('npm',…) still passes no explicit env (inline @ :252) — though cwd is now pinned to homedir() and the hardcoded --registry overrides env registry, which narrows the env sub-risk.
2. Does the discover-only / never-install boundary still hold? — YES, it holds; the P2 does NOT escalate to P1. I found no path in the new commits that triggers an install: check-update only runs npm view (read-only) and writes a frame to stdout; the frame's unattended_update verdict has no consumer that runs update. The pre-existing update action lives in the separately-unchanged service-manager and takes an explicit target version (no self-discovery). I did not find a way any added code executes npm install/npm ci.
3. Version-comparison edges: largely correct, with one residual. Firm: non-semver/malformed candidate versions are rejected (isProductReleaseVersion) → hard invalid_registry_metadata error, not default-can-upgrade. Missing/abnormal compatibility on either side fails closed to manual_action (never unattended_update). Reserve: a stable install can be framed unattended_update for a next-channel prerelease (e.g. stable 1.0.0 → 2.0.0-beta.1) with no prerelease marker on the frame (inline @ service-management-frame.ts:107). Behavior is intended per the tests, but downstream auto-install consumers have no way to decline prerelease promotion.
Open directions you asked me to check:
- Release metadata trust: the only decision inputs are
candidate.version+candidate.compatibility, both registry-served;dist.integrityandcompatibilityare validated for format (sha512 shape; positive integer) but not origin — that is the documented P2 instance, and I found no additional format-vs-origin instance beyond it. Positive:--registryis hardcoded (env registry can't redirect) and current-sidecompatibilityis read from the local installed manifest (trusted). - Old path / double authority: no duplicate authority for the same component. The desktop
window.maka.app.checkForUpdates()(electron auto-update for the desktop app) is pre-existing and not part of this PR (about-settings-page.tsxis not a changed file here); this PR's CLIcheck-updatetargets the managed Runtime Host service — a different process/scope. For the managed service there is exactly one discovery path (this one); the pre-existingupdatecommand does not self-discover. So the 871/6-additive pattern does not leave a second truth for the same update decision.
Findings summary: 1 × P2 (open, not escalated — the two closed/open facts above are the basis), 2 × P3 (spawn env residual; prerelease-unmarked frame). No P0/P1. I did not invent a finding to escalate; I corroborated the P2 as not closed and did not find grounds to move it to P1.
Per-coverage note: "not found within my coverage = the two open directions show no defect" is scoped to the discovery authority + wire contract and the consumer grep; I did not assert anything about the unchanged install implementation.
简体中文
精读发现权威全链路（FULL），merge-tree 无冲突、CI 在 exact head 绿。三答：①旧 P2 未闭合——unattended_update 仍只看 compatibility 相等，dist.integrity 校验了但没进决策；spawn 仍无显式 env（但 cwd 已钉 homedir、硬编码 --registry 压过 env 仓库，env 风险收窄）。②「只发现不升级」边界守住——无任何新路径触发安装，check-update 只跑只读 npm view、写 frame，unattended_update 无消费者触发 update；P2 不升 P1。③版本比较大致正确：非语义/畸形 version 硬拒、compatibility 缺失/异常 fail-closed 到 manual；保留项——稳定版可被 next 预发布（如 2.0.0-beta.1）框成 unattended_update 但 frame 无 pre-release 标记（下游自动安装消费方无法拒绝）。开放面：release 元数据只做格式校验非来源校验（正是已知 P2，未发现新实例）；无双权威（desktop electron 自动更新是另一进程/非本 PR；服务只有这一个发现路径）。结论：1×P2（未升 P1）+2×P3，无 P0/P1。
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Independent review at exact head 16968be64711fae550130912c4916f0a0d643a9b — one of several lines reviewing this PR separately. No P0–P2. Two P3, and a re-grade of our own earlier P2.
Coverage: the whole of runtime-host-update-discovery.ts (364 new lines), update-package-evidence.ts, the check_update frame schema in service-management-frame.ts, the CLI selector parser and the cli-core.ts dispatch. Excluded: the 222-line test file was read for what it pins, not audited as a suite; the README/package.json/release-cli-package.mjs edits were read but not independently verified against the release process; runtime-host-ssh-terminal.ts and the two touched test files outside this feature were not reviewed.
Re-grading our own [P2] from review 5004496546 down to [P3], and saying why
That review — filed against the earlier head 5bc77138 — held that unattended_update turns solely on a packument field while dist.integrity is format-checked, carried, and takes no part in the decision, and that spawn('npm', …) passes no env.
All three facts still hold at this head. assessRuntimeHostUpdate (runtime-host-update-discovery.ts:148-167) consumes only the version relation and the two compatibility integers; integrity is validated at :212 and placed in the frame at :130, and is compared against nothing.
But the reachability that justified P2 does not hold, and we should say so rather than leave the grade standing. We traced the installing path: runtime-host-service-update dispatches to runManagedRuntimeHostUpdateCli with sourcePackageRoot and version (cli-core.ts:280-295) — it takes no registry candidate and no integrity at all. A tree-wide search for consumers of candidate.integrity returns exactly one hit: the schema field that declares it. So a wrong verdict here cannot steer an install toward attacker-chosen bytes; the worst outcome is a misleading report.
So the substance is different from what we originally filed, and it is a design point, not a security one:
[P3] candidate.integrity is published evidence with no consumer
The module is named update-package-evidence.ts and the frame validates the field through PACKAGE_INTEGRITY_SCHEMA, which makes it read as verified provenance. It is not: isSha512PackageIntegrity (update-package-evidence.ts:57-63) checks the sha512- prefix, an 88-character base64 body, and a 64-byte round-trip — that is a well-formedness check on a string the registry sent, in the same response as the version it accompanies. Nothing in the tree compares it to a downloaded artifact.
Two honest resolutions, and we do not think this PR has to pick the second:
- drop the field until a consumer exists — the strict schema means adding it back later is a one-line change and an epoch bump you are already paying for; or
- keep it, and state in the schema comment what it is (a registry assertion recorded for a future unattended updater) and what it is not (evidence the bytes were verified). The current naming asserts the stronger reading.
[P3] The npm child inherits TLS and proxy configuration from the caller's environment
runNpmView (:248-259) passes cwd: homedir(), stdio: ['ignore','pipe','ignore'], windowsHide, a timeout and killSignal — and no env.
Credit where it is due: passing --registry explicitly (:181-182) does defend against registry substitution. We tested npm's precedence rather than assuming it:
$ NPM_CONFIG_REGISTRY=https://evil.example.com/ npm config get registry --registry https://registry.npmjs.org/
https://registry.npmjs.org/
$ NPM_CONFIG_REGISTRY=https://evil.example.com/ npm config get registry
https://evil.example.com/
The flag wins. But the same test shows what the flag does not cover:
$ NPM_CONFIG_STRICT_SSL=false npm config get strict-ssl --registry https://registry.npmjs.org/
false
So NPM_CONFIG_STRICT_SSL, proxy settings and NPM_CONFIG_USERCONFIG still apply to this child. Given the finding above — nothing installs from this output — the consequence is bounded to a false or attacker-influenced verdict, which is why this is P3 and not higher. An explicit env allowlist would close it, and cwd: homedir() shows the author was already thinking about this class of exposure.
What we tried to break and could not
- Version comparison, including prereleases.
parseProductReleaseVersion(:65-79) rejects build metadata outright, rejects leading-zero numeric prerelease identifiers, andcompareProductReleaseVersionsimplements semver precedence correctly: release outranks prerelease at equal core (:37-38), numeric identifiers compare numerically viaBigInt(:47-48), numeric outranks alphanumeric in the right direction (:50-51), and a shorter identifier list loses (:42-43). We could not construct a pair that misorders, and1.2.3-rc.1vs1.2.3resolves totarget_not_newer, not an update. - Missing or malformed fields default to "cannot update", not "can". Unknown current compatibility, unknown target compatibility, and any mismatch all return
manual_action(:158-166);positiveInteger(:350-352) rejects0, non-integers and non-numbers;readPackageCompatibility(:220-241) returnsundefinedon a name mismatch, a version mismatch, an oversized manifest, or any thrown error. Every branch we found fails closed. - The "discovery only" boundary holds.
runtime-host-service-updatealready existed on the merge base; this PR adds exactly one dispatch case, and the new module contains no install or exec path. We were prepared to raise this to P1 if any path could install — none can. - Command injection through the selector.
shell: trueon Windows is safe here becauseparseUpdateSelectoradmits onlylatest,next, or a string passingisProductReleaseVersion; no range, no separator, no metacharacter survives. The comment at:250-251states this and is accurate. - Exact-selector confusion. Enforced twice — in the resolver (
:210) and again by the frame'ssuperRefine. - Oversized and slow registry responses. Both bounded (64 KiB, 30 s) and both land on a typed failure code.
Where the surface widened without an old path retiring
+871/-6 is a new capability rather than a replacement, so we looked for the usual failure — an older path left in place giving a second answer. We did not find one: check-update is a new action with a new frame variant, and no prior update-discovery path exists to disagree with it.
Gates at this exact head
test and audit both terminal success; MERGEABLE. We are deliberately not issuing an APPROVE: other independent lines are still reviewing this PR at the maintainer's request, and a second opinion arriving after an approval is worth less than one arriving before it.
中文
在 exact head 16968be6 上的独立审查（本 PR 有多条独立线各自发布）。无 P0–P2，两条 P3，并且下调我们自己此前的一条 P2。
覆盖面：runtime-host-update-discovery.ts 全文、update-package-evidence.ts、check_update frame schema、CLI selector 解析、cli-core.ts 分派。排除面：222 行测试只读了它钉住什么，未作为套件审计；README / package.json / release-cli-package.mjs 只读未独立核对发布流程；本特性之外的三个文件未审。
把我们自己的 P2 降为 P3，并说明为什么：原 P2 的三条事实在新 head 上全部仍然成立——判定只用版本关系与两个 compatibility 整数，integrity 只在 :212 校验、:130 放进 frame，不参与任何比较。但支撑 P2 的可达性不成立：安装路径 runtime-host-service-update 用的是 sourcePackageRoot 与 version（cli-core.ts:280-295），完全不接收 registry candidate 和 integrity；全树搜索 candidate.integrity 的消费者只有声明它的那个 schema 字段。所以错误判定无法把安装导向攻击者选定的字节，最坏只是一份误导性报告。
[P3] candidate.integrity 是发布出来但无人消费的证据。 模块名叫 update-package-evidence.ts、字段过 PACKAGE_INTEGRITY_SCHEMA，读起来像已验证的来源证明；实际上 isSha512PackageIntegrity 只校验前缀、88 字符 base64 与 64 字节回环——是对 registry 自己发来的字符串做格式检查。两个诚实的处理：删掉直到有消费者（strict schema 让日后加回只是一行 + 一次你本来就要付的 epoch），或者留下并在 schema 注释里写清它是什么（为将来的无人值守更新器记录的 registry 断言）和不是什么（不代表字节被验证过）。现在的命名主张了更强的那一层。
[P3] npm 子进程继承调用方环境里的 TLS 与代理配置。 runNpmView（:248-259）传了 cwd: homedir()、stdio、windowsHide、超时与 killSignal，没传 env。该给的分要给：显式 --registry 确实防住了 registry 替换，我们实测了 npm 的优先级而不是假设——带 --registry 时 flag 压过 NPM_CONFIG_REGISTRY，不带时环境变量生效。但同一组实测也显示 flag 覆盖不到的部分：NPM_CONFIG_STRICT_SSL=false 在带 --registry 时仍然生效。结合上一条（此输出不驱动任何安装），后果被限制在判定失真，所以是 P3 而非更高。加一个显式 env 白名单即可关闭；cwd: homedir() 说明作者本来就在防这一类暴露。
攻不动的部分：预发布版本比较（parseProductReleaseVersion 拒 build metadata、拒前导零数字标识；compareProductReleaseVersions 的 semver 优先级四个分支都对，1.2.3-rc.1 vs 1.2.3 落在 target_not_newer）；字段缺失/异常每一支都 fail closed（positiveInteger 拒 0 与非整数，readPackageCompatibility 名称/版本/超长/抛错全返回 undefined）；"只发现不升级"的边界守住了（update 动作在 merge base 上就存在，本 PR 只加一个分派，新模块无任何安装路径——若有可安装路径我们会升 P1）；Windows 上 shell: true 安全（selector 只收 latest/next/严格版本号）；exact selector 双重校验；超大与超时响应都有界且落在有类型的失败码上。
新增能力未留双权威：+871/-6 是新增而非替换，我们找过"旧路径没退场"这个常见失败，没找到——此前不存在会给出不同答案的更新发现路径。
门禁：test 与 audit 均终态 success，MERGEABLE。我们有意不发 APPROVE：按 maintainer 要求还有其他独立线在审，晚于批准到达的第二意见价值低于早于批准到达的。
Keep Desktop management results limited to actions its IPC surface can issue, and remove an unreachable Windows subprocess path from the Unix-only managed update check. Generated-by: Codex
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Re-binding my review 5005061272 to the new head 3e618aafc2dd1d6cdc40574a69ca74dd88637e3a. Both P3s still stand; nothing new, and one thing in the new commit that looks alarming is actually correct.
The delta since 16968be6 is three files, +13/-7.
The part worth stopping on: runNpmView lost shell: process.platform === 'win32' and windowsHide, along with the comment that justified them ("Windows requires its command shell to resolve npm.cmd").
That reads like a regression, because the removed comment was factually right — Node cannot execute npm.cmd without a shell. It is not a regression, and here is why: runManagedRuntimeHostUpdateCheckCli calls createPlatformRuntimeHostServiceBackend at runtime-host-update-discovery.ts:90, well before runNpmView is reachable, and that factory is closed to two platforms:
if (platform === 'linux') return createSystemdUserRuntimeHostService(serviceId);
if (platform === 'darwin') return createLaunchAgentRuntimeHostService(serviceId);
throw new RuntimeHostServiceManagerError(
'unsupported_platform',
'Managed Runtime Host services currently require Linux or macOS',
);So on Windows this command already fails earlier with a typed unsupported_platform, and those two spawn options could never execute. Deleting them removes a claim the code could not honour — the previous state was worse than the new one, because a reader would take the comment as evidence that Windows was a supported path here. Good change.
Both P3s from my earlier review survive unchanged:
candidate.integrityis still published evidence with no consumer —assessRuntimeHostUpdateis untouched by this delta, and the only reader of the field in the tree is still the schema that declares it.- The
npmchild still receives no explicitenv. The delta removed options from thatspawncall but did not addenv.
Gates at this head: audit terminal success; test was still in_progress (run 32697750743) when this was written, so no APPROVE from me — and other independent lines are still working, which is the other reason to hold.
中文
把我的 review 5005061272 重新绑定到新 head 3e618aafc。两条 P3 均仍成立，无新增；而新提交里一处看着像回归的改动其实是对的。
自 16968be6 起的 delta 是三个文件、+13/-7。
值得停下来看的一处：runNpmView 删掉了 shell: process.platform === 'win32' 和 windowsHide，连带删掉了为它们辩护的注释（"Windows 需要命令外壳才能解析 npm.cmd"）。
这读起来像回归，因为被删的注释在事实上是对的——Node 不通过 shell 无法执行 npm.cmd。但它不是回归：runManagedRuntimeHostUpdateCheckCli 在 runtime-host-update-discovery.ts:90 就调用了 createPlatformRuntimeHostServiceBackend，远早于 runNpmView 可达，而那个工厂只对两个平台开放，其余一律抛 unsupported_platform（"Managed Runtime Host services currently require Linux or macOS"）。
所以在 Windows 上这条命令本来就更早失败于一个有类型的错误，那两个 spawn 选项永远执行不到。删掉它们移除的是一个代码兑现不了的声明——原状态比现在更糟，因为读者会把那条注释当成"此处支持 Windows"的证据。这个改动是好的。
两条 P3 原样成立：① candidate.integrity 仍是发布出来却无人消费的证据——本次 delta 未触及 assessRuntimeHostUpdate，全树唯一读它的仍是声明它的那个 schema；② npm 子进程仍未收到显式 env，本次 delta 只从那个 spawn 调用里删选项，没有加 env。
门禁：audit 终态 success；撰写时 test 仍 in_progress（run 32697750743），故我不发 APPROVE——另有独立线在审，这是不发的第二个理由。
There was a problem hiding this comment.
Review — exact head 3e618aafc2dd1d6cdc40574a69ca74dd88637e3a
Coverage: FULL for the 17-file fixed-point diff and issue #3664: discovery CLI/parser/dispatch, service verification and local manifest evidence, registry resolution and bounded subprocess behavior, version ordering, framed operator contract, release manifest generation, Desktop management narrowing, tests/docs, and repo-wide searches for discovery consumers, installation reachability, and prior Host-update authorities. Sampled/excluded: the pre-existing explicit update transaction was traced only far enough to prove no new check_update path reaches staging/retirement/replacement; I did not re-audit its unchanged internal mutation mechanics. The original head and the freshness commit were both checked against their exact diffs.
Verdict: GO within that scope — no actionable P0–P3. No hard CONTRIBUTING/ARCHITECTURE violation. Exact-head test run 32697750743 completed SUCCESS (11m38s); exact-head audit run 32697750791 completed SUCCESS; PR is MERGEABLE. This is COMMENT only and does not merge.
Required questions
- Earlier integrity/env P2: not sustained at this head. The outcome uses version relation plus current/target compatibility; integrity is validated and emitted as the immutable candidate identity, but does not directly select the outcome. That is honest for discovery: the new path has no consumer that installs bytes, and the README explicitly requires a future updater to verify the downloaded archive against the emitted integrity and confirm compatibility from its extracted manifest.
npm viewhard-pins the official registry. The child still inherits TLS/proxy config, but I could not establish an additional privilege boundary or deterministic false admission; this remains defense-in-depth, not a blocker. - Discovery-only boundary: holds.
check_updateperforms status/deployment verification, local manifest reads, one boundednpm view, framing, and presentation. It never imports or calls staging, retirement, activation, replacement, or update. The existing explicit transaction remains the sole mutation path, so there is no P1 escalation. - Version edges: prerelease ordering is correct; malformed/non-product versions and missing/malformed compatibility fail closed. A provisional build-metadata concern was withdrawn after tracing product identity: published releases reuse the canonical no-build grammar, and generated development archives append only canonical
dev-<hash>prerelease forms. The looser SSH package-specifier guard is transport validation, not a second product-version authority.
Open directions: version/integrity/compatibility come from one fixed-registry lookup, with artifact revalidation explicitly deferred to the future updater; no second metadata-trust defect was established. No older managed-Host discovery authority exists: this module owns discovery, while the pre-existing exact transaction owns mutation. The module is deep at the intended seam; nonblocking smell candidates are repeated outcome policy across assessment/format/frame validation and repeated installed/candidate evidence primitives, but neither is an actionable defect here.
简体中文
覆盖 17 文件完整 fixed-point diff 与 issue #3664；显式 update 的既有内部仅抽样到足以证明 check_update 不可达 mutation。最终 GO：无 P0–P3，exact-head test/audit 均绿。旧 integrity/env P2 不维持：当前只有只读 discovery、无安装 consumer，README 明确要求未来 updater 同时复验 archive integrity 与 extracted manifest compatibility；环境继承至多是 defense-in-depth。只发现不升级边界守住。预发布/畸形/compatibility 缺失均正确 fail-closed；build metadata 候选经发布与开发归档流水线可达性挑战后撤回。无第二套 Host discovery 权威。
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Approving. All four points I raised are answered, and I checked the one your answers rest on rather than taking it on trust.
The integrity question turns on whether unattended_update is documented as a compatibility verdict rather than a bytes-verified one. It is: packages/cli/README.md:137-138 states that an updater must still verify the downloaded archive against the integrity and confirm the compatibility value from the extracted package manifest, and README.zh-CN.md:128-129 says the same. With that written down, carrying integrity as the pin for the mutation layer — instead of gating discovery on it — is the right split, and a second lookup in that layer would be strictly worse.
On the npm environment: --registry is a command-line argument, so it wins over NPM_CONFIG_REGISTRY regardless of what the caller exports, and pinning cwd to homedir() closes the project-local .npmrc path. The residual is a same-principal concern, and an allowlist would break supported proxy setups without moving the boundary.
On prerelease visibility: the selector plus the full canonical version already carry the release line. A prerelease flag would be a second representation that can disagree with them.
test and audit are both green on 3e618aaf.
中文
批准。四条都答复了，其中最关键的一条我复核了它依赖的判据：unattended_update 是否被写明只是兼容性结论、不代表字节已验证。README.md:137-138 和 README.zh-CN.md:128-129 都明确要求更新程序自行用 integrity 校验下载的 archive 并从解压后的 manifest 再确认兼容性值——写下来了，那么把 integrity 当作交给变更层的锚点、而不是在发现阶段设卡，就是对的划分。
npm 环境那条：--registry 是命令行参数，压得过 NPM_CONFIG_REGISTRY；cwd 固定到 homedir() 也堵掉了项目级 .npmrc。剩下的属于同一本地主体的范畴。prerelease 那条：selector 加完整版本号已经带了发布线，再加布尔字段反而多一份可能互相矛盾的表述。
3e618aaf 上 test 与 audit 均为绿。
|
LGTM — merging. 中文看过了，合并。 |
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
I found one issue that needs correction before this can safely advertise unattended updates.
|
|
||
| function runNpmView(args: readonly string[]): Promise<NpmViewResult> { | ||
| return new Promise((resolve, reject) => { | ||
| const child = spawn('npm', args, { |
There was a problem hiding this comment.
[P2] Isolate registry lookup from ambient npm configuration
This call inherits PATH and all npm configuration. --registry selects the URL, but inherited proxy/CA/TLS configuration (or a replacement npm executable found through PATH) can alter the response. A response containing an arbitrary newer version, any well-formed SHA-512 string, and a matching compatibility value reaches unattended_update; the integrity check only validates encoding and no archive is fetched here. That lets an untrusted parent environment make this command report a target as eligible for unattended update.
Invoke a fixed trusted npm path with a minimal environment (including a controlled npm userconfig and TLS/proxy settings), or fetch the registry metadata with a pinned client. Add a regression test that supplies hostile PATH/npm configuration and asserts no unattended result.
4 participants
English
Summary
runtime-host service check-updatecommand forlatest,next, or an exact versionFixes #3664
Verification
npm --workspace maka-agent test— 432 tests passednpm --workspace @maka/runtime-host test— 1120 tests passednpm --workspace @maka/desktop run typechecknpm run release:cli:pack -- --allow-dirty— development tarball contains the compatibility epochnextchannel and an unavailable exact versiongit diff --check, andnpm run check:asf-headersNot run: the full repository test suite; the affected CLI suite and Desktop type boundary were run directly
AI use
Select exactly one:
Tool(s) and scope: Codex authored implementation, focused tests, and documentation under maintainer direction
Checklist
Does this PR entail a change in behavior?
简体中文
摘要
runtime-host service check-update命令，支持latest、next或精确版本修复 #3664
验证
npm --workspace maka-agent test— 432 项测试通过npm --workspace @maka/runtime-host test— 1120 项测试通过npm --workspace @maka/desktop run typechecknpm run release:cli:pack -- --allow-dirty— development tarball 已携带兼容性 epochnext频道和不存在的精确版本git diff --check与npm run check:asf-headers未运行仓库完整测试套件；已直接运行受影响的 CLI 套件与 Desktop 类型边界检查
AI 使用
Codex 在维护者指导下完成实现、聚焦测试与文档
检查清单
本 PR 是否改变行为？是，具体见上方摘要