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.
2026-11-07 14:38:23 -10:00 · 2026-11-02 00:49:59 +08:00
1 parent 1a57910d87
commit 3509c2d1aa
@@ -35,8 +35,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)

@@ -209,46 +210,79 @@ 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: the proxy holds the real key, so it must never execute a
-script from the PR-head checkout.
+stripping. The CI lane deliberately keeps its own inline copy.

-1. **Start the proxy on the host** (after prepare has written `context.json`):
+Two checkouts are involved, and they are not interchangeable:
+
+- `<trusted-clone>` — a checkout of `main` (or another trusted revision) that
+  the PR cannot write. The proxy and the checker always run 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 runs here
+  on the host and refuses any other HEAD.
+
+Never start the key-holding proxy or the post-round checker from
+`<pr-clone>`: a PR can rewrite a script in its own tree, and those two steps
+run with the real key and the maintainer's `gh` access.
+
+1. **Prepare on the host, then copy the full snapshot out of the tree:**

   ```bash
-   REVIEW_OPENAI_BASE_URL=<upstream-url> REVIEW_OPENAI_API_KEY=<real-key> \
-     node scripts/verify-pr-model-proxy.mjs --listen 172.17.0.1
-   # => {"port":...,"baseUrl":"http://172.17.0.1:<port>","token":"...","nonce":"..."}
+   node <pr-clone>/scripts/verify-pr-prepare.mjs <n> --repo <owner>/<repo>
+   mkdir -p ~/.cache/qwen-verify/pr<n>
+   cp <pr-clone>/tmp/verify-context/pr<n>.json \
+     ~/.cache/qwen-verify/pr<n>/pr.json
+   ```
+
+   The artifact directory's `context.json` carries only the four coordinates;
+   the round reads the full gh snapshot (title, body, author, commits) from
+   `$QWEN_VERIFY_CONTEXT`. The host copy is also what pins the expected head
+   after the round, because PR code can rewrite anything inside `<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):
+
+   ```bash
+   read -rsp 'REVIEW_OPENAI_API_KEY: ' REVIEW_OPENAI_API_KEY; echo
+   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>&1 &
+   echo $! > ~/.cache/qwen-verify/pr<n>/proxy.pid
   ```

   Bind the Docker bridge address, not `127.0.0.1` — a container cannot reach
   the host's loopback. The bearer check stays mandatory: the port is visible
-   to every container on the bridge. The real key is never printed.
+   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.

-2. **Start the boundary**, mounting nothing credential-shaped:
+3. **Start the boundary as a named, detached container** so every later step
+   can be issued without a TTY:

   ```bash
-   docker run --rm -it \
-     -v <pr-worktree>:/work \
-     -v <round-dir>/context.json:/verify-context/pr.json:ro \
+   docker run -d --rm --name qwen-verify-<n> \
+     --user "$(id -u):$(id -g)" -e HOME=/tmp/qwen-home \
+     -v <pr-clone>:/work \
+     -v ~/.cache/qwen-verify/pr<n>:/verify-context:ro \
     -e QWEN_VERIFY_CONTEXT=/verify-context/pr.json \
-     -e QWEN_VERIFY_CREDENTIAL_FREE=1 \
     -e QWEN_VERIFY_PROXY_TOKEN=<token> \
-     -w /work node:22-bookworm bash
+     -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.
-   `QWEN_VERIFY_CREDENTIAL_FREE=1` remains the attestation — set it only
-   after confirming the mounts carry nothing.
+   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.

-3. **Point the provider at the proxy** in the container's fresh
+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:

-   ```json
+   ```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>" },
@@ -257,18 +291,57 @@ script from the PR-head checkout.
         {
           "id": "<model-id>",
           "envKey": "QWEN_VERIFY_PROXY_TOKEN",
-           "baseUrl": "<baseUrl printed by the proxy>"
+           "baseUrl": "<baseUrl from proxy.json>"
         }
       ]
     }
   }
+   JSON
   ```

-4. **Inside, the round's first command** is still
-   `node scripts/verify-pr-prepare.mjs --require-isolation` (exit 4 = not a
-   boundary). Then install the CLI, build as the local install strategy
-   requires, and run the round. Coordinate resolution and publishing stay on
-   the host: they need `gh`, which the boundary excludes.
+5. **Attest inside, after a check — never on the `docker run` line.** The
+   gate only sees known credential names and home paths, and gitignored files
+   in the round tree are invisible to its clean-tree check, so review the
+   mount list and reject credential-shaped ignored files first:
+
+   ```bash
+   docker exec -i qwen-verify-<n> bash -lc '
+     set -e
+     test -z "$(git -C /work status --porcelain --ignored -- .env ".env.*")" ||
+       { echo "ignored credential file in the round tree"; exit 1; }
+     export QWEN_VERIFY_CREDENTIAL_FREE=1
+     node /work/scripts/verify-pr-prepare.mjs --require-isolation
+   '
+   ```
+
+   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:
+
+   ```bash
+   docker exec -i qwen-verify-<n> bash -lc '
+     set -e
+     export PATH="$HOME/.local/bin:$PATH"
+     npm install -g --prefix "$HOME/.local" @qwen-code/qwen-code
+     # build the tree as the local install strategy requires, then:
+     qwen --auth-type openai --approval-mode yolo \
+       --prompt "/verify-pr <n> --repo <owner>/<repo>"
+   '
+   ```
+
+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)"
+   ```
+
+   `<round-dir>` is `<pr-clone>/tmp/pr<n>-verify-<ts>`. Publishing still
+   needs `gh`, so it stays on the host.

 ## Watch mode (local only)

