feat(pr-verify): add the key proxy for the driven capture session by kyle-sexton · Pull Request #13662 · melodic-software/claude-code-plugins

kyle-sexton · 2026-11-06T05:15:11Z

Summary

PR C Phase 2 of #12232. The pr-verify UI capture will run the driven Claude Code inside a sibling sandbox holding only a dummy key. This adds the process that holds the real key outside that sandbox: a stdlib Node HTTP proxy on a unix socket that swaps the dummy credential for the key and lets through only the traffic a capture session sends, within a per-job token cap. Nothing runs it yet; Phase 5 wires it into pr-verify-capture.yml.

Fix

.github/scripts/pr-verify/key-proxy.mjs (new) with key-proxy.test.mjs:

Reads the key from stdin only (never argv or env) and logs neither the key nor a request body.
Allows POST /v1/messages and /v1/messages/count_tokens, with ?beta=true only when the client sent exactly that; everything else is refused.
Allowlists from config, measured on Claude Code 2.1.318's driven-session traffic: models, anthropic-beta values (a refusal names the betas to add), top-level body keys, content block types and source types (url sources and server tools are refused, since they make the API reach a PR-chosen host). Bodies are re-serialized from the checked object, so duplicate keys cannot smuggle a value past the check.
Spend: each main request is preflighted through count_tokens and reserves its input tokens plus max_tokens; maxInFlight and a per-job count_tokens request limit bound concurrency; usage is summed from the stream without double counting; past the cap the answer is a 429 with x-should-retry: false. A request whose body was fully sent upstream commits its reservation even if it then fails or stalls before any response (it may still be billed, and Claude Code retries it); it is released only when the failure came before the body was sent.
An upstreamTimeoutMs (required) aborts a stalled upstream with a retryable answer and settles its slot; upstream always gets accept-encoding: identity; only a fixed set of response headers is relayed; an oversized body gets 413 and the connection is closed.
Comments that give upstream behavior as a reason carry a docs pointer, an as-of date and a recheck trigger, or state our own decision.

No changelog fragment: no plugin changed.

Verification

node --test .github/scripts/pr-verify/key-proxy.test.mjs: 35 tests, 35 pass; expected values are hand-computed literals.
Fresh-context phase verifier rounds: C1-C6 passed at a266b2b; the last round passed B1-B10 and T at d9c856b, with two comment wordings fixed after it (2760bfb). It found no path that lets PR code spend past the cap or reach the key.
A security review of the proxy drove the canonical bodies, the preflight reservation, the allowlists and the response header filter.
scripts/check-affected.sh: 9 passed on the pushed head.

ci(pr-verify): add the capture job that drives the affected app (S7 of #8651) #12232 (UI capture of PR plugin code); PR C Phase 4 merged as feat(control-claude-code-plugins): map hook and mod changes to UI-surface features #13638; the launch broker (Phase 3) is feat(harness-ops): add the driver's launch broker and broker mode #13661.
Phase 5 runs this proxy as its own OS user before the runner is isolated, with the allowlists and cap in docs/conventions/pr-pipeline.yaml.

Follow-ups

Pre-existing (advisory): a reused keep-alive upstream connection that resets after the body is written commits a reservation for a request upstream never ran; this only over-counts, so the cap holds.

🤖 Generated with Claude Code

A stdlib Node HTTP server on a unix socket that holds the API key (read from stdin) and is the only path from the driven Claude Code to the Messages API. An ordered check pipeline (path, size, parse, model, max_tokens, server tools, cap reservation) refuses with statuses Claude Code does not retry; allowed requests go upstream with the credential replaced, the body and anthropic-* headers unchanged, and the response relayed unbuffered. One budget tracks committed and in-flight reserved tokens; usage follows the cumulative message_delta counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

An upstream socket that closed after the response headers left the client open until Claude Code's stall watchdog fired, with the reservation still held. The proxy now drops the client and commits the reservation, as for a client disconnect. Also tests the socketMode option and drops wording from the server-tool refusal that implied a retry path a 403 does not take. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A content block whose source.type is "url" makes the API fetch a URL the client chose, an egress path out of the no-network sandbox. The proxy now refuses such a block anywhere in messages or system, at any depth and tool_result content included, with the same 403 as server tools. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- Forward the re-serialized checked object (fatal UTF-8 decode), so a duplicate key cannot smuggle a value past the checks. - Reserve the upstream count_tokens preflight plus max_tokens; add maxInFlight and a per-job client count_tokens limit, refuse count_tokens once the cap is spent, and commit the reservation when a stream ends without message_delta. - Allowlist request headers (unlisted betas refused), top-level body keys, content block and source types, the beta=true query and relayed response headers. - Destroy the request on an oversized body. - Cite a pointer for each upstream behavior the code relies on, and test with hand-computed values. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A preflight 429, 5xx or network error now reaches the client with its status, retry headers and body, so Claude Code's own retry covers an upstream blip; the in-flight slot is freed and nothing is reserved. Any other preflight 4xx stays a non-retry refusal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…y proxy - An upstream failure before any response byte (no answer, a dropped connection, a stall) is answered 502 with x-should-retry true, on the main request and the count_tokens preflight alike; policy refusals keep x-should-retry false. - New required config key upstreamTimeoutMs: an upstream socket idle that long is aborted. Before any byte is relayed the reservation is released; after, the client is dropped and the reservation committed, as for a client disconnect. The in-flight slot is freed either way. - Every upstream request now sends accept-encoding: identity. Before, none was sent, which by RFC 9110 12.5.3 accepts any coding, so a compressed body could hide usage. The header-allowlist test now asserts identity upstream instead of an absent header, a stronger check. - A tools field that is not a list, or a tools entry that is not an object, is refused. - A beta refusal names the refused betas (up to ten) in the response body and the log line. - Comments giving upstream behavior as a reason carry a pointer, as-of date and recheck trigger, or are stated as our decision. Drop the unused MESSAGES constant. - Tests for each, plus a 429 and a dropped-connection preflight. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…sources Give the test file's count_tokens, streaming and spend-limit comments a pointer, as-of date and recheck trigger, and bump the two proxy pointers re-read on 2026-11-06. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…the body is sent A non-streaming response sends nothing until generation ends, and Claude Code resends a retryable failure, so a timeout or upstream error before response headers could leave billed generations uncounted. Once the body has been fully written upstream, such a failure now commits the full reservation; it is released only when the connection failed before the body was written. Both still get the retryable 502. The two tests that expected a release after the upstream had read the body now expect the commit (150 and 650, hand-computed from the counted input plus max_tokens); a new test shows a connect failure releases it. Document how to size upstreamTimeoutMs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector · 2026-11-06T05:17:07Z

Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	❌ Failed 2026-11-06T05:17:04.918428Z We couldn’t complete this review. Comment `@codex review` to try again.	`a2b7dd0`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-11-06T05:17:09Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

github-actions · 2026-11-06T05:20:03Z

claude-security-review reviewed 480a6b94515394f545e03c665f0e62c54966e34a: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental since `b1fcd70`): no security issues found

