diff --git a/.qwen/skills/verify-pr/SKILL.md b/.qwen/skills/verify-pr/SKILL.md index 089dc43705a..e3a86dea30d 100644 --- a/.qwen/skills/verify-pr/SKILL.md +++ b/.qwen/skills/verify-pr/SKILL.md @@ -264,16 +264,23 @@ certify the round. node /scripts/verify-pr-model-proxy.mjs \ --listen > ~/.cache/qwen-verify/pr/proxy.json 2>&1 & echo $! > ~/.cache/qwen-verify/pr/proxy.pid + for _ in $(seq 1 100); do + node -e 'const j=JSON.parse(require("node:fs").readFileSync(process.argv[1], "utf8")); if (!j.token) process.exit(1)' ~/.cache/qwen-verify/pr/proxy.json 2>/dev/null && break + sleep 0.1 + done 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)" + test -n "$QWEN_VERIFY_PROXY_TOKEN" || { echo "proxy did not become ready"; exit 1; } ``` 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. The per-run token is - read from the file into the environment, never typed; Docker copies it with - the bare `-e` below. + round and stop it at the end — it holds the real key. The background + process truncates `proxy.json` before it prints, so the loop waits for a + complete line with a token, and the `test -n` catches a read that `export` + would otherwise hide. The 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: diff --git a/docs/design/verify-pr-local-model-proxy.md b/docs/design/verify-pr-local-model-proxy.md index 713b9e05cf8..0ac746fe8a1 100644 --- a/docs/design/verify-pr-local-model-proxy.md +++ b/docs/design/verify-pr-local-model-proxy.md @@ -96,15 +96,18 @@ the real key. 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. + or `.netrc` is refused; installed `node_modules`/`dist` are ignored at any + depth, including inside a workspace package; enumeration is path-limited so + a large ignored dependency tree is never buffered; 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 (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. + lines, the proxy's coordinates are awaited before the token is exported, 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 90f9953f199..8213f46f5d0 100644 --- a/docs/design/verify-pr-local-model-proxy.zh-CN.md +++ b/docs/design/verify-pr-local-model-proxy.zh-CN.md @@ -35,8 +35,8 @@ CI 车道保留内联代理：那份副本运行在 base-pinned workflow 的 roo ## 验收 - 代理：bearer 拒绝、精确路径白名单、base path 前缀、真实 key 注入、chunked 请求体下 hop-by-hop 剥离（固定集合与 `Connection` 点名的头）、gzip 上游响应（头被剥离）、响应头前断连中止、下游暂停存活、慢但健康的流、上游重置存活、IPv6 监听格式化，以及 CLI 的 JSON 坐标。 -- 凭据扫描：嵌套 ignored 的 `.env`、`*.pem`、`credentials*`、`.npmrc`、`.netrc` 被拒绝；已安装的 `node_modules` 与 `dist/` 不被误拒；去掉 `--ignored` 模式或删掉某条模式会让测试变红。 +- 凭据扫描：嵌套 ignored 的 `.env`、`*.pem`、`credentials*`、`.npmrc`、`.netrc` 被拒绝；`node_modules`/`dist` 在任意深度（含 workspace 包内）被忽略；枚举由 pathspec 限制，不会先缓冲整棵依赖树；去掉 `--ignored` 模式或删掉某条模式会让测试变红。 - 检查器：`--context` 覆盖可写产物副本、不可用快照时拒绝而不是回退（产物副本先种一个合法 head），并保持既有退出码。 -- 配方：每个脚本都点名可信 checkout、base 提交的 `.qwen` 只读挂载、两个凭据都来自文件而非命令行、宣誓在轮次 shell 但不在 `docker run` 行；任一被还原都会让配方 pin 变红。 +- 配方：每个脚本都点名可信 checkout、base 提交的 `.qwen` 只读挂载、两个凭据都来自文件而非命令行、导出 token 前等待代理坐标就绪、宣誓在轮次 shell 但不在 `docker run` 行；任一被还原都会让配方 pin 变红。 - 变异证据：移除上述任一守卫，对应 pin 必须变红。 - 真实 Docker 冒烟：`QWEN_VERIFY_PROXY_TOKEN` 时门禁通过、`OPENAI_API_KEY` 时门禁拒绝、容器请求到达 mock 上游且真实 key 已注入。 diff --git a/scripts/tests/verify-pr-credential-scan.test.js b/scripts/tests/verify-pr-credential-scan.test.js index 9bb19e3f62c..e6f051724a4 100644 --- a/scripts/tests/verify-pr-credential-scan.test.js +++ b/scripts/tests/verify-pr-credential-scan.test.js @@ -11,6 +11,7 @@ import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterEach, describe, expect, it } from 'vitest'; import { + gitScanArgs, isCredentialShaped, scanIgnoredCredentials, } from '../../scripts/verify-pr-credential-scan.mjs'; @@ -78,6 +79,17 @@ describe('credential basename matching', () => { }); }); +describe('gitScanArgs', () => { + it('enumerates with depth-aware credential pathspecs', () => { + const args = gitScanArgs('/repo'); + expect(args).toContain(':(glob)**/.env'); + expect(args).toContain(':(glob)**/*.pem'); + // Excludes apply at workspace depth, not only the repository root. + expect(args).toContain(':(exclude,glob)**/node_modules/**'); + expect(args).toContain(':(exclude,glob)**/dist/**'); + }); +}); + describe('scanIgnoredCredentials', () => { it('reports nested and top-level ignored credential files', () => { const dir = makeRepo( @@ -102,14 +114,36 @@ describe('scanIgnoredCredentials', () => { ]); }); - it('does not refuse an installed tree or build output', () => { + it('does not refuse an installed tree or build output at any depth', () => { const dir = makeRepo( - ['node_modules/pkg/credentials.json', 'dist/app.pem', 'src/app.ts'], + [ + 'node_modules/pkg/credentials.json', + 'dist/app.pem', + 'packages/foo/node_modules/pkg/credentials.json', + 'packages/foo/dist/credentials.js', + 'packages/foo/dist/app.pem', + 'src/app.ts', + ], ['node_modules/', 'dist/', '*.pem', 'credentials*'], ); expect(scanIgnoredCredentials(dir)).toEqual([]); }); + it('limits enumeration with pathspecs instead of buffering the tree', () => { + const files = Array.from( + { length: 3000 }, + (_, index) => + `packages/foo/node_modules/pkg/dependency-with-a-long-name-${index}.js`, + ); + files.push('vault/.env'); + const dir = makeRepo(files, ['node_modules/', 'vault/']); + // A 64 KiB ceiling is far below the dependency listing: only the + // pathspec-restricted output (the nested .env) fits without ENOBUFS. + expect(scanIgnoredCredentials(dir, { maxBuffer: 64 * 1024 })).toEqual([ + 'vault/.env', + ]); + }); + it('stays silent on a clean tree', () => { const dir = makeRepo(['src/app.ts'], ['node_modules/']); const result = run(dir); diff --git a/scripts/tests/verify-pr-model-proxy.test.js b/scripts/tests/verify-pr-model-proxy.test.js index 395c4dab366..655596c5da0 100644 --- a/scripts/tests/verify-pr-model-proxy.test.js +++ b/scripts/tests/verify-pr-model-proxy.test.js @@ -52,6 +52,15 @@ function listen(server) { const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); +async function waitFor(predicate, timeoutMs = 1000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (predicate()) return; + await sleep(5); + } + throw new Error('condition not met before timeout'); +} + /** * A local upstream stand-in: records the request and the sequence it took to * answer (or to be aborted). No outbound network is involved. @@ -307,20 +316,20 @@ describe('verify-pr-model-proxy', () => { const upstream = await startUpstream({ delayMs: 100 }); const proxy = await startProxy(upstream); const { port } = new URL(proxy); - await new Promise((resolve) => { - const req = http.request({ - host: '127.0.0.1', - port, - path: '/chat/completions', - method: 'POST', - headers: { authorization: `Bearer ${TOKEN}` }, - }); - req.on('error', () => resolve()); - req.end('{}', () => { - req.destroy(); - resolve(); - }); + const req = http.request({ + host: '127.0.0.1', + port, + path: '/chat/completions', + method: 'POST', + headers: { authorization: `Bearer ${TOKEN}` }, }); + req.on('error', () => {}); + req.end('{}'); + // The end callback does not prove the proxy's upstream request exists yet; + // wait for the mock to record it, then disconnect while headers are still + // delayed so the pre-header abort is what the test exercises. + await waitFor(() => upstream.requests.length === 1); + req.destroy(); await sleep(400); expect(upstream.requests).toHaveLength(1); // The upstream must never answer a client that is already gone: with the @@ -585,6 +594,10 @@ describe('local container recipe pins', () => { expect(skill).not.toContain('REVIEW_OPENAI_API_KEY='); expect(skill).toContain('QWEN_VERIFY_PROXY_TOKEN="$(node -e'); expect(skill).toContain('chmod 600'); + // The background process truncates proxy.json before printing; the loop + // and the non-empty check keep an empty read from reaching Docker. + expect(skill).toContain('for _ in $(seq 1 100); do'); + expect(skill).toContain('test -n "$QWEN_VERIFY_PROXY_TOKEN"'); expect(skill).toMatch(/-e QWEN_VERIFY_PROXY_TOKEN[ \t]*\\?\n/); expect(skill).not.toContain('-e QWEN_VERIFY_PROXY_TOKEN='); }); diff --git a/scripts/verify-pr-credential-scan.mjs b/scripts/verify-pr-credential-scan.mjs index 6be59f4da02..84f1385af19 100644 --- a/scripts/verify-pr-credential-scan.mjs +++ b/scripts/verify-pr-credential-scan.mjs @@ -16,9 +16,12 @@ * 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 + * Installed dependencies and build output are excluded at any depth: a + * workspace package's own `node_modules` and `dist` are as irrelevant as the + * root ones, and `node_modules` alone ships hundreds of `credentials*` files. + * The pathspecs both restrict enumeration to credential-shaped names and + * exclude those directories, so an installed tree's multi-megabyte ignored + * listing never reaches the output buffer. A credential file placed in the * project (including a nested ignored directory) is still reported. * * Usage: @@ -31,21 +34,52 @@ 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 EXCLUDED_SEGMENTS = new Set(['node_modules', 'dist', '.git']); const CREDENTIAL_BASENAME = /^(?:\.env(?:\..*)?|.*\.env|.*\.pem|.*\.key|credentials.*|\.npmrc|\.netrc)$/; +// Git applies these pathspecs while enumerating, before any output is +// buffered: a full ignored listing of an installed checkout is several MB and +// used to overflow spawnSync's buffer before the JS filter could run. +const CREDENTIAL_PATHSPECS = [ + ':(glob)**/.env', + ':(glob)**/.env.*', + ':(glob)**/*.env', + ':(glob)**/*.pem', + ':(glob)**/*.key', + ':(glob)**/credentials*', + ':(glob)**/.npmrc', + ':(glob)**/.netrc', + ':(exclude,glob)**/node_modules/**', + ':(exclude,glob)**/dist/**', +]; + /** 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. */ +/** + * True for a path under an installed/build directory at any depth, including + * a workspace package's own `node_modules` and `dist`. + */ export function isExcluded(path) { - return EXCLUDED_DIRS.some( - (dir) => path === dir.slice(0, -1) || path.startsWith(dir), - ); + return path.split('/').some((segment) => EXCLUDED_SEGMENTS.has(segment)); +} + +export function gitScanArgs(root) { + return [ + '-C', + root, + 'ls-files', + '--others', + '--ignored', + '--exclude-standard', + '-z', + '--', + ...CREDENTIAL_PATHSPECS, + ]; } /** @@ -53,20 +87,14 @@ export function isExcluded(path) { * 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' }, - ); +export function scanIgnoredCredentials( + root, + { maxBuffer = 8 * 1024 * 1024 } = {}, +) { + const result = spawnSync('git', gitScanArgs(root), { + encoding: 'utf8', + maxBuffer, + }); if (result.status !== 0) { throw new Error( `could not list ignored files: ${(result.stderr || '').trim() || 'git failed'}`,