feat(triage): add sandboxed /verify deep-verification lane - #7710
Conversation
@qwen-code /verify on a PR now runs a local-verification-style evidence round in the isolated /tmux sandbox contract (container, token-free agent env, loopback model proxy, author-write gate) and publishes the report via a separate PR-code-free job: - new verify job: merge-ref checkout at depth 2 (base tip + PR head for A/B), skills pinned from base so the tree under test can never rewrite its own verifier, PR-planted tmp/*-verify-* artifacts dropped, git exec-vector sweep for the persistent workspace, agent verdict allowlisted before it reaches workflow outputs - new publish-verify job: upserts one marker comment (running status -> final report), HTML-escapes the untrusted report, reports skip/na/ prepare-fail/infra outcomes explicitly since /verify is always an explicit request - new verify-pr skill: A/B load-bearing proof, vacuity check on new tests, mock-free wire-oracle harnesses, targeted gates, fixed report/ verdict/assertions artifact contract, counts-are-sacred rules - triage skill Stage 2c now names /verify (not just /tmux) as the trigger to recommend when a PR's central claim needs behavioral evidence The verify check-runs ride the issue_comment event, which the finalize workflow's event == "pull_request" universe structurally excludes, so they cannot pollute the CI table or the deferred-approval gate.
|
Thanks for the PR! Template looks good ✓ Problem: this is a feature addition, not a bug fix — the "problem" is an observed workflow gap. Maintainers currently produce deep verification rounds (A/B against base, wire oracles, load-bearing proofs) entirely by hand (referenced #7586 round 3, #7632, #7656, #7686). The triage agent is static-only by design (its env carries a write PAT), and the only behavioral lane today ( Direction: aligned. This is CI/triage infrastructure — squarely within the project's scope. It's purely additive (new comment command, new skill, one recommendation sentence in the triage skill). Doesn't touch auth, sandbox, model selection, telemetry, release, or public contract. The verify check-runs ride the Size: no core paths touched ( Approach: the scope feels right for what it's doing. The security hardening is extensive but justified — case-insensitive command matching, ephemeral proxy port with nonce + bearer auth, re-pinning the verifier after PR lifecycle scripts, symlink-safe cleanup, HEAD movement detection, verdict allowlisting, HTML escaping with size capping. The duplication with the tmux job's loopback-proxy machinery is acknowledged as a deliberate follow-up to keep this additive — reasonable. The three commits fold in patterns from hand-run rounds (follow-up rounds, image evidence, quantified verification rules), each earning its place. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:这是一个功能新增,而非 bug 修复——"问题"是一个已观测到的工作流缺口。维护者目前完全靠手工产出深度验证轮次(对 base 的 A/B、wire oracle、load-bearing 证明——引用了 #7586 第 3 轮、#7632、#7656、#7686)。triage agent 在设计上只做静态分析(其环境携带写权限 PAT),目前唯一的行为验证车道 方向:对齐。这是 CI/triage 基础设施——完全在项目范围内。纯增量改动(新评论命令、新 skill、triage skill 中一句推荐语)。不触及 auth、sandbox、模型选择、遥测、发布或公共契约。verify 的 check-run 挂在 规模:未触及核心路径。生产逻辑约 2,091 行(workflow YAML 1,685 + skill 386 + pr-workflow.md 20),测试 1,638 行。触发 1,000+ 大 PR 建议——改动量大,但范围与目标相称:两个新 CI job 带大量安全加固(执行不可信 PR 代码)、一个方法论 skill、以及全面的测试。不阻断。 方案:范围与目标相称。安全加固广泛但有正当理由——大小写不敏感的命令匹配、临时端口代理 + nonce + bearer 认证、PR 生命周期脚本后重新 pin 验证器、符号链接安全的清理、HEAD 移动检测、verdict 白名单、HTML 转义与尺寸上限。与 tmux job 的 loopback-proxy 机制的重复被明确标注为后续重构——合理。三个提交分别固化了手工验证轮次的模式(后续轮次、图片证据、量化验证规则),各有其价值。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewIndependent proposal: for a sandboxed deep-verification lane, I would add two jobs to Comparison: the PR matches and substantially exceeds this baseline. The security hardening goes well beyond what I would have proposed in a first pass — ephemeral proxy port with nonce identity + bearer auth, re-pinning the verifier after PR lifecycle scripts run (not just before checkout), killing leftover build processes before agent start, symlink-safe cleanup paths, worktree-scoped git config neutralization, HEAD movement detection between authorization and execution, verdict allowlisting, HTML escaping applied before size capping. Each of these addresses a concrete attack vector the PR documents in comments. The test suite (1,638 lines) executes the shell scripts rather than substring-matching them — the authorize gate is replayed with a stubbed No critical blockers found. No AGENTS.md violations. The duplication with the tmux job's loopback-proxy machinery is acknowledged as a deliberate follow-up — reasonable for an additive change. Comments throughout explain why (the attack vector, the race condition, the failure mode), not what. Files changed (4 of 4)
CI test evidenceAll checks completed on the reviewed commit — no failures, no pending runs.
macOS/Windows tests and integration tests are skipped — expected for a workflow/skill/test-only change that doesn't touch the CLI runtime. Real-scenario testingN/A — this PR changes CI workflow YAML, a skill file, and workflow tests. There is no user-visible CLI behavior change; the 中文说明代码审查独立方案: 对于沙箱深度验证车道,我会在 对比: PR 与独立方案吻合,并大幅超出。安全加固远超首次提案——临时端口代理 + nonce 身份 + bearer 认证、PR 生命周期脚本后重新 pin 验证器、agent 启动前杀死残留构建进程、符号链接安全的清理路径、worktree 级 git config 中和、授权与执行间的 HEAD 移动检测、verdict 白名单、HTML 转义在尺寸上限之前应用。每一项都针对 PR 注释中记录的具体攻击向量。测试套件(1,638 行)执行 shell 脚本而非子串匹配——authorize 门禁用 stub 未发现关键阻断项。无 AGENTS.md 违规。与 tmux job 的 loopback-proxy 机制的重复被明确标注为后续重构——对增量改动合理。注释全程解释"为什么"(攻击向量、竞态条件、故障模式),而非"做什么"。 CI 测试证据所有检查在受审提交上已完成——无失败、无待处理运行。macOS/Windows 测试和集成测试被跳过——对仅改动 workflow/skill/测试的 PR 属预期行为。 真实场景测试不适用——本 PR 改动 CI workflow YAML、skill 文件和 workflow 测试。无用户可见的 CLI 行为变更; — Qwen Code · qwen3.8-max-preview Reviewed at |
…hods Fold two more hand-verification patterns into the verify lane: - round continuity: the resolve step snapshots the previous verify report (if any) into the agent context before the status upsert overwrites it, and the skill re-checks each prior finding at the new head (fixed/stands/superseded), scoping new probes to the delta - harness quality: prefer configuration seams over module interception, encode the upstream's real semantics in the fake peer, add decoy targets - artifact-matched methods: per-commit load-bearing tables for multi-commit PRs; workflow/CI PRs get embedded-script replay against real data, repo lint gates, and day-one trigger cost math from real event history; every new config knob must trace to an observable effect, and default-path dispatch combinations get probed - findings quality: blockers enumerate blast radius, demonstrate the sharpest consequence end-to-end when budget allows, and carry a collapsed minimal suggested fix preserving the original commit's intent
|
Confidence: 4/5 — solid, well-hardened implementation; the live CI path hasn't run yet but every failure mode publishes an explicit comment, so a bad first run is visible and contained. This PR does one thing — makes maintainer-grade deep verification available on demand in CI — and does it thoroughly. The security model is the standout: it treats every boundary as hostile (PR lifecycle scripts, PR-committed artifact dirs, the agent's own verdict file, the report content, evidence images) and defends each with a specific, documented mechanism rather than a blanket "sandbox" hand-wave. The test suite executes the embedded shell scripts rather than pattern-matching them, which is the right discipline for workflow code where the shell IS the product. The independent proposal I wrote before reading the diff covered the same two-job split and base-pinning, but the PR's hardening goes significantly deeper — ephemeral proxy port with nonce + bearer, re-pinning after lifecycle scripts, symlink-safe cleanup, HEAD movement detection. Each addresses a concrete attack I didn't think of, and the comments say so. Non-blocking nits: the loopback-proxy duplication with the tmux job will want a shared-steps refactor (acknowledged as a follow-up), and the live Actions runner path is untested by definition until merge — but the lane is opt-in, per-PR concurrency-isolated, and every failure mode (skip, docs-only, install/build failure, infra error, cancel) publishes an explicit comment instead of silence. Approving. ✅ 中文说明置信度:4/5 —— 实现扎实、加固到位;实际 CI 路径尚未运行过,但每种故障模式都会发布明确评论,因此首次运行出问题也是可见且可控的。 本 PR 只做一件事——让维护者级深度验证在 CI 中按需可得——而且做得彻底。安全模型是亮点:它将每条边界都视为敌对的(PR 生命周期脚本、PR 预埋的产物目录、agent 自身的 verdict 文件、报告内容、证据图片),并用具体的、有文档记录的机制逐一防御,而非笼统地一句"沙箱"带过。测试套件执行内嵌 shell 脚本而非模式匹配——对于 shell 即产品的 workflow 代码,这是正确的纪律。 独立方案覆盖了相同的两 job 拆分和 base pinning,但 PR 的加固显著更深——临时端口代理 + nonce + bearer、生命周期脚本后重新 pin、符号链接安全的清理、HEAD 移动检测。每一项都针对我没想到的具体攻击,且注释说明了原因。 非阻断建议:与 tmux job 的 loopback-proxy 重复后续需要共享步骤重构(已标注为后续),实际 Actions runner 路径在合并前按定义无法测试——但车道是按需触发、按 PR 并发隔离的,且每种故障模式都发布明确评论而非沉默。 批准。✅ — Qwen Code · qwen3.8-max-preview Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
…rules Borrow the image-evidence and quantified-verification patterns from hand-run rounds (#7265, #7471, #7686 r2 and the pr-assets convention): - publish-verify now hosts agent-produced evidence/*.png on the pr-assets branch (verify/pr<N>-<run>-<attempt>/) and appends them below the escaped report. Untrusted-payload discipline: strict filename allowlist, 8-image / 2 MB caps enforced in the find predicates, racing-push retry, and every failure degrades to a text-only comment. VERIFY_ASSETS_REMOTE is a test seam; the block was dry-run against a local bare remote covering hosting, hostile filenames, oversize files, dotfiles, missing branch, and no-image runs - skill: evidence images are named as kebab-case captions binding image to claim, before/after pairs over lone after-shots; follow-up rounds lead with a previous-finding status table (fixed/stands/superseded/declined, with adjudication) and re-measure instead of diffing the old report; size/perf claims get measured-metric Δ tables with residual deltas accounted for; unreachable branches get the configuration that reaches them constructed; defensive guards get their accept path checked against real production artifacts, not just mocked rejects
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
— qwen3.7-max via Qwen Code /review
- skill: local invocation resolves --repo and passes it to every gh call - skill: call out the dependency confound when the base A/B side reuses the PR-installed node_modules and the PR touches package.json/lockfile - workflow: document the pin step's bootstrap logic — issue_comment jobs run the default branch's YAML, so base always carries the verify-pr skill by the time this job exists
|
@qwen-code /triage |
|
Triage re-run completed without a new review. The stage comments above were updated with the latest result. View workflow run. |
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.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.7-max via Qwen Code /review
Review:
|
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
…g per review Address review round 5078770575 items 1-3 plus the cheap follow-ups: - authorize: /verify now requires write from BOTH the PR author (whose code runs) and the commenter (who spends a scarce runner slot + model budget) — a drive-by account can no longer burn 45 minutes of ecs-qwen on someone else's PR; duplicates check once; /tmux and /triage gates unchanged. Replayed 8 principal scenarios against a stubbed gh - authorize acks /verify with the eyes reaction from the always-hosted job, so a queued/saturated sandbox pool no longer means total silence - publish: emit_block escapes FIRST and caps the escaped size (45 KB for the report) — a raw-side cap let dense <>& content inflate past GitHub's 65,536-char comment limit, 422 the post, and strand the running status with no report at all; iconv -c keeps a UTF-8 sequence split by the byte cut (likely, given the mandated 中文 summary) from shipping broken; replayed: 50 KB dense report -> 45,873-byte body - publish: image cap is byte-exact (-size -2097153c; find's -2M rounds sizes UP to MiB, silently making the documented 2 MB cap 1 MiB), bytes must carry the PNG magic (extension is attacker-choosable), duplicate sanitized names dedupe instead of overwriting + double-rendering, and dropped images are reported in the comment instead of vanishing - publish: weak terminal notices (cancelled/infra/skipped/n-a) only replace this run's own running status; a previous round's real report survives as the marker comment and the notice posts fresh - publish: report.md/assertions.json lookups pin the artifact-dir shape and sort (bare find -name order is filesystem-dependent); the verify job's verdict.txt lookup sorts likewise - verify: global npm install runs from RUNNER_TEMP (the persistent workspace still holds the PREVIOUS run's tree, whose .npmrc would apply to a root install); both cleanup passes remove leftover tmp/ worktrees (git worktree prune alone only drops metadata); the run step no longer re-chowns 50k node_modules files; pr-assets clone sets its committer identity once so the racing-push rebase retry can commit - skill: worktree guidance now tells the agent to remove its base tree itself, with the workflow sweep as backstop only
Response to the review round (comment) — all three pre-merge items fixed in
|
| # | Finding | Resolution |
|---|---|---|
| 1 | Any GitHub user can trigger a 45-min self-hosted job + model budget | Fixed. /verify now requires write from both the PR author and the commenter; duplicates check once; any API error or non-write denies. Replayed 8 principal scenarios (drive-by deny, both-write allow, author-without-write deny, self-comment dedupe, 404 fail-closed, /tmux+/triage unchanged) — 8/8. |
| 2 | Escaped report can exceed the 65,536 comment cap → 422 → no comment | Fixed. emit_block escapes first, then caps the escaped size (report cap 45,000; 400 KB input bound on the escape itself); iconv -c drops a UTF-8 sequence split by the byte cut. Replayed: the 50 KB dense <>& report that previously assembled to >65k now produces a 45,873-byte body with a truncation note, valid UTF-8; a 中文-heavy report cut at the cap decodes clean. |
| 3 | -size -2M is a 1 MiB cap; over-cap images vanish silently |
Fixed. Byte-exact -size -2097153c (verified: exactly-2-MiB accepted, +1 byte rejected); bytes must carry the PNG magic (extension is attacker-choosable); duplicate sanitized names dedupe instead of overwrite-and-render-twice; dropped images are now counted in the comment ("N additional image(s) did not pass the hosting checks…") instead of vanishing. |
| 4 | Run-time forgery: unordered find -name report.md |
Hardened. report.md/assertions.json/verdict.txt lookups pin the *-verify-*/ dir shape, add -mindepth 2, and sort. Full run-time forgery by a steered agent remains inherently in-scope of the agent's own powers — the report is advisory and labeled as such; noted as a residual, not closable by lookup discipline alone. |
| 5 | npm install -g under the previous PR's .npmrc |
Fixed for the verify lane: the global install runs from RUNNER_TEMP. The identical pattern in the tmux job is pre-existing and left for the follow-up PR. |
| 6 | skip/n-a/cancelled overwrite a previous real report | Fixed. Weak terminal notices only PATCH the marker comment when it is this run's own "running" status; over a previous round's real report they post fresh. Replayed all three arms of the matrix. |
| 7a | rebase retry lacks committer identity | Fixed — identity set once on the clone; the per-command -c pairs are gone. |
| 7b | Total silence when the ECS pool is saturated | Improved — the 👀 ack moved to the always-hosted authorize job, so an accepted request is acknowledged even while the sandbox job queues. The runs-on fallback question (hardcoded ECS vs MAINTAINER_ECS_RUNNER_DISABLED) is deferred: it mirrors the tmux job's deliberate ECS-only choice and deserves its own decision. |
| 7c | Duplicate-filename collision | Fixed (dedupe, see #3). |
| 7d | No PNG validation / unbounded pr-assets growth |
Magic check fixed (see #3). Retention policy for pr-assets is a repo-level decision (the hand-run rounds have the same growth) — deferred to the follow-up. |
| 7e | UTF-8 split at the cut | Fixed (see #2). |
| 7f | Double chown -R $GITHUB_WORKSPACE |
Fixed — the run step now chowns only the two RUNNER_TEMP dirs. |
| 7g | Worktree guidance inaccurate | Fixed — the skill now has the agent git worktree remove its own base tree, and both workflow cleanup passes sweep leftover tmp/ worktrees via git worktree list --porcelain (prune alone only drops metadata). |
| 7h | Disjoint verify/tmux concurrency groups | Confirmed safe, no change: a self-hosted runner instance executes one job at a time and each instance owns its own _work directory, so /tmux and /verify running concurrently land on different instances with separate workspaces. The tmux comment's "same workspace" concern applies to two runs of the same job family on the same instance over time, which each lane's own per-PR group already serializes. |
Deferred to the follow-up PR (tmux-side siblings, as the review suggested): tmux's *-tmux-* planted-artifact sweep, its raw-side emit_block caps, its .npmrc-exposed global install, and the pr-assets retention story.
中文说明
针对本轮 review 的处理(修复提交 1db0136cd,推送前全部经本地回放验证):
- 1(任意账号可触发 45 分钟 self-hosted job)已修:
/verify现要求 PR 作者与评论者双方均具写权限,重复主体只查一次,API 错误或权限不足一律拒绝;8 个场景回放全过(路人拒绝、双写放行、作者无写拒绝、自评去重、404 fail-closed、/tmux、/triage行为不变)。 - 2(转义后报告超 65,536 上限 → 422 → 无评论)已修:
emit_block先转义再按转义后大小截断(报告上限 45,000),iconv -c处理被字节切割劈开的 UTF-8 序列;50 KB 高密度<>&报告现在产出 45,873 字节评论体 + 截断注記,中文重载报告切割后解码无损。 - 3(-2M 实为 1 MiB 上限、超限图片静默消失)已修:字节级精确
-size -2097153c(实测恰 2 MiB 收、+1 字节拒);字节必须带 PNG magic;重名去重;被丢弃图片在评论中显式计数。 - 4 查找已钉死目录形态并排序;被劫持 agent 的运行时伪造属其固有能力范围,报告本身即标注为参考性证据。
- 5 verify 侧全局安装已移至
RUNNER_TEMP执行;tmux 同款问题留后续 PR。 - 6(弱终态覆盖旧报告)已修:cancelled/infra/skipped/n-a 只替换本轮的"运行中"状态;旧报告存活,弱通知另发新评论;三臂矩阵回放通过。
- 7 系列:rebase 身份、👀 移至 hosted authorize、重名碰撞、PNG magic、UTF-8 切割、双重 chown、worktree 指引与清扫均已修;verify/tmux 并发组经分析确认安全(单 runner 实例同时只跑一个 job,各实例
_work独立);ECS runs-on 回退与 pr-assets 保留策略留待后续决策。
后续 PR 待办(review 建议的 tmux 侧同款问题):*-tmux-* 预埋产物清扫、emit_block 原始侧上限、.npmrc 暴露的全局安装、pr-assets 保留策略。
Review —
|
…erify Address review round 2 (comment 5079157987) and the CHANGES_REQUESTED round on the verify lane: - run step re-sweeps tmp/*-verify-* AFTER npm ci/build and before the agent starts: the pin step's sweep runs before PR lifecycle scripts (postinstall etc.), which could re-plant a fake artifact dir whose zeroed timestamp deterministically wins the sorted collector. From the sweep on, only the agent writes those dirs; a steered agent forging its own artifacts remains the documented advisory-report residual - RUNNER_TEMP verify-results/verify-context are rm'd before mkdir: the pool is persistent and runner temp hygiene is runner-managed — a stale report or previous-report.md from ANOTHER PR must never ride along - symlinks are stripped from verify-results before upload: actions/upload-artifact dereferences them, so a node-planted link would exfiltrate whatever it points at into the artifact - a trusted commenter invoking /verify on a PR whose author lacks write now gets an explanation comment from the hosted authorize job instead of total silence (the commenter is checked first; drive-by accounts and API errors still get nothing); job timeout 45->60 so a slow install can never let the JOB limit kill the agent past its own graceful 25m budget - stale tmp/base-tree (skill's canonical scratch worktree) is removed by name at job start — a plain dir isn't git-registered, so the worktree sweep alone misses it and the next worktree add would fail - scripts/tests/qwen-triage-workflow.test.js gains a verify-lane describe block: an 8-arm stub-gh replay of the dual principal gate (drive-by deny, author-without-write deny + explain flag, self-comment dedupe, 404 fail-closed, /tmux and /triage unchanged) plus guards for the post-prepare sweep placement, the symlink strip, and the RUNNER_TEMP resets — the replay found this commit's sweep edit had silently not applied, which is exactly the regression class it exists to catch
Response to review round 2 (comment) — all three findings fixed in
|
| Finding | Resolution |
|---|---|
High — runtime-planted artifact deterministically wins the collector (lifecycle scripts run after the pin sweep; 00000000 sorts ahead of real timestamps) |
The run step now re-sweeps tmp/*-verify-* after npm ci/npm run build and immediately before the agent launches — from that point only the agent writes those dirs, so the sorted selectors can no longer meet a plant. sort stays for determinism. The alternative (workflow-owned artifact dir under $RUNNER_TEMP) was considered and set aside: verify-results is node-writable during the agent phase anyway, so it offers the same trust envelope as a swept workspace dir at more plumbing. Residual: a steered agent forging its own artifacts — inherent to its power envelope, which is why the report is advisory and never a review. Placement is regression-tested (sweep must precede start_openai_proxy). |
Medium — /verify on an untrusted-author PR is totally silent |
The gate now checks the commenter first; when the author check is what fails, the hosted authorize job posts a bilingual explanation ("author lacks write; the sandbox executes the author's code; use /triage instead"). Drive-by commenters and API-error denials stay silent — nothing is owed there, and it avoids handing an unauthenticated account a comment-trigger oracle. Covered by the replay test (explain flag asserted on exactly the trusted-commenter/untrusted-author arm). |
| Low — upload-artifact dereferences node-planted symlinks | find "$RUNNER_TEMP/verify-results" -type l -delete runs between collection and upload; guarded by test. |
Also from this round's overlap with the CHANGES_REQUESTED pass: $RUNNER_TEMP/verify-{results,context} are flushed before reuse (stale-run leakage on the persistent pool), job timeout 45→60 so the agent's graceful 25m budget survives slow installs, and the authorize principal routing got the 8-arm stub-gh replay test — which promptly caught an edit in this very commit that had silently failed to apply.
中文说明
第 2 轮三项发现已全部在 844469dbc 修复:
- High(生命周期脚本可在 pin 清扫后重新预埋伪造产物目录,
00000000时间戳在排序下必胜):run 步骤在npm ci/build之后、agent 启动之前二次清扫tmp/*-verify-*,此后只有 agent 会写这些目录;sort保留用于确定性。评估过"workflow 专属$RUNNER_TEMP产物目录"方案后搁置——agent 阶段该目录同样是 node 可写的,信任包络相同而管线更复杂。残余风险(被劫持的 agent 自伪造产物)属其固有能力范围,报告因此定位为参考证据而非评审。清扫位置有回归测试。 - Medium(作者无写权限时 /verify 完全静默):门禁改为先查评论者;当失败方是作者时,由常驻 hosted 的 authorize job 发布双语说明(作者缺写权限、沙箱执行作者代码、请改用
/triage)。路人评论者与 API 错误仍保持静默。回放测试断言 explain 标志只在"可信评论者 + 不可信作者"场景出现。 - Low(upload-artifact 解引用 symlink):收集与上传之间删除全部符号链接,配测试。
另与 CHANGES_REQUESTED 轮重叠的修复:$RUNNER_TEMP 两目录复用前冲洗、job 超时 45→60、8 场景 stub-gh 回放测试守卫双主体门禁(该测试当场抓到本提交中一处未生效的编辑)。
wenshao
left a comment
There was a problem hiding this comment.
— Codex GPT-5 via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.8-max-preview via Qwen Code /review
… in /verify Address the Codex /review round (19 findings) and the bot's follow-up. Each fix was replayed locally; the proxy fix has a decisive A/B. Gate and routing: - the shell command match is case-insensitive: GitHub Actions expression comparisons ignore case, so `@QWEN-CODE /VERIFY` reached the step and fell through to the commenter-only branch — running the PR author's code with the author never checked - the verify ack and denial notice require github.event.issue.pull_request: /verify on a plain issue was acknowledged but could never report - publish-verify joins the verify job's per-PR concurrency group, and a failed PATCH falls back to posting fresh instead of going silent Untrusted-input paths: - the model proxy binds an EPHEMERAL port, reports it through a root-owned file, and its health check must echo a per-run nonce with the recorded PID alive. A/B with a squatter on 8787: the old code's proxy dies EADDRINUSE yet still reports enabled and points qwen at the squatter; the new code comes up unaffected on an ephemeral port - worktree-scoped git config is deleted before hooksPath is resolved: `extensions.worktreeConfig` is allowlisted and .git/config.worktree is invisible to `git config --local`, so a prior run could set core.hooksPath=/ and make the hook sweep's recursive delete walk / as root (verified locally). The sweep now also refuses any hooks path outside the repository's git dir - marker-comment lookups accept only bot-owned comments that START with the marker: any user can paste the marker and divert the bot into PATCHing a stranger's comment - the upload staging dir is re-flushed after npm lifecycle scripts Honest verdicts: - the docs-only classifier no longer uses a pipeline (grep -q made the writer take SIGPIPE, so under pipefail a long file list with an early code file classified a code PR as docs-only and skipped verification), and executable markdown/YAML (.qwen, .github/workflows, scripts) is classified as behavioral before the extension rule - tee's status is checked alongside qwen's: a full results volume made a truncated evidence stream publish as pass - 137 is split by elapsed budget into watchdog timeout vs crash/OOM - the agent's verdict is honored only for VERDICT=pass with a report and zero failed assertions; otherwise the process outcome headlines and the scope paragraph says the run did not complete - verdict.txt is read through a bounded prefix (SIGPIPE under pipefail) Skill contract corrections: per-commit tables only when the commits are reachable at depth 2 (else aggregate + Not covered); internal workspace symlinks must have their realpaths asserted before a base control is trusted; repo lint gates and event-history cost math are qualified to what the token-free container can actually run; --repo is never inferred from `origin` (a fork holds a different PR under the same number). Tests: 9 new guards, all mutation-verified (reverting each fix turns one red), including an executable escaping/size-cap/UTF-8 test for the publisher's own emit_block and a fix to the earlier command-file test, which matched the tmux job's identically named step.
Response to the Codex review round + bot follow-up — all 22 threads addressed in
|
wenshao
left a comment
There was a problem hiding this comment.
route failed; Agent 7 selected no build/test command for the non-workspace files (the targeted workflow test was run separately: 27 passed).
— Codex GPT-5 via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: Invariant agent A: state, timers, collections — .github/workflows/qwen-triage.yml, Invariant agent B: counters, return values, error taxonomies — .github/workflows/qwen-triage.yml, Invariant agent C: config fields, early returns — .github/workflows/qwen-triage.yml — its prompt was built, but no agent on record was launched with it.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 2 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
— qwen3.7-max via Qwen Code /review
…egressions Fold techniques from the re-verification on #7709 that the skill had no equivalent for: - the mutation matrix must report the mutations that changed NOTHING, not only the ones that failed. Each survivor gets classified as an ordinary coverage gap or as dead code — a guard whose deletion leaves every test green is one of those two, and the difference is what the author needs. Survivors mirroring a pre-existing gap are labelled as such, and the set is framed as completeness reporting rather than merge conditions - the sharper case that report demonstrates: a test that passes for the WRONG REASON. If deleting the new guard leaves its own new test green, that test is pinned by an earlier early-return, not by the change, and asserts nothing about it. Name what actually pins it - and do not generalize from one dead guard to its siblings: the same report shows a clause that is unreachable on one path while being the only protection on another. Check each, report the contrast - observability regressions: when a change suppresses output, follow the value before calling the suppression correct. A bare catch on the path plus a field with no readers anywhere in the repo means the cause is now unobservable even in devtools — a real loss that no behavioural assertion can see - report structure gains a Corrections section: when an earlier round or bot comment described the code inaccurately, state the correct fact with evidence and label it as a correction to the description, not a request to change code. A wrong description left standing costs the next reader more than the original finding did
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
— qwen3.7-max via Qwen Code /review
| PR_NUMBER: '${{ needs.tmux-testing.outputs.pr_number }}' | ||
| # Same reason as publish-verify: a tmux job cancelled while | ||
| # pending never evaluates its outputs. | ||
| PR_NUMBER: '${{ needs.tmux-testing.outputs.pr_number || github.event.issue.number }}' |
There was a problem hiding this comment.
[Suggestion] The tmux PR_NUMBER fallback (needs.tmux-testing.outputs.pr_number || github.event.issue.number) has no test asserting its presence, while the identical pattern in publish-verify is covered by the 'still identifies the PR when verify was cancelled before it started' test. — Failure scenario: a refactor removes the || github.event.issue.number fallback. When the tmux job is cancelled while pending, PR_NUMBER resolves to empty, the null guard fires (exit 0), and the result comment silently does not post.
| PR_NUMBER: '${{ needs.tmux-testing.outputs.pr_number || github.event.issue.number }}' | |
| PR_NUMBER: '${{ needs.tmux-testing.outputs.pr_number || github.event.issue.number }}' # tested: see publish-verify equivalent |
中文说明
[Suggestion] tmux 的 PR_NUMBER 回退值(needs.tmux-testing.outputs.pr_number || github.event.issue.number)缺少断言其存在的测试,而 publish-verify 中完全相同的模式已被 'still identifies the PR when verify was cancelled before it started' 测试覆盖。— 失败场景:重构删除 || github.event.issue.number 回退。当 tmux 任务在等待中被取消时,PR_NUMBER 解析为空,空值守卫触发(exit 0),结果评论不会发布。
— qwen3.7-max via Qwen Code /review
yiliang114
left a comment
There was a problem hiding this comment.
Re-approve ✅
The PR has gone through 19 commits of extensive security hardening since my initial review:
- Close runtime-plant and stale-RUNNER_TEMP channels
- Close proxy-hijack, gate-bypass, and false-verdict paths
- Re-establish trust boundary after PR code runs
- Repair PIPESTATUS read and six forgeable controls
- Answer dropped /verify requests and prove proxy rejects
- Resolve hooks hermetically and mirror symlink guards at job end
- Fix silent drop and survivor accounting
The security posture has significantly improved. The earlier P1 (SKILL.md GFM rendering) and P2s have been addressed through iterative hardening. Production-ready.
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed exact head 218aa08c3d7e933bfe7a58888efe356b23a29b0b. Requesting changes for three new Critical findings inline. The existing unresolved model-proxy credential-boundary Critical also still stands at this head.
Verification performed: exact pinned actions/checkout and actions/upload-artifact source inspection; safe temporary-directory reproductions for external core.worktree clean/reset and post-sweep symlink replanting; downstream tracing through Qwen's real shell environment sanitization; 59/59 targeted workflow tests; actionlint; yamllint. The green suite does not exercise these trust boundaries.
This PR has already exceeded roughly five review rounds, so I am intentionally not adding new Suggestion-level scope.
| rm -f "$(git rev-parse --git-path config.worktree 2>/dev/null || echo /nonexistent)" 2>/dev/null || true | ||
| git config --local --unset-all extensions.worktreeConfig 2>/dev/null || true | ||
| git config --local --name-only --list 2>/dev/null \ | ||
| | { grep -ivE '^(core\.(repositoryformatversion|bare|filemode|symlinks|ignorecase|precomposeunicode|logallrefupdates|worktree|hidedotfiles|protecthfs|protectntfs)|remote\.|branch\.|extensions\.|gc\.|pack\.|fetch\.|index\.|safe\.|submodule\.[^.]+\.(url|active|branch))' || true; } \ |
There was a problem hiding this comment.
[Critical] Preserve no attacker-controlled core.worktree before the root checkout. The previous run recursively chowns the workspace, including .git/config, to node, so PR code can persist core.worktree=/__w/_temp (or another mounted path). This allowlist explicitly keeps that key. On the next run, the pinned actions/checkout defaults clean=true; for an existing repository it runs git clean -ffdx and then git reset --hard HEAD, and Git applies both to the configured worktree rather than GITHUB_WORKSPACE.
A safe isolated replay with an external temporary worktree produced Would remove outside-untracked, and the actual clean deleted that file; reset likewise overwrote a tracked file in the external target. This crosses the repository boundary and lets one verification run make the next root-owned checkout delete or overwrite persistent runner paths. Unset core.worktree before any worktree-sensitive Git command (and preferably require git rev-parse --show-toplevel to resolve exactly to GITHUB_WORKSPACE) instead of preserving it here.
中文说明
[严重] 上一次运行可写 .git/config,能够植入指向 workspace 外部的 core.worktree;本白名单又保留该值。下一次 root 权限的 actions/checkout 会在该外部路径执行 git clean -ffdx 与 git reset --hard。临时目录回放已确认外部文件会被删除/覆盖。应在任何 worktree 相关 Git 操作前清除该键,并校验真实顶层目录就是 GITHUB_WORKSPACE。
| esac | ||
| STAGE_DIR="$(mktemp -d)" | ||
| git archive "$BASE_OID" -- .qwen | tar -x -C "$STAGE_DIR" | ||
| rm -rf .qwen |
There was a problem hiding this comment.
[Critical] Keep the PR-side .qwen tree available as the subject under test. The resolver deliberately classifies .qwen/** as behavioral code that this lane exists to verify, but both pin passes replace the entire directory with HEAD^1. The skill simultaneously promises the agent that the working tree is the merge ref. For a future PR that changes another skill or project setting, normal filesystem reads and the head side of an A/B therefore see the base file; a .qwen-only PR can be compared base-vs-base and receive false clean evidence.
This is separate from correctly pinning verify-pr itself. Keep that verifier in a non-shadowable trusted location, while preserving (or separately staging and explicitly documenting) the PR's .qwen content for inspection and execution. Add an end-to-end fixture where a non-verifier .qwen file differs between base and head and prove the head cell observes the changed content.
中文说明
[严重] 解析器把 .qwen/** 视为本车道要验证的可执行行为,但这里把整个 .qwen 替换成 base,且 skill 又声称 working tree 就是 merge ref。未来修改其他 skill/配置的 PR 会在 head 侧实际读取 base 文件,甚至形成 base-vs-base 的伪绿证据。应只把 verifier 放到不可覆盖的可信位置,同时保留或单独暂存 PR 侧 .qwen 作为被测对象,并增加真实 A/B 回归用例。
| # cp -r copies symlinks as symlinks (no deref), but | ||
| # actions/upload-artifact FOLLOWS them — a node-planted link would | ||
| # exfiltrate whatever it points at into the artifact. Drop links. | ||
| find "$RUNNER_TEMP/verify-results" -type l -delete 2>/dev/null || true |
There was a problem hiding this comment.
[Critical] Stop agent descendants before treating this one-time symlink sweep as an upload boundary. The pre-agent pkill only removes lifecycle descendants. Qwen then runs in YOLO mode as node, and a tool/test can launch a detached child that closes its stdio and survives the Qwen pipeline. verify-results was chowned to node; such a child can wait for this find ... -delete to remove a marker symlink and immediately replant a top-level link before the next upload-artifact step.
The exact pinned upload action sets followSymbolicLinks: true and uses fs.stat, so its root-running process dereferences the replanted link. A safe watcher replay deterministically restored a link after the sweep. That reopens root-readable file exfiltration into the artifact and leaves the final root cleanup racing a live PR-controlled process—the same reason the earlier lifecycle one-shot sweep was not sufficient. After Qwen exits, kill/fail closed on every remaining node process, then stage into a fresh root-owned, non-node-writable upload directory before stripping links and uploading.
中文说明
[严重] agent 以 node + YOLO 运行,可启动脱离会话的后台子进程;而上传目录仍归 node 所有。该子进程可等待本次一次性 symlink 删除完成后重新植入链接。固定版本的 upload-artifact 明确跟随符号链接,并以 root 读取目标,从而重新打开 artifact 越权读取与末尾 root 清理竞态。应在 agent 结束后再次终止并确认无 node 进程,再把产物冻结到 root 所有、node 不可写的新目录。
…LM#7753) * feat(triage): add sandboxed /verify deep-verification lane @qwen-code /verify on a PR now runs a local-verification-style evidence round in the isolated /tmux sandbox contract (container, token-free agent env, loopback model proxy, author-write gate) and publishes the report via a separate PR-code-free job: - new verify job: merge-ref checkout at depth 2 (base tip + PR head for A/B), skills pinned from base so the tree under test can never rewrite its own verifier, PR-planted tmp/*-verify-* artifacts dropped, git exec-vector sweep for the persistent workspace, agent verdict allowlisted before it reaches workflow outputs - new publish-verify job: upserts one marker comment (running status -> final report), HTML-escapes the untrusted report, reports skip/na/ prepare-fail/infra outcomes explicitly since /verify is always an explicit request - new verify-pr skill: A/B load-bearing proof, vacuity check on new tests, mock-free wire-oracle harnesses, targeted gates, fixed report/ verdict/assertions artifact contract, counts-are-sacred rules - triage skill Stage 2c now names /verify (not just /tmux) as the trigger to recommend when a PR's central claim needs behavioral evidence The verify check-runs ride the issue_comment event, which the finalize workflow's event == "pull_request" universe structurally excludes, so they cannot pollute the CI table or the deferred-approval gate. * feat(triage): teach /verify round continuity and artifact-matched methods Fold two more hand-verification patterns into the verify lane: - round continuity: the resolve step snapshots the previous verify report (if any) into the agent context before the status upsert overwrites it, and the skill re-checks each prior finding at the new head (fixed/stands/superseded), scoping new probes to the delta - harness quality: prefer configuration seams over module interception, encode the upstream's real semantics in the fake peer, add decoy targets - artifact-matched methods: per-commit load-bearing tables for multi-commit PRs; workflow/CI PRs get embedded-script replay against real data, repo lint gates, and day-one trigger cost math from real event history; every new config knob must trace to an observable effect, and default-path dispatch combinations get probed - findings quality: blockers enumerate blast radius, demonstrate the sharpest consequence end-to-end when budget allows, and carry a collapsed minimal suggested fix preserving the original commit's intent * feat(triage): host /verify evidence images and encode quantified-A/B rules Borrow the image-evidence and quantified-verification patterns from hand-run rounds (QwenLM#7265, QwenLM#7471, QwenLM#7686 r2 and the pr-assets convention): - publish-verify now hosts agent-produced evidence/*.png on the pr-assets branch (verify/pr<N>-<run>-<attempt>/) and appends them below the escaped report. Untrusted-payload discipline: strict filename allowlist, 8-image / 2 MB caps enforced in the find predicates, racing-push retry, and every failure degrades to a text-only comment. VERIFY_ASSETS_REMOTE is a test seam; the block was dry-run against a local bare remote covering hosting, hostile filenames, oversize files, dotfiles, missing branch, and no-image runs - skill: evidence images are named as kebab-case captions binding image to claim, before/after pairs over lone after-shots; follow-up rounds lead with a previous-finding status table (fixed/stands/superseded/declined, with adjudication) and re-measure instead of diffing the old report; size/perf claims get measured-metric Δ tables with residual deltas accounted for; unreachable branches get the configuration that reaches them constructed; defensive guards get their accept path checked against real production artifacts, not just mocked rejects * fix(triage): address /review suggestions on the verify lane - skill: local invocation resolves --repo and passes it to every gh call - skill: call out the dependency confound when the base A/B side reuses the PR-installed node_modules and the PR touches package.json/lockfile - workflow: document the pin step's bootstrap logic — issue_comment jobs run the default branch's YAML, so base always carries the verify-pr skill by the time this job exists * fix(triage): harden /verify gate, comment budget, and evidence hosting per review Address review round 5078770575 items 1-3 plus the cheap follow-ups: - authorize: /verify now requires write from BOTH the PR author (whose code runs) and the commenter (who spends a scarce runner slot + model budget) — a drive-by account can no longer burn 45 minutes of ecs-qwen on someone else's PR; duplicates check once; /tmux and /triage gates unchanged. Replayed 8 principal scenarios against a stubbed gh - authorize acks /verify with the eyes reaction from the always-hosted job, so a queued/saturated sandbox pool no longer means total silence - publish: emit_block escapes FIRST and caps the escaped size (45 KB for the report) — a raw-side cap let dense <>& content inflate past GitHub's 65,536-char comment limit, 422 the post, and strand the running status with no report at all; iconv -c keeps a UTF-8 sequence split by the byte cut (likely, given the mandated 中文 summary) from shipping broken; replayed: 50 KB dense report -> 45,873-byte body - publish: image cap is byte-exact (-size -2097153c; find's -2M rounds sizes UP to MiB, silently making the documented 2 MB cap 1 MiB), bytes must carry the PNG magic (extension is attacker-choosable), duplicate sanitized names dedupe instead of overwriting + double-rendering, and dropped images are reported in the comment instead of vanishing - publish: weak terminal notices (cancelled/infra/skipped/n-a) only replace this run's own running status; a previous round's real report survives as the marker comment and the notice posts fresh - publish: report.md/assertions.json lookups pin the artifact-dir shape and sort (bare find -name order is filesystem-dependent); the verify job's verdict.txt lookup sorts likewise - verify: global npm install runs from RUNNER_TEMP (the persistent workspace still holds the PREVIOUS run's tree, whose .npmrc would apply to a root install); both cleanup passes remove leftover tmp/ worktrees (git worktree prune alone only drops metadata); the run step no longer re-chowns 50k node_modules files; pr-assets clone sets its committer identity once so the racing-push rebase retry can commit - skill: worktree guidance now tells the agent to remove its base tree itself, with the workflow sweep as backstop only * fix(triage): close runtime-plant and stale-RUNNER_TEMP channels in /verify Address review round 2 (comment 5079157987) and the CHANGES_REQUESTED round on the verify lane: - run step re-sweeps tmp/*-verify-* AFTER npm ci/build and before the agent starts: the pin step's sweep runs before PR lifecycle scripts (postinstall etc.), which could re-plant a fake artifact dir whose zeroed timestamp deterministically wins the sorted collector. From the sweep on, only the agent writes those dirs; a steered agent forging its own artifacts remains the documented advisory-report residual - RUNNER_TEMP verify-results/verify-context are rm'd before mkdir: the pool is persistent and runner temp hygiene is runner-managed — a stale report or previous-report.md from ANOTHER PR must never ride along - symlinks are stripped from verify-results before upload: actions/upload-artifact dereferences them, so a node-planted link would exfiltrate whatever it points at into the artifact - a trusted commenter invoking /verify on a PR whose author lacks write now gets an explanation comment from the hosted authorize job instead of total silence (the commenter is checked first; drive-by accounts and API errors still get nothing); job timeout 45->60 so a slow install can never let the JOB limit kill the agent past its own graceful 25m budget - stale tmp/base-tree (skill's canonical scratch worktree) is removed by name at job start — a plain dir isn't git-registered, so the worktree sweep alone misses it and the next worktree add would fail - scripts/tests/qwen-triage-workflow.test.js gains a verify-lane describe block: an 8-arm stub-gh replay of the dual principal gate (drive-by deny, author-without-write deny + explain flag, self-comment dedupe, 404 fail-closed, /tmux and /triage unchanged) plus guards for the post-prepare sweep placement, the symlink strip, and the RUNNER_TEMP resets — the replay found this commit's sweep edit had silently not applied, which is exactly the regression class it exists to catch * fix(triage): close proxy-hijack, gate-bypass, and false-verdict paths in /verify Address the Codex /review round (19 findings) and the bot's follow-up. Each fix was replayed locally; the proxy fix has a decisive A/B. Gate and routing: - the shell command match is case-insensitive: GitHub Actions expression comparisons ignore case, so `@QWEN-CODE /VERIFY` reached the step and fell through to the commenter-only branch — running the PR author's code with the author never checked - the verify ack and denial notice require github.event.issue.pull_request: /verify on a plain issue was acknowledged but could never report - publish-verify joins the verify job's per-PR concurrency group, and a failed PATCH falls back to posting fresh instead of going silent Untrusted-input paths: - the model proxy binds an EPHEMERAL port, reports it through a root-owned file, and its health check must echo a per-run nonce with the recorded PID alive. A/B with a squatter on 8787: the old code's proxy dies EADDRINUSE yet still reports enabled and points qwen at the squatter; the new code comes up unaffected on an ephemeral port - worktree-scoped git config is deleted before hooksPath is resolved: `extensions.worktreeConfig` is allowlisted and .git/config.worktree is invisible to `git config --local`, so a prior run could set core.hooksPath=/ and make the hook sweep's recursive delete walk / as root (verified locally). The sweep now also refuses any hooks path outside the repository's git dir - marker-comment lookups accept only bot-owned comments that START with the marker: any user can paste the marker and divert the bot into PATCHing a stranger's comment - the upload staging dir is re-flushed after npm lifecycle scripts Honest verdicts: - the docs-only classifier no longer uses a pipeline (grep -q made the writer take SIGPIPE, so under pipefail a long file list with an early code file classified a code PR as docs-only and skipped verification), and executable markdown/YAML (.qwen, .github/workflows, scripts) is classified as behavioral before the extension rule - tee's status is checked alongside qwen's: a full results volume made a truncated evidence stream publish as pass - 137 is split by elapsed budget into watchdog timeout vs crash/OOM - the agent's verdict is honored only for VERDICT=pass with a report and zero failed assertions; otherwise the process outcome headlines and the scope paragraph says the run did not complete - verdict.txt is read through a bounded prefix (SIGPIPE under pipefail) Skill contract corrections: per-commit tables only when the commits are reachable at depth 2 (else aggregate + Not covered); internal workspace symlinks must have their realpaths asserted before a base control is trusted; repo lint gates and event-history cost math are qualified to what the token-free container can actually run; --repo is never inferred from `origin` (a fork holds a different PR under the same number). Tests: 9 new guards, all mutation-verified (reverting each fix turns one red), including an executable escaping/size-cap/UTF-8 test for the publisher's own emit_block and a fix to the earlier command-file test, which matched the tmux job's identically named step. * fix(triage): re-establish the /verify trust boundary after PR code runs Third review round (31 findings). The unifying defect: everything the lane pinned or swept happened BEFORE npm ci/build executed PR-authored lifecycle scripts as node, so each control could be undone afterwards. Trust boundary, rebuilt in order before the agent starts: - kill every process owned by the build user and fail the step if any survives — a detached postinstall child could otherwise wait out each one-shot sweep and re-plant afterwards - re-pin .qwen from the base commit again, now root-owned and read-only: the prepare step chowns the workspace to node, so a lifecycle script could rewrite the very skill that defines /verify-pr - give the agent a fresh HOME/QWEN_HOME: qwen loads user-scope file commands from $HOME/.qwen, and /home/node belongs to the build user, so a planted commands/verify-pr.toml could shadow the pinned skill - the model proxy now requires a per-run bearer token, closing the blind-localhost-scan path to an unauthenticated signer for the real model credential (a command the agent itself launches still inherits it — documented residual, not closed) Authorization and lifecycle: - re-verify the PR author's write permission at execution time and pin the authorized head OID; refuse if the checked-out HEAD^2 differs, so a push during the runner wait cannot smuggle in unreviewed code - validate each principal separately: an empty author vanished in word splitting and left only the commenter checked - honor MAINTAINER_ECS_RUNNER_DISABLED with an explicit notice instead of queueing forever against a disabled pool - status comments carry a machine state marker; inferring 'running' from prose let a report quoting that sentence be overwritten - previous-report.md snapshots the newest substantive report, never a weak/cancelled notice, so prior findings survive into the next round - bot-identity lookup failures fail closed instead of widening the ownership filter to every user's comments - publish-verify uses a per-run concurrency group: a per-PR group holds only one pending job, so a second /verify could cancel a completed run's pending publisher Correctness: - install/build failures are classified: signals, ENOSPC, registry and network errors are infra-error, not a PR verdict - watchdog classification measures the child's own elapsed time, not shell-global $SECONDS which includes proxy setup - assertions.json must be three non-negative integers with a positive total and total == pass + fail before it counts as evidence - the proxy keeps its upstream deadline armed until the body ends and aborts upstream when the client disconnects - cleanups remove .qwen/tmp itself: PR code can make it a symlink, and globbing below it deleted the target's contents as root (verified) - emit_block materializes the escaped text and truncates on a character boundary via node — iconv -c passes an incomplete trailing sequence through on BSD (measured), which the new test caught Skill: local mode requires the same isolation CI provides and must not assume HEAD^1/HEAD^2 on a plain head checkout; shallow boundaries make rev-list counts unreliable for per-commit claims; never run scripts/lint.js with no arguments (it runs prettier --write and rewrites the tree under the harnesses); a vacuity check must fail the intended assertion, not the import. pr-workflow.md now says both sandboxed lanes need the author to have write, so triage stops recommending a guaranteed denial on external PRs. Tests: 9 more guards, all mutation-verified, including executable replays of the docs-only classifier (SIGPIPE + executable-markdown cases), the uppercase-command gate, the empty-principal deny, and the untrusted-image hosting path against a bare pr-assets remote. * test(triage): pass the classifier fixture through a file, not argv The new docs-only classifier replay passed on macOS and failed on CI with `Cannot read properties of undefined (reading 'trim')`: its 60,001-entry fixture is ~889 KB and was passed as a single argv element. Linux caps one argument at MAX_ARG_STRLEN (128 KB), so the spawn failed with E2BIG and stdout was undefined; macOS has no per-argument limit and only a ~1 MB total, so the same call succeeded locally (verified both). Write the list to a temp file and pass the path. The harness now also asserts the spawn succeeded, so a future spawn failure reports itself instead of surfacing as a TypeError on undefined output. * fix(triage): make the /verify report match what the run actually produced Three publisher findings, all introduced by my own previous round: - an artifact download failure (the step is continue-on-error) let the full-report path run with no results: the headline read 'completed' and the scope paragraph claimed the A/B, the harnesses and the gates had run when nothing had been delivered. The download outcome is now an input, and its failure gets its own body saying the results could not be retrieved - the prepare-failure branch ignored the verdict the prepare step had just computed, so an install killed by a registry outage or OOM (classified infra-error) still told the author 'this is treated as a PR failure verdict rather than an infrastructure failure' — the exact opposite. It now branches on the verdict, and an infra-classified prepare failure is a weak body that cannot overwrite a real report - weak notices were being snapshotted as the follow-up round's previous-report.md: they lack the running marker, so 'newest non-running comment' selected them. Bodies that carry findings now mark themselves (qwen-triage:verify-substantive) and the snapshot selects on that marker. A/B on the real jq: report A then cancelled B now snapshots A (101), the old filter picked B (102) Tests: 4 more guards, all mutation-verified — the publisher is rendered for each outcome with a stubbed gh and the assertions read the body it would post, and the snapshot test runs the workflow's own jq program verbatim against a paginate-shaped fixture. * fix(triage): stop PR build output from masquerading as an infra failure Two review findings plus a test-helper hazard: - classify_failure grepped the prepare log for bare words like ENOSPC and ETIMEDOUT, but that log is written by PR-controlled code: a genuine build failure that merely prints 'expected ETIMEDOUT to equal ok' would be published as an infrastructure incident, telling the author to re-run something that fails identically. The patterns are now anchored to lines only npm's reporter or the kernel emits ('npm ERR! code E…', 'npm ERR! network …', kernel OOM, bare 'Killed'); a signal exit still needs no log evidence. Replayed 10 cells: four PR-authored logs quoting infra words stay 'fail', five real diagnostics and one signal exit are 'infra-error' - the two execution-time controls added last round — re-verifying the author's permission after the runner wait, and refusing a head that moved since authorization — had no tests. Both are now executed: the re-auth snippet against a stubbed permission API (write proceeds and pins head_oid; read skips with a publishable reason), and the pin step against a real git repo with a real merge commit (matching head proceeds, moved head exits non-zero) - add a stepIn(job, step) test helper. Several step names exist in both the tmux and verify jobs, and the unscoped step() returns the first match, so a verify-lane assertion silently tests the tmux copy — that has now bitten this suite three times, including in this commit. * docs(triage): teach verify-pr test-only PRs, differential oracles, gate liveness Fold techniques from the round-2 verification on QwenLM#7620 (an ANSI parser PR) that the skill had no equivalent for: - test-only PRs get their own method: a mutation A/B across TEST FILES (same mutants of the unmodified production file, only the test file swapped), reporting killed/total on both sides, requiring that no mutant regressed from killed to survived, checking that the killing assertion is the one the commit claims to have strengthened, and adjudicating every survivor as coverage gap or defect with independent evidence rather than by inspection - when the code emulates a known implementation, that implementation is the oracle: feed identical input to both and report disagreement counts per side, lift reference tables verbatim out of the shipped dependency, and build the corpus from bytes captured off a real producer alongside synthesized sweeps - prove a gate is live before citing it: plant a violation the linter must catch, confirm it is reported, remove it — a linter that matched no files exits 0 exactly like one that passed - attribute pre-existing failures by byte-identical failing file AND test names on both sides, with deltas, not just totals - when the base is far behind, verify the merge: trial-merge into current main, confirm it is conflict-free, and re-run the affected suite on the merged tree - round continuity gains its one legitimate shortcut: a production file proven byte-identical (sha256 quoted at both heads) carries prior evidence forward by construction * style(triage): reflow verify-pr skill to prettier's markdown wrapping The previous commit's added paragraphs were hand-wrapped and prettier --check flagged the file; the repo runs prettier over all of it. * test(triage): cover the disabled-runner-pool notice The kill-switch path had no test: a refactor could drop the notice and leave a /verify request acknowledged with 👀 but permanently unanswered, since the verify job refuses to start and publish-verify skips with it. Fold the step into the existing PR-guard loop (now scoped through stepIn, so it cannot match a same-named step in another job) and assert the parts that make the answer useful — the kill-switch and permission conditions, both languages, the alternative it points at, and the verify job's own exclusion of the disabled pool. All three mutations turn it red: removing the step, dropping its PR guard, or letting the verify job queue against the disabled pool. * fix(triage): repair a step-killing PIPESTATUS read and six forgeable controls Sixth review round, 12 findings. Several are regressions from my own two previous rounds; the first would have broken every single run. - `AGENT_STATUS=${PIPESTATUS[0]}` is itself a command and resets PIPESTATUS, so the next line's ${PIPESTATUS[1]} was unset and `set -u` aborted the step immediately after the agent finished — before artifact collection, the verdict, or anything else. Verified by replaying the exact structure: 'PIPESTATUS[1]: unbound variable'. Both elements are now snapshotted in one command - concurrency predicates were broader than the job conditions they guard, and GitHub evaluates concurrency BEFORE the job `if`: a /verify comment entered the triage job's shared per-PR group (where it could displace a pending /triage and then skip), and a /verify queued while the runner kill switch was on did the same to a real verification. Both predicates now match their job's runnable set exactly - an outward-resolving .git/hooks entry was only warned about and left in place, so the next root-owned git command would run it. It is now unlinked without traversing its target, a root-owned hooks directory is restored, and core.hooksPath is unset - the second .qwen pin re-derived HEAD^1 from git metadata after the workspace, including .git, had been handed to the build user. The base OID is now recorded while .git is still root-owned and the re-pin archives that content-addressed OID - classify_failure took both of its inputs from PR-controlled sources: a lifecycle script can exit with a signal status and can print any line the log patterns matched, turning its own deterministic breakage into 'infrastructure, please re-run' — which hid the failure and preserved a stale report. No infra verdict is derivable there, so the prepare step reports `fail` and lets the embedded log speak for itself - cleanups descended through PR-writable parents: `.qwen` itself can be a symlink, and the worktree sweep trusted git metadata with only a lexical prefix check. Symlinks are unlinked without traversal and worktree paths must canonicalize inside the workspace. Replayed all three escapes - skipped and docs-only outcomes upload no artifact, so the new download-failure branch pre-empted them and made their real reason unreachable; they are answered first now - a run that crashed before writing report.md still claimed the substantive marker, letting a headline overwrite the previous round's evidence. The marker now requires a report Skill: the byte-identical shortcut needs the whole input closure, not one file hash; the credential-free local path cannot call `gh` at all (fetch the metadata outside and mount it read-only); and the A/B base is `baseRefOid` in local mode, not `HEAD^1`. Tests: 7 new guards plus 4 updated to the new shapes, all mutation-verified (50/50). * fix(triage): answer dropped /verify requests and prove the proxy rejects Maintainer review (yiliang114), 7 items: - a third /verify while two runs are in flight is dropped by the concurrency group with no job and therefore no comment. The hosted authorize job now counts this workflow's other in-flight runs and says so; an API hiccup leaves the request alone rather than denying it - the proxy's bearer check had no executable test. It now starts the real proxy against a real upstream and issues real requests: no header and a wrong token are 401, this run's token is 200, and a route other than /chat/completions is 403 — with the health endpoint echoing the nonce - the 502 path forwarded the raw upstream error, which can name resolved hosts and TLS detail to PR code. It logs server-side and returns a generic failure - publish-verify inherited the 360-minute default; it downloads one artifact and posts one comment, so it is bounded at 10 - removing the log classifier last round left the comment block it replaced, which still said failures are classified from the exit status and the log. Deleted - that removal also left every install failure reported as the PR's fault, including a registry outage. There is exactly one signal here PR code cannot write — asking the registry ourselves, as root, with the container's resolver — so an install failure is downgraded to infra-error only when that probe fails. It proves reachability now rather than at failure time, so it can only ever downgrade, never confirm; a build failure has no equivalent and stays the tree's problem - the skill's local-invocation warning ran into the preceding sentence, which GFM renders as one paragraph Tests: 5 new guards, all mutation-verified (55/55). * fix(triage): resolve hooks hermetically and mirror symlink guards at job end Maintainer review round (doudouOUC), 6 findings. Two were Critical and both reproduced: - the hooks sweep resolved its path with the ambient git config in play. With a global core.hooksPath set — which the reviewer has and I do not, which is why my earlier replay showed a false pass — `git rev-parse --git-path hooks` returns that global path, the in-git-dir guard reads 'outside', and a planted `.git/hooks` symlink survives untouched. A/B: old code leaves the symlink under a global hooksPath, new code removes it in both environments and never touches the link target. Resolution now runs with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM pointed at /dev/null - the END-of-job cleanup still used the bare `rm -rf .qwen/tmp` that the start-of-job cleaner was hardened against two rounds ago. The agent executes PR code between the two, so the end is no safer than the start: it now unlinks symlinks without descending and canonicalizes worktree paths inside the workspace before deleting Plus four suggestions, all valid: - the saturation notice counted this workflow's in-flight runs across every PR while the concurrency group is per-PR, so a run on another PR would trigger a warning about a queue that does not exist. It now matches on the PR title (the only per-PR handle an issue_comment run record carries) and stays silent when that cannot be resolved - the skill recommended `require.resolve` for the workspace-realpath check; these packages are ESM-only with import-only exports, so it throws ERR_PACKAGE_PATH_NOT_EXPORTED and reads like a missing module. Verified, and replaced with `readlink -f node_modules/@qwen-code/...` - the symlink-escape test inherited the developer's git config, which is what hid the first finding. It now runs with global/system config neutralized AND repeats the case with a global core.hooksPath planted - the publisher's build-phase arm was never rendered by any test (every case used 'install'), so a typo in that command name would have shipped. Now covered, along with an unrecognized phase Mutation-verified 4/4. The hooks guard needed a discriminating assertion: git's own `*.sample` files must survive the sweep, because the outward-path fallback removes the whole directory and would otherwise satisfy a bare 'planted hook is gone' check. * fix(triage): count only /verify runs for saturation, and test the PATCH arm Bot review round, 2 suggestions, both valid: - the saturation notice matched runs by PR title, which narrowed to this PR but not to /verify. /triage and /tmux live in their own concurrency groups, so two of those in flight would warn about a verify queue that is actually empty. It now also requires the run to have a job named 'verify' — the run record carries no command, but its job list does. Replayed: two non-verify runs stay silent, two verify runs warn - every publish fixture returned an empty comments listing, so the PATCH arm was never executed: a broken PATCH would have stranded the running status comment and posted a duplicate below it, with the suite green. The publisher now runs against a stubbed listing and the test asserts which verb went to which comment id — bot-owned live status is PATCHed in place, an absent comment posts fresh, and a marker comment owned by someone else is left alone and posted around Mutation-verified 3/3: counting every command, never PATCHing, and accepting foreign-owned markers each turn one test red. Two stub bugs found while writing these, both mine and both silent: ${*#pattern} applies per positional parameter rather than to the joined string (yielding a wrong run id), and the paginate fixture needs one array per page, not an array of pages. * fix(triage): fix the real silent drop and drop the step built on a wrong premise Review round 4. The blocker was mine twice over: the saturation notice I added last round had GitHub's concurrency semantics backwards, and the silent drop it claimed to cover was somewhere else entirely. - GitHub cancels the OLDER pending run in a group and admits the new one (confirmed against the workflow-syntax reference). My step told the person who had just typed /verify that their request might be dropped, when theirs is the one that runs — and said nothing to the person whose queued run actually died. This PR already had it right in publish-verify's own comment, so the file contradicted itself and the user-facing copy followed the wrong half. The step is removed rather than reworded: with the fix below there is nothing left for it to warn about, and it cost 2+N API calls on every /verify. - the actual drop: a verify job cancelled while still PENDING never reaches a runner, so its outputs block — where the "|| github.event.issue.number" fallback lived — is never evaluated. publish-verify then read an empty PR_NUMBER, hit its own guard and exited 0, making the cancelled branch unreachable in exactly the scenario that produces cancellations. The fallback now lives where the value is read. Reproduced both arms by executing the real step: with a number the cancelled notice posts, with an empty one it only warns. - same one-line class in publish-tmux, fixed alongside. Two copy defects from the classifier removal, both mis-attribution pointed the other way: - the infra-error body still named a signal/OOM kill and a full disk, none of which the current prepare step can produce — infra-error now requires npm ci to fail AND the registry probe to fail. It names that condition only, and offers a re-run instead of asserting it is the fix. - the code comment above it still described the deleted classifier. Also fixes the indentation break an earlier scripted edit left in the publish body builder, and replaces the saturation test with one that executes the cancelled path. Mutation-verified 2/2; the copy needed its own guard, since reverting the wording alone left every test green. * docs(triage): teach verify-pr survivor accounting and observability regressions Fold techniques from the re-verification on QwenLM#7709 that the skill had no equivalent for: - the mutation matrix must report the mutations that changed NOTHING, not only the ones that failed. Each survivor gets classified as an ordinary coverage gap or as dead code — a guard whose deletion leaves every test green is one of those two, and the difference is what the author needs. Survivors mirroring a pre-existing gap are labelled as such, and the set is framed as completeness reporting rather than merge conditions - the sharper case that report demonstrates: a test that passes for the WRONG REASON. If deleting the new guard leaves its own new test green, that test is pinned by an earlier early-return, not by the change, and asserts nothing about it. Name what actually pins it - and do not generalize from one dead guard to its siblings: the same report shows a clause that is unreachable on one path while being the only protection on another. Check each, report the contrast - observability regressions: when a change suppresses output, follow the value before calling the suppression correct. A bare catch on the path plus a field with no readers anywhere in the repo means the cause is now unobservable even in devtools — a real loss that no behavioural assertion can see - report structure gains a Corrections section: when an earlier round or bot comment described the code inaccurately, state the correct fact with evidence and label it as a correction to the description, not a request to change code. A wrong description left standing costs the next reader more than the original finding did * fix(triage): carry the /verify lane's hardening across to /tmux The /tmux job executes the same untrusted PR code, as the same user, on the same persistent self-hosted pool as the /verify lane that QwenLM#7710 hardened. Five of those controls had no equivalent here. Each was found on the verify side by reproducing an attack or a failure, not by reading the code, so the same evidence applies unchanged. - the model proxy bound a FIXED port (8787). PR lifecycle scripts run before it, so a detached child can squat that port: the real proxy then dies with EADDRINUSE while the health probe succeeds against the squatter, and the agent takes its chat completions. Now an ephemeral port published through a root-owned file, a per-run nonce the health endpoint must echo, and a liveness check on the PID we started. Replayed with 8787 occupied: the proxy comes up on an OS-chosen port and answers with the nonce. - nothing swept planted artifact directories. npm ci/build run the PR's lifecycle scripts, which can create tmp/<name>-tmux-<ts>/ holding a report.md and a transcript; the collector globs *-tmux-* and the publisher takes the first match, so a planted directory could supply the comment's contents. Swept after the last PR-controlled process and before the agent. - the global npm install ran with the workspace as cwd, where the PREVIOUS run's checked-out tree still sits. npm reads a cwd .npmrc, and a --registry flag does not override script-shell or hooks, so that config reached a root-privileged install. It now runs from RUNNER_TEMP. - the end-of-job cleanup globbed below .qwen/tmp. PR code ran in this workspace, so either .qwen or .qwen/tmp can be a symlink out of the tree — verified on the verify lane, where the glob deleted the link target's contents as root. Symlinks are unlinked without descending. - emit_block capped the raw log then escaped it. Escaping inflates every & < > by 4-5 bytes, so dense content can push the assembled body past GitHub's 65,536-character comment limit, 422 the post, and leave no comment at all. It now escapes first, caps the escaped bytes, and truncates on a character boundary via node — BSD iconv -c passes an incomplete trailing UTF-8 sequence through unchanged. Tests: a tmux-lane-parity suite, all six mutations verified (restoring the fixed port, dropping the sweep, moving the install back, dropping the symlink guard, reverting to a raw-side cap, and dropping the character-boundary truncation each turn one test red; a no-op control correctly changes nothing). One pre-existing assertion updated: it pinned emit_block's old inline-capture shape, and the guarantee it protects — a render failure is caught — is asserted in the new form. Also adds the regression guard for the publish-tmux PR_NUMBER fallback that landed in QwenLM#7710 without one: a job cancelled while pending never evaluates its outputs, so without the fallback the result comment silently does not post. * fix(triage): address review — symlink guard, artifact strip, bearer auth (QwenLM#7753) * fix(triage): address R2 review — proxy parity, bearer wire tests, process kill (QwenLM#7753) * fix(triage): address R3 review — publisher parity, dedup ownership, cap budget tests (QwenLM#7753) * fix(triage): address R4 review — drop redundant tmux-lane .mjs guards (QwenLM#7753) * fix(triage): address R5 review — tmp symlink sweep guard, proxy timer clear (QwenLM#7753) * fix(triage): address R6 review — hoist proxy timer out of try, dead-upstream 502 tests (QwenLM#7753) * fix(triage): address R7 review — make proxy watchdog idle, end stalled response (QwenLM#7753) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
|
Released in v0.21.1. |
|
This is just the CI bot confirming that PR #7710 (feat(triage): add sandboxed /verify deep-verification lane) has been released in v0.21.1. No code changes or reply needed — purely informational. Acknowledged. ✅ ✅ completed |
What this PR does
Adds an on-demand deep-verification lane to the triage workflow: commenting
@qwen-code /verifyon a PR now runs a maintainer-grade evidence round against the PR's real build — an A/B load-bearing proof against the base side, a vacuity check on new tests, mock-free wire-oracle harnesses, and targeted workspace gates — and publishes the report back to the PR as one upserted comment (live "running" status → final report). The report is explicitly advisory evidence for human reviewers: it is never a review, an approval, or a CI check.Concretely: two new jobs in
qwen-triage.yml(verifyexecutes the PR inside the same isolation contract as the existing/tmuxjob — container, no GitHub token in the agent env, model key behind a loopback proxy that only allowsPOST /chat/completions, gated on write permission from both the PR author (whose code runs) and the commenter (who spends a scarce runner slot plus a model budget);publish-verifyposts the report from a clean hosted runner that never checks out PR code), plus a newverify-prskill encoding the methodology and artifact contract, and a Stage 2c update so the static triage agent recommends/verifywhen a PR's central claim needs behavioral evidence.Hardening built in from the start: the agent's skill is pinned from the base branch via
git archive HEAD^1so the tree under test can never rewrite its own verifier; PR-committedtmp/*-verify-*directories are deleted after checkout so a PR cannot pre-plant a fake report that the artifact collector would publish; the persistent self-hosted workspace gets the same git exec-vector sweep (config allowlist + hook removal) the triage job already uses; and the agent-written verdict is allowlisted (merge-ready|findings|blocked|inconclusive) before it touches workflow outputs.The second commit folds in patterns from recent hand-run verification rounds (#7632 round 2, #7656): re-running
/verifyis a follow-up round — the resolve step snapshots the previous report before overwriting it, and the skill re-checks each prior finding at the new head (fixed/stands/superseded); harnesses prefer configuration seams (abaseUrl, an env var) over module interception with the fake peer encoding the upstream's real semantics; multi-commit PRs get per-commit load-bearing tables; workflow/CI PRs get embedded-script replay against real data, the repo's own lint gates, and day-one trigger cost math from real event history; blockers enumerate their blast radius and carry a collapsed minimal suggested fix.The third commit adds image evidence, borrowing the layout and hosting conventions from image-bearing PRs and rounds (#7265, #7471, #7686 round 2): the agent may save
evidence/*.png(kebab-case filenames that double as claim-binding captions; before/after pairs preferred), andpublish-verifyhosts them on thepr-assetsbranch underverify/pr<N>-<run>-<attempt>/and renders them below the escaped report. Untrusted-payload discipline throughout: strict filename allowlist, 8-image / 2 MB caps, racing-push retry, and any hosting failure degrades to a text-only comment. It also encodes the quantified-verification rules from #7686 round 2: follow-up rounds lead with a previous-finding status table and re-measure instead of diffing the old report; size/perf claims get measured-metric Δ tables with every residual delta accounted for; unreachable branches get the configuration that reaches them constructed; defensive guards get their accept path verified against real production artifacts, not just mocked rejects.Why it's needed
The triage agent is static-analysis-only by design — its environment carries a write PAT, so it must never execute PR code, and its test evidence is limited to reading the PR's own CI. The only behavioral lane today,
/tmux, is scoped to TUI surfaces and text captures. Maintainers currently produce deep verification rounds (A/B against base, wire oracles, load-bearing proofs — e.g. the round-3 verification on #7586) entirely by hand. This PR makes that style of evidence available on demand in CI, with the isolation guarantees the hand-run version doesn't need to think about, and without widening the existing approval surface: the verify check-runs ride theissue_commentevent, which the finalize workflow'sevent == "pull_request"universe structurally excludes from both the CI table and the deferred-approval gate.Reviewer Test Plan
How to verify
Static/replay verification (all reproducible locally):
node scripts/lint.js --actionlintand yamllint with the repo config pass; the only shellcheck classes actionlint reports (SC2016/SC2129) are in the lint script's explicit ignore set and pre-exist in the cloned tmux blocks.run:blocks belonging to this lane (authorize, verify, publish-verify), extracted from the YAML, passbash -nandshellcheck -S warning; the embedded loopback-proxy JS passesnode --check. The independent Linux replication extended this to all 29 blocks in the workflow.publish-verifyscript was executed (not just read) across all six terminal branches — cancelled, infra-failure, skipped, n/a, prepare-fail, report — with a stubbedgh; the rendered bodies are correct and report content containing</code></pre></details>,@everyone, and<img onerror=…>stays fully entity-escaped inside the<details><pre><code>block.PWNED @everyone,findings; rm -rf /→ discarded); the assertions jq coerces non-numeric JSON ({"pass":"<script>"}) to zeros; the docs-only decision grep was replayed against mixed/docs-only/empty file lists with GNU grep semantics.Live end-to-end run (local replica of the CI job, faithful to every contract knob — merge-ref worktree of #7547 at depth-2 parents,
.qwenpinned from this branch,QWEN_VERIFY_CONTEXTmetadata snapshot, credential-lessgh, isolatedQWEN_HOME): the agent (qwen3.8-max-preview) completed in 25 turns / 3.5 minutes API time, produced every artifact in the contract, and returned verdictmerge-readywith 318/318 assertions. The numbers were then audited adversarially: 318 = 304 real vitest tests (23+68+213 across three targeted gate files, log-verified) + 13 A/B assertions + 1 vacuity check; re-running the agent's ownab-harness.mjsreproduced 13/13; reverting the PR's fix and re-running that same harness went exactly 7/13 (the 3 PR-side and 3 flip assertions failed), and restoring returned 13/13 — proving the A/B senses the fix rather than comparing a file to itself. The agent made zeroghinvocations. Feeding the real artifacts through the actualpublish-verifyscript rendered the exact 4.7 KB comment CI would post.Evidence (Before & After)
Agent's A/B harness, rerun by hand at head and under mutation (fix reverted, then restored):
Publish rendering of the real run's artifacts (headline of the comment CI would post):
Tested on
Linux detail: the static and replay layer (YAML parse, 36/36 workflow tests, all 29 extracted
run:blocks throughbash -n, both proxy heredocs throughnode --check, and the verdict-allowlist replay) was independently replicated on Linux atc25afbd8with zero failures. What stays untested there is the live path — the jobs running on a real Actions runner with the runner secrets — which nothing before merge can cover.Environment (optional)
macOS 15 / Node 22 / qwen CLI 0.20.1; agent model
qwen3.8-max-preview(DashScope); verification worktree built with a realnpm ci+npm run buildfrom the PR lockfile.Risk & Scope
pr-assets(v1 ships text + run artifacts); the adjacent pre-existing gap that the tmux job's*-tmux-*artifact collection can likewise be spoofed by PR-committed directories — same attack class fixed here for verify, left for a separate PR on the tmux side.Linked Issues
The methodology this encodes is the hand-run deep-verification style used on recent PRs (e.g. the round-3 verification comment on #7586). No issues closed.
中文说明
本 PR 做了什么
为 triage workflow 增加一条按需触发的深度验证车道:在 PR 上评论
@qwen-code /verify,即可对该 PR 的真实构建执行一轮维护者级别的证据验证——与 base 侧的 A/B load-bearing 对照、新增测试的空测检查、无 mock 的 wire-oracle harness、以及受影响 workspace 的定向门禁——并把报告以单条评论("运行中"状态 → 最终报告原位升级)发回 PR。报告明确定位为供人类评审者参考的证据:它永远不是 review、不是批准、也不是 CI 检查。具体实现:
qwen-triage.yml新增两个 job(verify在与现有/tmux完全相同的隔离契约下执行 PR 代码——容器、agent 环境无任何 GitHub token、模型 key 藏在只放行POST /chat/completions的 loopback 代理后、要求 PR 作者(其代码被执行)与评论者(其消耗稀缺 runner 与模型预算)双方都具有写权限;publish-verify在干净的 hosted runner 上发布报告,绝不 checkout PR 代码),加上一个新的verify-prskill 固化方法论与产物契约,以及 Stage 2c 的更新,使静态 triage agent 在 PR 的核心声明需要行为证据时主动推荐/verify。内置的安全加固:agent 的 skill 通过
git archive HEAD^1从 base 分支 pin 取,被测代码树无法改写自己的验证器;checkout 后立即删除 PR 预埋的tmp/*-verify-*目录,杜绝伪造报告被产物收集器发布;持久 self-hosted workspace 沿用 triage job 的 git exec-vector 清扫(config 白名单 + hook 清除);agent 写出的 verdict 经白名单(merge-ready|findings|blocked|inconclusive)过滤后才进入 workflow 输出。第二个提交把近期手工验证轮次(#7632 第 2 轮、#7656)的模式固化进车道:重复触发
/verify即为后续轮次——resolve 步骤在覆盖前快照上一份报告,skill 在新 head 上逐项复核既往发现(已修复/仍存在/已被取代);harness 优先使用配置级 seam(baseUrl、环境变量)而非模块拦截,且伪造对端需编码上游的真实语义;多 commit PR 逐 commit 出 load-bearing 表;workflow/CI 类 PR 用真实数据回放内嵌脚本、跑仓库自身 lint 门禁、并用真实事件历史做上线首日成本测算;阻断级发现需枚举影响面并附保持原提交意图的最小修复建议(折叠展示)。第三个提交增加图片证据能力,借鉴带图 PR 与验证轮次(#7265、#7471、#7686 第 2 轮)的版式与托管惯例:agent 可产出
evidence/*.png(kebab-case 文件名即绑定断言的图注;优先 before/after 成对),publish-verify将其托管到pr-assets分支verify/pr<N>-<run>-<attempt>/目录并在转义报告下方渲染。全程按不可信载荷处理:严格文件名白名单、8 张 / 单张 2 MB 上限、竞争推送重试、托管失败一律降级为纯文本评论。同时固化 #7686 第 2 轮的量化验证规则:后续轮次以既往发现状态表开篇并重新测量(而非对比旧报告);体积/性能类声明用带 Δ 列的实测指标表且残差必须解释;默认配置下不可达的分支要构造出能触达它的配置;防御性 guard 的接受路径要用真实生产产物验证,而非只测被 mock 的拒绝路径。为什么需要
triage agent 在设计上只做静态分析——其环境携带写权限 PAT,绝不能执行 PR 代码,测试证据只能来自读取 PR 自己的 CI。目前唯一的行为验证车道
/tmux仅覆盖 TUI 表面和文本捕获。维护者现在完全靠手工产出深度验证轮次(对 base 的 A/B、wire oracle、load-bearing 证明——例如 #7586 上的第 3 轮验证)。本 PR 把这种证据风格变成 CI 里按需可得的能力,自带手工验证无需操心的隔离保证,并且不扩大现有审批面:verify 的 check-run 挂在issue_comment事件上,finalize workflow 的event == "pull_request"过滤在结构上将其排除在 CI 表格与延迟审批门禁之外。评审验证方案
如何验证
静态/回放验证(均可本地复现):
node scripts/lint.js --actionlint与仓库配置的 yamllint 通过;从 YAML 抽出的、属于本车道的全部 13 个run:块(authorize、verify、publish-verify)通过bash -n与shellcheck -S warning(第三方 Linux 复现进一步覆盖了整个 workflow 的 29 个块);内嵌 loopback 代理 JS 通过node --check;publish-verify脚本用 stubgh在全部六种终态分支上真实执行过,含</code></pre></details>、@everyone、<img onerror=…>的报告内容在<details><pre><code>中保持完全实体转义;verdict 白名单拒绝注入串;assertions jq 把非数字 JSON 强制为 0;docs-only 判定 grep 用 GNU grep 语义在混合/纯文档/空文件列表上回放。真实端到端运行(本地忠实复刻 CI job 的全部契约:#7547 的 merge-ref worktree、depth-2 父提交、
.qwen从本分支 pin、QWEN_VERIFY_CONTEXT元数据快照、无凭证gh、隔离QWEN_HOME):agent(qwen3.8-max-preview)25 turns / API 3.5 分钟完成,产物契约全部满足,verdictmerge-ready,318/318 断言。随后对数字做了对抗式审计:318 = 304 个真实 vitest 测试(23+68+213,日志核对)+ 13 个 A/B 断言 + 1 个空测检查;亲手复跑 agent 的ab-harness.mjs得到 13/13;回退 PR 修复后复跑同一 harness 精确变为 7/13(恰为 3 个 PR 侧断言 + 3 个 flip 断言失败),恢复后回到 13/13——证明 A/B 真实感知修复而非同文件自比。agent 全程 0 次gh调用。把真实产物喂给真正的publish-verify脚本,渲染出 CI 将要发布的 4.7 KB 评论。证据(Before & After)
见英文部分的 harness 复跑与变异测试输出,以及发布渲染的评论头部。
测试平台
macOS ✅;Windows N/A;Linux:静态与回放层 ✅——YAML 解析、36/36 workflow 测试、29 个抽取的⚠️ (workflow 尚未在真实 Actions runner 上带 secret 执行,这部分在合并前无法由任何复现覆盖)。
run:块过bash -n、两处代理 heredoc 过node --check、verdict 白名单回放,均已由第三方在c25afbd8上独立复现且零失败;实际运行层仍环境
macOS 15 / Node 22 / qwen CLI 0.20.1;agent 模型
qwen3.8-max-preview(DashScope);验证 worktree 用 PR lockfile 真实npm ci+npm run build构建。风险与范围
pr-assets(v1 只有文本 + run artifacts);相邻的既有缺口——tmux job 的*-tmux-*产物收集同样可被 PR 预埋目录伪造(与本 PR 为 verify 修复的是同一攻击类),留待 tmux 侧单独 PR。关联 Issue
本 PR 固化的方法论即近期 PR 上手工执行的深度验证风格(如 #7586 的第 3 轮验证评论)。不关闭任何 issue。