Repository navigation
feat(harness-ops): add the driver's launch broker and broker mode - #13661
Conversation
A launch broker (driver/broker.mjs) runs launch, ready and log inside the sandbox it is started in. It takes only those verbs, a session name of 1-32 letters, digits, _ or -, and --plugin-dir and --out paths that resolve inside the roots it starts with, runs the base driver by absolute path with --unattended, and hands that one child the dummy key it read from stdin. With CCD_BROKER_SOCK set, the driver sends launch, ready and log to the broker, paths made absolute, and relays its streamed output and exit, treating every reply as untrusted. Sessions listen under CCD_RUNTIME_DIR ahead of XDG_RUNTIME_DIR. Attended use is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Broker mode serves a Unix socket, and the request and path tests assume POSIX path rules and symlinks, so they skip on Windows as the other socket tests do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tests driver.test.sh now runs broker.test.mjs beside driver.test.mjs, failing on either suite missing. driver.test.mjs clears CCD_RUNTIME_DIR and CCD_BROKER_SOCK, so a runner exporting either cannot change what its tests see. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ient The broker starts only on a ccd-dummy- key and refuses an sk-ant- key. It also refuses a driver directory, driver config directory or deps directory it can write, and a deps directory inside a root. It refuses NUL in any argument, passes each option as one --opt=value argument, and answers a spawn that throws with stderr and exit 1 rather than exiting. The driver opens --out without following symlinks. It checks the broker socket's directory and owner before connecting and gives broker and holder replies a deadline and a 1 MiB cap. It strips control characters from relayed text and prefixes each line with "broker| ", accepts only the requested path back from screenshot and record start, and refuses a relative CCD_RUNTIME_DIR. Session names may no longer start with -. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nes, fix record stop's reply - The broker refuses to start unless its own real directory is the --driver's, which the unwritable-directory check already covers, and it no longer imports driver.mjs, so nothing beside it runs before its checks; a pin test keeps its session rule equal to the driver's. - Holder and broker deadlines are capped at setTimeout's 2^31-1 ms maximum; brokerDeadline added the grace after its clamp and could still overflow. - The holder answers record-stop with a fixed reply, and the client fails on any other answer, so it never prints a path the holder names. - SKILL.md names the driver verb log, not logs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
claude-security-review reviewed Security review: PR #13661, incremental (64eefa3 → 78af76e)Result: No security issues in this push. critical-findings: 0 Scope: I reviewed only what changed since
The file matches none of the trusted-path globs, and no plugin manifest changed. This lane has already reviewed an earlier head, so this review only reports CRITICAL and IMPORTANT findings. What changed
Earlier findingsMy one earlier inline finding (the Unicode line separators in |
|
claude-review reviewed Code review (incremental, blocking only): 64eefa3 → 78af76eResult: no blocking findings. This lane has reviewed earlier heads of this pull request, so this round reports blocking findings only. There are none. Scope. One file changed since the last review: What changed
The cleanup target is the same directory as before. It is now named directly, so it no longer depends on how Checks
Earlier findings: none are open. The Closing issue: none. The PR refers to an issue without closing one, so there are no acceptance criteria to check. Plugin manifests: no manifest changed in this push. Spend: none beyond the existing test run. Embedded text: the PR context, settled-threads and prior-findings files contain no instructions aimed at this lane. |
…utput U+2028 and U+2029 render as line breaks, so they could start a relayed line without the broker| prefix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
plan-test-lanes.sh pairs each Node suite with a sibling .test.sh, so broker.test.mjs run from driver.test.sh had no lane and failed select-tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…name check-test-tmp-cleanup.sh failed lint-shell: the folder was created inside join() and never bound to the name its finally removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Post-merge mutation testingKilled 5, survived 2, timeout 0, no coverage 0, invalid 0, not run 0.
Surviving mutants
The run's summary has the full report. |
Refs: #12232
Summary
PR C Phase 3 of #12232. The pr-verify UI capture needs the driven Claude Code to run inside a sibling sandbox that holds only a dummy key, while the outer capture session drives it from outside. This adds the piece that crosses that boundary: a launch broker beside
driver.mjsthat runs the driver'slaunch,readyandloginside the sandbox, and a driver broker mode that sends those verbs to it.Fix
driver/broker.mjs(new): listens on a unix socket and runs only the realpath'd--driverby absolute path, with--unattended. It accepts onlylaunch,readyandlog, session names matching^[A-Za-z0-9_][\w-]{0,31}$, NUL-free args passed as--opt=value, and--plugin-dir/--outpaths under its--roots. It refuses to start on a key that is notccd-dummy-…, when the driver, config or deps directory is writable by it, when--deps-dirsits inside a root, or from any directory but the driver's. It hands the dummy key only to the driver child it spawns.driver.mjs: broker mode viaCCD_BROKER_SOCK(the socket's parent must be a private dir and the socket owned by this uid); relayed broker output is stripped of control and escape sequences and prefixedbroker|; deadlines on broker and holder requests, capped at 2^31-1 ms; a 1 MiB reply cap;screenshot,record startandrecord stopfail unless the holder's answer is exactly what was asked for;log --outopens withO_NOFOLLOW.runtimeDir()readsCCD_RUNTIME_DIR(absolute only) ahead ofXDG_RUNTIME_DIR, because Claude Code's subprocess scrub removesXDG_RUNTIME_DIRfrom Bash (measured on 2.1.318).SKILL.md: the--unattendedparagraph describes the broker and names the pr-verify capture as its intended consumer; nothing wires it yet.broker.test.sh(new) runs the broker suite as its own lane; the driver test setup clearsCCD_*..changes/harness-ops/.Socket answers and capture files carry no integrity (PR code in the sandbox can forge them); that is accepted in the PR C plan and they are treated as untrusted data.
Verification
driver.test.sh(77) andbroker.test.sh(24): 101 tests, 101 pass, plain and with a hostile environment (CCD_BROKER_SOCK=/nonexistent/broker.sock CCD_RUNTIME_DIR=relative/rt ANTHROPIC_API_KEY=sk-ant-hostile XDG_RUNTIME_DIR=/nonexistent).record stop, SKILL verb) passed at 69bba00 with no assertion loosened.scripts/check-affected.shpasses on the pushed head.Related
pr-verify-capture.yml.Follow-ups
wait-for --timeoutabove 2^31-1 ms reaches the session holder's engine wait unclamped; whether the engine overflows on it is UNVERIFIED.broker.test.mjstest 16 builds a RegExp from the checkout path without escaping or realpath, so it would break on a path with a symlink or a regex metacharacter.🤖 Generated with Claude Code