critical-findings: 0

Scope. This review covers only the push since b1fcd70e055c. I read every hunk of both files on the incremental list: .github/scripts/pr-verify/key-proxy.mjs and .github/scripts/pr-verify/key-proxy.test.mjs. Neither file matches a TRUSTED PATHS: glob. The push changes no trust control, merge hold, plugin manifest or standing instruction. This lane already reviewed an earlier head, so the review was blocking only: it reports CRITICAL and IMPORTANT findings and drops SUGGESTION ones.

What changed. The request end handler now runs its work in a try/catch (key-proxy.mjs#L462-L497). This fixes the crash from an unhandled rejection that the settled thread at L462 raised. The owner fixed it at this head, so I am not raising it again.

What I checked (no exploitable path found):

Can the catch release spend that was already sent upstream? The catch settles the reservation at 0. This could under-count only if a throw reached the catch after the main request body went upstream. It can't:
- Inside forward, the only work that runs synchronously before upReq.end(body) is JSON.stringify(ctx.body) (L512) and building the request. upReq.end(body) is the last statement (L586).
- Every settle that happens after the body is sent runs in an event callback outside the try.
- In the preflight, a throw inside the executor (L409) happens before anything is sent. Its upstream call is count_tokens in any case, not a generation.
- The per-job token cap still holds on this path.
Settling or releasing twice. settle is guarded by the reservation's settled flag (L151-L156), and release by released (L328-L333). A throw after a partial refusal can't double-count the budget or push the in-flight counter below zero.
Response handling. The proxy answers with a 400 only when !clientGone && !res.headersSent. Otherwise it destroys the response, so no second writeHead can throw.
Logging. The catch logs only err.name, not the message, stack or body. The key and request bodies stay out of the log.
Who can reach it. The only client is the capture sandbox on the proxy's unix socket. Before this change, a malformed body could take the per-job proxy down. Now it gets a non-retried 400. That improves availability and opens no leak, compromise or charge path.

The new test sends a deeply nested tool_use input to a local stub upstream. It reaches no external service.

No hold: nothing here can cause a real charge, and there is no CRITICAL finding.

github-actions

claude-review: 1 inline finding(s) on a2b7dd058eeb11d0861f0ef621d5541bc353b47f; the summary is in this lane's pull request comment.

claude-lane: claude-review

github-actions · 2026-11-06T05:20:32Z

claude-review reviewed 480a6b94515394f545e03c665f0e62c54966e34a: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental, blocking only): `try`/`catch` around the key proxy's request handler

Scope. This covers the change since b1fcd70e, in both listed files: .github/scripts/pr-verify/key-proxy.mjs and .github/scripts/pr-verify/key-proxy.test.mjs. I read every hunk of both. This lane already reviewed an earlier head, so I reported blocking findings only.

The fix for the earlier crash finding. The end handler now calls handle(ctx) inside a try/catch. That closes this lane's earlier finding that a throw in a check could kill the proxy process. Its thread is settled with no line changes since resolution. I checked the following by reading the code:

Release is idempotent. ctx.release sets a released guard, and a reservation's settle sets a settled guard (createBudget). If both the handle refusal path and the catch run, the in-flight count is still released only once and the budget is settled only once.
"Nothing went upstream" holds. A JSON.stringify overflow in preflight happens before the count_tokens request is created. In forward, the stringify at line 512 runs before upstreamRequest. So settle(0) frees a reservation for a main request that upstream never received.
Each client state gets one answer.
- Client still connected and no headers sent: a non-retried 400 through sendRefusal.
- Headers already sent: res.destroy().
- Client gone: nothing is written.
- In every case done logs the error name only, never the request body.
One known gap, not blocking. Throws inside forward's later upstream event callbacks are outside this try, since they run asynchronously. None of the paths the earlier finding named lead there.

The new test. It sends a tool_use input nested 50000 arrays deep and expects a 400. A normal request after it should get a 200, with exactly one main request seen upstream. That matches the earlier finding's reproduction. I did not run the suite: the 37/37 result is the author's claim and is unverified.

Other checks.

Closing issue: none. The PR refs ci(pr-verify): add the capture job that drives the affected app (S7 of #8651) #12232 but closes nothing, so there are no acceptance criteria to check.
Path rules: no .claude/rules rule matches these files.
Security: a separate security lane runs, so this review leaves security findings to it.
Embedded instructions: none found in the PR context, settled threads or prior findings.

Spend: none. Nothing runs this proxy yet.

Answering at once closed the socket while the client was still uploading, so it saw a reset, which Claude Code retries, instead of the non-retried 413. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions

claude-review: 1 inline finding(s) on b1fcd70e055cad0e332f287e90daffd41aa059a1; the summary is in this lane's pull request comment.

claude-lane: claude-review

… of exiting A body that passed the checks but overflowed the stack on re-serialization rejected the async end handler, and the unhandled rejection ended the proxy for the whole capture job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-11-06T05:51:55Z

Post-merge mutation testing

Range: 9d05599..085df29
Merged pull request: #13662
Outcome: reported

Killed 5, survived 2, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 71%; mutation score 71%.

File	Killed	Survived	Timeout	No coverage	Invalid	Not run
`.github/scripts/pr-verify/key-proxy.mjs`	5	2	0	0	0	0

Surviving mutants

Location	Operator	Disposition	Cause
`.github/scripts/pr-verify/key-proxy.mjs:149`	ROR	productive	input-gap
`.github/scripts/pr-verify/key-proxy.mjs:441`	ROR	productive	input-gap

The run's summary has the full report.

kyle-sexton and others added 10 commits November 5, 2026 19:02

docs(pr-verify): word two key proxy reasons as our decisions

2760bfb

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge branch 'main' into feat/12232-key-proxy

a2b7dd0

kyle-sexton marked this pull request as ready for review November 6, 2026 05:17

github-actions Bot reviewed Nov 6, 2026

View reviewed changes

Comment thread .github/scripts/pr-verify/key-proxy.mjs Resolved

fix(pr-verify): drain an oversized body before the key proxy's 413

b1fcd70

Answering at once closed the socket while the client was still uploading, so it saw a reset, which Claude Code retries, instead of the non-retried 413. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions Bot reviewed Nov 6, 2026

View reviewed changes

Comment thread .github/scripts/pr-verify/key-proxy.mjs Resolved

kyle-sexton added this pull request to the merge queue Nov 6, 2026

Merged via the queue into main with commit 085df29 Nov 6, 2026
43 checks passed

kyle-sexton deleted the feat/12232-key-proxy branch November 6, 2026 05:46

kyle-sexton mentioned this pull request Nov 6, 2026

feat(pr-pipeline): run the capture's driven Claude Code behind the key proxy in a sibling sandbox #13684

Merged

This was referenced Nov 6, 2026

fix(pr-verify): route the capture's outer leg through the key proxy and narrow its Bash #13710

Open

perf(pr-verify): set the capture key proxy's token-cap from a full measured capture #13724

Open

Navigation Menu

Repository navigation

feat(pr-verify): add the key proxy for the driven capture session - #13662

feat(pr-verify): add the key proxy for the driven capture session#13662
kyle-sexton merged 12 commits into
mainfrom
feat/12232-key-proxy

kyle-sexton commented Nov 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 6, 2026

Uh oh!

github-actions Bot commented Nov 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

kyle-sexton commented Nov 6, 2026

Summary

Fix

Verification

Related

Follow-ups

Uh oh!

chatgpt-codex-connector Bot commented Nov 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 6, 2026

Uh oh!

github-actions Bot commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review (incremental since b1fcd70): no security issues found

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review (incremental, blocking only): try/catch around the key proxy's request handler

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2026

Post-merge mutation testing

Surviving mutants

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Nov 6, 2026 •

edited

Loading

Security review (incremental since `b1fcd70`): no security issues found

github-actions Bot commented Nov 6, 2026 •

edited

Loading

Code review (incremental, blocking only): `try`/`catch` around the key proxy's request handler