QwenLM/qwen-code
1
0 Fork 0
mirror of https://github.com/QwenLM/qwen-code.git synced 2026-11-07 06:19:43 -10:00
Code Issues Packages Projects Releases 1 Wiki Activity

feat(verify-pr): add local Docker recipe with a host-side model proxy (#16077)

Browse Source 
* feat(verify-pr): add local Docker recipe with a host-side model proxy

A local /verify-pr round needs the same isolation the CI lane provides,
but the skill only documented the boundary check, not how to run the
model credential through it. Add scripts/verify-pr-model-proxy.mjs, a
host-side relay that holds the real key and lets the container call the
model with a per-run bearer token, mirroring the CI lane's inline proxy
(POST /chat/completions only, nonce health check, per-chunk idle
watchdog). The listen address is a flag so the proxy can bind the Docker
bridge, and readiness is printed as one JSON line.

The SKILL recipe pairs it with a docker run template: the token travels
as QWEN_VERIFY_PROXY_TOKEN (not OPENAI_API_KEY, which the isolation gate
refuses), and the container's provider entry reads it via envKey. The CI
lane keeps its inline copy — the proxy holds the real key, so it must
not execute a script from the PR-head checkout.

Verified: 11 new pin tests plus all existing verify-pr suites (120
passed), and a real docker smoke — the gate passes inside
node:22-bookworm with the proxy token, refuses OPENAI_API_KEY with exit
4, and a container POST reaches a mock upstream with the real key
injected and the /v1 prefix preserved.

* fix(verify-pr): harden the model proxy after adversarial audit

Audit round 1 found the relay forwarded client hop-by-hop headers
(connection, transfer-encoding, expect, proxy-authorization, ...) to the
upstream, which RFC 7230 forbids and which fights undici's own framing
for the relayed stream. Strip them, with a chunked-body test that pins
the behaviour. Round 1 also found the CLI entry had no coverage: add
tests for the missing-config exit code and the printed JSON coordinates
(token override, base-path prefix, no real-key leak, health nonce), and
document that the printed baseUrl uses the listen address, so binding
0.0.0.0 yields a URL clients cannot dial.

Rounds 2 and 3 (open-ended re-read plus adversarial passes over the
updated diff) found nothing further. All verify-pr suites: 123 passed.

* fix(verify-pr): address review findings in the model proxy and checker

Round-1 review (12 inline findings at 70aed891bf). Proxy: abort the
upstream when the client disconnects before headers instead of paying
for a completion nobody reads (R1-1); make the idle watchdog pause-aware,
re-arm on drain, and match the client's own 240 s stream tolerance
(R1-3); bracket IPv6 listen hosts via an exported formatter and normalize
base paths once (R1-12). Checker: add --context so a host-held snapshot
pins the expected head instead of the writable artifact copy, and refuse
an unusable snapshot rather than silently falling back (R1-9).

Each guard is pinned by a test that was verified to go red when the guard
is removed (six mutations: pre-header abort, pause guard, per-chunk
re-arm, body error handler, IPv6 bracket, --context).

* docs(verify-pr): harden the local container recipe and record the design

Recipe: run the key-holding proxy and the checker from a trusted
checkout, never the PR tree (R1-6, R1-9); background the proxy with a pid
file and stop it at the end (R1-4); read the key instead of inlining it
(R1-5); use a named detached container driven by docker exec (R1-7);
require a standalone clone, the host uid and a fresh HOME (R1-8); mount
the full host-held metadata snapshot rather than the four-key artifact
copy (R1-10); and attest only inside, after rejecting credential-shaped
ignored files (R1-11). Add the bilingual design record (R1-2), cross-link
it from the local-round design, and pin the recipe's trust lines in tests
so reverting them turns the suite red.

* fix(verify-pr): address round-2 verification findings

Round 2 at 3509c2d1aa reported one Low and four Suggestions.

- Relay the exact <basePath>/chat/completions path. A suffix match also
  forwarded /admin/chat/completions to the upstream.
- Pin the response content-encoding strip with a gzip upstream: undici
  hands the proxy decoded bytes, so forwarding the header makes the client
  gunzip plain text.
- Reject credential-shaped ignored entries at any depth (a nested
  secrets/.env included) instead of only a root .env/.env.*.
- Name the round's engine as the released CLI and offer the tree build for
  a round that must exercise the PR's own CLI.

The CI inline copies keep their existing two-header strip: that lane is
untouched by design and a change there needs its own workflow review.

Tests: 141 passed. Three mutations verified red (exact path, gzip header,
nested credential pathspec). The review-pr check at this PR is red because
the bot lane exhausted ten retries on "429 rpm exceeded", not because of
this diff.

* fix(verify-pr): close round-3 trust-boundary findings

Round 3 at 3ccf38697d reported two Criticals against the recipe, one
fix-induced Critical, and four Suggestions.

- prepare, the credential scan, the isolation gate and the checker now all
  come from <trusted-clone>; prepare runs with the PR clone as its cwd so
  the HEAD check still applies (R2-1).
- The base commit's .qwen is extracted and mounted read-only over the
  tree, so the round's instructions and project settings cannot come from
  the PR (R2-2).
- The attestation is passed to the shell that starts the round, because
  every docker exec is a fresh shell (R1-11, fix-induced).
- The proxy token is exported from proxy.json and passed with a bare -e;
  the file is chmod 600 and removed at cleanup (R2-3).
- Connection-nominated hop-by-hop headers are stripped (R2-6).
- Recipe pins now enumerate every checker call site, require a non-empty
  docker-run slice, and seed the writable checker snapshot so a fallback
  regression is caught (R2-4, R2-5).
- New verify-pr-credential-scan.mjs lists ignored files one by one,
  refuses credential-shaped names at any depth, and leaves node_modules
  and dist alone — replacing the pathspec check that refused every
  installed or built tree (round-3 Low).

Tests: 148 passed; ten mutations verified red.

* fix(verify-pr): address round-4 review findings

Four P2 findings at 441a523927:

- The credential scan now limits git enumeration with credential pathspecs
  and depth-aware node_modules/dist excludes, so an installed tree's
  multi-megabyte ignored listing never overflows the output buffer.
- Those excludes apply inside workspace packages too, so packages/*/dist
  and packages/*/node_modules are not mistaken for project credentials.
- The recipe waits for proxy.json to carry a token and validates the
  exported value, so a truncated background write cannot reach Docker.
- The pre-header disconnect test waits until the mock upstream has
  recorded the request before destroying the downstream socket, instead
  of racing the proxy's own (correct) early abort.

Tests: 150 passed; three mutations verified red (enumeration pathspecs,
excludes, readiness loop).

* fix(verify-pr): refuse nested repositories in the credential scan

Round-4 verification at 19b14b1a8f left three Suggestions.

- A nested git repository is now refused instead of silently skipped: git
  does not descend into it, so a .env under tmp/pr-clone/ would pass. A
  vendored .git under node_modules/dist stays excluded like the files there.
- isExcluded is removed: the pathspecs already exclude those directories,
  and the surviving JS filter was unpinnable.
- The spaced Connection-nomination list is pinned; dropping trim made the
  proxy answer 502.
- The node_modules/dist credential exclusions are documented as deliberate.

Tests: 152 passed; two mutations verified red (nested refusal, trim).

* fix(verify-pr): address round-5 review findings

Four Criticals:
- The key entry works without a TTY: an interactive shell prompts, an
  agent's shell reads REVIEW_OPENAI_KEY_FILE and fails with a named error.
- prepare disables fsmonitor, hooks and credential helpers on the
  PR-controlled clone, and the scan refuses credential-bearing remote URLs.
- The proxy token is re-derived from proxy.json in the docker run step,
  because each step is its own shell.
- The credential scan covers .env~, *.p12, id_rsa*, gha-creds-*, and the
  other credential families the repo's own .gitignore names.

Suggestions:
- Guard call site moved to <trusted-clone>, <clone> defined for watch mode.
- The newest verify comment is snapshotted as previous-report.md.
- The real key is unset once the proxy owns it; the token is unset at cleanup.
- `{1..100}` replaces seq for macOS; --user and the proxy token's
  non-credential status are pinned; the scan abort is structural (`|| exit 1`).
- Step 6 wording matches the base-pin; the docs narrow the base-pin scope for
  root AGENTS.md/QWEN.md, list the real CI divergence, explain content-encoding
  in English, fix the zh token wording, and record the checker --context
  contract and the ownership attribution.
- The readiness gate compares the health nonce; --port is removed.

Test gaps closed: gzip header assertion, mid-body disconnect, reset event,
PUT, bare-base exact path, positive header passthrough, 502 transport
failure, full paused-stream equality, token shape, publish --context pin,
checker second positional, scan path fixtures, headers-only idle window.

Tests: 159 passed; twelve mutations verified red.

* fix(verify-pr): close round-5 verification findings

Two Lows reproduced at b5c717a5d5:
- prepare's hardening named clean filters but only disabled fsmonitor,
  hooks and credential helpers. It now enumerates every local
  filter.*.clean/smudge/process and empties it with a -c override before
  any git read; a planted-filter fixture proves the override with a
  positive control that fires without it.
- The nested-repository refusal over-refused installed trees whose
  node_modules holds a real vendored git repo (git reports the directory
  entry, which the exclude pathspec cannot match). The enumeration is now
  segment-filtered in JS, so vendored dependencies stay excluded while a
  nested project repository is still refused.

Suggestions and nits:
- The remote-URL guard reads url.*.insteadOf as well as remote.*.url and
  refuses any http(s) userinfo, including a token used as the username;
  ssh://git@ stays benign.
- Public keys (.pub) are no longer treated as credentials, and a
  backup.env~ fixture pins the trailing-tilde family.

Tests: 163 passed; six mutations verified red (filter overrides, vendored
segment filter, userinfo breadth, insteadOf enumeration, public-key
exclusion, backup.env~ pathspec).

* fix(verify-pr): close round-6 verification findings

Co-authored-by: Qwen-Coder <[email protected]>

* fix(verify-pr): close round-7 verification findings

Co-authored-by: Qwen-Coder <[email protected]>

* fix(verify-pr): address round-8 review findings

Co-authored-by: Qwen-Coder <[email protected]>

* fix(verify-pr): close round-9 review findings

Co-authored-by: Qwen-Coder <[email protected]>

* fix(verify-pr): close round-10 review findings

Co-authored-by: Qwen-Coder <[email protected]>

---------

Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
Shaojin WenandQwen-Coder authored and GitHub committed 2026-11-03 14:13:59 +08:00
1 parent 02b8b0108c
commit 68c9fa27fb
13 files changed
+3432 -48
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

.qwen/skills/verify-pr/SKILL.md
+285 -16
View File
@@ -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 <n>`, starting with
`node scripts/verify-pr-prepare.mjs <n> --repo <owner>/<repo>` (the
preflight requires the repository explicitly; `origin` is not a fallback).
`cd <pr-clone> && node <trusted-clone>/scripts/verify-pr-prepare.mjs <n> --repo <owner>/<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
@@ -35,8 +37,9 @@ Two entry paths share the contract below.
`/verify-pr <n>` 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 <artifact dir>`; it applies the consistency
rules the publisher enforces in CI.
`node <trusted-clone>/scripts/verify-pr-check.mjs --context <host snapshot> <artifact dir>`
from a checkout the PR cannot write; it applies the
consistency rules the publisher enforces in CI.
## Environment contract (CI verify job)
@@ -145,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 <n> --repo <owner>/<repo>` on the host
before anything else: it fetches the metadata snapshot, resolves and verifies
Run `cd <pr-clone> && node <trusted-clone>/scripts/verify-pr-prepare.mjs <n> --repo <owner>/<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
@@ -157,9 +160,11 @@ creates `tmp/pr<n>-verify-<ts>/` with a `context.json` holding `repo`, `pr`,
`baseRefOid` and `headRefOid`, 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
The boundary check belongs inside: run the trusted copy of the preflight with
`--require-isolation` as the round's first command there, and treat exit 4 as
"this is not a boundary". The Docker recipe below mounts that copy at
`/trusted-scripts/verify-pr-prepare.mjs`; a VM boundary runs the same file
from the trusted checkout instead. 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
@@ -205,6 +210,258 @@ intended `owner/repo` is a match to confirm, not a guess to avoid.
`--watch` installs a standing trigger instead of running once; the contract is
in **Watch mode** below.
## Local container recipe (Docker + model proxy)
The boundary and the model credential are separate problems: the container
isolates PR code, and a host-side proxy keeps the real model key out of the
container. `scripts/verify-pr-model-proxy.mjs` is that proxy — the same relay
rules as the CI lane's inline one (only `POST /chat/completions`, a per-run
bearer token, a per-chunk idle watchdog), plus hop-by-hop request header
stripping. The CI lane deliberately keeps its own inline copy.
Two checkouts are involved, and they are not interchangeable:
- `<trusted-clone>` — a checkout of `main` (or another trusted revision) that
the PR cannot write. Every script the round runs on the host or inside the
boundary comes from here.
- `<pr-clone>` — 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 is given
this tree as its cwd and refuses any other HEAD.
Never execute a script from `<pr-clone>`: the proxy, the prepare preflight,
the credential scan, the isolation gate, the publish guard and the checker all
come from `<trusted-clone>`. 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 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
cd <pr-clone> && node <trusted-clone>/scripts/verify-pr-prepare.mjs <n> --repo <owner>/<repo>
rm -rf ~/.cache/qwen-verify/pr<n>/qwen-base
mkdir -p ~/.cache/qwen-verify/pr<n>/qwen-base
cp <pr-clone>/tmp/verify-context/pr<n>.json ~/.cache/qwen-verify/pr<n>/pr.json
ME="$(gh api user --jq .login)" || { echo "could not read the gh user" >&2; exit 1; }
PREV="$(gh api "repos/<owner>/<repo>/issues/<n>/comments" --paginate --slurp \
| node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{const all=JSON.parse(d).flat();const hit=all.filter(c=>(/bot$/.test(c.user.login)||c.user.login===process.argv[1])&&c.body.split("\n\n")[0].includes("Verdict:")).pop();process.stdout.write(hit?hit.body+"\n":"")})' "$ME")" ||
{ echo "could not read PR comments" >&2; exit 1; }
if [ -n "$PREV" ]; then printf '%s\n' "$PREV" > ~/.cache/qwen-verify/pr<n>/previous-report.md
else rm -f ~/.cache/qwen-verify/pr<n>/previous-report.md; fi
BASE_OID="$(node -e 'const fs=require("node:fs"); console.log(JSON.parse(fs.readFileSync(process.argv[1], "utf8")).baseRefOid)' ~/.cache/qwen-verify/pr<n>/pr.json)"
git -C <pr-clone> archive "$BASE_OID" -- .qwen \
| tar -x -C ~/.cache/qwen-verify/pr<n>/qwen-base
```
prepare resolves the tree from its cwd, so the trusted copy runs with
`cwd=<pr-clone>` 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`, and the newest substantive verify comment lands
beside it as `previous-report.md` so a follow-up round keeps its
carried-forward findings. That file exists only when a report was actually
found — its presence is the follow-up trigger, so an empty file would
misclassify every first round — and the selector matches the first
paragraph of a bot's or your own report, not a reply quoting `Verdict:`.
The `.qwen` extraction starts from an empty directory, so a file the base
deleted or renamed cannot survive from an earlier round. The host copy is
also what pins the expected head after the round, because PR code can
rewrite anything inside `<pr-clone>` — including `.git/config`. After a
round the clone is PR-controlled: re-clone before another round instead of
trusting it again, and note that prepare's host-side git commands disable
`core.fsmonitor`, hooks, credential helpers and `core.excludesFile`, and
empty every locally defined clean/smudge/process filter — the enumeration
follows `include.path`, reads key names NUL-framed and re-validated (a
filter value with a newline must not forge overrides), covers
`.git/config.worktree` when `extensions.worktreeConfig` is set, and the
fixed hardening is applied last so nothing outranks it. A filter name
containing `=` is refused outright: git splits `-c` at the first `=`, so
such a name cannot be safely disabled by an override. Prepare also
refuses a `.git/info/exclude` with active patterns: no `-c` can neutralize
that file, and a fresh clone has only the commented default.
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; the root `AGENTS.md`/`QWEN.md` are not covered and
still come from the PR clone.
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
lands in shell history). An interactive shell may prompt; an agent's shell
has no TTY, so the key comes from `REVIEW_OPENAI_KEY_FILE` (default
`~/.config/qwen-verify/review-openai-key`, mode 600):
```bash
if [ -t 0 ]; then
read -rsp 'REVIEW_OPENAI_API_KEY: ' REVIEW_OPENAI_API_KEY; echo
else
# `read` returns 1 at EOF even when it assigned the variable, and key
# files written by `security find-generic-password -w` / `op read` /
# `printf '%s'` carry no trailing newline — judge the variable, not the
# exit status.
IFS= read -r REVIEW_OPENAI_API_KEY \
< "${REVIEW_OPENAI_KEY_FILE:-$HOME/.config/qwen-verify/review-openai-key}" || true
[ -n "$REVIEW_OPENAI_API_KEY" ] ||
{ echo "REVIEW_OPENAI_API_KEY is unset and no key file was readable; set the file named by REVIEW_OPENAI_KEY_FILE" >&2; exit 1; }
fi
export REVIEW_OPENAI_API_KEY
REVIEW_OPENAI_BASE_URL=<upstream-url> \
node <trusted-clone>/scripts/verify-pr-model-proxy.mjs \
--listen <bridge-ip> > ~/.cache/qwen-verify/pr<n>/proxy.json 2> ~/.cache/qwen-verify/pr<n>/proxy.err &
echo $! > ~/.cache/qwen-verify/pr<n>/proxy.pid
unset REVIEW_OPENAI_API_KEY
for _ in {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<n>/proxy.json 2>/dev/null && break
sleep 0.1
done
chmod 600 ~/.cache/qwen-verify/pr<n>/proxy.json
node -e 'const fs=require("node:fs"); const j=JSON.parse(fs.readFileSync(process.argv[1], "utf8")); const u=new URL(j.baseUrl); fetch(`http://${u.hostname}:${u.port}/__health`).then(r=>r.text()).then(t=>process.exit(t===j.nonce?0:1)).catch(()=>process.exit(1))' ~/.cache/qwen-verify/pr<n>/proxy.json ||
{ echo "proxy health check failed"; exit 1; }
```
Bind the Docker bridge address, not `127.0.0.1` — a container cannot reach
the host's loopback. On Docker Desktop (macOS/Windows) the bridge lives
inside the VM and has no host-bindable address: this recipe is tested on a
Linux host, and on Desktop you must bind an address the VM can reach and
dial that same address from the container. 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 background process truncates `proxy.json` before it prints,
so the loop waits for a complete line with a token; stderr goes to a
separate `proxy.err`, because one diagnostic line folded into the JSON
file would break every parse of it. The nonce is checked at the origin
because the printed `baseUrl` carries the upstream path prefix. The key is
unset as soon as the proxy owns its copy, so later diagnostics cannot leak
it, and the token stays in the file rather than this shell.
3. **Start the boundary as a named, detached container** so every later step
can be issued without a TTY. Each step is its own shell: the token is
re-derived from `proxy.json` here, not inherited from step 2.
```bash
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<n>/proxy.json)"
test -n "$QWEN_VERIFY_PROXY_TOKEN" || { echo "proxy.json has no token" >&2; exit 1; }
docker run -d --rm --name qwen-verify-<n> \
--user "$(id -u):$(id -g)" -e HOME=/tmp/qwen-home \
-v <pr-clone>:/work \
-v <trusted-clone>/scripts:/trusted-scripts:ro \
-v ~/.cache/qwen-verify/pr<n>/qwen-base/.qwen:/work/.qwen:ro \
-v ~/.cache/qwen-verify/pr<n>:/verify-context:ro \
-e QWEN_VERIFY_CONTEXT=/verify-context/pr.json \
-e QWEN_VERIFY_PROXY_TOKEN \
-w /work node:22-bookworm sleep infinity
```
No host `HOME`, no `gh` token, no `SANDBOX_MOUNTS`. The token travels as
`QWEN_VERIFY_PROXY_TOKEN`, **not** `OPENAI_API_KEY`: `--require-isolation`
refuses any non-empty `OPENAI_API_KEY` (it cannot tell a proxy token from
a real key), and this name is not a credential signal. Running as the host
uid keeps files written into the mounted tree — including the round's
artifacts — owned by the maintainer, not root.
4. **Point the provider at the proxy** in the container's fresh
`$HOME/.qwen/settings.json`; the runtime reads the credential from
`envKey`, so no `OPENAI_API_KEY` is needed:
```bash
docker exec -i qwen-verify-<n> bash -lc 'mkdir -p "$HOME/.qwen" && cat > "$HOME/.qwen/settings.json"' <<'JSON'
{
"security": { "auth": { "selectedType": "openai" } },
"model": { "name": "<model-id>" },
"modelProviders": {
"openai": [
{
"id": "<model-id>",
"envKey": "QWEN_VERIFY_PROXY_TOKEN",
"baseUrl": "<baseUrl from proxy.json>"
}
]
}
}
JSON
```
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-<n> bash -lc '
node /trusted-scripts/verify-pr-credential-scan.mjs /work || exit 1
export QWEN_VERIFY_CREDENTIAL_FREE=1
node /trusted-scripts/verify-pr-prepare.mjs --require-isolation
'
```
The scan names credential-shaped ignored files (nested ones included, but
not public keys; the private-key families cover RSA, Ed25519, ECDSA and
DSA) and credential-bearing git config. It inspects every local config
value rather than a list of keys — any value can hide a token URL,
`http.proxy` included — and refuses http(s) userinfo (a token used as the
username counts; the bare `git@` account name, in any spelling, carries
no secret and stays benign, like `ssh://git@`) plus `Authorization` /
`Proxy-Authorization` header values. The URL-bearing key families are
judged too: `url.*.insteadOf` / `url.*.pushInsteadOf` rewrites and
`http.<url>.*` sections. The config enumeration follows `include.path`
and covers the enabled `.git/config.worktree` scope, so a credential in
an included worktree file or the worktree config is refused too. The
file is optional: absent means an empty scope, while an unreadable or
malformed one still refuses. It leaves installed dependencies and build
output alone — a vendored git dependency under `node_modules`/`dist` is
excluded, not refused — and refuses a nested git repository outright on
every surface (ignored clone, untracked clone, populated submodule): git
does not descend into it, so a credential file inside one would
otherwise pass silently. The abort is structural (`|| exit 1`), not a
side effect of `set -e`. 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. 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, and the PR
clone supplies the code under test; the round's instructions come from the
base commit's `.qwen` mounted in step 3, not from the PR. Root
`AGENTS.md`/`QWEN.md` do come from the PR clone — when the diff touches
them, treat the round's context as PR-influenced:
```bash
docker exec -i -e QWEN_VERIFY_CREDENTIAL_FREE=1 qwen-verify-<n> bash -lc '
set -e
export PATH="$HOME/.local/bin:$PATH"
npm install -g --prefix "$HOME/.local" @qwen-code/qwen-code
qwen --auth-type openai --approval-mode yolo \
--prompt "/verify-pr <n> --repo <owner>/<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:
```bash
node <trusted-clone>/scripts/verify-pr-check.mjs \
--context ~/.cache/qwen-verify/pr<n>/pr.json <round-dir>
docker rm -f qwen-verify-<n>
kill "$(cat ~/.cache/qwen-verify/pr<n>/proxy.pid)"
rm -f ~/.cache/qwen-verify/pr<n>/proxy.json ~/.cache/qwen-verify/pr<n>/proxy.err ~/.cache/qwen-verify/pr<n>/proxy.pid
unset QWEN_VERIFY_PROXY_TOKEN
```
`<round-dir>` is the artifact directory prepare printed —
`<pr-clone>/tmp/pr<n>-verify-<ts>`, never `tmp/verify-context`, which also
matches a loose `*verify*` glob. `<host snapshot>` throughout this recipe
is `~/.cache/qwen-verify/pr<n>/pr.json`, the host copy step 1 made.
Publishing still needs `gh`, so it stays on the host.
## Watch mode (local only)
`/verify-pr <n> --watch` installs a standing trigger instead of running once: a
@@ -220,6 +477,9 @@ directly: the probe also reports comments and failed checks, and every
non-empty gate output starts a turn whether or not the head moved. The wrapper
forwards only `action: "resync"` (a moved head) and `action: "stop"`.
`<clone>` below is the trusted checkout the watch scripts run from; its main
worktree holds the probe cursor under `.qwen/tmp/`.
Build the task in this order and stop at the first failure:
1. **Resolve a binary that carries the probe.** The gate runs in a fresh shell,
@@ -423,17 +683,25 @@ _— <model> via Qwen Code /verify-pr (v<cli-version>; skill <skill-hash>)_
Resolve the three values from the round's own runtime, never from memory:
the model is `$QWEN_CODE_MODEL_IDENTITY` when non-empty, else
`$QWEN_CODE_MODEL`; the CLI version is `qwen --version`; the skill hash is
the first 12 characters of
`git hash-object --no-filters .qwen/skills/verify-pr/SKILL.md` — the content
hash of the instructions this round actually loaded, which needs no git
history. When the model cannot be resolved, omit the footer rather than
the first 12 characters of the blob hash of the instructions this round
actually loaded. In the container recipe that is the base-pinned copy, so
hash `git -C <pr-clone> rev-parse "<baseRefOid>:.qwen/skills/verify-pr/SKILL.md"`
(equivalently, `git hash-object --no-filters` of the mounted
`/work/.qwen/skills/verify-pr/SKILL.md`) — never the PR head's copy: the two
blobs differ whenever the PR touches the skill, and hashing the head would
certify instructions the round never ran. A non-container round that loads
its own checkout hashes that file with
`git hash-object --no-filters .qwen/skills/verify-pr/SKILL.md`, which needs
no git history. When the model cannot be resolved, omit the footer rather than
post an empty identity — and never claim the CI lane's `sandboxed
verification` signature for a workstation round. Assemble the exact comment
body — report content plus the resolved footer — into `comment.md` in the
artifact directory and post that file: it is what the read-back below
compares against, and it gives the checker a place to verify the footer
actually landed. The local path has no publisher enforcing the fail/mismatch
rule, so run `node scripts/verify-pr-check.mjs <artifact dir>` and clear its
rule, so run
`node <trusted-clone>/scripts/verify-pr-check.mjs --context <host snapshot> <artifact dir>`
and clear its
errors and its verdict-shape and footer warnings before publishing (a
missing footer is legitimate only when the model could not be resolved),
act on a gates-size warning by recording the suites it ran under `gates`
@@ -467,8 +735,9 @@ GitHub rewrites the six-character escape text for NUL and SOH (a backslash,
`u`, then `0000` or `0001`) into a caret form, inside code spans and fences
too.
**Run both halves of that guard through `scripts/verify-pr-guard.mjs`, because
the hand-run version fails open.** `--pr <n> --repo <owner>/<repo> --snapshot
**Run both halves of that guard through
`<trusted-clone>/scripts/verify-pr-guard.mjs`, because the hand-run version
fails open.** `--pr <n> --repo <owner>/<repo> --snapshot
<file>` at round start and again before posting, then `--compare --start <a>
--now <b>`; it exits 1 on anything dirty. A fetch that errors returns no items,
so "is anything new?" answers "no" and a hand-rolled comparison reads clean —
docs/design/verify-pr-local-model-proxy.md
+209
View File
@@ -0,0 +1,209 @@
# Host-side model proxy for a containerised /verify-pr round
[English](verify-pr-local-model-proxy.md) | [简体中文](verify-pr-local-model-proxy.zh-CN.md)
Status: implemented.
## Problem
The local `/verify-pr` path must execute untrusted PR code inside a
credential-free boundary, and the skill's isolation gate says so. The recipe
stopped at that requirement: it had no way to let the round reach a model
without putting the real key inside the container, no container invocation,
and no host-side shape for the steps that must not run from the PR tree.
Three concrete failures followed:
- **The key had no safe path.** Exporting `OPENAI_API_KEY` inside the
container either hands the real credential to PR code or trips the isolation
gate, which refuses any non-empty `OPENAI_API_KEY` because it cannot tell a
proxy token from a real key.
- **The proxy and the checker are host steps with host privileges.** A
repo-relative `node scripts/...` executed from the round tree runs the PR's
own copy, while the proxy holds the real key and the checker reads the
artifact that certifies the round. The artifact directory is writable by the
code the round just executed.
- **The obvious container invocation does not support the round.** An
interactive `docker run -it` with no name cannot be driven by an agent;
mounting only the tree leaves a linked worktree's absolute gitfile
unreachable and produces root-owned files in the maintainer's checkout; and
the four-key artifact `context.json` is not the full metadata snapshot the
round reads.
## Proposal
A host-side relay plus a recipe that names its trust boundaries.
- `scripts/verify-pr-model-proxy.mjs` listens on a chosen host address (the
Docker bridge for container clients), holds `REVIEW_OPENAI_API_KEY`, and
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, and the local recipe compares that nonce before it uses
the proxy. Hop-by-hop request headers — both the fixed set and any name a
`Connection` header nominates — and the upstream `content-encoding` are
stripped; undici has already decoded the body, so forwarding that header
would make the client decompress it twice. 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. `<trusted-clone>` supplies every script the
round runs, including the prepare preflight, the credential scan, the
isolation gate, the publish guard and the post-round checker; `<pr-clone>`
is a standalone clone at the PR head that the container mounts and that
prepare is given as its cwd. The clone's `.git` is PR-controlled after a
round, so it is not reused, and host-side git commands disable fsmonitor,
hooks, credential helpers and `core.excludesFile`, and empty every locally
defined clean/smudge/process filter. The enumeration follows
`include.path`, reads key names NUL-framed and re-validated (a filter
value carrying a newline must not forge extra `-c` entries), covers
`.git/config.worktree` when `extensions.worktreeConfig` is set, and the
fixed hardening is applied last so no enumerated entry can outrank it. A
filter name containing `=` cannot be represented by `-c key=value` at all
(git splits the argument at the first `=`), so it is refused with its own
remedy rather than emitted as a broken override. `.git/info/exclude` has
no `-c` override, so active patterns there are refused outright — a fresh
clone ships only the commented default. 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`)
and runs with `--user "$(id -u):$(id -g)"` and `HOME=/tmp/qwen-home`, so
files written into the mounted tree stay maintainer-owned; the CLI installs
into the fresh home because there is no writable global prefix. The base
commit's `.qwen` is mounted read-only over the tree, so the round's skill
and project settings cannot come from the PR; the root `AGENTS.md`/`QWEN.md`
are not covered and still come from the PR clone. The full snapshot is
mounted from a host copy outside the tree, and the per-run proxy token is
derived from the proxy's JSON file in the step that starts the container, 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 and
credential-bearing git config. The config guard inspects VALUES, not a
list of keys — any value can hide a token URL (`http.proxy` included), so
a key allowlist never converges — plus the URL-bearing key families
(`url.*.insteadOf` / `url.*.pushInsteadOf` rewrites, `http.<url>.*`
sections), following `include.path` and covering the enabled
`.git/config.worktree` scope like prepare's filter enumeration. That file
is optional: absent means an empty scope (an explicit `--worktree --list`
would exit 128 and false-refuse a clean repo), while an existing but
unreadable or malformed one still refuses.
The file scan lists ignored files one by one and leaves
`node_modules`/`dist` alone, so a built tree is not refused; a nested git
repository is refused on every surface git collapses to a directory
entry — ignored clone, untracked clone, populated submodule. 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 `<trusted-clone>` with `--context <host copy>`,
so a writable artifact `context.json` cannot pin its own expected head.
The CI lane keeps its inline proxy: that copy runs in a root-owned step on the
base-pinned workflow, and pointing it at the PR checkout would let a PR read
the real key.
## Alternatives considered
- **Export the real key into the container.** Rejected: the boundary exists to
keep it out, and PR code can read its own environment.
- **Reuse the CI heredoc by extracting it and calling it from the workflow.**
Rejected for the CI lane: the script would come from the PR checkout. The
local script is therefore a deliberate second copy. It diverges in more than
transport wiring: it matches the exact path instead of a suffix, strips
`Connection`-nominated headers and `content-encoding`, uses a 240 s
pause-aware idle window instead of the CI copy's fixed 120 s, and aborts a
pre-header disconnect. A sync between the copies must carry those
behavioural differences, not just the flags.
- **Print `0.0.0.0` or normalise the listen host to loopback.** Rejected: the
printed value must be the address clients actually dial.
- **Mount the tree read-only.** Rejected as the only control: the round must
build in the tree and write its artifacts there. The trust boundary is
enforced by running the privileged host steps from `<trusted-clone>` and by
pinning the expected head from a host-held snapshot.
- **Scan `credential.helper` commands for inline tokens.** Declined as out
of the URL/header contract: a helper is a command, and spotting an inline
token in one means parsing shell. Prepare's own git reads already disable
`credential.helper`, and the recipe re-clones rather than reusing a
PR-controlled `.git`; the residual shape is recorded here rather than
silently dropped.
## Acceptance
- Proxies: bearer rejection, exact path allowlist, base-path prefixing,
real-key injection, hop-by-hop stripping with a chunked body (fixed set
and `Connection`-nominated names, with a non-nominated custom header as
the passthrough control), a gzip upstream body with the header stripped,
pre-header disconnect abort, paused-downstream survival, slow-but-healthy
streams, a slow request upload kept alive by the per-chunk re-arm,
upstream reset survival, a genuinely refused dial (bound-then-closed
port, asserted `ECONNREFUSED` — a blocked port would never dial), an
empty or flag-looking `--listen` rejected, IPv6 listen formatting, the
idle default read from the core constant's source rather than restated,
and the CLI's JSON coordinates.
- Credential scan: a nested ignored `.env`, `.env~`, `backup.env~`, `*.pem`,
`*.key`, `*.p12`, `*.pfx`, `id_rsa*`, `id_ed25519*`, `id_ecdsa*`, `id_dsa*`,
`credentials*`, `.git-credentials`, `.npmrc` (at depth too), `.netrc` or
`gha-creds-*.json` is refused (a `.pub` public key is not). The config
guard inspects every local value: any http(s) userinfo is refused,
including a token used as the username (the bare `git@` account name
carries no secret and stays benign in any spelling, like `ssh://git@`),
and so is any `Authorization` / `Proxy-Authorization` header value (other
header names stay clean); the URL-bearing key families (`insteadOf` /
`pushInsteadOf` rewrites, `http.<url>.*` sections) are judged on the
embedded URL. A novel key outside any historical list (`custom.mirror`,
`http.proxy`) is the witness that value inspection, not a key allowlist,
is the contract; the refusal names the key, never the value. Credentials
arriving through `include.path` or in the enabled `.git/config.worktree`
scope are refused; a credential-free include and benign worktree config
stay clean, and with the extension disabled the worktree file is ignored
exactly as git ignores it. The worktree file is optional: an enabled
extension with no file (or an empty file) is an empty scope on both the
scan and prepare paths, while an unreadable or malformed file refuses.
Installed `node_modules`/`dist` are ignored at any
depth, including inside a workspace package, and a vendored git
repository under them is excluded rather than refused; file enumeration
is path-limited so a large ignored dependency tree is never buffered
(the buffer fixture's files match a credential pathspec, so dropping the
restriction really overflows it). A nested git repository is refused on
every surface — ignored clone, untracked non-ignored clone, populated
submodule — while an unpopulated gitlink stays clean (its URL is covered
by the value scan); a missing `--ignored` mode or a dropped pattern turns
its test red.
- Host git hardening: prepare disables fsmonitor, hooks, credential helpers
and `core.excludesFile`, and empties every locally defined
clean/smudge/process filter — following `include.path`, NUL-framed key
names re-validated against forgery, `.git/config.worktree` covered when
`extensions.worktreeConfig` is set, fixed hardening applied last — before
reading the PR-controlled tree. Planted-filter fixtures prove the
override for a directly defined filter, one arriving through an included
worktree file, one in the worktree config, and a newline-bearing value
that must not forge a `-c` outranking the hardening, each with a positive
control where the vector fires without the guard. A filter name containing
`=` is refused with its own remedy on the module and the CLI path — no
override can represent it, and the refusal precedes any guarded status.
A hostile
`core.excludesFile` cannot hide untracked files from the hardened status;
active patterns in `.git/info/exclude` are refused with a re-clone
remedy, while the commented default passes. A non-git cwd is refused with
the worktree message (exit 3), not an uncaught enumeration error.
- 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 (including prepare, the
scan, the gate, the guard and the checker), the base commit's `.qwen` is
mounted read-only and extracted into a freshly emptied directory, both
credentials come from files rather than command lines (the key falls back
to `REVIEW_OPENAI_KEY_FILE` when stdin is not a TTY, is judged by the
variable rather than `read`'s EOF status so a newline-less key file works,
and is unset once the proxy owns it), the proxy's coordinates and nonce
are awaited before the container starts with stderr in a separate file so
the JSON stays parseable, the token is re-derived in the `docker run`
step, and the attestation is on the round's shell but not the `docker run`
line. `previous-report.md` is written only when a report was found — its
presence is the follow-up trigger — and selected by first-paragraph marker
from a bot or the maintainer, not by a substring any reply could quote.
The publish footer hashes the base-pinned skill copy the round actually
loaded, never the PR head's; 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
upstream with the real key injected.
docs/design/verify-pr-local-model-proxy.zh-CN.md
+44
View File
@@ -0,0 +1,44 @@
# 容器化 /verify-pr 轮次的宿主机模型代理
[English](verify-pr-local-model-proxy.md) | [简体中文](verify-pr-local-model-proxy.zh-CN.md)
状态：已实现。
## 问题
本地 `/verify-pr` 路径必须在无凭据边界内执行不可信的 PR 代码，skill 的隔离门禁也这样要求。但配方只停在要求上：它没有让轮次访问模型的途径而不把真实 key 放进容器，没有容器启动方式，也没有为「不能从 PR 树运行的宿主步骤」定义形态。
由此产生三个具体失败：
- **key 没有安全通道。** 在容器里导出 `OPENAI_API_KEY`，要么把真实凭据交给 PR 代码，要么触发隔离门禁——门禁拒绝任何非空的 `OPENAI_API_KEY`，因为它无法区分代理 token 与真实 key。
- **代理与检查器是持宿主权限的宿主步骤。** 从轮次树执行仓库相对路径的 `node scripts/...` 会运行 PR 自己的副本，而代理持有真实 key、检查器读取用于认证本轮产物的文件；产物目录恰恰可被轮次刚执行的代码写入。
- **显而易见的容器调用方式支撑不了轮次。** 没有名字的交互式 `docker run -it` 无法由 agent 驱动；只挂载树会让链接 worktree 的绝对 gitfile 不可达，并在维护者检出里留下 root 属主文件；四键的产物 `context.json` 也不是轮次读取的完整元数据快照。
## 方案
一个宿主机侧转发器，加一套点明信任边界的配方。
- `scripts/verify-pr-model-proxy.mjs` 监听选定的宿主地址（容器客户端用 Docker 网桥），持有 `REVIEW_OPENAI_API_KEY`，只转发精确的 `${basePath}/chat/completions` 路径。客户端出示每轮一个、整轮有效的 bearer token（`QWEN_VERIFY_PROXY_TOKEN`，非单次使用），代理在上游替换为真实 key。回显 nonce 的健康端点用于区分本进程与抢占者，本地配方在使用代理前比较该 nonce。hop-by-hop 请求头（固定集合与 `Connection` 点名的头）与上游 `content-encoding` 被剥离（undici 已解码，转发该头会让客户端二次解压）。空闲看门狗按块续期，与 qwen 客户端自身的 240 秒流容忍一致；下游暂停期间不中止，drain 时续期；上游响应头之前客户端断开时中止 fetch，而不是为无人读取的补全付费。
- 配方使用两个检出。`<trusted-clone>` 提供轮次运行的每个脚本，包括 prepare 预检、凭据扫描、隔离门禁、发布守卫与轮次后的检查器；`<pr-clone>` 是停在 PR head 的独立克隆，供容器挂载，并作为 prepare 的 cwd。轮次结束后该克隆的 `.git` 属 PR 控制，不复用；宿主机侧 git 命令禁用 fsmonitor、hooks、credential helper 与 `core.excludesFile`，并把本地定义的 clean/smudge/process filter 全部置空——枚举跟随 `include.path`，以 NUL 分隔只读键名并逐个重新校验（filter 值里的换行不得伪造额外的 `-c` 条目），`extensions.worktreeConfig` 为真时连 `.git/config.worktree` 一起枚举，固定加固项放在最后使任何枚举项都无法压过它。含 `=` 的 filter 名根本无法用 `-c key=value` 表示（git 在第一个 `=` 处切分参数），因此带自己的补救被拒绝，而不是生成一个失效的 override。`.git/info/exclude` 没有 `-c` 可压制，因此其中的有效模式被直接拒绝——新鲜克隆只有注释默认内容。链接 worktree 因其 gitfile 指向挂载外而在构造上被排除。
- 容器有名字且后台运行（`sleep infinity` 加 `docker exec`），以 `--user "$(id -u):$(id -g)"` 与 `HOME=/tmp/qwen-home` 运行，因此写入挂载树的文件保持维护者所有；CLI 装进新 home 是因为容器没有可写的全局前缀。base 提交的 `.qwen` 只读挂到树上（轮次的 skill 与项目设置不能来自 PR）；根 `AGENTS.md`/`QWEN.md` 不在覆盖范围内，仍来自 PR 克隆。完整快照从树外宿主副本挂载，代理 token 在启动容器的那一步从 JSON 文件重新取得，避免进入 shell history。无凭据宣誓传给启动轮次的 shell，且在可信的 `verify-pr-credential-scan.mjs` 拒绝凭据形状 ignored 文件与带凭据的 git config 之后才设置；config 守卫检查的是值而不是键名列表——任何值都可能藏 token URL（含 `http.proxy`），键名白名单永远不会收敛——外加携带 URL 的键族（`url.*.insteadOf`/`url.*.pushInsteadOf` 改写、`http.<url>.*` 段），并与 prepare 的 filter 枚举一样跟随 `include.path`、覆盖启用的 `.git/config.worktree` 作用域；该文件是可选的：缺失即空作用域（显式 `--worktree --list` 会退出 128 误拒干净仓库），存在但不可读或损坏仍拒绝；文件扫描逐文件列出 ignored 条目并放过 `node_modules`/`dist`，因此已构建的树不会被误拒；嵌套 git 仓库在 git 会折叠成目录条目的每个面上都被拒绝——被忽略的克隆、未跟踪的克隆、已填充的子模块。轮次的引擎是发布版 CLI，除非把树内构建装到同一前缀覆盖它。
- 轮次从完整快照读取 `$QWEN_VERIFY_CONTEXT`。轮次结束后，检查器从 `<trusted-clone>` 运行并带 `--context <宿主副本>`，因此可写的产物 `context.json` 无法自证其期望 head。
CI 车道保留内联代理：那份副本运行在 base-pinned workflow 的 root 步骤里，若改为指向 PR 检出，PR 就能读到真实 key。
## 备选方案
- **把真实 key 导出进容器。** 否决：边界的存在就是为了把它挡在外面，而 PR 代码能读取自己的环境。
- **抽取 CI heredoc 并由 workflow 调用。** 对 CI 车道否决：脚本会来自 PR 检出。因此本地脚本是有意的第二份副本。它的差异不止传输接线：精确路径而非后缀、剥离 `Connection` 点名的头与 `content-encoding`、240 秒 pause 感知空闲窗口而非 CI 的固定 120 秒、以及响应头前断连中止。两份同步时必须带上这些行为差异，而不只是参数。
- **打印 `0.0.0.0`，或把监听地址归一化成 loopback。** 否决：打印值必须是客户端实际拨号的地址。
- **仅把树挂成只读。** 否决：轮次必须在树里构建并写产物。信任边界靠「特权宿主步骤从 `<trusted-clone>` 运行」以及「从宿主快照钉住期望 head」来落实。
- **扫描 `credential.helper` 命令里的内联 token。** 否决，超出 URL/头契约：helper 是命令，从中识别内联 token 需要解析 shell。prepare 自身的 git 读取已禁用 `credential.helper`，配方也要求重新克隆而不是复用 PR 可控的 `.git`；这一残余形态在此记录，而不是悄悄丢掉。
## 验收
- 代理：bearer 拒绝、精确路径白名单、base path 前缀、真实 key 注入、chunked 请求体下 hop-by-hop 剥离（固定集合与 `Connection` 点名的头，并以未被点名的自定义头作透传正对照）、gzip 上游响应（头被剥离）、响应头前断连中止、下游暂停存活、慢但健康的流、慢请求上传由按块续期保活、上游重置存活、真实被拒的拨号（先绑定后关闭的端口，断言 `ECONNREFUSED`——被屏蔽的端口根本不会拨号）、空值或形似 flag 的 `--listen` 被拒、IPv6 监听格式化、空闲默认值从 core 常量源码读取而非复述，以及 CLI 的 JSON 坐标。
- 凭据扫描：嵌套 ignored 的 `.env`、`.env~`、`backup.env~`、`*.pem`、`*.key`、`*.p12`、`*.pfx`、`id_rsa*`、`id_ed25519*`、`id_ecdsa*`、`id_dsa*`、`credentials*`、`.git-credentials`、`.npmrc`（含深层目录）、`.netrc`、`gha-creds-*.json` 被拒绝（`.pub` 公钥不算凭据）。config 守卫检查每个本地值：http(s) 的任意 userinfo 都被拒绝（含把 token 当用户名；裸 `git@` 账号名不含秘密，任意大小写拼写都与 `ssh://git@` 一样保持良性），带 `Authorization`/`Proxy-Authorization` 头的值也被拒绝（其他头名保持干净）；携带 URL 的键族（`insteadOf`/`pushInsteadOf` 改写、`http.<url>.*` 段）按键中内嵌的 URL 判定。任何历史列表之外的新键（`custom.mirror`、`http.proxy`）是"契约是值检查而非键名白名单"的见证；拒绝消息只点名键、不打印值。经 `include.path` 引入或位于启用的 `.git/config.worktree` 作用域的凭据被拒绝；不含凭据的 include 与良性 worktree 配置保持干净，扩展禁用时 worktree 文件与 git 一样被忽略。worktree 文件是可选的：扩展启用但文件缺失（或为空）在 scan 与 prepare 两路都是空作用域，不可读或损坏的文件仍拒绝。`node_modules`/`dist` 在任意深度（含 workspace 包内）被忽略，其下的 vendored git 仓库被排除而非拒绝；文件枚举由 pathspec 限制，不会先缓冲整棵依赖树（缓冲夹具的文件本身就匹配凭据 pathspec，去掉限制真的会溢出）。嵌套 git 仓库在每个面上都被拒绝——被忽略的克隆、未跟踪且未被忽略的克隆、已填充的子模块——未填充的 gitlink 保持干净（其 URL 由值扫描覆盖）；去掉 `--ignored` 模式或删掉某条模式会让测试变红。
- 宿主机 git 硬化：prepare 禁用 fsmonitor、hooks、credential helper 与 `core.excludesFile`，并在读取 PR 可控树之前把本地定义的 clean/smudge/process filter 全部置空——枚举跟随 `include.path`、NUL 分隔的键名逐个重新校验防伪造、`extensions.worktreeConfig` 为真时覆盖 `.git/config.worktree`、固定加固项放在最后。植入夹具证明四种 filter 形态的覆盖：直接定义、经被 include 的工作树文件、worktree config、以及值含换行不得伪造压过加固的 `-c`，各带一个不覆盖就会触发的正对照。含 `=` 的 filter 名在模块与 CLI 两路都带自己的补救被拒绝——没有 override 能表示它，且拒绝先于任何加固 status。恶意 `core.excludesFile` 无法对加固后的 status 隐藏未跟踪文件；`.git/info/exclude` 的有效模式以重新克隆为补救被拒绝，注释默认内容放行。非 git 目录以 worktree 消息干净拒绝（exit 3），而不是未捕获的枚举错误。
- 检查器：`--context` 覆盖可写产物副本、不可用快照时拒绝而不是回退（产物副本先种一个合法 head），并保持既有退出码。
- 配方：每个脚本都点名可信 checkout（含 prepare、扫描、门禁、守卫与检查器）、base 提交的 `.qwen` 只读挂载且解压到先清空的目录、两个凭据都来自文件而非命令行（stdin 非 TTY 时 key 从 `REVIEW_OPENAI_KEY_FILE` 读取，按变量而非 `read` 的 EOF 返回码判断，因此无结尾换行的 key 文件可用，代理拿到副本后立即 unset）、启动容器前等待代理坐标与 nonce 且 stderr 写入独立文件以保持 JSON 可解析、token 在 `docker run` 那一步重新取得、宣誓在轮次 shell 但不在 `docker run` 行。`previous-report.md` 只在确实找到报告时写入——它的存在就是跟进轮触发器——并按首段标记从 bot 或维护者本人的评论中选取，而不是任何回复都能引用的子串。发布 footer 哈希轮次实际加载的 base-pin skill 副本，绝不哈希 PR head 的；任一被还原都会让配方 pin 变红。
- 变异证据：移除上述任一守卫，对应 pin 必须变红。
- 真实 Docker 冒烟：`QWEN_VERIFY_PROXY_TOKEN` 时门禁通过、`OPENAI_API_KEY` 时门禁拒绝、容器请求到达 mock 上游且真实 key 已注入。
docs/design/verify-pr-local-round.md
+9 -1
View File
@@ -71,7 +71,10 @@ them rather than at the end of the document.
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.
harness ran. With `--context <snapshot>` the expected head comes from that
host-held file instead of the artifact's writable `context.json`, and an
unusable snapshot is a hard exit 1, not a warning: the artifact directory is
writable by the PR code the round just ran.
- `scripts/verify-pr-guard.mjs` owns both halves of the pre-publish guard.
`--pr <n> --repo <owner>/<repo> --snapshot <file>` fetches the head, the PR
state and all three comment lists with `--paginate`, and writes the file only
@@ -417,6 +420,11 @@ A published verification comment ends with the same attribution shape
later preparation into a different artifact directory. Publishable round
snapshots contain no PR title or body.
## Related designs
- The container boundary and its host-side model proxy:
[verify-pr-local-model-proxy.md](verify-pr-local-model-proxy.md).
## Open questions
None at this scope.
docs/design/verify-pr-local-round.zh-CN.md
+6 -1
View File
@@ -25,7 +25,7 @@
新增三个独立脚本，并让 skill 在流程需要的位置指向它们，而不是放在文末。
- `scripts/verify-pr-prepare.mjs <pr> --repo <owner>/<repo>` 负责解析坐标：用 lane 相同的字段列表抓取快照、校验两个 OID 在本克隆中存在、拒绝不是 PR head 干净检出的树、创建带 `evidence/` 子目录的产物目录，并报告当前进程能观测到的隔离信号。它不据此设闸：解析元数据需要 `gh`，而 `gh` 与边界互斥，因此这条路径总是在宿主上运行。闸门是 `--require-isolation`，用于在边界内、轮次执行任何东西之前运行。
- `scripts/verify-pr-check.mjs <dir>` 把发布侧的规则套到本地产物目录：必需文件、四个判定词、为正且自洽的断言三元组、以及不允许带失败断言的 `merge-ready`。报告形态与证据命名只作为警告，因为纯文档轮次没有截图，检查器也无法知道是否跑过 harness。
- `scripts/verify-pr-check.mjs <dir>` 把发布侧的规则套到本地产物目录：必需文件、四个判定词、为正且自洽的断言三元组、以及不允许带失败断言的 `merge-ready`。报告形态与证据命名只作为警告，因为纯文档轮次没有截图，检查器也无法知道是否跑过 harness。带 `--context <snapshot>` 时期望 head 来自该宿主持有的文件，而不是产物里可写的 `context.json`；不可用快照是硬性 exit 1 而非警告：产物目录可被轮次刚执行的 PR 代码写入。
- `scripts/verify-pr-guard.mjs` 接管发布前守卫的两个半程。`--pr <n> --repo <owner>/<repo> --snapshot <file>` 抓取 head、PR 状态以及三个评论列表（带 `--paginate`），只有每次调用都成功才写文件。`--compare --start <a> --now <b>` 列出本轮 dirty 的全部原因并退出 1。
检查器在产物契约要求轮次写出的产物上新增三条规则。产物契约里三者都要点名：产物清单没要求的文件，轮次就不会产出，而对一个不存在的文件做检查是静默的。
@@ -97,6 +97,11 @@
- 所有脚本都不执行 PR 代码。
- 对于自带轮次快照的报告，在不同产物目录中准备后续轮次不会改变检查所用的提交号。可发布的轮次快照不包含 PR 标题或正文。
## 相关设计
- 容器边界与它的宿主机模型代理：
[verify-pr-local-model-proxy.zh-CN.md](verify-pr-local-model-proxy.zh-CN.md)。
## 未决问题
本范围内无。
scripts/tests/verify-pr-check.test.js
+74
View File
@@ -762,4 +762,78 @@ describe.skipIf(process.platform === 'win32')('check (CLI)', () => {
1,
);
});
// The artifact directory is writable by the PR code the round just ran, so
// its own context.json cannot pin the head. --context reads a copy the round
// never saw and must not fall back to the writable one.
it('pins the head from a host-held snapshot over the artifact copy', () => {
const dir = makeArtifactDir(validFiles(), ['01-ab-cells.png']);
writeFileSync(
join(dir, 'context.json'),
JSON.stringify({ headRefOid: 'e'.repeat(40) }),
);
const hostSnapshot = join(tmpdir(), `verify-pr-host-${Date.now()}.json`);
tmpDirs.push(hostSnapshot);
writeFileSync(hostSnapshot, JSON.stringify({ headRefOid: HEAD }));
const pinned = spawnSync(
process.execPath,
[SCRIPT, '--context', hostSnapshot, dir],
{ encoding: 'utf8' },
);
expect(pinned.status).toBe(0);
expect(pinned.stdout).toContain(`head source: ${hostSnapshot}`);
expect(pinned.stdout).not.toContain('does not name the verified head');
const artifactOnly = run(dir);
expect(artifactOnly.status).toBe(0);
expect(artifactOnly.stdout).toContain('does not name the verified head');
});
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, '{}');
for (const snapshot of [join(tmpdir(), 'verify-pr-no-such.json'), empty]) {
const result = spawnSync(
process.execPath,
[SCRIPT, '--context', snapshot, dir],
{ encoding: 'utf8' },
);
expect(result.status).toBe(1);
expect(result.stderr).toContain('--context');
expect(result.stdout).not.toContain('head source:');
}
});
it('exits 2 for an unknown option or a missing --context value', () => {
const dir = makeArtifactDir(validFiles(), ['01-ab-cells.png']);
expect(
spawnSync(process.execPath, [SCRIPT, '--bogus', dir], {
encoding: 'utf8',
}).status,
).toBe(2);
expect(
spawnSync(process.execPath, [SCRIPT, '--context'], {
encoding: 'utf8',
}).status,
).toBe(2);
// A forgotten --context turns the snapshot into a second positional:
// refuse instead of silently using the artifact's writable copy.
const hostSnapshot = join(tmpdir(), `verify-pr-host-${Date.now()}.json`);
tmpDirs.push(hostSnapshot);
writeFileSync(hostSnapshot, JSON.stringify({ headRefOid: HEAD }));
expect(
spawnSync(process.execPath, [SCRIPT, dir, hostSnapshot], {
encoding: 'utf8',
}).status,
).toBe(2);
});
});
scripts/tests/verify-pr-credential-scan.test.js
+702
View File
@@ -0,0 +1,702 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import { execFileSync, spawnSync } from 'node:child_process';
import {
chmodSync,
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 {
gitScanArgs,
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: '[email protected]',
GIT_COMMITTER_NAME: 't',
GIT_COMMITTER_EMAIL: '[email protected]',
};
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',
'.env~',
'tls.pem',
'id.key',
'auth.p12',
'tls.pfx',
'id_rsa',
'id_ed25519',
'id_ecdsa',
'id_dsa',
'credentials',
'credentials.json',
'.git-credentials',
'gha-creds-123.json',
'.npmrc',
'.netrc',
]) {
expect(isCredentialShaped(path), path).toBe(true);
}
for (const path of [
'README.md',
'package.json',
'keymap.ts',
'envelope.ts',
'environment.md',
'id_rsa.pub',
]) {
expect(isCredentialShaped(path), path).toBe(false);
}
});
});
describe('gitScanArgs', () => {
it('enumerates with depth-aware credential pathspecs', () => {
const args = gitScanArgs('/repo');
expect(args).toContain(':(glob)**/.env');
expect(args).toContain(':(glob)**/*.pem');
expect(args).toContain(':(glob)**/*.env~');
expect(args).toContain(':(glob)**/*.p12');
expect(args).toContain(':(glob)**/*.pfx');
expect(args).toContain(':(glob)**/id_rsa*');
expect(args).toContain(':(glob)**/id_ed25519*');
expect(args).toContain(':(glob)**/id_ecdsa*');
expect(args).toContain(':(glob)**/id_dsa*');
expect(args).toContain(':(glob)**/*credentials*');
expect(args).toContain(':(glob)**/gha-creds-*.json');
// 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(
[
'vault/.env',
'secrets/prod.env',
'.env~',
'.env.local',
'tls.pem',
'secrets/id.key',
'auth.p12',
'secrets/tls.pfx',
'id_rsa',
'id_rsa.pub',
'secrets/id_ed25519',
'secrets/id_ecdsa',
'secrets/id_dsa',
'backup.env~',
'secrets/credentials.json',
'.git-credentials',
'gha-creds-123.json',
'.npmrc',
'packages/foo/.npmrc',
'.netrc',
],
[
'.env*',
'*.env',
'*.env~',
'*.pem',
'*.key',
'*.p12',
'*.pfx',
'id_rsa*',
'id_ed25519*',
'id_ecdsa*',
'id_dsa*',
'*credentials*',
'.npmrc',
'.netrc',
'gha-creds-*.json',
'vault/',
],
);
expect(scanIgnoredCredentials(dir)).toEqual([
'.env.local',
'.env~',
'.git-credentials',
'.netrc',
'.npmrc',
'auth.p12',
'backup.env~',
'gha-creds-123.json',
'id_rsa',
'packages/foo/.npmrc',
'secrets/credentials.json',
'secrets/id.key',
'secrets/id_dsa',
'secrets/id_ecdsa',
'secrets/id_ed25519',
'secrets/prod.env',
'secrets/tls.pfx',
'tls.pem',
'vault/.env',
]);
});
it('refuses a remote URL that carries a credential', () => {
const dir = makeRepo(['src/app.ts'], ['node_modules/']);
execFileSync(
'git',
[
'config',
'--local',
'remote.origin.url',
'https://x-access-token:[email protected]/o/r.git',
],
{ cwd: dir, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir)).toThrow(
/credential-bearing git config/,
);
const result = run(dir);
expect(result.status).toBe(1);
expect(result.stderr).toContain('credential-bearing git config');
});
it('refuses a token used as the URL username and insteadOf credentials', () => {
const dir = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
[
'config',
'--local',
'remote.origin.url',
'https://[email protected]/o/r.git',
],
{ cwd: dir, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir)).toThrow(
/credential-bearing git config/,
);
const dir2 = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
[
'config',
'--local',
'url.https://x:[email protected]/.insteadOf',
'https://github.com/',
],
{ cwd: dir2, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir2)).toThrow(
/credential-bearing git config/,
);
});
// The same credential class as remote.*.url: a push URL is the natural
// place for a PAT, and submodule URLs and the Actions-written extraheader
// are equally readable from inside the tree with plain `git config`.
it('refuses pushurl, submodule URL and Authorization extraheader credentials', () => {
const cases = [
['remote.origin.pushurl', 'https://[email protected]/o/r.git'],
['submodule.lib.url', 'https://x:[email protected]/o/lib.git'],
[
'http.https://github.com/.extraheader',
'AUTHORIZATION: basic eC1hY2Nlc3MtdG9rZW46Z2hwX0ZBS0VQQVQ=',
],
[
'http.https://proxy.internal/.extraheader',
'Proxy-Authorization: Bearer ghp_FAKEPAT',
],
];
for (const [key, value] of cases) {
const dir = makeRepo(['src/app.ts'], []);
execFileSync('git', ['config', '--local', key, value], {
cwd: dir,
env: gitEnv,
});
expect(() => scanIgnoredCredentials(dir), key).toThrow(
/credential-bearing git config/,
);
const result = run(dir);
expect(result.status, key).toBe(1);
expect(result.stderr, key).toContain('credential-bearing git config');
}
});
it('keeps a non-Authorization extraheader out of the refusal', () => {
const dir = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
[
'config',
'--local',
'http.https://github.com/.extraheader',
'X-Request-Id: trace-123',
],
{ cwd: dir, env: gitEnv },
);
expect(scanIgnoredCredentials(dir)).toEqual([]);
});
// The guard inspects VALUES, not a key allowlist: any config value can
// carry a token URL, and the URL-bearing key families (rewrites, http
// sections) carry theirs in the key. Restoring a fixed key list must turn
// these red — a novel key is the witness that no list converges.
it('refuses credential-bearing values and keys outside any fixed list', () => {
const cases = [
// pushInsteadOf: the one-word sibling of insteadOf, credential in KEY.
[
'url.https://x:[email protected]/.pushInsteadOf',
'https://github.com/',
],
// http.proxy: credential in VALUE, a key no allowlist enumerated.
['http.proxy', 'http://user:[email protected]:3128'],
// A novel key entirely: only value inspection can see this.
['custom.mirror', 'https://[email protected]/o/r.git'],
// http.<url> section with the token in the KEY, benign value.
['http.https://[email protected]/.extraheader', 'X-Thing: 1'],
];
for (const [key, value] of cases) {
const dir = makeRepo(['src/app.ts'], []);
execFileSync('git', ['config', '--local', key, value], {
cwd: dir,
env: gitEnv,
});
expect(() => scanIgnoredCredentials(dir), key).toThrow(
/credential-bearing git config/,
);
const result = run(dir);
expect(result.status, key).toBe(1);
// The refusal names the key, never the value: when the credential sits
// in the value, the token must not appear in the output at all. Git
// lowercases the variable name but preserves the URL subsection's
// case, so only the last segment is folded here.
const reportedKey = key.replace(/\.[^.]+$/, (segment) =>
segment.toLowerCase(),
);
expect(result.stderr, key).toContain(reportedKey);
if (!key.toLowerCase().includes('ghp_fakepat')) {
expect(result.stderr, key).not.toContain('ghp_FAKEPAT');
}
}
// A proxy without userinfo stays clean: the widening added recognition,
// not a blanket refusal of the key.
const dir = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
['config', '--local', 'http.proxy', 'http://proxy.internal:3128'],
{ cwd: dir, env: gitEnv },
);
expect(scanIgnoredCredentials(dir)).toEqual([]);
});
// The credential-side twin of prepare's include.path filter bypass:
// `--local` alone names one file, while the `git config --get` PR code
// uses follows includes — an included credential must be refused too.
it('refuses credentials that arrive through include.path', () => {
const cases = [
'[remote "origin"]\n\turl = https://[email protected]/o/r.git\n',
'[remote "origin"]\n\tpushurl = https://[email protected]/o/r.git\n',
'[http "https://github.com/"]\n\textraheader = AUTHORIZATION: basic eC1hY2Nlc3MtdG9rZW46Z2hwX0ZBS0VQQVQ=\n',
];
for (const content of cases) {
const dir = makeRepo(['src/app.ts'], ['tmp/']);
mkdirSync(join(dir, 'tmp'), { recursive: true });
writeFileSync(join(dir, 'tmp', 'creds.inc'), content);
execFileSync(
'git',
['config', '--local', 'include.path', '../tmp/creds.inc'],
{ cwd: dir, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir), content).toThrow(
/credential-bearing git config/,
);
const result = run(dir);
expect(result.status, content).toBe(1);
expect(result.stderr, content).toContain('credential-bearing git config');
}
// A credential-free include is zero collateral: same shape, benign URL.
const dir = makeRepo(['src/app.ts'], ['tmp/']);
mkdirSync(join(dir, 'tmp'), { recursive: true });
writeFileSync(
join(dir, 'tmp', 'creds.inc'),
'[remote "origin"]\n\turl = https://github.com/o/r.git\n',
);
execFileSync(
'git',
['config', '--local', 'include.path', '../tmp/creds.inc'],
{ cwd: dir, env: gitEnv },
);
expect(scanIgnoredCredentials(dir)).toEqual([]);
});
// Ordinary `git config --get` reads .git/config.worktree when the
// extension is enabled; --local alone would report a credential there as
// clean while PR code reads it. The scan must enumerate the enabled scope.
it('refuses credentials in the enabled worktree config scope', () => {
const dir = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
['config', '--local', 'extensions.worktreeConfig', 'true'],
{ cwd: dir, env: gitEnv },
);
execFileSync(
'git',
[
'config',
'--worktree',
'http.https://github.com/.extraheader',
'Authorization: Bearer FAKE_WORKTREE',
],
{ cwd: dir, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir)).toThrow(
/credential-bearing git config/,
);
expect(run(dir).status).toBe(1);
execFileSync(
'git',
[
'config',
'--worktree',
'--unset',
'http.https://github.com/.extraheader',
],
{ cwd: dir, env: gitEnv },
);
execFileSync(
'git',
[
'config',
'--worktree',
'remote.origin.url',
'https://[email protected]/repo.git',
],
{ cwd: dir, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir)).toThrow(
/credential-bearing git config/,
);
expect(run(dir).status).toBe(1);
execFileSync(
'git',
['config', '--worktree', '--unset', 'remote.origin.url'],
{ cwd: dir, env: gitEnv },
);
// Benign worktree config stays clean.
execFileSync(
'git',
['config', '--worktree', 'core.logAllRefUpdates', 'true'],
{ cwd: dir, env: gitEnv },
);
expect(scanIgnoredCredentials(dir)).toEqual([]);
// With the extension disabled git itself ignores the worktree file, and
// so does the scan: the same planted credential reads as clean.
execFileSync(
'git',
[
'config',
'--worktree',
'remote.origin.url',
'https://[email protected]/repo.git',
],
{ cwd: dir, env: gitEnv },
);
execFileSync(
'git',
['config', '--local', '--unset', 'extensions.worktreeConfig'],
{ cwd: dir, env: gitEnv },
);
expect(scanIgnoredCredentials(dir)).toEqual([]);
expect(run(dir).status).toBe(0);
});
// Enabling the extension does not require config.worktree to exist: an
// absent optional file is an empty scope, not a refusal (explicit
// `--worktree --list` exits 128 on it). Unreadable or malformed files
// must still refuse — the tolerance is for absence only.
it('treats an absent optional worktree config as an empty scope', () => {
const dir = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
['config', '--local', 'extensions.worktreeConfig', 'true'],
{ cwd: dir, env: gitEnv },
);
expect(scanIgnoredCredentials(dir)).toEqual([]);
expect(run(dir).status).toBe(0);
// An existing empty file is the same empty scope.
writeFileSync(join(dir, '.git', 'config.worktree'), '');
expect(scanIgnoredCredentials(dir)).toEqual([]);
expect(run(dir).status).toBe(0);
// A malformed file refuses on both paths.
writeFileSync(join(dir, '.git', 'config.worktree'), '[broken\n');
expect(() => scanIgnoredCredentials(dir)).toThrow(
/could not (read local git config|list nested repositories)/,
);
expect(run(dir).status).toBe(1);
});
// POSIX permissions only: on Windows chmod 000 does not block reads.
it.skipIf(process.platform === 'win32')(
'refuses an unreadable worktree config instead of treating it as absent',
() => {
const dir = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
['config', '--local', 'extensions.worktreeConfig', 'true'],
{ cwd: dir, env: gitEnv },
);
writeFileSync(join(dir, '.git', 'config.worktree'), '');
chmodSync(join(dir, '.git', 'config.worktree'), 0o000);
try {
expect(() => scanIgnoredCredentials(dir)).toThrow(
/could not (read local git config|list nested repositories)/,
);
expect(run(dir).status).toBe(1);
} finally {
chmodSync(join(dir, '.git', 'config.worktree'), 0o644);
}
},
);
it('keeps ssh URLs, the bare git username and public keys out of the refusal', () => {
const dir = makeRepo(['id_rsa.pub'], ['id_rsa*']);
execFileSync(
'git',
[
'config',
'--local',
'remote.origin.url',
'ssh://[email protected]/o/r.git',
],
{ cwd: dir, env: gitEnv },
);
expect(scanIgnoredCredentials(dir)).toEqual([]);
// The conventional account name carries no secret on http(s) either;
// refusing it would disagree with ssh://git@ on the same non-secret.
const dir2 = makeRepo(['src/app.ts'], []);
execFileSync(
'git',
[
'config',
'--local',
'remote.origin.url',
'https://[email protected]/o/r.git',
],
{ cwd: dir2, env: gitEnv },
);
expect(scanIgnoredCredentials(dir2)).toEqual([]);
expect(run(dir2).status).toBe(0);
// Account names are case-insensitive: the uppercase spellings carry
// the same non-secret and must not re-introduce the false refusal.
for (const spelling of ['GIT', 'Git']) {
execFileSync(
'git',
[
'config',
'--local',
'remote.origin.url',
`https://${spelling}@github.com/o/r.git`,
],
{ cwd: dir2, env: gitEnv },
);
expect(scanIgnoredCredentials(dir2), spelling).toEqual([]);
expect(run(dir2).status, spelling).toBe(0);
}
// A password after the same username is still a credential.
execFileSync(
'git',
[
'config',
'--local',
'remote.origin.url',
'https://git:[email protected]/o/r.git',
],
{ cwd: dir2, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir2)).toThrow(
/credential-bearing git config/,
);
});
it('does not refuse a vendored real git repository under node_modules', () => {
const dir = makeRepo(['node_modules/pkg/index.js'], ['node_modules/']);
const nested = join(dir, 'node_modules', 'pkg');
execFileSync('git', ['init', '-q', '.'], { cwd: nested, env: gitEnv });
execFileSync('git', ['add', '.'], { cwd: nested, env: gitEnv });
execFileSync('git', ['commit', '-q', '-m', 'dep'], {
cwd: nested,
env: gitEnv,
});
expect(scanIgnoredCredentials(dir)).toEqual([]);
});
it('does not refuse an installed tree or build output at any depth', () => {
const dir = makeRepo(
[
'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', () => {
// The dependency files MATCH a credential pathspec (*credentials*): with
// the pathspecs dropped, this listing overflows the 64 KiB ceiling, so
// the test actually exercises the restriction it names.
const files = Array.from(
{ length: 3000 },
(_, index) =>
`packages/foo/node_modules/pkg/pkg-credentials-${index}.json`,
);
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('refuses a nested repository instead of silently skipping it', () => {
const dir = makeRepo(['tmp/pr-clone/.env'], ['tmp/']);
execFileSync('git', ['init', '-q', '.'], {
cwd: join(dir, 'tmp', 'pr-clone'),
env: gitEnv,
});
expect(() => scanIgnoredCredentials(dir)).toThrow(
/nested git repository under tmp\/pr-clone/,
);
const result = run(dir);
expect(result.status).toBe(1);
expect(result.stderr).toContain('nested git repository');
});
// A round's own `git clone` leaves an UNTRACKED, non-ignored clone: the
// ignored-only enumeration never saw it, and git does not descend into it
// for the credential file scan either.
it('refuses an untracked non-ignored nested clone', () => {
const dir = makeRepo(['src/app.ts'], []);
const nested = join(dir, 'tools', 'clone');
mkdirSync(nested, { recursive: true });
execFileSync('git', ['init', '-q', '.'], { cwd: nested, env: gitEnv });
writeFileSync(join(nested, '.env'), 'OPENAI_API_KEY=sk-from-clone\n');
expect(() => scanIgnoredCredentials(dir)).toThrow(
/nested git repository under tools\/clone/,
);
const result = run(dir);
expect(result.status).toBe(1);
expect(result.stderr).toContain('nested git repository');
});
// A populated submodule is a gitlink whose working tree git will not
// descend into; an unpopulated one has no tree to read (its URL is covered
// by the config value scan instead).
it('refuses a populated submodule but not an unpopulated gitlink', () => {
const dir = makeRepo(['src/app.ts'], []);
const nested = join(dir, 'vendor', 'up');
mkdirSync(nested, { recursive: true });
execFileSync('git', ['init', '-q', '.'], { cwd: nested, env: gitEnv });
writeFileSync(join(nested, '.env'), 'OPENAI_API_KEY=sk-from-submodule\n');
execFileSync('git', ['add', '.'], { cwd: nested, env: gitEnv });
execFileSync('git', ['commit', '-q', '-m', 'sub'], {
cwd: nested,
env: gitEnv,
});
const sha = execFileSync('git', ['rev-parse', 'HEAD'], {
cwd: nested,
env: gitEnv,
encoding: 'utf8',
}).trim();
execFileSync(
'git',
['update-index', '--add', '--cacheinfo', `160000,${sha},vendor/up`],
{ cwd: dir, env: gitEnv },
);
expect(() => scanIgnoredCredentials(dir)).toThrow(
/nested git repository under vendor\/up/,
);
expect(run(dir).status).toBe(1);
// Unpopulated: the gitlink stays, the working tree goes.
rmSync(nested, { recursive: true, force: true });
expect(scanIgnoredCredentials(dir)).toEqual([]);
expect(run(dir).status).toBe(0);
});
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).toMatch(
/could not list (nested repositories|ignored files)/,
);
});
});
scripts/tests/verify-pr-model-proxy.test.js
+906
View File
@@ -0,0 +1,906 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
import { spawn, spawnSync } from 'node:child_process';
import { once } from 'node:events';
import { readFileSync } from 'node:fs';
import http from 'node:http';
import { fileURLToPath } from 'node:url';
import { gzipSync } from 'node:zlib';
import { afterEach, describe, expect, it, vi } from 'vitest';
import {
DEFAULT_IDLE_TIMEOUT_MS,
createModelProxy,
formatLocalBaseUrl,
normalizeBasePath,
parseArgs,
} from '../../scripts/verify-pr-model-proxy.mjs';
const SCRIPT = fileURLToPath(
new URL('../../scripts/verify-pr-model-proxy.mjs', import.meta.url),
);
const TOKEN = 'test-proxy-token';
const NONCE = 'test-nonce';
const REAL_KEY = 'real-upstream-key';
let cleanups = [];
afterEach(async () => {
for (const cleanup of cleanups) await cleanup();
cleanups = [];
});
function listen(server) {
return new Promise((resolve, reject) => {
server.once('error', reject);
server.listen(0, '127.0.0.1', () => {
const { port } = server.address();
cleanups.push(
() =>
new Promise((done) => {
server.closeAllConnections();
server.close(done);
}),
);
resolve(port);
});
});
}
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.
*/
async function startUpstream({
status = 200,
chunks = ['data: ok\n\n'],
delayMs = 0,
chunkDelayMs = 0,
resetAfterFirstChunk = false,
respectBackpressure = false,
gzipBody = false,
headersOnly = false,
} = {}) {
const requests = [];
const events = [];
const server = http.createServer((req, res) => {
const body = [];
req.on('data', (chunk) => body.push(chunk));
req.on('end', async () => {
requests.push({
method: req.method,
url: req.url,
headers: req.headers,
authorization: req.headers.authorization,
body: Buffer.concat(body).toString('utf8'),
});
res.on('finish', () => events.push('upstream-finish'));
if (delayMs) await sleep(delayMs);
if (res.destroyed) {
events.push('aborted-before-headers');
return;
}
if (gzipBody) {
// The gzip header is what the relay must drop: undici hands the proxy
// decoded bytes, so forwarding `content-encoding: gzip` makes the
// client gunzip plain text.
res.writeHead(status, {
'content-type': 'text/event-stream',
'content-encoding': 'gzip',
'content-length': String(
gzipSync(Buffer.from(chunks.join(''))).length,
),
});
events.push('write-head');
requests.at(-1).sentContentEncoding = 'gzip';
res.end(gzipSync(Buffer.from(chunks.join(''))));
return;
}
res.writeHead(status, { 'content-type': 'text/event-stream' });
events.push('write-head');
if (headersOnly) {
// Flush so the proxy's fetch resolves on headers; the test is the
// first-body idle window, not the pre-header one.
res.flushHeaders();
return;
}
for (const [index, chunk] of chunks.entries()) {
if (chunkDelayMs) await sleep(chunkDelayMs);
if (res.destroyed) {
events.push('aborted-mid-body');
return;
}
// Respecting backpressure keeps the stream alive while the proxy's
// downstream is paused; without it the mock buffers the whole body in
// its own process and the pause never propagates.
if (!res.write(chunk) && respectBackpressure) {
await once(res, 'drain');
}
if (resetAfterFirstChunk && index === 0) {
// Register on the socket, not unconditionally: the test asserts
// the upstream really reset after headers.
res.on('close', () => events.push('upstream-reset'));
await sleep(50);
res.destroy();
return;
}
}
res.end();
});
});
const port = await listen(server);
return { port, requests, events };
}
async function startProxy(upstream, overrides = {}, basePath = '/v1') {
const server = createModelProxy({
baseUrl: `http://127.0.0.1:${upstream.port}${basePath}`,
apiKey: REAL_KEY,
token: TOKEN,
nonce: NONCE,
...overrides,
});
const port = await listen(server);
return `http://127.0.0.1:${port}`;
}
const chatRequest = (body = '{"model":"m"}') => ({
method: 'POST',
headers: {
authorization: `Bearer ${TOKEN}`,
'content-type': 'application/json',
},
body,
});
describe('verify-pr-model-proxy', () => {
it('health endpoint echoes the nonce without touching the upstream', async () => {
const upstream = await startUpstream();
const proxy = await startProxy(upstream);
const res = await fetch(`${proxy}/__health`);
expect(res.status).toBe(200);
expect(await res.text()).toBe(NONCE);
expect(upstream.requests).toHaveLength(0);
});
it('rejects a missing or wrong bearer token before the upstream', async () => {
const upstream = await startUpstream();
const proxy = await startProxy(upstream);
for (const authorization of [undefined, 'Bearer wrong']) {
const res = await fetch(`${proxy}/chat/completions`, {
method: 'POST',
headers: authorization ? { authorization } : {},
body: '{}',
});
expect(res.status).toBe(401);
}
expect(upstream.requests).toHaveLength(0);
});
it('allows only POST /chat/completions', async () => {
const upstream = await startUpstream();
const proxy = await startProxy(upstream);
const get = await fetch(`${proxy}/v1/models`, {
headers: { authorization: `Bearer ${TOKEN}` },
});
expect(get.status).toBe(403);
const post = await fetch(`${proxy}/v1/models`, chatRequest());
expect(post.status).toBe(403);
// Exact path, not a suffix: this must not reach the upstream as
// /v1/admin/chat/completions.
const admin = await fetch(`${proxy}/admin/chat/completions`, chatRequest());
expect(admin.status).toBe(403);
// The method half matters on its own: a PUT to the allowed path would
// otherwise spend the real key upstream.
const put = await fetch(`${proxy}/chat/completions`, {
...chatRequest(),
method: 'PUT',
});
expect(put.status).toBe(403);
expect(upstream.requests).toHaveLength(0);
});
it('relays chat completions with the real key and the base-path prefix', async () => {
const upstream = await startUpstream({
chunks: ['data: a\n\n', 'data: b\n\n'],
});
const proxy = await startProxy(upstream);
const res = await fetch(
`${proxy}/chat/completions?stream=true`,
chatRequest('{"model":"m","stream":true}'),
);
expect(res.status).toBe(200);
expect(await res.text()).toBe('data: a\n\ndata: b\n\n');
expect(upstream.requests).toHaveLength(1);
const seen = upstream.requests[0];
expect(seen.method).toBe('POST');
// The proxy's base URL carries /v1; the client path does not repeat it.
expect(seen.url).toBe('/v1/chat/completions?stream=true');
expect(seen.authorization).toBe(`Bearer ${REAL_KEY}`);
expect(seen.body).toBe('{"model":"m","stream":true}');
});
it('does not double-prefix a client path that already carries the base path', async () => {
const upstream = await startUpstream();
const proxy = await startProxy(upstream);
const res = await fetch(`${proxy}/v1/chat/completions`, chatRequest());
expect(res.status).toBe(200);
expect(upstream.requests[0].url).toBe('/v1/chat/completions');
});
it('matches the exact path even without a base-path prefix', async () => {
const upstream = await startUpstream();
const proxy = await startProxy(upstream, {}, '');
const admin = await fetch(`${proxy}/admin/chat/completions`, chatRequest());
expect(admin.status).toBe(403);
expect(upstream.requests).toHaveLength(0);
const ok = await fetch(`${proxy}/chat/completions`, chatRequest());
expect(ok.status).toBe(200);
expect(upstream.requests[0].url).toBe('/chat/completions');
});
it('strips the upstream content-encoding instead of forwarding it', async () => {
const upstream = await startUpstream({
gzipBody: true,
chunks: ['data: gzip-ok\n\n'],
});
const proxy = await startProxy(upstream);
const res = await fetch(`${proxy}/chat/completions`, chatRequest());
expect(res.status).toBe(200);
// The fixture must really have compressed; otherwise this test would
// pass against an uncompressed upstream and pin nothing.
expect(upstream.requests[0].sentContentEncoding).toBe('gzip');
expect(res.headers.get('content-type')).toBe('text/event-stream');
// undici already decoded the body; forwarding the header would make the
// client gunzip plain text.
expect(res.headers.get('content-encoding')).toBeNull();
expect(await res.text()).toBe('data: gzip-ok\n\n');
});
it('strips hop-by-hop headers and relays a chunked request body', async () => {
const upstream = await startUpstream();
const proxy = await startProxy(upstream);
const { port } = new URL(proxy);
const status = await new Promise((resolve, reject) => {
const req = http.request(
{
host: '127.0.0.1',
port,
path: '/chat/completions',
method: 'POST',
headers: {
authorization: `Bearer ${TOKEN}`,
connection: ' x-client-hop , keep-alive ',
'x-client-hop': 'client-secret',
'x-keep-me': 'kept',
'proxy-authorization': 'Basic client-proxy-secret',
'transfer-encoding': 'chunked',
expect: '100-continue',
'content-type': 'application/json',
},
},
(res) => {
res.resume();
res.on('end', () => resolve(res.statusCode));
},
);
req.on('error', reject);
req.write('{"model":');
req.end('"m"}');
});
expect(status).toBe(200);
const seen = upstream.requests[0];
expect(seen.body).toBe('{"model":"m"}');
expect(seen.authorization).toBe(`Bearer ${REAL_KEY}`);
expect(seen.headers['proxy-authorization']).toBeUndefined();
expect(seen.headers.expect).toBeUndefined();
// The Connection-nominated name must not survive under its own spelling;
// this is the assertion that goes red when the nominated-strip loop is
// removed (the fixed-set strip alone would still drop `connection`).
expect(seen.headers['x-client-hop']).toBeUndefined();
// Passthrough in the positive direction: a custom header NOT nominated
// by Connection, and the content type, must survive — the stripping is
// selective, not a wholesale header drop.
expect(seen.headers['x-keep-me']).toBe('kept');
expect(seen.headers['content-type']).toBe('application/json');
});
it('propagates an upstream error status and body', async () => {
const upstream = await startUpstream({
status: 429,
chunks: ['slow down'],
});
const proxy = await startProxy(upstream);
const res = await fetch(`${proxy}/chat/completions`, chatRequest());
expect(res.status).toBe(429);
expect(await res.text()).toBe('slow down');
});
it('answers 504 when the upstream stalls past the idle window', async () => {
const server = http.createServer(() => {
// Never respond: the watchdog must fire.
});
const port = await listen(server);
const proxy = await startProxy({ port }, { idleTimeoutMs: 50 });
const res = await fetch(`${proxy}/chat/completions`, chatRequest());
expect(res.status).toBe(504);
});
it('answers 502 and stays alive when the upstream is unreachable', async () => {
// Bind an ephemeral port and close it: the dial must really be refused.
// Port 9 would never dial at all — it is on undici's blocked-port list,
// so fetch rejects with `bad port` before a socket opens and the
// connect-refused path this test names would go unexercised.
const dead = http.createServer(() => {});
const deadPort = await listen(dead);
await new Promise((resolve) => {
dead.closeAllConnections();
dead.close(resolve);
});
const logged = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
const proxy = await startProxy({ port: deadPort }, {}, '');
const res = await fetch(`${proxy}/chat/completions`, chatRequest());
expect(res.status).toBe(502);
const failure = logged.mock.calls
.map((call) => call.find((arg) => arg instanceof Error))
.find(Boolean);
expect(failure?.cause?.code).toBe('ECONNREFUSED');
const health = await fetch(`${proxy}/__health`);
expect(health.status).toBe(200);
} finally {
logged.mockRestore();
}
});
// A slow request UPLOAD is activity: fetch resolves only on the upstream
// headers, so a window meant for stream silence must not tear a large
// prompt down mid-upload.
it('keeps a slow request upload alive past the idle window', async () => {
const upstream = await startUpstream();
const proxy = await startProxy(upstream, { idleTimeoutMs: 300 });
const { port } = new URL(proxy);
const status = await new Promise((resolve, reject) => {
const req = http.request(
{
host: '127.0.0.1',
port,
path: '/chat/completions',
method: 'POST',
headers: {
authorization: `Bearer ${TOKEN}`,
'content-type': 'application/json',
},
},
(res) => {
res.resume();
res.on('end', () => resolve(res.statusCode));
},
);
req.on('error', reject);
// Three chunks 150 ms apart: total 450 ms outlives the 300 ms window,
// and no single gap exceeds it — only the per-chunk re-arm survives.
req.write('{"model"');
setTimeout(() => req.write(':"m"'), 150);
setTimeout(() => req.write(',"stream":false}'), 300);
setTimeout(() => req.end(), 450);
});
expect(status).toBe(200);
expect(upstream.requests).toHaveLength(1);
expect(upstream.requests[0].body).toBe('{"model":"m","stream":false}');
});
it('aborts when headers arrive but no body follows', async () => {
const upstream = await startUpstream({ headersOnly: true });
const proxy = await startProxy(upstream, { idleTimeoutMs: 100 });
// The upstream flushes headers, then stalls: the first-body idle window
// must tear the stream down instead of letting it hang.
await expect(
fetch(`${proxy}/chat/completions`, chatRequest()),
).rejects.toThrow();
const health = await fetch(`${proxy}/__health`);
expect(health.status).toBe(200);
});
it('refuses to start without the full configuration', () => {
expect(() =>
createModelProxy({
baseUrl: 'http://x',
apiKey: '',
token: 't',
nonce: 'n',
}),
).toThrow('required');
});
it('aborts the upstream when the client disconnects before headers', async () => {
const upstream = await startUpstream({ delayMs: 100 });
const proxy = await startProxy(upstream);
const { port } = new URL(proxy);
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
// abort registered only after the fetch, it writes headers and the whole
// completion to a socket nobody reads.
expect(upstream.events).toContain('aborted-before-headers');
expect(upstream.events).not.toContain('write-head');
});
it('aborts the upstream when the client disconnects after headers', async () => {
const upstream = await startUpstream({
chunks: ['data: first\n\n', 'data: second\n\n', 'data: third\n\n'],
chunkDelayMs: 100,
});
const proxy = await startProxy(upstream, { idleTimeoutMs: 5000 });
const { port } = new URL(proxy);
const req = http.request(
{
host: '127.0.0.1',
port,
path: '/chat/completions',
method: 'POST',
headers: { authorization: `Bearer ${TOKEN}` },
},
(res) => {
res.once('data', () => {
res.destroy();
req.destroy();
});
res.on('error', () => {});
},
);
req.on('error', () => {});
req.end('{}');
await waitFor(() => upstream.events.includes('aborted-mid-body'), 3000);
expect(upstream.events).toContain('aborted-mid-body');
});
it('does not abort a healthy stream while the downstream is paused', async () => {
const chunks = Array.from(
{ length: 128 },
(_, index) => `${String(index).padStart(3, '0')}:${'x'.repeat(65_520)}\n`,
);
chunks.push('TAIL-MARKER');
const upstream = await startUpstream({
chunks,
respectBackpressure: true,
});
// The pause (800 ms) must exceed the window (300 ms) or the pause guard
// is never exercised; the margin keeps a loaded runner from flipping it.
const proxy = await startProxy(upstream, { idleTimeoutMs: 300 });
const { port } = new URL(proxy);
const body = await new Promise((resolve, reject) => {
const req = http.request(
{
host: '127.0.0.1',
port,
path: '/chat/completions',
method: 'POST',
headers: { authorization: `Bearer ${TOKEN}` },
},
(res) => {
const parts = [];
res.on('data', (chunk) => parts.push(chunk));
res.on('end', () => resolve(Buffer.concat(parts).toString('utf8')));
res.on('error', reject);
// Pause past the idle window while the upstream keeps producing:
// a slow but live client must not be read as upstream idleness.
res.pause();
setTimeout(() => res.resume(), 800);
},
);
req.on('error', reject);
req.end('{}');
});
expect(body).toBe(chunks.join(''));
});
it('keeps a slow-but-healthy stream alive past the window', async () => {
const upstream = await startUpstream({
chunks: [
'data: 1\n\n',
'data: 2\n\n',
'data: 3\n\n',
'data: 4\n\n',
'data: 5\n\n',
],
chunkDelayMs: 80,
});
const proxy = await startProxy(upstream, { idleTimeoutMs: 300 });
const res = await fetch(`${proxy}/chat/completions`, chatRequest());
expect(res.status).toBe(200);
// Total delivery (400 ms) outlives the 300 ms window while every gap
// (80 ms) stays well inside it: only the per-chunk re-arm keeps this
// from being cut, and the 220 ms margin survives a loaded runner.
expect(await res.text()).toBe(
'data: 1\n\ndata: 2\n\ndata: 3\n\ndata: 4\n\ndata: 5\n\n',
);
});
it('survives an upstream reset after headers', async () => {
const upstream = await startUpstream({
chunks: ['data: partial\n\n'],
resetAfterFirstChunk: true,
});
const child = spawn(process.execPath, [SCRIPT], {
env: {
PATH: process.env.PATH,
REVIEW_OPENAI_BASE_URL: `http://127.0.0.1:${upstream.port}/v1`,
REVIEW_OPENAI_API_KEY: REAL_KEY,
},
// The reset is the point of this test; the proxy's diagnostic for it
// would only look like a failure in the suite output.
stdio: ['ignore', 'pipe', 'ignore'],
});
cleanups.push(async () => {
child.kill('SIGTERM');
});
const line = await new Promise((resolve, reject) => {
let buffer = '';
child.stdout.on('data', (data) => {
buffer += data;
if (buffer.includes('\n')) resolve(buffer.split('\n')[0]);
});
child.on('error', reject);
child.on('exit', (code) =>
reject(new Error(`proxy exited before printing: ${code}`)),
);
});
const info = JSON.parse(line);
// The CLI-generated token is the container's only credential.
expect(info.token).toMatch(/^[0-9a-f]{48}$/);
const res = await fetch(`http://127.0.0.1:${info.port}/chat/completions`, {
method: 'POST',
headers: { authorization: `Bearer ${info.token}` },
body: '{}',
}).catch(() => null);
if (res) await res.text().catch(() => {});
await waitFor(() => upstream.events.includes('upstream-reset'), 2000);
await sleep(200);
// The client may see a torn stream; the proxy process must not die on it.
expect(child.exitCode).toBeNull();
expect(upstream.events).toContain('upstream-reset');
const health = await fetch(`http://127.0.0.1:${info.port}/__health`);
expect(health.status).toBe(200);
});
});
describe('proxy defaults and base url formatting', () => {
it('keeps the idle window at the client default', () => {
// Read the core constant from its source instead of restating it: two
// hardcoded copies can drift with the suite green. A tighter proxy would
// cut streams its own client still tolerates.
const constants = readFileSync(
fileURLToPath(
new URL(
'../../packages/core/src/core/openaiContentGenerator/constants.ts',
import.meta.url,
),
),
'utf8',
);
const match = /DEFAULT_STREAM_IDLE_TIMEOUT_MS\s*=\s*(\d+)/.exec(constants);
expect(match).not.toBeNull();
expect(DEFAULT_IDLE_TIMEOUT_MS).toBe(Number(match[1]));
});
it('brackets an IPv6 listen host', () => {
expect(formatLocalBaseUrl('::1', 8080, '/v1')).toBe('http://[::1]:8080/v1');
});
it('leaves an IPv4 host alone and tolerates an empty base path', () => {
expect(formatLocalBaseUrl('192.0.2.10', 8080)).toBe(
'http://192.0.2.10:8080',
);
expect(formatLocalBaseUrl('192.0.2.10', 8080, '/v1')).toBe(
'http://192.0.2.10:8080/v1',
);
});
it('normalizes base paths', () => {
expect(normalizeBasePath('/v1/')).toBe('/v1');
expect(normalizeBasePath('/v1///')).toBe('/v1');
expect(normalizeBasePath('/')).toBe('');
expect(normalizeBasePath('')).toBe('');
});
});
describe('parseArgs', () => {
it('defaults to loopback with an ephemeral port', () => {
expect(parseArgs([])).toEqual({ listen: '127.0.0.1' });
});
it('parses --listen', () => {
expect(parseArgs(['--listen', '172.17.0.1'])).toEqual({
listen: '172.17.0.1',
});
});
it('rejects --port, which the recipe does not use', () => {
expect(() => parseArgs(['--port', '8080'])).toThrow('unknown argument');
});
it('rejects unknown arguments and missing values', () => {
expect(() => parseArgs(['--docker'])).toThrow('unknown argument');
expect(() => parseArgs(['--listen'])).toThrow('requires a value');
});
// server.listen(port, '') binds EVERY interface — past the bridge a
// key-holding proxy is meant to stay on — and a flag-looking value is a
// missing value, not a host.
it('rejects an empty or flag-looking --listen value', () => {
expect(() => parseArgs(['--listen', ''])).toThrow('requires a value');
expect(() => parseArgs(['--listen', '--other'])).toThrow(
'requires a value',
);
});
});
describe('CLI', () => {
it('exits 2 when the upstream configuration is missing', () => {
const res = spawnSync(process.execPath, [SCRIPT], {
env: { PATH: process.env.PATH },
encoding: 'utf8',
});
expect(res.status).toBe(2);
expect(res.stderr).toContain('REVIEW_OPENAI_BASE_URL');
});
it('prints the listen coordinates as JSON and honors the token override', async () => {
const upstream = await startUpstream();
const child = spawn(process.execPath, [SCRIPT, '--listen', '0.0.0.0'], {
env: {
PATH: process.env.PATH,
REVIEW_OPENAI_BASE_URL: `http://127.0.0.1:${upstream.port}/v1`,
REVIEW_OPENAI_API_KEY: REAL_KEY,
QWEN_VERIFY_PROXY_TOKEN: 'fixed-token',
},
stdio: ['ignore', 'pipe', 'inherit'],
});
cleanups.push(async () => {
child.kill('SIGTERM');
});
const line = await new Promise((resolve, reject) => {
let buffer = '';
child.stdout.on('data', (data) => {
buffer += data;
if (buffer.includes('\n')) resolve(buffer.split('\n')[0]);
});
child.on('error', reject);
child.on('exit', (code) =>
reject(new Error(`proxy exited before printing: ${code}`)),
);
});
const info = JSON.parse(line);
expect(info.token).toBe('fixed-token');
// The listen value must reach the printed URL; hard-coding loopback here
// would hand the container an address it cannot dial.
expect(info.baseUrl).toBe(`http://0.0.0.0:${info.port}/v1`);
expect(info.nonce).toMatch(/^[0-9a-f]{48}$/);
// The printed baseUrl must not leak the real key, and the health
// endpoint must echo the printed nonce.
expect(line).not.toContain(REAL_KEY);
const health = await fetch(`http://127.0.0.1:${info.port}/__health`);
expect(await health.text()).toBe(info.nonce);
});
});
/**
* The recipe is the only place where the trust delegation lives, and a
* pleasing-looking revert (relative path, attestation on the run line) is
* invisible to the runtime tests. Pin the load-bearing lines.
*/
describe('local container recipe pins', () => {
const skill = readFileSync(
fileURLToPath(
new URL('../../.qwen/skills/verify-pr/SKILL.md', import.meta.url),
),
'utf8',
);
it('runs every round script from the trusted checkout', () => {
expect(skill).toContain(
'cd <pr-clone> && node <trusted-clone>/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 <trusted-clone>/scripts/verify-pr-model-proxy.mjs',
);
expect(skill).toContain('<trusted-clone>/scripts/verify-pr-guard.mjs');
expect(skill).toContain(
'`<clone>` below is the trusted checkout the watch scripts run from',
);
// Enumerate the checker call sites: the preamble also names the trusted
// path, so a single positive assertion cannot detect step 7 reverting to
// <pr-clone>/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('<trusted-clone>');
expect(skill).not.toMatch(
/node <pr-clone>\/scripts\/verify-pr-(prepare|check|model-proxy|guard)/,
);
expect(skill).not.toMatch(
/node scripts\/verify-pr-(prepare|check|model-proxy|guard)\.mjs/,
);
});
it('base-pins `.qwen` and states that root AGENTS/QWEN.md still come from the PR', () => {
expect(skill).toContain('git -C <pr-clone> archive "$BASE_OID" -- .qwen');
expect(skill).toContain(
'-v ~/.cache/qwen-verify/pr<n>/qwen-base/.qwen:/work/.qwen:ro',
);
expect(skill).toContain('`AGENTS.md`/`QWEN.md` do come from the PR clone');
});
it('mounts the round tree writable and the metadata snapshot read-only', () => {
expect(skill).toContain('--user "$(id -u):$(id -g)"');
expect(skill).toContain('-v <pr-clone>:/work');
// toContain alone is satisfied by `:/work:ro` — the exact mutation this
// pin exists to catch — so the read-only spelling is excluded outright.
expect(skill).not.toContain('-v <pr-clone>:/work:ro');
expect(skill).toContain('-v <trusted-clone>/scripts:/trusted-scripts:ro');
expect(skill).toContain('-v ~/.cache/qwen-verify/pr<n>:/verify-context:ro');
expect(skill).toContain('QWEN_VERIFY_CONTEXT=/verify-context/pr.json');
});
it('reads both credentials from files, including without a TTY', () => {
expect(skill).toContain('if [ -t 0 ]; then');
expect(skill).toContain('REVIEW_OPENAI_KEY_FILE');
// `read` returns 1 at EOF even when it assigned the variable: a key file
// without a trailing newline must be judged by the variable, not the
// exit status, or the documented agent path aborts with the opposite
// diagnosis.
expect(skill).toContain('[ -n "$REVIEW_OPENAI_API_KEY" ]');
expect(skill).not.toContain('REVIEW_OPENAI_API_KEY=<real-key>');
// The proxy owns its copy: later diagnostics must not see the key.
expect(skill).toContain('unset 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 {1..100}; do');
expect(skill).not.toContain('$(seq 1 100)');
expect(skill).toContain('test -n "$QWEN_VERIFY_PROXY_TOKEN"');
// The nonce, not just the JSON shape, is checked before use.
expect(skill).toContain('/__health');
expect(skill).toContain('fetch(`http://${u.hostname}:${u.port}/__health`)');
expect(skill).toContain('process.exit(t===j.nonce?0:1)');
// Each step is its own shell: the token is re-derived where it is used.
const dockerRun =
skill.match(/export QWEN_VERIFY_PROXY_TOKEN[\s\S]*?```/)?.[0] ?? '';
expect(dockerRun).toContain('proxy.json');
expect(dockerRun).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=<token>');
expect(skill).not.toContain('--port');
});
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-<n>',
);
const roundBlock = skill.split('qwen --auth-type openai')[0].split('```');
expect(roundBlock.at(-1)).toContain('QWEN_VERIFY_CREDENTIAL_FREE');
// The credential hit aborts structurally, not via `set -e`.
expect(skill).toContain(
'node /trusted-scripts/verify-pr-credential-scan.mjs /work || exit 1',
);
});
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('npm run build && npm run bundle');
expect(skill).toContain('npm install -g --prefix "$HOME/.local" /work');
});
it('refuses a nested repository in the scan description', () => {
expect(skill).toMatch(/nested\s+git repository outright/);
expect(skill).toMatch(/vendored\s+git\s+dependency/);
expect(skill).toContain('not public keys');
expect(skill).toContain('untracked clone, populated submodule');
});
it('names the value-inspection contract and the include-following enumeration', () => {
expect(skill).toContain('inspects every local config');
expect(skill).toContain('`http.proxy` included');
expect(skill).toContain('`url.*.pushInsteadOf`');
expect(skill).toContain('`Authorization` /');
expect(skill).toContain('`Proxy-Authorization` header values');
expect(skill).toContain('bare `git@` account name');
expect(skill).toMatch(/enumeration\s+follows\s+`include\.path`/);
expect(skill).toContain('covers the enabled `.git/config.worktree` scope');
expect(skill).toContain('absent means an empty scope');
expect(skill).toMatch(/filter name\s+containing `=` is refused outright/);
});
it('snapshots the previous report and pins both checker call sites', () => {
expect(skill).toContain('previous-report.md');
expect(skill).toContain('issues/<n>/comments');
const contextCalls = [
...skill.matchAll(/verify-pr-check\.mjs --context <host snapshot>/g),
];
expect(contextCalls.length).toBeGreaterThanOrEqual(2);
// The prose occurrences alone cannot catch step 7 dropping the flag on
// its continuation line — pin the executable call too.
expect(skill).toContain(
'--context ~/.cache/qwen-verify/pr<n>/pr.json <round-dir>',
);
});
it('writes previous-report.md only when a report exists', () => {
// The file's PRESENCE is the follow-up trigger: an empty file from a
// first round (or gh error text) misclassifies every round.
expect(skill).toContain('if [ -n "$PREV" ]; then');
expect(skill).toContain(
'else rm -f ~/.cache/qwen-verify/pr<n>/previous-report.md; fi',
);
expect(skill).toContain(
'{ echo "could not read PR comments" >&2; exit 1; }',
);
// First-paragraph marker + bot-or-self author, not a bare substring any
// reply quoting `Verdict:` would satisfy. gh's --slurp cannot combine
// with --jq, so the selection runs in node over the piped pages.
expect(skill).toContain('c.body.split("\\n\\n")[0].includes("Verdict:")');
expect(skill).toContain(
'/bot$/.test(c.user.login)||c.user.login===process.argv[1]',
);
});
it('keeps the proxy JSON parseable and the base extraction fresh', () => {
// One stderr line folded into proxy.json breaks every parse of it.
expect(skill).toContain(
'> ~/.cache/qwen-verify/pr<n>/proxy.json 2> ~/.cache/qwen-verify/pr<n>/proxy.err',
);
// Add-only extraction would keep a .qwen file the base deleted.
expect(skill).toContain('rm -rf ~/.cache/qwen-verify/pr<n>/qwen-base');
});
it('hashes the base-pinned skill copy in the publish footer', () => {
// The round loads the base commit's .qwen, not the PR head's: hashing
// the head would certify instructions the round never ran.
expect(skill).toContain(
'git -C <pr-clone> rev-parse "<baseRefOid>:.qwen/skills/verify-pr/SKILL.md"',
);
expect(skill).toContain("never the PR head's copy");
});
});
scripts/tests/verify-pr-prepare.test.js
+314
View File
@@ -20,9 +20,12 @@ import { join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { afterEach, describe, expect, it } from 'vitest';
import {
GIT_HARDENING,
artifactDirName,
detectCredentialSignals,
detectIsolation,
gitStatusArgs,
localFilterOverrides,
parseArgs,
parseRepo,
validateContext,
@@ -213,6 +216,264 @@ describe('detectCredentialSignals', () => {
'/home/t',
),
).toEqual(['SANDBOX_MOUNTS']);
// The recipe's proxy credential must stay outside the signal set, or a
// container round would fail the gate before it starts.
expect(
detectCredentialSignals(
{ QWEN_VERIFY_PROXY_TOKEN: 'x' },
() => false,
'/home/t',
),
).toEqual([]);
});
});
describe('git hardening', () => {
it('disables fsmonitor, hooks, credential helpers and excludesFile on every git read', () => {
expect(GIT_HARDENING).toEqual([
'-c',
'core.fsmonitor=false',
'-c',
'core.hooksPath=/dev/null',
'-c',
'credential.helper=',
'-c',
'core.excludesFile=/dev/null',
]);
const { dir } = makeRepo();
expect(localFilterOverrides(dir)).toEqual([]);
expect(gitStatusArgs(dir)).toEqual([
...GIT_HARDENING,
'-C',
dir,
'status',
'--porcelain',
'--untracked-files=normal',
]);
});
// A filter VALUE may contain newlines; parsing the rendered `key value`
// output would accept the second line as a key and forge a `-c` that
// outranks the hardening (git applies the last -c for a key).
it('does not let a newline-bearing filter value forge overrides', () => {
const { dir } = makeRepo();
const hooks = join(dir, '.githooks');
mkdirSync(hooks, { recursive: true });
const marker = join(dir, 'hook-fired');
writeFileSync(
join(hooks, 'post-checkout'),
`#!/bin/sh\necho fired > ${marker}\n`,
);
chmodSync(join(hooks, 'post-checkout'), 0o755);
execFileSync(
'git',
[
'config',
'--local',
'filter.decoy.clean',
`harmless\ncore.hooksPath=${hooks}`,
],
{ cwd: dir, env: gitEnv },
);
mkdirSync(join(dir, '.git', 'info'), { recursive: true });
writeFileSync(join(dir, '.git', 'info', 'attributes'), '* filter=decoy\n');
expect(localFilterOverrides(dir)).toEqual(['-c', 'filter.decoy.clean=']);
// The forged hooksPath must not reach git: the name list never contained
// it, and the fixed hardening is applied last in any case.
const args = gitStatusArgs(dir);
expect(args.filter((a) => a === 'core.hooksPath=/dev/null')).toHaveLength(
1,
);
expect(args.join(' ')).not.toContain(hooks);
spawnSync('git', args, { encoding: 'utf8' });
expect(existsSync(marker)).toBe(false);
});
// git allows '=' inside a filter subsection (filter.a=b.clean), but -c
// splits its argument at the FIRST '=': the override would set a different
// key and leave the real filter armed. Such a name must be refused before
// any guarded status could run it, not emitted as a broken override.
it('refuses a filter name containing = instead of emitting a broken override', () => {
const { dir } = makeRepo();
const marker = join(dir, 'filter-fired');
execFileSync(
'git',
[
'config',
'--local',
'filter.a=b.clean',
`sh -c 'echo fired > ${marker} && cat'`,
],
{ cwd: dir, env: gitEnv },
);
mkdirSync(join(dir, '.git', 'info'), { recursive: true });
writeFileSync(join(dir, '.git', 'info', 'attributes'), '* filter=a=b\n');
// Same-byte-length change: a size change would let git skip the clean
// conversion and the positive control would prove nothing.
writeFileSync(join(dir, 'base.txt'), 'BaSe\n');
spawnSync('git', [...GIT_HARDENING, '-C', dir, 'status', '--porcelain'], {
encoding: 'utf8',
});
expect(existsSync(marker)).toBe(true);
rmSync(marker);
expect(() => localFilterOverrides(dir)).toThrow(
/cannot be safely disabled/,
);
expect(() => gitStatusArgs(dir)).toThrow(/cannot be safely disabled/);
expect(existsSync(marker)).toBe(false);
});
// extensions.worktreeConfig makes git status read .git/config.worktree,
// which --local never reads; a filter planted there must be emptied too.
it('empties a filter planted in .git/config.worktree', () => {
const { dir } = makeRepo();
const marker = join(dir, 'filter-fired');
execFileSync(
'git',
['config', '--local', 'extensions.worktreeConfig', 'true'],
{ cwd: dir, env: gitEnv },
);
execFileSync(
'git',
[
'config',
'--worktree',
'filter.wt.clean',
`sh -c 'echo fired > ${marker} && cat'`,
],
{ cwd: dir, env: gitEnv },
);
mkdirSync(join(dir, '.git', 'info'), { recursive: true });
writeFileSync(join(dir, '.git', 'info', 'attributes'), '* filter=wt\n');
// Same byte length as 'base\n': a size change lets git skip the clean
// conversion, which makes the positive control depend on racy timing.
writeFileSync(join(dir, 'base.txt'), 'BaSe\n');
// Positive control: without the worktree-scope override the filter runs.
spawnSync(
'git',
[
'-c',
'core.fsmonitor=false',
'-c',
'core.hooksPath=/dev/null',
'-C',
dir,
'status',
'--porcelain',
],
{ encoding: 'utf8' },
);
expect(existsSync(marker)).toBe(true);
rmSync(marker);
expect(localFilterOverrides(dir)).toEqual(['-c', 'filter.wt.clean=']);
spawnSync('git', gitStatusArgs(dir), { encoding: 'utf8' });
expect(existsSync(marker)).toBe(false);
});
// The extension does not require config.worktree to exist. prepare's
// --get-regexp enumeration already treats the absent file as no matches
// (exit 1); pin that so a future switch to --list (exit 128 on absence)
// cannot silently false-refuse a clean repo the way the scan once did.
it('tolerates an enabled worktree extension with no config.worktree file', () => {
const { dir, base, head } = makeRepo();
execFileSync(
'git',
['config', '--local', 'extensions.worktreeConfig', 'true'],
{ cwd: dir, env: gitEnv },
);
expect(localFilterOverrides(dir)).toEqual([]);
const status = spawnSync('git', gitStatusArgs(dir), { encoding: 'utf8' });
expect(status.status).toBe(0);
const env = installFakeGh(snapshotFor({ base, head }));
const result = runPrepare(dir, ['42', '--repo', REPO], env);
expect(result.status).toBe(0);
});
// core.excludesFile is steerable from the same PR-controlled config; the
// hardened status must see untracked files even when it points at a
// catch-all exclude file.
it('sees untracked files through a hostile core.excludesFile', () => {
const { dir } = makeRepo();
writeFileSync(join(dir, '.git', 'info', 'leave'), '*\n');
execFileSync(
'git',
['config', '--local', 'core.excludesFile', '.git/info/leave'],
{ cwd: dir, env: gitEnv },
);
writeFileSync(join(dir, 'leftover-modified.ts'), 'export {};\n');
const steered = spawnSync(
'git',
['-C', dir, 'status', '--porcelain', '--untracked-files=normal'],
{ encoding: 'utf8', env: gitEnv },
);
expect(steered.stdout.trim()).toBe('');
const hardened = spawnSync('git', gitStatusArgs(dir), {
encoding: 'utf8',
env: gitEnv,
});
expect(hardened.stdout).toContain('?? leftover-modified.ts');
});
it('empties a planted clean filter before reading status', () => {
const { dir } = makeRepo();
const marker = join(dir, 'filter-fired');
execFileSync(
'git',
[
'config',
'--local',
'filter.pwn.clean',
`sh -c 'echo fired > ${marker}'`,
],
{ cwd: dir, env: gitEnv },
);
mkdirSync(join(dir, '.git', 'info'), { recursive: true });
writeFileSync(join(dir, '.git', 'info', 'attributes'), '* filter=pwn\n');
// Same byte length as 'base\n': a size change lets git skip the clean
// conversion, which makes the positive control depend on racy timing.
writeFileSync(join(dir, 'base.txt'), 'BaSe\n');
// Positive control: without the filter overrides the planted filter runs
// on the host during an ordinary status.
spawnSync('git', [...GIT_HARDENING, '-C', dir, 'status', '--porcelain'], {
encoding: 'utf8',
});
expect(existsSync(marker)).toBe(true);
rmSync(marker);
expect(localFilterOverrides(dir)).toEqual(['-c', 'filter.pwn.clean=']);
spawnSync('git', gitStatusArgs(dir), { encoding: 'utf8' });
expect(existsSync(marker)).toBe(false);
});
// --local alone names one file and skips include.path, while the config
// parsing `git status` uses follows it: a filter defined in an included
// worktree file must be enumerated too, or it runs on the host unopposed.
it('empties a clean filter that arrives through include.path', () => {
const { dir } = makeRepo();
const marker = join(dir, 'filter-fired');
writeFileSync(
join(dir, 'evil.inc'),
`[filter "pwn"]\n\tclean = sh -c 'echo fired > ${marker}'\n`,
);
execFileSync('git', ['config', '--local', 'include.path', '../evil.inc'], {
cwd: dir,
env: gitEnv,
});
mkdirSync(join(dir, '.git', 'info'), { recursive: true });
writeFileSync(join(dir, '.git', 'info', 'attributes'), '* filter=pwn\n');
// Same byte length as 'base\n': a size change lets git skip the clean
// conversion, which makes the positive control depend on racy timing.
writeFileSync(join(dir, 'base.txt'), 'BaSe\n');
// Positive control: an ordinary hardened status follows the include and
// executes the planted filter on the host.
spawnSync('git', [...GIT_HARDENING, '-C', dir, 'status', '--porcelain'], {
encoding: 'utf8',
});
expect(existsSync(marker)).toBe(true);
rmSync(marker);
expect(localFilterOverrides(dir)).toEqual(['-c', 'filter.pwn.clean=']);
spawnSync('git', gitStatusArgs(dir), { encoding: 'utf8' });
expect(existsSync(marker)).toBe(false);
});
});
@@ -545,6 +806,49 @@ describe.skipIf(process.platform === 'win32')('prepare (CLI)', () => {
expect(existsSync(join(realpathSync(dir), 'tmp'))).toBe(false);
});
// The refusal must reach the CLI as a clean exit 3 with its own remedy,
// before any guarded status runs — not as an uncaught enumeration stack.
it('refuses a tree whose filter name cannot be safely disabled', () => {
const { dir, base, head } = makeRepo();
const env = installFakeGh(snapshotFor({ base, head }));
execFileSync('git', ['config', '--local', 'filter.a=b.clean', 'true'], {
cwd: dir,
env: gitEnv,
});
const result = runPrepare(dir, ['42', '--repo', REPO], env);
expect(result.status).toBe(3);
expect(result.stderr).toContain("contains '='");
expect(result.stderr).toContain('cannot be safely disabled');
expect(existsSync(join(realpathSync(dir), 'tmp'))).toBe(false);
});
// .git/info/exclude has no -c override: active patterns there could hide
// untracked files from the clean-tree check, so the gate refuses the file
// itself. A fresh clone carries only git's commented default.
it('refuses a .git/info/exclude with active patterns', () => {
const { dir, base, head } = makeRepo();
const env = installFakeGh(snapshotFor({ base, head }));
mkdirSync(join(dir, '.git', 'info'), { recursive: true });
writeFileSync(join(dir, '.git', 'info', 'exclude'), '*\n');
writeFileSync(join(dir, 'hidden.ts'), 'export {};\n');
const refused = runPrepare(dir, ['42', '--repo', REPO], env);
expect(refused.status).toBe(3);
expect(refused.stderr).toContain('.git/info/exclude');
expect(refused.stderr).toContain('re-clone');
expect(existsSync(join(realpathSync(dir), 'tmp'))).toBe(false);
// The commented default a fresh clone ships with must pass.
writeFileSync(
join(dir, '.git', 'info', 'exclude'),
'# git ls-files --others --exclude-standard\n# *.[oa]\n',
);
rmSync(join(dir, 'hidden.ts'));
const clean = runPrepare(dir, ['42', '--repo', REPO], env);
expect(clean.status).toBe(0);
});
// git status trusts the index: skip-worktree / assume-unchanged entries
// (a sparse checkout sets them en masse) hide tracked divergence, so the
// gate must refuse them instead of stamping a partial tree as clean.
@@ -608,6 +912,16 @@ describe.skipIf(process.platform === 'win32')('prepare (CLI)', () => {
expect(result.status).toBe(2);
expect(result.stderr).toContain('--repo <owner>/<repo> is required');
});
// Outside a worktree the filter enumeration fails before any git read;
// the refusal must stay the clean exit 3, not an uncaught stack trace.
it('refuses a non-git cwd with the worktree message, not a stack trace', () => {
const dir = makeTmpDir();
const result = runPrepare(dir, ['42', '--repo', REPO], gitEnv);
expect(result.status).toBe(3);
expect(result.stderr).toContain('not inside a git worktree');
expect(result.stderr).not.toContain('Error: could not enumerate');
});
});
// The gate requires an empty `git status`, and prepare writes its artifacts
scripts/verify-pr-check.mjs
+63 -18
View File
@@ -56,7 +56,7 @@ const HEADING_TITLE_MAX = 120;
const FOOTER =
/^_— .+ via Qwen Code \/verify-pr \(v[^;]+; skill [0-9a-f]{12}\)_$/;
const HELP = `Usage: node scripts/verify-pr-check.mjs <artifact-dir>
const HELP = `Usage: node scripts/verify-pr-check.mjs [--context <snapshot>] <artifact-dir>
Checks a local round against the rules the CI publisher enforces:
@@ -80,6 +80,10 @@ ${SUITE_SIZED_TOTAL} assertions come with no gate results (an existing test suit
counted as the round's own), or when comment.md — the exact body a local round
posts, optional — repeats the heading shape or lacks the attribution footer.
With --context <snapshot>, the head-OID check reads that file instead of the
round's own context.json. Pass a copy the round never saw: the artifact
directory is writable by the PR code the round executed.
Exit codes: 0 no errors, 1 errors found, 2 usage.
`;
@@ -416,10 +420,29 @@ function main(argv) {
process.stdout.write(HELP);
return 0;
}
const dir = argv.find((arg) => !arg.startsWith('-'));
if (!dir) {
let contextPath = null;
const positional = [];
for (let index = 0; index < argv.length; index++) {
const arg = argv[index];
if (arg === '--context') {
contextPath = argv[++index];
if (!contextPath) {
process.stderr.write(
`verify-pr check: --context requires a snapshot path\n\n${HELP}`,
);
return 2;
}
} else if (arg.startsWith('-')) {
process.stderr.write(`verify-pr check: unknown option ${arg}\n\n${HELP}`);
return 2;
} else {
positional.push(arg);
}
}
const dir = positional[0];
if (!dir || positional.length > 1) {
process.stderr.write(
`verify-pr check: an artifact directory is required\n\n${HELP}`,
`verify-pr check: exactly one artifact directory is required\n\n${HELP}`,
);
return 2;
}
@@ -448,25 +471,47 @@ function main(argv) {
// rather than silently downgrading the check.
let expectedHead = null;
let snapshotWarning = null;
let snapshotPath = join(dir, 'context.json');
const prNumber = /^pr(\d+)-verify-/.exec(basename(resolve(dir)))?.[1];
if (!existsSync(snapshotPath) && prNumber) {
snapshotPath = join(
dirname(resolve(dir)),
'verify-context',
`pr${prNumber}.json`,
);
}
if (existsSync(snapshotPath)) {
let snapshotPath = null;
if (contextPath) {
// A host-held snapshot: the artifact directory is writable by the PR code
// the round just executed, so its own context.json cannot pin the head
// when the maintainer supplies a copy that code never saw.
try {
const snapshot = JSON.parse(readFileSync(snapshotPath, 'utf8'));
const snapshot = JSON.parse(readFileSync(contextPath, 'utf8'));
if (/^[0-9a-f]{40}$/.test(snapshot?.headRefOid ?? '')) {
expectedHead = snapshot.headRefOid;
} else {
snapshotWarning = `snapshot ${snapshotPath} has no valid head OID; the head-OID check fell back to presence-only`;
snapshotPath = contextPath;
}
} catch {
snapshotWarning = `snapshot ${snapshotPath} is unreadable; the head-OID check fell back to presence-only`;
// Reported below; never falls back to the writable artifact copy.
}
if (!expectedHead) {
process.stderr.write(
`verify-pr check: --context ${contextPath} is missing, unreadable, or carries no valid head OID\n`,
);
return 1;
}
} else {
snapshotPath = join(dir, 'context.json');
const prNumber = /^pr(\d+)-verify-/.exec(basename(resolve(dir)))?.[1];
if (!existsSync(snapshotPath) && prNumber) {
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;
} else {
snapshotWarning = `snapshot ${snapshotPath} has no valid head OID; the head-OID check fell back to presence-only`;
}
} catch {
snapshotWarning = `snapshot ${snapshotPath} is unreadable; the head-OID check fell back to presence-only`;
}
}
}
scripts/verify-pr-credential-scan.mjs
+329
View File
@@ -0,0 +1,329 @@
#!/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 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. Two
* exclusions are deliberate: credentials inside `node_modules`/`dist` are
* accepted as installed/build content, and a nested git repository is refused
* outright because git will not descend into it — scan that clone separately.
*
* Usage:
* node scripts/verify-pr-credential-scan.mjs <root>
*
* Exit codes: 0 clean, 1 credential-shaped entries found, 2 usage error.
*/
import { spawnSync } from 'node:child_process';
import { existsSync } from 'node:fs';
import { basename, join } from 'node:path';
import { isEntryPoint } from './lib/entry-point.js';
const CREDENTIAL_BASENAME =
/^(?:\.env.*|.*\.env|.*\.env~|.*\.pem|.*\.key|.*\.p12|.*\.pfx|id_(?:rsa|ed25519|ecdsa|dsa).*|.*credentials.*|\.git-credentials|\.npmrc|\.netrc|gha-creds-.*\.json)$/;
// 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)**/*.env~',
':(glob)**/*.pem',
':(glob)**/*.key',
':(glob)**/*.p12',
':(glob)**/*.pfx',
':(glob)**/id_rsa*',
':(glob)**/id_ed25519*',
':(glob)**/id_ecdsa*',
':(glob)**/id_dsa*',
':(glob)**/*credentials*',
':(glob)**/.git-credentials',
':(glob)**/.npmrc',
':(glob)**/.netrc',
':(glob)**/gha-creds-*.json',
':(exclude,glob)**/node_modules/**',
':(exclude,glob)**/dist/**',
];
// A nested clone is a repository git does not descend into, so a credential
// file inside it would silently pass an ignored-file scan. Dependencies and
// build output keep their own exclusion: a vendored `.git` is no more a
// project credential than a vendored `.env`.
const NESTED_REPO_PATHSPECS = [
':(glob)**/.git',
':(exclude,glob)**/node_modules/**',
':(exclude,glob)**/dist/**',
];
const EXCLUDED_SEGMENTS = new Set(['node_modules', 'dist']);
/** True for a path whose basename is credential-shaped. */
export function isCredentialShaped(path) {
const name = basename(path);
// A public key is not a credential; refusing id_rsa.pub would be noise.
if (name.endsWith('.pub')) return false;
return CREDENTIAL_BASENAME.test(name);
}
export function gitScanArgs(root) {
return [
'-C',
root,
'ls-files',
'--others',
'--ignored',
'--exclude-standard',
'-z',
'--',
...CREDENTIAL_PATHSPECS,
];
}
/**
* Returns the nested repositories found under `root`, sorted. Three surfaces
* are enumerated, because git collapses a nested working tree to a directory
* entry on each of them: an ignored clone, an untracked NON-ignored clone
* (the shape a round's own `git clone` leaves behind), and a populated
* submodule (a mode-160000 gitlink whose directory carries a `.git`).
* Dependencies and build output keep their exclusion on every surface: a
* vendored `.git` is no more a project credential than a vendored `.env`.
*/
export function nestedGitRepos(root, { maxBuffer = 8 * 1024 * 1024 } = {}) {
const git = (args) => {
const result = spawnSync('git', ['-C', root, ...args], {
encoding: 'utf8',
maxBuffer,
});
if (result.status !== 0) {
throw new Error(
`could not list nested repositories: ${(result.stderr || '').trim() || 'git failed'}`,
);
}
return result.stdout;
};
const excluded = (path) =>
path.split('/').some((segment) => EXCLUDED_SEGMENTS.has(segment));
const found = new Set();
const ignored = git([
'ls-files',
'--others',
'--ignored',
'--exclude-standard',
'-z',
'--',
...NESTED_REPO_PATHSPECS,
]);
for (const path of ignored.split('\0')) {
if (!path || excluded(path)) continue;
found.add(path.replace(/\/$/, ''));
}
const untracked = git(['ls-files', '--others', '--exclude-standard', '-z']);
for (const entry of untracked.split('\0')) {
if (!entry.endsWith('/') || excluded(entry)) continue;
if (existsSync(join(root, entry, '.git'))) {
found.add(entry.replace(/\/$/, ''));
}
}
const staged = git(['ls-files', '--stage', '-z']);
for (const record of staged.split('\0')) {
if (!record.startsWith('160000 ')) continue;
const tab = record.indexOf('\t');
const path = tab === -1 ? '' : record.slice(tab + 1);
if (!path || excluded(path)) continue;
// An unpopulated gitlink has no working tree to read; its URL is covered
// by the config value scan instead.
if (existsSync(join(root, path, '.git'))) found.add(path);
}
return [...found].sort();
}
/**
* Returns local config entries that carry a credential PR code could read
* with plain `git config`. The scan inspects VALUES, not a list of keys: any
* config value can hold a URL with a token (`http.proxy`, a novel mirror
* key, tomorrow's sibling of `pushurl`), so a key allowlist never converges.
* A value hits when it embeds http(s) userinfo or when it is an
* `Authorization` / `Proxy-Authorization` header (the RFC 7235 pair GitHub
* Actions writes into `http.*.extraheader`); other header names stay clean.
* The two URL-bearing KEY families are judged too: `url.*.insteadOf` /
* `url.*.pushInsteadOf` rewrites and `http.<url>.*` sections carry their URL
* in the key, not the value. On an http(s) URL any userinfo counts — a token
* used as the username (`https://<pat>@…`) is as readable as `user:pass@` —
* except the bare conventional account name `git@` in any spelling, which
* carries no secret, matching `ssh://git@…`. `--includes` follows
* `include.path` the way the `git config --get` PR code uses does, and
* `--list --null` frames each `key\nvalue` record so a multi-line value
* cannot forge entries. When `extensions.worktreeConfig` is set, ordinary
* `git config --get` also reads `.git/config.worktree`, so that scope (with
* its includes) is enumerated too — `--local` alone would report a
* credential there as clean. The file is optional: absent means an empty
* scope, while an existing but unreadable or malformed one refuses. A
* `credential.helper` COMMAND is out of this
* URL/header contract by design (see the design record); prepare disables
* helpers for its own reads.
*/
export function credentialBearingConfig(
root,
{ maxBuffer = 8 * 1024 * 1024 } = {},
) {
const readScope = (scope) => {
const result = spawnSync(
'git',
['-C', root, 'config', scope, '--includes', '--list', '--null'],
{ encoding: 'utf8', maxBuffer },
);
if (result.status !== 0) {
throw new Error(
`could not read local git config: ${(result.stderr || '').trim() || 'git failed'}`,
);
}
return result.stdout;
};
const worktreeConfig = spawnSync(
'git',
[
'-C',
root,
'config',
'--local',
'--includes',
'--bool',
'extensions.worktreeConfig',
],
{ encoding: 'utf8', maxBuffer },
);
const scopes = ['--local'];
if (worktreeConfig.status === 0 && worktreeConfig.stdout.trim() === 'true') {
// Enabling the extension does not require the file to exist: an absent
// optional config.worktree is an EMPTY scope (explicit `--worktree
// --list` would exit 128 on it and false-refuse a clean repo). An
// existing but unreadable or malformed file still refuses, because
// readScope throws on any git error.
const gitDir = spawnSync(
'git',
['-C', root, 'rev-parse', '--absolute-git-dir'],
{ encoding: 'utf8', maxBuffer },
);
if (gitDir.status !== 0) {
throw new Error(
`could not read local git config: ${(gitDir.stderr || '').trim() || 'git failed'}`,
);
}
if (existsSync(join(gitDir.stdout.trim(), 'config.worktree'))) {
scopes.push('--worktree');
}
}
const hasUserinfo = (value) => {
const match = /^https?:\/\/([^/@\s]+)@/i.exec(value);
// `git` is the conventional account name, not a secret, and account
// names are case-insensitive; any other username-only userinfo may be
// a token, so it stays refused.
return match !== null && match[1].toLowerCase() !== 'git';
};
const isAuthHeader = (value) =>
/^\s*(?:proxy-)?authorization\s*:/i.test(value);
const hits = [];
for (const scope of scopes) {
for (const record of readScope(scope).split('\0')) {
if (!record) continue;
const newline = record.indexOf('\n');
const key = newline === -1 ? record : record.slice(0, newline);
const value = newline === -1 ? '' : record.slice(newline + 1);
const rewrite = /^url\.(.*)\.(insteadof|pushinsteadof)$/.exec(key);
const httpSection = /^http\.(.+)\.[^.]+$/.exec(key);
if (
hasUserinfo(value) ||
isAuthHeader(value) ||
(rewrite && hasUserinfo(rewrite[1])) ||
(httpSection && hasUserinfo(httpSection[1]))
) {
if (!hits.includes(key)) hits.push(key);
}
}
}
return hits;
}
/**
* 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, and a nested repository is refused because a
* scan of the outer tree cannot see into it.
*/
export function scanIgnoredCredentials(
root,
{ maxBuffer = 8 * 1024 * 1024 } = {},
) {
const nested = nestedGitRepos(root, { maxBuffer });
if (nested.length > 0) {
throw new Error(
`nested git repository under ${nested[0]} — scan that clone separately or remove it`,
);
}
const configHits = credentialBearingConfig(root, { maxBuffer });
if (configHits.length > 0) {
throw new Error(
`credential-bearing git config in ${configHits[0]} — remove the credential from .git/config`,
);
}
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'}`,
);
}
return result.stdout
.split('\0')
.filter(Boolean)
.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 <root>\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));
}
scripts/verify-pr-model-proxy.mjs
+345
View File
@@ -0,0 +1,345 @@
#!/usr/bin/env node
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
/**
* Host-side model proxy for a local `/verify-pr` round.
*
* A local round needs the same isolation the CI lane provides: the container
* executes untrusted PR code, so the real model credential must stay outside
* it. This proxy holds the real key on the host and lets the container reach
* the model with a per-run bearer token instead. It mirrors the CI lane's
* inline proxy (qwen-triage.yml `start_openai_proxy`), with changes the local
* shape needs: the listen address is a flag (a container cannot reach the
* host's loopback, so bind the Docker bridge address instead), and the
* coordinates are printed as one JSON line for the maintainer to consume.
* Hop-by-hop request headers are stripped rather than forwarded.
* The printed baseUrl uses the listen address, so bind the address clients
* dial — 0.0.0.0 listens on every interface but is not a dialable URL.
*
* The CI lane deliberately keeps its own inline copy: the proxy handles the
* real key, so it must not execute a script from the PR-head checkout.
*
* Usage (from a trusted checkout, never the PR-head tree — this process holds
* the real key and a PR can rewrite a script in its own checkout):
* if [ -t 0 ]; then
* read -rsp 'REVIEW_OPENAI_API_KEY: ' REVIEW_OPENAI_API_KEY; echo
* else
* IFS= read -r REVIEW_OPENAI_API_KEY \
* < ~/.config/qwen-verify/review-openai-key
* fi
* export REVIEW_OPENAI_API_KEY
* REVIEW_OPENAI_BASE_URL=<url> \
* node <trusted-clone>/scripts/verify-pr-model-proxy.mjs \
* [--listen <host>]
*
* Reading the key from a terminal when one exists and from a file otherwise
* keeps it out of shell history (an inline assignment is recorded) and keeps
* an agent-driven run, whose shell has no TTY, from blocking on `read`.
*
* Options:
* --listen <host> address to bind (default 127.0.0.1; use the Docker
* bridge address, e.g. 172.17.0.1, for container clients).
* The port is always ephemeral: a fixed port is a binding
* race the CI lane avoids too.
*
* Environment:
* REVIEW_OPENAI_BASE_URL upstream base URL, may carry a path prefix
* REVIEW_OPENAI_API_KEY the real key; never leaves this process
* QWEN_VERIFY_PROXY_TOKEN bearer clients must present (default: random)
*
* On readiness, prints one JSON line: {port, baseUrl, token, nonce}. Give the
* container `baseUrl` as the provider base URL and `token` as its credential;
* `GET /__health` echoes `nonce`. Only `POST <basePath>/chat/completions` is
* relayed — the path is matched exactly, not as a suffix.
*/
import { randomBytes } from 'node:crypto';
import http from 'node:http';
import { Readable } from 'node:stream';
import { isEntryPoint } from './lib/entry-point.js';
export const DEFAULT_IDLE_TIMEOUT_MS = 240_000;
// The qwen client tolerates 240 s of silence inside one stream
// (DEFAULT_STREAM_IDLE_TIMEOUT_MS in the OpenAI content generator), so a
// tighter proxy window would cut a completion its own client would have
// waited out. Keep the default equal; a caller may still pass its own value.
// RFC 7230 hop-by-hop headers describe the client's connection to us, not the
// upstream one. Forwarding them is never correct, and a copied
// transfer-encoding or expect also fights the framing undici chooses for the
// relayed stream. `host` and `content-length` ride along for the same reason.
const STRIPPED_REQUEST_HEADERS = [
'host',
'content-length',
'connection',
'keep-alive',
'proxy-authorization',
'proxy-authenticate',
'te',
'trailer',
'transfer-encoding',
'upgrade',
'expect',
];
export function parseArgs(argv) {
const options = { listen: '127.0.0.1' };
for (let i = 0; i < argv.length; i++) {
const arg = argv[i];
if (arg === '--listen') {
const value = argv[++i];
// An empty value would reach server.listen(port, ''), which binds
// every interface — past the bridge this proxy is meant to stay on.
if (value === undefined || value === '' || value.startsWith('--')) {
throw new Error('--listen requires a value');
}
options.listen = value;
} else {
throw new Error(`unknown argument: ${arg}`);
}
}
return options;
}
/** Strips the trailing slashes a base path may carry; `''` means none. */
export function normalizeBasePath(pathname) {
const trimmed = (pathname ?? '').replace(/\/+$/, '');
return trimmed === '/' ? '' : trimmed;
}
/**
* Assembles the base URL the maintainer hands to the container. A host that
* carries a colon is an IPv6 literal and needs brackets, or `new URL()`
* rejects the string downstream.
*/
export function formatLocalBaseUrl(listen, port, basePath = '') {
const host = listen.includes(':') ? `[${listen}]` : listen;
return `http://${host}:${port}${basePath}`;
}
/**
* Returns an http.Server that relays only POST /chat/completions to the
* upstream, replacing the client's bearer token with the real key. The idle
* watchdog is per-chunk, not total: qwen tolerates minutes of silence inside
* one stream, so a total cap would cut healthy long completions. It also
* stays quiet while the downstream is paused — backpressure from a slow but
* live client is not upstream idleness — and re-arms on 'drain'.
*/
export function createModelProxy({
baseUrl,
apiKey,
token,
nonce,
idleTimeoutMs = DEFAULT_IDLE_TIMEOUT_MS,
}) {
if (!baseUrl || !apiKey || !token || !nonce) {
throw new Error('baseUrl, apiKey, token and nonce are all required');
}
const base = new URL(baseUrl);
const basePath = normalizeBasePath(base.pathname);
return http.createServer(async (req, res) => {
if (req.url === '/__health') {
// Identity, not just liveness: a squatter cannot know the nonce.
res.writeHead(200, { 'content-type': 'text/plain' });
res.end(nonce);
return;
}
let timer;
try {
const incoming = new URL(req.url || '/', 'http://127.0.0.1');
const target = new URL(base.origin);
let path = incoming.pathname;
if (
basePath &&
basePath !== '/' &&
path !== basePath &&
!path.startsWith(`${basePath}/`)
) {
path = `${basePath}${path.startsWith('/') ? '' : '/'}${path}`;
}
target.pathname = path;
target.search = incoming.search;
// Exact path, not a suffix: `endsWith('/chat/completions')` would also
// relay `/admin/chat/completions` to the upstream.
const allowedPath = basePath
? `${basePath}/chat/completions`
: '/chat/completions';
if (req.method !== 'POST' || target.pathname !== allowedPath) {
res.writeHead(403, { 'content-type': 'text/plain' });
res.end(`proxy: only POST ${allowedPath} is allowed\n`);
return;
}
// Only the agent knows this run's token; anything else that finds the
// port cannot get the key used.
if (req.headers.authorization !== `Bearer ${token}`) {
res.writeHead(401, { 'content-type': 'text/plain' });
res.end('proxy: unauthorized\n');
return;
}
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 };
init.body = req;
init.duplex = 'half';
const controller = new AbortController();
let body = null;
let finished = false;
const arm = () => {
if (finished) return;
clearTimeout(timer);
timer = setTimeout(() => {
// A paused body is downstream backpressure, not upstream silence:
// the client is alive but slow, and aborting here would cut a
// healthy completion. The 'drain' listener re-arms this window.
if (body?.isPaused()) {
arm();
return;
}
controller.abort();
}, idleTimeoutMs);
};
// Registered before the fetch: a client that disconnects while the
// upstream headers are still in flight has already emitted 'close', so
// registering after the response would never abort that request.
res.on('close', () => {
finished = true;
clearTimeout(timer);
controller.abort();
});
res.on('drain', arm);
// A slow request UPLOAD is activity too: fetch resolves only on the
// upstream headers, so without this a large prompt on a slow link
// would be torn down mid-upload by a window meant for stream silence.
// 'readable', not 'data': a data listener switches the request to
// flowing mode and steals the body chunks from the relayed stream.
req.on('readable', arm);
arm();
let upstream;
try {
upstream = await fetch(target, { ...init, signal: controller.signal });
} catch (error) {
if (error instanceof Error && error.name === 'AbortError') {
if (!res.destroyed && !res.writableEnded) {
res.writeHead(504, { 'content-type': 'text/plain' });
res.end('proxy error: upstream request timed out\n');
}
return;
}
throw error;
}
if (res.destroyed) {
finished = true;
clearTimeout(timer);
return;
}
// The timer is deliberately NOT cleared here: fetch() resolves on
// HEADERS, and the armed timer doubles as the first idle window for
// the body; the per-chunk handler below refreshes it.
const responseHeaders = {};
upstream.headers.forEach((value, key) => {
const lower = key.toLowerCase();
if (lower !== 'content-encoding' && lower !== 'content-length') {
responseHeaders[key] = value;
}
});
res.writeHead(upstream.status, responseHeaders);
if (upstream.body) {
body = Readable.fromWeb(upstream.body);
const done = () => {
finished = true;
clearTimeout(timer);
};
body.on('end', done);
// An idle watchdog firing mid-body aborts the upstream, which
// surfaces here as an error: tear down the downstream response too.
body.on('error', () => {
done();
res.destroy();
});
body.pipe(res);
body.on('data', arm);
} else {
finished = true;
clearTimeout(timer);
res.end();
}
} catch (error) {
clearTimeout(timer);
console.error('proxy upstream failure:', error);
res.writeHead(502, { 'content-type': 'text/plain' });
res.end('proxy error: upstream request failed\n');
}
});
}
async function main(argv) {
let options;
try {
options = parseArgs(argv);
} catch (error) {
process.stderr.write(`${error.message}\n`);
return 2;
}
const baseUrl = process.env.REVIEW_OPENAI_BASE_URL;
const apiKey = process.env.REVIEW_OPENAI_API_KEY;
if (!baseUrl || !apiKey) {
process.stderr.write(
'REVIEW_OPENAI_BASE_URL and REVIEW_OPENAI_API_KEY must both be set\n',
);
return 2;
}
const token =
process.env.QWEN_VERIFY_PROXY_TOKEN || randomBytes(24).toString('hex');
const nonce = randomBytes(24).toString('hex');
const server = createModelProxy({ baseUrl, apiKey, token, nonce });
await new Promise((resolve, reject) => {
server.once('error', reject);
server.listen(0, options.listen, resolve);
});
const { port } = server.address();
const localBaseUrl = formatLocalBaseUrl(
options.listen,
port,
normalizeBasePath(new URL(baseUrl).pathname),
);
// One JSON line, on stdout: the maintainer pastes baseUrl/token into the
// container's provider configuration. The real key is never printed.
process.stdout.write(
`${JSON.stringify({ port, baseUrl: localBaseUrl, token, nonce })}\n`,
);
return 0;
}
if (isEntryPoint(import.meta.url, process.argv[1])) {
main(process.argv.slice(2)).then(
(code) => {
process.exitCode = code;
},
(error) => {
console.error(error);
process.exitCode = 1;
},
);
}
scripts/verify-pr-prepare.mjs
+146 -12
View File
@@ -46,7 +46,7 @@
*/
import { spawnSync } from 'node:child_process';
import { existsSync, mkdirSync, writeFileSync } from 'node:fs';
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs';
import { homedir } from 'node:os';
import { join } from 'node:path';
import { isEntryPoint } from './lib/entry-point.js';
@@ -272,6 +272,126 @@ function run(command, args) {
return spawnSync(command, args, { encoding: 'utf8' });
}
// The tree under inspection is PR-controlled and can persist hostile git
// config in .git/config: fsmonitor, hooks and clean/smudge/process filters all
// execute on an ordinary `git status`. On the host these run as the maintainer,
// outside the boundary, so every git command disables the fixed vectors and
// empties every locally defined filter before reading the tree. The fixed
// vectors come LAST in every argument list: git applies the last `-c` for a
// key, so an enumerated entry must never be able to outrank them.
export const GIT_HARDENING = [
'-c',
'core.fsmonitor=false',
'-c',
'core.hooksPath=/dev/null',
'-c',
'credential.helper=',
'-c',
'core.excludesFile=/dev/null',
];
const FILTER_NAME = /^filter\..+\.(clean|smudge|process)$/;
/**
* A filter name git allows (e.g. `filter.a=b.clean`) but `-c key=value`
* cannot represent: git splits the argument at the FIRST `=`, so emitting an
* override would set a different key and leave the real filter armed. The
* gate refuses such names instead, with their own remedy.
*/
export class UnsafeFilterNameError extends Error {
constructor(name) {
super(
`filter name ${name} contains '=' and cannot be safely disabled with -c — remove the filter from the tree's git config, or re-clone, then re-run prepare`,
);
this.name = 'UnsafeFilterNameError';
}
}
/**
* Empty `-c` overrides for every locally defined filter command. The filter
* name is attacker-chosen, so the override set has to be enumerated from the
* tree's own config instead of hardcoded. Three properties are load-bearing:
* `--includes` follows `include.path` the way the config parsing `git status`
* uses does; `--null --name-only` frames KEY NAMES only, because a filter
* VALUE may contain newlines that would forge extra entries if the rendered
* `key value` output were parsed; and each name is re-validated before it
* becomes a `-c` argument. When `extensions.worktreeConfig` is set,
* `git status` also reads `.git/config.worktree`, so that scope is enumerated
* too. Names containing `=` throw UnsafeFilterNameError — no override can
* represent them.
*/
export function localFilterOverrides(root) {
const prefix = root ? ['-C', root] : [];
const scopes = ['--local'];
const worktreeConfig = run('git', [
...prefix,
'config',
'--local',
'--includes',
'--bool',
'extensions.worktreeConfig',
]);
if (worktreeConfig.status === 0 && worktreeConfig.stdout.trim() === 'true') {
scopes.push('--worktree');
}
const overrides = [];
for (const scope of scopes) {
const result = run('git', [
...prefix,
'config',
scope,
'--includes',
'--null',
'--name-only',
'--get-regexp',
'^filter\\..*\\.(clean|smudge|process)$',
]);
if (result.status === 1) continue;
if (result.status !== 0) {
throw new Error('could not enumerate local git filters');
}
for (const name of result.stdout.split('\0')) {
if (!name || !FILTER_NAME.test(name)) continue;
if (name.includes('=')) throw new UnsafeFilterNameError(name);
overrides.push('-c', `${name}=`);
}
}
return overrides;
}
export function gitStatusArgs(root) {
return [
...localFilterOverrides(root),
...GIT_HARDENING,
'-C',
root,
'status',
'--porcelain',
'--untracked-files=normal',
];
}
function runGit(args) {
return run('git', [...localFilterOverrides(), ...GIT_HARDENING, ...args]);
}
/**
* True when `.git/info/exclude` carries active patterns. A fresh clone has
* only git's commented default there; patterns hide untracked files from the
* clean-tree check, and unlike core.excludesFile no `-c` can neutralize this
* file, so the gate refuses it instead of reading a steered status.
*/
export function hasHostileInfoExclude(root) {
const path = join(root, '.git', 'info', 'exclude');
if (!existsSync(path)) return false;
return readFileSync(path, 'utf8')
.split('\n')
.some((line) => {
const trimmed = line.trim();
return trimmed !== '' && !trimmed.startsWith('#');
});
}
function report(lines) {
process.stdout.write(`${lines.join('\n')}\n`);
}
@@ -296,7 +416,17 @@ function main(argv) {
const { owner, name } = parseRepo(options.repo);
const slug = `${owner}/${name}`;
const toplevel = run('git', ['rev-parse', '--show-toplevel']);
let toplevel;
try {
toplevel = runGit(['rev-parse', '--show-toplevel']);
} catch (error) {
// A hostile filter name is a refusal with its own remedy, not a missing
// worktree; it must surface before any guarded status could run it.
if (error instanceof UnsafeFilterNameError) {
return fail(3, error.message);
}
return fail(3, 'not inside a git worktree — run this from the clone');
}
if (toplevel.status !== 0) {
return fail(3, 'not inside a git worktree — run this from the clone');
}
@@ -338,7 +468,7 @@ function main(argv) {
];
for (const [label, oid, ref] of probes) {
if (
run('git', ['-C', root, 'cat-file', '-e', `${oid}^{commit}`]).status !== 0
runGit(['-C', root, 'cat-file', '-e', `${oid}^{commit}`]).status !== 0
) {
return fail(
3,
@@ -351,7 +481,7 @@ function main(argv) {
// than the PR head or an uncommitted change would measure code the report
// does not name. Checked before the artifact directory exists, so a refusal
// leaves nothing behind.
const treeHead = run('git', ['-C', root, 'rev-parse', 'HEAD']);
const treeHead = runGit(['-C', root, 'rev-parse', 'HEAD']);
if (treeHead.status !== 0) {
return fail(3, 'could not read HEAD in this clone');
}
@@ -361,15 +491,19 @@ function main(argv) {
`current tree is at ${treeHead.stdout.trim()}, not PR head ${context.head} — run: git checkout ${context.head}, then re-run prepare`,
);
}
// The ignore channel is steerable from the same PR-controlled .git:
// core.excludesFile is neutralized by GIT_HARDENING, but .git/info/exclude
// has no `-c` override, so active patterns there are refused outright
// rather than read through.
if (hasHostileInfoExclude(root)) {
return fail(
3,
'.git/info/exclude carries local exclude patterns, which can hide untracked files from the clean-tree check — re-clone (a fresh clone has only the commented default), or clear the file, then re-run prepare',
);
}
// Explicit mode: a maintainer's status.showUntrackedFiles=no would otherwise
// hide the untracked files this check exists to refuse.
const dirty = run('git', [
'-C',
root,
'status',
'--porcelain',
'--untracked-files=normal',
]);
const dirty = run('git', gitStatusArgs(root));
if (dirty.status !== 0) {
const detail = (dirty.stderr || '').trim() || `exit ${dirty.status}`;
return fail(3, `could not read the working tree status: ${detail}`);
@@ -387,7 +521,7 @@ function main(argv) {
// git status trusts the index: skip-worktree / assume-unchanged entries
// (a sparse checkout sets them en masse) hide tracked divergence from it,
// so refuse anything that is not a plain cached 'H' entry.
const flagged = run('git', ['-C', root, 'ls-files', '-v']);
const flagged = runGit(['-C', root, 'ls-files', '-v']);
if (flagged.status !== 0) {
const detail = (flagged.stderr || '').trim() || `exit ${flagged.status}`;
return fail(3, `could not read the index flags: ${detail}`);