[kyle-sexton]

Refs: #12232

Summary

PR C Phase 5 of #12232: wire the key proxy (#13662) and the launch broker (#13661) into pr-verify-capture.yml, so the UI capture of a hooks or mods change drives the PR's plugin code inside a sibling sandbox that holds only a dummy key. The capture stays off: merge.verify-capture.app is unset until a runner probe (Phase 5b) passes on a hosted runner.

Fix

• capture job:
  • Before "Isolate the runner": creates a ccd-proxy OS user and root-owned /opt/ccd. Installs the Claude Code version the root lock pins, from a pruned lock with npm ci and npm audit signatures. Installs and checks the driver engine. Writes the driven managed settings and the proxy config from main's pr-pipeline.yaml. Starts the proxy as ccd-proxy under env -i, with the key on stdin only.
  • After isolation:
    • A sentinel credential outside the sandbox.
    • The sibling bwrap under env -i (PATH, HOME, TERM, LANG, CCD_RUNTIME_DIR), with --unshare-pid --proc /proc --unshare-net --unshare-ipc --unshare-uts --unshare-user --disable-userns --cap-drop ALL. tmpfs over /tmp, $HOME, $RUNNER_TEMP and /run, with only the proxy socket directory bound back. The workspace, .base and head are read-only; only capture-run and the runtime dir are writable.
    • A socat bridge from loopback to the proxy socket.
  • "Check the sibling sandbox" fails the job closed unless all of these hold:
    • The reach probe misses the sentinel.
    • PID 1's environment is exactly the allowlist.
    • Egress and name resolution fail.
    • The proxy's socket is the only reachable unix socket.
    • Writes land only in capture-run.
    • User namespaces are refused.
    • The broker listens.
  • The outer probe also reads, writes and greps through planted links, and must be refused.
  • The driver step reaches the broker through CCD_RUNTIME_DIR and CCD_BROKER_SOCK.
  • The proxy and the sandbox are stopped whenever they started; the sandbox stop fails if the sandbox survives. Collect copies nothing unless the stop succeeded, copies no symlink, and drops the run directory's dot entries first.
• capture-subscription reads no secret and runs no model: after its gates it uploads a capture recorded blocked (reason: a subscription token cannot go behind the key proxy, R41). capture-fallback is removed, with its lane-funding.json rows, and a permanent leg exemption in callers.test.mjs cites the owner's key-only decision (ci(pr-verify): add the capture job that drives the affected app (S7 of #8651) #12232 Q5).
• merge.verify-capture.proxy in pr-pipeline.yaml and the schema: token cap, models, max_tokens ceiling, body size, in-flight limit, count_tokens and preflight limits, upstream timeout, and the beta, body-key and block-type allowlists measured on Claude Code 2.1.318. A test ties claude-code-version to the lock pin, so a Claude Code bump fails until the betas are re-measured.
• key-proxy.mjs: a request is refused before its count_tokens preflight once the cap is spent or a per-job preflight limit is reached.
• pr-pipeline changelog fragment.

Security & Cost

