fix(autofix): bind the sandbox image to its pulled digest - #9527
Conversation
The sandbox image was exported as a mutable tag. `docker run <tag>` resolves against the local store without re-pulling, so a co-resident process with daemon access can `docker tag` different content under the same name between the resolve step and the consumer. Export the `<repo>@sha256:...` RepoDigests entry that matches both the pulled repository and the digest the pull itself reported: RepoDigests is shared by every tag of the same content, so index 0 can move off the pulled repo under a same-content retag, and retagged foreign content keeps its own repo — only the pair binds the export to what the pull fetched. Pin the daemon endpoint for both spawns. The docker CLI resolves its endpoint from DOCKER_HOST, then --context, then DOCKER_CONTEXT, then `currentContext` in the pool-shared config.json; clearing DOCKER_CONTEXT falls through to that last one, so the context is named explicitly and DOCKER_HOST is dropped from the child environment. An inspect answered by someone else's daemon hands back any digest it likes. Write the step files through a non-blocking, type-checked append. $GITHUB_ENV and $GITHUB_OUTPUT live under the runner-writable temp tree, where a planted FIFO turns a plain append into a block until the step timeout. Extracted from #9214, which is frozen; these were R11-1 and R11-2 there. The inspect timeout is now injectable so the tests can pin it, and the suite covers the endpoint pin on both spawns, the FIFO and directory refusals, cross-chunk stdout accumulation, and the timeout itself. Each new test was checked against a mutant of the code it pins. Refs #9089, #9524.
|
Third gate pass — re-run after review rounds 4–6. The previous pass approved Template: unchanged — the body uses its own sectioning ("What it does" / "Verification") rather than the template headings. Substance pass per precedent (#9517, #9518): everything the template asks for is present, including the mutation-verified reviewer test plan. Noted, not blocked. Problem: exists and is observed, on both fronts. The underlying threat model is #9089 (PAT-bearing jobs co-resident with untrusted branch code), with the maintainer's process-level reproduction of the dead-output hole and the round-4 probes: R4-2 came with an empirical docker 29.1.3 probe showing Direction: aligned — residual closure for the #9089 runner-isolation effort, scoped to the CI resolver script, the one workflow that consumes it, and the 6-line CLI presence check the digest export surfaced. CI infrastructure, so no CHANGELOG signal applies. Size: not applicable — no core paths. Production logic is ~262 lines: Approach: the two fixes map one-to-one onto the round-4 findings with nothing extra: step-env Risk: Moving on to code review. 🔍 中文说明第三次门检查——评审第 4–6 轮之后的复跑。上一轮在 模板：不变——正文使用自定义小节而非模板标题。按先例（#9517、#9518）实质通过：模板要求的信息齐全，含带变异验证的审阅测试计划。只记录，不阻塞。 问题：真实存在且有观测，两条都是。底层威胁模型是 #9089（携带 PAT 的 job 与不可信分支代码同宿），maintainer 提供过"输出无人消费"漏洞的进程级复现，第 4 轮有探针佐证：R4-2 附 docker 29.1.3 实测（本地已存在的 digest 引用在 方向：对齐——#9089 runner 隔离工作的收尾，范围限于 CI 解析脚本、消费它的那一个 workflow、以及 digest 导出暴露出的 6 行 CLI 存在性检查。CI 基础设施，不涉及 CHANGELOG。 规模：不适用——未触及核心路径。生产逻辑约 262 行： 方案：两个修复与第 4 轮的发现一一对应，无额外内容：两个 resolver 步骤的 step env 固定 风险： 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal for the two round-4 Criticals, before reading the fix commits: for R4-1, pin the resolver binary in each resolver step's
No blockers found this round. The standing non-blocking items are the round-6 deferred list (consumer-side Test evidenceThis is an unattended CI run — no PR code was built or executed here; the evidence below is the PR's own CI on the reviewed commit, fetched via the API. All
The ubuntu unit run is the one that exercises this PR's suites (the resolver's Sandboxed verification would settle that residual: 中文说明代码审查针对第 4 轮的两个 Critical，我在读修复 commit 之前的独立方案是：R4-1 在每个 resolver 步骤的
本轮未发现阻断项。遗留的非阻塞项为第 6 轮延后清单（消费方的 测试证据这是无人值守 CI 运行——此处未构建或执行任何 PR 代码；以下为经 API 获取的该 commit 自身 CI 结果。所有 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — both round-4 Criticals are closed exactly as prescribed and pinned by tests that cannot pass vacuously; the remaining items are tracked non-blocking follow-ups. Stepping back: this PR has now survived three layers of adversarial review. The first pass missed that a protected value nobody consumes protects nothing — the maintainer's reproduction proved it. The second pass verified the consumption wiring; round 4 then found that the fixes had extended the attack surface sideways (the export became a function of a steerable binary's stdout, and the digest export broke the consumer's presence check). Both came back closed in a way I can verify at the head tree rather than take on faith: the step-env Verdict: approve. All 中文说明信心度：4/5 —— 第 4 轮的两个 Critical 均按要求原样关闭，并由不会空过的测试钉住；剩余项为已跟踪的非阻塞后续。 退一步看：这个 PR 已经历三层对抗性评审。首轮漏掉了"无人消费的保护值等于没有保护"——maintainer 的复现证明了这一点；第二轮验证了消费接线，第 4 轮又发现修复把攻击面横向扩展（导出变成了可被操纵二进制的 stdout 的函数，digest 导出又打破了消费方的存在性检查）。两者如今都以我能在 head 树上验证（而非凭信任接受）的方式关闭：step env 的 结论：批准。 — 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.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 8a": none (≈20 of 46 calls used)..
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 8a"：none (≈20 of 46 calls used).。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| } catch { | ||
| // Non-JSON output carries no digests. | ||
| } | ||
| const repo = repoOfImage(image); |
There was a problem hiding this comment.
[Suggestion] This comparison uses the textual repo of the requested reference, but docker records Docker Hub repos in canonical short form — docker.io/library/busybox:stable is stored in RepoDigests as busybox@sha256:…, so the exact pair-match can never succeed for a fully-qualified Hub reference. The result is fail-closed but wrong: the pull succeeds, prints its digest, and the resolver then exits 1 with "refusing to export a foreign or mutable reference", blaming attacker content for a legitimate, correctly-pinned image. Both configured images and the fallback are GHCR today (GHCR keeps the fully-qualified name), so nothing hits this yet — but the day config.sandboxImageUri points at a Hub official image, the gate hard-fails and the error sends the debugger hunting for an attack. Canonicalize the way docker's reference store does before matching, and pin the behaviour with a test.
Witness (docker 24.0.9):
$ node .github/scripts/resolve-sandbox-image.mjs docker.io/library/busybox:stable
Pulled image docker.io/library/busybox:stable resolved to digests none of which is
'docker.io/library/busybox@sha256:73aaf090…' (busybox@sha256:73aaf090…);
refusing to export a foreign or mutable reference. → EXIT=1
$ docker image inspect --format '{{json .RepoDigests}}' docker.io/library/busybox:stable
["busybox@sha256:73aaf090…"]
$ node .github/scripts/resolve-sandbox-image.mjs busybox:stable # control, same content
→ EXIT=0, image=busybox@sha256:73aaf090…
| const repo = repoOfImage(image); | |
| const repo = repoOfImage(image).replace(/^docker\.io\/(library\/)?/, ''); |
中文说明
这里的比较使用的是所请求引用的仓库原文，但 docker 会以规范短名记录 Docker Hub 仓库——docker.io/library/busybox:stable 在 RepoDigests 中记录为 busybox@sha256:…，因此对完全限定的 Hub 引用，精确成对匹配永远无法命中。结果是 fail-closed 但错误：pull 成功并打印了 digest，resolver 却以 “refusing to export a foreign or mutable reference” 退出码 1 收场，把一个合法且 digest 绑定正确的镜像归咎为外来内容。当前配置镜像与回退镜像均为 GHCR（GHCR 保留完全限定名），所以暂无影响——但一旦 config.sandboxImageUri 指向 Hub 官方镜像，gate 会硬性失败，且错误信息会把排查引向“遭受攻击”的歧途。已在 docker 24.0.9 上复现（证据见英文部分）：完全限定写法退出码 1，同一内容用非限定写法即可正常导出。建议按 docker 引用存储的规范化规则处理（折叠 docker.io/ 与 docker.io/library/ 前缀）后再比较，并补充测试固定该行为（上方 suggestion 为单行修复示例）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const child = spawn( | ||
| command, | ||
| ['image', 'inspect', '--format', '{{json .RepoDigests}}', image], | ||
| { stdio: ['ignore', 'pipe', 'pipe'], env: sandboxSpawnEnv() }, |
There was a problem hiding this comment.
[Suggestion] The spawn-guard skeleton — spawn + sandboxSpawnEnv() pin + settle-once flag + kill timer + stdout accumulation + error/close wiring — now exists in two near-verbatim copies, pullImage and repoDigestOf, and they have already drifted (stderr inherit vs pipe, {ok, digest} vs raw string result, hard-coded vs injectable timeout). The security invariants this PR exists to establish are thereby maintained in two places: a fix to one kill/error path will not automatically reach the other, and a future third docker invocation copied from either skeleton can silently drop env: sandboxSpawnEnv(), re-opening exactly the DOCKER_HOST/DOCKER_CONTEXT reroute this PR closes. Consider extracting one guarded-spawn helper (e.g. spawnGuarded(command, args, { timeoutMs, label, teeStdout }) returning { code, stdout }) that applies the env pin internally, so the pin is structural for both and for any future callers.
中文说明
spawn 守护骨架——spawn + sandboxSpawnEnv() 端点固定 + settle-once 标志 + kill 定时器 + stdout 累积 + error/close 接线——现在以两份几乎逐字相同的副本存在于 pullImage 与 repoDigestOf 中，且两者已经出现漂移（stderr inherit 对 pipe、{ok, digest} 对裸字符串、硬编码超时对可注入超时）。本 PR 要确立的安全不变量因此要在两处维护：修一处的 kill/error 路径不会自动传到另一处；未来若从任一份骨架复制出第三个 docker 调用，可能悄悄漏掉 env: sandboxSpawnEnv()，从而重新打开本 PR 正要关闭的 DOCKER_HOST/DOCKER_CONTEXT 重定向通道。建议抽取一个统一的守护 spawn 辅助函数（如 spawnGuarded(command, args, { timeoutMs, label, teeStdout })，返回 { code, stdout }），在内部应用环境固定，使该固定对现有两处及未来所有调用方都是结构性的。
— qwen3.8-max via Qwen Code /review (v0.21.14)
There was a problem hiding this comment.
Still deferred — this round is the gate-mandated same-run verification repair (a single commit fixing the footprint rejection by reverting repo-hygiene.yml and rescoping the contract test), and bundling a refactor into it would exceed the repair scope. The extraction itself gets cheaper each round: the previous one aligned both skeletons (stderr inherit in both, exit-code ::error:: logging in both, injectable timeouts in both), so spawnGuarded(command, args, { timeoutMs, label }) applying the sandboxSpawnEnv() pin internally remains a clean follow-up for the next normal round.
中文说明
继续延后——本轮是门禁要求的同轮验证修复（单提交修复足迹拒绝：回退 repo-hygiene.yml 并收缩契约测试范围），把重构捆绑进来会超出修复范围。抽取本身正变得越来越便宜：上一轮已把两份骨架对齐（双方 stderr 均为 inherit、双方均有退出码 ::error:: 日志、双方超时均可注入），因此在内部应用 sandboxSpawnEnv() 端点固定的 spawnGuarded(command, args, { timeoutMs, label }) 仍是下一个正常轮次的干净后续改动。
| if (!fallbackPull.digest) { | ||
| throw new Error( | ||
| `'${command} pull ${fallbackImage}' reported no Digest line; refusing to export an unbound image reference.`, |
There was a problem hiding this comment.
[Suggestion] The "pull reported no Digest line → throw" refusal plus the digest-bound export is duplicated identically in main() for the requested path (lines 301-308) and this fallback path, differing only in variable names. Any change to that refusal policy — different error wording a consumer keys on, or an additional refusal condition — must then be edited in both copies, and editing one leaves the requested and fallback paths enforcing different rules for the same security property. Consider extracting e.g. async function exportDigestBoundImage(command, image, pull) that throws on !pull.digest and calls exportImage(await repoDigestOf(command, image, pull.digest)), invoked from both paths.
中文说明
“pull 未报告 Digest 行 → 抛错”的拒绝逻辑加上 digest 绑定导出，在 main() 的请求路径（301-308 行）与该回退路径中逐字重复，仅变量名不同。对该拒绝策略的任何修改——消费方依赖的错误措辞变化、新增的拒绝条件——都必须同时编辑两处副本；只改一处会让请求路径与回退路径对同一安全属性执行不同的规则。建议抽取如 async function exportDigestBoundImage(command, image, pull)：在 !pull.digest 时抛错并调用 exportImage(await repoDigestOf(command, image, pull.digest))，两条路径统一调用。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| image, | ||
| expectedDigest = '', | ||
| timeoutMs = FETCH_TIMEOUT_MS, |
There was a problem hiding this comment.
[Suggestion] The = '' default keeps a second, weaker binding mode — accept any repo@sha256: entry — that no production caller ever takes: both calls in main() throw on an empty pull digest before reaching repoDigestOf, so the prefix branch is exercised exclusively by tests that omit the parameter. A future caller that omits expectedDigest would silently get exactly the suffix/presence-only digest-check class this PR exists to close (the file's own comment: "a suffix-only digest check still passes"). Making the parameter required removes a branch main() can never reach; the five test call sites that omit it need an explicit digest.
| image, | |
| expectedDigest = '', | |
| timeoutMs = FETCH_TIMEOUT_MS, | |
| image, | |
| expectedDigest, | |
| timeoutMs = FETCH_TIMEOUT_MS, |
中文说明
= '' 默认值保留了第二种更弱的绑定模式——接受任意 repo@sha256: 条目——而没有任何生产调用方会走到它：main() 的两处调用都在到达 repoDigestOf 之前就因 pull 无 digest 而抛错，因此前缀匹配分支只被省略该参数的测试执行。未来若有调用方省略 expectedDigest，就会悄悄得到本 PR 正要关闭的那类“仅看后缀/存在性”的 digest 检查（文件自身注释：“a suffix-only digest check still passes”）。把该参数改为必填可以移除 main() 永远走不到的分支；省略它的五处测试调用需改为传入显式 digest。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| child.on('close', (code) => { | ||
| finish(code === 0 ? stdout.trim() : ''); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] Unlike its twin pullImage (stderr inherit, ::error:: with the exit code on failure), repoDigestOf pipes stderr without ever reading it and drops the exit code here without logging. When docker image inspect fails — daemon unreachable for the pinned default context on a freshly re-registered runner, daemon mid-restart, image store pruned between pull and inspect — the step fails with resolved to no repository digest ('') while docker's own stderr (Cannot connect to the Docker daemon…, Error: No such image: …) sits in an unread pipe, so the operator cannot tell a daemon outage from a missing image from a tampered inspect — precisely the failure class this PR hardens. Switch stderr to 'inherit' at line 220 and log the exit code before resolving '':
| child.on('close', (code) => { | |
| finish(code === 0 ? stdout.trim() : ''); | |
| }); | |
| child.on('close', (code) => { | |
| if (code !== 0) { | |
| console.error( | |
| `::error::'${command} image inspect ${image}' exited with code ${code}.`, | |
| ); | |
| } | |
| finish(code === 0 ? stdout.trim() : ''); | |
| }); |
中文说明
与它的孪生函数 pullImage（stderr 用 inherit、失败时输出带退出码的 ::error::）不同，repoDigestOf 把 stderr 接入管道却从不读取，并在此处不记日志地丢弃退出码。当 docker image inspect 失败时——新注册 runner 上固定的 default context 连不到 daemon、daemon 重启中、pull 与 inspect 之间镜像存储被清理——步骤会以 resolved to no repository digest ('') 失败，而 docker 自己的 stderr（Cannot connect to the Docker daemon…、Error: No such image: …）留在无人读取的管道里，运维无法区分 daemon 故障、镜像缺失还是 inspect 被篡改——这正是本 PR 要加固的故障类别。建议把 220 行的 stderr 改为 'inherit'，并在返回 '' 之前记录退出码（上方 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (requestedPull.ok) { | ||
| if (!requestedPull.digest) { | ||
| throw new Error( |
There was a problem hiding this comment.
[Suggestion] This refusal — the headline behaviour of the PR — lives in main(), which is not exported and has no test; both new throw branches and the fallback-path digest wiring are untested, so the load-bearing decision (what main() passes as expectedDigest) is invisible to the suite. Mutant probe: deleting this if (!requestedPull.digest) throw block leaves all 24 tests green, yet an end-to-end run with a stub whose pull exits 0 without a Digest line and whose inspect reports a same-repo different digest then exports the attacker-retagged content — the exact race this PR exists to close — because repoDigestOf is called with an empty expectedDigest and takes the prefix-match branch. Export main() (or extract the orchestration into an exported function) and add a stub test asserting the throw and empty step files, or drive the script end-to-end via execFileSync with SANDBOX_COMMAND pointing at the stub, the pattern the FIFO test already uses.
PRISTINE: 'docker-stub pull ghcr.io/qwenlm/qwen-code:1.2.3' reported no Digest line;
refusing to export an unbound image reference. EXIT=1, GITHUB_OUTPUT empty
MUTANT (digest check deleted): EXIT=0
GITHUB_OUTPUT: image=ghcr.io/qwenlm/qwen-code@sha256:dc2d74b2…
MUTANT vs suite: 24 pass / 0 fail
中文说明
这个拒绝分支——本 PR 的标志性行为——位于未被导出、也没有任何测试的 main() 中；两处新增的抛错分支与回退路径的 digest 接线都没有测试覆盖，因此承重决策（main() 把什么作为 expectedDigest 传入）对测试套件不可见。变异探测：删除这个 if (!requestedPull.digest) throw 块后 24 个测试仍然全绿，但用 stub 做端到端运行（pull 退出码 0 且不打印 Digest 行、inspect 返回同仓库不同 digest）时，脚本会导出被攻击者 retag 的内容——正是本 PR 要关闭的竞态——因为 repoDigestOf 收到空 expectedDigest 而走了前缀匹配分支。建议导出 main()（或把编排逻辑抽成导出函数）并加一个断言抛错且步骤文件为空的 stub 测试；或者像 FIFO 测试那样，用 execFileSync 把 SANDBOX_COMMAND 指向 stub 做端到端驱动。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (!fstatSync(fd).isFile()) { | ||
| throw new Error( |
There was a problem hiding this comment.
[Suggestion] This post-open type check — which the comment above names as the actual guard ("the type check, not the path, is what holds") — is dead in the test suite: the FIFO test plants a FIFO with no reader, so openSync itself fails with ENXIO before fstatSync runs (its assertion allows /ENXIO|not a regular file/), and the directory test fails at openSync with EISDIR behind a bare assert.throws. Mutant probe: deleting this whole block keeps the suite 24/24 green, yet real inputs the threat model names do reach the check — a FIFO with a reader held open (the write-side open then succeeds) and a symlink to /dev/null both pass openSync and are refused only here. Without the check, the image= line is swallowed by the attacker's reader or the null device, the gate sees an empty image, and every round takes the gate-crashed retry path. Add a test that holds a FIFO reader open (openSync(fifo, O_RDONLY | O_NONBLOCK) succeeds without a writer) and asserts the /not a regular file/ refusal.
MUTANT (fstat block deleted) vs suite: 24 pass / 0 fail
MUTANT vs symlink→/dev/null: write accepted, line went to /dev/null
PRISTINE vs symlink→/dev/null: "… is not a regular file; refusing to write step state to it."
PRISTINE vs FIFO with reader held open: refused (open succeeded, fstat fired)
中文说明
这个 open 之后的类型检查——上方注释称它是真正的守护（“the type check, not the path, is what holds”）——在测试套件中是死代码：FIFO 测试放置的是无读者的 FIFO，openSync 本身就会以 ENXIO 失败，根本到不了 fstatSync（其断言允许 /ENXIO|not a regular file/ 任一）；目录测试则在 openSync 处以 EISDIR 失败，且只用了裸 assert.throws。变异探测：把整块删掉后套件仍是 24/24 全绿，但威胁模型点名的真实输入确实会走到该检查——有读者保持打开的 FIFO（此时写端 open 会成功）与指向 /dev/null 的符号链接都能通过 openSync，只在这里被拒绝。若没有该检查，image= 行会被攻击者的读者或空设备吞掉，gate 看到空镜像，每一轮都走 gate 崩溃重试路径。建议新增测试：保持 FIFO 读者打开（无写者时 openSync(fifo, O_RDONLY | O_NONBLOCK) 成功），断言出现 /not a regular file/ 拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| timer = setTimeout(() => { | ||
| console.error( | ||
| `::error::Timed out pulling ${image} after ${PULL_TIMEOUT_MS / 1000}s.`, |
There was a problem hiding this comment.
[Suggestion] This 10-minute timeout has no injectable bound and no test, in contrast to repoDigestOf, whose identical timeout the diff made injectable (timeoutMs) precisely so a test could pin it — the added test's own comment states the rationale: without an injectable bound a timer mutant ships green. Mutant probe: deleting this setTimeout block leaves the suite 24/24 green (PULL_TIMEOUT_MS is a 10-minute module constant no test can wait out). In production a wedged docker pull against a stalled registry/daemon would then hold the autofix step until the job's 60-minute timeout instead of failing closed after 10 minutes and taking the fallback path. Mirror repoDigestOf: give pullImage a timeoutMs = PULL_TIMEOUT_MS parameter and add a sleep 30 stub test with a small bound asserting { ok: false, digest: '' } returns promptly.
MUTANT (pullImage setTimeout block deleted) vs suite: 24 pass / 0 fail
中文说明
这个 10 分钟超时没有可注入的上限、也没有测试，与 repoDigestOf 形成对比——diff 特意为后者的同类超时加了可注入的 timeoutMs，以便测试固定它；新增测试的注释也说明了原因：没有可注入的上限，定时器变异就能绿灯合入。变异探测：删除这个 setTimeout 块后套件仍 24/24 全绿（PULL_TIMEOUT_MS 是 10 分钟的模块常量，测试等不起）。生产中若 docker pull 卡死在停滞的 registry/daemon 上，autofix 步骤会一直挂到 job 的 60 分钟超时，而不是 10 分钟后 fail-closed 并走回退路径。建议对齐 repoDigestOf：给 pullImage 加 timeoutMs = PULL_TIMEOUT_MS 参数，并新增一个 sleep 30 stub 测试，用小上限断言 { ok: false, digest: '' } 快速返回。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Local merge-readiness validation — changes requestedValidated the exact PR head Verdict: do not merge yet. The digest-selection, endpoint-sanitization, and step-file mechanisms behaved correctly in local process and real-daemon testing, but the new protected Blocking findingCritical — the pinned step output is dead. All six Process-level reproduction:
Affected resolver steps: two in Local test matrixEnvironment: macOS 26.5.2 arm64 host; isolated Ubuntu 24.04 Colima VM; Docker client/server 29.5.2; Node 24.18.0; npm 11.16.0.
The real-daemon run used the PR's supported Required change before mergeGive every resolver step an 中文测试报告本地合并就绪验证——请求修改验证对象为 PR 精确提交 结论：暂不可合并。 digest 选择、Docker 端点清理和 step-file 防护在本地进程级与真实 daemon 测试中均表现正确，但新增的受保护 阻塞问题Critical——固定 digest 的 step output 是死输出。 六个 进程级复现：
受影响范围包括 本地测试矩阵环境：macOS 26.5.2 arm64 宿主机；隔离的 Ubuntu 24.04 Colima VM；Docker client/server 29.5.2；Node 24.18.0；npm 11.16.0。
真实 daemon 测试通过 PR 支持的 合并前必须修改为六个 resolver 步骤全部设置 |
jifeng
left a comment
jifeng
left a comment
There was a problem hiding this comment.
Requesting changes for one Critical integration defect: the protected digest output is produced but never consumed, so downstream gates remain steerable through the appendable environment value. The full local validation report is posted in the PR conversation.
中文说明
请求修改：存在一个 Critical 集成缺陷。固定 digest 的受保护输出虽被生成但从未被消费，下游 gate 仍可通过可追加的环境变量被重新指向。完整本地验证报告已发布在 PR 对话中。
| // so a consumer that must not be steered by branch code (the verification | ||
| // gate's container image) reads the expression-context value instead. | ||
| if (process.env.GITHUB_OUTPUT) { | ||
| appendStepFile(process.env.GITHUB_OUTPUT, `image=${image}\n`); |
There was a problem hiding this comment.
[Critical] Make the pinned output the value the gates actually consume
None of the six Resolve sandbox image steps has an id, and there is no steps.<id>.outputs.image consumer anywhere in this commit, so this protected output is not addressable downstream. The agents and direct Docker verification still inherit QWEN_SANDBOX_IMAGE from appendable GITHUB_ENV; in a process-level reproduction, appending a second mutable value after resolution replaced the effective image while this pinned output remained unused. Give every resolver step an ID and bind every agent/gate image input directly to its image output; otherwise the pull-to-digest binding does not prevent post-resolution steering.
中文说明
六个 Resolve sandbox image 步骤均未设置 id，当前提交中也没有任何 steps.<id>.outputs.image 消费者，因此这个受保护的输出无法被下游引用。Agent 和直接 Docker 验证仍从可追加的 GITHUB_ENV 继承 QWEN_SANDBOX_IMAGE。进程级复现表明，在解析完成后追加第二个可变标签会覆盖实际生效镜像，而固定 digest 的 output 完全未被使用。应为所有解析步骤设置 ID，并让所有 agent/gate 直接绑定其 image 输出，否则该修复无法阻止解析后的镜像劫持。
There was a problem hiding this comment.
Split by footprint. In scope — resolved in da1d9ad44b: both Resolve sandbox image steps in qwen-autofix.yml and qwen-autofix-recovery.yml now carry id: 'sandbox_image', and every security-sensitive consumer in those jobs (Assess candidates, Develop fix, Triage and address, Repair deterministic rejection) binds QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}' at step level, which outranks the appendable GITHUB_ENV value; the workflow contract test pins all of it. Out of scope — deferred and escalated: the identical defect in repo-hygiene.yml (two resolver steps without ids, three env-inheriting consumers) is real, but that file is outside this PR's footprint and the deterministic gate rejected the previous commit precisely because it touched it — review feedback alone cannot authorize changes to CI machinery the PR never touched. The repo-hygiene change has been reverted byte-for-byte (01452e0358) and the binding is recorded in the deferred review findings queue so it survives merge. Open question for the maintainer: should the same id + step-level binding land in repo-hygiene.yml as a maintainer-owned change (direct commit or a small separate PR)? The contract test only needs repo-hygiene.yml added back to its workflow list once that lands.
中文说明
按足迹拆分处理。范围内 —— 已在 da1d9ad44b 中解决： qwen-autofix.yml 与 qwen-autofix-recovery.yml 中的两个 Resolve sandbox image 步骤均已带 id: 'sandbox_image'，这些 job 中所有安全敏感的消费者（Assess candidates、Develop fix、Triage and address、Repair deterministic rejection）都在 step 级别绑定 QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}'，step 级 env 优先于可追加的 GITHUB_ENV 值；工作流契约测试已将上述全部接线固化。范围外 —— 延后并升级： repo-hygiene.yml 中的同类缺陷（两个 resolver 步骤没有 id、三个消费者继承环境变量）确实存在，但该文件在本 PR 足迹之外，确定性门禁正是因为上一提交触碰了它而拒绝——仅凭审查反馈不能授权修改 PR 从未触碰的 CI 机制。repo-hygiene 的改动已逐字节回退（01452e0358），该绑定已记入延迟审查发现队列，确保合并后不丢失。给维护者的开放问题：同样的 id + step 级绑定是否应以维护者主导的改动（直接提交或单独的小 PR）落入 repo-hygiene.yml？届时契约测试只需把 repo-hygiene.yml 加回其工作流列表即可。
|
@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 冲突，直到移除标签或达到轮次上限。移除 |
The deterministic gate rejected the previous commit because repo-hygiene.yml is CI machinery this PR never touched; review feedback alone cannot authorize changes there. Restore the file byte-for-byte and scope the workflow contract test to the two autofix workflows this PR binds. The repo-hygiene binding is real and is deferred to the review-findings follow-up queue for a maintainer-owned change.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #9527 (same-run verification repair)Commit: Same-run verification repair — deterministic rejection fixedThe previous commit was rejected because it expanded into CI machinery the PR itself never touched: Reproduced before fixing. Restoring Changes this round:
No base-conflict work was required ( Critical — pinned step output was dead (rv:4978687431, ic:5350731728, rc:3818302300)Split by footprint:
Suggestions from the automated review (rv:4978661441)All seven were implemented in
VerificationCommands actually run this round:
中文说明Autofix 审查轮次总结 — PR #9527（同轮验证修复）提交： 同轮验证修复 —— 确定性门禁拒绝已修复上一个提交被拒，原因是它扩展到了 PR 自身从未触碰的 CI 机制： 修复前已复现。 先把 本轮变更：
无需处理 base 冲突（ Critical —— 固定 digest 的 step output 是死输出（rv:4978687431、ic:5350731728、rc:3818302300）按足迹拆分处理：
自动审查的 Suggestion（rv:4978661441）七项均已在
验证本轮实际执行的命令：
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.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix-recovery.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.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/scripts/resolve-sandbox-image.test.mjs:68 — [review] GATE_IMAGE comment names a consumer that does not exist and inverts the empty-value failure modepackages/cli/src/utils/sandbox.ts:948 — [probe] CLI image presence check misses digest references after a tag pull (re-pull per first launch; FatalSandboxError on registry blip)
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix-recovery.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const child = spawn( | ||
| command, | ||
| ['image', 'inspect', '--format', '{{json .RepoDigests}}', image], | ||
| { stdio: ['ignore', 'pipe', 'inherit'], env: sandboxSpawnEnv() }, |
There was a problem hiding this comment.
[Suggestion] R1-2: Still stands — the spawn-guard skeleton (spawn + sandboxSpawnEnv() endpoint pin + settle-once flag + SIGKILL timer + stdout accumulation + error/close wiring) exists in two near-verbatim copies, pullImage and repoDigestOf, and they have already drifted in shape: pullImage tees stdout to the step log and returns {ok, digest} with an early-return close handler; repoDigestOf buffers silently, finishes with a ternary, and returns a string. This is round-1 finding R1-2, deferred to "the next normal round" — this is that round.
The cost is drift: a future fix to one kill/error path (timeout handling, a signal race, a stdout-buffering fix) does not automatically reach the other copy, leaving the pull and the inspect with different failure behaviour — one fails closed while the other hangs to the step timeout or exports on stale data. And a third docker invocation copied from either skeleton can silently drop env: sandboxSpawnEnv(), re-opening exactly the DOCKER_HOST/DOCKER_CONTEXT endpoint reroute this PR closes.
Suggested fix (spans both functions, so no one-click suggestion):
function spawnDockerCapture(command, args, { timeoutMs, label, onChunk }) {
// spawn with env: sandboxSpawnEnv(), settle-once guard, SIGKILL timer,
// stdout accumulation, error/close wiring — resolves { exitCode, stdout }
}Build pullImage (tee + parsePullDigest) and repoDigestOf (JSON post-processing) on it, so the endpoint pin is structural for both and for any future callers.
中文说明
R1-2 仍然存在 —— spawn 守护骨架（spawn + sandboxSpawnEnv() 端点固定 + settle-once 标志 + SIGKILL 定时器 + stdout 累积 + error/close 接线）在 pullImage 与 repoDigestOf 中以两份几乎逐字相同的副本存在，且形状已经漂移：pullImage 会把 stdout 同步到步骤日志并返回 {ok, digest}（close 回调提前返回），repoDigestOf 静默缓冲、以三元表达式收尾并返回字符串。这是第一轮的 R1-2，当时被延后到"下一个正常轮次"处理 —— 本轮即是。
代价是漂移：未来对某一条 kill/error 路径的修复（超时处理、信号竞态、stdout 缓冲修复）不会自动到达另一份副本，pull 与 inspect 将出现不同的失败行为 —— 一个 fail-closed，另一个挂到步骤超时或基于过期数据导出。而从任一份骨架复制出的第三个 docker 调用可能悄悄漏掉 env: sandboxSpawnEnv()，重新打开本 PR 正要关闭的 DOCKER_HOST/DOCKER_CONTEXT 端点重定向通道。
建议抽取统一的守护 spawn 辅助函数（如上方 spawnDockerCapture，内部应用端点固定并返回 { exitCode, stdout }），让 pullImage 与 repoDigestOf 都构建于其上，使端点固定对现有两处及未来所有调用方都是结构性的。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| (typeof step.env?.SETTINGS_JSON === 'string' && | ||
| step.env.SETTINGS_JSON.includes('"sandbox": "docker"')), | ||
| ); | ||
| if (consumers.length === 0) continue; |
There was a problem hiding this comment.
[Suggestion] The contract test's consumer detection has no floor. consumers is computed by literal-substring matching — the agent steps' run: bodies never mention QWEN_SANDBOX_IMAGE, so detection rides entirely on the exact-spaced SETTINGS_JSON literal "sandbox": "docker". If the predicate stops matching (a whitespace reformat to "sandbox":"docker", or a new consumer step that receives the image by another mechanism), consumers.length === 0 silently continues for every job and the binding requirement evaporates while the test stays green. The missed consumers keep inheriting QWEN_SANDBOX_IMAGE from the appendable $GITHUB_ENV channel (which exportImage still writes) — exactly the steering this test exists to prevent — and the regression never surfaces in CI.
Suggested fix — count per workflow and fail on zero:
for (const name of SANDBOX_WORKFLOWS) {
const doc = parse(readFileSync(join(workflowsDir, name), 'utf8'));
let totalConsumers = 0;
for (const [jobName, job] of Object.entries(doc.jobs ?? {})) {
// ... existing per-job checks ...
if (consumers.length === 0) continue;
totalConsumers += consumers.length;
// ...
}
assert.ok(
totalConsumers > 0,
`${name}: no sandbox consumers detected — the contract test would pass vacuously`,
);
}Detecting consumers structurally instead of by substring (JSON.parse(step.env.SETTINGS_JSON) and check tools.sandbox, or treating any step whose run invokes run-agent.mjs as a consumer) would also remove the whitespace dependency.
中文说明
契约测试的消费者检测没有下限。consumers 通过字面子串匹配计算 —— agent 步骤的 run: 正文从不出现 QWEN_SANDBOX_IMAGE，因此检测完全依赖空格精确的 SETTINGS_JSON 字面量 "sandbox": "docker"。一旦谓词不再匹配（空格被重排为 "sandbox":"docker"，或新增以其他机制接收镜像的消费者步骤），consumers.length === 0 会对每个 job 静默 continue，绑定要求在测试保持全绿的情况下凭空蒸发。被漏掉的消费者会继续从可追加的 $GITHUB_ENV 通道（exportImage 仍在写入）继承 QWEN_SANDBOX_IMAGE —— 正是本测试要防止的劫持 —— 且回归永远不会在 CI 中暴露。
建议按工作流计数并在为零时报错（上方代码），或改为结构化检测消费者（JSON.parse(step.env.SETTINGS_JSON) 后检查 tools.sandbox，或把 run 调用 run-agent.mjs 的步骤都视为消费者），以消除对空格的依赖。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| `'${command} pull ${image}' reported no Digest line; refusing to export an unbound image reference.`, | ||
| ); | ||
| } | ||
| exportImage(await repoDigestOf(command, image, pull.digest)); |
There was a problem hiding this comment.
[Suggestion] The digest-bound SUCCESS export has no end-to-end test. exportDigestBoundImage is not exported, and the only main() e2e test pins the refusal path (pull exits 0 with no Digest: line), so this line and the fallback branch's wiring are invisible to the suite. Mutation probe at the reviewed commit: replacing this line with exportImage(image) — exporting the mutable tag, the exact vulnerability class this PR exists to close — leaves the whole suite green (baseline 29/29 pass, mutant 29/29 pass). The same holds for wiring mutants in main()'s fallback branch. A future regression that ships the tag instead of repo@sha256:… would pass CI.
Suggested fix — add the success-path companion to the existing refusal e2e test: stub pull to print Digest: <GENUINE> and image inspect to print ["ghcr.io/qwenlm/qwen-code@<GENUINE>"], drive the script via execFileSync with SANDBOX_COMMAND pointing at the stub, and assert both step files contain ghcr.io/qwenlm/qwen-code@${GENUINE} and NOT the :1.2.3 tag.
中文说明
digest 绑定的成功导出路径没有端到端测试。exportDigestBoundImage 未导出，main() 唯一的 e2e 测试只固定了拒绝路径（pull 退出码 0 但不打印 Digest: 行），因此这一行与回退分支的接线对测试套件不可见。在受审提交上做变异探测：把这一行替换为 exportImage(image) —— 即导出可变 tag，正是本 PR 要关闭的漏洞类别 —— 整个套件仍然全绿（基线 29/29 通过，变异 29/29 通过）。main() 回退分支的接线变异同样存活。未来若回归为导出 tag 而非 repo@sha256:…，CI 会放行。
建议为现有拒绝 e2e 测试补充成功路径的姊妹测试：stub 的 pull 打印 Digest: <GENUINE>、image inspect 打印 ["ghcr.io/qwenlm/qwen-code@<GENUINE>"]，用 execFileSync 把 SANDBOX_COMMAND 指向 stub 驱动脚本，断言两个 step 文件都包含 ghcr.io/qwenlm/qwen-code@${GENUINE} 且不包含 :1.2.3 tag。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| ${{ always() && steps.verify.outputs.retryable == 'true' }} | ||
| timeout-minutes: 20 | ||
| env: | ||
| QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}' |
There was a problem hiding this comment.
[Suggestion] The daemon-endpoint pin stops at the resolver. sandboxSpawnEnv() is applied only inside the resolver's two spawns; the CLI sandbox driver that the bound agent steps launch spawns docker with the inherited environment untouched (packages/cli/src/utils/sandbox.ts), and this step's env: block overrides neither DOCKER_HOST nor DOCKER_CONTEXT. The diff's own rationale names the channel ("sets DOCKER_HOST through $GITHUB_ENV") and closes it only for the resolver.
In review-address, the Verification gate runs the branch's own build/test on the host as the runner uid after the resolver and before this consumer; that branch code can append DOCKER_HOST to $GITHUB_ENV. This step's docker run <repo>@sha256:… then resolves the digest-pinned reference against an attacker-controlled daemon that serves arbitrary content for that digest — the binding holds at export time and is severed at execution, and the agent runs attacker content with the forwarded model key. The window covers this repair consumer in the same run (the address agent runs before the gate; $GITHUB_ENV does not cross runs). Same shape in qwen-autofix-recovery.yml.
| QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}' | |
| QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}' | |
| DOCKER_HOST: '' | |
| DOCKER_CONTEXT: 'default' |
Step-level env outranks the appendable $GITHUB_ENV, and an empty DOCKER_HOST is skipped by the docker CLI (remediation untested in this review). Alternatively, pin in the CLI sandbox driver's docker spawns, which would also cover the deferred repo-hygiene.yml consumer.
中文说明
daemon 端点固定在 resolver 处就停止了。sandboxSpawnEnv() 只应用于 resolver 自身的两处 spawn；被绑定的 agent 步骤所启动的 CLI sandbox 驱动在 spawn docker 时原样继承环境（packages/cli/src/utils/sandbox.ts），且本步骤的 env: 块既未覆盖 DOCKER_HOST 也未覆盖 DOCKER_CONTEXT。diff 自身的理由注释点名了该通道（"sets DOCKER_HOST through $GITHUB_ENV"），却只在 resolver 处关闭了它。
在 review-address 中，Verification gate 在 resolver 之后、本消费者之前，以 runner uid 在宿主机上执行分支自身的 build/test；这些分支代码可以向 $GITHUB_ENV 追加 DOCKER_HOST。随后本步骤的 docker run <repo>@sha256:… 会在攻击者控制的 daemon 上解析 digest 固定的引用，而该 daemon 可以为该 digest 返回任意内容 —— 绑定在导出时成立、在执行时被切断，agent 将携带转发的模型密钥运行攻击者内容。窗口覆盖同一次运行中的本 repair 消费者（address agent 在 gate 之前运行；$GITHUB_ENV 不跨运行）。qwen-autofix-recovery.yml 中形状相同。
上方 suggestion 在 step 级 env 中补充 DOCKER_HOST: '' 与 DOCKER_CONTEXT: 'default'（step 级 env 优先于可追加的 $GITHUB_ENV，空 DOCKER_HOST 会被 docker CLI 跳过；该修复在本评审中未经测试）。或者在 CLI sandbox 驱动的 docker spawn 处固定端点，可同时覆盖延后处理的 repo-hygiene.yml 消费者。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| ${{ always() && steps.verify.outputs.retryable == 'true' }} | ||
| timeout-minutes: 20 | ||
| env: | ||
| QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}' |
There was a problem hiding this comment.
[Suggestion] This binding evaluates to the empty string whenever the resolver step fails, and this repair step is the one consumer reachable with an empty value: the Verification gate is always()-gated and can still emit retryable=true (in any round where the branch already exists at origin, its git checkout DWIM-succeeds without the prepare step). An empty QWEN_SANDBOX_IMAGE is not nullish, so loadSandboxConfig's ?? chain stops at '' and the CLI runs the agent with no sandbox at all — where pre-PR the unset variable fell through to the package.json default image and stayed containerized. Today the unsandboxed run is prevented only by an incidental fail-stop: the step body's cat "${WORKDIR}/feedback.md" dies under bash -eo pipefail before the run-agent.mjs call, because the skipped prepare step never wrote the file. Probe evidence: loadSandboxConfig with QWEN_SANDBOX_IMAGE='' → undefined (no sandbox), unset → { command: 'docker', image: <package default> }; bash -eo pipefail -c '{ cat <missing>; echo NEVER_REACHED; } > f' → rc=1, nothing after the cat runs. Same shape in qwen-autofix.yml.
Make the safety deliberate instead of incidental — either gate the step on the resolver's success:
if: |-
${{ always() && steps.verify.outputs.retryable == 'true' && steps.sandbox_image.outcome == 'success' }}or refuse at the top of its run: block when QWEN_SANDBOX_IMAGE is empty. A future edit that tolerates the cat (e.g. || true) or reorders the body would otherwise silently re-open unsandboxed execution of branch code on the persistent pool host.
中文说明
当 resolver 步骤失败时，该绑定求值为空字符串，而本 repair 步骤正是唯一可能在空值下运行的消费者：Verification gate 带 always() 门控，且仍可能输出 retryable=true（在分支已存在于 origin 的轮次中，其 git checkout 无需 prepare 步骤即可 DWIM 成功）。空 QWEN_SANDBOX_IMAGE 不是 nullish 值，loadSandboxConfig 的 ?? 链会停在 ''，CLI 将完全不带 sandbox 运行 agent —— 而 PR 之前该变量未设置时会落到 package.json 默认镜像、保持容器化。如今未沙箱化的运行只被一个偶然的 fail-stop 阻止：步骤正文的 cat "${WORKDIR}/feedback.md" 在 bash -eo pipefail 下先于 run-agent.mjs 调用而失败，因为被跳过的 prepare 步骤从未写过该文件。探测证据：QWEN_SANDBOX_IMAGE='' 时 loadSandboxConfig → undefined（无 sandbox），未设置时 → { command: 'docker', image: <包默认镜像> }；bash -eo pipefail -c '{ cat <missing>; echo NEVER_REACHED; } > f' → 退出码 1，cat 之后的内容均未执行。qwen-autofix.yml 中形状相同。
请把该安全性变为显式设计而非偶然结果 —— 要么让步骤以 resolver 成功为门控（上方 YAML），要么在 run: 块开头于 QWEN_SANDBOX_IMAGE 为空时拒绝执行。否则未来任何容忍该 cat 的编辑（如 || true）或正文重排都会悄悄重新打开在常驻池宿主机上无沙箱执行分支代码的通道。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-autofix-recovery.yml was deleted on main (#9517, the oversized-file incident fix): accept the deletion — the clone carried the same jobs as qwen-autofix.yml, where this branch's bindings already landed — and drop it from the sandbox-image contract test's workflow list.
|
@qwen-code /resolve |
- R1-2: extract the duplicated spawn guard (endpoint pin, settle-once finish, SIGKILL timer, stdout capture, error/close wiring) into one spawnDockerCapture helper; pullImage and repoDigestOf share it. - R2-1: contract test fails when a workflow detects zero sandbox consumers instead of passing vacuously. - R2-2: success-path e2e test for the digest-bound export; verified it kills the exportImage(image) mutant. - R2-3: pin the daemon endpoint (DOCKER_HOST: '', DOCKER_CONTEXT: default) on every sandbox-consuming step, closing the $GITHUB_ENV and pool-shared currentContext channels past the resolver; contract test enforces the pin. - R2-4: gate the repair step on the resolver outcome so a failed resolver can never relaunch the agent unsandboxed. Also updates the workflow source pin in scripts/tests to the shared helper's literals (required by the R1-2 refactor).
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9527Conflict resolution (base merge)Merged Feedback dispositions
Notes
Verification
中文说明Autofix 审查轮次 — PR #9527冲突解决（基线合并）已合并 反馈处理
说明
验证
🧭 Gate advisory — this round modified areas outside the PR footprint (machine-measured, not agent-authored):
Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 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 did not run conflict resolution for this request. PR #9527 does not currently have merge conflicts with main. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 509 passed · 0 failed · 509 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：509 通过 · 0 失败 · 509 总计 抖动门： Verification reportPR #9527 Deep Verification —
|
| # | Cell | Oracle | HEAD | BASE |
|---|---|---|---|---|
| C1 | happy path; Digest: line split across two writes |
GITHUB_ENV / GITHUB_OUTPUT contents |
QWEN_SANDBOX_IMAGE=<repo>@sha256:… and image=<repo>@sha256:… |
mutable tag in env; no output channel at all |
| C2 | same-content retag: RepoDigests[0] = a/a@<same digest> |
exported reference | pulled-repo entry (exact match, not index 0) | mutable tag (oblivious) |
| C3 | foreign content: RepoDigests = ["aaa.example/backdoor@<other>"] |
exit code + files | exit ≠ 0, none of which is, both files untouched |
exports mutable tag — attack succeeds |
| C4 | pull exits 0 without a Digest: line |
exit code + files | exit ≠ 0, reported no Digest line, both files untouched |
exports mutable tag — attack succeeds |
| C5 | planted FIFO (no reader) at $GITHUB_ENV |
elapsed + exit | ENXIO refusal in 179 ms, exit 1 | blocks until SIGKILL at 8.0 s (measured 8002 ms isolated) |
| C5b | planted FIFO at $GITHUB_OUTPUT |
elapsed + exit | immediate refusal; FIFO untouched | n/a — base never wrote this channel |
| C6 | hostile parent env: DOCKER_HOST=tcp://attacker…, DOCKER_CONTEXT=rogue |
shim-recorded env per spawn | 2 spawns (pull + inspect), both HOST=[unset] CTX=[default] |
pull saw HOST=[tcp://attacker.invalid:2375] CTX=[rogue] — env passed through unchanged |
30/30 scripted assertions across these cells; every base cell demonstrates the hole the PR closes, every head cell the closure. Suite-level witness of the same behaviors: 01-resolver-suite-head.png (30/30 TAP).
Corrections
- Description test counts: the body says
node --test … 24 pass, 0.5s (was 16). Measured at this head: 30 tests (# tests 30 / # pass 30, 0.9 s on this runner); the base of this PR (HEAD^1blob) has 4 tests, not 16 — the "16" appears to describe the frozen feat(autofix): run the verification gate in an ephemeral container #9214 lineage the change was salvaged from. No code impact; correcting the record for the next reader.
Findings (non-blocking)
1. Suggestion — the R2-4 repair-outcome gate is pinned by no test (mutation survivor M11)
Deleting && steps.sandbox_image.outcome == 'success' from the repair step's if: leaves both suites fully green (matrix mutant M11: 30/30; the vitest contract suite pins the repair step only by the substring steps.verify.outputs.retryable == 'true', which survives the deletion, and contains zero sandbox_image references). The clause itself is present and correct at head (verified via diff + actionlint expression validation), so this is a completeness gap, not a defect: everything else R2-1..R2-3 added is pinned (M9/M10/M12 killed), but a future edit could silently drop the fail-closed gate on the repair pass.
Measured fix (append to .github/scripts/resolve-sandbox-image.test.mjs; scratch-verified, preserves intent):
suggested fix + measurement
test('the repair step is gated on the resolver outcome (fail closed)', () => {
const workflowsDir = join(
dirname(fileURLToPath(import.meta.url)),
'..',
'workflows',
);
for (const name of SANDBOX_WORKFLOWS) {
const doc = parse(readFileSync(join(workflowsDir, name), 'utf8'));
for (const [jobName, job] of Object.entries(doc.jobs ?? {})) {
const repair = (job.steps ?? []).find((step) => step.id === 'repair');
if (!repair) continue;
assert.ok(
String(repair.if ?? '').includes(
"steps.sandbox_image.outcome == 'success'",
),
`${name} job '${jobName}': repair must not run when the resolver failed`,
);
}
}
});Measurement (reverse mutation run): with the test, head suite is 31/31 green; applying the M11 deletion turns exactly one test red (not ok 31 - the repair step is gated on the resolver outcome (fail closed), 30/31) — the unpinned axis is now pinned. Logs: logs/m11-fix-green.log, logs/m11-fix-vs-mutant.log.
2. Informational (disclosed, deferred) — repo-hygiene.yml keeps the unbound consumer shape
Confirmed by measurement, matching the author's disclosure (commit 01452e03 and the test-file comment): repo-hygiene.yml runs the same resolver twice with no step id: (lines ~146, ~349), and its consumers run docker run "${SANDBOX_ARGS[@]}" "${QWEN_SANDBOX_IMAGE}" (lines 699, 782–798) reading the appendable $GITHUB_ENV value, with no endpoint pin on those spawns. Residual shape after this PR: the exported value is at least digest-bound there too (the resolver change benefits every caller), so the resolve→consume retag channel is closed; the env-append steering and DOCKER_HOST-via-$GITHUB_ENV channels remain open in that workflow only. Tracked per the author's note in the deferred review-findings queue for a maintainer-owned change — no action for this PR.
3. Observation — C5b partial write is contained
When $GITHUB_OUTPUT is a FIFO, exportImage writes the GITHUB_ENV line before the output-file refusal throws (observed: env file carried the digest-bound line while the step exited non-zero). Contained: the step's non-zero outcome fails the workflow step, every consumer binds the step output (empty), and the repair pass is outcome-gated — fail-closed overall. No action needed; noting because the write ordering is the only place a successful-looking env value can outlive a failed resolve.
Mutation matrix (vacuity proof)
Baseline unmutated head: 30/30 green (or the kills would mean nothing). Witness: 03-mutation-matrix.png; full per-mutant failing-test lists in logs/mutation-run.log and logs/mutation.tally.json.
| Mutant | Change | Expected | Observed |
|---|---|---|---|
| M1 | sandboxSpawnEnv returns parent env (author claim) |
3 endpoint tests fail | KILLED — exactly those 3 |
| M2 | stdout = chunk (author claim) |
cross-chunk test fails | KILLED — 12 tests (overwrite assigns a raw Buffer → stdout.trim is not a function on every spawn path) |
| M2b | finer: stdout = String(chunk) |
only cross-chunk test can catch it | KILLED — exactly 1, clean attribution |
| M3 | appendStepFile → plain appendFileSync (author claim) |
FIFO tests fail | KILLED — exactly the 2 FIFO tests |
| M4 | inspect timeout back to fixed constant (author claim) | timeout test fails ~30 s | KILLED — exactly that test, ~30 s |
| M5 | main() exports the requested tag (pre-PR behavior; R2-2) | success-path e2e fails | KILLED — both main() e2e tests (same call site) |
| M6 | exact repo@digest match → suffix-only |
retag test fails | KILLED — exactly that test |
| M7 | drop docker.io/library fold |
Hub short-form test fails | KILLED — exactly that test |
| M8 | drop the GITHUB_OUTPUT write |
output tests fail | KILLED — exactly the 2 |
| M9 | workflow: remove one DOCKER_HOST: '' pin (R2-3) |
contract test fails | KILLED |
| M10 | workflow: bind image from ${{ env.… }} not step output (R2-3) |
contract test fails | KILLED |
| M11 | workflow: drop repair outcome gate (R2-4) | does anything notice? | SURVIVED — Finding #1 |
| M12 | workflow: zero detectable consumers (R2-1) | must fail, not pass vacuously | KILLED — no sandbox consumers detected |
All four author-claimed mutation/test pairs independently confirmed; no mutant regressed in the wrong direction, no off-target kills beyond the explained same-call-site sets (M2, M5).
Gates
| Gate | Result |
|---|---|
node --test .github/scripts/resolve-sandbox-image.test.mjs (HEAD) |
30/30 pass, 0.9 s (01-resolver-suite-head.png) |
| Same suite against BASE workflow (liveness cell) | exactly 1 expected red: the resolver step needs an id… (04-contract-test-vs-base-workflow.png) — proves the contract test is live for this PR's workflow changes |
vitest … scripts/tests/qwen-autofix-workflow.test.js (HEAD) |
192/192 pass |
| Same suite on base worktree (A/A control) | 192/192 pass — identical, including one [vitest-worker]: Timeout calling "onTaskUpdate" RPC error with exit 1 on both arms: pre-existing/environmental (68 s, 192-test suite on the shared loaded runner), not introduced by this PR |
| actionlint 1.7.12 (SHA-verified, repo wrapper ignore flags) | 0 findings on head and base workflow; liveness probe caught both planted violations (invalid step id → unresolved steps.sandbox_image reference) |
| ESLint on the 3 changed files | clean; liveness probe (planted unused var) caught |
Type-boundary probes (parsePullDigest / repoOfImage) |
10/10: CRLF, trailing space, uppercase hex rejected, 63/65-hex rejected, ^ anchor holds, first-of-two wins, tag+digest strip, registry port kept (logs/boundary-probes.log) |
Not covered
- Per-commit attribution: depth-2 checkout —
git rev-list HEAD^1..HEAD^2returns 1 commit while the metadata lists 5; intermediate commits (92c9a4e2,da1d9ad4,01452e03,65a541c8) are unreachable. Verified the aggregateHEAD^1..HEADdiff. - Real docker daemon/CLI: no docker binary in this container. The shim reproduces the wire shape (pull stdout format, inspect JSON, env plumbing), not real CLI behavior. In particular the workflow-level pin relies on "the docker CLI skips an empty
DOCKER_HOST" (the PR's own comment) — not exercisable here; the resolver-side pin uses the strongerdelete. The retag/foreign scenarios encode the PR's stated live probes (docker 29.1.3 / 24.0.9), not a fresh live-daemon reproduction. - GHCR fallback path (
fetchLatestGhcrSemver): cells keep the requested pull successful; no live GHCR probe (behavior unchanged by this PR apart from the shared spawn guard, which M1/M2/M4 pin). - yamllint (
pip3permission-denied in-container) and repo-wide shellcheck (norun:blocks are changed by this PR — onlyid:,env:,if:); actionlint + the contract test's YAML parse stand in. - Repo-wide gates (full workspace test suites, tsc, prettier): the PR touches no TypeScript; targeted suites + ESLint on changed files only.
- The vitest RPC error was not chased beyond the A/A attribution above.
Methodology
Environment: the CI verify container (node:22-bookworm class, Node v22.23.2, no docker daemon, $RUNNER_TEMP=/__w/_temp), working tree at the merge ref eced5186bd. The harnesses drive the real .mjs sources (no stubs of code under test): the A/B harness (harness/ab-harness.mjs) spawns the resolver as a child process with SANDBOX_COMMAND pointed at a per-cell fake-docker shim and observes exit codes, stderr, step-file bytes, elapsed time, and shim-recorded endpoint env; the base arm is the git show HEAD^1: blob of the script (node-builtin-only module, no workspace links — the realpath control does not apply). The mutation matrix (harness/mutation-matrix.mjs) materializes per-mutant .github trees and runs the unmodified head suite against each. The base vitest A/A ran in a git worktree of HEAD^1 with the root node_modules symlinked (lockfile untouched by the PR; worktree removed afterwards). harness/final-audit.mjs re-asserts every log-derived outcome (contract-base cell, actionlint, ESLint, M11-fix measurement, vitest both arms) as scripted comparisons — 13/13. Raw logs: logs/ (A/B run + per-cell progress, mutation run + tally, vitest head/rerun/base, actionlint head/base, contract-vs-base-workflow, boundary probes, ESLint, M11-fix measurement, final audit); harness scripts in harness/; per-mutant trees retained under scratch/mut/.
Flakiness gate log
rounds=5 files=2 skipped=0
file .github/scripts/resolve-sandbox-image.test.mjs: (cd .) node --test ./.github/scripts/resolve-sandbox-image.test.mjs
file scripts/tests/qwen-autofix-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-autofix-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
.github/scripts/resolve-sandbox-image.test.mjs: PPPPP
scripts/tests/qwen-autofix-workflow.test.js: FFFFF
verdict: consistent-fail
summary: 1 of 2 changed test file(s) failed identically in every round — deterministic, so CI owns that signal
--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/resolve-sandbox-image.test.mjs: P (exit 0)
round 1 · scripts/tests/qwen-autofix-workflow.test.js: F (exit 1)
--- output tail · round 1 · scripts/tests/qwen-autofix-workflow.test.js ---
ory.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
�[32m✓�[39m scripts/tests/qwen-autofix-workflow.test.js �[2m(�[22m�[2m192 tests�[22m�[2m)�[22m�[33m 71350�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mholds a round while review-pr is in flight on the head (#8888) �[33m 448�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mauto-updates a PR red only from a stale base, gated on green-on-main �[33m 609�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mauto-reruns a check that died on infrastructure, once, guarded by run_attempt �[33m 651�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the stale-duplicate revalidation, including the conflict-only transition �[33m 8121�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the eligibility recheck across lifecycle and label states �[33m 3425�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mreleases the dispatch-pending marker when the recheck discards a target �[33m 1100�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mraises the round cap to TAKEOVER_MAX_ROUNDS while the label is present �[33m 454�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the takeover-command toggle across all four paths �[33m 3805�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally resets round counting at the latest takeover engage ack �[33m 856�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally seeds the round counter from the window anchor and only from it �[33m 1701�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mrecovers transient forced-target reads and reports terminal takeover blocks �[33m 725�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mwires forced admission end to end: reader, classifier, permission gate, reporter �[33m 440�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mposts the non-main base refusal without depending on any other API call �[33m 2020�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mswitches to Critical-only feedback after five change rounds �[33m 406�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mescalates to a maintainer-decision handoff when the diff keeps growing past budget (non-convergence) �[33m 829�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mposts a takeover milestone digest as rounds accumulate, with a residual bucket �[33m 656�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mrejects a round that expands into CI machinery outside the PR footprint �[33m 1201�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22msurfaces deny-by-default footprint expansions, rejecting only when enforcement says so �[33m 419�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mupserts deferred findings into a per-PR issue that survives the merge �[33m 8498�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbite check: rejects a round whose changed tests pass on the pre-round tree �[33m 2872�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mstops a PR that fails to push for CONSECUTIVE_FAILURE_CAP rounds in a row �[33m 580�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mre-arms a stranded PR from a marker instead of a deleted comment �[33m 570�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22maddress-side stale check mirrors the scan-side re-arm logic under bash �[33m 867�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally posts the re-arm marker only after verifying the PAT identity �[33m 375�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mresolves only the review threads whose findings it implemented �[33m 2153�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mdoes not flag an API error that appears after a real verdict or a loop guard �[33m 301�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mflags recoverable API renders without a leading status code, and skips non-recoverable ones �[33m 370�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mclassifies permanent API failures terminal and records the cause class �[33m 1130�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mpreserves an agent-written handoff when the budget kills qwen after it �[33m 652�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m review verification gate: baseline A/B on deterministic rejection�[2m > �[22mclassifies an unchanged branch by its verdict files (handoff contract) �[33m 587�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mkills a silent agent at the idle window, naming the idle limit �[33m 1247�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mnever fires while the agent emits protocol events, however slowly �[33m 3256�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mnever fires while the agent talks on stderr only �[33m 3256�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mdoes not treat an unterminated stdout byte stream as progress �[33m 799�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mrequests streamed partial progress so active headless work refreshes the watchdog �[33m 3256�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m stale sandbox container cleanup�[2m > �[22man idle kill removes only the running sandbox its own agent launched �[33m 1548�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m stale sandbox container cleanup�[2m > �[22ma budget kill removes only the running sandbox its own agent launched �[33m 1248�[2mms�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Unhandled Errors �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯�[39m
�[31m�[1m
Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Unhandled Error �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[31m�[1mError�[22m: [vitest-worker]: Timeout calling "onTaskUpdate"�[39m
�[90m �[2m❯�[22m Object.onTimeoutError node_modules/vitest/dist/chunks/rpc.-pEldfrD.js:�[2m53:10�[22m�[39m
�[90m �[2m❯�[22m Timeout._onTimeout node_modules/vitest/dist/chunks/index.B521nVV-.js:�[2m59:62�[22m�[39m
�[90m �[2m❯�[22m listOnTimeout node:internal/timers:�[2m585:17�[22m�[39m
�[90m �[2m❯�[22m processTimers node:internal/timers:�[2m521:7�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯�[39m
�[2m Test Files �[22m �[1m�[32m1 passed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[1m�[32m192 passed�[39m�[22m�[90m (192)�[39m
�[2m Errors �[22m �[1m�[31m1 error�[39m�[22m
�[2m Start at �[22m 13:01:36
�[2m Duration �[22m 72.01s�[2m (transform 236ms, setup 16ms, collect 318ms, tests 71.35s, environment 0ms, prepare 93ms)�[22m
round 2 · .github/scripts/resolve-sandbox-image.test.mjs: P (exit 0)
round 2 · scripts/tests/qwen-autofix-workflow.test.js: F (exit 1)
--- output tail · round 2 · scripts/tests/qwen-autofix-workflow.test.js ---
ory.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
�[32m✓�[39m scripts/tests/qwen-autofix-workflow.test.js �[2m(�[22m�[2m192 tests�[
...truncated -- full content in the run artifacts.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. ✅ The maintainer's Critical (protected output never consumed) is closed: both resolver steps are id'd, all four sandbox consumers bind the step output at step-env level with endpoint pins, and a non-vacuous contract test pins the shape. Full reasoning in the stage comments above.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
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.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
.github/scripts/resolve-sandbox-image.mjs:273 (+2 locations) — [review] GATE_IMAGE comments name a consumer that does not exist and invert the empty-value failure mode.github/scripts/resolve-sandbox-image.mjs:318 — [probe] fallback path's digest-bound export has no test (mutation survives).github/scripts/resolve-sandbox-image.mjs:170 — [probe] spawnDockerCapture's start-failure branch has no behavioural test.github/scripts/resolve-sandbox-image.test.mjs:644 — [probe] contract test never asserts the resolver precedes its consumers.github/scripts/resolve-sandbox-image.test.mjs:635 — [probe] consumer detection rides on a verbatim substring match (R2-1 residual).github/scripts/resolve-sandbox-image.mjs:105 — [probe] appendStepFile omits O_NOFOLLOW; the comment justifying it is factually wrong
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| assert.equal( | ||
| step.env?.DOCKER_CONTEXT, | ||
| 'default', |
There was a problem hiding this comment.
[Suggestion] The new workflow contract test asserts the QWEN_SANDBOX_IMAGE output binding and the DOCKER_HOST/DOCKER_CONTEXT pins for every consumer, but it never reads if: conditions — so the fail-closed steps.sandbox_image.outcome == 'success' guard this same PR adds to the always()-gated 'Repair deterministic rejection' consumer (qwen-autofix.yml:5003) is pinned by no test. A future edit that drops the guard — or adds a new always()-gated consumer with the binding but without the outcome check — ships green through both suites: a FAILED resolver with steps.verify.outputs.retryable == 'true' then runs the repair round with an empty QWEN_SANDBOX_IMAGE binding, and per loadSandboxConfig's ?? semantics an empty image relaunches the CLI without any sandbox on the self-hosted pool. Witness — guard removed from qwen-autofix.yml:5003, both suites still green:
node --test .github/scripts/resolve-sandbox-image.test.mjs → # pass 30 / # fail 0
npx vitest run scripts/tests/qwen-autofix-workflow.test.js → Tests 192 passed (192)
In the consumer loop, additionally assert that any consumer whose if contains always() also contains steps.${resolver.id}.outcome == 'success' for a resolver in the same job — the test already parses each step, so step.if is inspectable.
中文说明
新的工作流契约测试断言了每个消费者的 QWEN_SANDBOX_IMAGE 输出绑定与 DOCKER_HOST/DOCKER_CONTEXT 固定，但从不读取 if: 条件——因此本 PR 为 always() 门控的 'Repair deterministic rejection' 消费者新增的 fail-closed 守卫 steps.sandbox_image.outcome == 'success'（qwen-autofix.yml:5003）没有任何测试固定。未来若删除该守卫——或新增一个带绑定但无 outcome 检查的 always() 消费者——两个测试套件仍会全绿放行：resolver 失败且 steps.verify.outputs.retryable == 'true' 时，repair 轮次会以空的 QWEN_SANDBOX_IMAGE 绑定运行，而按 loadSandboxConfig 的 ?? 语义，空镜像会让 CLI 在自托管池上完全不带沙箱重新启动。见证：从 qwen-autofix.yml:5003 移除守卫后两个套件仍全绿（见上方英文部分的命令输出）。
建议在消费者循环中补充断言：任何 if 含 always() 的消费者必须同时包含 steps.${resolver.id}.outcome == 'success'（resolver 须在同一 job 内）——测试已解析每个 step，step.if 可直接检查。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| 'if [ "$1" = "pull" ]; then', | ||
| ` printf '%s\\n' 'Status: Downloaded newer image' 'Digest: ${GENUINE}'`, |
There was a problem hiding this comment.
[Suggestion] This success-path e2e test hand-rolls the docker-stub scaffolding — mkdtempSync + writeFileSync(join(dir, 'docker-stub'), ..., { mode: 0o755 }) + rmSync in finally — exactly as the refusal-path e2e test above it does, and exactly as withDockerStub() (line 102, added by this PR) already does with the identical stub filename and mode. The helper exists because of the unlink race its comment documents (return await fn(stub) keeps the stub alive until the async body settles); today both e2e tests dodge that race only by being synchronous, so a future edit that makes either of them async silently reintroduces the exact race the helper was written to fix, and any stub-protocol change (name, mode bits, cleanup ordering) must be made in three places. Route both tests through withDockerStub(scriptBody, (stub) => { ... }) — the env-file paths and assertions stay as-is.
中文说明
这条成功路径的端到端测试手工搭建了 docker-stub 脚手架——mkdtempSync + writeFileSync(join(dir, 'docker-stub'), ..., { mode: 0o755 }) + finally 中 rmSync——与上方拒绝路径的 e2e 测试完全相同，也与本 PR 新增的 withDockerStub()（第 102 行）完全相同（stub 文件名与权限位都一致）。该辅助函数存在的理由正写在其注释里：unlink 竞态（return await fn(stub) 让 stub 存活到异步主体结束）。目前两个 e2e 测试仅因恰好是同步的才避开该竞态；未来任何把它改成异步的编辑都会悄悄重新引入辅助函数专门修复的这个竞态，且 stub 协议的任何改动（文件名、权限位、清理顺序）都必须在三处同步修改。建议把两个测试都改为经由 withDockerStub(scriptBody, (stub) => { ... }) 驱动——env 文件路径与断言保持不变。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // repo-hygiene.yml runs the same resolver/consumer shape and needs the same | ||
| // binding, but it is outside this PR's footprint and the gate rejects the | ||
| // change here; it is tracked in the deferred review findings queue (#9527). | ||
| const SANDBOX_WORKFLOWS = ['qwen-autofix.yml']; |
There was a problem hiding this comment.
[Suggestion] This list hand-enumerates the workflows the binding contract protects, while the set it exists to cover — every workflow that runs resolve-sandbox-image.mjs — is discoverable from the tree: grep -rln "resolve-sandbox-image.mjs" .github/workflows/ finds 2 resolver workflows (qwen-autofix.yml, repo-hygiene.yml); this list names 1. When the queued follow-up (named in the comment above) lands repo-hygiene.yml's binding, or a new workflow adds a 'Resolve sandbox image' step, without anyone remembering to extend the list the contract test still passes while enforcing nothing about that workflow; a later refactor can then drop the outputs.image binding or the DOCKER_HOST/DOCKER_CONTEXT pins there with no test going red, and the exact steering channels this PR closes silently reopen. Derive the set instead: scan .github/workflows/*.yml for steps whose run: invokes resolve-sandbox-image.mjs and run the existing assertions on each, keeping an explicit, comment-linked exception set (today: repo-hygiene.yml, referencing the deferred queue entry) that shrinks as bindings land.
中文说明
该列表手工枚举了绑定契约所保护的工作流，而它本应覆盖的集合——所有运行 resolve-sandbox-image.mjs 的工作流——完全可以从代码树中发现：grep -rln "resolve-sandbox-image.mjs" .github/workflows/ 能找到 2 个含 resolver 的工作流（qwen-autofix.yml、repo-hygiene.yml），而列表只写了 1 个。当上方注释提到的后续改动落地 repo-hygiene.yml 的绑定、或新工作流新增 'Resolve sandbox image' 步骤时，只要有人忘了扩展列表，契约测试就会在该工作流上什么都不强制却仍然全绿；之后的重构即便删掉那里的 outputs.image 绑定或 DOCKER_HOST/DOCKER_CONTEXT 固定也不会有任何测试变红——本 PR 正要关闭的劫持通道会被悄悄重新打开。建议改为推导该集合：扫描 .github/workflows/*.yml 中 run: 调用 resolve-sandbox-image.mjs 的步骤并逐个工作流执行现有断言，同时保留一个显式、带注释链接的例外集合（当前为 repo-hygiene.yml，指向延后队列条目），随绑定落地逐步缩小。
— qwen3.8-max via Qwen Code /review (v0.21.14)
- R3-1: the contract test now requires every always()-gated consumer to also gate on the resolver step outcome, pinning the R2-4 fail-closed clause; verified that deleting the guard from the repair step now fails the suite (the mutant shipped green before). - R3-2: route both main() e2e tests through withDockerStub; the refusal test's untouched-file asserts move before the temp-dir cleanup — they previously ran after rmSync, so they passed no matter what the resolver wrote. - R3-3: derive the contract test's protected workflow set from the tree instead of a hand-enumerated list, so a new resolver step cannot land untested; repo-hygiene.yml stays in an explicit, staleness-checked exception set until its deferred binding lands.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #9527 (review round 4)All three inline suggestions are resolved in code; the review body and the Feedback points and dispositions
Mutation probes (each new guard witnessed)
Verification
中文说明轮次总结 — PR #9527（评审第 4 轮）三条行内建议均已在代码中解决；评审正文与沙箱验证评论均无阻断问题。在 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/模型 |
qwen-code-ci-bot
left a comment
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.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
.github/scripts/resolve-sandbox-image.mjs:102 — [probe] step-file hardening is a per-writer patch — 60 unprotected shell appends in qwen-autofix.yml keep the FIFO hang/swallow class reachable (scan step :1096 runs immediately before the res…
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| async function main() { | ||
| const requestedImage = validateRequestedImage(process.argv[2]); | ||
|
|
||
| const command = process.env.SANDBOX_COMMAND || 'docker'; |
There was a problem hiding this comment.
[Critical] The digest binding derives the exported image from the spawned binary's stdout, but the binary is still selected by SANDBOX_COMMAND (and PATH) — the same $GITHUB_ENV-append channel this PR explicitly closes for DOCKER_HOST/DOCKER_CONTEXT, and neither Resolve sandbox image step has an env: block pinning them. Pre-PR the export was a fixed trusted string, so steering the command was DoS-only; this diff makes the export a function of the command's output, which newly enables substitution. A co-resident process at runner uid — the diff's own threat model — appends SANDBOX_COMMAND=/tmp/shim to the job's $GITHUB_ENV before the resolver step; the shim answers both pull (Digest: sha256:<X>) and image inspect (["ghcr.io/qwenlm/qwen-code@sha256:<X>"]) with a self-consistent attacker-chosen digest — e.g. an older, vulnerable sandbox image pre-pulled into the shared store. Every new check interrogates the same shim and passes, so the resolver exports the attacker-chosen "bound" reference and the consumers run it.
Probe (resolver run from this worktree):
arm A (SANDBOX_COMMAND=shim): exports ghcr.io/qwenlm/qwen-code@sha256:eeee… (attacker digest)
arm B (pinned trusted binary): exports the genuine digest; attacker digest absent
arm C (PATH-shadowed 'docker'): exports the attacker digest
Note that a SANDBOX_COMMAND: 'docker' pin alone is insufficient: both jobs GITHUB_PATH-prepend ${RUNNER_TEMP}/qwen-bin immediately before the resolver, so a docker planted there shadows the bare name. Pin the binary at step level in both resolver steps — an absolute path such as /usr/bin/docker and/or a step-level PATH pin — mirroring the DOCKER_HOST pin.
中文说明
[Critical] digest 绑定从被 spawn 二进制的 stdout 推导导出的镜像，但该二进制仍然由 SANDBOX_COMMAND（以及 PATH）选择——这正是本 PR 为 DOCKER_HOST/DOCKER_CONTEXT 显式关闭的同一个 $GITHUB_ENV 追加通道，而两个 Resolve sandbox image 步骤都没有用于固定它们的 env: 块。PR 之前导出的是固定的可信字符串，操纵该命令只能造成 DoS；本 diff 使导出成为命令输出的函数，从而新增了镜像替换能力。runner uid 下的同驻进程（即本 diff 自身的威胁模型）可以在 resolver 步骤之前向该 job 的 $GITHUB_ENV 追加 SANDBOX_COMMAND=/tmp/shim；该 shim 对 pull 返回 Digest: sha256:<X>，对 image inspect 返回 ["ghcr.io/qwenlm/qwen-code@sha256:<X>"]，即攻击者自选且自洽的 digest（例如预先拉入共享存储的旧版含漏洞镜像）。所有新增检查审问的都是同一个 shim，因此全部通过，resolver 会导出攻击者选定的“已绑定”引用，消费方直接运行它。
探针（在本 worktree 中运行 resolver）：arm A（SANDBOX_COMMAND=shim）导出攻击者 digest；arm B（固定可信二进制）导出真实 digest，攻击者 digest 不出现；arm C（PATH 影子 docker，未设 SANDBOX_COMMAND）同样导出攻击者 digest。
注意：仅固定 SANDBOX_COMMAND: 'docker' 是不够的——两个 job 都在 resolver 之前通过 GITHUB_PATH 前置 ${RUNNER_TEMP}/qwen-bin，植入其中的 docker 会影子掉裸名。请在两个 resolver 步骤的 step 级 env 中固定二进制：使用绝对路径（如 /usr/bin/docker）和/或 step 级 PATH 固定，与 DOCKER_HOST 的固定方式保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| exportImage(requestedImage); | ||
| const requestedPull = await pullImage(command, requestedImage); | ||
| if (requestedPull.ok) { | ||
| await exportDigestBoundImage(command, requestedImage, requestedPull); |
There was a problem hiding this comment.
[Critical] The newly exported repo@sha256:… reference is invisible to the consuming CLI's image-presence check (docker images -q <image> in packages/cli/src/utils/sandbox.ts:948): after a tag pull, listing by digest reference returns empty even though the content is local. Every first consumer start on a daemon is therefore forced onto a network docker pull <digest-ref>, and when the registry is unreachable or rate-limiting at exactly that moment, ensureSandboxImageIsPresent returns false and the CLI throws FatalSandboxError — the autofix round crashes although the resolver pulled the exact content minutes earlier in the same job. Pre-PR the exported tag matched the resolver's tag pull, so consumer startup needed no registry access; nothing recovers this crash either, since the repair step is gated on verify outputs a crashed start never sets.
Probe on docker 29.1.3 with the real ghcr.io/qwenlm/qwen-code:0.21.14 image:
BASE docker images -q ghcr.io/qwenlm/qwen-code:0.21.14 -> e0bec4421898 (offline-safe)
PR docker images -q ghcr.io/qwenlm/qwen-code@sha256:e0bec4… -> empty (content IS local: image inspect finds it)
PR docker pull <digest-ref>, registry healthy -> exit 0 "Image is up to date", listing then matches
PR docker pull <digest-ref>, host unreachable -> exit 1 "failed to resolve reference … no such host" -> FatalSandboxError
Make the presence check digest-aware on the consumer side: in imageExists (packages/cli/src/utils/sandbox.ts), use docker image inspect --format '{{.Id}}' <image> — probe-verified to resolve a digest reference against tag-pulled content with no registry access.
中文说明
[Critical] 新导出的 repo@sha256:… 引用对消费方 CLI 的镜像存在性检查（packages/cli/src/utils/sandbox.ts:948 中的 docker images -q <image>）不可见：按 tag 拉取之后，用 digest 引用列举会返回空，即使内容已在本地。因此每个 daemon 上的首次消费方启动都会被迫发起一次网络 docker pull <digest-ref>；若此刻 registry 不可达或被限流，ensureSandboxImageIsPresent 返回 false，CLI 抛出 FatalSandboxError——尽管 resolver 几分钟前刚在同一 job 中拉取了完全相同的内容，autofix 轮次仍会崩溃。PR 之前导出的 tag 与 resolver 的 tag 拉取相匹配，消费方启动无需任何 registry 访问；该崩溃也无法被恢复——repair 步骤依赖崩溃启动永远不会设置的 verify outputs。
探针（docker 29.1.3，真实 ghcr.io/qwenlm/qwen-code:0.21.14 镜像）：BASE 按 tag 查询可离线命中；PR 按 digest 查询为空（image inspect 证明内容在本地）；registry 健康时 pull-by-digest 返回 "Image is up to date" 后自修复；registry 不可达时 pull 退出码 1（"failed to resolve reference … no such host"）→ FatalSandboxError。
请在消费方将存在性检查改为 digest 感知：在 imageExists（packages/cli/src/utils/sandbox.ts）中使用 docker image inspect --format '{{.Id}}' <image>——经探针验证可在不访问 registry 的情况下解析 tag 拉取内容的 digest 引用。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #9527Commit: Feedback points and dispositions[rc:3823887744] Critical — resolver binary steerable via
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D5-1 contract-test consumer detection rides on a verbatim substring match of the SETTINGS_JSON fragment — already recorded in the round-3 deferred list (review 4983323483)
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.
中文说明
已审查——无阻断问题。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const scriptPath = fileURLToPath( | ||
| new URL('./resolve-sandbox-image.mjs', import.meta.url), | ||
| ); | ||
| const expected = `ghcr.io/qwenlm/qwen-code@${GENUINE}`; |
There was a problem hiding this comment.
[Suggestion] The two e2e tests added by this delta (the refusal path at ~line 510 and the success path at ~line 568) paste the same ~20-line resolver-subprocess scaffold twice — the envFile/outFile derivation, the scriptPath resolution, and the identical execFileSync(process.execPath, [scriptPath, 'ghcr.io/qwenlm/qwen-code:1.2.3'], { env: { ...process.env, SANDBOX_COMMAND: stub, GITHUB_ENV: envFile, GITHUB_OUTPUT: outFile }, timeout: 15_000, stdio: 'pipe' }) invocation. That duplicated block is exactly the resolver's invocation contract: arg passing plus the three env names the export depends on. This PR's own history shows the contract is mutable — the SANDBOX_COMMAND pin was added mid-review and the scaffold was copy-pasted across rounds — so the next contract change must edit both copies in lockstep, and updating only one leaves the other test exercising a stale interface while both stay green. Extract a shared helper next to withDockerStub so each test keeps only its stub body and its assertions:
async function runResolver(stub) {
const envFile = join(dirname(stub), 'env');
const outFile = join(dirname(stub), 'out');
const scriptPath = fileURLToPath(
new URL('./resolve-sandbox-image.mjs', import.meta.url),
);
execFileSync(
process.execPath,
[scriptPath, 'ghcr.io/qwenlm/qwen-code:1.2.3'],
{
env: {
...process.env,
SANDBOX_COMMAND: stub,
GITHUB_ENV: envFile,
GITHUB_OUTPUT: outFile,
},
timeout: 15_000,
stdio: 'pipe',
},
);
return { envFile, outFile };
}中文说明
本 delta 新增的两个 e2e 测试（约 510 行的拒绝路径与约 568 行的成功路径）把同一段约 20 行的 resolver 子进程脚手架粘贴了两次——envFile/outFile 推导、scriptPath 解析，以及完全相同的 execFileSync(process.execPath, [scriptPath, 'ghcr.io/qwenlm/qwen-code:1.2.3'], { env: { ...process.env, SANDBOX_COMMAND: stub, GITHUB_ENV: envFile, GITHUB_OUTPUT: outFile }, timeout: 15_000, stdio: 'pipe' }) 调用。这段重复代码恰恰是 resolver 的调用契约：参数传递加上导出所依赖的三个环境变量名。本 PR 自己的历史表明该契约是可变的——SANDBOX_COMMAND 固定是在评审中途加入的，脚手架是跨轮次复制粘贴的——因此下一次契约变更必须同步修改两处；只改一处会让另一个测试继续验证过期接口，而两者都保持绿色。建议在 withDockerStub 旁抽取共享助手 runResolver(stub)（见上方代码），让每个测试只保留 stub 主体与断言。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const UNBOUND_WORKFLOWS = ['repo-hygiene.yml']; | ||
| for (const name of UNBOUND_WORKFLOWS) { | ||
| assert.ok( |
There was a problem hiding this comment.
[Suggestion] The UNBOUND_WORKFLOWS exemption has a tripwire for "workflow no longer runs the resolver" but none for "workflow now binds the resolver output": the loop skips exempted workflows before any binding check, so once the deferred repo-hygiene.yml binding lands, nothing detects that the exclusion went stale. A probe reproduced the failure timeline in a scratch tree: with the bindings added and the stale entry kept, regressing one binding ships green (# pass 30 / # fail 0); removing the entry makes the same regression fail (not ok 30 — bind QWEN_SANDBOX_IMAGE to the resolver step output). The protected set is otherwise derived from the tree precisely so nothing lands untested; a stale exemption silently re-creates the hole the derivation closes. Run the same per-job binding check against exempted workflows and invert the assertion — extract the per-workflow check into a function and assert exempt ? !passes : passes, failing with repo-hygiene.yml now binds the resolver output — drop it from UNBOUND_WORKFLOWS once it passes.
中文说明
UNBOUND_WORKFLOWS 豁免列表有「工作流不再运行 resolver」的绊线，却没有「工作流已绑定 resolver 输出」的绊线：循环在执行任何绑定检查之前就跳过被豁免的工作流，因此一旦延后的 repo-hygiene.yml 绑定落地，没有任何机制能发现该豁免已过期。探针在临时树中复现了这条失败时间线：绑定加上且过期豁免保留时，回退其中一个绑定仍能全绿（# pass 30 / # fail 0）；删除豁免条目后，同样的回退会失败（not ok 30 — bind QWEN_SANDBOX_IMAGE to the resolver step output）。受保护集合本是从目录树推导出来的，正是为了让任何改动都不会未经测试落地；一个过期的豁免会悄悄重新打开推导所关闭的缺口。建议对被豁免的工作流同样执行逐 job 的绑定检查并反转断言——把逐工作流的检查抽成函数，断言 exempt ? !passes : passes，一旦通过则以 repo-hygiene.yml now binds the resolver output — drop it from UNBOUND_WORKFLOWS 报错。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| if (String(step.if ?? '').includes('always()')) { | ||
| assert.ok( |
There was a problem hiding this comment.
[Suggestion] The outcome-gate requirement fires only for always()-gated consumers, but any status function that can be true after a failure — notably failure() — leaves a consumer binding an empty steps.sandbox_image.outputs.image when the resolver fails; per loadSandboxConfig (command && image is falsy for '') the agent then relaunches with no sandbox at all — the precise hole the new outcome == 'success' check plugs for the repair step only. A probe demonstrated the gap in a scratch tree: a failure()-gated consumer with correct bindings and no outcome gate passes the suite (# pass 30 / # fail 0), while flipping the gate to always() fails it (not ok 30 — an always()-gated consumer must also gate on the resolver step outcome). The guard's own comment states the rule it only half-enforces. Require the outcome gate whenever the consumer's if contains a status function that can be true after a resolver failure:
| if (String(step.if ?? '').includes('always()')) { | |
| assert.ok( | |
| if (/always\(\)|failure\(\)/.test(String(step.if ?? ''))) { | |
| assert.ok( |
中文说明
outcome 门禁要求只对 always() 门禁的消费方生效，但任何在失败后仍可能为真的状态函数——尤其是 failure()——都会让消费方在 resolver 失败时绑定到空的 steps.sandbox_image.outputs.image；按 loadSandboxConfig 的语义（command && image 对 '' 为假），agent 会在完全没有沙箱的情况下重新启动——这正是新增的 outcome == 'success' 检查只为 repair 步骤堵上的那个洞。探针在临时树中证实了该缺口：一个带正确绑定但没有 outcome 门禁的 failure() 门禁消费方能让套件全绿（# pass 30 / # fail 0），而把门禁换成 always() 则会失败（not ok 30 — an always()-gated consumer must also gate on the resolver step outcome）。守卫自己的注释写明了它只执行了一半的规则。建议：只要消费方的 if 包含任何在 resolver 失败后仍可能为真的状态函数，就要求其带 outcome 门禁（见上方 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #9527Round 5 brought three Suggestion-level findings from the automated reviewer (review rv:4987061547, no blockers) and no failed checks. All three inline findings were verified against the code and implemented; all three inline threads are resolved in code. Feedback points and dispositionsrv:4987061547 — review body (COMMENTED, no blockers) — no actionInformational only. It confirms the one repeated Suggestion (D5-1, contract-test consumer detection riding on a verbatim SETTINGS_JSON substring) is already recorded in the round-3 deferred list, and notes the actionlint embedded-shell source mapping for rc:3825100832 (R5-1) — duplicated resolver-subprocess scaffold in the two e2e tests — implementedExtracted a shared rc:3825100837 (R5-2) — UNBOUND_WORKFLOWS exemption has no tripwire for "workflow now binds the resolver output" — implementedReproduced the hole first, in a scratch tree: with repo-hygiene.yml fully bound and the stale exemption kept, regressing one binding shipped green ( rc:3825100846 (R5-3) — outcome-gate requirement only fires for
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- fallback-path digest-bound export has no behavioural test (mutation to exporting the mutable tag survives the suite) — already recorded in the round-3 deferred list (review 4983323483) at .github/scripts/resolve-sandbox-image.mjs:318
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.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:1099 — [review] Security pin blocks pasted 2x/4x in one workflow file; the repo's composite-action mechanism is unused.github/scripts/resolve-sandbox-image.test.mjs:69 (+2 locations) — [review] Comments name a nonexistent 'verification gate' consumer of the image output and invert the empty-value failure mode.github/workflows/qwen-autofix.yml:5017 — [review] Consumer step's env pins bind values but not binaries: bare qwen/docker resolve through a PATH a preceding host-shell step can rewrite via GITHUB_PATH
中文说明
已审查——无阻断问题。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action taken this round.
No commits were made; the PR head is unchanged. 中文说明本轮未采取任何操作。
未创建任何提交；PR 头部保持不变。 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 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 515 passed · 0 failed · 515 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：515 通过 · 0 失败 · 515 总计 抖动门： Verification reportPR #9527 Deep Verification (follow-up round) —
|
| # | Previous finding | Severity | Status at new head eb2988bd |
|---|---|---|---|
| 1 | R2-4 repair-outcome gate pinned by no test (mutation survivor M11, with a measured suggested fix) | Suggestion | fixed — commit 9c5e7282 generalized the fix: every always()/failure()-gated consumer must also gate on steps.<resolver>.outcome == 'success' (contract test in the node suite). Re-measured: deleting the clause from the repair step (mutant M7) now turns exactly the contract test red citing "must also gate on the resolver step outcome" — the exact clause that survived last round. The author's generalized implementation supersedes this round's suggested patch; the pin holds. |
| 2 | repo-hygiene.yml keeps the unbound consumer shape (disclosed by the author, deferred to the maintainer-owned queue) |
Informational | stands, unchanged shape, now tripwired — re-measured at the new head: 2 resolver steps with no id: and no SANDBOX_COMMAND pin, 3 consumers with no output binding and no endpoint pin (probe output in logs/gates-audit.log). New this round: the UNBOUND_WORKFLOWS exemption is tripwired in both directions — M10 (give the workflow the full binding) fails with "now binds the resolver output — drop it from UNBOUND_WORKFLOWS"; M11 (remove its resolver steps) fails with "no longer runs the resolver — drop it from UNBOUND_WORKFLOWS". A stale exemption can no longer ship silently. |
| 3 | C5b observation: when $GITHUB_OUTPUT is a FIFO the $GITHUB_ENV line lands before the refusal throws |
Observation | stands, still contained — re-measured: env file carried the digest-bound line, exit 1 in 31 ms, output channel unreadable. Containment still holds: the step's non-zero outcome fails it, consumers bind the step output (empty here — pinned by M9), and the repair pass is outcome-gated (pinned by M7). |
| — | Correction: description says "24 pass (was 16)" | — | still corrects the record — re-measured at the new head and new base: 30 tests at head, 4 at the base of this PR (see Corrections). |
All four rows re-measured at the new head; none carried forward by diffing the old report. The resolver script itself is byte-identical to the previously verified head (git diff 0ff059ca eb2988bd -- .github/scripts/resolve-sandbox-image.mjs = 0 lines), and unchanged between the old and new base tips — yet every A/B cell and gate was re-run rather than inherited.
Central claim and A/B proof (re-measured)
Central claim: the resolver exports the <repo>@sha256:… RepoDigests entry that matches both the pulled repository and the digest the pull itself printed — never the mutable tag — and fails closed (zero writes, non-zero exit) when the pull prints no Digest: line, the inspect yields no RepoDigests, or no entry matches the pair.
Driven end-to-end through the SANDBOX_COMMAND seam: a fake docker shim plays the daemon (chunk-split Digest: pull stdout, inspect JSON, env recording per spawn); both step files observed on disk. Base arm = the exact HEAD^1 blob. Witness: 01-ab-cells-head-vs-base.png; raw: logs/ab-run.log, logs/ab-results.json.
| # | Cell | Oracle | HEAD | BASE |
|---|---|---|---|---|
| C1 | happy path; Digest: split across two writes |
step-file bytes | QWEN_SANDBOX_IMAGE=<repo>@sha256:… and image=<repo>@sha256:… |
mutable tag in env; no output channel |
| C2 | same-content retag: RepoDigests[0] = a/a@<same digest> |
exported reference | pulled-repo entry (exact match, not index 0) | mutable tag (oblivious) |
| C3 | foreign content: RepoDigests = ["aaa.example/backdoor@<other>"] |
exit + files | exit ≠ 0, none of which is, both files untouched |
exit 0, exports mutable tag — attack succeeds |
| C4 | pull exits 0 without a Digest: line |
exit + files | exit ≠ 0, reported no Digest line, both files untouched |
exit 0, exports mutable tag — attack succeeds |
| C5 | planted FIFO (no reader) at $GITHUB_ENV |
elapsed + exit | ENXIO refusal, exit 1, fast | blocks until SIGKILL at the 8 s harness cap |
| C5b | planted FIFO at $GITHUB_OUTPUT |
elapsed + files + exit | refusal in 31 ms, exit 1; env line landed first (finding #3) | exit 0 — base has no output channel |
| C6 | hostile parent env: DOCKER_HOST=tcp://attacker…, DOCKER_CONTEXT=rogue |
shim-recorded env per spawn | 2 spawns, both HOST=[unset] CTX=[default], still digest-bound |
1 spawn, HOST=[tcp://attacker.invalid:2375] CTX=[rogue] — passed through |
28/28 scripted assertions across these cells (20 head, 8 base controls). Suite-level witness of the same behaviors: 03-resolver-suite-head.png (30/30 TAP).
What the delta since the previous head adds, and how each piece is pinned
Three commits landed after the previous round (9c5e7282, 495a13c8, eb2988bd). The middle two sit beyond the depth-2 shallow boundary — git rev-list 0ff059ca..HEAD^2 returns 1 commit while the metadata lists 3, and git cat-file commit eb2988bd records parent 495a13c8 which the shallow file withholds — so per-commit attribution is out; the aggregate tree diff 0ff059ca..eb2988bd (4 files: the test file ±330, qwen-autofix.yml +14, sandbox.ts +7, sandbox.test.ts +45) is what this round verifies.
- R3-1, the previous Finding pre-release: fix ci #1 fix: the outcome-gate assertion generalized to every
always()/failure()-gated consumer. Mutant M7 (drop the repair clause) → exactly the contract test red. Last round this mutant shipped green. - R3-3, derived contract set + tripwires: the protected workflow set is scanned from the tree (only
qwen-autofix.ymlandrepo-hygiene.ymlrun the resolver); the exemption is tripwired both ways (M10/M11 both killed with the exact staleness messages). Side benefit measured: the base tip drifted 23 lines inqwen-autofix.ymlbetween rounds (review-reply idempotency), 0 sandbox-related lines — and any future sandbox consumer anywhere in the tree is caught by the derived set. - Binary pin (
495a13c8): both resolver steps pinSANDBOX_COMMAND: '/usr/bin/docker'in step env (outranks an appended$GITHUB_ENVvalue and a$GITHUB_PATHshadow); the contract test requires the absolute path (M12: baredocker→ red). actionlint resolves everysteps.sandbox_image.*reference at head (0 findings; the liveness probe's scoped 2-reference failure is in the Gates table). - Shared e2e scaffold (
eb2988bd): bothmain()e2e tests run through onerunResolvercontract copy; M5 (export the mutable tag) kills both at the same call site, M17 confirms the new 30-loop unlink-race test is live (3/3 attempts red, test 7 plus varying collateral race hits). - Digest-aware image check (
495a13c8, the only production TS change):imageExistsnow runsdocker image inspect --format {{.Id}} <image>instead ofimages -q— digest references resolve against local content offline instead of forcing a registry round-trip at every consumer startup. Gate 26/26; reverting the args array turns exactly the new test red on the behavioral args assertion (05-sandbox-ts-vacuity.png), restored green afterwards. - Updated vitest source pins (
spawnDockerCaptureliterals): M16 (drop one trailing period from the exit-code error literal) fails exactlykeeps sandbox image fallback covered by a reusable script(04-m16-vitest-literal-pin.png) — the updated pins are live, not rubber-stamped.
Corrections
- Description test counts (carried, re-measured): the body says
node --test … 24 pass, 0.5s (was 16). Measured at this head: 30 tests (# tests 30 / # pass 30); the base of this PR (HEAD^1blobs) has 4 tests, not 16 — "16" still appears to describe the frozen feat(autofix): run the verification gate in an ephemeral container #9214 lineage. Correcting the record only; no code impact.
Findings
None new. The two carried informational items, restated with this round's measurements:
1. Informational (disclosed, deferred) — repo-hygiene.yml keeps the unbound consumer shape
Re-measured at the new head via the contract test's own detection logic: job scan — 1 resolver (no id, no SANDBOX_COMMAND) + 1 unbound consumer; job fix — 1 resolver (same) + 2 unbound consumers (logs/gates-audit.log). Residual exposure unchanged from last round: the resolve→consume retag channel is closed there too (the resolver change benefits every caller), while env-append steering and the unpinned endpoint remain open in that workflow only. Tracked in the deferred review-findings queue per the author's commit (01452e03); the new bidirectional tripwire guarantees the exemption is dropped the moment the binding lands. No action for this PR.
2. Observation — C5b partial write remains contained
Re-measured ordering: $GITHUB_ENV receives the digest-bound line before the FIFO refusal at $GITHUB_OUTPUT throws (exit 1, 31 ms). Containment holds for the same three reasons as last round, each now pinned by a killed mutant: the step's non-zero outcome, consumers bound to the step output (M9), and the repair pass outcome-gated (M7). No action needed.
Mutation matrix (vacuity proof for the refactored suite and the new pins)
Baseline unmutated head: 30/30 green (or the kills would mean nothing). Witness: 02-mutation-matrix.png; raw: logs/mutation-run.log, logs/mutation-summary.json, logs/mutant-m15.log, logs/mutant-m16.log.
| Mutant | Change | Expected | Observed |
|---|---|---|---|
| M0 | none (scratch-tree control) | green | 30/30 green |
| M1 | sandboxSpawnEnv returns parent env (author claim) |
3 endpoint tests fail | KILLED — exactly those 3 |
| M2b | stdout = String(chunk) (author claim, fine form) |
cross-chunk test fails | KILLED — exactly that test |
| M3 | appendStepFile → plain appendFileSync (author claim) |
FIFO tests fail | KILLED — exactly the 2 FIFO tests |
| M4 | inspect timeout back to the fixed constant (author claim) | timeout test fails ~30 s | KILLED — exactly that test |
| M5 | main() exports the requested tag (pre-PR behavior) |
both e2e tests fail | KILLED — both, same call site |
| M6 | exact repo@digest match → suffix-only |
retag test fails | KILLED — exactly that test |
| M7 | workflow: drop repair outcome gate (prev-round survivor) | contract test fails | KILLED — "must also gate on the resolver step outcome" |
| M8 | workflow: drop one consumer's DOCKER_HOST: '' |
contract test fails | KILLED — names 'Assess candidates' |
| M9 | workflow: bind consumer from ${{ env.… }} |
contract test fails | KILLED — names 'Assess candidates' |
| M10 | tripwire: repo-hygiene.yml gains the full binding |
exemption must go stale | KILLED — "now binds the resolver output — drop it" |
| M11 | tripwire: repo-hygiene.yml stops running the resolver |
exemption must go stale | KILLED — "no longer runs the resolver — drop it" |
| M12 | workflow: SANDBOX_COMMAND: 'docker' (bare) |
contract test fails | KILLED — "pin SANDBOX_COMMAND to an absolute docker path" |
| M13 | workflow: drop the resolver step id: |
contract test fails | KILLED — "the resolver step needs an id" |
| M15 | pullImage ignores the injected timeout |
pull-timeout test fails | KILLED — exactly that test |
| M16 | drop the trailing period from the exit-code error literal | the vitest source pin fails | KILLED — exactly keeps sandbox image fallback… (1 failed | 191 skipped) |
| M17 | withDockerStub return fn(stub) (no await) |
race test fails | KILLED — 3/3 attempts (test 7 + varying collateral) |
| BASE-wf | head suite vs HEAD^1 workflows |
exactly 1 red | exactly the contract test, "the resolver step needs an id" |
All four author-claimed mutation/test pairs re-confirmed after the scaffold refactor; no mutant regressed in the wrong direction; no off-target kills beyond the explained same-call-site set (M5) and the inherently probabilistic collateral of M17. The previous round's sole survivor (M11 there = M7 here) is dead.
Gates
| Gate | Result |
|---|---|
node --test .github/scripts/resolve-sandbox-image.test.mjs (HEAD) |
30/30 pass (03-resolver-suite-head.png) |
Same suite at BASE (HEAD^1 blobs) |
4/4 pass |
| Same suite vs BASE workflows (liveness cell) | exactly 1 expected red: "the resolver step needs an id…" |
vitest scripts/tests/qwen-autofix-workflow.test.js (HEAD) |
192/192 pass — exit 1 driven solely by one [vitest-worker]: Timeout calling "onTaskUpdate" unhandled error |
| Same suite on base worktree (A/A control) | 192/192 pass with the same single RPC error and exit 1 — identical shape ⇒ environmental (68 s suite on the shared loaded runner), not PR-caused; the lane's flakiness gate will again read this file as consistent-fail and CI owns that signal |
vitest packages/cli/src/utils/sandbox.test.ts (HEAD) |
26/26 pass |
| ESLint on the 5 changed files | clean; liveness probe (planted unused var) caught by no-unused-vars |
Prettier --check on the 6 changed files |
clean |
| actionlint 1.7.12 (SHA256-verified against the release checksums) | 0 findings on head and base qwen-autofix.yml (-shellcheck=; shellcheck absent in-container); liveness probe on the id-removed tree caught the 2 steps.sandbox_image references of the affected job as "property sandbox_image is not defined" (the other job's resolver keeps its id, so its 2 references still resolve — exactly the scoped behavior expected) |
tsc --noEmit on packages/cli (the only TS change) |
clean |
Not covered
- Per-commit attribution for
9c5e7282and495a13c8— beyond the depth-2 shallow boundary (proven:rev-listreturns 1 where the metadata lists 3;cat-fileshows the withheld parent). Verified the aggregate0ff059ca..eb2988bdtree diff instead. - Real docker CLI/daemon semantics — no docker in this container. The shim reproduces the wire shape only. Two claims rest on the author's live probes, pinned here only at call-shape level:
docker images -qnever lists digest references whileimage inspectresolves them offline (thesandbox.tschange), and "the docker CLI skips an emptyDOCKER_HOST" (the workflow-side consumer pin; the resolver-side pin uses the strongerdelete). /usr/bin/dockeron the actual self-hosted runners — the binary pin's path assumption cannot be checked from this container; a wrong path fails the resolver step closed (job fails; no unsandboxed relaunch), so the failure mode is loud, not silent.- GHCR fallback network path (
fetchLatestGhcrSemver) — cells keep the requested pull successful; the shared spawn guard it would travel through is pinned by M1/M2b/M4/M15. - shellcheck (binary absent; actionlint run with
-shellcheck=) and yamllint (no pip3 in-container); norun:block content is changed by this PR — onlyid:,env:,if:keys. - Repo-wide gates (full workspace suites, repo-wide
eslint ., prettier over the tree): the PR touches no TypeScript beyondsandbox.ts(typechecked) — targeted suites + per-file lint only. - The vitest RPC error was not chased beyond the A/A attribution above.
- The contract test's consumer detection (run text contains
QWEN_SANDBOX_IMAGE, orSETTINGS_JSONcarries"sandbox": "docker") is a textual heuristic by design; a consumer that referenced the image through some future other mechanism would not be detected. Pre-existing design shape, unchanged by this PR; noted, not a finding.
Methodology
Environment: the CI verify container (Node v22.23.2, no docker daemon, no RUNNER_TEMP set in this session), working tree at merge ref 27af77d0f4; npm ci + npm run build pre-run. Harnesses drive the real sources (no stubs of code under test): the A/B harness (harness/ab-harness.mjs) spawns each resolver arm as a child process with SANDBOX_COMMAND pointed at a per-cell fake-docker shim and observes exit codes, stderr, step-file bytes, elapsed time, and shim-recorded endpoint env; the base arm is the git show HEAD^1: blob (node-builtin-only module — the workspace-realpath control does not apply). The mutation matrix (harness/mutation-matrix.mjs) materializes per-mutant .github trees under scratch/mut/ (nested in the repo so yaml resolves from the root node_modules) and runs the unmodified head suite against each; harness/mutant-m15.mjs, the M16 working-tree mutation (restored via git checkout, verified clean), and the M17 race probe cover the remaining pins. The base A/A ran in a git worktree of HEAD^1 with the root node_modules symlinked (lockfile untouched by the PR; the suite imports no workspace packages — realpath verified and irrelevant); worktree removed afterwards. harness/gates-audit.mjs and harness/final-tally.mjs re-derive every log-sourced number as scripted comparisons — the assertion journal (logs/assertions.jsonl, 71 records) plus the suite tests executed as gates (30 + 4 + 192 + 192 + 26) give the 515/515 total in assertions.json. Evidence captures rendered by scripts/verify-capture.mjs. Raw logs: logs/ (A/B run + results JSON, suite head/base, mutation run + summary, M15/M16, vitest head/base, sandbox head/mutant, ESLint + liveness, actionlint head/base/liveness, typecheck, gates audit, findings audit).
Flakiness gate log
rounds=5 files=3 skipped=0
file .github/scripts/resolve-sandbox-image.test.mjs: (cd .) node --test ./.github/scripts/resolve-sandbox-image.test.mjs
file packages/cli/src/utils/sandbox.test.ts: (cd packages/cli) npx --no-install vitest run ./src/utils/sandbox.test.ts
file scripts/tests/qwen-autofix-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-autofix-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
.github/scripts/resolve-sandbox-image.test.mjs: PPPPP
packages/cli/src/utils/sandbox.test.ts: PPPPP
scripts/tests/qwen-autofix-workflow.test.js: FFFFF
verdict: consistent-fail
summary: 1 of 3 changed test file(s) failed identically in every round — deterministic, so CI owns that signal
--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/resolve-sandbox-image.test.mjs: P (exit 0)
round 1 · packages/cli/src/utils/sandbox.test.ts: P (exit 0)
round 1 · scripts/tests/qwen-autofix-workflow.test.js: F (exit 1)
--- output tail · round 1 · scripts/tests/qwen-autofix-workflow.test.js ---
cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
�[32m✓�[39m scripts/tests/qwen-autofix-workflow.test.js �[2m(�[22m�[2m192 tests�[22m�[2m)�[22m�[33m 64509�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mholds a round while review-pr is in flight on the head (#8888) �[33m 414�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mauto-updates a PR red only from a stale base, gated on green-on-main �[33m 577�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mauto-reruns a check that died on infrastructure, once, guarded by run_attempt �[33m 597�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the stale-duplicate revalidation, including the conflict-only transition �[33m 4074�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the eligibility recheck across lifecycle and label states �[33m 3281�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mreleases the dispatch-pending marker when the recheck discards a target �[33m 1052�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mraises the round cap to TAKEOVER_MAX_ROUNDS while the label is present �[33m 428�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the takeover-command toggle across all four paths �[33m 3569�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally resets round counting at the latest takeover engage ack �[33m 814�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally seeds the round counter from the window anchor and only from it �[33m 1629�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mrecovers transient forced-target reads and reports terminal takeover blocks �[33m 679�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mwires forced admission end to end: reader, classifier, permission gate, reporter �[33m 412�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mposts the non-main base refusal without depending on any other API call �[33m 1891�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mswitches to Critical-only feedback after five change rounds �[33m 378�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mescalates to a maintainer-decision handoff when the diff keeps growing past budget (non-convergence) �[33m 768�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mposts a takeover milestone digest as rounds accumulate, with a residual bucket �[33m 605�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mrejects a round that expands into CI machinery outside the PR footprint �[33m 1092�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22msurfaces deny-by-default footprint expansions, rejecting only when enforcement says so �[33m 386�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mupserts deferred findings into a per-PR issue that survives the merge �[33m 8089�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbite check: rejects a round whose changed tests pass on the pre-round tree �[33m 2685�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mstops a PR that fails to push for CONSECUTIVE_FAILURE_CAP rounds in a row �[33m 526�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mre-arms a stranded PR from a marker instead of a deleted comment �[33m 542�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22maddress-side stale check mirrors the scan-side re-arm logic under bash �[33m 811�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally posts the re-arm marker only after verifying the PAT identity �[33m 351�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mresolves only the review threads whose findings it implemented �[33m 2006�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22manswers the threads it leaves open, in those threads �[33m 583�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mflags recoverable API renders without a leading status code, and skips non-recoverable ones �[33m 347�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mclassifies permanent API failures terminal and records the cause class �[33m 1049�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mpreserves an agent-written handoff when the budget kills qwen after it �[33m 645�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m review verification gate: baseline A/B on deterministic rejection�[2m > �[22mclassifies an unchanged branch by its verdict files (handoff contract) �[33m 558�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mkills a silent agent at the idle window, naming the idle limit �[33m 1244�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mnever fires while the agent emits protocol events, however slowly �[33m 3252�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mnever fires while the agent talks on stderr only �[33m 3253�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mdoes not treat an unterminated stdout byte stream as progress �[33m 796�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mrequests streamed partial progress so active headless work refreshes the watchdog �[33m 3254�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m stale sandbox container cleanup�[2m > �[22man idle kill removes only the running sandbox its own agent launched �[33m 1248�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m stale sandbox container cleanup�[2m > �[22ma budget kill removes only the running sandbox its own agent launched �[33m 1247�[2mms�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Unhandled Errors �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯�[39m
�[31m�[1m
Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Unhandled Error �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[31m�[1mError�[22m: [vitest-worker]: Timeout calling "onTaskUpdate"�[39m
�[90m �[2m❯�[22m Object.onTimeoutError node_modules/vitest/dist/chunks/rpc.-pEldfrD.js:�[2m53:10�[22m�[39m
�[90m �[2m❯�[22m Timeout._onTimeout node_modules/vitest/dist/chunks/index.B521nVV-.js:�[2m59:62�[22m�[39m
�[90m �[2m❯�[22m listOnTimeout node:internal/timers:�[2m585:17�[22m�[39m
�[90m �[2m❯�[22m processTimers node:internal/timers:�[2m521:7�[22m�[39m
�[31m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯�[39m
�[2m Test Files �[22m �[1m�[32m1 passed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[1m�[32m192 passed�[39m�[22m�[90m (192)�[39m
�[2m Errors �[22m �[1m�[31m1 error�[39m�[22m
�[2m Start at �[22m 23:54:15
�[2m Duration �[22m 65.07s�[2m (transform 219ms, setup 20ms, collect 282ms, tests 64.51s, environment 0ms, prepare 58ms)�[22m
round 2 · .github/scripts/resolve-sandbox-image.test.mjs: P (exit 0)
round 2 · packages/cli/src/utils/sandbox.test.ts: P (exit 0)
round 2 · scripts/tests/qwen-autofix-workflow.test.js: F (exit 1)
--- output tail · round 2 · scripts/tests/qwen-autofix-workflow.test.js ---
cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repository.
warning: You appear to have cloned an empty repos
...truncated -- full content in the run artifacts.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action takenCritical-only mode is active on PR #9527 (5 change-producing rounds are complete in this counting window). After triaging the workflow-prepared feedback for this round, there is nothing actionable:
The only feedback present is the deferred non-Critical item recorded in the No conflict was reported, so 中文说明Autofix 评审轮次：未执行任何操作PR #9527 当前处于仅处理 Critical 的模式（本计数窗口内已完成 5 个产生改动的轮次）。在对本轮工作流准备的反馈进行分类处理后，没有任何可操作的内容：
本轮唯一存在的反馈是记录在「延期非 Critical 反馈」区域中的非 Critical 条目（一条自动评审器的 PR 评论）。按照 Critical-only 规则，该区域属于审计记录，不是工作任务：未对其做任何代码修改、线程解决或评论回复。该条目保持开放状态，留待人工跟进。 本轮未报告冲突，因此没有合并 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 无冲突。 🧠 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.
LGTM, looks ready to ship. ✅
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at HEAD eb2988bd. All three unresolved Criticals are fixed by later commits — the threads are stale, not open defects (a thread's isResolved: false tracks its anchored line, not the fix). Verified each against the code as it stands:
1. "Protected output produced but never consumed" (@jifeng, line 277) — fixed. Both Resolve sandbox image steps now carry id: 'sandbox_image' (qwen-autofix.yml:1099, 3935), and all four consumers bind the expression-context output at step level — QWEN_SANDBOX_IMAGE: '${{ steps.sandbox_image.outputs.image }}' at lines 1147, 1313, 4884, 5020 — which outranks the appendable $GITHUB_ENV value. All four also pin DOCKER_HOST: '' / DOCKER_CONTEXT: 'default'. I ran the contract test that pins this: every sandbox-image consumer binds the resolver step output passes.
2. SANDBOX_COMMAND / PATH steering (line 296) — fixed. Both resolver steps pin SANDBOX_COMMAND: '/usr/bin/docker' in the step's own env:. The absolute path closes both arms of the probe: step env outranks an appended $GITHUB_ENV, and an absolute path cannot be shadowed by the ${RUNNER_TEMP}/qwen-bin / $GITHUB_PATH prepend that happens just before the resolver.
3. Digest reference invisible to the presence check (line 299) — fixed. imageExists now spawns image inspect --format {{.Id}} instead of images -q, which resolves repo@sha256:… against local content offline — no forced registry round-trip, no FatalSandboxError when the registry is unreachable. The new test is substantive: it asserts the exact spawn argv and that the next spawn is run, i.e. no pull was attempted.
Verification I ran locally
node --test .github/scripts/resolve-sandbox-image.test.mjs→ 30/30 pass, including the success-path e2e and the derived-consumer contract test.- The two newest Suggestions were also implemented in the final commit, not just deferred: the protected workflow set is now derived from the tree, and the exemption carries the inverted tripwire (
repo-hygiene.yml now binds the resolver output — drop it from UNBOUND_WORKFLOWS, test lines 740-748). scripts/tests/qwen-autofix-workflow.test.js: this PR's own changed assertions pass. The 14 local failures are the known macOS bash 3.2 environmental class (all embedded-bash behavioral cases, none touching sandbox-image resolution); CI on ubuntu is green.
The remaining 19 unresolved threads are Suggestions deferred under Critical-only mode, which matches this repo's own "after ~5 rounds land only Critical fixes" rule.
Two non-blocking items for the maintainer (raising as questions, not findings — I cannot verify either from here):
/usr/bin/dockeris the repo's first absolute docker path. It is correct onubuntu-latest; I cannot verify it for the self-hostedecs-qwenpool. If docker sits elsewhere there (e.g./usr/local/bin), every resolver run fails — fail-closed, so no security regression, but the pipeline stops. Worth confirming against the runner image before merge.repo-hygiene.ymlstill runs the resolver unbound. The out-of-footprint deferral is well-reasoned and the tripwire now prevents it being silently forgotten, but the open question in that thread — should the id + step-level binding land there as a maintainer-owned change? — is still unanswered.
Note this approval does not unblock the merge on its own: reviewDecision is still CHANGES_REQUESTED from @jifeng's review of 92c9a4e2, whose Critical is the one fixed in da1d9ad4 above. That review needs to be updated or dismissed.
|
Released in v0.22.0. |
5 participants
Salvaged from PR #9214, which is frozen without merging. This is the one piece of that diff that stands on its own, re-landed with the two Criticals the reviewer raised against it (R11-1, R11-2) fixed rather than carried.
What it does
Bind the exported sandbox image to the digest the pull reported. The image was exported as a mutable tag.
docker run <tag>resolves against the local store without re-pulling, so a co-resident process with daemon access candocker tagdifferent content under the same name between the resolve step and the consumer. The resolver now exports the<repo>@sha256:…RepoDigestsentry that matches both the pulled repository and theDigest:line the pull itself printed — necessary as a pair, becauseRepoDigestsis shared by every tag of the same content (a same-content retag can move index 0 off the pulled repo while a suffix-only@sha256:check still passes), and because retagged foreign content keeps its own repo.R11-1 — pin the daemon endpoint (also closes the same hole R9-9 named). The docker CLI resolves its endpoint from
DOCKER_HOST, then--context, thenDOCKER_CONTEXT, thencurrentContextin$DOCKER_CONFIG/config.json— a file shared by every runner registration on the pool. ClearingDOCKER_CONTEXTis not enough: an empty value falls through tocurrentContext. Both spawns now run with the context named explicitly andDOCKER_HOSTremoved from the child environment. There is deliberately no env override — a variable that selects the endpoint would be settable through the same$GITHUB_ENVchannel this closes.R11-2 — non-blocking, type-checked step-file appends.
$GITHUB_ENVand$GITHUB_OUTPUTlive under the runner-writable temp tree. A plain append opens whatever is at that path, and a planted FIFO with no reader blocksopen(2)until the 60-minute step timeout. The append now usesO_NONBLOCK(immediateENXIOon a FIFO) plus anfstatregular-file check, and drains the buffer rather than trusting a singlewriteSync.Verification
node --test .github/scripts/resolve-sandbox-image.test.mjs— 24 pass, 0.5s (was 16).Six new tests, each checked against a mutant of the code it pins, so none of them can pass vacuously:
sandboxSpawnEnvreturns the parent env unchangedstdout = chunkinstead ofstdout += chunkappendStepFile→ plainappendFileSyncThe two coverage gaps the #9214 review deferred against this file —
repoDigestOf's inspect timeout pinned by zero tests, andpullImage's cross-chunk stdout accumulation — are closed here. The inspect timeout became an optional parameter so a test can bound it.Refs #9089, #9524. Extracted from #9214.
中文说明
从已冻结、不会合入的 PR #9214 中摘出。这是该 diff 中唯一能独立成立的一块，并且把评审针对它提出的两个 Critical（R11-1、R11-2）就地修掉，而不是带着走。
改了什么
把导出的 sandbox 镜像绑定到 pull 报告的 digest。 原先导出的是可变 tag：
docker run <tag>直接在本地存储解析、不会重新拉取，因此在 resolve 步骤与消费方之间，任何有 daemon 访问权的同驻进程都能用docker tag把同名指向别的内容。现在导出的是同时匹配所拉取仓库与pull 自身Digest:行的那条<repo>@sha256:…RepoDigests条目——两者缺一不可：RepoDigests由同一内容的所有 tag 共享（同内容 retag 会让 index 0 移出被拉取的仓库，而只看@sha256:后缀的检查仍能通过），而被 retag 的外来内容会保留它自己的仓库名。R11-1——固定 daemon 端点（同时关闭 R9-9 指出的同一个洞）。docker CLI 的端点解析顺序是
DOCKER_HOST→--context→DOCKER_CONTEXT→$DOCKER_CONFIG/config.json里的currentContext，而后者是整个池所有 runner 注册共享的文件。仅把DOCKER_CONTEXT清空不够——空值会继续落到currentContext。现在两处 spawn 都显式指名 context 并从子进程环境中删除DOCKER_HOST。刻意不提供环境变量开关：能选择端点的变量本身就可以经由本次要关闭的$GITHUB_ENV通道设置。R11-2——步骤文件采用非阻塞 + 类型判别的追加写。
$GITHUB_ENV与$GITHUB_OUTPUT位于 runner 可写的临时目录下。普通追加会打开该路径上的任何东西，而一个无读者的 FIFO 会让open(2)阻塞到 60 分钟的步骤超时。现在使用O_NONBLOCK（遇 FIFO 立即ENXIO）加fstat常规文件校验，并循环写满缓冲区而不是信任单次writeSync。验证
node --test .github/scripts/resolve-sandbox-image.test.mjs——24 通过，0.5 秒（原为 16 个）。新增 6 个测试，每一个都对其所守护的代码做了变异验证，确保不会空过：把
sandboxSpawnEnv改成原样返回父环境 → 3 个端点测试失败；把stdout += chunk改成覆盖 → 跨块累积测试失败；把appendStepFile换回普通appendFileSync→ FIFO 测试在 5.5 秒内失败（该测试在子进程中执行追加，因此回归会让测试失败而不是像卡死步骤那样卡死整个测试套件）；把 inspect 超时改回固定常量 → 超时测试在 30 秒处失败。#9214 评审中针对该文件延后的两个覆盖缺口（
repoDigestOf的 inspect 超时零测试覆盖、pullImage的跨块 stdout 累积未固定）在此一并关闭。为便于测试限定时长，inspect 超时改为可选参数。Refs #9089、#9524。摘自 #9214。