mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
Shaojin WenandQwen-Coder
68c9fa27fb
* 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 at70aed891bf). 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 at3509c2d1aareported 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 at3ccf38697dreported 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 at441a523927: - 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 at19b14b1a8fleft 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 atb5c717a5d5: - 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]>