• The real key reaches three steps: the run-claude-cli probe, the driver (both as before), and "Start the key proxy" (new). The proxy step runs before isolation and passes the key on stdin to a process of another uid. callers.test.mjs's credential rule gets a narrow, shape-pinned exception for it. No PR code runs in any process that holds the key or above it.
• PR code runs in the driven session inside the sandbox with a ccd-dummy- key. It can spend promotional key credit only through the proxy, bounded by the per-job token cap and request limits (the owner's option A, ci(pr-verify): add the capture job that drives the affected app (S7 of #8651) #12232 issuecomment-6671043587).
• Cost: no paid key. The capture uses PR_VERIFY_CLAUDE_API_KEY's promotional credit and records blocked otherwise (owner Q5). No new runners or paid services.
• Accepted, recorded: socket answers and capture files carry no integrity (PR code can forge them). The outer capture claude still holds the real key (owner Q7, open).

Verification

• node --test .github/scripts/pr-verify/workflow.test.mjs: 73/73. .github/actions/resolve-config/callers.test.mjs: 264/264. .github/scripts/pr-verify/key-proxy.test.mjs: 40/40. shellcheck and actionlint clean; check-jsonschema on pr-pipeline.yaml ok; scripts/check-affected.sh 12 passed.
• Fresh-context acceptance check against the plan: V2-V9 pass. Security review: no CRITICAL. Its HIGH (sandbox could write the outer leg's workspace) and MEDIUM findings (host unix sockets, symlinked reads) were fixed and re-reviewed CLOSED; its two PARTIALs (write/grep through links, unbounded preflights) were fixed in a second round.
• Local bwrap 0.11.1 with these flags: /run empty, DNS refused, user namespaces refused, no sockets but the proxy's. Not verifiable here, left to Phase 5b on ubuntu-24.04: the pruned-lock npm ci, bubblewrap's support for --disable-userns, that actions/checkout fills the pre-made head the sandbox binds read-only, and the full proxy, sandbox and broker run.

Related

• ci(pr-verify): add the capture job that drives the affected app (S7 of #8651) #12232; PR C Phases 2-4: feat(pr-verify): add the key proxy for the driven capture session #13662, feat(harness-ops): add the driver's launch broker and broker mode #13661, feat(control-claude-code-plugins): map hook and mod changes to UI-surface features #13638.
• Phase 5b: a push-triggered throwaway runner probe with a stub proxy and no secrets, then a one-line PR that sets merge.verify-capture.app.

Follow-ups

• Pre-existing (advisory): several new workflow.test.mjs assertions take their expected values from the workflow text (step if strings, launcher counts), which makes them change detectors.
• Pre-existing (advisory): cp -P in collect guards only the last path component; the sandbox is stopped before collect, which bounds it.
• Owner question Q9 (not blocking): narrow the outer key-holding leg's Bash to the driver and git commands it needs, since it reads forgeable sandbox output.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

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-06T07:50:03.415633Z We couldn’t complete this review. Comment @codex review to try again.	e134330	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]

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

[github-actions]

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

Security review: PR #13684 (capture key proxy and sibling sandbox)

No security issues found. This was a full read of all 11 changed files, every hunk. This lane already reviewed an earlier head (e134330), so this pass reported blocking findings only. The new head aea2e1c merges origin/main into the branch.

critical-findings: 0

What I checked

• Where the real key goes. PR_VERIFY_CLAUDE_API_KEY reaches one new step: "Start the key proxy" (pr-verify-capture.yml#L1405-L1427 in diff terms).
  • That step runs before "Isolate the runner".
  • The key is piped with the shell builtin printf into env -u PROXY_KEY sudo -n -u ccd-proxy … env -i … /opt/ccd/bin/node key-proxy.mjs. The proxy runs as a separate uid from root-owned copies of node, the script and the config.
  • The key appears in no argument, no environment variable a later step inherits, and no file.
  • The exception in callers.test.mjs is pinned to that exact shape: the job, the step name, PROXY_KEY as its only env value, and no other secret reference.
  • capture-subscription no longer references any secret, and capture-fallback is removed. Both changes narrow where credentials are held.
• Pull request code in the sibling sandbox.
  • The sandbox starts under env -i with five names only.
  • It runs bwrap with --ro-bind / /, its own PID, network, IPC, UTS and user namespaces, plus --disable-userns and --cap-drop ALL.
  • Empty tmpfs mounts cover /tmp, /run, $HOME and $RUNNER_TEMP. Only the proxy socket directory is bound back.
  • Only capture-run and the runtime directory are writable.
  • "Check the sibling sandbox" fails the job closed before any head code is checked out, and it has no continue-on-error.
• Outer driver leg (holds the key).
  • This leg reads the runtime directory and capture-run, which the sandbox can write. Pull request code could forge what it finds there; the owner has accepted that risk (Q7/Q9).
  • It is no wider than the base workflow, where the app under test ran directly as a Bash child of that same key-holding leg. The reach probe still checks that position.
  • The probe now holds Write and Grep only with the driver's own Edit(./head/**)/Edit(./capture-run/**) allowances. It runs before any head code and must be refused through the planted links. The links are removed before the head checkout.
• Collect and cleanup.
  • Collect runs only after the sandbox stop succeeded.
  • It uses cp -P, re-checks that each copied file is a regular file, reads publishable from the copied summary, and deletes anything that is not a regular file or directory.
  • The stop step removes the top-level dot entries the sandbox could have planted.
• Proxy.
  • The upstream is fixed to the API, and the config has no upstream override (the test asserts upstream is undefined).
  • The new cap pre-check and maxPreflightRequests only reduce upstream calls.
  • "Report the key proxy's usage" prints only numbers jq computes from the proxy log. That log is owned by ccd-proxy, and the sandbox mounts its directory read-only.
• Trust controls.
  • The trusted-trigger check, the lane token request, the kill switch and the merge-hold files are unchanged.
  • docs/conventions/pr-pipeline.yaml adds only merge.verify-capture.proxy. That is not a lane slot, an activity effect or a merge setting, so nothing widens.
  • The lane-funding.json rows removed are for the dropped legs, which narrows the funding list.
• Plugin manifests: none changed. Instruction-surface deletions: none. Embedded instructions in the PR context: none.

Spend (not a finding, not a hold): the probe's max-turns goes from 6 to 10. When the capture is turned on, the driven session can spend up to the proxy's token-cap: 1500000 per capture job, from PR_VERIFY_CLAUDE_API_KEY's prepaid credit. The capture stays off because merge.verify-capture.app is unset.

[github-actions]

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

Code review: PR #13684, running the capture's driven Claude Code behind the key proxy in a sibling sandbox

Result: no blocking findings. This lane already reviewed an earlier head (e134330b), so this review reported blocking findings only. Advisory findings were left out.

I read every hunk of all 11 files listed in .claude-lane/files.txt. I also read the unchanged credit-gate and credential steps of pr-verify-capture.yml that the new steps depend on. Security findings are out of scope for this lane, because a separate security lane (pr-review-security-hosted.yml) runs.

What I checked this round

• The credit gate and the new sandbox steps. Every new step is gated on steps.credential.outputs.present == 'true'. That covers: create the user and directories, install, write the config, start the proxy, the sentinel, the sandbox and its check.
  • "Check the credential" itself runs only when steps.credit.outputs.skip-key != 'true' (pr-verify-capture.yml#L292-L294).
  • So when the key is skipped, none of the new steps run and the real key never reaches the proxy. capture-subscription then still runs on needs.capture.outputs.skip-key == 'true'.
• Cleanup after a probe handoff. "Stop the sibling sandbox" runs on always() && steps.sibling.outputs.child-pid != ''. The proxy stop and usage report run on steps.proxy.outcome != 'skipped', and both have continue-on-error.
  • Collect needs both probe-check and sibling-stop to succeed. It reads publishable from the copied summary rather than the live file, and drops anything that is neither a regular file nor a directory.
• capture-subscription. Its gate steps match the capture job's, and "Write the blocked capture" carries id: collect, so the scan and upload conditions still work. The job references no secret.
  • Removing capture-fallback is consistent across lane-funding.json, callers.test.mjs (the LEG_EXEMPTIONS entry and the widened EXEMPTION_ISSUE) and the README.
• key-proxy.mjs. The check that max_tokens would pass the cap now runs before the preflight. maxPreflightRequests is parsed as a positive integer, and the new tests' arithmetic matches reserve.
  • The config the workflow writes maps max-preflight-requests to maxPreflightRequests, and the schema requires it.
• sibling-init.sh. It writes the facts file and touches done before the broker receives the dummy key on stdin.
  • A missing getent, unshare or socat reports untested, never refused, and the check step rejects untested.
  • The run-directory write probe cleans up after itself.
• Closing issue. The PR only says Refs: #12232 and closes no issue, so there are no acceptance criteria to check.
• Verification. The PR description's test counts, shellcheck and actionlint results are the author's claims. I did not run them: this lane has no shell.

Credit spend (not a finding)

Prepaid API credit only, so no cost hold:

• Once merge.verify-capture.app is set, the driven session spends PR_VERIFY_CLAUDE_API_KEY promotional credit through the proxy. That spend is capped at token-cap: 1500000 tokens per capture job.
• The reach probe's max-turns rises from 6 to 10.

Carried-over note (not counted)

The earlier note still applies: the socket scan's timeout 90 in sibling-init.sh is longer than the check step's WAIT_SECONDS: "60". A slow scan fails closed with "did not report" rather than the sockets_scanned reason.

[melodic-automation-lanes]

The pr-unblock lane stopped resolve-conflicts on this pull request: a conflicted path is a workflow, submodule config or a settings file, which the lane does not resolve. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/conflict status on the head the lane gated.

• Reason: protected-path-conflict
• Head: e134330b4201a9058a5d0c159cecc3108bab721d
• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/42205728528

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=13684 -f activity=resolve-conflicts

[melodic-automation-lanes]

Unattended merge: risk HIGH at head e134330b4201a9058a5d0c159cecc3108bab721d. Only LOW, or MED on class, size and reported review-finding reasons alone, merges without a human. This pull request waits for one.

HIGH agent-surface-review: .github/workflows/pr-verify-capture.yml: new model package run @anthropic-ai/claude-code
HIGH secret-exposure-review: .github/workflows/pr-verify-capture.yml: new secrets reference ${{secrets.pr_verify_claude_api_key}}
MED cost-finding: docs/conventions/pr-pipeline.yaml: proxy:; claude-code-version: 2.1.318; driven-model: claude-opus-5-5; driven-small-model: claude-haiku-5-5; models: [claude-opus-5-5, claude-haiku-5-5]; token-cap: 1500000; max-tokens-ceiling: 16000; max-body-bytes: 8388608; max-in-flight: 4; max-count-tokens-requests: 50; max-preflight-requests: 1000; upstream-timeout-ms: 600000; betas: [claude-code-20250219, interleaved-thinking-2025-05-14, context-management-2025-06-27, per-turn-control-2026-07-01, mid-conversation-tool-changes-2026-07-01, thinking-token-count-2026-05-13, advanced-tool-use-2025-11-20, effort-2025-11-24, thinking-display-updates-2026-08-18, redact-thinking-2026-02-12, structured-outputs-2025-12-15]; body-keys: [context_management, max_tokens, messages, metadata, model, output_config, stream, system, thinking, tools]; block-types: [text, image, document, tool_use, tool_result, thinking, redacted_thinking]; subscription or prepaid spend, reported only
MED evidence-unbound:claude-review: not bound: evidence-missing
MED evidence-unbound:claude-security-review: not bound: evidence-missing

[melodic-automation-lanes]

Unattended merge: risk HIGH at head e134330b4201a9058a5d0c159cecc3108bab721d. Only LOW, or MED on class, size and reported review-finding reasons alone, merges without a human. This pull request waits for one.

HIGH agent-surface-review: .github/workflows/pr-verify-capture.yml: new model package run @anthropic-ai/claude-code
HIGH secret-exposure-review: .github/workflows/pr-verify-capture.yml: new secrets reference ${{secrets.pr_verify_claude_api_key}}
MED cost-finding: docs/conventions/pr-pipeline.yaml: proxy:; claude-code-version: 2.1.318; driven-model: claude-opus-5-5; driven-small-model: claude-haiku-5-5; models: [claude-opus-5-5, claude-haiku-5-5]; token-cap: 1500000; max-tokens-ceiling: 16000; max-body-bytes: 8388608; max-in-flight: 4; max-count-tokens-requests: 50; max-preflight-requests: 1000; upstream-timeout-ms: 600000; betas: [claude-code-20250219, interleaved-thinking-2025-05-14, context-management-2025-06-27, per-turn-control-2026-07-01, mid-conversation-tool-changes-2026-07-01, thinking-token-count-2026-05-13, advanced-tool-use-2025-11-20, effort-2025-11-24, thinking-display-updates-2026-08-18, redact-thinking-2026-02-12, structured-outputs-2025-12-15]; body-keys: [context_management, max_tokens, messages, metadata, model, output_config, stream, system, thinking, tools]; block-types: [text, image, document, tool_use, tool_result, thinking, redacted_thinking]; subscription or prepaid spend, reported only
MED evidence-unbound:claude-review: not bound: evidence-missing
MED evidence-unbound:claude-security-review: not bound: evidence-missing

This refusal has stood unchanged at this head, so a person is needed: the pr-merge lane added the human-gated label.

[melodic-automation-lanes]

The pr-merge lane escalated this pull request, and its head has moved (head-moved): its escalation names head e134330b4201a9058a5d0c159cecc3108bab721d, and the head is now aea2e1c43122b08e1fc1f8838f42d9dec460eb80. pr-merge removed needs-human so it judges aea2e1c43122b08e1fc1f8838f42d9dec460eb80 afresh, and escalates again if its refusal still holds. To keep a person on it, add needs-human back: a label a person adds is never removed.