You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
D2 isolates agents in pinned containers, but those containers still need a narrowly controlled path to the selected model vendor and a phase-specific tool policy. This change adds D3 / T2: each agent runs on an internal Docker network whose CONNECT-only proxy permits only the pinned vendor hosts, while Claude/Codex web and MCP access are disabled and phase policies deny writes and command dispatch where the plan requires.
This PR is stacked on #40 (codex/agent-isolation-d2) and should be reviewed as the D3 increment only.
Scope
Add ownership-bound internal vendor networks and a locked-down proxy container.
Pin Claude to api.anthropic.com; pin Codex to api.openai.com and chatgpt.com.
Attach agent containers only to the internal network and validate the daemon-resolved attachment and proxy environment before start.
Add trusted phase policies:
planning/questions: read, list, and search only; read-only worktree; no runner command
review: read-only worktree; exact approved runner argv only
execute/fix: writable worktree plus exact approved runner argv
Disable Claude Bash/WebFetch/WebSearch/NotebookEdit and all MCP servers; disable Codex web search and MCP servers.
Add real-Docker allow/deny coverage, all-phase worktree checks, and authenticated Codex/Claude probes through the restricted network.
npx vitest run --no-file-parallelism — 16 files, 255 tests passed (serialized locally to avoid Docker resource contention; CI runs the default command)
CLAUDE_CODE_OAUTH_TOKEN=... CODEBOOST_RUN_AUTH_PROBES=1 CODEBOOST_CODEX_AUTH_FILE=... npx vitest run test/agent-container.test.ts — 23 tests passed, including live Codex and Claude startup through vendor-only egress
Real-Docker network test proves the vendor host is reachable, an unrelated proxy target returns 403, and direct egress fails.
D4 owns the production Claude/Codex adapters, cancellation settlement, and bounded output.
D5 owns the combined real-Docker and hostile-input boundary gate.
No merge authorization has been given.
Review record
Pre-review and round-1 self-review: full D3 diff inspected; no remaining local findings.
Round 1: accepted all six findings. Container commands are now policy-generated opaque capabilities; every invocation owns a separate validated network; launch revalidates the proxy, network, subnet, and endpoint set; Claude receives an exact tool set; Codex shell is disabled. Added copied-command, cross-invocation, concurrent-network, and endpoint-mutation regressions. Nothing declined.
Round 2: accepted all five findings. Command capabilities now include exact vendor identity; both CLI builders reject the wrong vendor; proxy validation covers namespace/device state; network validation consumes the caller deadline. Added vendor-mismatch, proxy-replacement, and timeout regressions. Nothing declined.
Round 3: accepted all three findings. Proxy validation now requires the exact entrypoint and environment, binds the stored proxy address, and agent containers disable normal DNS while using that validated address. Added proxy-environment, DNS-mutation, and direct embedded-resolver regressions. Nothing declined.
Round 4: accepted all four findings. Container profiles now exclusively own and dispose their vendor network; the full agent container is revalidated immediately before start; proxy and agent validation reject daemon DNS, host and published-port overrides. Added cleanup, final-attachment and daemon-mutation regressions. Nothing declined.
Round 5: no new findings. Copilot confirmed all four round-4 findings resolved on 981e831.
Review lesson audit: command-policy binding is covered by the existing phase-enforcement and exact-argv rules; launch-time network revalidation and per-invocation network ownership are concrete applications of the existing external-state and isolation rules. No new broadly reusable rule is needed before merge.
Closes the D3 increment tracked in #28; it does not close the full lane.
createVendorNetwork now refuses an expired invocation, so capturing the
suite's invocation at module load with a 60 s deadline could fail the
whole file after a cold image build. Capture it in beforeAll after the
build, with a deadline covering the suite.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Validate the proxy uses a non-restarting Docker policy
agents/network/network.ts:78
The proxy's daemon-resolved configuration does not validate HostConfig.RestartPolicy, even though the agent validator rejects automatic restart policies. A proxy mutated with docker update --restart=always would pass assertVendorNetwork; after a crash or failed cleanup Docker could relaunch the egress endpoint and keep it alive beyond the invocation. Require the resolved policy to be the default non-restarting policy (Name empty/no and retry count zero) before accepting the proxy.
- Record the network and proxy container IDs returned at creation.
Validation inspects those IDs and requires the network name and proxy
endpoint to match them, so a removed-and-recreated network or proxy with
identical attributes is refused. removeVendorNetwork deletes by ID and
leaves a same-named stand-in in place, which surfaces as a cleanup
failure.
- Require the proxy's default no-restart policy, as for the agent and
keeper.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up on the "previously missed" finding in Copilot's review of 9876b04 (in e90b579):
Proxy restart policy (network.ts:78): network validation now requires the proxy's resolved RestartPolicy to be the default (name empty or no, zero retries), matching the agent and keeper checks. A regression runs docker update --restart=always on the proxy and confirms assertVendorNetwork refuses it; the previous code accepted it.
- The egress proxy applied its 8 KiB limit to everything received so far,
so a CONNECT header followed in the same read by a large ClientHello was
refused with 431. The limit now covers only the header.
- assertContainerProfile clamps its network revalidation budget to the
invocation deadline, including for callers using the default budget.
- Give the late-keeper regression enough budget to pass on a loaded daemon.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up on the summary-only findings in Copilot's review of e90b579 ("profile deadline handling and proxy header-size validation"). The review had no inline findings, so I checked both areas myself; both had real issues, fixed in e8e8d57:
Proxy header-size validation: the 8 KiB limit was applied to everything received so far, not just the CONNECT header. Once early tunnel bytes were forwarded (c31e790), a header followed in the same read by a large TLS ClientHello was wrongly refused with 431. The limit now applies only to the header. New tests send a header plus 16 KiB of payload in one write, which the previous proxy refused and the fixed one forwards intact, and confirm that a header over 8 KiB is still refused with 431.
Profile deadline handling:assertContainerProfile passed its default 30 s budget straight to network revalidation. The launch paths already passed a clamped budget, but a caller relying on the default could still validate after the invocation deadline. The budget is now clamped with profileTimeout, so every caller refuses once the deadline has passed. A regression waits past the deadline and calls it with the default budget; the previous code accepted it.
I also gave the late-keeper storage regression a budget that tolerates a loaded Docker daemon. It still fails against the storage code from before the fix.
- createVendorNetwork keeps the network and proxy IDs as soon as each
create returns and uses them for the rest of setup and for failure
cleanup. Names are used only for a create whose ID never came back, so
a same-named stand-in carrying the allocation label is not deleted.
- The proxy searches for the header end only within the 8 KiB limit and
never keeps more than an unfinished header between reads, so a streamed
unterminated header cannot grow its buffer.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The opt-in authenticated Codex and Claude probes now launch through
runContainer, so they exercise creation, validation and removal instead
of raw docker create and start. The Claude token is passed only as the
profile's secret.
- The network suite skips teardown when setup never produced a network,
so a setup failure is reported instead of a teardown TypeError.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up on the summary-only notes in Copilot's review of 647a0bb (no inline findings), both in 75fdee9:
Authenticated probes bypass validated launch helpers: the opt-in live Codex and Claude probes (CODEBOOST_RUN_AUTH_PROBES=1) now run through runContainer instead of raw docker create/docker start, so they exercise creation, pre-start validation and removal. The Claude token goes only through the profile's secrets. These probes need real credentials and did not run in this change; typecheck covers them.
Network teardown can mask setup failures: the network suite's afterAll now skips removal when beforeAll never produced a network, so a setup failure is reported instead of a teardown TypeError.
removeContainerOrThrow now refuses a profile that the trusted builder did
not register before running any Docker command, so a copied profile's
public name and ownership label cannot remove another invocation's
container. The launch paths already rejected copies through their first
profileTimeout call; this makes the cleanup boundary enforce it itself.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem and resulting behavior
D2 isolates agents in pinned containers, but those containers still need a narrowly controlled path to the selected model vendor and a phase-specific tool policy. This change adds D3 / T2: each agent runs on an internal Docker network whose CONNECT-only proxy permits only the pinned vendor hosts, while Claude/Codex web and MCP access are disabled and phase policies deny writes and command dispatch where the plan requires.
This PR is stacked on #40 (
codex/agent-isolation-d2) and should be reviewed as the D3 increment only.Scope
api.anthropic.com; pin Codex toapi.openai.comandchatgpt.com.Owned files:
.github/workflows/agent-isolation.ymlagents/container/Dockerfileagents/container/image.tsagents/container/profile.tsagents/container/run.tsagents/network/network.tsagents/network/proxy.mjsagents/policy.tstest/agent-container.test.tstest/agent-network.test.tstest/agent-policy.test.tstest/questions.test.tsValidation
Validated head:
981e831637c1d19c3b4c759e18cc5ec06e8a5f33npm run typechecknpx vitest run --no-file-parallelism— 16 files, 255 tests passed (serialized locally to avoid Docker resource contention; CI runs the default command)CLAUDE_CODE_OAUTH_TOKEN=... CODEBOOST_RUN_AUTH_PROBES=1 CODEBOOST_CODEX_AUTH_FILE=... npx vitest run test/agent-container.test.ts— 23 tests passed, including live Codex and Claude startup through vendor-only egressDependencies and deferrals
Review record
981e831.Closes the D3 increment tracked in #28; it does not close the full lane.