From 2f3d1b2ff53b6ae45bc022d1ff415e679d944015 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Tue, 27 Oct 2026 01:49:59 +0800 Subject: [PATCH] feat(verify-pr): add a local preflight and a verdict check (#15614) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(verify-pr): add a local preflight and a verdict check A maintainer round resolves the metadata snapshot, the base and head OIDs, the artifact directory name, and the verdict/count agreement by hand. The CI lane supplies all four; a workstation supplies none, and every one of them fails silently rather than loudly — a snapshot from the wrong repository, a parent commit taken for the base, a directory the collector never sees, or counts that contradict the stated verdict. Add a preflight that resolves the coordinates and reports whether this process is isolated, and a checker that applies the publisher's consistency rules to a local artifact directory. Point the skill at both where the flow needs them. * fix(verify-pr): address the local-helper review round The macOS sandbox marker is not an isolation boundary: its default profile still reads host files, so it is reported as a caveat instead of a pass. The coordinate path no longer gates on isolation. It must reach `gh`, which the boundary excludes, so it always runs on the host; gating there fired on every round and made the override the normal path. The check moved to `--require-isolation`, meant to run inside the boundary. A null or array assertions.json is now rejected instead of skipping every count check, the missing-OID hint names the `--repo` repository rather than `origin`, and the untrusted PR title is stripped of control characters before it reaches a terminal. The shared entry-point check moved under scripts/lib, and a workflow meta-test pins the checker's verdict vocabulary to the publisher's case labels. * fix(verify-pr): gate on credential-free readiness, not a container marker The CLI's own Docker/Podman launcher mounts ~/.config/gcloud and the ADC file, copies model API keys it can see, and then sets SANDBOX to the container name. Counting that marker as a passing boundary let a credentialed container through, which is the same class of false pass the Seatbelt fix removed. A marker is now an observation. Passing requires the explicit QWEN_VERIFY_CREDENTIAL_FREE=1 attestation and no known credential signal (cloud, SSH, kube, docker, git and gh variables plus their well-known paths, and SANDBOX_MOUNTS, which can carry anything). An attestation contradicted by a visible signal exits 4. * fix(verify-pr): close the three findings from the verification round The boundary gate's credential list now names ~/.config/gh and ~/.npmrc — the persistent form of the gh token and a registry token, already covered in their environment form. A report now has to cite the verified head when the round snapshot is present, not merely some 40-hex token that a base OID satisfies. An unreadable snapshot is reported instead of silently downgrading the check. A boolean-looking SANDBOX value no longer counts as a container marker. --- .qwen/skills/verify-pr/SKILL.md | 39 +- docs/design/verify-pr-local-round.md | 116 ++++++ docs/design/verify-pr-local-round.zh-CN.md | 53 +++ scripts/lib/entry-point.js | 28 ++ scripts/tests/qwen-triage-workflow.test.js | 24 +- scripts/tests/verify-pr-check.test.js | 237 +++++++++++ scripts/tests/verify-pr-prepare.test.js | 406 +++++++++++++++++++ scripts/verify-pr-check.mjs | 245 ++++++++++++ scripts/verify-pr-prepare.mjs | 433 +++++++++++++++++++++ scripts/verify-pr-watch-gate.mjs | 24 +- 10 files changed, 1581 insertions(+), 24 deletions(-) create mode 100644 docs/design/verify-pr-local-round.md create mode 100644 docs/design/verify-pr-local-round.zh-CN.md create mode 100644 scripts/lib/entry-point.js create mode 100644 scripts/tests/verify-pr-check.test.js create mode 100644 scripts/tests/verify-pr-prepare.test.js create mode 100644 scripts/verify-pr-check.mjs create mode 100644 scripts/verify-pr-prepare.mjs diff --git a/.qwen/skills/verify-pr/SKILL.md b/.qwen/skills/verify-pr/SKILL.md index fdddbc18fd3..470f6f66eb8 100644 --- a/.qwen/skills/verify-pr/SKILL.md +++ b/.qwen/skills/verify-pr/SKILL.md @@ -12,6 +12,25 @@ surface with mock-free harnesses, and report scripted pass/fail assertions — never impressions. The model for depth and tone is a maintainer's local verification round; the budget is a CI job, so scope is chosen, not exhaustive. +## Start here + +Two entry paths share the contract below. + +- **Sandboxed lane** — comment `@qwen-code /verify` on the PR. The workflow + supplies `$QWEN_VERIFY_CONTEXT`, an already-built tree and the time budget, + and publishes the report. There is nothing to prepare. +- **Maintainer round on a workstation** — `/verify-pr `, starting with + `node scripts/verify-pr-prepare.mjs --repo /` (the + preflight requires the repository explicitly; `origin` is not a fallback). + This round executes untrusted PR code, so it needs an isolation boundary + (see _Local invocation_). + +`/verify-pr ` proves the change and posts nothing. `--publish` adds the +report comment and its image evidence; `--watch` installs a standing trigger. +Before publishing a local round, run +`node scripts/verify-pr-check.mjs `; it applies the consistency +rules the publisher enforces in CI. + ## Environment contract (CI verify job) The workflow (`qwen-triage.yml` `verify` job) guarantees: @@ -119,6 +138,21 @@ profiles, or `gh` token. Do not run it in an ordinary working copy on a maintainer's machine; if that isolation is unavailable, ask the maintainer to trigger the sandboxed `@qwen-code /verify` lane instead. +Run `node scripts/verify-pr-prepare.mjs --repo /` on the host +before anything else: it fetches the metadata snapshot, resolves and verifies +both OIDs against this clone, creates `tmp/pr-verify-/`, and reports the +isolation signals it can see. It does not gate on them — resolving metadata +needs `gh`, which the boundary excludes, so this path always runs on the host. +The boundary check belongs inside: run +`node scripts/verify-pr-prepare.mjs --require-isolation` as the round's first +command there, and treat exit 4 as "this is not a boundary". A container +marker alone does not pass: the CLI's own Docker/Podman launcher mounts +`~/.config/gcloud`, the ADC file and every model API key it can see, so the +check also requires `QWEN_VERIFY_CREDENTIAL_FREE=1` — set it only after +confirming the boundary mounts nothing — and refuses when a known credential +signal is visible. The built-in macOS sandbox (`SANDBOX=sandbox-exec`) never +counts, because its default profile still reads host files. + ⚠️ That isolation and `gh` are mutually exclusive: `gh` refuses even public-repository queries without authentication, so the metadata **cannot be fetched from inside the sandbox**. Resolve it outside — `gh pr view @@ -297,8 +331,9 @@ JSON payload — `-f content="$(base64 …)"` blows past ARG_MAX around 100 KB. Keep the report.md contract in the comment body: verdict line first, collapsed `
` Chinese summary immediately after, images referenced by the same kebab-case names the files carry. The local path has no -publisher enforcing the fail/mismatch rule, so state the verdict word from -`verdict.txt` verbatim and keep the counts honest yourself. A published +publisher enforcing the fail/mismatch rule, so run +`node scripts/verify-pr-check.mjs ` and clear its errors before +publishing, then state the verdict word from `verdict.txt` verbatim. A published filename is spent: raw.githubusercontent serves a URL from cache, so overwriting `02-matrix.png` leaves every earlier link showing the old image. Republish as `02-matrix-v2.png`, update the body and every comment that quotes diff --git a/docs/design/verify-pr-local-round.md b/docs/design/verify-pr-local-round.md new file mode 100644 index 00000000000..f66cf3d3372 --- /dev/null +++ b/docs/design/verify-pr-local-round.md @@ -0,0 +1,116 @@ +# Local round helpers for /verify-pr + +[English](verify-pr-local-round.md) | [简体中文](verify-pr-local-round.zh-CN.md) + +Status: implementing with this change. + +## Problem + +`/verify-pr` has two entry paths. The sandboxed lane supplies the PR metadata +snapshot, a clone holding both OIDs, and a publication step that refuses a +verdict contradicting its own assertion counts. A maintainer's round on a +workstation supplies none of the three, and the skill answers with prose: a +hand-typed `gh pr view` field list, "do not assume `HEAD^1`/`HEAD^2`", a +directory name the collector globs, and "keep the counts honest yourself". +Each of those is a deterministic rule with no executable form, so every round +re-derives it and the mistakes fail silently: a snapshot from the wrong +repository, a parent commit mistaken for the base, a directory the publication +step never sees, or counts that contradict the verdict. + +## Current state + +- The local path asks the agent to fetch the snapshot and mount it as + `$QWEN_VERIFY_CONTEXT`, resolve `baseRefOid`/`headRefOid`, and create + `tmp/pr-verify-/` before any verification work begins. +- The isolation requirement ("credential-free container or VM") and its + fallback ("trigger the sandboxed lane") are prose. Nothing tells the agent + which situation the current process is in. +- The publish job refuses `merge-ready` with a nonzero `fail` and rejects an + inconsistent `assertions.json`, so the CI path is machine-checked end to end. + A local round has no equivalent and can publish counts that contradict the + verdict it states. + +## Proposal + +Two standalone scripts, plus the skill pointing at them where the flow needs +them rather than at the end of the document. + +- `scripts/verify-pr-prepare.mjs --repo /` resolves the + coordinates: fetch the snapshot with the field list the lane uses, verify both + OIDs exist in this clone, create the artifact directory with its `evidence/` + subdirectory, and report the isolation signals the process can observe. It + does not gate on them: resolving metadata needs `gh`, and `gh` and the + boundary are mutually exclusive, so this path always runs on the host. The + gate is `--require-isolation`, meant to run inside the boundary before the + round executes anything. +- `scripts/verify-pr-check.mjs ` applies the publisher's rules to a local + artifact directory: required files, the four verdict words, a positive and + self-consistent assertion triple, and no `merge-ready` carrying a failed + assertion. Report shape and evidence naming are warnings, because a + documentation-only round has no capture and a checker cannot know whether a + harness ran. + +## Design decisions + +1. **`--repo` is mandatory.** A script cannot distinguish a maintainer's clone + from a contributor's fork, and there `origin` resolves the same PR number to + an unrelated PR. Failing closed costs one flag and removes exactly the + failure the skill already warns about. +2. **The scripts never touch the round, and the coordinate path does not gate on + isolation.** Prepare fetches metadata and creates directories; it does not + checkout a worktree, build, or execute PR code. It also cannot decide whether + this process may run that code: it must reach `gh`, which the boundary + excludes by design, so it always runs on the host. A gate there would fire on + every round and turn the override into the normal path, so the check is a + separate mode intended to run inside the boundary. +3. **The local checker mirrors the publisher instead of replacing it.** + Extracting the publish job's assertions into shared code would touch the + workflow and its meta-tests; the rule set is four checks, stable, and stated + in one place. Divergence is the cost, recorded below. +4. **Warnings, not errors, for report shape.** The publisher does not enforce + report structure either. A checker stricter than the publisher would reject + artifact the lane accepts, which teaches maintainers to ignore it. + +## Constraints and risks + +- The isolation detector counts only container-backed signals. The built-in + macOS sandbox sets `SANDBOX=sandbox-exec` while its default profile still + reads host files, so it is reported as a caveat rather than a boundary. A + marker is not credential-free readiness either: the CLI's own Docker/Podman + launcher mounts the gcloud config and ADC file and copies model keys, so the + gate also requires an explicit attestation and refuses on a visible signal. +- Two implementations of the publish rules can drift. A workflow meta-test pins + the checker verdict vocabulary to the publisher case labels, so a fifth word + on one side fails the suite. The CI publisher remains authoritative. +- The PR title is untrusted input reaching a maintainer terminal, so control + characters are dropped before it is printed. +- These are local helpers: the lane pins `.qwen` from the base branch, so the + skill text that references them takes effect only after this change merges. + +## Validation plan + +- Unit tests for the pure helpers: repository slug, artifact directory name, + isolation signals, snapshot shape, and argument parsing; and for the + checker's rule set. +- End-to-end tests that drive both CLIs: a temporary clone with a `gh` + stand-in, asserting exit codes, the written snapshot, and the artifact + directory; and artifact fixtures for each checker outcome. +- A dry run of `prepare` against a real PR in a maintainer clone, and of + `check` against a real artifact directory. + +## Acceptance criteria + +- `prepare` on a clone holding both OIDs writes a snapshot whose + `baseRefOid`/`headRefOid` match the PR, creates the glob-matching directory, + and exits 0 on the host. `--require-isolation` passes only when a container + marker is present, `QWEN_VERIFY_CREDENTIAL_FREE=1` attests to a clean + boundary, and no known credential signal is visible; otherwise it exits 4. +- A missing OID fails with a fetch naming the `--repo` repository, not `origin`. +- `check` rejects `merge-ready` with a failed assertion, an inconsistent + triple, and an unknown verdict word; it accepts `findings` with the same + counts. +- Neither script executes PR code. + +## Open questions + +None at this scope. diff --git a/docs/design/verify-pr-local-round.zh-CN.md b/docs/design/verify-pr-local-round.zh-CN.md new file mode 100644 index 00000000000..e4107f8146d --- /dev/null +++ b/docs/design/verify-pr-local-round.zh-CN.md @@ -0,0 +1,53 @@ +# /verify-pr 的本地轮次辅助脚本 + +[English](verify-pr-local-round.md) | [简体中文](verify-pr-local-round.zh-CN.md) + +状态：随本改动实现。 + +## 问题 + +`/verify-pr` 有两条入口。沙箱 lane 会提供 PR 元数据快照、同时含有两个 OID 的克隆，以及一个会拒绝「判定与自身断言计数矛盾」的发布步骤。维护者在自己机器上跑的轮次三者皆无，而 skill 给出的答案是散文式的：手写的 `gh pr view` 字段列表、「不要假设 `HEAD^1`/`HEAD^2`」、收集步骤用 glob 匹配的目录名，以及「自己把计数数清楚」。这些都是确定性的规则，却没有任何可执行形态，于是每一轮都要重新推导，出错时又是静默的：快照来自错误的仓库、把父提交当成基线、目录名不被发布步骤识别、计数与判定互相矛盾。 + +## 现状 + +- 本地路径要求 agent 抓取快照并挂成 `$QWEN_VERIFY_CONTEXT`、解析 `baseRefOid`/`headRefOid`、在开始任何验证工作前建好 `tmp/pr-verify-/`。 +- 隔离要求（「无凭据的容器或 VM」）及其退路（「触发沙箱 lane」）都是散文。没有任何东西告诉 agent 当前进程处于哪种情形。 +- 发布 job 会拒绝「`merge-ready` 且 `fail` 非零」，也会拒绝内部不一致的 `assertions.json`，因此 CI 路径端到端由机器校验。本地轮次没有对应物，可以发布与自己判定矛盾的计数。 + +## 方案 + +新增两个独立脚本，并让 skill 在流程需要的位置指向它们，而不是放在文末。 + +- `scripts/verify-pr-prepare.mjs --repo /` 负责解析坐标：用 lane 相同的字段列表抓取快照、校验两个 OID 在本克隆中存在、创建带 `evidence/` 子目录的产物目录，并报告当前进程能观测到的隔离信号。它不据此设闸：解析元数据需要 `gh`，而 `gh` 与边界互斥，因此这条路径总是在宿主上运行。闸门是 `--require-isolation`，用于在边界内、轮次执行任何东西之前运行。 +- `scripts/verify-pr-check.mjs ` 把发布侧的规则套到本地产物目录：必需文件、四个判定词、为正且自洽的断言三元组、以及不允许带失败断言的 `merge-ready`。报告形态与证据命名只作为警告，因为纯文档轮次没有截图，检查器也无法知道是否跑过 harness。 + +## 设计决策 + +1. **`--repo` 必填。** 脚本无法区分维护者克隆与贡献者 fork，而在 fork 上 `origin` 会把同一个 PR 号解析成另一个无关 PR。fail closed 的代价是一个参数，消掉的正是 skill 已经警告的那种失败。 +2. **脚本不碰轮次本身，坐标路径也不对隔离设闸。** prepare 只抓元数据、建目录；它不建 worktree、不构建、不执行 PR 代码。它也无法判断本进程是否有资格执行那些代码：解析元数据必须访问 `gh`，而边界按设计排除 `gh`，所以它总是在宿主上运行。在那里设闸会让每一轮都触发，并让覆盖开关变成常规路径，因此检查被拆成独立模式，用于在边界内运行。 +3. **本地检查器镜像发布侧，而不是替换它。** 把发布 job 里的断言抽成共享代码会动到 workflow 及其 meta 测试；规则只有四条、稳定，且集中在一处。代价是可能漂移，记录在下方。 +4. **报告形态只报警告。** 发布侧同样不强制报告结构。比发布侧更严格的检查器会拒绝 lane 本来接受的产物，结果只会教会维护者忽略它。 + +## 约束与风险 + +- 隔离检测只把容器类信号计为边界。内置 macOS 沙箱设置 `SANDBOX=sandbox-exec`，但其默认 profile 仍能读取宿主文件，因此只作为注意事项报告，不算边界。仅有标记也不等于无凭据就绪：CLI 自带的 Docker/Podman 启动器会挂载 gcloud 配置与 ADC 文件并复制模型 key，因此闸门还要求显式声明，并在看到已知信号时拒绝。 +- 发布规则存在两份实现，可能漂移。一条 workflow 元测试把检查器的判定词表钉到发布侧的 case 标签上，任一侧新增第五个词都会让套件失败。CI 发布侧仍是权威。 +- PR 标题是不可信输入且会到达维护者终端，因此在打印前剥离控制字符。 +- 这两个是本地辅助脚本：lane 从基线分支钉住 `.qwen`，因此引用它们的 skill 文本要在本改动合入后才生效。 + +## 验证计划 + +- 纯函数的单元测试：仓库 slug、产物目录名、隔离信号、快照形状、参数解析；以及检查器的规则集。 +- 驱动两个 CLI 的端到端测试：临时克隆加 `gh` 替身，断言退出码、写出的快照与产物目录；以及覆盖检查器每种结果的产物夹具。 +- 在维护者克隆上对真实 PR 干跑一次 `prepare`，并对真实产物目录跑一次 `check`。 + +## 验收标准 + +- 在同时含有两个 OID 的克隆上，`prepare` 写出的快照其 `baseRefOid`/`headRefOid` 与 PR 一致，创建符合 glob 的目录，且在宿主上退出 0。`--require-isolation` 只有在容器标记存在、`QWEN_VERIFY_CREDENTIAL_FREE=1` 声明边界干净、且看不到已知凭据信号时才通过；否则退出 4。 +- OID 缺失时失败，并给出用 `--repo` 指定仓库（而非 `origin`）的 fetch 命令。 +- `check` 拒绝带失败断言的 `merge-ready`、不一致的三元组、未知判定词；对相同计数下的 `findings` 予以通过。 +- 两个脚本都不执行 PR 代码。 + +## 未决问题 + +本范围内无。 diff --git a/scripts/lib/entry-point.js b/scripts/lib/entry-point.js new file mode 100644 index 00000000000..0bb2eca3871 --- /dev/null +++ b/scripts/lib/entry-point.js @@ -0,0 +1,28 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { realpathSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * Whether the module is the executed entry point rather than an import. + * + * `import.meta.url` is a percent-encoded file URL, so comparing it to a plain + * `file://` + path breaks on spaces or non-ASCII characters. Both sides are + * resolved through `realpath`, because Node resolves the entry module's + * symlinks while `argv[1]` keeps the path the shell was given. + */ +export function isEntryPoint(metaUrl, argv1) { + if (!argv1) return false; + try { + return ( + realpathSync(resolve(argv1)) === realpathSync(fileURLToPath(metaUrl)) + ); + } catch { + return false; + } +} diff --git a/scripts/tests/qwen-triage-workflow.test.js b/scripts/tests/qwen-triage-workflow.test.js index de30fbdbda5..3c84340b4e7 100644 --- a/scripts/tests/qwen-triage-workflow.test.js +++ b/scripts/tests/qwen-triage-workflow.test.js @@ -38,9 +38,13 @@ const prSkill = readFileSync( 'utf8', ); const triageSkillDoc = readFileSync('.qwen/skills/triage/SKILL.md', 'utf8'); +import { VERDICTS } from '../../scripts/verify-pr-check.mjs'; + const verifySkill = readFileSync('.qwen/skills/verify-pr/SKILL.md', 'utf8'); const hasGnuRealpath = - spawnSync('realpath', ['-m', '--', '/'], { stdio: 'ignore' }).status === 0; + spawnSync('realpath', ['-m', '--', '/'], { + stdio: 'ignore', + }).status === 0; function escapeRegExp(value) { return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); @@ -5324,6 +5328,24 @@ describe('qwen-triage verify hardening round 2', () => { expect(publishStep).toContain('.total == .pass + .fail'); expect(publishStep).toContain('all(type == "number" and . >= 0'); }); + + // The local artifact checker mirrors this step instead of sharing code with + // it (design decision 3), so the mirror needs a pin. The numeric rule is + // already asserted on both sides; the verdict vocabulary was the axis with + // nothing holding the two implementations together, so a fifth word added to + // the publisher would leave the local path silently accepting four. + it('keeps the local checker aligned with the publisher verdicts', () => { + const publishStep = step('Post verification report comment'); + const agentCase = publishStep + .slice(publishStep.indexOf('case "$AGENT_VERDICT"')) + .split('esac')[0]; + const labels = new Set( + [...agentCase.matchAll(/^\s+([a-z][a-z-]*)\)/gm)].map( + (match) => match[1], + ), + ); + expect(labels).toEqual(new Set(VERDICTS)); + }); }); describe('qwen-triage verify publish fidelity', () => { diff --git a/scripts/tests/verify-pr-check.test.js b/scripts/tests/verify-pr-check.test.js new file mode 100644 index 00000000000..ef8ca219110 --- /dev/null +++ b/scripts/tests/verify-pr-check.test.js @@ -0,0 +1,237 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, describe, expect, it } from 'vitest'; +import { checkArtifacts } from '../../scripts/verify-pr-check.mjs'; + +const SCRIPT = fileURLToPath( + new URL('../../scripts/verify-pr-check.mjs', import.meta.url), +); +const HEAD = 'b'.repeat(40); + +function validFiles(overrides = {}) { + return { + report: `merge-ready · 4 passed · 0 failed · 4 total · ${HEAD}\n\n# Report\n`, + verdict: 'merge-ready\n', + assertions: JSON.stringify({ pass: 4, fail: 0, total: 4 }), + ...overrides, + }; +} + +function check(overrides, evidence = ['01-ab-cells.png']) { + return checkArtifacts({ files: validFiles(overrides), evidence }); +} + +let tmpDirs = []; +afterEach(() => { + for (const dir of tmpDirs) rmSync(dir, { recursive: true, force: true }); + tmpDirs = []; +}); + +function makeArtifactDir(files, evidence = []) { + const dir = mkdtempSync(join(tmpdir(), 'verify-pr-check-')); + tmpDirs.push(dir); + writeFileSync(join(dir, 'report.md'), files.report); + writeFileSync(join(dir, 'verdict.txt'), files.verdict); + writeFileSync(join(dir, 'assertions.json'), files.assertions); + if (evidence.length > 0) { + mkdirSync(join(dir, 'evidence')); + for (const name of evidence) { + writeFileSync(join(dir, 'evidence', name), 'png'); + } + } + return dir; +} + +describe('checkArtifacts', () => { + it('accepts a consistent round', () => { + const result = check({}); + expect(result.errors).toEqual([]); + expect(result.warnings).toEqual([]); + expect(result.counts).toEqual({ pass: 4, fail: 0, total: 4 }); + expect(result.verdict).toBe('merge-ready'); + }); + + // The publisher refuses this pair, and the local path had no equivalent: + // a merge-ready report carrying failures is the shape that shipped once and + // degraded to "no usable structured verdict". + it('refuses merge-ready when an assertion failed', () => { + const result = check({ + verdict: 'merge-ready\n', + assertions: JSON.stringify({ pass: 3, fail: 1, total: 4 }), + }); + expect(result.errors.join('\n')).toMatch( + /merge-ready with 1 failed assertion/, + ); + }); + + it('accepts findings with a nonzero fail', () => { + const result = check({ + verdict: 'findings\n', + report: `findings · 3 passed · 1 failed · 4 total · ${HEAD}\n`, + assertions: JSON.stringify({ pass: 3, fail: 1, total: 4 }), + }); + expect(result.errors).toEqual([]); + }); + + it('requires the four verdict words and nothing else', () => { + for (const word of ['merge-ready', 'findings', 'blocked', 'inconclusive']) { + expect( + check({ verdict: `${word}\n`, report: `${word} · ${HEAD}\n` }).errors, + ).toEqual([]); + } + expect(check({ verdict: 'PASS\n' }).errors.join('\n')).toMatch( + /verdict.txt must be exactly one of/, + ); + expect(check({ verdict: 'merge-ready but\n' }).errors.join('\n')).toMatch( + /verdict.txt must be exactly one of/, + ); + }); + + it('validates the assertion triple rather than each field alone', () => { + const cases = [ + ['{"pass":4}', /non-negative integers pass, fail and total/], + ['{"pass":"4","fail":0,"total":4}', /non-negative integers/], + ['{"pass":4,"fail":-1,"total":3}', /non-negative integers/], + ['{"pass":4,"fail":0,"total":0}', /total is 0/], + ['{"pass":2,"fail":1,"total":4}', /inconsistent/], + ['not json', /not valid JSON/], + // JSON.parse accepts all three; the publisher's jq requires an object, + // and a null artifact used to skip every count check and read as clean. + ['null', /must be an object/], + ['[]', /must be an object/], + ['0', /must be an object/], + ]; + for (const [assertions, message] of cases) { + expect(check({ assertions }).errors.join('\n')).toMatch(message); + } + }); + + it('reports missing and empty artifacts as errors', () => { + const result = checkArtifacts({ + files: { report: null, verdict: '', assertions: ' ' }, + evidence: [], + }); + expect(result.errors).toContain('report.md is missing'); + expect(result.errors).toContain('verdict.txt is empty'); + expect(result.errors).toContain('assertions.json is empty'); + }); + + // A table is the claim, a capture is the witness; the contract calls these + // mandatory whenever a harness ran, which a checker cannot know, so the + // absence is a warning and the naming rule is what binds image to claim. + it('warns about missing or misnamed evidence', () => { + expect(check({}, []).warnings.join('\n')).toMatch(/no PNG capture/); + expect( + check({}, ['01-AB-Cells.PNG', '01_ab.png']).warnings.join('\n'), + ).toMatch(/01-AB-Cells\.PNG is not a kebab-case caption/); + expect(check({}, ['01_ab.png']).warnings.join('\n')).toMatch( + /01_ab\.png is not a kebab-case caption/, + ); + }); + + it('warns when the report does not lead with its verdict and head', () => { + const result = check({ report: '# Report\n\nbody\n' }); + expect(result.warnings.join('\n')).toMatch(/does not state "merge-ready"/); + expect(result.warnings.join('\n')).toMatch(/40-character head OID/); + }); + + // Presence is not identity: a base OID is also 40 hex characters. With the + // round snapshot at hand, the cited OID has to be the head it verified. + it('warns when the report cites an OID other than the verified head', () => { + const head = 'c'.repeat(40); + const base = 'd'.repeat(40); + const cites = (oid) => + `merge-ready · 1 passed · 0 failed · 1 total · ${oid}\n`; + + const correct = checkArtifacts({ + files: validFiles({ report: cites(head) }), + evidence: [], + expectedHead: head, + }); + expect(correct.warnings.join('\n')).not.toContain( + 'does not name the verified head', + ); + + const other = checkArtifacts({ + files: validFiles({ report: cites(base) }), + evidence: [], + expectedHead: head, + }); + expect(other.warnings.join('\n')).toContain( + `does not name the verified head ${head}`, + ); + expect(other.warnings.join('\n')).toContain('cites 1 other OID'); + }); +}); + +// The CLI is the path a maintainer runs, so the exit codes are the contract. +describe.skipIf(process.platform === 'win32')('check (CLI)', () => { + const run = (dir) => + spawnSync(process.execPath, [SCRIPT, dir], { encoding: 'utf8' }); + + it('exits 0 on a consistent round and 1 on a contradiction', () => { + const ok = run(makeArtifactDir(validFiles(), ['01-ab-cells.png'])); + expect(ok.status).toBe(0); + expect(ok.stdout).toContain( + 'ok: verdict=merge-ready (4 passed · 0 failed · 4 total)', + ); + + const bad = run( + makeArtifactDir( + validFiles({ + assertions: JSON.stringify({ pass: 3, fail: 1, total: 4 }), + }), + ), + ); + expect(bad.status).toBe(1); + expect(bad.stdout).toContain('error merge-ready with 1 failed assertion'); + expect(bad.stdout).toContain('failed: verdict=merge-ready'); + }); + + it('reads the verified head from the round snapshot', () => { + const root = mkdtempSync(join(tmpdir(), 'verify-pr-check-snapshot-')); + tmpDirs.push(root); + const artifacts = join(root, 'tmp', 'pr7-verify-20261026-120000'); + mkdirSync(join(artifacts, 'evidence'), { recursive: true }); + mkdirSync(join(root, 'tmp', 'verify-context'), { recursive: true }); + const head = 'e'.repeat(40); + const base = 'f'.repeat(40); + writeFileSync( + join(root, 'tmp', 'verify-context', 'pr7.json'), + JSON.stringify({ number: 7, headRefOid: head }), + ); + writeFileSync(join(artifacts, 'verdict.txt'), 'merge-ready\n'); + writeFileSync( + join(artifacts, 'assertions.json'), + JSON.stringify({ pass: 1, fail: 0, total: 1 }), + ); + writeFileSync( + join(artifacts, 'report.md'), + `merge-ready · 1 passed · 0 failed · 1 total · ${base}\n`, + ); + + const result = spawnSync(process.execPath, [SCRIPT, artifacts], { + encoding: 'utf8', + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain(`does not name the verified head ${head}`); + }); + + it('exits 2 without a directory and 1 on a missing one', () => { + expect( + spawnSync(process.execPath, [SCRIPT], { encoding: 'utf8' }).status, + ).toBe(2); + expect(run(join(tmpdir(), 'verify-pr-check-does-not-exist')).status).toBe( + 1, + ); + }); +}); diff --git a/scripts/tests/verify-pr-prepare.test.js b/scripts/tests/verify-pr-prepare.test.js new file mode 100644 index 00000000000..b9ccc195519 --- /dev/null +++ b/scripts/tests/verify-pr-prepare.test.js @@ -0,0 +1,406 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { execFileSync, spawnSync } from 'node:child_process'; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + artifactDirName, + detectCredentialSignals, + detectIsolation, + parseArgs, + parseRepo, + validateContext, +} from '../../scripts/verify-pr-prepare.mjs'; + +const SCRIPT = fileURLToPath( + new URL('../../scripts/verify-pr-prepare.mjs', import.meta.url), +); +const OID = (char) => char.repeat(40); +const REPO = 'QwenLM/qwen-code'; +const ESC = String.fromCharCode(27); + +const gitEnv = { + ...process.env, + GIT_CONFIG_GLOBAL: '/dev/null', + GIT_CONFIG_SYSTEM: '/dev/null', + GIT_AUTHOR_NAME: 't', + GIT_AUTHOR_EMAIL: 't@example.com', + GIT_COMMITTER_NAME: 't', + GIT_COMMITTER_EMAIL: 't@example.com', +}; + +let tmpDirs = []; +afterEach(() => { + for (const dir of tmpDirs) rmSync(dir, { recursive: true, force: true }); + tmpDirs = []; +}); + +function makeTmpDir() { + const dir = mkdtempSync(join(tmpdir(), 'verify-pr-prepare-')); + tmpDirs.push(dir); + return dir; +} + +/** A clone with one commit per side, so both OIDs exist locally. */ +function makeRepo() { + const dir = makeTmpDir(); + const git = (...args) => + execFileSync('git', args, { cwd: dir, env: gitEnv, encoding: 'utf8' }); + git('init', '-q'); + writeFileSync(join(dir, 'base.txt'), 'base\n'); + git('add', 'base.txt'); + git('commit', '-qm', 'base'); + writeFileSync(join(dir, 'head.txt'), 'head\n'); + git('add', 'head.txt'); + git('commit', '-qm', 'head'); + return { + dir, + base: git('rev-parse', 'HEAD^').trim(), + head: git('rev-parse', 'HEAD').trim(), + }; +} + +/** A `gh` stand-in that returns the snapshot this test controls. */ +function installFakeGh(dir, snapshot) { + const bin = join(dir, 'bin'); + mkdirSync(bin); + const snapshotPath = join(dir, 'snapshot.json'); + writeFileSync(snapshotPath, JSON.stringify(snapshot)); + const gh = join(bin, 'gh'); + writeFileSync(gh, '#!/bin/sh\ncat "$FAKE_GH_SNAPSHOT"\n'); + chmodSync(gh, 0o755); + return { + ...gitEnv, + PATH: `${bin}:${process.env.PATH ?? ''}`, + FAKE_GH_SNAPSHOT: snapshotPath, + }; +} + +function snapshotFor({ head, base, title = 'test(scope): subject' }) { + return { + number: 42, + title, + body: '', + author: { login: 'someone' }, + baseRefName: 'main', + baseRefOid: base, + headRefOid: head, + commits: [], + }; +} + +function runPrepare(dir, args, env) { + return spawnSync(process.execPath, [SCRIPT, ...args], { + cwd: dir, + env, + encoding: 'utf8', + }); +} + +describe('parseRepo', () => { + it('accepts a GitHub slug and rejects anything else', () => { + expect(parseRepo('QwenLM/qwen-code')).toEqual({ + owner: 'QwenLM', + name: 'qwen-code', + }); + expect(parseRepo(' QwenLM/qwen-code ')).toEqual({ + owner: 'QwenLM', + name: 'qwen-code', + }); + expect(parseRepo('qwen-code')).toBeNull(); + expect(parseRepo('QwenLM/qwen-code/extra')).toBeNull(); + expect(parseRepo('QwenLM//qwen-code')).toBeNull(); + expect(parseRepo('')).toBeNull(); + expect(parseRepo(undefined)).toBeNull(); + }); +}); + +describe('artifactDirName', () => { + // The publication step globs for the `-verify-` infix, so the shape is a + // contract rather than a readable name. + it('carries the PR number, the verify infix and a sortable stamp', () => { + const name = artifactDirName(42, new Date(2026, 9, 26, 13, 4, 5)); + expect(name).toBe('pr42-verify-20261026-130405'); + }); +}); + +describe('detectIsolation', () => { + it('reports a container marker as a boundary', () => { + expect(detectIsolation({}, () => false)).toEqual({ + isolated: false, + signals: [], + seatbelt: false, + }); + expect(detectIsolation({ container: 'podman' }, () => false)).toEqual({ + isolated: true, + signals: ['container=podman'], + seatbelt: false, + }); + expect(detectIsolation({ SANDBOX: 'docker' }, () => false)).toEqual({ + isolated: true, + signals: ['SANDBOX=docker'], + seatbelt: false, + }); + expect(detectIsolation({}, (path) => path === '/.dockerenv')).toEqual({ + isolated: true, + signals: ['/.dockerenv'], + seatbelt: false, + }); + }); + + // The built-in macOS sandbox sets this value and runs a profile that still + // reads host files, so counting it as a boundary would send untrusted PR code + // down the local path the gate exists to prevent. + it('does not count the built-in macOS sandbox as a boundary', () => { + expect(detectIsolation({ SANDBOX: 'sandbox-exec' }, () => false)).toEqual({ + isolated: false, + signals: [], + seatbelt: true, + }); + // A boolean-looking value reads as "disabled", not as a container name. + expect(detectIsolation({ SANDBOX: 'false' }, () => false)).toEqual({ + isolated: false, + signals: [], + seatbelt: false, + }); + }); +}); + +describe('detectCredentialSignals', () => { + it('names environment variables and well-known paths', () => { + expect(detectCredentialSignals({}, () => false, '/home/t')).toEqual([]); + expect( + detectCredentialSignals( + { OPENAI_API_KEY: 'x', GH_TOKEN: 'y' }, + () => false, + '/home/t', + ), + ).toEqual(['OPENAI_API_KEY', 'GH_TOKEN']); + expect( + detectCredentialSignals({}, (path) => path === '/home/t/.ssh', '/home/t'), + ).toEqual(['~/.ssh']); + expect( + detectCredentialSignals( + { SANDBOX_MOUNTS: '/x:/y' }, + () => false, + '/home/t', + ), + ).toEqual(['SANDBOX_MOUNTS']); + }); +}); + +describe('validateContext', () => { + it('requires a numeric PR and two full OIDs', () => { + const ok = { + number: 42, + title: 't', + baseRefName: 'main', + baseRefOid: OID('a'), + headRefOid: OID('b'), + }; + expect(validateContext(ok)).toEqual({ + pr: 42, + base: OID('a'), + head: OID('b'), + baseRefName: 'main', + title: 't', + }); + expect(validateContext({ ...ok, number: '42' })).toBeNull(); + expect(validateContext({ ...ok, number: 0 })).toBeNull(); + expect(validateContext({ ...ok, baseRefOid: 'abc' })).toBeNull(); + expect(validateContext({ ...ok, headRefOid: OID('Z') })).toBeNull(); + expect(validateContext(null)).toBeNull(); + }); +}); + +describe('parseArgs', () => { + it('takes the PR number, --repo in both spellings, and --require-isolation', () => { + expect(parseArgs(['42', '--repo', REPO])).toMatchObject({ + pr: 42, + repo: REPO, + requireIsolation: false, + }); + expect(parseArgs(['42', `--repo=${REPO}`])).toMatchObject({ + pr: 42, + repo: REPO, + }); + // The boundary check needs no coordinates: it is meant to run inside. + expect(parseArgs(['--require-isolation'])).toMatchObject({ + requireIsolation: true, + }); + expect(parseArgs(['--help'])).toMatchObject({ help: true }); + }); + + it('rejects a missing PR, a missing repo, and a stray argument', () => { + expect(parseArgs(['--repo', REPO]).error).toMatch(/PR number/); + expect(parseArgs(['42']).error).toMatch(/--repo/); + expect(parseArgs(['42', '--repo']).error).toMatch(/--repo/); + expect(parseArgs(['42', '--repo', REPO, 'extra']).error).toMatch( + /unexpected argument/, + ); + // The removed escape hatch must not fall through as a PR. + expect(parseArgs(['42', '--repo', REPO, '--allow-host']).error).toMatch( + /unexpected argument/, + ); + }); +}); + +// These cases drive the real CLI through a POSIX `gh` stand-in; a Windows +// runner cannot express it, and the Linux lane owns that coverage. +describe.skipIf(process.platform === 'win32')('prepare (CLI)', () => { + it('resolves the coordinates and reports a container boundary', () => { + const { dir, base, head } = makeRepo(); + const env = installFakeGh(dir, snapshotFor({ base, head })); + env.SANDBOX = 'docker'; + + const result = runPrepare(dir, ['42', '--repo', REPO], env); + expect(result.stderr).toBe(''); + expect(result.status).toBe(0); + expect(result.stdout).toContain(`repo ${REPO}`); + expect(result.stdout).toContain(`base ${base}`); + expect(result.stdout).toContain(`head ${head}`); + expect(result.stdout).toContain('isolation SANDBOX=docker'); + expect(result.stdout).toContain('--require-isolation'); + + const root = realpathSync(dir); + const contextPath = join(root, 'tmp', 'verify-context', 'pr42.json'); + const context = JSON.parse(readFileSync(contextPath, 'utf8')); + expect(context.baseRefOid).toBe(base); + expect(context.headRefOid).toBe(head); + const artifacts = result.stdout.match(/artifacts {2}(\S+)/)?.[1]; + expect(artifacts).toMatch(/pr42-verify-\d{8}-\d{6}$/); + expect(existsSync(join(artifacts, 'evidence'))).toBe(true); + }); + + // The coordinate path always runs on the host, because resolving metadata + // needs `gh`. Gating there would fire every round; the gate is the separate + // mode, run where PR code actually executes. + it('reports a seatbelt process without claiming it is a boundary', () => { + const { dir, base, head } = makeRepo(); + const env = installFakeGh(dir, snapshotFor({ base, head })); + env.SANDBOX = 'sandbox-exec'; + if (detectIsolation().isolated) return; + + const prepared = runPrepare(dir, ['42', '--repo', REPO], env); + expect(prepared.status).toBe(0); + expect(prepared.stdout).toContain('isolation none detected'); + expect(prepared.stdout).toContain('not a boundary'); + + const boundary = runPrepare(dir, ['--require-isolation'], env); + expect(boundary.status).toBe(4); + expect(boundary.stderr).toContain('default profile still reads host files'); + }); + + // A marker says a container exists; it cannot say what the container mounts. + // The CLI's own launcher attaches gcloud config, the ADC file and model + // keys, so the gate also needs the attestation and a clean signal set. + it('does not pass the gate on a container marker alone', () => { + const { dir } = makeRepo(); + const env = { + PATH: process.env.PATH ?? '', + HOME: makeTmpDir(), + SANDBOX: 'docker', + }; + + const markerOnly = runPrepare(dir, ['--require-isolation'], env); + expect(markerOnly.status).toBe(4); + expect(markerOnly.stderr).toContain( + 'QWEN_VERIFY_CREDENTIAL_FREE=1 is not set', + ); + + const attested = runPrepare(dir, ['--require-isolation'], { + ...env, + QWEN_VERIFY_CREDENTIAL_FREE: '1', + }); + expect(attested.status).toBe(0); + expect(attested.stdout).toContain('credential-free per attestation'); + }); + + it('refuses an attested boundary that still shows credentials', () => { + const { dir } = makeRepo(); + const home = makeTmpDir(); + const env = { + PATH: process.env.PATH ?? '', + HOME: home, + SANDBOX: 'docker', + QWEN_VERIFY_CREDENTIAL_FREE: '1', + }; + mkdirSync(join(home, '.ssh')); + + const mounted = runPrepare(dir, ['--require-isolation'], env); + expect(mounted.status).toBe(4); + expect(mounted.stderr).toContain('~/.ssh'); + + // Persistent forms of the same credentials: `gh auth login` and `npm login`. + for (const relative of ['.config/gh', '.npmrc']) { + const home = makeTmpDir(); + mkdirSync(join(home, relative), { recursive: true }); + const result = runPrepare(dir, ['--require-isolation'], { + ...env, + HOME: home, + }); + expect(result.status).toBe(4); + expect(result.stderr).toContain(`~/${relative}`); + } + + const keyed = runPrepare(dir, ['--require-isolation'], { + ...env, + HOME: makeTmpDir(), + OPENAI_API_KEY: 'sk-test', + }); + expect(keyed.status).toBe(4); + expect(keyed.stderr).toContain('OPENAI_API_KEY'); + }); + + it('strips control characters from an untrusted title', () => { + const { dir, base, head } = makeRepo(); + const env = installFakeGh( + dir, + snapshotFor({ base, head, title: `evil${ESC}[2Jtitle` }), + ); + env.SANDBOX = 'docker'; + + const result = runPrepare(dir, ['42', '--repo', REPO], env); + expect(result.status).toBe(0); + expect(result.stdout).not.toContain(ESC); + expect(result.stdout).toContain('evil[2Jtitle'); + }); + + it('names the fetch that brings a missing OID into the clone', () => { + const { dir, base } = makeRepo(); + const env = installFakeGh(dir, snapshotFor({ base, head: OID('f') })); + env.SANDBOX = 'docker'; + + const result = runPrepare(dir, ['42', '--repo', REPO], env); + expect(result.status).toBe(3); + expect(result.stderr).toContain(`head ${OID('f')} is not in this clone`); + // Named explicitly: `origin` may be the contributor's fork, which resolves + // the same PR number to an unrelated PR. + expect(result.stderr).toContain( + `git fetch https://github.com/${REPO}.git pull/42/head`, + ); + }); + + it('refuses to run without a repository', () => { + const { dir } = makeRepo(); + const result = runPrepare(dir, ['42'], gitEnv); + expect(result.status).toBe(2); + expect(result.stderr).toContain('--repo / is required'); + }); +}); diff --git a/scripts/verify-pr-check.mjs b/scripts/verify-pr-check.mjs new file mode 100644 index 00000000000..26c5a99344a --- /dev/null +++ b/scripts/verify-pr-check.mjs @@ -0,0 +1,245 @@ +#!/usr/bin/env node + +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Applies the publisher's consistency rules to a local `/verify-pr` round. + * + * In CI the publish job refuses an artifact whose verdict contradicts its own + * assertion counts, so a `merge-ready` report carrying `fail: 7` degrades to + * "no usable structured verdict". A local round has no publisher: the skill + * asks the maintainer to "keep the counts honest yourself", which is the one + * step of that path a machine should own. + * + * Usage: + * node scripts/verify-pr-check.mjs + * + * Exit codes: 0 when no error was found (warnings may still print), 1 on any + * error, 2 on a usage error. + */ + +import { existsSync, readFileSync, readdirSync } from 'node:fs'; +import { basename, dirname, join, resolve } from 'node:path'; +import { isEntryPoint } from './lib/entry-point.js'; + +export const VERDICTS = ['merge-ready', 'findings', 'blocked', 'inconclusive']; + +const KEBAB_PNG = /^[a-z0-9]+(?:-[a-z0-9]+)*\.png$/; + +const HELP = `Usage: node scripts/verify-pr-check.mjs + +Checks a local round against the rules the CI publisher enforces: + + - report.md, verdict.txt and assertions.json exist and carry content + - verdict.txt is exactly one of merge-ready | findings | blocked | inconclusive + - assertions.json holds non-negative integers, a positive total, and + total == pass + fail + - a merge-ready verdict carries no failed assertion + +and warns (without failing) when the report does not lead with its verdict and +head OID, or when evidence/ carries no kebab-case PNG capture. + +Exit codes: 0 no errors, 1 errors found, 2 usage. +`; + +function readIfExists(path) { + return existsSync(path) ? readFileSync(path, 'utf8') : null; +} + +/** + * Pure check over already-read artifacts. `files` carries raw text or null; + * `evidence` is the PNG file names under evidence/. + */ +export function checkArtifacts({ files, evidence = [], expectedHead = null }) { + const errors = []; + const warnings = []; + + for (const name of ['report.md', 'verdict.txt', 'assertions.json']) { + const key = name.split('.')[0]; + const text = files[key]; + if (text === null || text === undefined) { + errors.push(`${name} is missing`); + } else if (text.trim() === '') { + errors.push(`${name} is empty`); + } + } + + let verdict = null; + if (files.verdict && files.verdict.trim() !== '') { + verdict = files.verdict.trim(); + if (!VERDICTS.includes(verdict)) { + errors.push( + `verdict.txt must be exactly one of ${VERDICTS.join(' | ')}; got ${JSON.stringify(verdict)}`, + ); + verdict = null; + } else if (files.verdict.trimEnd() !== verdict) { + warnings.push('verdict.txt has leading whitespace; it must be one word'); + } + } + + let counts = null; + if (files.assertions && files.assertions.trim() !== '') { + let parsed; + let parsedOk = true; + try { + parsed = JSON.parse(files.assertions); + } catch { + parsedOk = false; + errors.push('assertions.json is not valid JSON'); + } + // JSON.parse('null') succeeds, and the publisher's jq requires + // type == "object": without this branch a null or array artifact skips + // every count check and still reads as evidence. + if ( + parsedOk && + (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) + ) { + errors.push( + 'assertions.json must be an object with pass, fail and total', + ); + } else if (parsedOk) { + const fields = ['pass', 'fail', 'total']; + const valid = fields.every( + (field) => Number.isInteger(parsed?.[field]) && parsed[field] >= 0, + ); + if (!valid) { + errors.push( + 'assertions.json must carry non-negative integers pass, fail and total', + ); + } else if (parsed.total === 0) { + errors.push( + 'assertions.json total is 0: a round with no executed assertion has no structured verdict', + ); + } else if (parsed.total !== parsed.pass + parsed.fail) { + errors.push( + `assertions.json is inconsistent: total ${parsed.total} != pass ${parsed.pass} + fail ${parsed.fail}`, + ); + } else { + counts = { pass: parsed.pass, fail: parsed.fail, total: parsed.total }; + } + } + } + + if (verdict === 'merge-ready' && counts && counts.fail > 0) { + errors.push( + `merge-ready with ${counts.fail} failed assertion${counts.fail === 1 ? '' : 's'}: expected A/B control reds count as passes, so a real fail contradicts the verdict`, + ); + } + + if (files.report && files.report.trim() !== '') { + const lead = files.report + .split('\n') + .filter((line) => line.trim() !== '') + .slice(0, 10); + if (verdict && !lead.some((line) => line.includes(verdict))) { + warnings.push( + `report.md does not state "${verdict}" in its first 10 non-empty lines`, + ); + } + const cited = lead.join('\n').match(/\b[0-9a-f]{40}\b/g) ?? []; + if (expectedHead) { + if (!cited.includes(expectedHead)) { + warnings.push( + `report.md does not name the verified head ${expectedHead} in its first 10 non-empty lines${cited.length > 0 ? ` (it cites ${cited.length} other OID${cited.length === 1 ? '' : 's'})` : ''}`, + ); + } + } else if (cited.length === 0) { + warnings.push( + 'report.md does not name a 40-character head OID in its first 10 non-empty lines', + ); + } + } + + if (evidence.length === 0) { + warnings.push( + 'evidence/ holds no PNG capture: a table is the claim, a capture is the witness', + ); + } + for (const name of evidence) { + if (!KEBAB_PNG.test(name)) { + warnings.push( + `evidence/${name} is not a kebab-case caption; the filename becomes the published caption`, + ); + } + } + + return { errors, warnings, counts, verdict }; +} + +function main(argv) { + if (argv.includes('--help') || argv.includes('-h')) { + process.stdout.write(HELP); + return 0; + } + const dir = argv.find((arg) => !arg.startsWith('-')); + if (!dir) { + process.stderr.write( + `verify-pr check: an artifact directory is required\n\n${HELP}`, + ); + return 2; + } + if (!existsSync(dir)) { + process.stderr.write(`verify-pr check: ${dir} does not exist\n`); + return 1; + } + + const files = { + report: readIfExists(join(dir, 'report.md')), + verdict: readIfExists(join(dir, 'verdict.txt')), + assertions: readIfExists(join(dir, 'assertions.json')), + }; + const evidenceDir = join(dir, 'evidence'); + const evidence = existsSync(evidenceDir) + ? readdirSync(evidenceDir).filter((name) => + name.toLowerCase().endsWith('.png'), + ) + : []; + + // When the round's own snapshot is beside the artifact directory, the report + // has to cite the head it verified — not merely some 40-hex token, which a + // base OID satisfies. A snapshot that exists but cannot be read is reported + // rather than silently downgrading the check. + let expectedHead = null; + let snapshotWarning = null; + const prNumber = /^pr(\d+)-verify-/.exec(basename(resolve(dir)))?.[1]; + if (prNumber) { + const snapshotPath = join( + dirname(resolve(dir)), + 'verify-context', + `pr${prNumber}.json`, + ); + if (existsSync(snapshotPath)) { + try { + const snapshot = JSON.parse(readFileSync(snapshotPath, 'utf8')); + if (/^[0-9a-f]{40}$/.test(snapshot?.headRefOid ?? '')) { + expectedHead = snapshot.headRefOid; + } + } catch { + snapshotWarning = + 'the verify-context snapshot is unreadable; the head-OID check fell back to presence-only'; + } + } + } + + const result = checkArtifacts({ files, evidence, expectedHead }); + if (snapshotWarning) result.warnings.unshift(snapshotWarning); + const lines = [`verify-pr check: ${dir}`]; + for (const warning of result.warnings) lines.push(` warn ${warning}`); + for (const error of result.errors) lines.push(` error ${error}`); + const counts = result.counts + ? `${result.counts.pass} passed · ${result.counts.fail} failed · ${result.counts.total} total` + : 'no usable assertion counts'; + lines.push( + ` ${result.errors.length === 0 ? 'ok' : 'failed'}: verdict=${result.verdict ?? 'none'} (${counts})`, + ); + process.stdout.write(`${lines.join('\n')}\n`); + return result.errors.length === 0 ? 0 : 1; +} + +if (isEntryPoint(import.meta.url, process.argv[1])) { + process.exitCode = main(process.argv.slice(2)); +} diff --git a/scripts/verify-pr-prepare.mjs b/scripts/verify-pr-prepare.mjs new file mode 100644 index 00000000000..a1f061bafeb --- /dev/null +++ b/scripts/verify-pr-prepare.mjs @@ -0,0 +1,433 @@ +#!/usr/bin/env node + +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Local preflight for a `/verify-pr` maintainer round. + * + * The CI lane supplies three things a workstation does not: the PR metadata + * snapshot (`$QWEN_VERIFY_CONTEXT`), a clone holding both the base and the head + * OID, and the artifact directory the publication step globs. Resolving them by + * hand is the part of the local path that fails silently — a snapshot from the + * wrong repository, `HEAD^1` mistaken for the base on a plain PR-head checkout, + * or a directory named without the `-verify-` infix all produce a round whose + * report is about another PR or is discarded outright. + * + * This script resolves those coordinates and nothing else. It does not checkout + * a worktree, build, or execute PR code. It also cannot gate on isolation: + * resolving the coordinates needs `gh`, and `gh` and the boundary are mutually + * exclusive, so this path always runs on the host. The gate is the separate + * `--require-isolation` mode, meant to run inside the boundary before the round + * executes anything. + * + * Usage: + * node scripts/verify-pr-prepare.mjs --repo / + * node scripts/verify-pr-prepare.mjs --require-isolation + * + * Options: + * --repo / required for the coordinate path: never inferred + * from `origin`, because a fork's `origin` resolves the + * same PR number to an unrelated PR + * --require-isolation check the process this runs in and exit 4 when no + * boundary is detected; run it inside the boundary + * + * Exit codes: + * 0 coordinates resolved, or the boundary check passed + * 2 usage error + * 3 metadata, OID, or filesystem failure + * 4 --require-isolation found no boundary — do not run PR code here + */ + +import { spawnSync } from 'node:child_process'; +import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { isEntryPoint } from './lib/entry-point.js'; + +const OID = /^[0-9a-f]{40}$/; +const REPO = /^([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)$/; +const FIELDS = [ + 'number', + 'title', + 'body', + 'author', + 'baseRefName', + 'baseRefOid', + 'headRefOid', + 'commits', +].join(','); + +const HELP = `Usage: node scripts/verify-pr-prepare.mjs --repo / + node scripts/verify-pr-prepare.mjs --require-isolation + +Resolves the coordinates a local /verify-pr round needs and writes the +metadata snapshot the round reads as $QWEN_VERIFY_CONTEXT: + + - fetches the PR snapshot with gh, from the repository you name + - verifies the base and head OIDs exist in this clone + - creates tmp/pr-verify-/ with an evidence/ subdirectory + - reports the isolation signals this process can observe + +The coordinate path runs on the host by construction: it needs gh, and gh and +the boundary are mutually exclusive. Run --require-isolation inside the +boundary, as the round's first command; exit 4 means this process is not +isolated and must not execute PR code. A container marker alone does not pass: +the CLI's own Docker/Podman launcher mounts ~/.config/gcloud and the ADC file +and copies model API keys, so the check also requires the credential-free +attestation and refuses when a known credential signal is visible. The +built-in macOS sandbox (SANDBOX=sandbox-exec) never counts. +`; + +/** Returns `{owner, name}` for a GitHub repository slug, or null. */ +export function parseRepo(value) { + const match = REPO.exec(String(value ?? '').trim()); + if (!match) return null; + return { owner: match[1], name: match[2] }; +} + +/** + * Returns the artifact directory name the publication step globs. The + * `-verify-` infix is part of that contract, not decoration. + */ +export function artifactDirName(pr, now = new Date()) { + const pad = (value) => String(value).padStart(2, '0'); + const stamp = [ + now.getFullYear(), + pad(now.getMonth() + 1), + pad(now.getDate()), + ].join(''); + const clock = [ + pad(now.getHours()), + pad(now.getMinutes()), + pad(now.getSeconds()), + ].join(''); + return `pr${pr}-verify-${stamp}-${clock}`; +} + +/** + * The built-in macOS sandbox announces itself with this value while its + * default `permissive-open` profile still lets the process read host files, so + * it is not the credential-free boundary the round needs. + */ +export const SEATBELT = 'sandbox-exec'; + +/** + * Set this only after confirming the boundary mounts no host credentials. A + * marker says a container exists; it cannot say the container is clean, and + * the CLI's own Docker/Podman launcher mounts `~/.config/gcloud`, the ADC file + * and every model API key it can see, so the two questions are separate. + */ +export const CREDENTIAL_FREE_ATTESTATION = 'QWEN_VERIFY_CREDENTIAL_FREE'; + +const CREDENTIAL_ENV = [ + 'GOOGLE_APPLICATION_CREDENTIALS', + 'GEMINI_API_KEY', + 'GOOGLE_API_KEY', + 'OPENAI_API_KEY', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY', + 'AWS_SESSION_TOKEN', + 'AZURE_CLIENT_SECRET', + 'SSH_AUTH_SOCK', + 'GH_TOKEN', + 'GITHUB_TOKEN', + 'KUBECONFIG', + // An arbitrary mount list can carry anything, so it cannot be attested to. + 'SANDBOX_MOUNTS', +]; + +const CREDENTIAL_PATHS = [ + ['.config/gcloud', '~/.config/gcloud'], + ['.aws', '~/.aws'], + ['.azure', '~/.azure'], + ['.ssh', '~/.ssh'], + ['.kube/config', '~/.kube/config'], + ['.git-credentials', '~/.git-credentials'], + ['.netrc', '~/.netrc'], + ['.docker/config.json', '~/.docker/config.json'], + // The persistent form of GH_TOKEN/GITHUB_TOKEN and a registry token. + ['.config/gh', '~/.config/gh'], + ['.npmrc', '~/.npmrc'], +]; + +/** + * Known credential signals. An empty list is not proof of a clean boundary — + * it is what the process can see — which is why the gate also needs the + * attestation below rather than treating the absence as a pass. + */ +export function detectCredentialSignals( + env = process.env, + exists = existsSync, + home = homedir(), +) { + const signals = []; + for (const name of CREDENTIAL_ENV) { + if (env[name]) signals.push(name); + } + for (const [relative, label] of CREDENTIAL_PATHS) { + if (exists(join(home, relative))) signals.push(label); + } + return signals; +} + +/** + * Reports the isolation signals this process can observe. Only a + * container-backed signal counts: a VM carries none of them, which is why the + * caller decides what a missing signal means, and why the seatbelt flag is + * reported separately rather than folded into `isolated`. + */ +export function detectIsolation(env = process.env, exists = existsSync) { + const signals = []; + if (env.container) signals.push(`container=${env.container}`); + if (exists('/.dockerenv')) signals.push('/.dockerenv'); + if (exists('/run/.containerenv')) signals.push('/run/.containerenv'); + // A boolean-looking value reads as "sandbox disabled"; the CLI's transport + // writes a container name or `sandbox-exec`, neither of which is a marker. + const sandbox = (env.SANDBOX ?? '').trim(); + if (sandbox && !['false', 'true', '0', '1'].includes(sandbox)) { + if (sandbox !== SEATBELT) signals.push(`SANDBOX=${sandbox}`); + } + return { + isolated: signals.length > 0, + signals, + seatbelt: sandbox === SEATBELT, + }; +} + +/** Returns the normalized snapshot fields, or null when the shape is wrong. */ +export function validateContext(json) { + if (!json || typeof json !== 'object') return null; + const pr = json.number; + const base = json.baseRefOid; + const head = json.headRefOid; + if (!Number.isInteger(pr) || pr <= 0) return null; + if (typeof base !== 'string' || !OID.test(base)) return null; + if (typeof head !== 'string' || !OID.test(head)) return null; + return { + pr, + base, + head, + baseRefName: typeof json.baseRefName === 'string' ? json.baseRefName : '', + title: typeof json.title === 'string' ? json.title : '', + }; +} + +/** Drops C0/C1 control characters, which a terminal would interpret. */ +export function printable(text) { + return [...String(text ?? '')] + .filter((char) => { + const code = char.codePointAt(0); + return code > 0x1f && (code < 0x7f || code > 0x9f); + }) + .join(''); +} + +/** Parses argv into options, or `{error}` for a usage failure. */ +export function parseArgs(argv) { + const options = { + pr: null, + repo: null, + requireIsolation: false, + help: false, + }; + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === '--help' || arg === '-h') { + options.help = true; + } else if (arg === '--require-isolation') { + options.requireIsolation = true; + } else if (arg === '--repo') { + options.repo = argv[(index += 1)] ?? null; + if (!options.repo) return { error: '--repo needs /' }; + } else if (arg.startsWith('--repo=')) { + options.repo = arg.slice('--repo='.length); + } else if (/^\d+$/.test(arg) && options.pr === null) { + options.pr = Number(arg); + } else { + return { error: `unexpected argument: ${arg}` }; + } + } + if (options.help || options.requireIsolation) return options; + if (!Number.isInteger(options.pr) || options.pr <= 0) { + return { error: 'a PR number is required' }; + } + if (!parseRepo(options.repo)) { + return { error: '--repo / is required' }; + } + return options; +} + +function run(command, args) { + return spawnSync(command, args, { encoding: 'utf8' }); +} + +function report(lines) { + process.stdout.write(`${lines.join('\n')}\n`); +} + +function fail(code, message) { + process.stderr.write(`verify-pr prepare: ${message}\n`); + return code; +} + +function main(argv) { + const options = parseArgs(argv); + if (options.error) { + process.stderr.write(`verify-pr prepare: ${options.error}\n\n${HELP}`); + return 2; + } + if (options.help) { + process.stdout.write(HELP); + return 0; + } + if (options.requireIsolation) return requireBoundary(); + + const { owner, name } = parseRepo(options.repo); + const slug = `${owner}/${name}`; + + const toplevel = run('git', ['rev-parse', '--show-toplevel']); + if (toplevel.status !== 0) { + return fail(3, 'not inside a git worktree — run this from the clone'); + } + const root = toplevel.stdout.trim(); + + const view = run('gh', [ + 'pr', + 'view', + String(options.pr), + '--repo', + slug, + '--json', + FIELDS, + ]); + if (view.status !== 0) { + const detail = + (view.stderr || '').trim() || + view.error?.message || + `exit ${view.status}`; + return fail(3, `gh pr view failed: ${detail}`); + } + let snapshot; + try { + snapshot = JSON.parse(view.stdout); + } catch { + return fail(3, 'gh pr view returned output that is not JSON'); + } + const context = validateContext(snapshot); + if (!context) { + return fail( + 3, + 'gh returned a snapshot without a numeric PR and two 40-character OIDs', + ); + } + + const probes = [ + ['base', context.base, context.baseRefName || context.base], + ['head', context.head, `pull/${context.pr}/head`], + ]; + for (const [label, oid, ref] of probes) { + if ( + run('git', ['-C', root, 'cat-file', '-e', `${oid}^{commit}`]).status !== 0 + ) { + return fail( + 3, + `${label} ${oid} is not in this clone — run: git fetch https://github.com/${slug}.git ${ref}`, + ); + } + } + + const contextDir = join(root, 'tmp', 'verify-context'); + const contextPath = join(contextDir, `pr${context.pr}.json`); + const artifacts = join(root, 'tmp', artifactDirName(context.pr)); + try { + mkdirSync(contextDir, { recursive: true }); + // Recursive, so a retry within the same second lands in the directory this + // script just made instead of failing on it. + mkdirSync(artifacts, { recursive: true }); + mkdirSync(join(artifacts, 'evidence'), { recursive: true }); + } catch (error) { + return fail(3, `could not create the artifact directory: ${error.message}`); + } + writeFileSync(contextPath, `${JSON.stringify(snapshot, null, 2)}\n`); + + const isolation = detectIsolation(); + // The title comes from an untrusted fork, and a control character would reach + // the maintainer's terminal from a tool whose job is to protect it. + const title = printable(context.title).replace(/\s+/g, ' ').trim(); + report([ + 'verify-pr prepare', + ` repo ${slug}`, + ` pr #${context.pr}${title ? ` ${title}` : ''}`, + ` base ${context.base}`, + ` head ${context.head}`, + ` context ${contextPath}`, + ` artifacts ${artifacts}`, + ` isolation ${isolation.isolated ? isolation.signals.join(', ') : 'none detected'}`, + ]); + if (isolation.seatbelt) { + report([ + ' seatbelt SANDBOX=sandbox-exec (default profile allows host reads —', + ' not a boundary)', + ]); + } + report([ + '', + `Next: build the round in ${artifacts}. This preflight ran on the host,`, + 'because resolving the coordinates needs `gh` and `gh` is mutually', + 'exclusive with the boundary. Before executing PR code, run inside it:', + '', + ' node scripts/verify-pr-prepare.mjs --require-isolation', + ]); + return 0; +} + +/** + * The in-boundary check, and the only place its answer means anything: the + * coordinate path necessarily runs on the host, so a gate there would fire + * every round and teach the maintainer to ignore it. + */ +function requireBoundary() { + const isolation = detectIsolation(); + const credentials = detectCredentialSignals(); + const attested = process.env[CREDENTIAL_FREE_ATTESTATION] === '1'; + + if (!isolation.isolated) { + const detail = isolation.seatbelt + ? 'SANDBOX=sandbox-exec is the built-in macOS sandbox, whose default profile still reads host files' + : 'no container marker was found'; + process.stderr.write( + `verify-pr boundary: ${detail} — do not execute PR code in this process\n`, + ); + return 4; + } + + const refusals = []; + if (!attested) { + refusals.push( + `${CREDENTIAL_FREE_ATTESTATION}=1 is not set: a marker says a container exists, not that it carries no host credentials`, + ); + } + if (credentials.length > 0) { + refusals.push( + `credential signals contradict a credential-free boundary: ${credentials.join(', ')}`, + ); + } + if (refusals.length > 0) { + process.stderr.write( + `verify-pr boundary: ${isolation.signals.join(', ')} but ${refusals.join('; ')} — do not execute PR code here\n`, + ); + return 4; + } + report([ + `verify-pr boundary: ${isolation.signals.join(', ')}; credential-free per attestation, no known credential signal`, + ]); + return 0; +} + +if (isEntryPoint(import.meta.url, process.argv[1])) { + process.exitCode = main(process.argv.slice(2)); +} diff --git a/scripts/verify-pr-watch-gate.mjs b/scripts/verify-pr-watch-gate.mjs index 23bca9dcf8c..8216a9aaf1d 100644 --- a/scripts/verify-pr-watch-gate.mjs +++ b/scripts/verify-pr-watch-gate.mjs @@ -31,12 +31,13 @@ import { existsSync, mkdirSync, readFileSync, - realpathSync, unlinkSync, writeFileSync, } from 'node:fs'; import { join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { isEntryPoint } from './lib/entry-point.js'; + +export { isEntryPoint } from './lib/entry-point.js'; /** The probe's action values that justify a model turn. */ export const PASSING_ACTIONS = new Set(['resync', 'stop']); @@ -163,25 +164,6 @@ export function mainWorktree(cwd = process.cwd()) { return first ? first.slice('worktree '.length).trim() : cwd; } -/** - * Whether this module is the executed entry point. `import.meta.url` is a - * percent-encoded file URL, so comparing it to a plain `file://` + path breaks - * on spaces or non-ASCII characters — and a wrapper that silently does nothing - * looks exactly like a quiet tick to the scheduler. Both sides are resolved - * through `realpath`, because Node resolves the entry module's symlinks while - * `argv[1]` keeps the path the shell was given. - */ -export function isEntryPoint(metaUrl, argv1) { - if (!argv1) return false; - try { - return ( - realpathSync(resolve(argv1)) === realpathSync(fileURLToPath(metaUrl)) - ); - } catch { - return false; - } -} - function parseArgs(argv) { const out = { pr: null,