@@ -481,7 +554,9 @@ 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`
@@ -0,0 +1,90 @@
+# 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 `POST /chat/completions`. Clients present a per-run bearer token
+  (`QWEN_VERIFY_PROXY_TOKEN`); the proxy swaps it for the real key upstream.
+  A nonce-echoing health endpoint distinguishes the process from a squatter.
+  Hop-by-hop request headers are stripped. The idle watchdog is per-chunk,
+  matches the qwen client's own 240 s stream tolerance, stays quiet while the
+  downstream is paused, and re-arms on drain; a disconnect before upstream
+  headers aborts the fetch instead of paying for a completion nobody reads.
+- The recipe uses two checkouts. `<trusted-clone>` runs the proxy and the
+  post-round checker; `<pr-clone>` is a standalone clone at the PR head that
+  the container mounts. A linked worktree is rejected by construction because
+  its gitfile points outside the mount.
+- The container is named and detached (`sleep infinity` plus `docker exec`),
+  runs as the host uid with `HOME=/tmp/qwen-home`, mounts the full snapshot
+  from a host copy outside the tree, and sets the credential-free attestation
+  inside only after rejecting credential-shaped ignored files. The CLI
+  installs into the fresh home, so nothing in the tree becomes root-owned.
+- 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, and the divergence is
+  limited to the listen flag, the JSON coordinates, and header stripping.
+- **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.
+
+## Acceptance
+
+- Proxies: bearer rejection, path allowlist, base-path prefixing, real-key
+  injection, hop-by-hop stripping with a chunked body, pre-header disconnect
+  abort, paused-downstream survival, slow-but-healthy streams, upstream reset
+  survival, IPv6 listen formatting, and the CLI's JSON coordinates.
+- Checker: `--context` overrides the writable artifact copy, refuses an
+  unusable snapshot instead of falling back, and keeps its existing exit
+  codes.
+- 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.
@@ -0,0 +1,40 @@
+# 容器化 /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`，只转发 `POST /chat/completions`。客户端出示一次性 bearer token（`QWEN_VERIFY_PROXY_TOKEN`），代理在上游替换为真实 key。回显 nonce 的健康端点用于区分本进程与抢占者。hop-by-hop 请求头被剥离。空闲看门狗按块续期，与 qwen 客户端自身的 240 秒流容忍一致；下游暂停期间不中止，drain 时续期；上游响应头之前客户端断开时中止 fetch，而不是为无人读取的补全付费。
+- 配方使用两个检出。`<trusted-clone>` 运行代理与轮次后的检查器；`<pr-clone>` 是停在 PR head 的独立克隆，供容器挂载。链接 worktree 因其 gitfile 指向挂载外而在构造上被排除。
+- 容器有名字且后台运行（`sleep infinity` 加 `docker exec`），以宿主 uid 运行、`HOME=/tmp/qwen-home`，从树外的宿主副本挂载完整快照，并在拒绝凭据形状的 ignored 文件之后才在容器内设置无凭据宣誓。CLI 安装进这个新 home，因此树里不会出现 root 属主文件。
+- 轮次从完整快照读取 `$QWEN_VERIFY_CONTEXT`。轮次结束后，检查器从 `<trusted-clone>` 运行并带 `--context <宿主副本>`，因此可写的产物 `context.json` 无法自证其期望 head。
+
+CI 车道保留内联代理：那份副本运行在 base-pinned workflow 的 root 步骤里，若改为指向 PR 检出，PR 就能读到真实 key。
+
+## 备选方案
+
+- **把真实 key 导出进容器。** 否决：边界的存在就是为了把它挡在外面，而 PR 代码能读取自己的环境。
+- **抽取 CI heredoc 并由 workflow 调用。** 对 CI 车道否决：脚本会来自 PR 检出。因此本地脚本是有意的第二份副本，差异仅限监听参数、JSON 坐标与请求头剥离。
+- **打印 `0.0.0.0`，或把监听地址归一化成 loopback。** 否决：打印值必须是客户端实际拨号的地址。
+- **仅把树挂成只读。** 否决：轮次必须在树里构建并写产物。信任边界靠「特权宿主步骤从 `<trusted-clone>` 运行」以及「从宿主快照钉住期望 head」来落实。
+
+## 验收
+
+- 代理：bearer 拒绝、路径白名单、base path 前缀、真实 key 注入、chunked 请求体下 hop-by-hop 剥离、响应头前断连中止、下游暂停存活、慢但健康的流、上游重置存活、IPv6 监听格式化，以及 CLI 的 JSON 坐标。
+- 检查器：`--context` 覆盖可写产物副本、不可用快照时拒绝而不是回退，并保持既有退出码。
+- 变异证据：移除上述任一守卫，对应 pin 必须变红。
+- 真实 Docker 冒烟：`QWEN_VERIFY_PROXY_TOKEN` 时门禁通过、`OPENAI_API_KEY` 时门禁拒绝、容器请求到达 mock 上游且真实 key 已注入。
@@ -417,6 +417,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.
@@ -97,6 +97,11 @@
 - 所有脚本都不执行 PR 代码。
 - 对于自带轮次快照的报告，在不同产物目录中准备后续轮次不会改变检查所用的提交号。可发布的轮次快照不包含 PR 标题或正文。

+## 相关设计
+
+- 容器边界与它的宿主机模型代理：
+  [verify-pr-local-model-proxy.zh-CN.md](verify-pr-local-model-proxy.zh-CN.md)。
+
 ## 未决问题

 本范围内无。