fix(autofix): pass CI=true through the gate's env -i launches - #9649
Conversation
The verification gate launches (first pass + repair pass) run the branch's
build/typecheck/lint/test through an env -i clean child that allowlisted
only 8 variables and dropped the runner-provided CI=true. Without it the
gate's checks run with inverted CI semantics relative to the repo's
regular CI: packages/cli/src/ui/auth/AuthDialog.test.tsx skips 18
TUI-input tests on CI as unreliable, and without CI=true they un-skip
inside the gate and one flakes (~5s vi.waitFor) — reject_fix fires
retryable on a fix the PR's own CI passes green, burning the repair pass
and mislabeling the round's A/B baseline.
Add CI="${CI:-true}" to both env -i allowlists (probe: CI=true → file
green; env -u CI → the TUI test fails 1/25), and pin the full allowlist
contents in the contract tests — the old pin counted env -i occurrences
only, so a missing variable shipped green.
Follow-up from PR QwenLM#9262 (R5-1); issue QwenLM#9648.
|
Re-run — this pass reviews the current head; the first pass saw Template looks good ✓ Problem: observed, with strong evidence. Issue #9648 tracks it (deferred from #9262 R5-1), and the field evidence in this thread is concrete: four autofix rounds on 2026-08-21 died in the identical shape — agent pass green, gate failing on Direction: aligned — restores CI-semantic parity between the gate and the repo's regular CI, where Size: not applicable — no core package paths (+165/-5 across the workflow file and its contract test; CI machinery). Approach: the original two-line fix grew through review rounds 2–6: each addition (the structural launch pin, the step-level Risk: no elevated risk signals — no high-risk paths touched. Moving on to code review. 🔍 中文说明Re-run——本轮审查当前 head;首轮看到的是 模板完整 ✓ 问题:已观测到,证据充分。issue #9648 跟踪(自 #9262 R5-1 延期),本帖中的现场证据具体:2026-08-21 四轮 autofix 形态完全一致——agent 主轮绿、闸门仅挂 方向:对齐——恢复 gate 与仓库常规 CI(始终带 规模:不适用——未触及核心包路径(+165/-5,涉及 workflow 文件与其契约测试;CI 机制)。 方案:最初的两行修复在评审第 2–6 轮中扩展:每一处新增(结构化启动 pin、step 级 风险:无升级风险信号——未触及高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Re-run — re-reviewed the full +165/-5 diff at the current head, not just the delta the first pass saw. Code reviewNo blockers. My independent proposal for the stated problem — pass the runner's
Non-blocking notes, all already on the round-7 review ledger as deferred: (1) Test evidenceThis pass carries the PR's own CI signal on the reviewed commit, fetched via API (PR code is never executed in triage). Everything completed, nothing red: What PR CI cannot exercise is the gate itself end-to-end — it only runs during real autofix rounds. The sandboxed verification lane triggered alongside this triage (run 32551085388, in progress) covers that gap and will post its own report here; my attestation rests on the statically verified mechanism chain plus the green CI above, and the failure shape itself is documented by four production rounds in the thread, not by author claim.
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Real-scenario testing (local run): N/A — CI machinery with no user-visible surface. 中文说明代码审查无阻塞项。我对该问题的独立方案——把 runner 的
非阻塞项(均已在第 7 轮评审台账中记录为延后):(1) 测试证据本轮携带被审 commit 的 PR 自身 CI 信号(经 API 获取——triage 从不执行 PR 代码)。全部完成、无红项:运行 PR CI 无法端到端运行 gate 本身——它只在真实 autofix 轮次中运行。随本次 triage 触发的沙箱验证通道(run 32551085388,进行中)覆盖该缺口,会在本帖发布自己的报告;我的背书建立在上述静态核实的机制链与绿色 CI 之上,失败形态本身由本帖中四个生产轮次记录,而非作者单方声明。 真实场景测试(本地运行):N/A——CI 机制,无用户可见面。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — solid across every stage; the reservations are real but non-blocking, and two of them already sit on the review ledger's deferred list. Stepping back: this started as a two-line fix for a reproduced, production-costing bug — four dead autofix rounds and one terminated PR with discarded work are documented in the thread — and at its core it still is exactly that fix. The surrounding growth happened to the PR, not because of it: the review loop spent five rounds attacking the new contract pin, and the diff absorbed every surviving finding (structural launch pin, step-level env pins, absolute-path digest check), converging at round 7 with zero new ones. Reviewing the result cold, the hardenings hold up — both gate steps symmetric, pinned values match the job-level ones, the structural pin matches the workflow text verbatim — CI is fully green on the reviewed commit including the contract suite, and the causal chain is verified end-to-end in the tree. If I had to maintain this in six months, the dense pin comments are exactly the documentation I'd want at 3 a.m. Why 4 and not 5, named plainly: (1) the PR body still describes the pre-takeover scope — "only adds the runner-provided variable" no longer matches a +165/-5 diff; (2) CI is fully settled on this head — no pending 中文说明置信度:4/5 —— 各阶段均扎实;保留意见真实存在但都不阻塞,其中两条已在评审台账的延后清单上。 退后一步看:这最初是一个两行修复,针对的是已复现、在生产上持续付出代价的 bug——帖中记录了四个死掉的 autofix 轮次和一个被终结、已完成工作遭丢弃的 PR——其核心至今仍是那个修复。外围的扩展是评审循环作用于 PR 的结果,而非作者主动扩权:五轮评审反复攻击新契约 pin,diff 吸收了全部幸存发现(结构化启动 pin、step 级 env pin、绝对路径摘要检查),第 7 轮以零新发现收敛。冷审最终结果:加固站得住——两次 gate 启动对称、钉住的值与 job 级一致、结构化 pin 与 workflow 文本逐字吻合——被审 commit 上 CI 全绿(含契约套件),因果链已在树中端到端核实。若六个月后由我维护,这些密集的 pin 注释正是凌晨三点想要的文档。 为什么是 4 而不是 5,直说:(1) PR 描述仍停留在 takeover 之前的范围——"只补充 runner 提供的变量"已不匹配 +165/-5 的 diff;(2) CI 已在本 head 上完全收敛——无进行中的 — Qwen Code · qwen3.8-max Reviewed at |
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
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": none (no check was cut short; the only substituted check — a true vitest execution — was replaced by the equivalent standalone replication above, which exercise…; "agent 3a": none — but note the vitest run of the actual test ( npm run test:scripts ) could not be executed here because the review worktree has no installed dependencies;….
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.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 1a":none (no check was cut short; the only substituted check — a true vitest execution — was replaced by the equivalent standalone replication above, which exercise…;"agent 3a":none — but note the vitest run of the actual test ( npm run test:scripts ) could not be executed here because the review worktree has no installed dependencies;…。
未检查(工具限制,非阻断):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 passed = ( | ||
| argList.match(/[A-Z_][A-Z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? [] | ||
| ) |
There was a problem hiding this comment.
[Suggestion] The new allowlist pin's regex matches uppercase-only variable names ([A-Z_][A-Z0-9_]*), but the comment above claims it enumerates "the full allowlist of BOTH gate launches" — a lowercase or mixed-case entry is invisible to the sorted-multiset check.
This matters because lowercase npm_config_* is npm's own configuration convention, and the gate child runs npm build/test: if a future edit (or an autofix repair round touching this workflow) adds e.g. npm_config_registry="https://evil.example" \ to one gate launch, this pin stays green. A later change that drops that entry from one launch only then produces exactly the asymmetric-allowlist regression this test was written to catch — first-pass and repair-pass gates running with different environments — with CI green the whole time.
Verified by probe at this commit: injecting npm_config_registry="https://evil.example" \ into the FIRST gate launch only leaves the whole file green (215/215); widening the name class to [A-Za-z_][A-Za-z0-9_]* makes the same mutant fail with + "npm_config_registry" in the assertion diff (and stays green on the pristine workflow).
| const passed = ( | |
| argList.match(/[A-Z_][A-Z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? [] | |
| ) | |
| const passed = ( | |
| argList.match(/[A-Za-z_][A-Za-z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? [] | |
| ) |
中文说明
新增允许列表 pin 的正则只匹配大写变量名([A-Z_][A-Z0-9_]*),但上方注释声称它枚举了「两次 gate 启动的完整允许列表」——小写或混合大小写的条目对排序多重集合检查不可见。
这之所以重要:小写 npm_config_* 是 npm 自身的配置约定,而 gate 子进程会运行 npm build/test。若未来某次编辑(或触及此 workflow 的 autofix repair 轮次)向其中一个 gate 启动添加例如 npm_config_registry="https://evil.example" \,该 pin 仍为绿色;之后若某次变更只从其中一个启动删除该条目,就会产生本测试要捕获的允许列表不对称回归——首过与修复过 gate 以不同环境运行——而 CI 全程全绿。
已在本提交上通过探针验证:仅向第一个 gate 启动注入 npm_config_registry="https://evil.example" \,整个文件仍全绿(215/215);将名称类放宽为 [A-Za-z_][A-Za-z0-9_]* 后,同一突变体在断言 diff 中以 + "npm_config_registry" 失败(在原始 workflow 上保持全绿)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
@qwen-code /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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
The allowlist pins extract passed variables with [A-Z_][A-Z0-9_]*, so a lowercase or mixed-case entry — e.g. npm's own npm_config_* convention — is invisible to the sorted-multiset check: adding one to a single launch ships green, and only a later asymmetric drop then fails, producing exactly the divergent-environment regression the pins exist to catch while CI stayed green the whole way. Widen the name class to [A-Za-z_][A-Za-z0-9_]* in both pins — the gate launches pin added in 8db672e and the sibling run_deferred_upsert pin that shares the identical regex and blind spot (probe: inject npm_config_registry="..." into one launch → old regex 215/215 green, widened regex fails with + "npm_config_registry" at each pin; pristine workflow stays green). Review round 1 finding R1-1.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Address-review round summary — PR #9649Feedback points[rc:3828417467] [Suggestion] Allowlist pin regex matches uppercase-only variable namesClassification: Optional (Suggestion) — a checkable defect claim about the pin this PR added, verified by reproduction before implementing. Reproduction (before fix): injected Fix: widened the variable-name class to
No comment or expectation-list changes were needed: the expected allowlists are all-uppercase and unchanged; the widened regex only stops missing non-uppercase entries. Mutation probes (witnesses for the widened guards):
Disposition: resolved in code (commit Other items
Environment repair (affects verification runs)The first Verification
中文说明处理评审轮次总结 — PR #9649反馈点[rc:3828417467] [Suggestion] 允许列表 pin 的正则只匹配大写变量名分类: Optional(建议)——针对本 PR 新增 pin 的可检验缺陷声明,在实施前先复现验证。 复现(修复前): 向 修复: 将变量名字符类放宽为
无需修改注释或期望列表:期望的允许列表全为大写且保持不变;放宽后的正则只是不再漏掉非大写条目。 突变探针(放宽后守卫的见证):
处置: 已在代码中解决(提交 其他事项
环境修复(影响验证运行)首次 验证
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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-autofix-workflow.test.js:11196 — [probe] No pin covers the gate launches' LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH command-prefix strip
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const argList = step.slice(argStart, step.indexOf('bash --norc')); | ||
| const passed = ( |
There was a problem hiding this comment.
[Critical] The new gateAllowlist pin enumerates text tokens, not the shell command structure the runner actually executes. Five distinct shell-level mutations — each probe-verified to keep all 215 contract pins green — silently defeat the gate's clean-child isolation:
- Window truncation / smuggling — the slice ends at the FIRST textual
bash --norc. Moving an entry whose value is nowhere pinned by value (e.g.BRANCH) last with the value"bash --norc"truncates the enumeration window, so an entry appended after it passes undetected whileenvstill parses it. Observed at runtime:CHILD_SAW_TOKEN=smuggled-v, and a smuggledBASH_ENVsourced attacker code inside the child before the digest-verified runner started (BASH_ENV_SCRIPT_EXECUTED=yes). - Commented-out entry —
# CI="${CI:-true}" \still satisfies both the multiset and the containment pin, but at runtime the#breaks the continuation chain and the launch splits: the gate script runs as a second command inheriting the parent step's FULL environment — the$GITHUB_ENVplant class the launch's own comment saysenv -iexists to drop. Observed:PLANT_VERDICT=[evil_verdict]in the child (baseline<unset>), with the runner even inheritingCI=truefrom the parent, masking the very symptom this pin was written for. - Dropped continuation — removing one trailing
\relocates every later assignment out of the clean child the same way; this pin and all other launch pins stay green while the child inherits the parent environment (observed as above). =-less operand hijack — inserting one=-less operand (e.g."${PLANT_CMD}") beforebash --norcis invisible to theNAME=valuegrammar, yetenv(1)execs exactly that operand as the command word, demoting the digest-verified gate script to argv. Observed:/usr/bin/env -i A=1 /bin/echo HIJACKED-CMD bash --norc scriptprintsHIJACKED-CMD bash --norc script. The hijacked word still receives the allowlistedGITHUB_OUTPUT, so it writes the verdict that decides the PAT push.- Suffix after the closing quote —
CI="${CI:-true}"xpasses both checks while bash concatenates the suffix into the value, so the child seesCI=truex;process.env['CI'] === 'true'then becomes false in the gate, the 18 deliberately-skipped TUI tests un-skip and one flakes — the exact inversion this PR exists to prevent.
The entry space — shell edits that preserve the matched token text — is unbounded: a new bypass shape was found in each of four successive audit rounds, so patching shape by shape will not close it. Pin the launch structurally instead: one verbatim adjacency regex over the whole launch block (launch head, every assignment in order with exact values and trailing \ continuations, through bash --norc), mirroring the sibling upsert pin's LD_* adjacency regex — probe-verified red on shapes 2 and 3; a residue-exclusivity check (strip the matched NAME=value tokens and expect the remainder to equal /usr/bin/env -i) — probe-verified red on shape 4 and also catches in-window comments; the CI token pinned WITH its trailing delimiter (CI="${CI:-true}" \) — probe-verified red on shape 5, where a bare exact-token check is not enough because the suffix sits outside the regex match; and terminator uniqueness (expect((step.match(/bash --norc/g) ?? []).length).toBe(1)) or slicing to the full launch token — closes shape 1.
Witness:
Shape 1: mutant suite Tests 215 passed (215); runtime CHILD_SAW_TOKEN=smuggled-v, BASH_ENV_SCRIPT_EXECUTED=yes
Shapes 2/3: mutant suites green; BASE PLANT_VERDICT=[<unset>] -> MUTANT GATE RUNNER PLANT_VERDICT=[evil_verdict], PATH=<parent>, exit 0
Shape 4: Tests 215 passed (215) on "${PLANT_CMD}" mutant; env -i A=1 /bin/echo HIJACKED-CMD bash --norc s -> HIJACKED-CMD bash --norc s
Shape 5: mutant lane green; MUTANT child CI=[truex] vs BASELINE child CI=[true]
中文说明
新的 gateAllowlist pin 枚举的是文本 token,而不是 runner 实际执行的 shell 命令结构。五种不同的 shell 层面改动——每一种都经探针验证可让全部 215 个契约 pin 保持绿色——会悄无声息地击穿 gate 的干净子进程隔离:
- 窗口截断/走私——切片在第一个文本
bash --norc处结束。把一个值未被任何 pin 锁定的条目(如BRANCH)移到最后并赋值为"bash --norc",枚举窗口即被截断,其后追加的条目不会被 pin 看到,但env仍会解析它。运行时观测到:CHILD_SAW_TOKEN=smuggled-v;走私的BASH_ENV在 digest 校验过的 runner 启动之前就在子进程内 source 了攻击者代码(BASH_ENV_SCRIPT_EXECUTED=yes)。 - 注释掉的条目——
# CI="${CI:-true}" \同时满足多重集与包含检查,但运行时#会打断续行链、把启动拆成两条命令:gate 脚本作为第二条命令运行,继承父步骤的完整环境——正是启动自身注释所说env -i要丢弃的$GITHUB_ENV植入类。观测到子进程中PLANT_VERDICT=[evil_verdict](基线为<unset>),runner 甚至从父环境继承CI=true,把这个 pin 要防的症状也掩盖了。 - 丢失续行符——删掉一个行尾
\会以同样方式把其后所有赋值移出干净子进程;该 pin 与其他所有启动 pin 保持绿色,子进程继承父环境(观测同上)。 - 无
=操作数劫持——在bash --norc之前插入一个无=操作数(如"${PLANT_CMD}")对NAME=value语法不可见,但env(1)恰恰会把该操作数作为命令 exec,digest 校验过的 gate 脚本被降级为 argv。观测:/usr/bin/env -i A=1 /bin/echo HIJACKED-CMD bash --norc script输出HIJACKED-CMD bash --norc script。被劫持的命令仍能拿到允许列表中的GITHUB_OUTPUT,从而写入决定 PAT push 的 verdict。 - 闭引号后的后缀——
CI="${CI:-true}"x能同时通过两项检查,而 bash 会把后缀拼进值里,子进程得到CI=truex;gate 内process.env['CI'] === 'true'变为 false,18 个被刻意跳过的 TUI 测试解除跳过并出现 flake——正是本 PR 要防止的语义反转。
入口空间——保持被匹配 token 文本不变的 shell 编辑——是无界的:连续四轮审计每轮都发现新的绕过形态,逐个形态打补丁无法收敛。请改为结构性地钉住整个启动块:对整个启动块使用一条逐字邻接正则(启动头、按序的每条赋值及其精确值与行尾 \ 续行,直到 bash --norc),镜像 sibling upsert pin 的 LD_* 邻接正则——探针验证对形态 2、3 变红;残差排他检查(剥掉匹配到的 NAME=value token 后,剩余部分必须恰好等于 /usr/bin/env -i)——探针验证对形态 4 变红,且能捕获窗口内注释;CI token 连同其后分隔符一起钉住(CI="${CI:-true}" \)——探针验证对形态 5 变红(仅精确 token 检查不够:后缀位于正则匹配之外);钉住终止符唯一性(expect((step.match(/bash --norc/g) ?? []).length).toBe(1))或切片到完整启动 token——关闭形态 1。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| .map((m) => m.split('=')[0]) | ||
| .sort(); |
There was a problem hiding this comment.
[Suggestion] The multiset compares variable NAMES only, so a symmetric value swap on any of the six non-CI entries is invisible to this pin, while the sibling upsert pin pins every entry by exact token. Probe-verified: swapping PATH="${TRUSTED_PATH}" to PATH="${PATH}" in both gate launches keeps all 215 pins green. The bare swap is masked at runtime by the gate body's preceding export PATH="${TRUSTED_PATH}" — but that export line is itself unpinned for the gate steps: the compound mutant (swap + export-line deletion in both gates) is also green, and the host PATH then serves both the pre-launch digest check (sha256sum -c) and the gate child, so host-planted binaries win both silently.
After the multiset check, mirror the sibling's exact-token loop for the six non-CI entries — e.g. for (const entry of ['PATH="${TRUSTED_PATH}"', 'HOME="${HOME}"', 'RUNNER_TEMP="${RUNNER_TEMP}"', 'WORKDIR="${WORKDIR}"', 'BRANCH="${BRANCH}"', 'GITHUB_OUTPUT="${GITHUB_OUTPUT}"']) expect(argList).toContain(entry); — and pin the gate body's export PATH="${TRUSTED_PATH}" line to close the compound shape.
Witness: PATH-swap mutant and swap+export-deletion compound both Tests 215 passed (215); with the exact-token loop the compound fails: expected '/usr/bin/env -i …' to contain 'PATH="${TRUSTED_PATH}"'.
中文说明
多重集只比较变量名,因此对六个非 CI 条目中任意一个做对称的值替换,该 pin 都不可见;而 sibling upsert pin 对每个条目都按精确 token 锁定。探针验证:在两次 gate 启动中把 PATH="${TRUSTED_PATH}" 换成 PATH="${PATH}",全部 215 个 pin 保持绿色。裸替换在运行时会被 gate 主体中先行的 export PATH="${TRUSTED_PATH}" 掩盖——但该 export 行在 gate 步骤中同样没有被 pin:复合改动(值替换 + 删除两个 gate 的 export 行)同样全绿,随后宿主 PATH 会同时服务于启动前的 digest 检查(sha256sum -c)与 gate 子进程,宿主植入的二进制可以悄无声息地同时赢下两者。
建议:在多重集检查之后,仿照 sibling 的精确 token 循环锁定六个非 CI 条目——例如 for (const entry of ['PATH="${TRUSTED_PATH}"', 'HOME="${HOME}"', 'RUNNER_TEMP="${RUNNER_TEMP}"', 'WORKDIR="${WORKDIR}"', 'BRANCH="${BRANCH}"', 'GITHUB_OUTPUT="${GITHUB_OUTPUT}"']) expect(argList).toContain(entry);——并把 gate 主体的 export PATH="${TRUSTED_PATH}" 行也 pin 上,以关闭复合形态。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| expect(argStart, 'gate step lacks the env -i launch').toBeGreaterThan(-1); | ||
| const argList = step.slice(argStart, step.indexOf('bash --norc')); | ||
| const passed = ( | ||
| argList.match(/[A-Za-z_][A-Za-z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? [] |
There was a problem hiding this comment.
[Suggestion] The env-assignment token grammar is now duplicated byte-for-byte at two sites — here and in the upsert clean-child pin (~line 12809) — and this diff's second hunk exists solely to re-sync the two copies after they drifted apart (the older copy stayed uppercase-only until this delta aligned it; that stale form failed OPEN, hiding lowercase-named entries). If the grammar ever needs a new alternative (e.g. single-quoted values) and only one site is updated, the stale regex silently under-matches, the extracted multiset still equals the sanctioned list, and an unsanctioned environment widening ships green — exactly the mutation class these pins exist to catch.
Hoist a shared module-scope helper (matching this file's existing helper convention) and call it from both pins; anchors, expected lists, and per-site token pins stay local:
const envTokenNames = (argList) =>
(argList.match(/[A-Za-z_][A-Za-z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? []).map(
(m) => m.split('=')[0],
);中文说明
环境赋值 token 语法现在在两个位置逐字节重复——此处与 upsert 干净子进程 pin(约第 12809 行)——而本 diff 第二个 hunk 存在的唯一原因,就是在两份拷贝漂移之后重新同步它们(较旧的一份在本 delta 对齐之前一直只匹配大写;那个陈旧形态是失效开放的,会隐藏小写命名的条目)。如果该语法未来需要新的分支(例如单引号值)而只更新了一处,陈旧的正则会静默漏匹配,提取出的多重集仍等于许可列表,未经认可的环境放宽就会绿灯通过——恰恰是这些 pin 要捕获的改动类别。
建议提升为模块级共享辅助函数(与本文件既有的辅助函数约定一致),两个 pin 都调用它;锚点、期望列表与逐位置的 token pin 保持局部:
const envTokenNames = (argList) =>
(argList.match(/[A-Za-z_][A-Za-z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? []).map(
(m) => m.split('=')[0],
);— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证,但本 PR 落后于 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32484864137 🧠 Handled by Qwen Code · model/模型 |
|
Field evidence for this fix, from four autofix rounds today (2026-08-21). The flake described here is not hypothetical — it terminated one takeover PR and has started on a second. Every one of the four runs has the identical shape: the agent's own pass succeeds, the verification gate then fails on #9340 — three consecutive rounds, then terminated
Terminal state at
Then at Note the line number moves between runs — #9394 — same failure, one hour laterOne down, two to go before #9394 hits the same circuit breaker. Why this is worth landingBoth PRs are unrelated to auth: #9340 is
Cost so far: 4 rounds × ~18 min of agent budget plus 60–137 min of runner wall-clock each, one PR terminated with completed work thrown away, and a second PR one-third of the way to the same end. Any takeover PR whose gate happens to enter a repair pass is exposed. Happy to re-run this check on further rounds if that helps close out the 中文说明本 PR 修复的问题在今天(2026-08-21)四次 autofix 轮次中都有实测证据,并非假设——它已经终结了一个 takeover PR,第二个已经开始。 四次形态完全一致: agent 主轮成功 → 验证闸门仅因 #9340 连续三轮(run 32469529030 / 32470852671 / 32479699617),主轮均报 #9394(run 32484864137)一小时后重演: 两个 PR 都与 auth 无关(#9340 是 review-approach-signal,#9394 是钉钉 Workspace channel),永远不可能让那个测试变绿。目前代价:4 轮 × 约 18 分钟 agent 预算,外加每轮 60~137 分钟 runner 墙钟;一个 PR 被终结且已完成的工作被丢弃,另一个已走完三分之一。任何进入 Repair 轮的 takeover PR 都暴露在这个风险下。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 6c": none — I could not run the actual vitest suite (no node_modules in the review worktree), but the probe above replicates the changed assertions byte-for-byte aga….
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/workflows/qwen-autofix.yml:5222 — [probe] CI is passed through the gate allowlist without the step/job-level pin its siblings get — a $GITHUB_ENV plant of CI=false flows through "${CI:-true}" and re-inverts the CI semantics this PR …
中文说明
未探索到全部深度(达到工具调用预算):"agent 6c":none — I could not run the actual vitest suite (no node_modules in the review worktree), but the probe above replicates the changed assertions byte-for-byte aga…。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const argList = step.slice(argStart, step.indexOf('bash --norc')); | ||
| const passed = ( |
There was a problem hiding this comment.
[Critical] R2-1: Still stands at HEAD — this pin region is byte-identical to round 2 (only a merge of main landed since; git diff 0fcdd5af..4d87d79c does not touch it). A fresh round-3 probe re-ran the REAL pin against every bypass shape at this commit: all six keep the suite green while the runtime isolation is broken.
The gateAllowlist pin enumerates text tokens, not the shell command structure the runner actually executes:
- Window truncation / smuggling — the slice ends at the FIRST textual
bash --norc. An entry whose value is nowhere pinned (e.g.BRANCH="bash --norc") placed last truncates the enumeration window, so an entry appended after it is invisible to the pin but parsed byenv. Round-2 runtime: a smuggledBASH_ENVsourced attacker code inside the child before the digest-verified runner started. - Commented-out entry —
# CI="${CI:-true}" \satisfies both the multiset and the containment check, but the#breaks the continuation chain and the gate script runs as a second command inheriting the parent step's FULL environment. Round-2 runtime:PLANT_VERDICT=[evil_verdict]in the child (baseline<unset>). - Dropped continuation — removing one trailing
\relocates every later assignment out of the clean child the same way; the pin stays green while the child inherits the parent environment. =-less operand hijack — an operand without=beforebash --norcis invisible to the NAME=value grammar, yetenv(1)execs it as the command word, demoting the digest-verified gate script to argv; the hijacked word still receives the allowlistedGITHUB_OUTPUT, so it writes the verdict that decides the PAT push.- Suffix after the closing quote —
CI="${CI:-true}"xpassestoContain('CI="${CI:-true}"')while the child seesCI=truex;process.env['CI'] === 'true'becomes false and the 18 deliberately-skipped TUI tests un-skip — the exact inversion this PR exists to prevent. - NEW this round (reverse audit, probe-confirmed): the window is anchored on the FIRST textual mention of
/usr/bin/env -i \— delete the real launch head and let a preceding comment carry the anchor string, and the nine assignments become a bareVAR=… bash --norcprefix (bash runs it with the CURRENT environment, isolation gone) while every pin stays green (215/215), including the workflow-wideenv -icount pin.
Witness (round 3, at HEAD, real vitest pin + bash runtime oracle):
m1 truncation/smuggle: Tests 1 passed; child env carries SMUGGLED_VERDICT=approved
m2 commented entry: Tests 1 passed; child inherits full parent env (PLANT_FROM_PARENT, GH_TOKEN)
m3 dropped `\`: Tests 1 passed; same full-parent-env inheritance
m4 =-less operand: Tests 1 passed; evil binary executed with the allowlisted env, gate script never ran
m5 `…x` suffix: Tests 1 passed; child CI=truex
shape-6 comment head: Tests 215 passed (215); no env -i left in either gate step
The entry space — shell edits that preserve the matched token text — is unbounded (a new shape found in each of five audit rounds now); patching shape by shape will not close it. Pin the launch structurally instead: one verbatim adjacency regex over the whole launch block anchored on the launch head (as the sibling upsert pin anchors on LD_PRELOAD= LD_AUDIT=), every assignment in order with exact values and trailing \ continuations through bash --norc; a residue-exclusivity check (strip the matched NAME=value tokens, expect the remainder to equal /usr/bin/env -i); the CI token pinned with its trailing delimiter (CI="${CI:-true}" \); and terminator uniqueness (expect((step.match(/bash --norc/g) ?? []).length).toBe(1)).
中文说明
R2-1 在当前 HEAD 仍然成立——该 pin 区域与第 2 轮逐字节相同(此后仅合入了 main;git diff 0fcdd5af..4d87d79c 未触及此区域)。第 3 轮的新探针在该提交上用真实 pin 重跑了每一种绕过形态:六种形态全部让套件保持绿色,而运行时隔离均被击穿。
gateAllowlist pin 枚举的是文本 token,而不是 runner 实际执行的 shell 命令结构:
- 窗口截断/走私——切片在第一个文本
bash --norc处结束。把一个值未被任何 pin 锁定的条目(如BRANCH="bash --norc")移到最后会截断枚举窗口,其后追加的条目对 pin 不可见,但env仍会解析。第 2 轮运行时:走私的BASH_ENV在 digest 校验过的 runner 启动之前就在子进程内 source 了攻击者代码。 - 注释掉的条目——
# CI="${CI:-true}" \同时满足多重集与包含检查,但#会打断续行链,gate 脚本作为第二条命令运行并继承父步骤的完整环境。第 2 轮运行时:子进程出现PLANT_VERDICT=[evil_verdict](基线为<unset>)。 - 丢失续行符——删掉一个行尾
\会以同样方式把其后所有赋值移出干净子进程;pin 保持绿色而子进程继承父环境。 - 无
=操作数劫持——在bash --norc之前插入无=操作数对 NAME=value 语法不可见,但env(1)恰恰把它作为命令 exec,digest 校验过的 gate 脚本被降级为 argv;被劫持的命令仍拿到允许列表中的GITHUB_OUTPUT,从而写入决定 PAT push 的 verdict。 - 闭引号后的后缀——
CI="${CI:-true}"x能通过toContain检查,而子进程得到CI=truex;process.env['CI'] === 'true'变为 false,18 个被刻意跳过的 TUI 测试解除跳过——正是本 PR 要防止的语义反转。 - 本轮新发现(反向审计,探针确认):窗口锚定在第一个文本
/usr/bin/env -i \上——删掉真实启动头、让前面的注释携带锚定字符串,九条赋值就变成裸的VAR=… bash --norc前缀(bash 以当前环境运行它,隔离消失),而所有 pin 保持绿色(215/215),包括全 workflow 的env -i计数 pin。
见证(第 3 轮,HEAD,真实 vitest pin + bash 运行时 oracle):m1 截断/走私:Tests 1 通过,子进程环境携带 SMUGGLED_VERDICT=approved;m2 注释条目:Tests 1 通过,子进程继承完整父环境(PLANT_FROM_PARENT、GH_TOKEN);m3 丢失 \:Tests 1 通过,同样继承完整父环境;m4 无 = 操作数:Tests 1 通过,恶意二进制以允许列表环境执行、gate 脚本从未运行;m5 …x 后缀:Tests 1 通过,子进程 CI=truex;形态 6 注释启动头:Tests 215 passed (215),两个 gate 步骤中均不再有 env -i。
入口空间——保持被匹配 token 文本不变的 shell 编辑——是无界的(至今五轮审计每轮都发现新形态),逐个形态打补丁无法收敛。请改为结构性钉住整个启动块:以启动头为锚(如同 sibling upsert pin 锚定 LD_PRELOAD= LD_AUDIT=)的一条逐字邻接正则,覆盖按序的每条赋值及其精确值与行尾 \ 续行直到 bash --norc;残差排他检查(剥掉匹配到的 NAME=value token 后,剩余部分恰好等于 /usr/bin/env -i);CI token 连同尾分隔符一起钉住(CI="${CI:-true}" \);终止符唯一性(expect((step.match(/bash --norc/g) ?? []).length).toBe(1))。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| // allowlist of BOTH gate launches (first pass + repair pass) as a sorted | ||
| // multiset — a symmetric duplicate or a dropped entry both fail here. | ||
| const gateAllowlist = (step) => { | ||
| const argStart = step.indexOf('/usr/bin/env -i \\'); |
There was a problem hiding this comment.
[Critical] No pin covers the gate launches' LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH= command-prefix strip or the body-side unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH. This pin's window starts at /usr/bin/env -i \ — AFTER the prefix — and every other LD_* pin in this file applies to other steps (the PAT steps and the upsert children). The sibling upsert pin ~40 lines away includes its prefix in its window and pins it by regex, documenting exactly why: LD_* is stripped by a command-prefix assignment BEFORE /usr/bin/env — the one channel env -i cannot block (ld.so preloads into the env binary itself at execve). The removal side of the gate's LD_* guard is unpinned, and no step/job/workflow-level LD_* pin exists on the gate steps.
Failure: a symmetric edit deleting the prefix + the body unset from both launches ships green — probe-verified at HEAD: baseline Tests 215 passed (215); mutant with both gates' prefix + unset removed: Tests 215 passed (215). An earlier shell-capable step — this step's own comment says the agent runs branch code on the host — plants LD_PRELOAD via $GITHUB_ENV; ld.so maps the attacker's DSO into /usr/bin/env itself at execve, and its constructor runs before env -i is processed, with the full step environment including GITHUB_OUTPUT — the branch writes its own verdict through the very launch this pin suite exists to isolate. Flip check: with pins for the unset + prefix adjacency added, the same mutant fails at this pin.
// start the window at the launch line, as the sibling does
const argStart = step.indexOf('LD_PRELOAD= LD_AUDIT=');
// + pin the prefix by regex (as the sibling upsert pin) and each gate body's
// `unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH`This fix also independently kills shape 6 of the R2-1 thread (the comment-anchor hijack) — land both repairs together.
中文说明
没有任何 pin 覆盖 gate 启动的 LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH= 命令前缀剥离,也没有 pin 覆盖步骤体内的 unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH。本 pin 的窗口从 /usr/bin/env -i \ 开始——位于该前缀之后——而文件中所有其他 LD_* pin 都作用于其他步骤(PAT 步骤与 upsert 子进程)。约 40 行外的 sibling upsert pin 把前缀纳入窗口并用正则钉住,其注释恰好说明了原因:LD_* 由 /usr/bin/env 之前的命令前缀赋值剥离——这是 env -i 唯一无法阻断的通道(ld.so 在 execve 时就把预加载注入 env 二进制本身)。gate 的 LD_* 守卫的「删除侧」无人钉守,且 gate 步骤上不存在任何 step/job/workflow 级别的 LD_* pin。
失败路径:对称地删除两处启动的前缀 + 体内 unset 可以全程绿灯——已在 HEAD 探针验证:基线 Tests 215 passed (215);删除两个 gate 前缀 + unset 的突变体:Tests 215 passed (215)。更早的 shell 级步骤——本步骤自己的注释说明 agent 会在宿主机上运行分支代码——可通过 $GITHUB_ENV 植入 LD_PRELOAD;ld.so 会在 execve 时把攻击者的 DSO 映射进 /usr/bin/env 本身,其构造函数先于 env -i 处理、在包含 GITHUB_OUTPUT 的完整步骤环境中运行——分支就此通过这套 pin 本要隔离的启动写入自己的 verdict。翻转验证:加入 unset + 前缀邻接 pin 后,同一突变体在该 pin 处失败。
修复(同时独立关闭 R2-1 线程的形态 6 注释锚定劫持——两处修复应一起落地):像 sibling 一样把窗口起点设为启动行 const argStart = step.indexOf('LD_PRELOAD= LD_AUDIT=');,并用正则钉住前缀、钉住每个 gate 体内的 unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| .map((m) => m.split('=')[0]) | ||
| .sort(); |
There was a problem hiding this comment.
[Suggestion] R2-2: Still stands at HEAD (code unchanged since round 2). The multiset compares variable NAMES only, so a symmetric value swap on any of the non-CI entries is invisible to this pin, while the sibling upsert clean-child pin pins every entry by exact token. Probe-verified at HEAD: swapping PATH="${TRUSTED_PATH}" → PATH="${PATH}" keeps the real test green (215/215) while the runtime child receives the parent's PATH instead of the trusted value. Pin each entry by exact token as the sibling does (loop the expected NAME=value tokens with toContain), or fold into R2-1's structural adjacency regex, which pins every value.
中文说明
R2-2 在当前 HEAD 仍然成立(代码自第 2 轮以来未变)。该多重集只比较变量名,因此对任一非 CI 条目做对称的值替换对本 pin 不可见,而 sibling upsert 清洁子进程 pin 逐条以精确 token 钉值。已在 HEAD 探针验证:把 PATH="${TRUSTED_PATH}" 换成 PATH="${PATH}",真实测试仍全绿(215/215),而运行时子进程收到的是父进程的 PATH 而非受信路径。请像 sibling 一样逐条以精确 token 钉值(对期望的 NAME=value token 逐一 toContain),或并入 R2-1 的结构性邻接正则(每条值都被钉住)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| expect(argStart, 'gate step lacks the env -i launch').toBeGreaterThan(-1); | ||
| const argList = step.slice(argStart, step.indexOf('bash --norc')); | ||
| const passed = ( | ||
| argList.match(/[A-Za-z_][A-Za-z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? [] |
There was a problem hiding this comment.
[Suggestion] R2-3: Still stands at HEAD — the env-assignment token grammar is duplicated byte-for-byte at exactly two sites (here and the upsert clean-child pin, ~line 12813), and this diff's second hunk exists solely to re-sync the two copies after they drifted (the round-1 fix had to widen both sites in one commit). If the workflow's NAME=value quoting shape ever changes, the tokenizer must be fixed in two places; missing one makes that allowlist pin silently under-match — the quietly-weakened-pin failure mode the surrounding comments warn about. Extract the shared tokenizer into scripts/tests/workflow-helpers.js (already imported here):
export function extractEnvAssignments(argList) {
return argList.match(/[A-Za-z_][A-Za-z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? [];
}and call it from both sites, each keeping its own slice anchor, expected set, and extra pins.
中文说明
R2-3 在当前 HEAD 仍然成立——env 赋值 token 语法在两处逐字节重复(此处与 upsert 清洁子进程 pin,约 12813 行),而本 diff 的第二个 hunk 正是为了在两份拷贝漂移后重新同步(第 1 轮的修复不得不在一个提交里同时放宽两处)。如果 workflow 的 NAME=value 引号形态发生变化,tokenizer 必须在两处修改;漏掉一处就会让那个允许列表 pin 悄然欠匹配——正是周围注释反复警告的「pin 被悄悄削弱」失败模式。把共享 tokenizer 抽到 scripts/tests/workflow-helpers.js(此处已引入该模块):export function extractEnvAssignments(argList) { return argList.match(/[A-Za-z_][A-Za-z0-9_]*=(?:"[^"]*"|[^\s\\]*)/g) ?? []; },两处调用它,各自保留自己的切片锚点、期望集合与附加 pin。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summaryCommit: Feedback points and dispositions[Critical] R2-1 — rc:3830343017, rc:3831502386 (the Design notes vs the suggested recipe:
[Critical] R3-1 — rc:3831502396 (LD_ command-prefix strip and body unset unpinned on the gate steps): RESOLVED.* [Suggestion] R2-2 — rc:3830343022, rc:3831502407 (multiset compares names only; symmetric value swap invisible): RESOLVED. [Suggestion] R2-3 — rc:3830343032, rc:3831502409 (env-token grammar duplicated byte-for-byte at two sites): RESOLVED by subtraction. Review states rv:4993414577, rv:4994870562 (CHANGES_REQUESTED): their substance is the inline findings above; the disclosed gaps (reverse audit not started in round 2, tool budget in round 3, actionlint embedded-shell lint unsupported) are reviewer-side limitations, not requested changes. Deferred by the reviewer, not requested this round: Issue comment ic:5371388569 (@qqqys): field evidence from four autofix runs (#9340 terminated, #9394 progressing toward the same breaker) that the flake this PR fixes is real and costly. Informational — no change requested; noted with thanks. ConflictNone ( VerificationAll commands actually run this round, in order:
No settings source changed, so 中文说明Autofix 审查轮次总结提交: 反馈点与处置[Critical] R2-1 — rc:3830343017、rc:3831502386( 相对建议方案的实现说明:
[Critical] R3-1 — rc:3831502396(gate 步骤的 LD_ 命令前缀剥离与体内 unset 无 pin 覆盖):已解决。* [Suggestion] R2-2 — rc:3830343022、rc:3831502407(多重集只比较变量名,对称值替换不可见):已解决。 [Suggestion] R2-3 — rc:3830343032、rc:3831502409(env token 语法在两处逐字节重复):以减法解决。 审查状态 rv:4993414577、rv:4994870562(CHANGES_REQUESTED): 其实质内容即上述行内发现;披露的缺口(第 2 轮未开始反向审计、第 3 轮工具预算耗尽、actionlint 内嵌 shell lint 不支持)属于审查方限制,并非请求的变更。 审查方延后、本轮不要求: Issue 评论 ic:5371388569(@qqqys): 四次 autofix 运行的现场证据(#9340 被终结、#9394 正走向同样的熔断)表明本 PR 修复的 flake 真实且代价高昂。信息性内容——未请求变更;致谢记录。 冲突无( 验证本轮实际执行的全部命令及结果(按顺序):
未改动任何 settings 源,故不适用 🦷 Gate advisory — this round resolves a Critical/Request-changes finding with test-only changes (machine-measured): the bite check cannot verify a test-side fix, so the resolution rests on the round summary alone. · 本轮以纯测试改动解决 Critical/Request-changes 反馈(门自动测量):bite 检查无法验证测试侧修复,该解决仅以轮次摘要为凭。 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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 3a": none — no check was cut short..
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
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/workflows/qwen-autofix.yml:5222 — [probe] CI passes through the gate allowlist without the step/job-level pin its siblings get — a $GITHUB_ENV plant of CI=false flows through "${CI:-true}" and re-inverts the CI semantics this PR res…scripts/tests/qwen-autofix-workflow.test.js:12823 — [probe] upsert allowlist audit misses env operands whose names have no letter/underscore (1=x, =x) — the widened regex closed the lowercase corner but not the no-letter subclass (code unch…
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 3a":none — no check was cut short.。
未审查:反向审计——评审时间预算不足,未能开始第 4 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| 'bash --norc "${RUNNER_TEMP}/run-autofix-review-verification.sh"', | ||
| ] | ||
| .map((token) => token.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) | ||
| .join(' \\\\\n\\s*'), |
There was a problem hiding this comment.
[Suggestion] The inter-token separator of gateLaunchPin treats every JS \s member as harmless indentation, but bash recognizes only space/tab/newline as whitespace — two edit shapes keep all four loop assertions green while breaking the isolation this pin exists to guarantee. A BLANK LINE between two continued allowlist entries splits the launch into two commands: the orphaned env -i prints and exits 0, and the second command (... bash --norc run-autofix-review-verification.sh) runs with the FULL inherited step environment — isolation gone, and NODE_OPTIONS is a live plant channel the gate script does not strip. A GLUED NBSP (U+00A0) or U+2028 before an entry name merges into the operand and silently renames it — probe-verified: the child then carries \xa0KISS_AUDIT=... with KISS_AUDIT itself unset (glued to CI= the exact inversion this PR fixes returns; to FOOTPRINT_ENFORCE= enforcement degrades to advisory). The full 215-test suite stays green under both mutants, and the previously suggested [^\S\n]* does NOT close the Unicode shape — it still matches NBSP/U+2028 — only [ \t]* closes both (fix-flip verified: mutants red, baseline 215/215 green). The sibling upsert pin (~line 12800) uses the same separator shape; the same tightening applies when it is next touched.
Witness:
blank_line: all4_green_under_HEAD_pin=true, all4_green_under_FIXED_pin=false
harm run — child env had MUTANT_PLANT: YES (isolation broken), step exit 0
nbsp_glued: all4_green_under_HEAD_pin=true
child env bytes 302 240 K I S S _ A U D I T = f a l s e, real KISS_AUDIT absent
full suite under both mutants: Tests 215 passed (215)
| .join(' \\\\\n\\s*'), | |
| .join(' \\\\\n[ \\t]*'), |
中文说明
gateLaunchPin 的 token 间分隔符把 JS \s 类的每个成员都当作无害缩进,但 bash 只认空格/制表符/换行为空白——两种改动形态能让全部四条循环断言保持绿色,同时击穿这个 pin 本要保障的隔离。在两条续行条目之间插入空行会把启动拆成两条命令:落单的 env -i 打印后以 0 退出,第二条命令(... bash --norc run-autofix-review-verification.sh)带着完整的父步骤环境运行——隔离消失,且 NODE_OPTIONS 是 gate 脚本不会剥离的活植入通道。在条目前粘贴 NBSP(U+00A0)或 U+2028 会并入操作数并悄悄将其改名——探针验证:子进程随后携带 \xa0KISS_AUDIT=... 而 KISS_AUDIT 本身未设置(粘到 CI= 上则本 PR 要修复的语义反转复现;粘到 FOOTPRINT_ENFORCE= 上则执行降级为 advisory)。两种突变体下完整 215 测试套件均保持绿色;此前提出的 [^\S\n]* 无法关闭 Unicode 形态(它仍匹配 NBSP/U+2028),只有 [ \t]* 能同时关闭两者(修复翻转验证:突变体变红,基线 215/215 绿)。sibling upsert pin(约 12800 行)使用同样的分隔符形态,下次触及该处时应做同样收紧。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| // check are pinned with it (R3-1, R2-2). | ||
| const gateLaunchPin = new RegExp( | ||
| [ | ||
| 'LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH=', |
There was a problem hiding this comment.
[Suggestion] gateLaunchPin is an unanchored substring match whose surroundings nothing bounds, and the literal bash --norc count is the only launch-shape guard — three probe-verified shapes keep the full 215-test suite green while defeating the gate. (a) A PREFIX command word before the LD_* head (e.g. printf 'outcome=pass\n' >> "$GITHUB_OUTPUT" \) turns the whole chain into argv — the gate never executes, the forged outcome is written, the step exits 0. (b) A command APPENDED after the launch line executes whenever the gate exits 0 and overwrites the gate's documented last-write-wins step outputs (verified_head / audit_verdict / preexisting), which drive the repair trigger and push routing. (c) A DEMOTED ARM — the pinned chain wrapped in if false; then ... fi plus an early if true; then bash -c '...'; exit 0; fi (a --n""orc spelling keeps the literal count at 1) — runs the runner with NO env -i isolation while all four assertions and all 11 other presence assertions stay green. The tripwire comment claims it catches a demoted pinned block; it catches only demotions that re-launch via the literal bash --norc. Fix (flip-verified: each mutant 1 failed | 214 passed, baseline 215/215 green) — anchor the chain head to a statement start and assert the launch ends the step's run body (the extraction lookahead cuts at the next - name:, and the launch line ends the slice modulo one trailing LF, so the tail check is cut-point-safe); this also defeats shape (c), whose if false; then prefix and trailing fi both break the anchored match. The same "anchor on the launch LINE" doctrine is already applied by the upsert audit at ~12813.
Witness:
prefix_forgery: {"step_exit":0,"gate_script_ran":false,"GITHUB_OUTPUT_content":"outcome=pass"}
demoted_arm: {"step_exit":0,"gate_script_ran":true,"child_env_has_plant":true}
each mutant: Tests 215 passed (215) on the unmodified PR
const gateLaunchPin = new RegExp(
'(?:^|\\n)[ \\t]*' +
[
'LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH=',
/* …remaining tokens unchanged… */
]
.map((token) => token.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))
.join(' \\\\\n[ \\t]*'),
);
// …and in the loop, after toMatch, assert the launch ends the run body:
const launchMatch = step.match(gateLaunchPin);
expect(step.slice(launchMatch.index + launchMatch[0].length).trim()).toBe('');中文说明
gateLaunchPin 是无锚定的子串匹配,其前后内容不受任何约束,而字面 bash --norc 计数是唯一的启动形态守卫——三种经探针验证的形态能让完整 215 测试套件保持绿色同时击穿 gate。(a) 在 LD_* 头部之前加前缀命令字(如 printf 'outcome=pass\n' >> "$GITHUB_OUTPUT" \)会把整条链变成该命令的 argv——gate 从不执行、伪造的 outcome 被写入、步骤以 0 退出。(b) 在启动行之后追加的命令会在 gate 以 0 退出时执行,并覆盖 gate 文档化的 last-write-wins 步骤输出(verified_head / audit_verdict / preexisting),这些输出驱动 repair 触发与 push 路由。(c) 降级臂——把被钉住的链包进 if false; then ... fi,并在前面加 if true; then bash -c '...'; exit 0; fi(用 --n""orc 拼写让字面计数保持为 1)——在没有 env -i 隔离的情况下运行 runner,而全部四条断言及其余 11 条存在性断言保持绿色。绊线注释声称能捕获被降级的被钉块,实际只能捕获经由字面 bash --norc 重新启动的降级。修复(翻转验证:每种突变体 1 failed | 214 passed,基线 215/215 绿)——把链头锚定到语句起始,并断言启动行结束步骤运行体(提取前瞻在下一个 - name: 处截断,且启动行就是切片末尾、仅差一个行尾 LF,尾部检查对截断点是安全的);这同时击败形态 (c),其 if false; then 前缀与行尾 fi 都会破坏锚定匹配。同样的「锚定到启动行」纪律在 ~12813 的 upsert 审计中已有应用。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| reviewVerificationGateStep, | ||
| repairVerificationGateStep, | ||
| ]) { | ||
| expect(step).toMatch(gateLaunchPin); |
There was a problem hiding this comment.
[Suggestion] This block pins the chain "through the digest-verified script", but the pre-launch digest check itself is pinned only by a workflow-wide toHaveLength(2) count (line ~10529, regex not end-anchored) — never per gate step, never ordered before the launch, never guarded against weakening. Probe-verified mutants, baseline all-green and each mutant all-green against every pin touching the gate steps: relocating both digest lines out of the gate steps (or after the launch line) — the launch executes the staged script with no pre-execution verification, and RUNNER_TEMP is branch/agent-writable between staging and the gate per the stage step's own comment, with both gate steps continue-on-error so forged $GITHUB_OUTPUT verdicts survive; and appending || true to a digest line — the count regex still matches, and under the runner's bash -e the mismatch no longer aborts, so a tampered staged script launches. This is the exact mutant the resanitize sibling's comment (~10354) records as turning "the tamper gate into a decorative no-op while presence/order assertions stayed green" — that sibling pins all three properties (whole-line toContain + indexOf ordering before the call + no-|| true); this new block should mirror the doctrine in its own loop. The relocation and || true shapes also survive the head/tail anchoring proposed in the other thread (the count pin is workflow-global, the weakening intra-line).
Witness:
M1a (digest after launch): exit=0 | Tests 215 passed
M1b (digest removed from gates): exit=0 | Tests 215 passed
M2 (|| true appended): exit=0 | Tests 215 passed
with mirrored doctrine: M1a "expected 3407 to be less than 3333", M1b/M2 red, baseline green
| expect(step).toMatch(gateLaunchPin); | |
| expect(step).toMatch(gateLaunchPin); | |
| expect(step).toContain('echo "${VERIFY_RUNNER_SHA256} ${RUNNER_TEMP}/run-autofix-review-verification.sh" | sha256sum -c - > /dev/null'); | |
| expect( | |
| step.indexOf('sha256sum -c - > /dev/null'), | |
| ).toBeLessThan(step.search(gateLaunchPin)); | |
| expect(step).not.toMatch(/sha256sum -c[^\n]*\|\| true/); |
中文说明
本块把链钉到「digest 校验过的脚本」,但启动前的 digest 检查本身只由一个全 workflow 的 toHaveLength(2) 计数 pin(~10529 行,正则无尾锚定)——既不按 gate 步骤钉,也不钉其在启动之前的顺序,更不防削弱。探针验证的突变体:基线全绿,且每个突变体对触及 gate 步骤的所有 pin 均保持绿色。把两条 digest 行移出 gate 步骤(或挪到启动行之后)——启动会在无执行前校验的情况下运行暂存脚本,而按 stage 步骤自己的注释,RUNNER_TEMP 在 staging 与 gate 之间可被分支/agent 代码写入,两个 gate 步骤又都是 continue-on-error,伪造的 $GITHUB_OUTPUT verdict 会留存;在 digest 行尾追加 || true——计数正则仍能匹配,而在 runner 的 bash -e 下不匹配不再中止步骤,被篡改的暂存脚本照样启动。这正是 resanitize sibling 注释(~10354)记录过的突变体——「防篡改 gate 变成装饰性空操作,而存在/顺序断言保持绿色」;该 sibling 钉住了全部三个属性(整行 toContain + 调用前 indexOf 排序 + 禁止 || true),本新块应在自己的循环里镜像该纪律。迁移与 || true 形态在另一线程提议的头/尾锚定下依然存活(计数 pin 是全 workflow 的,削弱发生在行内)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| // Exactly one launch per step: a second, unpinned `bash --norc` (the | ||
| // pinned block demoted into a never-run arm) must fail here (R2-1). | ||
| expect((step.match(/bash --norc/g) ?? []).length).toBe(1); | ||
| expect(step).toContain('unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH'); |
There was a problem hiding this comment.
[Suggestion] toContain('unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH') and toContain('export PATH="${TRUSTED_PATH}"') match inside a # comment or after the launch — nothing anchors them to an executed position, though the block's comment claims the body-side unset and PATH export "are pinned with it". Probe-verified mutants, each all-green against every pin touching the gate steps: commenting out the unset line — a $GITHUB_ENV plant of LD_PRELOAD=/tmp/evil.so (the stated threat model; the step env block pins BASH_ENV/SHELLOPTS but no LD_* variable) survives into the gate step's parent shell and loads into the first exec — the digest check's sha256sum — code execution before the env -i child launches, able to write forged verdict outputs; commenting out the PATH export — a planted PATH resolves sha256sum to an attacker binary (same forgery outcome); relocating both lines after the launch. The launch's own LD_* prefix + env -i keep the child-side isolation pins honest; the unprotected exec is the parent-shell digest check. The PAT-step preamble is held to a stricter bar (its adjacency regex fails if its unset line is commented). Fix (flip-verified: adjacency flips the comment-out mutants red, adding the ordering assertion also flips the relocation mutant, baseline green) — anchor the pair to statement position and order it before the digest check.
Witness:
M3 (unset commented): exit=0 | Tests 215 passed
M4 (export commented): exit=0 | Tests 215 passed
M5 (pair relocated later): exit=0 | Tests 215 passed
run-body order verified at 5205-5207: export -> unset -> digest check -> launch
anchored adjacency: M3/M4 red; + ordering: M5 "expected 3476 to be less than 2357"
| expect(step).toContain('unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH'); | |
| expect(step).toMatch(/(?:^|\n)[ \t]*export PATH="${TRUSTED_PATH}"\n[ \t]*unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH/); | |
| expect( | |
| step.indexOf('unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH'), | |
| ).toBeLessThan(step.indexOf('echo "${VERIFY_RUNNER_SHA256}')); |
中文说明
toContain('unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH') 与 toContain('export PATH="${TRUSTED_PATH}"') 在 # 注释内或启动之后同样匹配——没有任何东西把它们锚定到被执行的位置,尽管本块注释声称体内 unset 与 PATH 导出「一并被钉住」。探针验证的突变体,每一个对触及 gate 步骤的所有 pin 均保持绿色:注释掉 unset 行——按既有威胁模型由更早 shell 级步骤通过 $GITHUB_ENV 植入的 LD_PRELOAD=/tmp/evil.so(步骤 env 块钉住了 BASH_ENV/SHELLOPTS,但没有钉任何 LD_* 变量)会存活进 gate 步骤的父 shell,并加载进第一个 exec——digest 检查的 sha256sum——在 env -i 子进程启动之前实现代码执行,可写入伪造的 verdict 输出;注释掉 PATH 导出——植入的 PATH 会把 sha256sum 解析为攻击者二进制(同样的伪造结果);把两行挪到启动之后。启动自身的 LD_* 前缀 + env -i 使子进程侧隔离 pin 依然成立;失去保护的是父 shell 中的 digest 检查 exec。PAT 步骤前缀被要求更严格(其邻接正则在 unset 行被注释时会失败)。修复(翻转验证:邻接锚定使注释突变体变红,再加排序断言使迁移突变体也变红,基线绿)——把这对语句锚定到语句位置,并钉住其先于 digest 检查的顺序。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #9649 review feedback (round 4 findings)All four inline findings from the automated review are implemented in one Findings and decisions
Changes
Conflict notes
Not touched this round
Verification
中文说明Autofix 轮次总结 — PR #9649 审查反馈(第 4 轮发现)自动审查的 4 条行内发现已在一次提交中全部实现。四条均针对 发现与处置
改动
冲突说明
本轮未触及
验证
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
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 5": full vitest run of scripts/tests/qwen-autofix-workflow.test.js (no node_modules in the review worktree; a monorepo install exceeded the tool budget) — substitut….
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 5, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:5222 — [review] CI passes through the gate allowlist without the step-level pin its siblings get — a $GITHUB_ENV plant of CI=false re-inverts the CI semantics this PR restores
中文说明
未探索到全部深度(达到工具调用预算):"agent 5":full vitest run of scripts/tests/qwen-autofix-workflow.test.js (no node_modules in the review worktree; a monorepo install exceeded the tool budget) — substitut…。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🔀 Base updated: red check(s) [Dependency CVE audit] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Dependency CVE audit] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
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:5225 — [review] CI passes through the gate allowlist without a step-level pin — a $GITHUB_ENV plant of CI=false flows through "${CI:-true}" and re-inverts the CI semantics this PR restoresscripts/tests/qwen-autofix-workflow.test.js:11278 — [review] statement-list pin slices from indexOf('run: |-') without the -1 guard the file's own doctrine mandates
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——评审时间预算不足,未能开始第 6 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| .map((line) => line.trim()) | ||
| .filter((line) => line !== '' && !line.startsWith('#')), |
There was a problem hiding this comment.
[Critical] The statement-list pin's comment filter classifies lines differently from bash. JS trim() strips Unicode whitespace (NBSP, U+2000–U+200A, U+202F, U+205F, U+3000, U+2028) before startsWith('#') runs, but bash recognizes only space/tab/newline as whitespace — so a line beginning with NBSP followed by # is a comment to the pin yet a command word to bash. A tampered workflow — the very threat this pin exists for; the design comments name workflow edits by a shell-capable/prompt-injected round — can insert one such line into either gate's run body: the adjacency regex spans only LD_*→bash --norc, the filtered statement list still equals gateBodyStatements, the bash --norc count stays 1, so all three new pins stay green while bash executes the line in the PARENT shell — before the digest check and the env -i child — with the full step environment. That is exactly the “command inserted around the launch” shape this pin's own comment claims to close (“each adds, drops, reorders, or renames a statement here”); the statement list never sees the smuggled line because trim strips the leading NBSP.
Witness (probe at this commit):
tamper: line " \u00a0# x" inserted before `export PATH=` in 'Verification gate' run body
PR pins vs tamper: Test Files 1 passed (1) / Tests 215 passed (215) <- smuggle invisible to all pins
bash --norc: line 1: \u00a0#: command not found (exit 127)
bash -e variant: PWNED_NO_PLANTED_BINARY (exit 0, no planted binary)
with the fix below: AssertionError naming the smuggled line at test.js:11282
With a $GITHUB_ENV-planted PATH prefix and a file named \xc2\xa0# dropped by earlier branch code, the smuggled line is arbitrary code before any digest check runs. Strip only ASCII whitespace before the comment/blank check so any line carrying non-ASCII “whitespace” survives into the statement list and fails the exact-match — closing the whole Unicode class at once (probe-verified red on the smuggle, green on the clean workflow):
| .map((line) => line.trim()) | |
| .filter((line) => line !== '' && !line.startsWith('#')), | |
| .map((line) => line.replace(/^[ \t]+|[ \t]+$/g, '')) | |
| .filter((line) => line !== '' && !line.startsWith('#')), |
中文说明
[Critical] statement-list pin 的注释过滤器对行的分类与 bash 不一致。JS trim() 会先剥掉 Unicode 空白(NBSP、U+2000–U+200A、U+202F、U+205F、U+3000、U+2028)再做 startsWith('#') 判断,而 bash 只认空格/制表符/换行为空白——因此以 NBSP 开头、后跟 # 的行对 pin 是注释,对 bash 却是命令词。被篡改的 workflow(正是本 pin 要防的威胁:设计注释明确点名 shell 级/被 prompt 注入的 round 可能编辑 workflow)可以向任一 gate 的 run 体插入这样一行:邻接正则只覆盖 LD_*→bash --norc,过滤后的语句列表仍等于 gateBodyStatements,bash --norc 计数仍为 1,三个新 pin 全部保持绿色,而 bash 会在父 shell 中执行该行——先于 digest 检查与 env -i 子进程——并携带完整步骤环境。这正是本 pin 自身注释声称已关闭的「在启动周围插入命令」形态(「每一种都会在此增加、删除、重排或重命名一条语句」);语句列表永远看不到被走私的行,因为 trim 剥掉了行首 NBSP。
见证(本提交上的探针):在 'Verification gate' run 体 export PATH= 之前插入 " \u00a0# x";PR pin 对篡改仍 Tests 215 passed (215)(走私对所有 pin 不可见);bash --norc 报 line 1: \u00a0#: command not found(exit 127);bash -e 变体输出 PWNED_NO_PLANTED_BINARY(exit 0,无需植入二进制);应用下方修复后:AssertionError 在 test.js:11282 指明被走私的行。
配合 $GITHUB_ENV 植入的 PATH 前缀与更早的分支代码放置的名为 \xc2\xa0# 的文件,被走私的行就是先于任何 digest 检查执行的任意代码。修复:注释/空行判断前只剥 ASCII 空白,使携带非 ASCII「空白」的行进入语句列表并让精确匹配失败——一次关闭整个 Unicode 类别(探针验证:对篡改变红,对干净 workflow 保持绿色)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| // on the LD_* prefix pins the one channel env -i cannot block; the | ||
| // body-side unset and PATH export that protect the pre-launch digest | ||
| // check are pinned with it (R3-1, R2-2). The shapes AROUND the chain |
There was a problem hiding this comment.
[Critical] The protection this rationale claims for the pre-launch digest check does not hold against a startup-time LD_* plant. A $GITHUB_ENV-planted LD_PRELOAD is loaded by ld.so into the gate step's bash at process STARTUP — before line 1 of the body — and the step-level env blocks pin BASH_ENV/SHELLOPTS empty (with the doctrine “a body-side unset is one hop late”) while leaving LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH unpinned. The pinned body-side unset removes the variable for children but cannot unload the library from the already-running bash; an execve/fork/waitpid hook inside it forges the digest-check result and can rewrite what the env -i … bash --norc launch execs — a forged outcome=fixed reaches the PAT push while every new pin stays green. actions/runner accepts the plant (FileCommandManager.cs blocklist is NODE_OPTIONS only). This contradicts the added claim above: “a planted LD_PRELOAD or PATH reached the sha256sum exec (R4-4)” is named as the shape the pinned defenses close, yet the startup-plant entrance still reaches it.
Witness (probe at this commit, gate body's exact defended sequence):
Arm B (planted): [preload] constructor ran in pid 3833452 (BEFORE body line 1)
-> [body] unset done; LD_PRELOAD in env now: '<absent>'
-> [preload] parent-bash execve hook fired for: /usr/bin/sha256sum
-> DIGEST_CHECK_EXIT=0 (wrong digest - tamper passes)
Arm A (clean) / Arm C (empty LD_PRELOAD, as a step-level pin delivers):
sha256sum: WARNING: 1 computed checksum did NOT match; DIGEST_CHECK_EXIT=1
Fix: mirror the BASH_ENV/SHELLOPTS doctrine already in both gate steps' env blocks — pin LD_PRELOAD: '', LD_AUDIT: '', LD_LIBRARY_PATH: '' at step level on both gates (empty values are ignored by ld.so; probe-validated), and extend this pin block to assert those step-env entries.
中文说明
[Critical] 该理由注释声称为启动前 digest 检查提供的保护,在启动时 LD_* 植入面前不成立。经 $GITHUB_ENV 植入的 LD_PRELOAD 会被 ld.so 在 gate 步骤 bash 的进程启动时加载——先于体内第 1 行——而 step 级 env 块把 BASH_ENV/SHELLOPTS 钉为空(其学说正是「体内 unset 晚了一跳」),却未钉住 LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH。被钉住的体内 unset 能为子进程移除该变量,但无法从已在运行的 bash 中卸载已加载的库;其中的 execve/fork/waitpid 钩子可以伪造 digest 检查结果,并改写 env -i … bash --norc 启动实际 exec 的内容——伪造的 outcome=fixed 到达 PAT push,而所有新 pin 保持绿色。actions/runner 接受该植入(FileCommandManager.cs 的屏蔽名单仅 NODE_OPTIONS)。这与上方新增的声称相矛盾:注释点名「植入的 LD_PRELOAD 或 PATH 到达 sha256sum exec(R4-4)」是被钉住的防御已关闭的形态,但启动时植入入口仍能到达。
见证(本提交上的探针,按 gate 体的原样防御序列):Arm B(植入):[preload] constructor ran in pid 3833452 (BEFORE body line 1) → [body] unset done; LD_PRELOAD in env now: '<absent>' → [preload] parent-bash execve hook fired for: /usr/bin/sha256sum → DIGEST_CHECK_EXIT=0(错误摘要也通过——篡改得逎);Arm A(干净)/ Arm C(空 LD_PRELOAD,即 step 级 pin 交付的值):sha256sum: WARNING: 1 computed checksum did NOT match; DIGEST_CHECK_EXIT=1。
修复:按两个 gate 步骤 env 块中已有的 BASH_ENV/SHELLOPTS 学说,在两个 gate 上 step 级钉住 LD_PRELOAD: ''、LD_AUDIT: ''、LD_LIBRARY_PATH: ''(空值被 ld.so 忽略,已探针验证),并扩展本 pin 块断言这些 step-env 条目。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| WORKDIR="${WORKDIR}" \ | ||
| BRANCH="${BRANCH}" \ | ||
| GITHUB_OUTPUT="${GITHUB_OUTPUT}" \ |
There was a problem hiding this comment.
[Critical] The allowlist this PR's new pin canonizes as the SANCTIONED chain re-injects path/state variables (WORKDIR, and with it RUNNER_TEMP, HOME, BRANCH) verbatim from the step environment into the clean child, and none of them is pinned at step level — unlike TRUSTED_PATH/VERIFY_RUNNER_SHA256/FOOTPRINT_ENFORCE/KISS_AUDIT in the same env blocks. actions/runner settles the plant premise: JobExtension.cs seeds runner defaults, merges workflow/job env into the same dict last-write-wins, and FileCommandManager.cs applies $GITHUB_ENV writes AFTER both — overriding runner defaults and job env for subsequent steps, blocklist NODE_OPTIONS only. The digest check then reads its PATH from the planted env while its oracle is expression-context, so it verifies whichever file the attacker points at.
Failure: branch code writes RUNNER_TEMP=/tmp/decoy to $GITHUB_ENV and places a byte-identical copy of the trusted runner script at the decoy. In the gate, sha256sum -c verifies ${RUNNER_TEMP}/run-autofix-review-verification.sh at the decoy path — passes — and the child execs the decoy with RUNNER_TEMP=/tmp/decoy, resolving check-settings-schema.sh, check-autofix-contracts.sh, resolve-owning-packages.sh and GIT_CONFIG_GLOBAL from the decoy: attacker code executes inside the otherwise-clean child and writes the verdict that gates the PAT push. WORKDIR-plant variant: the gate builds/tests a pre-greened decoy tree and the push steps consume the same planted value. All pins this PR adds stay green (allowlist text unchanged).
Witness: not run on a live GitHub runner — merge premise settled against the actions/runner implementation (JobExtension.cs InitializeJob seeds runner defaults then merges workflow/job env into Global.EnvironmentVariables last-write-wins; FileCommandManager.cs applies $GITHUB_ENV after both, blocklist {NODE_OPTIONS} only; ExecutionContext.SetEnvContext checks only NotNullOrEmpty(name)); value-source trace through this workflow is deterministic given it
Fix: pin the path variables from trusted expression context at step level, mirroring TRUSTED_PATH/VERIFY_RUNNER_SHA256, in both gate steps' env blocks, e.g.:
RUNNER_TEMP: '${{ runner.temp }}'
WORKDIR: '/tmp/autofix-review-${{ matrix.target.pr }}'and extend the new pins to assert the step-env entries.
中文说明
[Critical] 本 PR 新 pin 宣告为 SANCTIONED 链的允许列表,把路径/状态变量(WORKDIR,以及 RUNNER_TEMP、HOME、BRANCH)逐字从步骤环境重新注入干净子进程,而它们都没有 step 级钉住——同一个 env 块里的 TRUSTED_PATH/VERIFY_RUNNER_SHA256/FOOTPRINT_ENFORCE/KISS_AUDIT 却有。actions/runner 确认了植入前提:JobExtension.cs 先播下 runner 默认值,再以后写覆盖方式把 workflow/job env 合入同一字典;FileCommandManager.cs 在两者之后应用 $GITHUB_ENV 写入——对后续步骤覆盖 runner 默认与 job env,屏蔽名单仅 NODE_OPTIONS。digest 检查随后从被植入的环境读取路径,而其预期摘要来自表达式上下文,因此它会校验攻击者指向的任何文件。
失败路径:分支代码向 $GITHUB_ENV 写入 RUNNER_TEMP=/tmp/decoy,并在该假目录放置与受信任 runner 脚本逐字节一致的副本。gate 中 sha256sum -c 在假路径上校验 ${RUNNER_TEMP}/run-autofix-review-verification.sh——通过——子进程以 RUNNER_TEMP=/tmp/decoy exec 该副本,从假目录解析 check-settings-schema.sh、check-autofix-contracts.sh、resolve-owning-packages.sh 与 GIT_CONFIG_GLOBAL:攻击者代码在本应干净的子进程内执行,并写入决定 PAT push 的 verdict。WORKDIR 植入变体:gate 在预先绿化的假树上 build/test,push 步骤消费同一植入值。本 PR 新增的所有 pin 保持绿色(允许列表文本未变)。
见证:未在真实 GitHub runner 上运行——合并前提依据 actions/runner 实现确认(JobExtension.cs InitializeJob 先播下 runner 默认值再以后写覆盖合并 workflow/job env 到 Global.EnvironmentVariables;FileCommandManager.cs 在两者之后应用 $GITHUB_ENV,屏蔽名单仅 {NODE_OPTIONS};ExecutionContext.SetEnvContext 仅检查 NotNullOrEmpty(name));给定该前提,本 workflow 内的取值源追踪是确定性的。
修复:仿照 TRUSTED_PATH/VERIFY_RUNNER_SHA256,在两个 gate 步骤的 env 块中从受信任表达式上下文 step 级钉住路径变量(如 RUNNER_TEMP: '${{ runner.temp }}'、WORKDIR: '/tmp/autofix-review-${{ matrix.target.pr }}'),并扩展新 pin 断言这些 step-env 条目。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const gateDigestCheck = | ||
| 'echo "${VERIFY_RUNNER_SHA256} ${RUNNER_TEMP}/run-autofix-review-verification.sh" | sha256sum -c - > /dev/null'; |
There was a problem hiding this comment.
[Critical] This pin canonizes the bare sha256sum spelling — the function-shadowable form. Bash imports BASH_FUNC_<name>%% environment variables as functions at startup even under --norc, and function lookup precedes builtins and PATH, so the pinned export PATH="${TRUSTED_PATH}" and unset LD_* (variables, not functions) give no protection: a BASH_FUNC_sha256sum%%=() { return 0; } planted via $GITHUB_ENV makes the pre-launch digest check pass for any digest on any file, and the env -i child then executes the tampered staged runner, forging the verdict that gates the PAT push — all three new pins stay green (they assert static workflow text). The plant flows through: actions/runner FileCommandManager.cs splits env-file lines on the first = with no identifier validation, blocklist NODE_OPTIONS only. Body-side removal is unavailable: unset/export are regular builtins, themselves BASH_FUNC-shadowable (probe). The sibling workflow qwen-triage.yml documents and defends this exact channel at step level (“BASH_FUNC_%% env entries are imported as shell functions BEFORE this body runs … No in-script purge is safe”); the two gate steps have none of that. The added rationale's unqualified claim that the body-side defenses “protect the pre-launch digest check” stands falsified for this channel.
Witness (probe at this commit):
plant active, gate body + runner-exact flags (bash --noprofile --norc -e -u -o pipefail):
sha256sum is a function
GATE BODY SURVIVED digest check, exit=0 (wrong digest, nonexistent file)
control (no plant): digest-check exit=1
body-side purge unavailable: BASH_FUNC_unset%% -> "HIJACKED-unset"; BASH_FUNC_export%% -> "HIJACKED-export"
fix probes: /usr/bin/sha256sum -c - under plant -> exit 1 (slash-bearing words skip function lookup);
BASH_FUNC_sha256sum%%='' imports no function
Fix: slash-path the digest line in both gate bodies (/usr/bin/sha256sum -c - — a slash-bearing word skips function/alias lookup, the doctrine this file already pins for /usr/bin/env at line ~12842) and/or pin BASH_FUNC_sha256sum%%: '' at step level on both gates (probe: an empty-valued BASH_FUNC entry imports no function); update this gateDigestCheck constant to match. The fix spans the workflow bodies and this pin, so no one-click suggestion is attached.
中文说明
[Critical] 本 pin 把裸的 sha256sum 拼写固化了下来——那正是可被函数遮蔽的形式。bash 即使在 --norc 下也会在启动时把 BASH_FUNC_<name>%% 环境变量导入为函数,且函数查找先于内建命令与 PATH,因此被钉住的 export PATH="${TRUSTED_PATH}" 与 unset LD_*(变量,非函数)不提供任何保护:经 $GITHUB_ENV 植入 BASH_FUNC_sha256sum%%=() { return 0; } 后,启动前 digest 检查对任意文件的任意摘要都会通过,随后 env -i 子进程执行被篡改的阶段化 runner,伪造决定 PAT push 的 verdict——三个新 pin 全部保持绿色(它们断言的是静态 workflow 文本)。植入可以透过:actions/runner FileCommandManager.cs 按第一个 = 切分 env 文件行、不做标识符校验,屏蔽名单仅 NODE_OPTIONS。体内清除不可行:unset/export 是普通内建命令,本身也可被 BASH_FUNC 遮蔽(探针证实)。sibling workflow qwen-triage.yml 记录并在 step 级防御了完全相同的通道(「BASH_FUNC_%% 环境条目在本体运行前被导入为 shell 函数…任何脚本内清除都不安全」);两个 gate 步骤没有任何这样的防御。新增理由注释中不加限定的「体内防御保护启动前 digest 检查」声称,在该通道上被证伪。
见证(本提交上的探针):植入生效、按 gate 体原样与 runner 相同的标志(bash --noprofile --norc -e -u -o pipefail):sha256sum is a function;GATE BODY SURVIVED digest check, exit=0(错误摘要、不存在的文件);对照(无植入):digest 检查 exit=1;体内清除不可用:BASH_FUNC_unset%% → "HIJACKED-unset";BASH_FUNC_export%% → "HIJACKED-export";修复探针:植入下 /usr/bin/sha256sum -c - → exit 1(带斜杠的词跳过函数查找);BASH_FUNC_sha256sum%%='' 不导入任何函数。
修复:在两个 gate 体中给 digest 行加斜杠路径(/usr/bin/sha256sum -c -——带斜杠的词跳过函数/别名查找,即本文件约 12842 行已为 /usr/bin/env 钉住的学说),和/或在两个 gate 上 step 级钉住 BASH_FUNC_sha256sum%%: ''(探针:空值的 BASH_FUNC 条目不导入函数);同步更新本 gateDigestCheck 常量。修复跨越 workflow 体与本 pin,故不附一键 suggestion。
— qwen3.8-max via Qwen Code /review (v0.21.15)
…ck (QwenLM#9649) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9649 (commit
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
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 7, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:5248 (+2 locations) — [review] CI passes through the gate allowlist without the step-level pin its siblings get — a $GITHUB_ENV plant of any non-empty value other than true re-inverts the CI semantics this…scripts/tests/qwen-autofix-workflow.test.js:19432 — [probe] the new step-level env pins are asserted only with bare toContain — a commented-out entry survives the whole 215-test suite
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 7 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no actionNo actionable feedback arrived for this round, so no changes were made and nothing was committed.
The PR head remains at the last committed state ( 中文说明Autofix 审查轮次 —— 无操作本轮没有收到任何可执行的反馈,因此未做任何改动,也没有提交任何内容。
PR 的 head 仍停留在最后一次提交的状态( 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: 446 passed · 0 failed · 446 total Flakiness gate: 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:446 通过 · 0 失败 · 446 总计 抖动门: Verification reportPR #9649 deep verification —
|
| cell | oracle | base (HEAD^1) |
head (PR) |
|---|---|---|---|
| benign, both launches (×2 steps/arm) | child env of the env -i child |
8 allowlisted vars, CI absent |
9 vars, CI=true |
hostile $GITHUB_ENV plants |
plants in child env | none leak (env -i pre-existing) | none leak |
BASH_FUNC_sha256sum%% shadow + mid-run overwrite of staged runner |
forged digest passes? runner executes? | passes, attacker runner EXECUTES | aborts exit 1, nothing executes |
RUNNER_TEMP plant pointing at decoy dir |
which runner runs | decoy runner executes | pin beats plant, fail closed |
LD_PRELOAD plant (compiled marker .so) |
marker created at parent-bash startup | mapped before line 1 | step-level '' pin blocks it |
Per-arm scripted checks: base 16/16, head 16/16 as expected (logs/01-replay-{base,head}.txt).
Behavior probe (harness 06-behavior-assert.mjs over logs/02-authdialog-*.txt): the exact gate launch command line (/usr/bin/env -i PATH=… HOME=… … bash --norc -c '…') running vitest run src/ui/auth/AuthDialog.test.tsx:
| cell | result |
|---|---|
head gate env (CI=true added) |
7 passed | 18 skipped (25) — flaky TUI tests never run |
base gate env (no CI), run 1 |
24✓ 1× (summary line lost to a harness coverage-dir collision; per-test lines intact) |
| base gate env, solo rerun | 1 failed | 24 passed (25), failing test = drives API key provider steps from endpoint options metadata at 5239 ms |
The failing test is registered via itWhenTuiInputReliable (= it.skip when process.env['CI'] === 'true'), one of exactly 18 such tests (7 unconditional + 18 = 25 total, matching the PR's numbers). Negative controls: the staged gate runner neither sets CI itself nor strips it (strip_runner_channels only drops GITHUB_* channels), so the allowlist line is the only carrier. Witness: 02-authdialog-skip-flip-base-vs-head.png.
Secondary claim 1 — contract pins hold the allowlist. Mutation matrix (harness 05-mutation-matrix.mjs, full 215-test suite per mutation, in-place mutation + sha256-verified restore; witness 03-mutation-matrix-as-printed.png):
| mutation | expected | got |
|---|---|---|
| pristine head | green (215/215) | green |
| A/A base files | green | green (also 215/215 — PR adds no new tests, pins live inside existing ones) |
M1 drop CI from first launch |
red | red — 1 failed | 214 passed, chain pin in allows non-package fixes after deterministic verification |
M2 drop CI from both launches |
red | red — 1 failed | 214 passed, same pin |
M3 duplicate CI symmetrically |
red | red — 1 failed | 214 passed, chain pin (adjacency broken) |
M4 comment out CI (token text preserved) |
red | red — chain pin + statement list (comment filtered, entry missing) |
| M5 blank line inside the continuation chain | red | red — chain pin only: exactly one AssertionError (the chain regex); the statement list stays green because blank lines are filtered, proving the two pins are complementary |
M6 NBSP smuggled into the CI operand |
red | red — chain pin trips first; the statement list's NBSP visibility is pinned by the test's own unit check `gateBodyStatementsOf('run: |
M7 inject lowercase npm_config_registry into ONE deferred-upsert launch |
red | red — widened multiset regex names npm_config_registry (childCore equality also trips) |
M8 M7 injection + regex reverted to [A-Z_]… |
(expected green) | red via the pre-existing childCore cross-launch equality — see Corrections |
| M8b SYMMETRIC lowercase injection + old regex | green | green — childCore holds, old regex blind: the real blind spot, proven |
| M9 SYMMETRIC lowercase injection + widened regex | red | red — the widening is what catches the symmetric case |
Secondary claim 2 — step-level startup-channel pins. Proven by the replay cells above (shadow/decoy/ldpre rows) and by the live actionlint/bash -n/shellcheck gates below; the pinned values are consistent with runtime: step-level WORKDIR/BRANCH pins are byte-identical to the review-address job-level definitions (workflow lines 3662/3664 vs 5209/5211 and 5442/5444), and RUNNER_TEMP: '${{ runner.temp }}' is the runner's own value.
Targeted gates
- Contract suite at head:
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js→215 passed (215), twice, plus the pristine control run (logs/04-contract-suite-head*.txt,logs/mutants/.mut-pristine.log). - bash -n on all 58 YAML-extracted
run:blocks: 0 syntax failures on head and on base (logs/03-bash-n-{head,base}.txt). - actionlint 1.7.12 (repo wrapper + direct): exit 0 on the head workflow; liveness proven by a planted scratch workflow whose undefined-step reference and shell syntax error were both reported (
logs/lint-live.txt). - shellcheck 0.11.0
-s bash --severity=erroron the 58 extracted head blocks: 0 failures (logs/lint-shellcheck.txt). - yamllint: not runnable in this container (see Not covered).
Corrections
- Commit
0fcdd5afmessage overstates the old regex's blindness. The message's probe claims injectingnpm_config_registry="…"into one launch left the old uppercase-only regex at 215/215 green. Measured here against the BASE suite (M8): the injection goes red even with the old regex, via the pre-existingchildCorecross-launch equality (expect(childCore(reviewAddressReportStep)).toBe(childCore(pushAndReportStep)), present in base). An asymmetric change was never invisible — childCore sees any text divergence between the two launches. The blind spot the widening actually closes is the symmetric lowercase addition: M8b (symmetric injection + old regex) is green, M9 (same injection, widened regex) is red. The widening is load-bearing exactly as the PR needs; only the probe narrative in the commit message is wrong. Severity: nit on the message, no code impact. - Confirmed, not corrected: the PR description's claim that the old gate-launch pin counted occurrences only is accurate — base had
workflow.split('/usr/bin/env -i \\').length - 1 === 4plustoContain('/usr/bin/env -i')presence checks, no allowlist content; a dropped variable did ship green. And the gate runner script neither sets nor stripsCI, so the allowlist line is its only carrier.
Findings
No blocking findings.
- (note, environmental) On this lane's container the contract suite exits 1 even when all 215 tests pass, due to a reproducible
[vitest-worker]: Timeout calling "onTaskUpdate"unhandled error. A/A control: the identical error appears running the BASE test file against the BASE workflow, so it predates this PR and is load/environment-related (shared runner). It does not affect the test results themselves, but a CI gate keyed on exit code rather than the summary would misread it. - (note) The PR's "1/25 flakes" rate was measured as 2/2 failures on this loaded shared runner (both base-env runs failed the same test at ~5.2 s). The flake probability is machine-load dependent; on this box the base gate env is not merely risky, it fails.
Not covered
- Per-commit attribution: checkout is depth 2 (merge commit + two parents only); the metadata lists 8 commits,
git rev-list HEAD^1..HEAD^2returns 1 under the shallow boundary. Verified the aggregateHEAD^1..HEADdiff only. - Replay calibration: first round, no token, no previously emitted gate artifact available in the sandbox — the replay is uncalibrated against a real production emission (a real gate's
$GITHUB_STEP_SUMMARY/verdict outputs would have calibrated it). It exercises the exact step text under the step's own shell contract, which is what a calibration artifact would compare against. - End-to-end gate run: the staged runner is stubbed in the replay;
run-autofix-review-verification.sh's internals (reject_fix logic, baseline A/B, build/typecheck/lint/test execution) were not run end to end (~22 min per pass, out of budget and orthogonal to this change). - yamllint: not installable in this container (
pip3permission-denied and nopipmodule forpython3 -m pip).bash -n(58/58 run blocks, both arms), actionlint (live-proven, clean), and shellcheck-s bash --severity=error(0/58) were run instead. - Real
$GITHUB_ENVacceptance ofBASH_FUNC_…%%names: the shadow cells inject the exported functions directly into the step environment (the bash-level mechanism the defense targets); whether Actions'$GITHUB_ENVappend syntax accepts such names in production is an Actions-side detail not testable here. - The two sibling deferred findings from feat(autofix): audit the approach instead of stopping on growth-budget breach #9262 (autofix gate: verdict/control-plane integrity never established against branch-code execution #9646, autofix conflict-park: wake set counts loop-generated check events as trusted wakes #9647) — tracked separately, out of this PR's scope by design.
Methodology
Environment: CI verify container (node:22-bookworm class), npm ci + npm run build pre-done at refs/pull/9649/merge; shared loaded runner. The A/B harness parses both gate steps out of each arm's YAML with the yaml library, evaluates every ${{ }} expression against a concrete context, materializes the step env with Actions precedence, and executes the run body via bash --noprofile --norc -eo pipefail with a stubbed staged runner whose child dumps its env (harness/01-gate-replay.mjs, logs 01-replay-{base,head}.txt). The behavior probe reuses the literal gate launch line with a vitest invocation in place of the staged runner (logs/02-authdialog-*.txt). The mutation matrix mutates the workflow/test files in place, runs the full 215-test contract suite per mutant, restores byte-identically (sha256-verified), and judges by the vitest summary, not the exit code (harness/05-mutation-matrix.mjs, logs 05-matrix-batch{1,2,3}.txt, per-mutant logs/mutants/). Workflow lint: every run: block extracted via YAML parse (harness/03-bash-n-extract.mjs) under bash -n; actionlint 1.7.12 + shellcheck 0.11.0 installed by the repo's scripts/lint.js --setup (sha256-verified), gate liveness proven with a planted violation before trusting the clean result. Evidence images rendered by scripts/verify-capture.mjs.
Flakiness gate log
rounds=5 files=1 skipped=0
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):
scripts/tests/qwen-autofix-workflow.test.js: FFFFF
verdict: consistent-fail
summary: 1 of 1 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 · scripts/tests/qwen-autofix-workflow.test.js: F (exit 1)
--- output tail · round 1 · scripts/tests/qwen-autofix-workflow.test.js ---
ed 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�[2m215 tests�[22m�[2m)�[22m�[33m 70120�[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 412�[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 569�[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 588�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the stale-duplicate revalidation, including the conflict-only transition �[33m 4413�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the eligibility recheck across lifecycle and label states �[33m 3267�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mreleases the dispatch-pending marker when the recheck discards a target �[33m 1034�[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 431�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally replays the takeover-command toggle across all four paths �[33m 3573�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally resets round counting at the latest takeover engage ack �[33m 819�[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 1643�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mrecovers transient forced-target reads and reports terminal takeover blocks �[33m 677�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mwires forced admission end to end: reader, classifier, permission gate, reporter �[33m 411�[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 1903�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mswitches to Critical-only feedback after five change rounds �[33m 382�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mturns a budget breach into a growth-audit round instead of a divergence stop �[33m 1671�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mposts a takeover milestone digest as rounds accumulate, with a residual bucket �[33m 607�[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 380�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mupserts deferred findings into a per-PR issue that survives the merge �[33m 8101�[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 2805�[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 676�[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 543�[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 857�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mbehaviorally posts the re-arm marker only after verifying the PAT identity �[33m 353�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mresolves only the review threads whose findings it implemented �[33m 2080�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22manswers the threads it leaves open, in those threads �[33m 617�[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 354�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mclassifies permanent API failures terminal and records the cause class �[33m 1051�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mpreserves an agent-written handoff when the budget kills qwen after it �[33m 647�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m growth-audit hardening: park wake set and verdict pipeline (round 3)�[2m > �[22mskips the scan stale-base update while a conflict handoff pends �[33m 530�[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 577�[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 1248�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mnever fires while the agent emits protocol events, however slowly �[33m 3254�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mnever fires while the agent talks on stderr only �[33m 3255�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m run-agent idle watchdog�[2m > �[22mdoes not treat an unterminated stdout byte stream as progress �[33m 797�[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 1545�[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 1249�[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�[32m215 passed�[39m�[22m�[90m (215)�[39m
�[2m Errors �[22m �[1m�[31m1 error�[39m�[22m
�[2m Start at �[22m 04:20:01
�[2m Duration �[22m 70.70s�[2m (transform 224ms, setup 18ms, collect 292ms, tests 70.12s, environment 0ms, prepare 70ms)�[22m
round 2 · scripts/tests/qwen-autofix-workflow.test.js: F (exit 1)
--- output tail · round 2 · scripts/tests/qwen-autofix-workflow.test.js ---
ed 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�[2m215 tests�[22m�[2m)�[22m�[33m 69317�[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 413�[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 573�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m qwen-autofix workflow�[2m > �[22mauto-reruns a check that died on infrastructure, onc
...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
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round: no action takenPR: #9649 ( Triage resultCritical-only mode is active in this counting window (5 change-producing rounds are complete), so non-Critical feedback is excluded from this round's actionable sections and remains open for human follow-up. After that filtering, every actionable section for this round is empty:
Deferred (not worked, by design)
Diff growthNet diff for this window is source 46 / test 29 lines against budgets of 400/400, with no prior over-budget rounds, so no growth audit was required. 中文说明Autofix 评审轮次:未执行任何操作PR: #9649( 分诊结果本计数窗口已启用仅处理 Critical 的模式(已完成 5 个产生改动的轮次),因此非 Critical 反馈被排除在本轮可执行区域之外,保持开放留待人工跟进。经过该过滤后,本轮所有可执行区域均为空:
延后项(按设计未处理)
Diff 增长本窗口净 diff 为源码 46 行 / 测试 29 行,预算为 400/400,且此前没有超预算轮次,因此无需进行增长审计。 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/模型 |
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.
— qwen3.8-max via Qwen Code /review (v0.21.10)
doudouOUC
left a comment
There was a problem hiding this comment.
Review Summary (deepseek-v4-flash, round 1)
This PR restores CI=true to the autofix verification gate's env -i launches.
Findings
- Critical (R1-1) —
CIis not pinned at the step level. The gate passesCI="${CI:-true}"into theenv -ichild, but the step-levelenv:block does not includeCI. The:-truedefault only protects against an unsetCI; if an earlier step injectsCI=falsevia$GITHUB_ENV, the default is bypassed and the TUI-test flake can still fire with inverted semantics. - Suggestion (R1-2) —
scripts/tests/qwen-autofix-workflow.test.jslives outside all npm workspaces, so it is not exercised by workspace-scoped test suites and cannot be probed automatically.
Result
COMMENT — 1 inline Critical finding + 1 deferred Suggestion.
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 6/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #9649Critical-only mode is active (5 change-producing rounds complete). This round addresses the single Critical finding in the actionable sections. Feedback triage[rc:3835335216] Critical — CI is not pinned at step level in Claim: Reproduced before implementing anything:
Fix: pin Mutation probe (before commit): removed the [rv:4999279178] COMMENTED review (round 1) — its Critical R1-1 is rc:3835335216 (resolved above). Its Suggestion R1-2 ( [rv:4999255695] COMMENTED review — informational, no action. This is the Changes
No conflicts ( VerificationCommands actually run and their results:
中文说明Autofix 轮次总结 — PR #9649当前处于仅处理 Critical 的模式(已完成 5 个产生改动的轮次)。本轮处理可执行区域中唯一的 Critical 反馈。 反馈分类[rc:3835335216] Critical — 论断: 实施前已完成复现:
修复:在两个 gate( 变异探针(提交前):从 workflow 中移除 [rv:4999279178] COMMENTED 评审(第 1 轮) — 其中的 Critical R1-1 即 rc:3835335216(已在上方解决)。其中的 Suggestion R1-2( [rv:4999255695] COMMENTED 评审 — 信息性,不处理。 这是 改动
无冲突( 验证实际执行的命令及结果:
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Verified against the head:
- The #9648 failure mode is real:
AuthDialog.test.tsx:237-239skips the 18 TUI-input tests onprocess.env['CI'] === 'true', so a gate child launched without CI un-skips them and inherits the flaky one. Both launches now passCI="${CI:-true}", and the step-levelCI: 'true'pin closes the gap the default cannot —:-trueonly covers an UNSET CI, so a$GITHUB_ENVplant ofCI=falsewould otherwise survive the expansion. - The companion hardening is sound against the actual semantics:
LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATHare mapped by ld.so at process startup, so the body-side unset cannot unload a library already mapped into the digest-check step's own bash (step-level empty pins fix that);RUNNER_TEMP/WORKDIR/BRANCHsteer the digest oracle and the child's tree, so pinning them from expression context kills the decoy-path plant; and/usr/bin/echo+/usr/bin/sha256sumbypassBASH_FUNC_*%%function imports, which shadow builtins at bash startup even under--norc. - The test upgrade is the right shape: a verbatim adjacency chain plus an ASCII-only-stripped exact statement list (with the NBSP self-check), one
bash --norcper step, and a sorted-multiset allowlist pin that now fails on any dropped variable — including the broadened assignment regex that catches lowercase names.
One observation, not blocking: the title/body lead with the CI fix, while the diff also lands the R6-2/R6-3/R6-4 startup-channel pins from the same deferred #9262 backlog — cohesive (one gate step, one attack surface), but worth naming in the body for future readers.
CI: 12 checks passing on head; the review-pr automated check is still pending at approval time.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
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 8, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-autofix-workflow.test.js:19444 — [probe] step-level env pins asserted only with bare toContain — a commented-out entry survives the whole 215-test suite
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 8 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| export PATH="${TRUSTED_PATH}" | ||
| unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH |
There was a problem hiding this comment.
[Critical] The gate body's first two statements resolve bare command words in the parent shell, and bash imports $GITHUB_ENV-planted BASH_FUNC_export%% / BASH_FUNC_unset%% entries as functions at startup even under --norc — function lookup precedes builtins. An earlier shell-capable step (the agent runs branch code on the host) can therefore execute attacker code inside this verdict-gating step: in the probe below, a shadowed export installs a DEBUG trap that overwrites ${RUNNER_TEMP}/run-autofix-review-verification.sh AFTER the digest check passes and BEFORE the env -i launch executes it — the attacker's copy runs and writes the forged verdict that gates the PAT-bearing push. A simpler variant writes outcome=fixed straight to $GITHUB_OUTPUT and exits 0. The R6-4 comment above claims the BASH_FUNC import channel is closed by the step-level pins and the absolute digest path, but those cover only the digest line's words; these two adjacent words remain shadowable.
Witness (probe, bash 5.2.21, gate body extracted verbatim at HEAD, hostile step environment):
ARM A (current body): MARKER-export FIRED / MARKER-unset FIRED
digest check passed; staged runner swapped at launch
GITHUB_OUTPUT: outcome=fixed (forged)
ARM B (both stmts removed): no markers; child env byte-identical to ARM A
ARM C (builtin prefix): MARKER-builtin FIRED
Fix: delete both statements from both gate bodies — they are redundant. PATH reaches the child through the env -i allowlist (PATH="${TRUSTED_PATH}"), and LD_* is already neutralized three ways: the step-level LD_*: '' pins above (this step's bash startup), the LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH= command prefix on the /usr/bin/env execve, and env -i for the grandchild. ARM B verifies the child environment is byte-identical without them. Do NOT prefix with builtin/command — any spelling that keeps a command word stays a BASH_FUNC target (ARM C). Update the gateBodyStatements entries in the contract test in lockstep.
中文说明
[Critical] gate 体的前两条语句在父 shell 中以裸命令词解析,而 bash 即使在 --norc 下也会在启动时把 $GITHUB_ENV 植入的 BASH_FUNC_export%% / BASH_FUNC_unset%% 导入为函数——函数查找先于内建命令。因此更早的 shell 级步骤(agent 会在宿主机上运行分支代码)可以在这个决定 verdict 的步骤内执行攻击者代码:下方探针中,被遮蔽的 export 安装了一个 DEBUG trap,在 digest 检查通过之后、env -i 启动执行之前覆写 ${RUNNER_TEMP}/run-autofix-review-verification.sh——攻击者副本得以运行,并写入决定 PAT push 的伪造 verdict。更简单的变体直接向 $GITHUB_OUTPUT 写入 outcome=fixed 并以 0 退出。上方 R6-4 注释声称 BASH_FUNC 导入通道已被 step 级 pin 与绝对路径 digest 关闭,但那只覆盖 digest 行的命令词;这两条相邻语句的裸词仍然可被遮蔽。
修复:从两个 gate 体中删除这两条语句——它们是冗余的。PATH 通过 env -i 允许列表(PATH="${TRUSTED_PATH}")到达子进程;LD_* 已被三重中和:上方 step 级 LD_*: '' pin(本步骤 bash 启动时)、/usr/bin/env execve 上的 LD_PRELOAD= LD_AUDIT= LD_LIBRARY_PATH= 命令前缀、以及孙子进程的 env -i。ARM B 验证删除后子环境变量逐字节不变。不要加 builtin/command 前缀——任何保留命令词的拼写仍是 BASH_FUNC 目标(ARM C)。同步更新契约测试中的 gateBodyStatements 条目。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| 'export PATH="${TRUSTED_PATH}"', | ||
| 'unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH', |
There was a problem hiding this comment.
[Critical] Lockstep with the workflow-side finding: gateBodyStatements canonizes the bare export PATH="${TRUSTED_PATH}" / unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH spellings by exact equality, so the pin suite now mandates the function-shadowable form the R6-4 rationale claims to have closed. When the two statements are removed from both gate bodies (the fix for the workflow-side finding — removal, not builtin-prefixing, which a probe shows stays shadowable), drop these two entries here: the exact-list match expect(gateBodyStatementsOf(step)).toEqual(gateBodyStatements) below fails otherwise. Note the PAT-step toContain('export PATH=…') pins near line 10403 target different steps and need no change.
中文说明
[Critical] 与 workflow 侧发现联动:gateBodyStatements 以精确相等钉住了裸的 export PATH="${TRUSTED_PATH}" / unset LD_PRELOAD LD_AUDIT LD_LIBRARY_PATH 拼写,pin 套件因此把 R6-4 理由声称已关闭的函数可遮蔽形态固化了下来。当两条语句从两个 gate 体中删除时(workflow 侧发现的修复——是删除而非 builtin 前缀,探针表明后者仍可被遮蔽),请同步删除此处这两个条目:否则下方的精确列表匹配 expect(gateBodyStatementsOf(step)).toEqual(gateBodyStatements) 会失败。注意约 10403 行处 PAT 步骤的 toContain('export PATH=…') pin 作用于不同步骤,无需改动。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| RUNNER_TEMP: '${{ runner.temp }}' | ||
| WORKDIR: '/tmp/autofix-review-${{ matrix.target.pr }}' | ||
| BRANCH: '${{ matrix.target.branch }}' |
There was a problem hiding this comment.
[Critical] This pin block closes every $GITHUB_ENV→gate channel except HOME — the only allowlisted child variable (HOME="${HOME}" in the launch below, both gates) whose value still flows from the plantable step environment. Git is defended in the gate child, but npm resolves its userconfig from $HOME and has no defense: a planted HOME whose .npmrc sets script-shell wraps every npm run build|typecheck|lint|test invocation in an attacker-controlled shell, so a genuinely failing check exits 0 — every verdict-determining check reports green for a red branch, and the PAT-bearing push runs. actions/runner accepts the plant (FileCommandManager blocklist is NODE_OPTIONS only; ScriptHandler re-injects only RUNNER_*/allowlisted GITHUB_*, so HOME carries the plant through), which is exactly the doctrine this block applies to RUNNER_TEMP/WORKDIR/BRANCH/CI.
Witness (probe, node 22.23.2 / npm 10.9.8):
CONTROL (real HOME): npm run fail -> exit=7
PLANTED (HOME with .npmrc script-shell): npm run fail -> exit=0
wrap.log: WRAPPED argv=-c exit 7
Fix: capture HOME at stage time with the existing trusted-value doctrine — add echo "trusted_home=${HOME}" >> "${GITHUB_OUTPUT}" to 'Stage trusted schema gate and agent runner' (it runs before any branch code), then pin HOME: '${{ steps.stage.outputs.trusted_home }}' in both gates' env blocks here and extend the pin assertions to it.
中文说明
[Critical] 本 pin 块关闭了所有 $GITHUB_ENV→gate 通道,唯独剩 HOME——它是允许列表中唯一一个(下方启动的 HOME="${HOME}",两个 gate 皆然)取值仍来自可植入步骤环境的变量。gate 子进程中 git 已有防御,但 npm 从 $HOME 解析 userconfig 且无防御:植入的 HOME 目录里 .npmrc 设置 script-shell 后,每一次 npm run build|typecheck|lint|test 都会被攻击者控制的 shell 包裹,真正失败的检查也会以 0 退出——红分支的所有 verdict 判定检查全绿,PAT push 随之执行。actions/runner 接受该植入(FileCommandManager 屏蔽名单仅 NODE_OPTIONS;ScriptHandler 只重新注入 RUNNER_*/允许列表内的 GITHUB_*,HOME 带着植入值透传)——这正是本块对 RUNNER_TEMP/WORKDIR/BRANCH/CI 所应用的学说。
修复:按既有的受信任值学说在 stage 阶段捕获 HOME——在 'Stage trusted schema gate and agent runner'(先于任何分支代码运行)中加 echo "trusted_home=${HOME}" >> "${GITHUB_OUTPUT}",然后在此处两个 gate 的 env 块中钉住 HOME: '${{ steps.stage.outputs.trusted_home }}',并扩展 pin 断言覆盖它。
— qwen3.8-max via Qwen Code /review (v0.21.15)
…ing (QwenLM#9649) The two gate bodies' leading statements were bare command words: bash imports $GITHUB_ENV-planted BASH_FUNC_export%%/BASH_FUNC_unset%% (or BASH_FUNC_builtin%% for a builtin-prefixed spelling) as functions at startup even under --norc, and a shadowed pin can arm a DEBUG trap that swaps the staged runner after the digest check passes and before the env -i launch executes it — forging the verdict that gates the PAT push. Both statements are redundant: PATH reaches the child through the env -i allowlist, and LD_* is closed by the step-level pins, the env execve prefix, and env -i. Probed: hostile plants fire on the pre-fix body and are inert on the fixed body; child env is byte-identical without them. HOME was the remaining $GITHUB_ENV channel into the gate child: npm resolves its userconfig from HOME, and a planted HOME's .npmrc script-shell wraps every verdict-determining npm run, so a red branch reports green (probed: exit 7 becomes exit 0). Capture HOME at stage time, before any branch code runs, and pin it at step level in both gates — the trusted_path doctrine. Contract test updated in lockstep: the pinned statement list drops the two entries, and the pin assertions cover the HOME pin and its stage-time capture (mutation-probed).
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 7/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #9649Feedback points and dispositions[rc:3835686578] Gate bodies' leading statements are bare command words — FixedReproduced before changing anything: the gate body was extracted verbatim from the workflow and run under runner-like bash ( Both statements are redundant: PATH reaches the child through the Change: deleted [rc:3835686580] Contract test canonizes the shadowable spellings — FixedLockstep with the workflow fix: dropped the [rc:3835686581] HOME is the remaining
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
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 9, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-autofix.yml:5215 (+2 locations) — [probe] WORKDIR/BRANCH gate step pins hand-copy the job-level formula — three literal copies no test ties togetherscripts/tests/qwen-autofix-workflow.test.js:11218 — [review] Stale contract-test comment claims the removed body-side unset/PATH export are pinned (R3-1, R2-2)scripts/tests/qwen-autofix-workflow.test.js:10544 — [review] Stale 'with PATH pinned first' clause above the digest-count pin describes the pre-PR gate body
中文说明
无阻断问题。LGTM!✅
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
doudouOUC
left a comment
There was a problem hiding this comment.
Review — APPROVE (C=0, no new findings)
Reviewed at 6aaf14ff against merge-base df768f09. All 20 open threads re-checked against the code as it stands — every one is fixed at this commit. Most read isOutdated: true because the hardening moved the anchored lines, not because the finding was dropped.
Disposition of the open threads
| Threads | Finding | At 6aaf14ff |
|---|---|---|
| R1-1 (mine) | CI not pinned at step level — a $GITHUB_ENV plant of CI=false survives ${CI:-true} |
fixed — CI: 'true' in both gates' env: (5217, 5464) |
| R8-3 | HOME was the last plantable channel; a planted .npmrc script-shell makes a red branch report green |
fixed — HOME: '${{ steps.stage.outputs.trusted_home }}' (5229, 5476) |
| R6-3 | RUNNER_TEMP/WORKDIR/BRANCH steer the digest oracle and the built tree |
fixed — pinned from expression context (5214-5216, 5461-5463) |
| R6-2, R8-1 | LD_* map at ld.so startup, before a body-side unset can run |
fixed — LD_PRELOAD/LD_AUDIT/LD_LIBRARY_PATH: '' at step level |
| R6-4 | bare sha256sum/echo/export/unset are BASH_FUNC_ shadow targets |
fixed — /usr/bin/echo, /usr/bin/sha256sum; the body now carries no bare command word |
| R2-1 | the pin enumerated text tokens, not shell structure — 5 shell-level mutations stayed green | fixed — gateLaunchPin joins tokens with a literal \\\n[ \t]* adjacency chain, gateBodyStatements pins the whole comment-stripped statement list by toEqual, and bash --norc count is 1 per step |
| R2-2, R4-1, R6-1 | value swaps invisible; JS \s ≠ bash whitespace; trim() misclassified NBSP-prefixed lines as comments |
fixed — tokens pin exact values; separator is [ \t] only; ASCII-only strip, plus a meta-assertion that '\u00a0# x' survives the filter |
| R2-3, R1-x | uppercase-only assignment regex; grammar duplicated across two pin sites | fixed — widened to [A-Za-z_][A-Za-z0-9_]*; the two copies re-synced |
| R3-1, R4-2/3/4, R5-1 | no per-step pin on the digest check; statements could be added/reordered/demoted into a never-run arm | fixed — the digest line is the first entry of gateBodyStatements, asserted per step in the loop over both gates |
What I verified independently, beyond re-reading the pins
trusted_homehas a real producer, in the right job, at the right time. It is emitted once (echo "trusted_home=${HOME}", line 3868) inside thereview-addressjob'sstagestep (id: stage, 3827) — the same job whose gates consumesteps.stage.outputs.trusted_home, so the step-output scoping is correct. Step order in that job isCheckout trusted base→Stage …→ … →Prepare branch and feedback(PR branch checkout) →Triage and address(agent runs branch code) →Verification gate. So HOME is captured before the branch is checked out and before any branch code executes, andInstall dependenciesruns against the trusted base. The capture is genuinely pre-plant.- The
WORKDIR/BRANCHstep pins match the job-level values byte-for-byte. Job env at 3662/3664 isWORKDIR: '/tmp/autofix-review-${{ matrix.target.pr }}'andBRANCH: '${{ matrix.target.branch }}'; the four step-level copies (5215/5216, 5462/5463) are identical. A drifted pin would have silently pointed the gate at a tree the agent never wrote to, so this was worth confirming rather than assuming. - The
CI=trueblast radius is exactly what the PR claims — and no wider. Repo-wide, exactly one test file gates onprocess.env['CI'] === 'true':AuthDialog.test.tsx:237-239, whereitWhenTuiInputReliableguards 18 of the file's 25 tests. That matches the PR's1 failed | 24 passed (25)probe precisely. This mattered because the fix works by making tests skip, so the inverse risk is a gate that verifies less — with a single file affected, the gate's coverage is materially unchanged, and it now agrees with the CI semantics the repo's own green runs are measured under. - The third gate is correctly out of scope.
issue-autofix's 'Verification gate' (1370) runs its checks directly in the step shell with noenv -ichild, so it inherits the runner's ownCI=trueand needs no re-declaration. The suite pins this reasoning explicitly (trusted_home=${HOME}must appear exactly once). - The failure mode is closed, not open. If
stagenever ran,TRUSTED_PATH/HOMEarrive empty,env -icannot resolvebash, the step fails, and 'Finalize verification' falls through toexit 1. An absent pin degrades to rejection, never to a forged pass. - The contract suite really runs in CI.
test:cichainsnpm run test:scripts(rootpackage.json), which ci.yml invokes at line 528 — so the new structural pins are green on ubuntu/bash 5.x, where the local macOS bash-3.2mapfilenoise does not apply. All checks on this PR pass.
One non-blocking note, explicitly deferred
The WORKDIR and BRANCH literals now exist in three places per job (job env: + two gate step env: blocks), and the suite pins the step spelling without tying it to the job value. A future edit to the job-level WORKDIR would leave both step pins stale and green, and the gate would then build a tree the agent never wrote to. It fails closed and would break loudly on the first run, so it is a maintenance trap rather than a hole — a expect(reviewAddressJobEnv).toContain(WORKDIR_LITERAL) style cross-assertion would close it.
I am not opening a thread for this. This PR is at 8 review rounds; per AGENTS.md ("land only Critical fixes … defer remaining Suggestions to a follow-up"), it belongs in a follow-up, and I am recording it here so it is not silently dropped.
Assessment
The PR outgrew its title — it started as "add CI to the allowlist" and ended as a full hardening of the gate's parent-shell surface (startup channels, path-variable redirection, function shadowing, and a structural rather than textual pin). Each addition traces to a concrete, named attack path, both gates stayed symmetric throughout, and the pins now fail on shell-structure edits that previously passed. The original one-line fix is still in there and still correct.
中文说明
在 6aaf14ff 上复核,同意合并:20 个未解决线程逐条按 HEAD 核对,全部已修复——多数显示 isOutdated: true 是因为加固改动移动了锚定行,而非结论被丢弃。我自己的 R1-1(CI 未在 step 级钉住)已通过两个 gate 的 CI: 'true' 修复。
除复读 pin 之外我独立核验了六点:
trusted_home确有产出方(3868 行,review-addressjob 的stage步骤内),与消费它的两个 gate 同 job,step-output 作用域正确;且该步骤在 PR 分支 checkout 与 agent 执行分支代码之前运行,捕获时机可信。WORKDIR/BRANCH的 step 级钉值与 job 级定义(3662/3664)逐字节一致——若漂移,gate 会去校验 agent 从未写入的目录树,因此必须实测而非假设。CI=true的影响面与 PR 声明完全一致且不更宽:全仓库仅AuthDialog.test.tsx:237-239一个文件依赖CI === 'true',其中itWhenTuiInputReliable覆盖 25 个用例中的 18 个,与 PR 的1 failed | 24 passed (25)探针吻合。这点很关键——修复方式是让测试跳过,反向风险是 gate 校验变少;仅一个文件受影响说明覆盖面实质未变。- 第三个 gate(
issue-autofix的 1370 行)不在范围内是正确的:它直接在步骤 shell 内跑检查、无env -i子进程,因此自然继承 runner 的CI=true。 - 失败形态是 fail-closed:
stage未运行时PATH/HOME为空,env -i无法解析bash,步骤失败并由 'Finalize verification' 落到exit 1,绝不会伪造通过。 - 契约套件确实在 CI 中运行(
test:ci→test:scripts,ci.yml:528),在 ubuntu/bash 5.x 上为绿,本机 macOS bash 3.2 的mapfile环境噪音不适用;本 PR 全部检查通过。
一个非阻塞、明确延期的观察:WORKDIR/BRANCH 字面量现在每个 job 出现三处(job env: + 两个 gate step env:),而测试只钉 step 侧拼写、未与 job 侧取值做交叉断言。未来若改动 job 级 WORKDIR,两处 step pin 会保持陈旧且测试仍绿,gate 将去构建 agent 从未写入的树。它 fail-closed 且首次运行即会显性失败,属维护陷阱而非漏洞。鉴于本 PR 已 8 轮,按 AGENTS.md「5 轮后只落 Critical,其余延期」的规则不新开线程,在此记录以免静默丢弃。
Verification report — real-environment evidence for #9649I rebuilt this change's threat model in a real Linux environment and on a real GitHub runner, rather than reading the diff. Everything below is measured. Note on timing: the PR merged ( Verdict: the change is sound, and does more than its description claims. I got a working attack against the pre-PR body for every hardening rationale in it. Five stand fully; the sixth (R6-3) stands for MethodThe gate step's
1. The stated motivation is real — and understated
The PR body reports "1 failed | 24 passed". On this machine it is far worse — this is not a single flaky test but a whole class un-skipping, so the gate's exposure was larger than the description suggests. One more thing the description doesn't mention, and which sharpens the argument: the 2. Every attack the comments describe actually works on the pre-PR bodyBoth bodies, same harness, same tampered-runner scenario:
Two controls worth stating, because they show the attacks land specifically through the channels this PR closes, not through a broken harness:
3. On a real runner the threat model holds — with one exceptionRunner provides
The one inaccuracy. The R6-3 comment says a Applicability caveat: the gate normally runs on the self-hosted 4. Both gates got the change, symmetricallyParsed back out of the merged YAML: two launches carry the gate ( 5. The new contract pins are load-bearing, and close a real gap on mainKeyed on assertion failures, not on suite red/green — one of the three pinning tests spawns real shells and times out under load, which would otherwise read as "the pin caught it".
Pre-PR the launch was pinned with a series of 6. Suite healthFull
No failure appears on Scope notes / follow-ups
ReproduceThe core loop is: parse the step out of the workflow YAML, replay it under 中文版验证报告 — #9649 的真实环境证据我没有只读 diff,而是在真实 Linux 环境和真实 GitHub runner 上重建了这个改动的威胁模型。下面全部是实测。配图见上方英文版 Fig 1–5(Fig 1/2 容器实证、Fig 3 真实 runner、Fig 4 变异矩阵、Fig 5 AuthDialog)。时间说明: 验证进行期间这个 PR 已经合并( 结论:改动是可靠的,而且做的比描述里说的更多。 对它列出的每一条加固理由,我都在合并前的正文上把对应攻击真正打通了。其中五条完全成立;第六条(R6-3)对 方法从 workflow YAML 里逐字抽取(解析得到,绝不手抄)三棵树的 gate step
1. PR 声称的动机成立,而且被低估了
PR 正文写的是「1 failed | 24 passed」。在我这台机器上要严重得多 —— 这不是单个 flaky 测试,而是一整类测试被解除跳过,所以 gate 此前的暴露面比描述中更大。 还有一点描述里没提、但能让论证更锋利: 2. 注释里描述的每种攻击,在合并前的正文上都真的打得通两份正文,同一套 harness,同一个「runner 被中途篡改」的场景:
有两个对照值得单独说,因为它们证明攻击是恰好从这个 PR 关掉的通道进来的,而不是 harness 本身有问题:
3. 在真实 runner 上威胁模型成立 —— 但有一个例外runner 确实提供
唯一那处不准确。 R6-3 的注释说 适用范围提醒: 这个 gate 平时跑在自建的 4. 两处 gate 都改到了,而且是对称的从合并后的 YAML 反解出来:承载 gate 的有两处 launch( 5. 新增的契约 pin 是承重的,并且堵上了 main 上一个真实缺口判据是断言失败,而不是套件红/绿 —— 三个 pin 测试里有一个会 spawn 真实 shell,在负载下超时,否则会被误读成「pin 抓住了」。用
合并前那处 launch 是用一串 6. 套件健康度在真正的 before/after 两棵树(
范围说明 / 后续
复现核心做法是:把 step 从 workflow YAML 里解析出来,在 |
|
Released in v0.22.0. |








What this PR does
Restores the runner-provided
CI=trueto both autofix verification-gate launches. The gate runs the branch's own build/typecheck/lint/test through anenv -iclean child whose allowlist covered only 8 variables;CI=truewas not among them, so the gate's checks ran with inverted CI semantics relative to the repository's regular CI.Why it's needed
packages/cli/src/ui/auth/AuthDialog.test.tsxtreatsprocess.env['CI'] === 'true'as "TUI-input tests are unreliable here" and skips 18 such tests on CI. WithoutCI=truein the gate child they un-skip inside the gate, and one of them flakes (~5svi.waitFortimeout) —reject_fixthen fires (retryable) on a fix the PR's own CI passes green, burning the repair pass on an unreachable failure and potentially mislabeling the round's A/B baseline. Reproduced locally: withCI=truethe AuthDialog file is green; with CI unset it fails 1/25 on exactly that TUI test. This was deferred from PR #9262 (R5-1) and tracked as issue #9648.Reviewer Test Plan
How to verify
The new pin enumerates the full
env -iallowlist of both gate launches as a sorted multiset, so a symmetric drop or duplicate of any entry — includingCI— fails the suite. The old pin countedenv -ioccurrences only.Evidence (Before & After)
N/A (CI machinery). Behavior probe:
CI=true npx vitest run packages/cli/src/ui/auth/AuthDialog.test.tsx→ file green (18 TUI tests skipped);env -u CI npx vitest run ...→1 failed | 24 passed (25), the failing test being the flaky TUI-input one the gate would otherwise trip on.Tested on
Environment (optional)
Contract suite (211 passed / 4 environment-only bash-3.2
mapfilefailures, unrelated) + behavior probe above.Risk & Scope
Linked Issues
Closes #9648 (deferred from PR #9262, R5-1).
中文说明
本 PR 做了什么
把 runner 提供的
CI=true恢复到 autofix 验证 gate 的两次启动中。gate 通过env -i干净子进程运行分支自身的 build/typecheck/lint/test,其允许列表只覆盖 8 个变量;CI=true不在其中,导致 gate 内的检查以与仓库常规 CI 相反的 CI 语义运行。为什么需要
packages/cli/src/ui/auth/AuthDialog.test.tsx把process.env['CI'] === 'true'视为「TUI 输入测试在此环境不可靠」并在 CI 上跳过 18 个此类测试。gate 子进程里没有CI=true时它们被解除跳过,其中一个会 flake(约 5 秒vi.waitFor超时)——reject_fix就会对一个 PR 自身 CI 全绿的修复触发 retryable,为一个不可复现的失败烧掉 repair 通道,并可能误标该轮的 A/B 基线。本地复现:CI=true时 AuthDialog 文件全绿;去掉 CI 时 1/25 失败,恰好是该 TUI 测试。从 PR #9262(R5-1)延期,跟踪于 issue #9648。审查者测试计划
验证方式:
grep -n 'CI="${CI:-true}"' .github/workflows/qwen-autofix.yml(两处启动都有);契约套件npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-autofix-workflow.test.js。新 pin 以排序多重集合枚举两次 gate 启动的完整env -i允许列表——任何条目的对称删除或重复(包括CI)都会使套件失败;旧 pin 只数env -i出现次数。证据:N/A(CI 机制)。行为探针:
CI=true时 AuthDialog 全绿(18 个 TUI 测试跳过);env -u CI时1 failed | 24 passed (25),失败者正是 gate 会误触发的 flaky TUI 测试。已在 macOS 上验证契约套件(211 通过 / 4 个无关的 bash-3.2
mapfile环境失败)与行为探针。风险与范围
关联 Issue
Closes #9648(从 PR #9262 延期,R5-1)。