Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changes/pr-pipeline/feat-12232-capture-wiring-8f364a88.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
bump: minor
---

### Added

- `merge.verify-capture.proxy`: the key proxy the capture's driven Claude Code reaches the API through, with its token cap, model, `max_tokens`, body, beta and block allowlists, the driven session's models, and the Claude Code version the capture installs, which must equal `package-lock.json`'s pin.
- `pr-verify-capture`'s `capture` job starts the key proxy as its own OS user with the key on stdin before isolating the runner, then a sibling sandbox with no network and no secret, whose reach probe must also miss a sentinel credential planted outside it, and runs the driver's launch, ready and log there through the launch broker, so the pull request's plugin code runs with a dummy key. The sandbox writes only the run directory (the workspace, the head and main's checkout are read-only), sees an empty `/run` but for the proxy's socket directory, and can make no user namespace and hold no capability; its check fails the job unless those hold, no name resolves, and the proxy's is the only unix socket that answers. The probe also reads through links to `/proc/self/environ` and its fence file, and holds the driver's Write and Grep: a Write through a link to `~/.claude` and a Grep through the environ link must be refused too. The sandbox and the proxy are stopped whenever they started, the collect step copies nothing unless the sandbox stopped, and the run directory loses its top-level dot entries once it has. The proxy limits per job the requests that reach its count_tokens preflight (`max-preflight-requests`). `merge.verify-capture.app` stays unset until a runner probe passes.

### Changed

- `capture-subscription` reads no secret and runs no model: after the capture job's gates it uploads a capture recorded `blocked`, because a subscription token cannot go behind the key proxy.
- The capture's collect step copies no symlink and drops anything but regular files and directories before the upload.

### Removed

- The `capture-fallback` job, whose fallback token is a subscription token too.
62 changes: 56 additions & 6 deletions .github/actions/resolve-config/callers.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2230,11 +2230,54 @@ test("a job reads a Claude API key only when its catalog lane runs on run-claude
}
});

// The one step outside a launcher that may read a Claude credential:
// pr-verify-capture.yml's key proxy start, which pipes pr-verify's key on
// stdin to a proxy running as its own OS user before the runner is isolated,
// so the driven Claude Code and the pull request code beside it reach the API
// with a dummy key (owner decision Q6, option A). Only in this shape: that
// job, that step name, the key as its one env value PROXY_KEY, and no other
// secret reference anywhere in the step.
const KEY_PROXY_START = {
key: "pr-verify-capture.yml capture",
name: "Start the key proxy",
env: { PROXY_KEY: "${{ secrets.PR_VERIFY_CLAUDE_API_KEY }}" },
};
const isKeyProxyStart = (key, step) =>
key === KEY_PROXY_START.key &&
step.name === KEY_PROXY_START.name &&
step.uses === undefined &&
step.with === undefined &&
JSON.stringify(step.env) === JSON.stringify(KEY_PROXY_START.env) &&
[...secretRefs({ ...step, env: undefined })].length === 0;

test("a Claude credential reaches only the launcher step of its catalog lane", () => {
const jobs = claudeSecretJobs();
for (const { lane, key } of catalogLanes()) {
assert.deepEqual(strayClaudeReaders(jobs.get(key), lane.launcher), [], key);
}
const { doc, job } = jobs.get(key);
const steps = (job.steps ?? []).filter((step) => !isKeyProxyStart(key, step));
assert.deepEqual(strayClaudeReaders({ doc, job: { ...job, steps } }, lane.launcher), [], key);
}
});

