diff --git a/.qwen/skills/verify-pr/SKILL.md b/.qwen/skills/verify-pr/SKILL.md index 1a0bd015f89..089dc43705a 100644 --- a/.qwen/skills/verify-pr/SKILL.md +++ b/.qwen/skills/verify-pr/SKILL.md @@ -20,8 +20,10 @@ Two entry paths share the contract below. 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). + `cd && node /scripts/verify-pr-prepare.mjs --repo /` + (the trusted copy, with the PR clone only as its + cwd; see the container recipe below). The preflight requires the repository + explicitly; `origin` is not a fallback. It writes the round's coordinates (`repo`, `pr`, `baseRefOid`, `headRefOid`) to `context.json` and refuses a tree that is not a clean checkout of the PR head — when it passes, the current tree is the head side of the A/B, so stay @@ -146,8 +148,8 @@ 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 +Run `cd && 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, refuses a tree that is not a clean, fully materialised checkout of the PR head (a wrong HEAD names the `git checkout` to run; uncommitted changes name the entries and a stash form that clears @@ -159,7 +161,8 @@ creates `tmp/pr-verify-/` with a `context.json` holding `repo`, `pr`, 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 +`node /trusted-scripts/verify-pr-prepare.mjs --require-isolation` (the trusted +copy mounted read-only) 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 @@ -215,29 +218,40 @@ stripping. The CI lane deliberately keeps its own inline copy. Two checkouts are involved, and they are not interchangeable: - `` — a checkout of `main` (or another trusted revision) that - the PR cannot write. The proxy and the checker always run from here. + the PR cannot write. Every script the round runs on the host or inside the + boundary comes from here. - `` — a **standalone** clone checked out at the PR head. The round runs here, so its `.git` lives inside the tree; a linked worktree's gitfile - points at an absolute host path the container cannot see. Prepare runs here - on the host and refuses any other HEAD. + points at an absolute host path the container cannot see. Prepare is given + this tree as its cwd and refuses any other HEAD. -Never start the key-holding proxy or the post-round checker from -``: a PR can rewrite a script in its own tree, and those two steps -run with the real key and the maintainer's `gh` access. +Never execute a script from ``: the proxy, the prepare preflight, +the credential scan, the isolation gate and the checker all come from +``. A PR can rewrite a script in its own tree, and these steps +run with the real key, the maintainer's `gh` access, or the authority to +certify the round. -1. **Prepare on the host, then copy the full snapshot out of the tree:** +1. **Prepare from the trusted copy with the PR clone as its cwd**, then copy + the full snapshot out of the tree and base-pin `.qwen`: ```bash - node /scripts/verify-pr-prepare.mjs --repo / - mkdir -p ~/.cache/qwen-verify/pr - cp /tmp/verify-context/pr.json \ - ~/.cache/qwen-verify/pr/pr.json + cd && node /scripts/verify-pr-prepare.mjs --repo / + mkdir -p ~/.cache/qwen-verify/pr/qwen-base + cp /tmp/verify-context/pr.json ~/.cache/qwen-verify/pr/pr.json + BASE_OID="$(node -e 'const fs=require("node:fs"); console.log(JSON.parse(fs.readFileSync(process.argv[1], "utf8")).baseRefOid)' ~/.cache/qwen-verify/pr/pr.json)" + git -C archive "$BASE_OID" -- .qwen \ + | tar -x -C ~/.cache/qwen-verify/pr/qwen-base ``` - The artifact directory's `context.json` carries only the four coordinates; - the round reads the full gh snapshot (title, body, author, commits) from + prepare resolves the tree from its cwd, so the trusted copy runs with + `cwd=` and still verifies that this tree is the PR head. The + artifact directory's `context.json` carries only the four coordinates; the + round reads the full gh snapshot (title, body, author, commits) from `$QWEN_VERIFY_CONTEXT`. The host copy is also what pins the expected head after the round, because PR code can rewrite anything inside ``. + Mounting the base commit's `.qwen` over the tree (step 3) keeps the + round's instructions and project settings out of PR control, as the CI + lane's base-pin does. 2. **Start the proxy from the trusted checkout, in the background**, with the key read rather than typed onto the command line (a literal assignment @@ -250,12 +264,16 @@ run with the real key and the maintainer's `gh` access. node /scripts/verify-pr-model-proxy.mjs \ --listen > ~/.cache/qwen-verify/pr/proxy.json 2>&1 & echo $! > ~/.cache/qwen-verify/pr/proxy.pid + chmod 600 ~/.cache/qwen-verify/pr/proxy.json + export QWEN_VERIFY_PROXY_TOKEN="$(node -e 'const fs=require("node:fs"); console.log(JSON.parse(fs.readFileSync(process.argv[1], "utf8")).token)' ~/.cache/qwen-verify/pr/proxy.json)" ``` Bind the Docker bridge address, not `127.0.0.1` — a container cannot reach the host's loopback. The bearer check stays mandatory: the port is visible to every container on the bridge. Keep the proxy running for the whole - round and stop it at the end — it holds the real key. + round and stop it at the end — it holds the real key. The per-run token is + read from the file into the environment, never typed; Docker copies it with + the bare `-e` below. 3. **Start the boundary as a named, detached container** so every later step can be issued without a TTY: @@ -264,9 +282,11 @@ run with the real key and the maintainer's `gh` access. docker run -d --rm --name qwen-verify- \ --user "$(id -u):$(id -g)" -e HOME=/tmp/qwen-home \ -v :/work \ + -v /scripts:/trusted-scripts:ro \ + -v ~/.cache/qwen-verify/pr/qwen-base/.qwen:/work/.qwen:ro \ -v ~/.cache/qwen-verify/pr:/verify-context:ro \ -e QWEN_VERIFY_CONTEXT=/verify-context/pr.json \ - -e QWEN_VERIFY_PROXY_TOKEN= \ + -e QWEN_VERIFY_PROXY_TOKEN \ -w /work node:22-bookworm sleep infinity ``` @@ -299,43 +319,49 @@ run with the real key and the maintainer's `gh` access. JSON ``` -5. **Attest inside, after a check — never on the `docker run` line.** The - gate only sees known credential names and home paths, and gitignored files - in the round tree are invisible to its clean-tree check, so review the - mount list and reject credential-shaped ignored entries at any depth first: +5. **Attest inside, after trusted checks — never on the `docker run` line.** + The gate only sees known credential names and home paths, and gitignored + files are invisible to its clean-tree check, so scan for project + credential files and run the gate from the trusted copies first: ```bash docker exec -i qwen-verify- bash -lc ' set -e - credentials="$(git -C /work status --porcelain --ignored -- \ - ":(glob)**/.env" ":(glob)**/.env.*" ":(glob)**/*.env" \ - ":(glob)**/*.pem" ":(glob)**/*.key" ":(glob)**/credentials*" \ - ":(glob)**/.npmrc" ":(glob)**/.netrc")" - test -z "$credentials" || - { printf "%s\n" "$credentials"; exit 1; } + node /trusted-scripts/verify-pr-credential-scan.mjs /work export QWEN_VERIFY_CREDENTIAL_FREE=1 - node /work/scripts/verify-pr-prepare.mjs --require-isolation + node /trusted-scripts/verify-pr-prepare.mjs --require-isolation ' ``` - Exit 4 means this is not a boundary: stop and run nothing else. + The scan names credential-shaped ignored files (nested ones included) while + leaving installed dependencies and build output alone. Exit 4 means this is + not a boundary: stop and run nothing else. 6. **Run the round inside**, installing the CLI into the fresh home because - the container is not root. The round's engine is the released CLI; the PR - clone supplies the workspace skill and code. To run the tree's own CLI - instead, build it and install `/work` into the same prefix: + the container is not root. Each `docker exec` is a new shell, so the + attestation is passed to the shell that starts the round; it stays off the + `docker run` line. The round's engine is the released CLI; the PR clone + supplies the workspace skill and code: ```bash - docker exec -i qwen-verify- bash -lc ' + docker exec -i -e QWEN_VERIFY_CREDENTIAL_FREE=1 qwen-verify- bash -lc ' set -e export PATH="$HOME/.local/bin:$PATH" npm install -g --prefix "$HOME/.local" @qwen-code/qwen-code - # build the tree as the local install strategy requires, then: qwen --auth-type openai --approval-mode yolo \ --prompt "/verify-pr --repo /" ' ``` + To run the tree's own CLI instead, replace the install line with a build + and a local install after resolving the dependencies: + + ```bash + corepack pnpm install --frozen-lockfile + npm run build && npm run bundle + npm install -g --prefix "$HOME/.local" /work + ``` + 7. **After the round, check and clean up from the host.** The checker and its expected head both come from outside the round tree: @@ -344,6 +370,7 @@ run with the real key and the maintainer's `gh` access. --context ~/.cache/qwen-verify/pr/pr.json docker rm -f qwen-verify- kill "$(cat ~/.cache/qwen-verify/pr/proxy.pid)" + rm -f ~/.cache/qwen-verify/pr/proxy.json ~/.cache/qwen-verify/pr/proxy.pid ``` `` is `/tmp/pr-verify-`. Publishing still diff --git a/docs/design/verify-pr-local-model-proxy.md b/docs/design/verify-pr-local-model-proxy.md index 2d1c9bd8b38..713b9e05cf8 100644 --- a/docs/design/verify-pr-local-model-proxy.md +++ b/docs/design/verify-pr-local-model-proxy.md @@ -39,23 +39,31 @@ A host-side relay plus a recipe that names its trust boundaries. relays only the exact `${basePath}/chat/completions` path. Clients present a per-run bearer token (`QWEN_VERIFY_PROXY_TOKEN`); the proxy swaps it for the real key upstream. A nonce-echoing health endpoint distinguishes the process - from a squatter. Hop-by-hop request headers and the upstream - `content-encoding` are stripped. The idle watchdog is per-chunk, matches the + from a squatter. Hop-by-hop request headers — both the fixed set and any + name a `Connection` header nominates — and the upstream `content-encoding` + are stripped. The idle watchdog is per-chunk, matches the qwen client's own 240 s stream tolerance, stays quiet while the downstream is paused, and re-arms on drain; a disconnect before upstream headers aborts the fetch instead of paying for a completion nobody reads. -- The recipe uses two checkouts. `` runs the proxy and the - post-round checker; `` is a standalone clone at the PR head that - the container mounts. A linked worktree is rejected by construction because - its gitfile points outside the mount. +- The recipe uses two checkouts. `` supplies every script the + round runs, including the prepare preflight, the credential scan, the + isolation gate and the post-round checker; `` is a standalone + clone at the PR head that the container mounts and that prepare is given as + its cwd. A linked worktree is rejected by construction because its gitfile + points outside the mount. - The container is named and detached (`sleep infinity` plus `docker exec`), - runs as the host uid with `HOME=/tmp/qwen-home`, mounts the full snapshot - from a host copy outside the tree, and sets the credential-free attestation - inside only after rejecting credential-shaped ignored entries at any depth - (`.env`/`*.env`/`*.pem`/`*.key`/`credentials*`/`.npmrc`/`.netrc`). The CLI - installs into the fresh home, so nothing in the tree becomes root-owned; the - round's engine is the released CLI unless the tree's own build is installed - over it. + runs as the host uid with `HOME=/tmp/qwen-home`, mounts the base commit's + `.qwen` read-only over the tree so the round's instructions and project + settings cannot come from the PR, and mounts the full snapshot from a host + copy outside the tree. The per-run proxy token is exported from the proxy's + JSON file rather than typed, so it stays out of shell history. The + credential-free attestation is passed to the shell that starts the round, + after `verify-pr-credential-scan.mjs` (trusted copy) rejects + credential-shaped ignored files; that scan lists ignored files one by one + and leaves `node_modules`/`dist` alone, so a built tree is not refused. The + CLI installs into the fresh home, so nothing in the tree becomes + root-owned; the round's engine is the released CLI unless the tree's own + build is installed over it. - The round reads `$QWEN_VERIFY_CONTEXT` from the full snapshot. After the round, the checker runs from `` with `--context `, so a writable artifact `context.json` cannot pin its own expected head. @@ -82,13 +90,21 @@ the real key. ## Acceptance - Proxies: bearer rejection, exact path allowlist, base-path prefixing, - real-key injection, hop-by-hop stripping with a chunked body, a gzip upstream - body with the header stripped, pre-header disconnect abort, - paused-downstream survival, slow-but-healthy streams, upstream reset - survival, IPv6 listen formatting, and the CLI's JSON coordinates. + real-key injection, hop-by-hop stripping with a chunked body (fixed set and + `Connection`-nominated names), a gzip upstream body with the header + stripped, pre-header disconnect abort, paused-downstream survival, + slow-but-healthy streams, upstream reset survival, IPv6 listen formatting, + and the CLI's JSON coordinates. +- Credential scan: a nested ignored `.env`, `*.pem`, `credentials*`, `.npmrc` + or `.netrc` is refused; an installed `node_modules` tree and `dist/` are + not; a missing `--ignored` mode or a dropped pattern turns its test red. - Checker: `--context` overrides the writable artifact copy, refuses an - unusable snapshot instead of falling back, and keeps its existing exit - codes. + unusable snapshot instead of falling back (with a usable writable copy + seeded), and keeps its existing exit codes. +- Recipe: every script names the trusted checkout, the base commit's `.qwen` + is mounted read-only, both credentials come from files rather than command + lines, and the attestation is on the round's shell but not the `docker run` + line; reverting any of those turns a recipe pin red. - Mutation evidence: removing each of those guards turns its pin red. - A real Docker smoke covers the gate passing with `QWEN_VERIFY_PROXY_TOKEN`, the gate refusing `OPENAI_API_KEY`, and a container request reaching a mock diff --git a/docs/design/verify-pr-local-model-proxy.zh-CN.md b/docs/design/verify-pr-local-model-proxy.zh-CN.md index 99e0defcca4..90f9953f199 100644 --- a/docs/design/verify-pr-local-model-proxy.zh-CN.md +++ b/docs/design/verify-pr-local-model-proxy.zh-CN.md @@ -18,9 +18,9 @@ 一个宿主机侧转发器，加一套点明信任边界的配方。 -- `scripts/verify-pr-model-proxy.mjs` 监听选定的宿主地址（容器客户端用 Docker 网桥），持有 `REVIEW_OPENAI_API_KEY`，只转发精确的 `${basePath}/chat/completions` 路径。客户端出示一次性 bearer token（`QWEN_VERIFY_PROXY_TOKEN`），代理在上游替换为真实 key。回显 nonce 的健康端点用于区分本进程与抢占者。hop-by-hop 请求头与上游 `content-encoding` 被剥离（undici 已解码，转发该头会让客户端二次解压）。空闲看门狗按块续期，与 qwen 客户端自身的 240 秒流容忍一致；下游暂停期间不中止，drain 时续期；上游响应头之前客户端断开时中止 fetch，而不是为无人读取的补全付费。 -- 配方使用两个检出。`` 运行代理与轮次后的检查器；`` 是停在 PR head 的独立克隆，供容器挂载。链接 worktree 因其 gitfile 指向挂载外而在构造上被排除。 -- 容器有名字且后台运行（`sleep infinity` 加 `docker exec`），以宿主 uid 运行、`HOME=/tmp/qwen-home`，从树外的宿主副本挂载完整快照，并在拒绝任意深度的凭据形状 ignored 条目（`.env`/`*.env`/`*.pem`/`*.key`/`credentials*`/`.npmrc`/`.netrc`）之后才在容器内设置无凭据宣誓。CLI 安装进这个新 home，因此树里不会出现 root 属主文件；轮次的引擎是发布版 CLI，除非把树内构建装到同一前缀覆盖它。 +- `scripts/verify-pr-model-proxy.mjs` 监听选定的宿主地址（容器客户端用 Docker 网桥），持有 `REVIEW_OPENAI_API_KEY`，只转发精确的 `${basePath}/chat/completions` 路径。客户端出示一次性 bearer token（`QWEN_VERIFY_PROXY_TOKEN`），代理在上游替换为真实 key。回显 nonce 的健康端点用于区分本进程与抢占者。hop-by-hop 请求头（固定集合与 `Connection` 点名的头）与上游 `content-encoding` 被剥离（undici 已解码，转发该头会让客户端二次解压）。空闲看门狗按块续期，与 qwen 客户端自身的 240 秒流容忍一致；下游暂停期间不中止，drain 时续期；上游响应头之前客户端断开时中止 fetch，而不是为无人读取的补全付费。 +- 配方使用两个检出。`` 提供轮次运行的每个脚本，包括 prepare 预检、凭据扫描、隔离门禁与轮次后的检查器；`` 是停在 PR head 的独立克隆，供容器挂载，并作为 prepare 的 cwd。链接 worktree 因其 gitfile 指向挂载外而在构造上被排除。 +- 容器有名字且后台运行（`sleep infinity` 加 `docker exec`），以宿主 uid 运行、`HOME=/tmp/qwen-home`，把 base 提交的 `.qwen` 只读挂到树上（轮次指令与项目设置不能来自 PR），并从树外的宿主副本挂载完整快照。一次性代理 token 从代理的 JSON 文件导出而非键入，避免进入 shell history。无凭据宣誓传给启动轮次的 shell，且在可信的 `verify-pr-credential-scan.mjs` 拒绝凭据形状 ignored 文件之后才设置；该扫描逐文件列出 ignored 条目并放过 `node_modules`/`dist`，因此已构建的树不会被误拒。CLI 安装进这个新 home，因此树里不会出现 root 属主文件；轮次的引擎是发布版 CLI，除非把树内构建装到同一前缀覆盖它。 - 轮次从完整快照读取 `$QWEN_VERIFY_CONTEXT`。轮次结束后，检查器从 `` 运行并带 `--context <宿主副本>`，因此可写的产物 `context.json` 无法自证其期望 head。 CI 车道保留内联代理：那份副本运行在 base-pinned workflow 的 root 步骤里，若改为指向 PR 检出，PR 就能读到真实 key。 @@ -34,7 +34,9 @@ CI 车道保留内联代理：那份副本运行在 base-pinned workflow 的 roo ## 验收 -- 代理：bearer 拒绝、精确路径白名单、base path 前缀、真实 key 注入、chunked 请求体下 hop-by-hop 剥离、gzip 上游响应（头被剥离）、响应头前断连中止、下游暂停存活、慢但健康的流、上游重置存活、IPv6 监听格式化，以及 CLI 的 JSON 坐标。 -- 检查器：`--context` 覆盖可写产物副本、不可用快照时拒绝而不是回退，并保持既有退出码。 +- 代理：bearer 拒绝、精确路径白名单、base path 前缀、真实 key 注入、chunked 请求体下 hop-by-hop 剥离（固定集合与 `Connection` 点名的头）、gzip 上游响应（头被剥离）、响应头前断连中止、下游暂停存活、慢但健康的流、上游重置存活、IPv6 监听格式化，以及 CLI 的 JSON 坐标。 +- 凭据扫描：嵌套 ignored 的 `.env`、`*.pem`、`credentials*`、`.npmrc`、`.netrc` 被拒绝；已安装的 `node_modules` 与 `dist/` 不被误拒；去掉 `--ignored` 模式或删掉某条模式会让测试变红。 +- 检查器：`--context` 覆盖可写产物副本、不可用快照时拒绝而不是回退（产物副本先种一个合法 head），并保持既有退出码。 +- 配方：每个脚本都点名可信 checkout、base 提交的 `.qwen` 只读挂载、两个凭据都来自文件而非命令行、宣誓在轮次 shell 但不在 `docker run` 行；任一被还原都会让配方 pin 变红。 - 变异证据：移除上述任一守卫，对应 pin 必须变红。 - 真实 Docker 冒烟：`QWEN_VERIFY_PROXY_TOKEN` 时门禁通过、`OPENAI_API_KEY` 时门禁拒绝、容器请求到达 mock 上游且真实 key 已注入。 diff --git a/scripts/tests/verify-pr-check.test.js b/scripts/tests/verify-pr-check.test.js index 91617f8f898..0676391484d 100644 --- a/scripts/tests/verify-pr-check.test.js +++ b/scripts/tests/verify-pr-check.test.js @@ -792,6 +792,12 @@ describe.skipIf(process.platform === 'win32')('check (CLI)', () => { it('refuses an unusable --context instead of falling back', () => { const dir = makeArtifactDir(validFiles(), ['01-ab-cells.png']); + // Seed a usable writable copy: a fallback regression would exit 0 here + // and print its head source, so the exit-1 assertion pins the property. + writeFileSync( + join(dir, 'context.json'), + JSON.stringify({ headRefOid: HEAD }), + ); const empty = join(tmpdir(), `verify-pr-host-empty-${Date.now()}.json`); tmpDirs.push(empty); writeFileSync(empty, '{}'); diff --git a/scripts/tests/verify-pr-credential-scan.test.js b/scripts/tests/verify-pr-credential-scan.test.js new file mode 100644 index 00000000000..9bb19e3f62c --- /dev/null +++ b/scripts/tests/verify-pr-credential-scan.test.js @@ -0,0 +1,139 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { execFileSync, 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 { + isCredentialShaped, + scanIgnoredCredentials, +} from '../../scripts/verify-pr-credential-scan.mjs'; + +const SCRIPT = fileURLToPath( + new URL('../../scripts/verify-pr-credential-scan.mjs', import.meta.url), +); + +let tmpDirs = []; +afterEach(() => { + for (const dir of tmpDirs) rmSync(dir, { recursive: true, force: true }); + tmpDirs = []; +}); + +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', +}; + +function makeRepo(files, ignore) { + const dir = mkdtempSync(join(tmpdir(), 'verify-pr-scan-')); + tmpDirs.push(dir); + execFileSync('git', ['init', '-q', '.'], { cwd: dir, env: gitEnv }); + writeFileSync(join(dir, '.gitignore'), `${ignore.join('\n')}\n`); + for (const file of files) { + mkdirSync(join(dir, file, '..'), { recursive: true }); + writeFileSync(join(dir, file), 'secret'); + } + return dir; +} + +const run = (dir) => + spawnSync(process.execPath, [SCRIPT, dir], { encoding: 'utf8' }); + +describe('credential basename matching', () => { + it('matches the shapes a provider key or certificate uses', () => { + for (const path of [ + '.env', + '.env.local', + 'app.env', + 'secrets/prod.env', + 'tls.pem', + 'id.key', + 'credentials', + 'credentials.json', + '.npmrc', + '.netrc', + ]) { + expect(isCredentialShaped(path), path).toBe(true); + } + for (const path of [ + 'README.md', + 'package.json', + 'keymap.ts', + 'envelope.ts', + 'environment.md', + ]) { + expect(isCredentialShaped(path), path).toBe(false); + } + }); +}); + +describe('scanIgnoredCredentials', () => { + it('reports nested and top-level ignored credential files', () => { + const dir = makeRepo( + ['vault/.env', 'secrets/prod.env', 'tls.pem', '.npmrc', '.netrc'], + [ + '.env', + '*.env', + '*.pem', + '*.key', + 'credentials*', + '.npmrc', + '.netrc', + 'vault/', + ], + ); + expect(scanIgnoredCredentials(dir)).toEqual([ + '.netrc', + '.npmrc', + 'secrets/prod.env', + 'tls.pem', + 'vault/.env', + ]); + }); + + it('does not refuse an installed tree or build output', () => { + const dir = makeRepo( + ['node_modules/pkg/credentials.json', 'dist/app.pem', 'src/app.ts'], + ['node_modules/', 'dist/', '*.pem', 'credentials*'], + ); + expect(scanIgnoredCredentials(dir)).toEqual([]); + }); + + it('stays silent on a clean tree', () => { + const dir = makeRepo(['src/app.ts'], ['node_modules/']); + const result = run(dir); + expect(result.status).toBe(0); + expect(result.stdout).toBe(''); + }); + + it('exits 1 and names each entry when one is found', () => { + const dir = makeRepo(['vault/.env'], ['.env', 'vault/']); + const result = run(dir); + expect(result.status).toBe(1); + expect(result.stdout).toContain( + 'credential-shaped ignored entry: vault/.env', + ); + }); + + it('exits 2 without a root and 1 when git cannot list files', () => { + expect( + spawnSync(process.execPath, [SCRIPT], { encoding: 'utf8' }).status, + ).toBe(2); + const notRepo = mkdtempSync(join(tmpdir(), 'verify-pr-scan-nogit-')); + tmpDirs.push(notRepo); + const result = run(notRepo); + expect(result.status).toBe(1); + expect(result.stderr).toContain('could not list ignored files'); + }); +}); diff --git a/scripts/tests/verify-pr-model-proxy.test.js b/scripts/tests/verify-pr-model-proxy.test.js index 192c98c9752..395c4dab366 100644 --- a/scripts/tests/verify-pr-model-proxy.test.js +++ b/scripts/tests/verify-pr-model-proxy.test.js @@ -245,6 +245,7 @@ describe('verify-pr-model-proxy', () => { headers: { authorization: `Bearer ${TOKEN}`, connection: 'x-client-hop', + 'x-client-hop': 'client-secret', 'proxy-authorization': 'Basic client-proxy-secret', 'transfer-encoding': 'chunked', expect: '100-continue', @@ -266,6 +267,7 @@ describe('verify-pr-model-proxy', () => { expect(seen.authorization).toBe(`Bearer ${REAL_KEY}`); expect(seen.headers['proxy-authorization']).toBeUndefined(); expect(seen.headers.expect).toBeUndefined(); + expect(seen.headers['x-client-hop']).toBeUndefined(); expect(seen.headers.connection).not.toBe('x-client-hop'); }); @@ -535,41 +537,76 @@ describe('local container recipe pins', () => { 'utf8', ); - it('runs the key-holding proxy and the checker from a trusted checkout', () => { + it('runs every round script from the trusted checkout', () => { + expect(skill).toContain( + 'cd && node /scripts/verify-pr-prepare.mjs', + ); + expect(skill).toContain( + 'node /trusted-scripts/verify-pr-prepare.mjs --require-isolation', + ); + expect(skill).toContain( + 'node /trusted-scripts/verify-pr-credential-scan.mjs /work', + ); expect(skill).toContain( 'node /scripts/verify-pr-model-proxy.mjs', ); - expect(skill).toContain('node /scripts/verify-pr-check.mjs'); + // Enumerate the checker call sites: the preamble also names the trusted + // path, so a single positive assertion cannot detect step 7 reverting to + // /scripts. + const checkerCalls = [ + ...skill.matchAll(/node (\S+)\/scripts\/verify-pr-check\.mjs/g), + ].map((match) => match[1]); + expect(checkerCalls.length).toBeGreaterThanOrEqual(3); + for (const prefix of checkerCalls) expect(prefix).toBe(''); expect(skill).not.toMatch( - /node scripts\/verify-pr-(model-proxy|check)\.mjs/, + /node \/scripts\/verify-pr-(prepare|check|model-proxy)/, + ); + expect(skill).not.toMatch( + /node scripts\/verify-pr-(prepare|check|model-proxy)\.mjs/, + ); + }); + + it('base-pins the round instructions and mounts them read-only', () => { + expect(skill).toContain('git -C archive "$BASE_OID" -- .qwen'); + expect(skill).toContain( + '-v ~/.cache/qwen-verify/pr/qwen-base/.qwen:/work/.qwen:ro', ); }); it('mounts the round tree writable and the metadata snapshot read-only', () => { expect(skill).toContain('-v :/work'); + expect(skill).toContain('-v /scripts:/trusted-scripts:ro'); expect(skill).toContain('-v ~/.cache/qwen-verify/pr:/verify-context:ro'); expect(skill).toContain('QWEN_VERIFY_CONTEXT=/verify-context/pr.json'); }); - it('reads the real key instead of putting it on a command line', () => { + it('reads both credentials instead of putting them on a command line', () => { expect(skill).toContain("read -rsp 'REVIEW_OPENAI_API_KEY: '"); expect(skill).not.toContain('REVIEW_OPENAI_API_KEY='); + expect(skill).toContain('QWEN_VERIFY_PROXY_TOKEN="$(node -e'); + expect(skill).toContain('chmod 600'); + expect(skill).toMatch(/-e QWEN_VERIFY_PROXY_TOKEN[ \t]*\\?\n/); + expect(skill).not.toContain('-e QWEN_VERIFY_PROXY_TOKEN='); }); - it('keeps the attestation off the docker run line and behind a check', () => { - const dockerRun = skill.split('docker run -d')[1]?.split('```')[0] ?? ''; - expect(dockerRun).not.toContain('QWEN_VERIFY_CREDENTIAL_FREE'); - // Any depth, not only a root .env: a nested secrets/.env must be caught. - expect(skill).toContain(':(glob)**/.env'); - expect(skill).toContain(':(glob)**/*.pem'); - expect(skill).toContain(':(glob)**/credentials*'); - expect(skill).not.toMatch(/--ignored -- \.env\b/); - expect(skill).toContain('export QWEN_VERIFY_CREDENTIAL_FREE=1'); + it('keeps the attestation off the docker run line and on the round shell', () => { + const dockerRunMatch = skill.match(/docker run -d[\s\S]*?```/); + expect(dockerRunMatch).not.toBeNull(); + expect(dockerRunMatch[0]).not.toContain('QWEN_VERIFY_CREDENTIAL_FREE'); + // A fresh docker exec is a new shell: the round's shell must carry the + // attestation or its first isolation check exits 4. + expect(skill).toContain( + 'docker exec -i -e QWEN_VERIFY_CREDENTIAL_FREE=1 qwen-verify-', + ); + const roundBlock = skill.split('qwen --auth-type openai')[0].split('```'); + expect(roundBlock.at(-1)).toContain('QWEN_VERIFY_CREDENTIAL_FREE'); + expect(skill).toContain('verify-pr-credential-scan.mjs /work'); }); - it('names the released CLI as the engine and offers the tree build', () => { + it('names the released CLI as the engine and gives the tree-build route', () => { expect(skill).toContain("The round's engine is the released CLI"); - expect(skill).toContain('install `/work` into the same prefix'); + expect(skill).toContain('npm run build && npm run bundle'); + expect(skill).toContain('npm install -g --prefix "$HOME/.local" /work'); }); it('pins the checker head from the host snapshot', () => { diff --git a/scripts/verify-pr-credential-scan.mjs b/scripts/verify-pr-credential-scan.mjs new file mode 100644 index 00000000000..6be59f4da02 --- /dev/null +++ b/scripts/verify-pr-credential-scan.mjs @@ -0,0 +1,106 @@ +#!/usr/bin/env node + +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Refuses credential-shaped ignored files in a `/verify-pr` round tree. + * + * The isolation gate checks known environment names and home-relative paths, + * and `verify-pr-prepare.mjs` refuses untracked files; a gitignored credential + * file is invisible to both. This scan lists ignored files one by one (git + * status collapses ignored directories into one line, which is why a widened + * pathspec set used to refuse every installed tree) and matches the basenames + * a provider key or certificate typically uses. + * + * Installed dependencies and build output are excluded: `node_modules` alone + * ships hundreds of `credentials*` files, and refusing a built tree would make + * the recipe's own tree-build route unusable. A credential file placed in the + * project (including a nested ignored directory) is still reported. + * + * Usage: + * node scripts/verify-pr-credential-scan.mjs + * + * Exit codes: 0 clean, 1 credential-shaped entries found, 2 usage error. + */ + +import { spawnSync } from 'node:child_process'; +import { basename } from 'node:path'; +import { isEntryPoint } from './lib/entry-point.js'; + +const EXCLUDED_DIRS = ['node_modules/', 'dist/', '.git/']; + +const CREDENTIAL_BASENAME = + /^(?:\.env(?:\..*)?|.*\.env|.*\.pem|.*\.key|credentials.*|\.npmrc|\.netrc)$/; + +/** True for a path whose basename is credential-shaped. */ +export function isCredentialShaped(path) { + return CREDENTIAL_BASENAME.test(basename(path)); +} + +/** True for an ignored path under an installed/build directory. */ +export function isExcluded(path) { + return EXCLUDED_DIRS.some( + (dir) => path === dir.slice(0, -1) || path.startsWith(dir), + ); +} + +/** + * Returns the ignored credential-shaped files under `root`, sorted. An empty + * array is the clean state; a git failure is thrown so the caller can refuse + * rather than treat it as clean. + */ +export function scanIgnoredCredentials(root) { + const result = spawnSync( + 'git', + [ + '-C', + root, + 'ls-files', + '--others', + '--ignored', + '--exclude-standard', + '-z', + ], + { encoding: 'utf8' }, + ); + if (result.status !== 0) { + throw new Error( + `could not list ignored files: ${(result.stderr || '').trim() || 'git failed'}`, + ); + } + return result.stdout + .split('\0') + .filter(Boolean) + .filter((path) => !isExcluded(path)) + .filter((path) => isCredentialShaped(path)) + .sort(); +} + +function main(argv) { + const root = argv[0]; + if (!root || argv.length > 1) { + process.stderr.write( + 'Usage: node scripts/verify-pr-credential-scan.mjs \n', + ); + return 2; + } + let found; + try { + found = scanIgnoredCredentials(root); + } catch (error) { + process.stderr.write(`verify-pr credential scan: ${error.message}\n`); + return 1; + } + for (const path of found) { + process.stdout.write(`credential-shaped ignored entry: ${path}\n`); + } + return found.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-model-proxy.mjs b/scripts/verify-pr-model-proxy.mjs index 759596a4f1c..aa94faaf18e 100644 --- a/scripts/verify-pr-model-proxy.mjs +++ b/scripts/verify-pr-model-proxy.mjs @@ -189,6 +189,13 @@ export function createModelProxy({ const headers = new Headers(req.headers); for (const name of STRIPPED_REQUEST_HEADERS) headers.delete(name); + // RFC 7230 §6.1: a header nominated by Connection is hop-by-hop too, + // so a client cannot smuggle one through under an unlisted name. + const nominated = String(req.headers.connection ?? '') + .split(',') + .map((name) => name.trim().toLowerCase()) + .filter(Boolean); + for (const name of nominated) headers.delete(name); headers.set('authorization', `Bearer ${apiKey}`); const init = { method: req.method, headers };