fix(release): decouple Nightly from registry audit - #4703
Conversation
jackwener
left a comment
There was a problem hiding this comment.
I reviewed exact head 0fbbe11e87e8e4312551e1be8ef289d059de6258. Two P2s, no P0 or P1.
The motivation holds. A registry timeout discarding already-built artifacts is a real failure — I have watched audit go red on an npm 503 on another PR today — and five matrix rows making the same audit request for one lockfile is genuine duplication. The skip is also tighter than it first looks: the Nightly identity is not a flag but MAKA_CLI_NIGHTLY_VERSION, validated by assertProductNightlyVersion against the current package.json version, and in CI only the Nightly cli job passes a non-empty value, sourced from the identity job. The formal callers pass nothing, so checkProductionAudit() still runs for them. release.yml, dependency-audit.yml and asf-source-candidate.yml are untouched.
So "formal candidates still fail closed" is true. The two findings are about what replaced the removed checks, and about what now protects that invariant.
P2 — the daily audit is not a Nightly gate, so a known advisory can ship
The summary positions dependency audit as the continuing authority. It is a good scanner; it is not a gate on this path.
dependency-audit.yml:20-23 runs at 03:17 UTC, and its change trigger covers only the lockfile/manifest inputs it lists. npm-publication.yml:21-23 runs the npm Nightly at 18:17 UTC — a nominal 15-hour gap — and the Nightly job graph does not depend on the audit's result. Desktop Nightly only requires npm Nightly to have succeeded, and after desktop-nightly.yml:131-132 there is no advisory query left before packaging and publishing.
So an advisory landing after a day's scan and before the Nightly publishes ships in both the npm Nightly and all five Desktop artifacts, while it is already public. The next daily scan is the first automatic discovery point, which puts nominal detection latency close to 24 hours — and even then, because the Nightly graph does not depend on that job, a red scan does not stop or withdraw anything on its own.
Real runs rather than arithmetic: the last successful scheduled audit started 2026-09-03T04:44:27Z, the npm Nightly at 18:32:36Z, and Desktop Nightly finished at 19:12:37Z — the audit backing that release was about 14 hours old. Recent scheduled audits have also been delayed, and one was cancelled.
What the removed steps actually did is worth stating, because the two are not interchangeable. Each matrix row ran npm audit --omit=dev --audit-level=moderate, which blocks moderate+ advisories in the production closure and also fails on registry errors — and audit-shipped-dependencies.mjs, which reads the full report and blocks only the exact vulnerable copies that land in the Desktop shipped closure. The second one covers roots that are declared as devDependencies but bundled into dist-renderer by Vite — React among them. That class is invisible to npm audit --omit=dev, so removing it is not covered by the daily scan either. With publish.needs=[identity, desktop], any row failing blocked the whole prerelease.
The 5× duplication was real. The fix for duplication is to lift the decision into one preflight, not to delete the last decision before release.
The shape I would suggest: keep a single Nightly preflight, and separate "a valid report came back and it names a moderate+ shipped vulnerability" from "the registry or the audit service is unavailable". Block on the first, warn and proceed on the second — that meets this PR's availability goal without giving up the gate. If real-time querying is to go entirely, then the tolerated staleness window and the fact that the daily scan only warns should be written down as explicit policy, because it is not equivalent coverage.
P2 — the only test that could falsify the central claim was removed
The earlier head carried a contract named "Nightly packaging does not depend on the registry audit service": the formal release-cli-stage package_version must be empty, no npm audit or audit-shipped-dependencies step may remain in the Desktop jobs, and every npm ci in the two Nightly workflows must carry --no-audit. Making the skip unconditional, forgetting a --no-audit, or letting a formal path pass a package_version would each turn it red.
This head deletes all 25 lines, and that deletion is the entire delta from the previous head. The description now says no workflow command-shape test is added.
That matters more here than usual, because the reassurance in the section above — formal paths still audit — is now maintained only by people reading workflow files. An invariant a test pins and an invariant a reviewer re-derives are not the same invariant. Restoring that test, or an equivalent asserting that formal paths still reach checkProductionAudit() while only Nightly gets --no-audit, would close it.
Evidence boundary: release tests 192/192 and focused workflow-policy 19/19 pass, with an independent YAML graph probe; the synthetic merge against current main is clean. A full npm run rebuild fails in UI typing this PR does not touch, so no whole-repo local build is claimed. Hosted test and the immutable tarball job had not reached terminal state when I posted.
简体中文
我审的是 0fbbe11e87e8e4312551e1be8ef289d059de6258。两条 P2,没有 P0/P1。
动机是成立的。 registry 一次超时就丢掉已经构建好的产物,是真实的失败——我今天在另一个 PR 上就看到 audit 因 npm 503 变红;而五个矩阵行为同一份 lockfile 发出相同的审计请求,也确实是重复。跳过条件也比乍看更严:Nightly 身份不是一个标志位,而是 MAKA_CLI_NIGHTLY_VERSION,由 assertProductNightlyVersion 对着当前 package.json 版本校验;在 CI 里只有 Nightly 的 cli job 会传非空值,且来源是 identity job。正式调用方一律不传,所以它们仍然会执行 checkProductionAudit()。release.yml、dependency-audit.yml、asf-source-candidate.yml 一行未动。
所以「正式候选仍然 fail closed」这句是真的。 下面两条针对的是:被移除的检查由什么接替,以及现在靠什么守住这个不变量。
P2:每日审计不是 Nightly 的门禁,所以一个已知公告可以随产物发出去
摘要把 dependency audit 定位为持续权威。它是个好扫描器,但在这条路径上它不是门禁。
dependency-audit.yml:20-23 在每天 03:17 UTC 运行,其变更触发只覆盖它自己列出的 lockfile/manifest 输入。npm-publication.yml:21-23 的 npm Nightly 在 18:17 UTC——名义间隔 15 小时——而 Nightly 的 job graph 并不依赖审计的结果。Desktop Nightly 只要求 npm Nightly 成功,而在 desktop-nightly.yml:131-132 之后,直到打包与发布之间再没有任何公告查询。
于是,一个在当日扫描之后、Nightly 发布之前进入 registry 的公告,会随 npm Nightly 和全部五个平台的 Desktop 产物一起发出去,而此时它已经是公开的。下一次每日扫描才是第一个自动发现点,这让名义检测延迟接近 24 小时——而且即便那次扫描红了,由于 Nightly 图并不依赖它,也不会自动停止或撤回任何产物。
用真实运行记录而不是算术:最近一次成功的定时审计开始于 2026-09-03T04:44:27Z,npm Nightly 开始于 18:32:36Z,Desktop Nightly 直到 19:12:37Z 才完成——支撑那次发布的审计已经约 14 小时旧。近期的定时审计还出现过延迟,以及一次被取消。
被删掉的那些步骤原本做什么,值得说清楚,因为两者并不能互换。 每个矩阵行跑两种检查:npm audit --omit=dev --audit-level=moderate,它拦截 production closure 中的 moderate+ 公告,同时也会因 registry 错误而失败;以及 audit-shipped-dependencies.mjs,它读取完整报告,只拦截那些真正落入 Desktop 发货闭包的、确切的易受攻击副本。后者覆盖的是那些被声明为 devDependency、却被 Vite 打包进 dist-renderer 的根依赖——React 就在其中。这一类对 npm audit --omit=dev 是不可见的,所以移除它也不在每日扫描的覆盖范围内。而由于 publish.needs=[identity, desktop],任何一行失败都会阻止整个预发布。
5 倍重复是真实存在的。但消除重复的办法是把决策提升成一个 preflight,而不是把发布前最后一次决策整个删掉。
我建议的形状:保留一个 Nightly preflight,并把*「拿到了一份有效报告,而且它点名了一个 moderate+ 的发货漏洞」与「registry 或审计服务不可用」*分开。前者继续阻止发布,后者告警放行——这既满足这个 PR 的可用性目标,又不放弃这道门。如果确实要完全取消实时查询,那么容忍的陈旧窗口、以及「每日扫描只告警、不构成发布门」这一事实,必须写成显式政策,因为它不是等价覆盖。
P2:唯一能证伪核心主张的那条测试被删掉了
上一个 head 带着一条名为*「Nightly packaging does not depend on the registry audit service」*的契约:正式 release-cli-stage 的 package_version 必须为空;Desktop 作业里不得再残留 npm audit 或 audit-shipped-dependencies 步骤;两个 Nightly workflow 里每一条 npm ci 都必须带 --no-audit。把跳过改成无条件、漏掉一个 --no-audit、或让某条正式路径传入 package_version,都会让它变红。
这个 head 删掉了那全部 25 行,而且这次删除就是相对上一个 head 的全部增量。描述现在写的是不新增 workflow command-shape 测试。
这在此处比通常更要紧,因为上一节里那句令人安心的结论——正式路径仍然审计——现在只靠人去读 workflow 文件来维持。一个由测试钉住的不变量,和一个需要审查者每次重新推导的不变量,不是同一个不变量。 把那条测试加回来,或用一条等价断言(正式路径仍会到达 checkProductionAudit()、只有 Nightly 拿到 --no-audit),就能补上。
证据边界:release 测试 192/192、focused workflow-policy 19/19 通过,并有一个独立的 YAML 图探针;与当前 main 的合成合并是干净的。完整的 npm run rebuild 在本 PR 未触及的 UI 类型处失败,因此不声称全仓本地构建通过。我发布时,托管 test 与不可变 tarball 作业尚未进入终态。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
|
On the second P2, I am not restoring the 25-line workflow command-shape test. It asserted current spellings ( I am addressing the first P2 with one Nightly preflight and will test the new decision at its actual behavior seam: a valid report naming a shipped moderate+ advisory blocks, while an unavailable audit service warns and proceeds. Existing release contracts continue to cover workflow integrity and formal release identity. That protects the new policy without reinstating a maintenance test for deleted duplication. |
Generated-by: Codex
Generated-by: Codex
0fbbe11 to
b271e4f
Compare
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
|
Updating my earlier comment against the current head First, what now holds — measured, not readThe class I was most concerned about is still blocked.
So the category P2 — the restored test pins one slice, and the parts that were deleted have no equivalentThe production contract lives in four places: The new test at
So the conditional skip, the install flags, and the formal/Nightly isolation — everything the deleted 25-line contract covered — have no equivalent. The restored coverage is the classifier, not the seam. There is a routing consequence too: Fix: add a production-seam contract that separately pins the Desktop closure, a valid advisory still exiting 1 under The daily-scan timing point from my earlier comment is unchanged: Evidence boundary: the runs above are local probes against this head with mutations reverted afterwards; hosted release contracts are 194/194. A full local 简体中文针对当前 head 先说现在成立的部分——是测出来的,不是读出来的 我最担心的那个类别仍然被拦住。
所以 P2:补回来的测试只钉住了一个切片,被删掉的那些部分没有等价物 生产契约分布在四处: 新测试
所以条件跳过、安装参数、正式与夜版隔离——被删掉那 25 行契约所覆盖的全部内容——都没有等价物。补回来的覆盖是 classifier,不是接缝。 还有一个路由后果: 修法:补一条 production-seam 契约,分别钉住 Desktop 闭包、 我先前那条关于每日扫描时机的意见不变: 证据边界:以上运行是针对这个 head 的本地探针,变异事后已全部复原;托管 release contracts 为 194/194。不声称本地完整
|
Generated-by: Codex
|
Thanks for rerunning this against the current head. I agree with the routing gap: the production owner itself must select the release contract, not only its test. Fixed in I am not restoring the broader workflow/flag command-shape matrix. The multi-file mutations demonstrate that such a matrix could freeze more orchestration spellings; they do not show a defect in the current behavior. The valid-advisory versus unavailable decision is covered at the classifier seam, while the shipped closure remains owned by the shared The earlier daily-scan timing concern is also no longer applicable to this head: npm Nightly now runs |
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Summary
Keep Product Nightly availability independent from npm Registry audit availability without letting a valid shipped advisory pass.
The npm and Desktop Nightly workflows repeated platform-independent online audits inside packaging. A single Registry timeout could therefore discard already-built artifacts; the Desktop matrix made up to ten identical audit requests for one lockfile. The daily and formal workflows also issued a production audit immediately before the shipped-closure audit, doubling the same Registry request.
This change:
--no-audit, including the isolated CLI release tree;--allow-dirtyCLI package from contacting the Registry;Verification
npm run check:release— 194 passednode --test scripts/audit-shipped-dependencies.test.mjs scripts/release-cli-workflow-policy.test.mjs scripts/ci-test-plan.test.mjs— 48 passednpm run formatnpm run lintgit diff --checknpm_config_registry=http://127.0.0.1:9,npm run release:cli:pack -- --allow-dirtypassed the former audit boundary and completed all TypeScript builds; the local probe then stopped at the unrelated missingcargo-denytoolThe focused regression covers the new decision boundary rather than workflow command text: a valid shipped advisory blocks, while an audit service error is classified as unavailable.
AI use
Select exactly one:
Tool(s) and scope: Codex diagnosed the Nightly audit availability coupling, implemented the workflow and packaging changes, and added the regression contract.
Checklist
Does this PR entail a change in behavior?