feat(autofix): run the verification gate in an ephemeral container - #9214
feat(autofix): run the verification gate in an ephemeral container#9214wenshao wants to merge 7 commits into
Conversation
…ernal head moves Tightens the growth-divergence comparability window (PR #9104 follow-up, tracked as #9114): - measured_at (R2-6): the growth-now marker now carries the prepare-time measurement instant, and the divergence read filters on it instead of the comment's created_at. The report posts the marker only after the agent's ~120-minute run, so a round in flight when a concurrent base update landed would otherwise pass a created_at filter while carrying sums measured against the old base. - external head move (R2-8, subsumes R6-3): prior sums are measured against origin/main, so any commit an external actor (author push) or a stale-base merge added since the bot last evaluated the branch inflates this round's sum relative to them. BASE_UPD_AT only tracks the bot's own update-branch merge; the new GROWTH_NOW_CUTOFF also re-anchors (drops all prior sums) whenever the checked-out head is not the bot's last judged head (LIVE_RED_HEAD), covering author pushes and base updates alike. The reader now dedups/orders per run by measured= (a re-run's fresh measurement wins). Contract tests cover the measured-based cutoff, the external-head-move re-anchor (both branches), and the writer→reader round-trip with the new field. 172/172. R6-6 (markers don't store the effective budget, so a mid-window budget raise counts old rounds against the new regime — fail-safe, one round early) stays tracked in #9114.
Phase 1+2 of #9089. The gate executes the branch's OWN build/test; run on the host, that code shares the OS user, $HOME, $GITHUB_ENV and $GITHUB_OUTPUT with the same job's later PAT-bearing steps. Several channels fire before any in-step guard can run — BASH_ENV/BASH_FUNC_* and LD_PRELOAD/LD_AUDIT are applied by the shell/loader at startup (the runner's $GITHUB_ENV blocklist is NODE_OPTIONS-only), and a forged `outcome=fixed` appended to $GITHUB_OUTPUT beats any digest check on the gate script's bytes. The trust boundary cannot be a job boundary: a job that executed attacker code cannot vouch for what it emits, and a second job that re-verifies would have to run that code itself. So the boundary is the container wall and the PAT never crosses it. - The gate now runs via a staged, digest-verified wrapper that invokes it inside an ephemeral container (the sandbox image the agent already uses, resolved from the resolve step's OUTPUT rather than $GITHUB_ENV so branch code cannot choose it). docker does not inherit the host environment, so the PAT, $GITHUB_ENV and the real $GITHUB_OUTPUT are absent inside; $HOME is a throwaway and only three paths are mounted (workspace, the round's workdir, a copy-staged container temp). The real RUNNER_TEMP — staged agent runner, the PAT steps' throwaway configs — is never mounted. - The verdict crosses back as a host-created file plus the container EXIT CODE. Branch code can append to the mounted file, but it cannot make a failing gate exit 0, so a pass is accepted only on exit 0; exit 1 forces `failed` regardless of the file; any other code leaves the outcome unset for the existing gate-crash retry path. - Phase 2 pins the durable property structurally: a contract test fails if any step whose env carries CI_DEV_BOT_PAT invokes branch-authored code (gate script, agent runner, npm/npx) outside a container. The one recorded exception is issue triage, which runs the agent before any branch is checked out, so its working tree is still the trusted base. Probed against the real wrapper and the real sandbox image family: inside the container the PAT, $GITHUB_ENV and the host RUNNER_TEMP are all unreachable; a forged `outcome=fixed` on a gate that exits 1 lands on the host as `outcome=failed`; a verdict-less exit 0 and a docker failure both leave the outcome unset. Contract suite 174/174. Not in scope: the issue-autofix job's inline gate (same class, agent- authored branch rather than an external PR) and Phase 3 (per-job ephemeral runners), both of which stay tracked in #9089.
|
|
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewReviewed the wrapper, the two gate-step rewirings, the resolver change, and the new tests against the base tree. No critical blockers — the construction is sound:
Three non-blocking notes:
Also echoing the Stage 1 scope note: the growth-divergence re-anchoring ( The new tests are the right shape for this change: one pins the exact container invocation (allowlist, mounts, image source, digests) and drives the verdict translation behaviourally through real bash with controlled exit codes — including the forged-pass-on-exit-1 case; the other asserts the durable invariant (no PAT-bearing step invokes branch-authored code outside a container) with the one recorded exception listed explicitly rather than silently allowlisted. sequenceDiagram
participant P1 as Host verify step
participant P2 as Gate wrapper
participant P3 as Ephemeral container
P1->>P1: digest-verify wrapper and gate script
P1->>P2: bash run-autofix-gate-container.sh
P2->>P3: docker run - env allowlist, 3 mounts, throwaway HOME
P3->>P3: gate runs branch build, typecheck, lint, test
P3-->>P2: verdict file plus exit code N
P2->>P2: translate - pass only if N is 0 and verdict says fixed or noop
P2-->>P1: GITHUB_OUTPUT outcome - N is 1 forces failed, other leaves unset
TestingThis is an unattended CI run — no PR code was built or executed here; the evidence below is the PR's own CI read via the API plus static review. This PR classifies as the full CI profile (none of the four files is in the docs-only or github-ci-only allowlists), so the in-progress ubuntu Test job runs the whole gate: build, typecheck, lint, unit tests, actionlint/shellcheck/yamllint, the No failures at fetch time; the unit suite takes ~30 minutes, so this table is a snapshot — the finalize workflow updates it once CI settles. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 What remains unsubstantiated: an end-to-end gate run inside the container on the real pool — sibling-container availability, mount permissions, ownership under Not verified: full-pool container gate run (no lane can exercise the workflow runtime from here); Windows/Linux local author testing (marked 中文说明代码审查对照基线树审查了 wrapper、两个门步骤的改接、resolver 改动和新增测试。没有关键阻塞项——整体构造是健全的：
三条非阻塞提醒：
另外重申 Stage 1 的范围提醒：增长分歧的重锚定（ 新测试的形态与这个改动匹配：一个钉住精确的容器调用（白名单、挂载、镜像来源、digest）并用受控退出码通过真实 bash 行为性地驱动裁决翻译——包括"退出码 1 时伪造通过"的用例；另一个断言持久不变量（携带 PAT 的步骤不在容器外调用分支编写的代码），并把唯一记录的例外显式列出，而不是悄悄放进白名单。 测试这是无人值守的 CI 运行——此处未构建或执行任何 PR 代码；以下证据来自经 API 读取的 PR 自身 CI 结果与静态审查。 本 PR 被分类为 full CI 档位（四个文件均不在 docs-only 或 github-ci-only 白名单中），因此进行中的 ubuntu Test job 会跑完整套门：build、typecheck、lint、单测、actionlint/shellcheck/yamllint、 抓取时刻无失败；单测套件约需 30 分钟，因此上表只是快照——finalize 工作流会在 CI 落定后更新它。 尚未证实的部分： 在真实池上容器内的端到端门运行——兄弟容器可用性、挂载权限、 未验证：完整池上容器门运行（此处没有通道能执行工作流运行时）；作者的 Windows/Linux 本地测试（标记 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, well-pinned security fix for a documented hole; the only reservations are non-blocking nits and the pool-side behavior that no pre-merge lane can exercise. My independent proposal for this problem — don't run branch code where the PAT lives, use the sandbox image we already ship, allowlist the env, and treat the container exit code as the only unforgeable verdict signal — is exactly what this PR builds, and I didn't find a simpler path it missed. The verdict translation fails closed in every direction I could poke at, the image can't be steered by branch code, and the Phase 2 contract test pins the durable property ("no PAT-bearing step invokes branch-authored code outside a container") rather than just this patch, which is the part that keeps the class closed six months from now. What keeps this at 4 rather than 5: the three nits from the review ( Approval is deferred only because the PR's own CI is still in flight: the full-profile ubuntu Test job (which collects the new workflow-contract tests) had not completed at review time. Once every check lands green on the reviewed commit, the approval below is recorded against that exact commit; if anything lands red or the head moves, it is withheld. 中文说明置信度：4/5 —— 对一个已记录在案的漏洞给出的干净、钉扎到位的安全修复；唯一的保留是几条非阻塞的小问题，以及任何合并前通道都无法执行的池侧行为。 我对这个问题的独立方案——不要在 PAT 所在处运行分支代码、使用我们已有的 sandbox 镜像、对 env 做白名单、并把容器退出码当作唯一不可伪造的裁决信号——正是这个 PR 所构建的，我也没有找到它遗漏的更简路径。裁决翻译在我能试探的每个方向上都是失败即保守（fail closed），镜像无法被分支代码操纵，而阶段 2 的契约测试钉住的是持久性质（"携带 PAT 的步骤不在容器外调用分支编写的代码"）而非仅仅这个补丁——这正是六个月后仍能让这一类问题保持关闭的部分。 之所以是 4 而不是 5：审查中的三条小问题（ 批准被推迟，仅因为 PR 自身的 CI 还在进行中：full 档位的 ubuntu Test job（会收集新增的工作流契约测试）在审查时尚未完成。当所有检查在被审查的提交上全部变绿后，下面的批准将记录在该确切提交上；若有检查变红或 head 移动，则不予批准。 — 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
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
[Critical] .github/scripts/run-autofix-gate-container.sh:32 SC2034 — GATE_SCRIPT appears unused. Verify use (or export if used externally). [lint]
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
[Critical] .github/scripts/run-autofix-gate-container.sh:32 SC2034 — GATE_SCRIPT appears unused. Verify use (or export if used externally). [lint]
— qwen3.8-max via Qwen Code /review (v0.21.12)
| --volume "${WORKDIR}:${WORKDIR}" \ | ||
| --volume "${CTEMP}:${CTEMP}" \ |
There was a problem hiding this comment.
[Critical] The rw bind-mounts defeat the wrapper's central invariant ("the exit code is the unforgeable half"). CTEMP holds the staged gate entrypoint + helpers but is mounted without :ro, and the container runs as the same uid that owns those files. bash reads script files incrementally — so branch code the gate itself executes (the branch's own build/test) can rewrite the remainder of the running gate script and turn a failing gate into exit 0 + outcome=fixed + a forged verified_head, which the wrapper forwards to the PAT push. Both operands of the push guard are container-writable: HEAD_NOW reads the same rw-mounted .git the container can move, and VERIFIED_HEAD is the forwarded (forgeable) verdict value. Second shape (survives a read-only script mount): on a legitimate exit-0 pass, branch code can move the branch ref in the rw .git and append verified_head=<sha> to the mounted verdict file before teardown — the wrapper's last-wins extraction forwards the forged value. — Failure scenario: the build/test leg spawns a same-uid writer that overwrites the script tail from bash's read offset with a payload writing outcome=fixed + verified_head=$(git rev-parse HEAD) + exit 0 → bash parses the attacker tail, assert_verification_tree never runs → container exits 0 → the PAT pushes a commit the gate never verified.
Witness (executed): in-flight rewrite probe — WRITER: bash pid=2399149 read-offset=71 filesize=70034 → ATTACKER CODE EXECUTED: forged verdict + exit 0 / VICTIM EXIT CODE: 0; baseline without writer: BASELINE EXIT CODE: 1. Real wrapper translation at rc=0 with a forged append forwarded outcome=fixed + verified_head=attacker999.
Suggested fix: mount the staged scripts read-only (:ro), separated from the writable $HOME the build needs (e.g. stage scripts into ${CTEMP}/scripts mounted :ro, point HOME at a distinct writable scratch dir); defense-in-depth: capture host-side git rev-parse HEAD before/after docker run (mismatch → crash path) and re-digest the CTEMP copies after the run.
中文说明
读写挂载破坏了 wrapper 的核心不变量（“退出码是不可伪造的一半”）。CTEMP 存放暂存的门入口脚本与辅助脚本，却未以 :ro 挂载，而容器又以与这些文件属主相同的 uid 运行。bash 逐段读取脚本文件——因此门自身执行的分支代码（分支自己的 build/test）可以改写正在运行的门脚本的剩余部分，把失败的门变成退出码 0 + outcome=fixed + 伪造的 verified_head，wrapper 会将其转发给 PAT 推送。推送守卫的两个操作数都可被容器写入：HEAD_NOW 读取的是容器可移动的同一个 rw 挂载 .git；VERIFIED_HEAD 是可伪造的裁决文件转发值。第二种形态（即使把脚本挂载改为只读仍然存在）：在一次合法的退出码 0 通过中，分支代码可在容器拆除前移动 rw .git 中的分支引用，并向挂载的裁决文件追加 verified_head=<sha>——wrapper 的“最后一行生效”提取会转发伪造值。——失败场景：build/test 环节派生一个同 uid 写者，从 bash 当前读取偏移起把脚本尾部覆写为写入 outcome=fixed + verified_head=$(git rev-parse HEAD) + exit 0 的载荷 → bash 解析攻击者尾部，assert_verification_tree 不再执行 → 容器以 0 退出 → PAT 推送了一个门从未校验过的提交。
证据（已执行）：运行中改写探针——WRITER: bash pid=2399149 read-offset=71 filesize=70034 → ATTACKER CODE EXECUTED: forged verdict + exit 0 / VICTIM EXIT CODE: 0；无写者的基线：BASELINE EXIT CODE: 1。真实 wrapper 翻译在 rc=0 且带伪造追加时转发了 outcome=fixed + verified_head=attacker999。
建议修复：把暂存脚本以只读方式挂载（:ro），与 build 所需的可写 $HOME 分离（例如把脚本暂存到以 :ro 挂载的 ${CTEMP}/scripts，HOME 指向另一个可写临时目录）；纵深防御：在 docker run 前后各取一次宿主侧 git rev-parse HEAD（不一致则走崩溃路径），并在运行后重新校验 CTEMP 副本的 digest。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Fixed in 4e0931e — the staged scripts are now mounted :ro (${CBIN}), with a separate writable scratch mount (${CRW}) for what the gate legitimately writes; the gate's throwaway git config moved to a new GATE_TMPDIR that defaults to RUNNER_TEMP on host runs. Re-probed against the real image: branch code appending to $0 now gets Read-only file system, while the scratch mount stays writable. Your second shape (moving the branch ref in the rw workspace and forging verified_head) is NOT closed by this and I won't claim it is — the workspace must be writable for the gate to build, and branch code could already do that pre-PR when the gate ran on the host; it belongs to the same class #9089 tracks.
| GROWTH_NOW_CUTOFF="${BASE_UPD_AT}" | ||
| if [[ -n "${LIVE_RED_HEAD}" && "${LIVE_RED_HEAD}" != "${CHECKED_OUT_HEAD}" ]]; then | ||
| echo "🔁 branch head moved outside a bot round (last judged ${LIVE_RED_HEAD:0:9}, now ${CHECKED_OUT_HEAD:0:9}) — re-anchoring growth divergence" | ||
| GROWTH_NOW_CUTOFF="${MEASURED_AT}" | ||
| fi |
There was a problem hiding this comment.
[Critical] The new re-anchor logic is broken in both directions, because the cutoff is recomputed each round from a head-equality signal that does not track "the head moved outside a bot round" (the redcheck marker records the PRE-push judged head; the anchor is not persisted). (1) Suppression: after every successful bot push the next round checks out the post-push head while LIVE_RED_HEAD still carries the pre-push head → re-anchor fires → cutoff = this round's MEASURED_AT → the divergence filter drops all prior over-budget markers → OVER_ROUNDS_PRIOR resets to 0 every push round. The non-convergence handoff (line 4897) never fires in the exact runaway this clause targets, and the loop silently burns MAX_ROUNDS; pre-diff the filter was created_at > BASE_UPD_AT, which bot pushes did not reset — a regression introduced by this diff. (2) False fire: after an EXTERNAL push is judged by a no-push round (noop/handoff also post redcheck with the unchanged head), the following round sees equal heads → cutoff reverts to BASE_UPD_AT (empty if no base update) → the filter re-admits ALL pre-move markers measured against the old tree → GROWTH_DIVERGED=true escalates 1+ rounds early on mixed-tree arithmetic — the exact cross-tree comparison this diff's own comment declares invalid. — Failure scenario: (1) a runaway PR that pushes every round never escalates; the 🔁 log line also mislabels every successful bot round. (2) rounds 1-2 over budget on tree T_old, author pushes, round 3 judges the new tree and ends noop, round 4 escalates with only one new-tree prior round.
Witness (executed, workflow blocks driven verbatim): post-push round → 🔁 branch head moved outside a bot round … re-anchoring / OVER_ROUNDS_PRIOR=0 PREV_SUM=0 GROWTH_DIVERGED=false; comparator arm with equal heads: OVER_ROUNDS_PRIOR=2 PREV_SUM=850 GROWTH_DIVERGED=true. Revert arm: round-4 with BASE_UPD_AT='' cutoff='' → OVER_ROUNDS_PRIOR=3 PREV_SUM=550 GROWTH_DIVERGED=true (two old-tree markers re-entered); persisted-anchor flip → OVER_ROUNDS_PRIOR=0 GROWTH_DIVERGED=false.
Suggested fix: persist the anchor instead of recomputing it per round — record the POST-push head in the redcheck marker on the push path (PUSH_SHA is already computed) and/or stamp markers with the judged head (or a re-anchor epoch the read takes max of), so only genuinely external head moves re-anchor and pre-move markers stay excluded after the new head is judged. Add a test composing the cutoff block with the divergence count across a simulated push round.
中文说明
新的重锚逻辑在两个方向上都是坏的：cutoff 每轮都从一个并不能真正表示“head 在 bot 轮次之外发生了移动”的 head 相等信号重新计算（redcheck 标记记录的是推送前被裁决的 head；锚点没有被持久化）。(1) 抑制方向：每次 bot 成功推送后，下一轮 checkout 的是推送后的 head，而 LIVE_RED_HEAD 仍是推送前的 head → 触发重锚 → cutoff = 本轮的 MEASURED_AT → 差异过滤器丢弃所有此前的超预算标记 → OVER_ROUNDS_PRIOR 每个推送轮都归零。非收敛移交（第 4897 行）在该条款本要针对的失控场景中永远不会触发，循环会悄悄烧完 MAX_ROUNDS；改动前过滤器是 created_at > BASE_UPD_AT，bot 推送不会使其重置——这是本 diff 引入的回归。(2) 误触发方向：一次外部推送被一个不推送的轮次（noop/移交同样会以不变的 head 发送 redcheck）裁决后，再下一轮看到 head 相等 → cutoff 退回 BASE_UPD_AT（若无 base 更新则为空）→ 过滤器重新接纳所有在旧树上测得的移动前标记 → GROWTH_DIVERGED=true 在混合树的算术上提前 1 轮以上升级——正是本 diff 自己的注释宣称无效的跨树比较。——失败场景：(1) 每轮都推送的失控 PR 永远不会升级；🔁 日志行还会把每个成功的 bot 轮次误标为“外部移动”。(2) 第 1-2 轮在旧树 T_old 上超预算，作者推送，第 3 轮裁决新树并以 noop 结束，第 4 轮仅凭一个新树历史轮次就升级。
证据（已执行，逐字驱动工作流代码块）：推送后轮次 → 🔁 branch head moved outside a bot round … re-anchoring / OVER_ROUNDS_PRIOR=0 PREV_SUM=0 GROWTH_DIVERGED=false；head 相等的对照臂：OVER_ROUNDS_PRIOR=2 PREV_SUM=850 GROWTH_DIVERGED=true。回退臂：第 4 轮 BASE_UPD_AT='' cutoff='' → OVER_ROUNDS_PRIOR=3 PREV_SUM=550 GROWTH_DIVERGED=true（两个旧树标记重新进入）；持久化锚点翻转后 → OVER_ROUNDS_PRIOR=0 GROWTH_DIVERGED=false。
建议修复：持久化锚点而不是每轮重算——在推送路径的 redcheck 标记中记录推送后的 head（PUSH_SHA 已经算好），和/或在标记上盖上被裁决的 head（或一个读取时取 max 的重锚纪元），使得只有真正的外部队 head 移动才触发重锚，且新 head 被裁决后旧树标记持续被排除。补充一个把 cutoff 块与差异计数组合、跨过模拟推送轮次的测试。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
This is the withdrawn half of #9192 — you're right on both directions, and R1-1 there is the same regression (redcheck records the pre-push judged head). It has been removed from that PR (b328bd3) and returned to #9114 for a redesign that needs a bot-authored-move test plus a persisted cut. This branch now carries that withdrawal too, so the code is gone from this PR's diff as well.
| echo "outcome=failed" >> "${GITHUB_OUTPUT}" | ||
| [[ "${PREEXISTING}" == 'true' ]] && echo "preexisting=true" >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
[Critical] Forcing outcome=failed on ANY exit 1 deletes the gate's two deliberate exit-1-without-verdict crash paths — baseline A/B tree-restore failure (~lines 190-200) and bite-check tree-restore failure (~lines 998-1011) in run-autofix-review-verification.sh — each documenting verbatim that a plain outcome=failed is an EVALUATED rejection (watermark advances, item handed off for good) while outcome UNSET takes the report's gate-crashed retry path. Pre-PR, host execution left the outcome empty on those paths; post-PR the wrapper synthesizes outcome=failed, so the GATE_CRASHED detector ([[ -z "${OUTCOME}" … ]]) never fires. Any unguarded command in the 1063-line gate dying with status 1 under set -eo pipefail is in the same class. Forgery protection does not require the unconditional shape: reject_fix writes outcome=failed to the verdict file before exiting 1, and the wrapper's own *) arm restates the correct rationale this arm contradicts. — Failure scenario: a transient git/IO failure during tree restore → gate exits 1 with no verdict → wrapper writes outcome=failed → evaluated rejection: the agent's fix is discarded and the item stranded until a human intervenes — the exact #7329/#7336 stranding class the workflow's own comment cites.
Witness (executed): real wrapper translation driven at rc=1 with the crash shape — exit 1, no verdict (gate crash path) (rc=1) -> forwarded: [outcome=failed|] and exit 1, only committed= recorded (crash mid-gate) (rc=1) -> forwarded: [committed=true|outcome=failed|] — indistinguishable from a genuine reject_fix after translation.
Suggested fix: in the 1) arm, forward the rejection only when the verdict file says so (still forgery-proof — fixed/noop never pass on nonzero rc):
1)
if [[ "${OUTCOME}" == 'failed' ]]; then
echo "outcome=failed" >> "${GITHUB_OUTPUT}"
[[ "${PREEXISTING}" == 'true' ]] && echo "preexisting=true" >> "${GITHUB_OUTPUT}"
[[ "${RETRYABLE}" == 'true' ]] && echo "retryable=true" >> "${GITHUB_OUTPUT}"
else
echo "::warning::gate container exited 1 without a failed verdict — treating as a gate crash so the next scan retries."
fi
;;and add the missing harness case runTranslate(1, []) → ''.
中文说明
对任何退出码 1 强制写入 outcome=failed，删除了门脚本中两条刻意的“退出码 1 且不写裁决”的崩溃路径——run-autofix-review-verification.sh 中的基线 A/B 树恢复失败（约 190-200 行）与 bite 检查树恢复失败（约 998-1011 行）——两处都逐字写明：直接写 outcome=failed 是一次“已评估的拒绝”（水位前进、条目被永久移交），而保持 outcome 未设置会走报告步骤的门崩溃重试路径。改动前在宿主上执行时这些路径的 outcome 为空；改动后 wrapper 合成 outcome=failed，于是 GATE_CRASHED 检测器（[[ -z "${OUTCOME}" … ]]）永远不会触发。1063 行门脚本中任何未加保护、在 set -eo pipefail 下以状态 1 死掉的命令都属于同一类。防伪造并不需要这种无条件形态：reject_fix 在退出码 1 之前就已把 outcome=failed 写入裁决文件，而 wrapper 自己的 *) 分支恰恰重申了本分支所违背的正确理由。——失败场景：树恢复期间的一次瞬时 git/IO 失败 → 门以 1 退出且无裁决 → wrapper 写入 outcome=failed → 已评估拒绝：agent 的修复被丢弃、条目被搁置直到人工干预——正是工作流注释自己引用的 #7329/#7336 搁置类别。
证据（已执行）：以崩溃形态驱动真实 wrapper 翻译——exit 1, no verdict (gate crash path) (rc=1) -> forwarded: [outcome=failed|] 与 exit 1, only committed= recorded (crash mid-gate) (rc=1) -> forwarded: [committed=true|outcome=failed|]——翻译后与真正的 reject_fix 无法区分。
建议修复：在 1) 分支中，仅当裁决文件写明失败时才转发拒绝（仍然防伪造——非零 rc 下 fixed/noop 永不通过）：
1)
if [[ "${OUTCOME}" == 'failed' ]]; then
echo "outcome=failed" >> "${GITHUB_OUTPUT}"
[[ "${PREEXISTING}" == 'true' ]] && echo "preexisting=true" >> "${GITHUB_OUTPUT}"
[[ "${RETRYABLE}" == 'true' ]] && echo "retryable=true" >> "${GITHUB_OUTPUT}"
else
echo "::warning::gate container exited 1 without a failed verdict — treating as a gate crash so the next scan retries."
fi
;;并补上缺失的用例 runTranslate(1, []) → ''。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Fixed in 4e0931e — exit 1 no longer synthesizes a verdict. failed is taken from the verdict file only, so the gate's deliberate exit-1-without-verdict paths (baseline-A/B and bite tree-restore failures) keep reaching the gate-crashed retry instead of becoming EVALUATED rejections. Forgery protection is unchanged because it never depended on this: fixed is accepted only on exit 0, so a forged pass on a failing gate now leaves the outcome unset and the round retries. Probed: exit 1 + outcome=failed → failed+flags; exit 1 with no verdict → unset; exit 1 + forged outcome=fixed → unset.
| docker run --rm \ | ||
| --user "$(id -u):$(id -g)" \ | ||
| --workdir "${GITHUB_WORKSPACE}" \ |
There was a problem hiding this comment.
[Critical] No teardown on abnormal termination. There is no trap, no docker stop/rm, and the container is unnamed (--rm only fires on exit). A step timeout (timeout-minutes: 60 on both gate steps), the 300-minute job cap, or a cancel kills the step's process group — the docker run CLIENT — while the daemon keeps the container running. The job's stale-container janitors only remove name=qwen-code-* containers in exited/dead state, so the unnamed RUNNING gate container is invisible to them. The orphan keeps running branch code as the runner uid with rw bind-mounts on GITHUB_WORKSPACE (persistent across jobs on this self-hosted pool) and WORKDIR; the next job checks out the trusted base into the same path, and the orphan can rewrite .github/scripts/* BEFORE the stage step's cp (the recorded digests are then computed from the tampered copies, so sha256sum -c passes) or package.json/resolve-sandbox-image.mjs before 'Resolve sandbox image' reads them (steering GATE_IMAGE itself). Pre-change the gate ran in the step's own process tree, which a timeout kill reliably stopped — this diff removed that guarantee without replacing it. — Failure scenario: a hanging branch-authored test trips the 60-minute cap (attacker-controlled trigger) → the docker CLI is killed, the container survives → the next job's trusted-base checkout is tampered before staging/resolve reads it → the trust boundary this PR constructs is defeated cross-job; a true hang also leaks CPU on the shared pool indefinitely.
Witness (executed, real gate image): docker run --rm ghcr.io/qwenlm/qwen-code:0.21.11 sleep 300, then SIGKILL the docker CLI → docker ps shows 966706134b2a agitated_mclaren Up 4 seconds (container survives its client's death); janitor filter --filter 'name=qwen-code-' --filter 'status=exited' --filter 'status=dead' → empty. The workflow's own janitor comment documents this pool's kill semantics ("a JOB timeout still reaps only the HOST-side docker client, not the container … Observed directly").
Suggested fix: GATE_NAME="autofix-gate-${GITHUB_RUN_ID}"; trap 'docker rm -f "${GATE_NAME}" >/dev/null 2>&1 || true' EXIT INT TERM before docker run --name "${GATE_NAME}" … (the runner signals before SIGKILL, so the trap fires on timeout/cancel); extend the job-start janitor to force-remove RUNNING autofix-gate-* containers when no matching run is live.
中文说明
异常终止时没有拆除逻辑。没有 trap、没有 docker stop/rm，容器也没有名字（--rm 只在退出时生效）。步骤超时（两个门步骤都是 timeout-minutes: 60）、300 分钟 job 上限或取消，杀掉的是步骤进程组——即 docker run 客户端——而守护进程会让容器继续运行。job 的过期容器清理只移除 exited/dead 状态、name=qwen-code-* 的容器，因此无名的“运行中”门容器对它完全不可见。孤儿容器以 runner uid 继续运行分支代码，并保持对 GITHUB_WORKSPACE（该自托管池上跨 job 持久）与 WORKDIR 的 rw 挂载；下一个 job 会把可信基线 checkout 到同一路径，孤儿可以在 stage 步骤 cp 之前改写 .github/scripts/*（记录的 digest 将基于被篡改的副本计算，sha256sum -c 照样通过），或在 'Resolve sandbox image' 读取之前改写 package.json/resolve-sandbox-image.mjs（直接操纵 GATE_IMAGE）。改动前门运行在步骤自己的进程树中，超时杀进程能可靠地停止它——本 diff 移除了该保证却没有补上替代。——失败场景：挂起的分支测试触发 60 分钟上限（触发方式由攻击者控制）→ docker CLI 被杀、容器存活 → 下一个 job 的可信基线 checkout 在被读取前遭篡改 → 本 PR 建立的信任边界被跨 job 击穿；真正的死循环还会在共享池上无限泄漏 CPU。
证据（已执行，真实门镜像）：docker run --rm ghcr.io/qwenlm/qwen-code:0.21.11 sleep 300，随后 SIGKILL docker CLI → docker ps 显示 966706134b2a agitated_mclaren Up 4 seconds（容器在客户端死后存活）；清理过滤器 --filter 'name=qwen-code-' --filter 'status=exited' --filter 'status=dead' → 空。工作流自己的清理注释记录了该池的杀进程语义（“JOB 超时只会收割宿主侧的 docker 客户端，而不是容器……已直接观察到”）。
建议修复：在 docker run --name "${GATE_NAME}" … 之前加 GATE_NAME="autofix-gate-${GITHUB_RUN_ID}"; trap 'docker rm -f "${GATE_NAME}" >/dev/null 2>&1 || true' EXIT INT TERM（runner 在 SIGKILL 前会先发信号，超时/取消时 trap 会触发）；并扩展 job 开始时的清理逻辑，在没有匹配的活跃 run 时强制移除“运行中”的 autofix-gate-* 容器。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Fixed in 4e0931e — the container is named qwen-code-gate-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}-$$ so the pool's existing name=qwen-code-* janitors can see it, and a trap teardown EXIT INT TERM runs docker rm -f so a step timeout / job cap / cancel does not leave it running. Probed: no qwen-code-gate-* container remains after a run.
| --env HOME="${CTEMP}" \ | ||
| --env BRANCH="${BRANCH}" \ | ||
| --env WORKDIR="${WORKDIR}" \ |
There was a problem hiding this comment.
[Critical] The env allowlist omits CI. GitHub Actions sets CI=true in every host step, so the pre-change host gate ran with it — inside the container it is unset. packages/cli/src/ui/auth/AuthDialog.test.tsx gates 18 TUI-input-simulation tests on CI === 'true', documented as timing-flaky on CI runners. In the container all 18 run → a timing flake fails vitest → run_check_no_ab → reject_fix with outcome=failed. Package tests are A/B-exempt by design, so the rejection cannot route to preexisting: it is charged to the round, burns the repair leg on a flake no repair can reach, and on recurrence becomes an EVALUATED rejection (watermark advances, item handed off) — the stranding class the A/B machinery exists to prevent, now reachable through the wrapper alone. — Failure scenario: a round touching the auth UI's import closure runs the 18 unsuppressed TUI tests; one simulated-keypress timing flake → the gate rejects the fix; recurrence → evaluated rejection and handoff of an item whose only "defect" is this CI-detection gap.
Witness (executed, two arms): CI=true → Tests 7 passed | 18 skipped (25); env -u CI → Tests 1 failed | 24 passed (25) (failing test 'AuthDialog > drives API key provider steps from endpoint options metadata', 2/2 runs). In-image probe with the wrapper's exact allowlist: CI=[].
Suggested fix (also add CI to the test's expected passedEnv allowlist):
| --env HOME="${CTEMP}" \ | |
| --env BRANCH="${BRANCH}" \ | |
| --env WORKDIR="${WORKDIR}" \ | |
| --env HOME="${CTEMP}" \ | |
| --env BRANCH="${BRANCH}" \ | |
| --env WORKDIR="${WORKDIR}" \ | |
| --env CI=true \ |
中文说明
env 白名单遗漏了 CI。GitHub Actions 在每个宿主步骤中都设置 CI=true，改动前的宿主门带着它运行——而在容器内它未设置。packages/cli/src/ui/auth/AuthDialog.test.tsx 以 CI === 'true' 为条件跳过 18 个 TUI 输入模拟测试，其注释明确记录这些测试在 CI runner 上时序性 flaky。容器内这 18 个测试全部运行 → 一次时序 flake 使 vitest 失败 → run_check_no_ab → reject_fix 且 outcome=failed。按设计包测试不做 A/B，因此该拒绝无法归入 preexisting：它记在本轮账上、烧掉修复环节去修一个修复根本无法触及的 flake，再次发生则成为“已评估拒绝”（水位前进、条目移交）——正是 A/B 机制要防止的搁置类别，如今仅通过 wrapper 即可触发。——失败场景：某个触及 auth UI 导入闭包的轮次运行这 18 个未被跳过的 TUI 测试；一次模拟按键时序 flake → 门拒绝修复；再次发生 → 一个唯一“缺陷”只是此 CI 检测缺口的条目被已评估拒绝并移交。
证据（已执行，双臂）：CI=true → Tests 7 passed | 18 skipped (25)；env -u CI → Tests 1 failed | 24 passed (25)（失败测试 'AuthDialog > drives API key provider steps from endpoint options metadata'，2/2 次运行均失败）。以 wrapper 完全相同的白名单在镜像内探测：CI=[]。
建议修复（同时把 CI 加入测试期望的 passedEnv 白名单）：
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Fixed in 4e0931e — --env CI=true added. Probed inside the container: CI='true'. Thanks for tracing it to the 18 AuthDialog TUI tests; the failure mode you describe (a timing flake charged to the round, unreachable by the A/B because package tests are exempt) is exactly the stranding class this must not introduce.
| GATE_CONTAINER_SHA256: '${{ steps.stage.outputs.gate_container_sha256 }}' | ||
| # The image the gate runs in, from the resolve step's OUTPUT rather | ||
| # than $GITHUB_ENV: branch code must not be able to choose it. | ||
| GATE_IMAGE: '${{ steps.sandbox.outputs.image }}' |
There was a problem hiding this comment.
[Suggestion] 'Verification gate' and 'Repair verification gate' carry byte-identical env+run blocks, and this PR extended both in duplicate (the GATE_CONTAINER_SHA256/GATE_IMAGE env and the second digest-verify line — every change made twice; verified diff: BYTE-IDENTICAL, 26 lines each). The structural test loops over both gates with toContain assertions but nothing binds the two blocks EQUAL, so a future hardening (new digest, image-source change, env addition) applied to only one copy leaves the first-pass or the repair gate silently running the softer configuration — an asymmetric security posture between the two passes, caught only if a test happens to assert that exact line. — Failure scenario: a future digest or env hardening lands on one gate step only; one pass runs the softer configuration while all tests stay green.
Suggested fix: extract the shared digest-verify + wrapper invocation into one staged script invoked by both steps (if that fits the workflow's structural-test style), or add cross-reference comments plus a test assertion binding the two blocks equal.
中文说明
'Verification gate' 与 'Repair verification gate' 携带逐字节相同的 env+run 块，本 PR 以重复的方式扩展了两者（GATE_CONTAINER_SHA256/GATE_IMAGE env 与第二条 digest 校验行——每个改动都做两次；已用 diff 验证：逐字节相同，各 26 行）。结构测试用 toContain 断言遍历两个门，但没有任何断言约束两个块相等，因此未来某次加固（新 digest、镜像来源变更、新增 env）若只应用到其中一份，首轮或修复轮的门就会悄悄运行较弱的配置——两道关口之间出现不对称的安全姿态，只有当测试恰好断言到那一行时才会被发现。——失败场景：未来的 digest 或 env 加固只落在一个门步骤上；其中一个关口运行较弱配置，而所有测试保持绿色。
建议修复：把共享的 digest 校验 + wrapper 调用抽成一个暂存脚本、由两个步骤共同调用（若符合 workflow 结构测试的风格），或添加互相引用的注释并补充“两块相等”的测试断言。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Fixed in 051d88a — the two gate steps' run bodies are now pinned identical (whitespace-normalized), so a guard added to the first pass cannot silently miss the repair pass.
| MEASURED_AT="$(date -u +%Y-%m-%dT%H:%M:%SZ)" | ||
| echo "measured_at=${MEASURED_AT}" >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
[Suggestion] The growth-divergence hunks in this diff (measured_at=/MEASURED_AT, GROWTH_NOW_CUTOFF, the measured= jq rewrite — the #9114 R2-6/R2-8/R6-3 fixes) are the ENTIRE scope of separate, still-OPEN PR #9192 stacked underneath this branch. This PR's own commit touches only the four container-gate files; the compare API shows the branch is #9192's head + exactly 1 commit. The PR description never names #9192 or #9114. — Failure scenario: if a maintainer merges this PR first (GitHub presents the combined change as this PR's diff), the #9114 fix lands via a PR whose description never mentions it, #9192 is left stale and conflicting but open, and #9114's tracking diverges permanently. (Context for reviewers: the growth-divergence findings in this review concern hunks that belong to #9192.)
Witness (executed): gh api compare/91eb70be...844d6825 → status: ahead, ahead_by: 1, behind_by: 0; gh pr view 9192 → state: OPEN, mergedAt: null, headRefOid: 91eb70be… (exactly this PR's parent commit); grep of the PR body for 9192/9114/growth/stack → no match.
Suggested fix: state in the PR description that this branch is stacked on #9192 and #9192 must merge first (after which this diff shrinks to the container-gate commit), or close #9192 and adopt its scope here with #9114 named in the body.
中文说明
本 diff 中的增长差异相关代码块（measured_at=/MEASURED_AT、GROWTH_NOW_CUTOFF、measured= jq 重写——即 #9114 R2-6/R2-8/R6-3 的修复）是另一个仍然 OPEN 的 PR #9192 的全部范围，该 PR 垫在本分支之下。本 PR 自己的提交只触及四个容器门文件；compare API 显示本分支 = #9192 的 head + 恰好 1 个提交。PR 描述从未提及 #9192 或 #9114。——失败场景：若维护者先合并本 PR（GitHub 会把合并后的变更呈现为本 PR 的 diff），#9114 的修复将经由一个描述中从未提及它的 PR 落地，#9192 则变得陈旧、冲突却仍然开放，#9114 的跟踪将永久失准。（给审阅者的背景：本评审中关于增长差异的发现，涉及的是属于 #9192 的代码块。）
证据（已执行）：gh api compare/91eb70be...844d6825 → status: ahead, ahead_by: 1, behind_by: 0；gh pr view 9192 → state: OPEN, mergedAt: null, headRefOid: 91eb70be…（恰为本 PR 的父提交）；对 PR 描述 grep 9192/9114/growth/stack → 无匹配。
建议修复：在 PR 描述中说明本分支叠加在 #9192 之上、需先合并 #9192（之后本 diff 收缩为容器门提交），或关闭 #9192 并在本 PR 描述中点名 #9114、承接其范围。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
You're right — this branch was cut from the #9192 branch, so it carried those hunks. It now merges the CURRENT #9192 head, so the withdrawn head-move code is gone from this diff; the remaining growth hunks are #9192's measurement-time filter. Un-stacking them completely needs a force-push over a merge commit the maintainer pushed to this branch, so I have left that to them.
| const dockerRun = | ||
| wrapper.match(/docker run --rm[\s\S]*?"\$\{GATE_IMAGE\}"/)?.[0] ?? ''; |
There was a problem hiding this comment.
[Suggestion] The new container test extracts the docker run invocation with a non-greedy match stopping at "${GATE_IMAGE}", so the container's COMMAND — that it runs the staged ${CTEMP} copy of the gate, not some other script — falls outside every assertion. The staging comment's whole purpose ("an in-branch copy would let branch code define its own gate") is thus unguarded by the test that pins the rest of the invocation. — Failure scenario: concrete surviving mutant: change the wrapper's final line to bash "${GITHUB_WORKSPACE}/.github/scripts/run-autofix-review-verification.sh" — the container executes the gate from the branch-controlled working tree, letting a branch hollow out its own referee; every assertion in the new test still passes.
Witness (executed): mutant applied → Test Files 1 passed (1) / Tests 174 passed (174) (survives the whole suite); mutant + the suggested assertion → AssertionError: expected '' to be truthy (the fix catches it). Both reverted.
Suggested fix: extend the match past the image to the command (e.g. /docker run --rm[\s\S]*?bash "\$\{CTEMP\}\/run-autofix-review-verification\.sh"/) and assert it, mirroring the existing step-level invocation assertions.
中文说明
新的容器测试以非贪婪匹配提取 docker run 调用、到 "${GATE_IMAGE}" 为止，因此容器的命令部分——即它运行的是暂存的 ${CTEMP} 门副本而非其他脚本——落在所有断言之外。暂存注释的全部目的（“分支内副本会让分支代码定义自己的门”）因此恰恰由钉住调用其余部分的测试所失守。——失败场景：可具体存活的突变：把 wrapper 最后一行改为 bash "${GITHUB_WORKSPACE}/.github/scripts/run-autofix-review-verification.sh"——容器将从分支可控的工作树执行门，让分支掏空自己的裁判；新测试的所有断言仍然通过。
证据（已执行）：应用突变 → Test Files 1 passed (1) / Tests 174 passed (174)（整套存活）；突变 + 建议断言 → AssertionError: expected '' to be truthy（修复可捕获）。均已还原。
建议修复：把匹配延伸到镜像之后的命令（例如 /docker run --rm[\s\S]*?bash "\$\{CTEMP\}\/run-autofix-review-verification\.sh"/）并断言之，与现有步骤级调用断言保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Addressed in 051d88a — the extraction still stops at "${GATE_IMAGE}", but everything after it is now pinned separately (the entrypoint bash "${CBIN}/run-autofix-review-verification.sh" is asserted, and the two gate steps' bodies are pinned equal), so a trailing-argument mutant no longer hides behind the non-greedy match.
| --volume "${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}" \ | ||
| --volume "${WORKDIR}:${WORKDIR}" \ |
There was a problem hiding this comment.
[Suggestion] The rw WORKDIR mount carries verdict-INPUT files consumed by the gate AFTER its build legs: branch code inside the container can plant ${WORKDIR}/no-action.md to manufacture a false noop (the core-rebuild leg runs the branch's own build BEFORE the no-op check; assert_verification_tree cannot see writes outside the workspace), or delete gate-advisories.md (written before the build legs) to erase machine-measured warnings from the report. A manufactured noop publishes the planted file's content as the bot's "no changes needed" rationale, advances the watermark, records a redcheck — and can additionally resolve human review threads via the equally plantable resolved-comments.txt. The access predates this PR (the host gate had the same reach), but the diff establishes the container wall as the stated trust boundary and this channel crosses it ungated — a genuine exit 0 produced by a gate fooled through its inputs, a sibling state the wrapper's rationale never argues. — Failure scenario: a PR touches packages/core; the agent run crashes producing no commit/failure.md/no-action.md; the gate runs via if: always(); the branch's build script writes ${WORKDIR}/no-action.md → gate sees unchanged branch + no-action.md → outcome=noop exit 0 → the bot publishes attacker-authored text under bot identity and marks the feedback evaluated.
Witness (executed, real gate): ARM 1 (planted no-action.md) → gate exit: 0 / outcome=noop; ARM 2 (no file) → gate exit: 1 / outcome=failed. Wrapper translation of ARM-1's verdict at rc=0 → host output outcome=noop + verified_head forwarded.
Suggested fix: snapshot the verdict-relevant WORKDIR inputs the gate reads after build legs (no-action.md, address-summary.md, gate-advisories.md, resolved-comments.txt, comment-replies.json) host-side before docker run — copy into a host-owned dir bind-mounted read-only, or hash pre-run and re-verify before the translation honors a noop/advisory-bearing verdict.
中文说明
rw 的 WORKDIR 挂载携带了门在 build 环节之后才消费的裁决输入文件：容器内的分支代码可以放置 ${WORKDIR}/no-action.md 来制造假 noop（core 重建环节在“无操作”检查之前运行分支自己的 build；assert_verification_tree 看不到工作区之外的写入），或删除 gate-advisories.md（在 build 环节之前写入）以抹掉报告中的机器测量告警。制造出的 noop 会把被放置文件的内容作为 bot 的“无需修改”理由发布、推进水位、记录 redcheck——还能通过同样可被放置的 resolved-comments.txt 解决人类评审线程。该访问权限在本 PR 之前就存在（宿主门有同样的触及范围），但本 diff 把容器墙确立为声明的信任边界，而这条通道未经把关地穿越了它——一个被输入欺骗的门产生出诚实的退出码 0，是 wrapper 理由从未论证过的姊妹状态。——失败场景：某 PR 触及 packages/core；agent 运行崩溃，未产生提交/failure.md/no-action.md；门经 if: always() 运行；分支的 build 脚本写入 ${WORKDIR}/no-action.md → 门看到未变更分支 + no-action.md → outcome=noop 退出码 0 → bot 以 bot 身份发布攻击者编写的文本并把反馈标记为已评估。
证据（已执行，真实门）：ARM 1（放置 no-action.md）→ gate exit: 0 / outcome=noop；ARM 2（无文件）→ gate exit: 1 / outcome=failed。wrapper 对 ARM-1 裁决在 rc=0 下的翻译 → 宿主输出 outcome=noop + verified_head 被转发。
建议修复：在 docker run 之前于宿主侧快照门在 build 环节后读取的裁决相关 WORKDIR 输入（no-action.md、address-summary.md、gate-advisories.md、resolved-comments.txt、comment-replies.json）——复制到以只读绑定挂载的宿主属目录，或在运行前取哈希、并在翻译承认 noop/含告警裁决之前重新校验。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Fixed in 051d88a — the four agent-authored verdict inputs (no-action.md, address-summary.md, resolved-comments.txt, comment-replies.json; the gate writes none of them) are fingerprinted before the run and re-checked after; any change refuses the verdict with exit 125, which takes the gate-crashed retry path. Probed with your ARM 1: a planted no-action.md + outcome=noop now yields an empty host output and the refusal error instead of publishing attacker text as the bot's rationale. gate-advisories.md is deliberately not in the set — the gate writes it itself during the run.
| const mounts = [...dockerRun.matchAll(/--volume "([^:]+):/g)].map( | ||
| (m) => m[1], | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The docker-run assertions pin the env-name allowlist, the three redirected env values, the mount SOURCES (regex --volume "([^:]+): captures only the source side), --user and --rm — but not the --workdir "${GITHUB_WORKSPACE}" flag, the mount TARGETS, or the BRANCH/WORKDIR/FOOTPRINT_ENFORCE env values, all of which sit inside the extracted dockerRun text. — Failure scenario: a future edit drops --workdir "${GITHUB_WORKSPACE}": the image sets no WORKDIR, the container starts at /, the gate dies exit 128 (fatal: not in a git directory at git config core.hooksPath under set -e), every gate run reports as a gate crash (bot never pushes) while every assertion stays green. A mount-retargeting mutant likewise survives (sources-only comparison).
Witness (executed): --workdir-drop mutant → Tests 1 passed | 173 skipped (survives); with the suggested assertion → AssertionError: expected 'docker run --rm …' to contain '--workdir "${GITHUB_WORKSPACE}"' (flips). Gate repro with a non-git cwd: GATE_EXIT=128, fatal: not in a git directory.
Suggested fix: expect(dockerRun).toContain('--workdir "${GITHUB_WORKSPACE}"'); capture full mount pairs (match --volume "([^"]+)" and assert source === target for each of the three); pin --env BRANCH="${BRANCH}" and --env WORKDIR="${WORKDIR}".
中文说明
docker-run 断言钉住了 env 名称白名单、三个重定向的 env 值、挂载源（正则 --volume "([^:]+): 只捕获源端）、--user 与 --rm——但没有钉住 --workdir "${GITHUB_WORKSPACE}" 标志、挂载目标、以及 BRANCH/WORKDIR/FOOTPRINT_ENFORCE 的 env 值，而这些都在提取出的 dockerRun 文本之内。——失败场景：未来的编辑删掉 --workdir "${GITHUB_WORKSPACE}"：镜像未设置 WORKDIR，容器从 / 启动，门在 set -e 下于 git config core.hooksPath 处以退出码 128 死亡（fatal: not in a git directory），每次门运行都报告为门崩溃（bot 永不推送），而所有断言保持绿色。重定向挂载目标的突变同样存活（只比较源端）。
证据（已执行）：删除 --workdir 的突变 → Tests 1 passed | 173 skipped（存活）；加上建议断言后 → AssertionError: expected 'docker run --rm …' to contain '--workdir "${GITHUB_WORKSPACE}"'（翻转）。以非 git 的 cwd 复现门：GATE_EXIT=128、fatal: not in a git directory。
建议修复：expect(dockerRun).toContain('--workdir "${GITHUB_WORKSPACE}"')；捕获完整挂载对（匹配 --volume "([^"]+)" 并对三者分别断言 源 === 目标）；钉住 --env BRANCH="${BRANCH}" 与 --env WORKDIR="${WORKDIR}"。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Fixed in 051d88a — the test now compares each mount's TARGET to its source, and pins --workdir "${GITHUB_WORKSPACE}" plus --env BRANCH/--env WORKDIR. Thanks for the exit-128 trace; that mutant would have been invisible.
… filter Review found the head-move half of this change broken in three ways (all probe-verified), so it is withdrawn and returned to #9114 rather than patched under review: - R1-1 (regression): `autofix-redcheck` records the head the agent was GIVEN, frozen before its push — so after any pushing round the next round's head differs and the cutoff was set to now, dropping every prior sum. In the push regime OVER_ROUNDS_PRIOR could never reach the threshold and the #9104 handoff would never fire at all. - R1-2: the cut was stateless — the round after a correct re-anchor fell back to an empty cutoff and re-admitted every pre-move sum. - R1-3: with no redcheck marker (a crash round) the `-n` guard skipped re-anchoring across a genuine external move. A correct version needs both a bot-authored-move test and a PERSISTED cut; that is its own change. What remains is the measurement-time filter (R2-6), which stands on its own: the marker carries the prepare-time instant and the divergence read filters/orders on it instead of the comment's post-agent created_at. Also from this review: - R1-4: `measured=` is OPTIONAL in the scan, falling back to the comment's created_at, so deploying does not blank an in-flight window's census. - R1-9: the per-run collapse now runs BEFORE the over/window/cutoff filters — a re-run whose fresh attempt came back under budget was still represented by its stale over=true attempt. - R1-7: comments corrected — run= is the DEDUP identity, measured= the ORDER key (four sites). - R1-8: recorded as a known residual next to the sibling growth-base reader, which still filters on created_at; tracked in #9114. - R1-5/R1-6: fixtures decouple created_at from measured=, cover a legacy marker (with and without the cutoff), and pin the measured_at source line in prepare.
…-gate-container-9089
All seven Criticals from the round-1 review: - The staged scripts are now mounted READ-ONLY, with a separate writable scratch mount for what the gate legitimately writes (its throwaway git config, via a new GATE_TMPDIR that defaults to RUNNER_TEMP on host runs). bash reads a script incrementally, so a writable copy of the running gate let the branch code the gate itself executes rewrite its own remaining bytes — the exit code was no longer unforgeable. - Exit 1 no longer synthesizes `outcome=failed`. The gate has deliberate exit-1-WITHOUT-verdict paths (baseline-A/B and bite tree-restore failures) where an evaluated rejection would advance the watermark and hand the item off for good; `failed` is now taken from the verdict file only. Forgery protection is unchanged: `fixed` is accepted only on exit 0, so a forged pass on a failing gate leaves the outcome unset and the round retries. - The container is named `qwen-code-gate-*` (so the pool's existing stale-container janitors can see it) and torn down by a trap: `--rm` only fires on a normal exit, but a step timeout / job cap / cancel kills the docker client and left an orphan running branch code as the runner uid with rw mounts on the shared workspace. - `CI=true` is passed: Actions sets it on every host step, and without it the branch's own suites take their non-CI path (18 TUI tests suppressed on CI as timing-flaky would run, and a flake is charged to the round). - The wrapper is added to the gate's `autofix-loop` sensitive class; it was falling through to the broader `ci-scripts` class, so any PR touching `.github/scripts/*` licensed a round to rewrite the referee. - The re-anchor finding is the withdrawn half of #9192, merged in here so this PR's diff no longer carries it. - `scripts/tests/package-scripts.test.js` asserted the old direct invocation — the red CI check at the reviewed SHA, and my miss for running only one suite locally. Re-probed against the real wrapper and sandbox image: the running gate script is read-only to the code it executes, the scratch mount is writable, CI/GATE_TMPDIR arrive, the PAT does not, no container is left behind, and all four verdict arms behave (genuine pass → fixed; genuine rejection → failed+flags; forged pass on a failing gate → unset/retry; exit 1 with no verdict → unset/retry). Full scripts/tests suite green, including package-scripts.
From the round-1 Suggestions, the substantive ones: - Sandbox posture now matches repo-hygiene.yml's SANDBOX_ARGS, which runs the same command set in the same image: --network none (the gate builds and tests against node_modules installed host-side and needs no egress, so a malicious branch's build script can no longer exfiltrate what it can read), --cap-drop ALL, --security-opt no-new-privileges, --init. - The agent-authored verdict INPUTS in the rw WORKDIR mount are fingerprinted around the run. Branch code could otherwise plant no-action.md to manufacture a `noop` — published under the bot's identity as its own "no changes needed" rationale, advancing the watermark — or plant resolved-comments.txt to close human review threads. A change to any of the four now refuses the verdict and takes the retry path. - The three helper scripts the gate EXECUTES (schema, contracts, resolver) are staged in the same writable RUNNER_TEMP as everything else but were copied across the wall undigested; they are now digested at staging and verified before the wrapper runs. Test hardening from the same review: - mount TARGETS are compared to their sources, --workdir/--env BRANCH/WORKDIR pinned (a --workdir drop would make every gate run die exit 128 in git while the suite stayed green), and the new posture flags are pinned. - verdict-translation arms added: noop, last-wins extraction, preexisting under rc=1, committed on a failing path. - the trust-boundary regex also catches npm install/i/rebuild/exec, pnpm and yarn. - the two gate steps' run bodies are pinned identical, so a guard added to the first pass cannot miss the repair pass. - `id: 'sandbox'` is pinned directly and `exportImage` gained a unit test: the step output is load-bearing (empty GATE_IMAGE → the wrapper refuses → every round takes the gate-crashed retry), and both mutants the review ran survived the suite before. Re-probed against the real image: no egress inside the container, a planted no-action.md is refused (host output empty, retry), and the normal pass still passes.
What this PR does
Runs the autofix verification gate inside an ephemeral container instead of on the host, and pins the resulting trust boundary with a structural test. This is Phase 1 + Phase 2 of the design agreed in #9089.
Verification gate,Repair verification gate) now invoke a staged, digest-verified wrapper that runsrun-autofix-review-verification.shin the sandbox image the agent already uses. The image name comes from the resolve step's output (expression context), not$GITHUB_ENV, so branch code cannot choose it.docker rundoes not inherit the host environment, so inside the container there is noCI_DEV_BOT_PAT, no$GITHUB_ENV, no real$GITHUB_OUTPUT,$HOMEis a throwaway, and only three paths are mounted: the workspace, the round's workdir, and a copy-staged container temp. The realRUNNER_TEMP— which holds the staged agent runner and the PAT steps' throwaway git/gh configs — is never mounted.failedregardless of the file, and any other code leaves the outcome unset for the existing gate-crash retry path.CI_DEV_BOT_PATinvokes branch-authored code (the gate script, the agent runner,npm/npx) outside a container.Why it's needed
The gate executes the branch's own
build/typecheck/lint/test. On the host that code shares the OS user,$HOME,$GITHUB_ENVand$GITHUB_OUTPUTwith the same job's later PAT-bearing steps, and #8961's review showed several channels that no in-step guard can reach:BASH_ENV/BASH_FUNC_*— applied at the step shell's startup, before line 1 (the runner's$GITHUB_ENVblocklist isNODE_OPTIONS-only).LD_PRELOAD/LD_AUDIT— mapped by the loader into the step's own bash at exec time, with the PAT already inenviron.$GITHUB_OUTPUT— a runner-provided file the gate-run branch code can appendoutcome=fixedto, regardless of any digest check on the gate script's bytes.The trust boundary cannot be a job boundary. A job that has executed attacker code cannot vouch for anything it emits (its outputs, its "gate passed" verdict, even a signature), and a second job that re-verified would have to run that same code — while holding the PAT. So the boundary has to be an execution-isolation boundary the PAT never crosses: the container wall.
Reviewer Test Plan
How to verify
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js— 174/174. Two new tests: one pins the container invocation (wrapper + both digests, image fromsteps.sandbox.outputs.image, the exact--envallowlist and--volumeset,--user/--rm) and drives the verdict translation behaviourally; the other is the PAT/branch-code invariant.ghcr.io/qwenlm/qwen-code), not a mock:CI_DEV_BOT_PATvisible inside the container<absent>✅$GITHUB_ENVvisible inside<absent>✅RUNNER_TEMPreachable inside$GITHUB_OUTPUTinside$GITHUB_ENVafter the runoutcome=fixedin the verdictoutcome=failed+retryable=true✅Evidence (Before & After)
Before:
bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"on the host — branch build/test in the same execution context as the PAT.After:
bash "${RUNNER_TEMP}/run-autofix-gate-container.sh"→docker runwith an explicit six-entry env allowlist and three mounts; the PAT/$GITHUB_ENV/host$GITHUB_OUTPUTare absent by construction.Tested on
Risk & Scope
docker infoand the agent already runs in this image against this same workspace with the samenpm run build/typecheck/lint+npx vitestcommands, so the toolchain is proven — but the first real round is where a mount/permission surprise would show.--user $(id -u):$(id -g)keeps container writes owned by the runner user (the ownership-restore step's failure mode). A docker failure is a gate crash (retry), never a silent pass.issue-autofixjob's inline gate (same class, but agent-authored branch rather than an external contributor's PR) and Phase 3 (per-job ephemeral runners, a pool-infra decision) — both stay tracked in autofix: PAT-bearing jobs share a host with untrusted branch code — needs runner-level isolation #9089.Linked Issues
中文说明
本 PR 做了什么
把 autofix 的验证门放进临时容器执行(不再在宿主上),并用结构性测试钉住由此建立的信任边界。这是 #9089 中已确认设计的阶段 1 + 阶段 2。
Verification gate、Repair verification gate)现在调用一个暂存且经 digest 校验的 wrapper，由它在 agent 已在使用的 sandbox 镜像中运行run-autofix-review-verification.sh。镜像名取自 resolve 步骤的输出(expression context)而非$GITHUB_ENV，因此分支代码无法选择它。docker run不继承宿主环境，故容器内没有CI_DEV_BOT_PAT、没有$GITHUB_ENV、没有真实的$GITHUB_OUTPUT，$HOME是一次性目录，且只挂载三个路径：workspace、本轮 workdir、以及拷贝暂存的容器 temp。真实的RUNNER_TEMP(存放暂存的 agent runner 与 PAT 步骤的一次性 git/gh 配置)从不挂载。failed(无视文件内容)，其他退出码则不设置 outcome，走既有的门崩溃重试路径。CI_DEV_BOT_PAT的步骤在容器之外调用分支编写的代码(门脚本、agent runner、npm/npx)"时失败。为什么需要
门执行的是分支自己的
build/typecheck/lint/test。在宿主上，这些代码与同一 job 后续携带 PAT 的步骤共享 OS 用户、$HOME、$GITHUB_ENV与$GITHUB_OUTPUT，而 #8961 的评审已证明有几条通道任何步骤内的防御都够不到：BASH_ENV/BASH_FUNC_*—— 在步骤 shell 启动时即生效，早于第 1 行(runner 对$GITHUB_ENV的屏蔽名单只有NODE_OPTIONS)。LD_PRELOAD/LD_AUDIT—— 由加载器在 exec 步骤自身 bash 时映射进去，此时 PAT 已在environ中。$GITHUB_OUTPUT—— runner 提供的文件，门中运行的分支代码可直接追加outcome=fixed，无论 digest 如何校验门脚本的字节。信任边界不能是 job 边界。 执行过攻击者代码的 job 无法为其产出的任何东西背书(输出、"门已通过"的裁决、甚至签名)，而负责重新校验的第二个 job 又必须运行同一批代码——同时还持有 PAT。因此边界必须是一道 PAT 永不跨越的执行隔离边界：容器墙。
审阅者测试计划
如何验证
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js—— 174/174。两个新测试：其一钉住容器调用(wrapper + 两个 digest、镜像取自steps.sandbox.outputs.image、精确的--env白名单与--volume集合、--user/--rm)并对裁决翻译做行为性验证；其二是 PAT/分支代码不变量。ghcr.io/qwenlm/qwen-code)探针验证，非 mock：CI_DEV_BOT_PAT<absent>✅$GITHUB_ENV<absent>✅RUNNER_TEMP$GITHUB_OUTPUT$GITHUB_ENVoutcome=fixedoutcome=failed+retryable=true✅证据(Before & After)
Before：宿主上
bash "${RUNNER_TEMP}/run-autofix-review-verification.sh"—— 分支 build/test 与 PAT 处于同一执行上下文。After：
bash "${RUNNER_TEMP}/run-autofix-gate-container.sh"→docker run，带显式六项 env 白名单与三个挂载；PAT/$GITHUB_ENV/宿主$GITHUB_OUTPUT由构造上即不存在。测试平台
风险与范围
docker info预检，且 agent 本就在这个镜像中、对同一 workspace 运行同样的npm run build/typecheck/lint与npx vitest，工具链已被证明可用——但首个真实轮次才是挂载/权限意外会暴露的地方。--user $(id -u):$(id -g)确保容器写入仍归 runner 用户所有(即 ownership-restore 步骤所针对的故障模式)。docker 失败属于门崩溃(重试)，绝不会成为静默通过。issue-autofixjob 的内联门(同类问题，但分支由 agent 编写而非外部贡献者 PR)与阶段 3(每 job 临时 runner，属池基础设施决策)——两者继续在 autofix: PAT-bearing jobs share a host with untrusted branch code — needs runner-level isolation #9089 跟踪。关联 Issue