test("the key proxy's start is the one step outside a launcher that reads a key, and only in its exact shape", () => {
const steps = workflow("pr-verify-capture.yml").jobs.capture.steps;
const start = steps.find((step) => step.name === KEY_PROXY_START.name);
assert.ok(start);
assert.equal(steps.filter((step) => isKeyProxyStart(KEY_PROXY_START.key, step)).length, 1);
// Without the exception it is a stray reader, so the exception is what lets it pass.
assert.deepEqual(strayClaudeReaders({ doc: {}, job: { steps: [start] } }, "run-claude-cli"), [`step ${KEY_PROXY_START.name}`]);
const stillStray = {
"another job": [KEY_PROXY_START.key.replace(" capture", " capture-subscription"), start],
"another workflow": [KEY_PROXY_START.key.replace("pr-verify-capture.yml", "pr-verify.yml"), start],
"another step name": [KEY_PROXY_START.key, { ...start, name: "Start the proxy" }],
"the key under another name": [KEY_PROXY_START.key, { ...start, env: { KEY: KEY_PROXY_START.env.PROXY_KEY } }],
"another secret as PROXY_KEY": [KEY_PROXY_START.key, { ...start, env: { PROXY_KEY: "${{ secrets.PR_VERIFY_CLAUDE_CODE_OAUTH_TOKEN }}" } }],
"a second env value": [KEY_PROXY_START.key, { ...start, env: { ...start.env, OTHER: "x" } }],
"a secret in the script": [KEY_PROXY_START.key, { ...start, run: `${start.run}\necho "\${{ secrets.FALLBACK_CLAUDE_CODE_OAUTH_TOKEN }}"` }],
"every secret in the script": [KEY_PROXY_START.key, { ...start, run: `${start.run}\necho '\${{ toJSON(secrets) }}'` }],
"an action": [KEY_PROXY_START.key, { ...start, uses: "./x" }],
};
for (const [why, [key, step]] of Object.entries(stillStray)) assert.equal(isKeyProxyStart(key, step), false, why);
});

test("the Claude credential scan sees every form a secret can be read in", () => {
Expand Down Expand Up @@ -3156,7 +3199,6 @@ test("API_BUDGET_HOLD is set step by step, only where the script holds, and neve
"pr-rate-merge-risk.yml rate-merge-risk: Resolve the pull request",
"pr-refine.yml fix-docs-head: Read the PR's head SHA",
"pr-unblock-dispatch.yml dispatch: Dispatch pr-unblock for conflicting, review-tier Dependabot, failing lanes App and Markdown-changing PRs",
"pr-verify-capture.yml capture-fallback: Resolve the pull request",
"pr-verify-capture.yml capture-subscription: Resolve the pull request",
"pr-verify-capture.yml capture: Resolve the pull request",
"pr-verify.yml check-intent-fallback: Resolve the pull request",
Expand Down Expand Up @@ -9674,16 +9716,24 @@ function missingLegs(catalog, launched) {
}
return missing;
}
// Lanes that cannot take every leg yet (#13586): each names the exact legs it
// waives and why. The test fails when an exempt lane lacks a leg its entry
// Lanes that cannot take every leg yet (#13586), or never can by the owner's
// decision (#12232): each names the exact legs it waives and why, ending in
// its tracking issue. The test fails when an exempt lane lacks a leg its entry
// does not name, or runs a leg its entry waives, so an entry never covers
// more than it says and goes once its lane gains the leg.
const LEG_EXEMPTIONS = {
"pr-verify-capture.yml pr-verify capture": {
legs: ["subscription", "fallback"],
reason:
"the capture runs on the key alone and records blocked otherwise (owner decision Q5): its driven Claude Code reaches the API only through a key proxy we run, and a subscription token may not be routed through one, so the lane never takes these legs (#12232)",
},
"pr-rate-merge-risk.yml pr-verify rate-merge-risk": {
legs: ["subscription", "fallback"],
reason: "a report-only rating (#13657, owner decision Q57): a key-leg stop records the Merge-Risk line unrated, which a subscription or fallback leg would not change for any merge (#13586)",
},
};
// The issues an exemption may name: the full-chain tracker, or the capture's permanent key-only decision.
const EXEMPTION_ISSUE = /\(#(?:13586|12232)\)$/;
// A secret name's leg: the role's API key, its subscription token, or the
// central fallback token.
const legOf = (name) => (name === FALLBACK_SECRET ? "fallback" : /_CLAUDE_API_KEY$/.test(name) ? "key" : "subscription");
Expand All @@ -9696,7 +9746,7 @@ function exemptionProblems(missing, exemptions, lanes) {
}
for (const [key, { legs, reason }] of Object.entries(exemptions)) {
if (!lanes.has(key)) problems.push(`${key} is not a catalog lane`);
if (!/\(#13586\)$/.test(reason ?? "")) problems.push(`${key} names no reason and tracking issue`);
if (!EXEMPTION_ISSUE.test(reason ?? "")) problems.push(`${key} names no reason and tracking issue`);
const lacking = new Set((missing.get(key) ?? []).map(legOf));
for (const leg of legs) if (!lacking.has(leg)) problems.push(`${key} now runs its ${leg} leg: drop it from the exemption`);
}
Expand Down
2 changes: 0 additions & 2 deletions .github/lane-funding.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@
{ "id": "check-intent-subscription", "workflow": "pr-verify.yml", "launcher": "run-claude-cli", "triggers": ["workflow_run"], "gate": "same-repo", "currentSecrets": ["PR_VERIFY_CLAUDE_CODE_OAUTH_TOKEN", "FALLBACK_CLAUDE_CODE_OAUTH_TOKEN"] },
{ "id": "check-intent-fallback", "workflow": "pr-verify.yml", "launcher": "run-claude-cli", "triggers": ["workflow_run"], "gate": "same-repo", "currentSecrets": ["FALLBACK_CLAUDE_CODE_OAUTH_TOKEN"] },
{ "id": "capture", "workflow": "pr-verify-capture.yml", "launcher": "run-claude-cli", "triggers": ["workflow_run"], "gate": "same-repo", "currentSecrets": ["PR_VERIFY_CLAUDE_API_KEY"] },
{ "id": "capture-subscription", "workflow": "pr-verify-capture.yml", "launcher": "run-claude-cli", "triggers": ["workflow_run"], "gate": "same-repo", "currentSecrets": ["PR_VERIFY_CLAUDE_CODE_OAUTH_TOKEN", "FALLBACK_CLAUDE_CODE_OAUTH_TOKEN"] },
{ "id": "capture-fallback", "workflow": "pr-verify-capture.yml", "launcher": "run-claude-cli", "triggers": ["workflow_run"], "gate": "same-repo", "currentSecrets": ["FALLBACK_CLAUDE_CODE_OAUTH_TOKEN"] },
{ "id": "rate-merge-risk", "workflow": "pr-rate-merge-risk.yml", "launcher": "run-claude-cli", "triggers": ["workflow_run"], "gate": "same-repo", "currentSecrets": ["PR_VERIFY_CLAUDE_API_KEY"] }
]
},
Expand Down
12 changes: 11 additions & 1 deletion .github/scripts/pr-verify/key-proxy.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// maxBodyBytes largest request body accepted
// maxInFlight concurrent forwarded requests, count_tokens included
// maxCountTokensRequests client count_tokens requests allowed per job
// maxPreflightRequests /v1/messages requests per job that may reach the count_tokens preflight
// betas anthropic-beta values a request may name
// bodyKeys top-level request body keys a request may carry
// blockTypes content block types allowed in messages and system
Expand Down Expand Up @@ -127,6 +128,7 @@ export function loadConfig(raw, upstreamOverride) {
maxBodyBytes: positiveInt("maxBodyBytes"),
maxInFlight: positiveInt("maxInFlight"),
maxCountTokensRequests: positiveInt("maxCountTokensRequests"),
maxPreflightRequests: positiveInt("maxPreflightRequests"),
upstreamTimeoutMs: positiveInt("upstreamTimeoutMs"),
betas: stringList(c, "betas"),
bodyKeys: nonEmpty("bodyKeys"),
Expand Down Expand Up @@ -335,6 +337,14 @@ export const CHECKS = Object.freeze([
},
async function cap(ctx) {
if (ctx.path === COUNT_TOKENS) return null;
// Its max_tokens alone already passes the cap: refused before the preflight,
// so a spent budget makes no further upstream call (our decision).
if (ctx.budget.committed + ctx.budget.reserved + ctx.body.max_tokens > ctx.budget.cap) return spendLimit("per-job token cap");
// Each preflight is an upstream call the cap does not count, and one refused
// at reserve costs the client nothing, so preflights are limited per job too
// (our decision).
if (ctx.state.preflights >= ctx.config.maxPreflightRequests) return spendLimit("per-job request limit");
ctx.state.preflights += 1;
const pre = await ctx.preflight(ctx);
// A failure the client would retry against the API itself (a 429 or a
// 5xx, relayed as sent; no answer, as a retryable 502) goes back to the
Expand Down Expand Up @@ -381,7 +391,7 @@ const relayHeaders = (incoming) => Object.fromEntries(Object.entries(incoming).f
export function createProxy({ key, config, log = () => {} }) {
if (typeof key !== "string" || !key) throw new Error("key is required");
const budget = createBudget(config.tokenCap);
const state = { inFlight: 0, countTokensRequests: 0 };
const state = { inFlight: 0, countTokensRequests: 0, preflights: 0 };
const lib = config.upstream.protocol === "https:" ? https : http;
const basePath = config.upstream.pathname.replace(/\/$/, "");
const upstreamPath = (ctx, path) => basePath + path + (ctx.query === ALLOWED_QUERY ? `?${ALLOWED_QUERY}` : "");
Expand Down
44 changes: 43 additions & 1 deletion .github/scripts/pr-verify/key-proxy.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const BODY_KEYS = ["context_management", "max_tokens", "messages", "metadata", "
const BLOCK_TYPES = ["text", "image", "document", "tool_use", "tool_result", "thinking", "redacted_thinking"];
const BASE_CONFIG = {
tokenCap: 1_000_000, models: MODELS, maxTokensCeiling: 1000, maxBodyBytes: 100_000, maxInFlight: 8,
maxCountTokensRequests: 100, betas: BETAS, bodyKeys: BODY_KEYS, blockTypes: BLOCK_TYPES,
maxCountTokensRequests: 100, maxPreflightRequests: 1000, betas: BETAS, bodyKeys: BODY_KEYS, blockTypes: BLOCK_TYPES,
// Longer than any test holds an upstream idle; the timeout tests override it.
upstreamTimeoutMs: 10_000,
};
Expand Down Expand Up @@ -714,6 +714,48 @@ test("past the cap the refusal is a 429 spend limit with x-should-retry false",
assert.equal(up.seen.length, 1);
});

test("a request whose max_tokens alone passes the spent cap is refused before any count_tokens preflight", async () => {
const up = await stubUpstream(okJson({ input_tokens: 475, output_tokens: 475 }), { countTokens: 10 });
const { socket, budget } = await startProxy(up, { tokenCap: 1000, maxTokensCeiling: 100 });
// Reserves 10 + 100, then commits 475 + 475 = 950.
assert.equal((await call(socket, { body: message() })).status, 200);
assert.equal(budget.committed, 950);
assert.equal(up.counts.length, 1);
// 950 + 100 = 1050 > 1000: refused with no upstream call at all.
assertRefused(await call(socket, { body: message({ max_tokens: 100 }) }), 429, "rate_limit_error");
assert.equal(up.counts.length, 1);
// 950 + 50 = 1000 is not past the cap, so the preflight runs; 950 + 10 + 50 = 1010 then is.
assertRefused(await call(socket, { body: message({ max_tokens: 50 }) }), 429, "rate_limit_error");
assert.equal(up.counts.length, 2);
assert.equal(up.seen.length, 1);
});

test("preflights are limited per job, so requests refused at reserve cannot repeat the count_tokens call without end", async () => {
// Each preflight counts 900 input tokens; 900 + max_tokens 200 = 1100 is past the cap of 1000, so every reserve fails.
const up = await stubUpstream(okJson(), { countTokens: 900 });
const { socket } = await startProxy(up, { tokenCap: 1000, maxTokensCeiling: 200, maxPreflightRequests: 3 });
for (let i = 1; i <= 3; i++) {
assertRefused(await call(socket, { body: message({ max_tokens: 200 }) }), 429, "rate_limit_error");
assert.equal(up.counts.length, i);
}
// The fourth is refused before its preflight: three upstream calls in all, none to /v1/messages.
const fourth = await call(socket, { body: message({ max_tokens: 200 }) });
assertRefused(fourth, 429, "rate_limit_error");
assert.match(JSON.parse(fourth.body).error.message, /^spend limit reached \(per-job request limit\)$/);
assert.equal(up.counts.length, 3);
assert.equal(up.seen.length, 0);
});

test("a request that passes counts toward the preflight limit too", async () => {
const up = await stubUpstream(okJson(), { countTokens: 10 });
const { socket } = await startProxy(up, { maxPreflightRequests: 2 });
assert.equal((await call(socket, { body: message() })).status, 200);
assert.equal((await call(socket, { body: message() })).status, 200);
assertRefused(await call(socket, { body: message() }), 429, "rate_limit_error");
assert.equal(up.counts.length, 2);
assert.equal(up.seen.length, 2);
});

// Runs the script as the workflow will.
function spawnProxy(args, stdin) {
const child = spawn(process.execPath, [SCRIPT, ...args], { stdio: ["pipe", "pipe", "pipe"], env: { PATH: process.env.PATH } });
Expand Down
Loading
Loading