Skip to content

Commit d0186ca

Browse files
GordonBeemingclaudegitbutler-client
authored
feat: Add brokered Docker socket (--dind, beta) for #20 (#93)
* feat: Add brokered Docker socket (--dind, beta) for #20 Adds a host-side Docker socket broker so the AI inside the container can spawn sibling containers for Testcontainers and on-the-fly image builds. Every Docker API call passes through a JSON allowlist owned by the host process, so the workload container never touches the real /var/run/docker.sock. The broker lives inside the copilot_here C# binary itself. On Linux/macOS it listens on a session-unique Unix Domain Socket (/tmp/copilot-broker-{sessionId}.sock) bind-mounted into the workload container at /var/run/docker.sock. On Windows it falls back to TCP loopback reached via host.docker.internal. Default rules allow the Testcontainers happy path (containers, images, networks, volumes, exec, build) and deny dangerous endpoint families (swarm, services, tasks, nodes, secrets, configs, plugins, session, distribution, auth, events). Configurable per-project (.copilot_here/docker-broker.json) and globally with enforce and monitor modes mirroring airlock. Phase 1 ships endpoint-level filtering only. Body inspection of POST /containers/create (rejecting Privileged: true, host bind mounts, host network/pid/ipc) will follow in a separate issue. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix: Two critical bugs in Docker broker for #20 End-to-end smoke testing against real Docker on macOS/OrbStack found two serious bugs in the brokered Docker socket implementation that made Gordon's `copilot_yolo --no-pull --dind` session unusable. Both are fixed here, with regression tests covering each one. ## Bug 1: macOS UDS bind-mounts don't proxy through to the host Docker Desktop and OrbStack run containers in a Linux VM and use a file-sharing layer (VirtioFS) to expose host paths inside containers. Bind-mounting an arbitrary host Unix Domain Socket *shows* the socket file inside the container (the inode appears as `srwxr-xr-x`), but calling connect() on it fails: the file-sharing layer doesn't proxy UDS connect() back to the macOS host. The host's own /var/run/docker.sock works only because the runtime has hard-coded special-case handling for it. Fix: switch macOS and Windows to TCP loopback (the same path Windows already used). The broker listens on 127.0.0.1:<ephemeral>, the container reaches it via host.docker.internal:<port>. UDS is only used on Linux native, where there's no VM in the way. ## Bug 2: HTTP keep-alive bypassed the rule engine after the first request The original implementation checked the rule engine on the first request arriving on a connection, then did bidirectional byte splicing to the upstream daemon. HTTP/1.1 keep-alive lets a single TCP connection carry many requests, so the second, third, and subsequent requests on the same connection were forwarded blindly without re-checking the allowlist. This was a security hole, not just a correctness bug: a malicious agent could send GET /_ping (allowed) to "open the gate", then send POST /containers/create with Privileged: true on the same connection and the broker would forward it without ever calling CheckRule. Fix: rewrite the buffered request to force `Connection: close` before forwarding to upstream. The upstream daemon closes after one response, the splice hits EOF, both sides close, and the next Docker API call from the client opens a fresh connection that re-enters the rule engine. Hijacked endpoints (exec/attach with Connection: Upgrade) keep their existing bidirectional-splice behavior — those connections are consumed by a single hijacked stream so keep-alive doesn't apply. Cost: one TCP handshake per Docker API call. Unmeasurable against a local Unix socket and entirely worth the security guarantee. ## Tests added - DockerBrokerEndToEndSmokeTests: live integration tests against real Docker. Spins up the broker, runs `docker version` from the host CLI and from inside an alpine+docker-cli container, asserts a tight- allowlist deny actually returns 403. - DindArgsTests: new TCP-path coverage so the macOS/Windows BuildDockerArgs branches are exercised in unit tests. 516/516 unit tests pass, including all 4 smoke tests against real Docker on OrbStack. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix: Substitute new {{DOCKER_BROKER_*}} placeholders in airlock integration scripts The airlock integration test scripts (tests/integration/test_airlock.sh and test_airlock.ps1) do their own template substitution against docker-compose.airlock.yml.template using awk/PowerShell instead of going through AirlockRunner.GenerateComposeFile. They had hardcoded lists of placeholders to substitute, so the three new --dind placeholders ({{DOCKER_BROKER_ENV}}, {{DOCKER_BROKER_MOUNT}}, {{DOCKER_BROKER_EXTRA_HOSTS}}) leaked through to the generated YAML verbatim. docker compose then choked on the literal `{{` braces with "yaml: line 50: could not find expected ':'". Fix is mechanical: empty out the new placeholders in both scripts. The integration tests don't exercise --dind, so empty substitutions collapse to blank lines, which compose tolerates. Manually verified by running test_airlock.sh against a freshly built binary — the YAML now parses, the proxy and app containers create successfully, and the integration suite proceeds past the previous failure point. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * test: Gate live Docker smoke tests behind COPILOT_HERE_RUN_LIVE_DOCKER_TESTS CI runs across ubuntu-24.04 and windows-latest; the smoke tests assumed a Linux Docker daemon and pulled docker:28-cli to run real container operations. Windows runners don't have Linux Docker, ubuntu runners don't pre-pull arbitrary images, and the tests failed on both. These tests are a debugging tool for me locally — they're how I caught the macOS UDS bind-mount and HTTP keep-alive bugs in the first place. They're not unit tests and don't belong in the default CI run. Make them opt-in via the COPILOT_HERE_RUN_LIVE_DOCKER_TESTS env var. To run locally: COPILOT_HERE_RUN_LIVE_DOCKER_TESTS=1 dotnet test. CI never sets it, so the tests skip silently and pass. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * debug: Log the docker run args when COPILOT_HERE_DEBUG=1 Adds a single DebugLogger.Log line in RunCommand after BuildDockerArgs returns, dumping the full docker run command line. This makes it trivial to verify that DOCKER_HOST=tcp://host.docker.internal:NNNN and the host-gateway --add-host entry are actually being passed to the container when --dind is active. This is the diagnostic line I used to confirm the broker wiring was intact end-to-end while debugging Gordon's "tests still failing" report. Keeping it because it's gated on the existing COPILOT_HERE_DEBUG env var, costs nothing in normal operation, and makes the next "did the env var actually get set?" question a 30-second answer instead of a 30-minute one. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix: Make brokered Docker socket actually work end-to-end with Testcontainers Real-world testing against the HubX Questions integration tests (463 tests using Testcontainers .NET to spin up SQL Server and Azurite containers) exposed four more bugs in the Docker socket broker that prevented anything beyond the simplest curl-style requests from working. With these fixes applied, the full hubx integration suite goes from 463/463 failing to 463/463 passing in under 90 seconds, with every Docker API call mediated by the broker (zero blocks, every request matched the allowlist). ## Bug 1: HTTP Upgrade requests had Connection: close injected The original code unconditionally rewrote every forwarded request to add Connection: close. That works for normal request/response calls but is mutually exclusive with HTTP Upgrade. docker run invokes POST /containers/{id}/attach with Connection: Upgrade to set up the hijacked stdin/stdout stream, and the upstream daemon refuses the upgrade when it sees a conflicting close header. The container ended up created but never attached or started. Fix: detect Connection: Upgrade BEFORE rewriting. For hijacked endpoints, forward the original request bytes verbatim and splice both directions until either side closes. The hijack consumes the entire connection so the keep-alive bypass concern doesn't apply. ## Bug 2: HTTP/1.1 bare-LF terminators not recognised Microsoft.Net.Http.Client.ManagedHandler (the HTTP client Docker.DotNet uses, which Testcontainers .NET uses internally) emits headers separated by bare \n (LF) instead of \r\n (CRLF), with only the trailing empty-line as \r\n. The full byte sequence ends with \n\r\n - a 3-byte terminator the strict \r\n\r\n matcher missed entirely. The broker waited for a terminator that never came, the client half-closed, and Testcontainers crashed with TaskCanceledException reading the response. RFC 7230 section 3.5 explicitly says recipients SHOULD accept bare LF. Fix: new IndexOfHeadersEnd helper that recognises all four valid terminators (\r\n\r\n, \n\n, \r\n\n, \n\r\n). The rewrite function now splits header lines on \n and trims trailing \r from each one, so it canonicalises to CRLF on the way out regardless of how the client emitted them. ## Bug 3: Single-segment glob couldn't match registry image paths The default rules used /images/*/json for image inspection, which only matches one segment between /images/ and /json. Real Docker image names contain literal slashes for registry/repo separation, e.g. /images/testcontainers/ryuk:0.14.0/json (3 segments) or /images/mcr.microsoft.com/mssql/server:2025-latest/json (4 segments). Testcontainers couldn't even inspect the ryuk reaper image to start itself. Fix: add ** multi-segment glob to the path matcher (matches zero or more segments) and update the relevant default rules to use it (/images/**/json, /images/**/history, /images/**). Single-segment * still works as before for endpoints like /containers/*/start. ## Bug 4: POST request bodies larger than the initial buffered read were dropped The broker reads the client until it sees the end-of-headers terminator, then forwards the buffered bytes (request line + headers + whatever bit of body happened to be in that initial read) plus the rewritten Connection header. After that, it only copied upstream to client. Any body bytes still on the client socket were never forwarded. For POST /containers/create with a Testcontainers JSON spec - easily a couple of kilobytes - the upstream daemon hung forever waiting for the rest of the body, the test process timed out waiting for the response, and the broker logged OperationCanceledException after 0 bytes when its CancellationToken finally fired. Fix: do bidirectional bytes splicing (client to upstream AND upstream to client) on a linked CancellationTokenSource. As soon as either direction finishes, the other is cancelled and both streams are torn down. Because we asked upstream for Connection: close, upstream to client EOFs naturally after one response, which triggers the cancellation chain. The client cannot slip a second request through onto an already-dead connection - its TCP buffer just gets discarded when the broker disposes the stream. ## Real-world verification Exercise: full dotnet test of HubX Questions.IntegrationTests (463 tests using Testcontainers MsSqlBuilder + Azurite + smtp4dev) running inside a copilot_here:copilot-dotnet container with the broker wired exactly the way RunCommand --dind wires it. Result: 463/463 passing in ~88 seconds, 0 broker blocks, 78 distinct API endpoints handled, including /containers/*/attach, /exec/*/start, /containers/*/logs, multi-segment image paths, and full container lifecycle (create, exec healthcheck, wait, stop, delete). Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix: Set NO_PROXY in airlock + DinD compose env so broker traffic bypasses the airlock HTTP proxy Found while testing the airlock + DinD combination against the HubX integration tests. The airlock compose template sets HTTP_PROXY and HTTPS_PROXY pointing at the airlock proxy, and HTTP clients (including Docker.DotNet's ManagedHandler used by Testcontainers .NET) dutifully route every connection - including the one to host.docker.internal:NNN where the broker is listening - through the airlock proxy. The airlock proxy then rejects it with "Host not allowed" because host.docker.internal isn't in the network allowlist, and the broker never sees the request. Fix: when --dind is enabled in airlock mode, also emit NO_PROXY=host.docker.internal,localhost,127.0.0.1 (and the lowercase no_proxy variant some clients prefer) into the app container env. The HTTP client now bypasses the airlock proxy for broker traffic and sends the request directly. Caveat: this is necessary but not sufficient for full airlock + DinD operation. The airlock network in compose is "internal: true", so even with NO_PROXY in place the workload container can't reach host.docker.internal directly - the route doesn't exist on an internal network. Making airlock + DinD actually work end-to-end needs the broker to live ON the airlock network as a sidecar container with the host docker socket mounted in. Tracked as #95. Standard --dind (no airlock) is fully working with the four broker bug fixes from the previous commit. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix(broker): hijack splice waited on wrong direction, dropping container output The hijacked-endpoint code path (POST /containers/*/attach, POST /exec/*/start) used `Task.WhenAny(clientToUpstream, upstreamToClient)` to keep both copy halves alive. That was the wrong signal — for `docker run alpine echo X`, the docker CLI shuts its write half immediately because it has no input for echo. clientToUpstream then completes within microseconds, WhenAny returns, both streams are disposed, and the alpine container's stdout never reaches the client. Users saw exit code 0 with empty output. The right signal is the upstream → client direction: when the daemon closes the upgraded socket (because the spawned container exited), all output has been delivered. Anything still owed to the client is in our send buffer. Fix: await upstreamToClient explicitly. clientToUpstream runs in the background on a linked CTS and is cancelled in the finally block. Found by the new BrokerSmokeTests.DockerRunAlpine_Succeeds_AndBrokerLogsCreate integration test, which is also added in this branch. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * feat(broker): Phase 2 body inspection for POST /containers/create Adds host-side inspection of the JSON body Testcontainers / docker run sends to spawn siblings. Phase 1 only filtered Docker API endpoints; the body could carry arbitrary HostConfig flags and the broker would forward them verbatim. Phase 2 closes that gap and adds two important security properties to --dind: ## Strict default-deny image allowlist body_inspection.allowed_images is now a strict whitelist with NO implicit allow-all. Empty list = NO sibling can spawn. Users must explicitly enumerate the images they trust: body_inspection: allowed_images: - "mcr.microsoft.com/mssql/server:*" - "testcontainers/ryuk:*" Glob syntax: '*' matches any sequence of characters (incl. /). The startup banner makes the posture loud and clear: ⚠️ No trusted images configured — NO sibling containers can be spawned. or: Trusted images: 4 pattern(s) — only matching images may be spawned. ## Per-rule body-inspection toggles body_inspection.{reject_privileged, reject_host_namespaces, reject_forbidden_binds, reject_dangerous_capabilities} default to true. Each can be flipped independently for workloads that legitimately need the otherwise-rejected setting (e.g. Testcontainers .NET sometimes spawns siblings with Privileged=true). ## What gets blocked by default - HostConfig.Privileged == true - HostConfig.{NetworkMode,PidMode,IpcMode,UsernsMode} == "host" - HostConfig.Binds containing /, /etc, /root, /var, /usr, /bin, /sbin, /proc, /sys, /var/run/docker.sock - HostConfig.CapAdd entries from a curated dangerous-capability list (SYS_ADMIN, SYS_MODULE, SYS_PTRACE, NET_ADMIN, etc.) - any Image not matched by allowed_images ## What gets injected (airlock + DinD) When the broker has a SiblingNetworkName set (AirlockRunner.Run sets it to <projectName>_airlock), POST /containers/create bodies that don't specify an explicit non-host NetworkMode get HostConfig.NetworkMode rewritten to that compose network. This is what unblocks airlock + DinD end-to-end: spawned siblings join the same internal-only network as the workload, and the workload reaches them by Docker DNS instead of via host.docker.internal (which doesn't route on internal: true networks). ## CLI surface New commands mirroring the airlock pattern: --add-docker-broker-image <pattern> --add-global-docker-broker-image <pattern> --remove-docker-broker-image <pattern> --remove-global-docker-broker-image <pattern> --allow-privileged-docker-broker --deny-privileged-docker-broker --allow-privileged-global-docker-broker --deny-privileged-global-docker-broker All idempotent. Add commands report "already present" if the pattern is there. Privileged toggles flip just body_inspection.reject_privileged without touching the other settings. ## Tests 24 new unit tests in DockerBrokerBodyInspectorTests covering each rejection rule, the image allowlist semantics, the NetworkMode injection, and the glob matcher. Plus updated DockerBrokerConfigTests asserting the clean-overlay shape of newly enabled local config. 550/550 unit tests pass. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * feat(broker): persist --enable-docker-broker across sessions + image overrides Two related plumbing changes that make --dind ergonomic: ## Persistent broker enable Before: `copilot_yolo --enable-docker-broker` wrote enabled=true to docker-broker.json but RunCommand only checked the --dind CLI flag at runtime, so users had to keep passing --dind on every invocation. Airlock works correctly because AirlockConfig is loaded into AppContext and RunCommand checks ctx.AirlockConfig.Enabled — the broker just hadn't been wired the same way. Fix: load DockerBrokerConfigLoader.LoadEnabledFlag(paths) into the new AppContext.DockerBrokerEnabled field, and change the broker activation gate in RunCommand from `if (dind)` to `if (dind || ctx.DockerBrokerEnabled)`. The CLI flag remains as a one-shot opt-in for sessions where no config file exists. ## CI image overrides ContainerRunner.GetImageName now honors $COPILOT_HERE_APP_IMAGE and AirlockRunner honors $COPILOT_HERE_PROXY_IMAGE. Both default to the canonical ghcr tags when unset. CI smoke tests use these to point at ephemeral -sha-<sha> tags built earlier in the workflow without having to fork the runner code paths; local devs can use them to test against locally-built proxy/app images. ## Banner update PrintDindBanner now surfaces the image-allowlist posture (the strict- default-deny semantics introduced in the previous commit) so users see loudly that an empty list means nothing can spawn. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * feat(airlock): host broker bridge inside the proxy container, not a sidecar Airlock + DinD on macOS / Windows requires bridging from the workload container (which sits on an internal: true network and can't reach host.docker.internal directly) to the host-side broker. The previous attempt added a separate broker-bridge socat sidecar service to the compose project; per Gordon's preference, airlock-only features should live IN the airlock proxy image rather than as extra services that need their own lifecycle and aren't useful outside airlock mode. This commit moves the bridge into the proxy container itself. ## Changes * docker/Dockerfile.proxy and Dockerfile.proxy-runtime install socat alongside the existing curl/su-exec/ca-certificates set. * proxy-entrypoint.sh starts socat in the background when the new BROKER_BRIDGE_TARGET env var is set, listening on tcp/2375 inside the proxy and forwarding to the target host:port. The proxy is already dual-homed on the airlock network and the external network, so this gets the workload a route to the host broker without creating a new container. * AirlockRunner.GenerateComposeFile drops the broker-bridge sidecar code and instead substitutes two new template placeholders into the proxy service: {{PROXY_BROKER_ENV}} — sets BROKER_BRIDGE_TARGET {{PROXY_BROKER_EXTRA_HOSTS}} — pins host.docker.internal:host-gateway Workload sets DOCKER_HOST=tcp://proxy:2375 + NO_PROXY=proxy,localhost so the docker CLI / Docker.DotNet ManagedHandler bypass the airlock HTTP proxy for daemon traffic. Sets SiblingNetworkName on the broker so Phase 2 body inspection injects the airlock network into spawned siblings (the unblocker for the test scenario). * Removes the now-unnecessary host.docker.internal auto-injection in the airlock allowlist — the workload no longer talks to that host directly. Reverts the temporary AllowInsecure NetworkRule field that the previous attempt added. * AirlockComposeDindTests gains a TCP-broker test asserting the new placeholders substitute correctly: DOCKER_HOST=tcp://proxy:2375, BROKER_BRIDGE_TARGET=host.docker.internal:<port>, NO_PROXY=proxy,... End-to-end: the workload talks to the broker via the proxy container's socat bridge, the broker enforces every Docker API call, Phase 2 body inspection rewrites HostConfig.NetworkMode so spawned siblings join the airlock network, and the workload reaches them by Docker DNS without ever crossing the airlock boundary. Refs #20 Refs #95 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * test: Add tests/CopilotHere.IntegrationTests + CI smoke stages Adds an in-repo integration test project that exercises the brokered Docker socket against real Docker, plus two new CI jobs that run it on PRs (broker smoke) and main (full airlock + DinD smoke). ## tests/CopilotHere.IntegrationTests A second TUnit project alongside CopilotHere.UnitTests, kept separate because it needs a live Docker daemon. All tests are gated by LiveDockerTest.ShouldRun (RUN_LIVE_DOCKER_TESTS=1 + reachable daemon), so a regular `dotnet run` from this project on a developer laptop silently no-ops. * BrokerSmokeTests — 4 tests against the public docker:cli + alpine images. Boots a real DockerSocketBroker, runs docker version, docker run alpine echo (which exercises the hijack/attach path that we just fixed in the previous commits), asserts privileged is rejected, asserts forbidden host bind mounts are rejected. The docker run alpine echo test was the one that found the Task.WhenAny → upstreamToClient bug in the broker — the integration tests caught a real bug in their first run. * AirlockSmokeTests — 1 end-to-end test that uses AirlockRunner.GenerateComposeFile via the InternalsVisibleTo grant, docker compose ups the project with the real proxy + app images, execs into the workload, runs docker run alpine echo as a sibling, asserts both the broker forwarded the call AND the broker rewrote NetworkMode to <projectName>_airlock. Skips when COPILOT_HERE_PROXY_IMAGE / COPILOT_HERE_APP_IMAGE aren't set, so it only runs when the caller has provided test images. ## CI workflow * integration-tests-broker (every push/PR, ubuntu-24.04) — runs BrokerSmokeTests against the runner's Docker daemon with no image builds required. Pre-pulls docker:cli and alpine:3.21 to keep runtime predictable. ~1 minute. * integration-tests-airlock (main only, after build-proxy + build-images) — runs the full airlock E2E with COPILOT_HERE_*_IMAGE pointed at the freshly published proxy-sha-<sha> and copilot-default-sha-<sha> tags. PRs from forks skip this stage automatically. * Both jobs are wired into publish-summary, build-cli, build-proxy-binary, prepare-versions, and publish-nuget needs lists so a failing smoke test blocks downstream publish steps the same way the unit suite does. ## Solution + project plumbing * copilot_here.slnx adds the new project. * app/CopilotHere.csproj adds InternalsVisibleTo for the integration project so AirlockSmokeTests can call AirlockRunner internals without reflection. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * chore(dev-build): emit COPILOT_HERE_PROXY_IMAGE export hints After ./dev-build.sh finishes, write the canonical proxy/app image overrides to a sourceable .dev-build.env file and print the source command. Without this, an airlock + --dind session that needs the freshly-rebuilt proxy (e.g. one that depends on the socat bridge in proxy-entrypoint.sh) silently keeps using whatever was previously cached under :proxy on the host. The exports give an unambiguous override the next `copilot_here --enable-airlock --dind` will pick up via the COPILOT_HERE_PROXY_IMAGE / COPILOT_HERE_APP_IMAGE env vars added in an earlier commit. When exactly one variant was built, also export an app image override. We don't override when zero variants were built (default-only) or when several were built, since there's no obvious "the" app image to point at in those cases. .gitignore picks up .dev-build.env so the generated file doesn't get committed by accident. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * chore(release): bump version to 2026.04.09 Every PR is a release. Bumps across: - VERSION - app/Infrastructure/BuildInfo.cs - copilot_here.sh - copilot_here.ps1 - packaging/winget/GordonBeeming.CopilotHere.yaml - packaging/winget/GordonBeeming.CopilotHere.installer.yaml - packaging/winget/GordonBeeming.CopilotHere.locale.en-US.yaml Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix(ci): unblock integration + airlock test jobs Two CI failures from the previous push, both unrelated to the broker itself — they were missing-update fallout from earlier scope changes. ## Integration Tests (Broker) — connection refused on Linux runners BrokerSmokeTests was binding the broker to IPAddress.Loopback. That works on macOS / OrbStack because Docker Desktop's VM does loopback magic for host.docker.internal, but on Linux native Docker (the CI runner) host-gateway resolves to the bridge IP (e.g. 172.17.0.1) and a loopback-only listener can't accept connections coming in from there. Switch the smoke broker to IPAddress.Any so it accepts connections on both interfaces. Also: the strict default-deny image allowlist introduced earlier in this branch rejected alpine because the smoke tests didn't enumerate trusted images. Add ["alpine:*", "alpine"] to BrokerSmokeTests and AirlockSmokeTests so the happy-path runs and the privileged / forbidden-bind rejection tests still trip on their respective rules (image passes, then inspection trips on the actual policy violation). ## Test Airlock — yaml parse error The airlock integration scripts (test_airlock.sh, test_airlock.ps1) do their own template substitution against docker-compose.airlock.yml.template with a hardcoded list of placeholders. The proxy-bridge refactor added {{PROXY_BROKER_ENV}} and {{PROXY_BROKER_EXTRA_HOSTS}} to the template which the scripts didn't know about, so the literal {{ braces leaked through to the generated YAML and docker compose choked on "line 36: could not find expected ':'" — the same shape of bug we fixed before for {{DOCKER_BROKER_*}}. Empty out both new placeholders in both scripts. The integration tests don't exercise --dind so empty substitutions collapse to blank lines. 5/5 integration tests pass locally with these changes. Refs #20 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix: Address PR #93 review feedback Fixes 6 unresolved review threads from PR #93: ## IsForbiddenHostPath subpath bypass (security) Caught by copilot-pull-request-reviewer. The previous implementation matched bind hosts against the deny list with strict equality, so `-v /etc/passwd:/mnt/passwd` slipped through despite "/etc" being on the deny list. Now matches exact paths AND any path under a forbidden directory. The "/" entry stays exact-match-only — broadening it to "any absolute path" would refuse harmless mounts like /tmp/work, and the other entries already cover the actually-dangerous host directories. Six new regression tests in DockerBrokerBodyInspectorTests cover the bypass cases: /etc/passwd, /etc/shadow, /var/lib/docker, /var/run/docker.sock, /proc/self/mem, /sys/kernel. ## LooksLikeHijack bare-LF support Caught by copilot-pull-request-reviewer. LooksLikeHijack split the header section on "\r\n" only, while IndexOfHeadersEnd and RewriteRequestForceClose elsewhere in the same file already accept bare-LF terminators. With Docker.DotNet's ManagedHandler emitting bare-LF separators, LooksLikeHijack would miss Upgrade/Connection: upgrade and the broker would force Connection: close on a hijacked exec/attach request, breaking it the same way as the earlier Bug 2. Fixed by splitting on "\n" then trimming trailing "\r" (same approach as RewriteRequestForceClose). ## XML doc on AllowedImages contradicted runtime behavior Caught by copilot-pull-request-reviewer. The doc comment said empty allowed_images "disables image filtering — every image is allowed", but the actual inspector treats empty as strict default-deny. Updated the comment to match the shipped behavior. ## README + docs/known-issues.md said body inspection was "next phase" Caught by copilot-pull-request-reviewer (twice — once per file). Both docs predated Phase 2. Updated to describe the actual current behavior: body inspection rejects Privileged, host namespaces, forbidden binds, dangerous capabilities, plus the strict default-deny image allowlist. In airlock mode, NetworkMode is rewritten so siblings join the airlock network. The "Phase 1 limitations" section in known-issues is replaced with the real current limitations: chunked bodies skip inspection, bodies > 2 MiB skip inspection, path canonicalization is string-level (no symlink/.. resolution). ## README example used --java instead of --dotnet Caught by Gordon. Switched the docker-broker quick-start example from --java to --dotnet to match how he and most contributors actually run Testcontainers via this image. 556/556 unit tests pass. Refs #93 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> * fix(ci): apostrophe in awk-block comment closed the bash quote The previous commit added a comment in the awk template-substitution block of test_airlock.sh that contained the word "doesn't". The single-quote in that contraction terminated the surrounding shell single-quoted heredoc-like awk argument, so everything from the next line onward was parsed as bash and choked on the awk gsub syntax: ./tests/integration/test_airlock.sh: line 340: syntax error near unexpected token \`/\\{\\{PROXY_BROKER_ENV\\}\\}/,' Fix: rewrote the comment to "does not" and added a NOTE explaining the apostrophe trap so future edits do not regress this. Verified with bash -n. Refs #93 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: GitButler <gitbutler@gitbutler.com>
1 parent 992e2e0 commit d0186ca

69 files changed

Lines changed: 4927 additions & 82 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/publish.yml

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,44 @@ jobs:
201201
- name: Run airlock integration tests
202202
run: ./tests/integration/test_airlock.sh --use-local
203203

204+
# Live-Docker smoke tests for the brokered Docker socket (--dind).
205+
# Runs on every PR and push: starts the real DockerSocketBroker against
206+
# the runner's Docker daemon, executes docker commands inside a workload
207+
# container that points DOCKER_HOST at the broker, and asserts both the
208+
# happy path (docker version, docker run alpine) AND the Phase 2 body
209+
# inspection rejections (privileged, forbidden host binds).
210+
#
211+
# No copilot_here images required — uses the public docker:cli + alpine
212+
# images so this stage is fast (~1 minute) and unblocked from the main
213+
# build matrices.
214+
integration-tests-broker:
215+
name: Integration Tests (Broker)
216+
needs: [test-cli]
217+
runs-on: ubuntu-24.04
218+
steps:
219+
- name: Checkout repository
220+
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
221+
222+
- name: Setup .NET
223+
uses: actions/setup-dotnet@67a3573c9a986a3f9c594539f4ab511d57bb3ce9 # v4
224+
with:
225+
global-json-file: global.json
226+
227+
- name: Pre-pull test images
228+
run: |
229+
docker pull docker:cli
230+
docker pull alpine:3.21
231+
232+
- name: Run integration tests
233+
env:
234+
RUN_LIVE_DOCKER_TESTS: "1"
235+
working-directory: tests/CopilotHere.IntegrationTests
236+
run: dotnet run --configuration Release
237+
204238
# Build proxy Rust binary natively on each architecture (much faster than QEMU/Docker)
205239
build-proxy-binary:
206240
name: Build Proxy Binary (${{ matrix.arch }})
207-
needs: [test-cli-integration, test-shell-functions, test-airlock]
241+
needs: [test-cli-integration, test-shell-functions, test-airlock, integration-tests-broker]
208242
runs-on: ${{ matrix.runner }}
209243
if: github.ref == 'refs/heads/main'
210244
strategy:
@@ -336,7 +370,7 @@ jobs:
336370
# Fetch latest dependency versions for image builds
337371
prepare-versions:
338372
name: Prepare Versions
339-
needs: [test-cli-integration, test-shell-functions, test-airlock]
373+
needs: [test-cli-integration, test-shell-functions, test-airlock, integration-tests-broker]
340374
runs-on: ubuntu-24.04
341375
if: github.ref == 'refs/heads/main'
342376
outputs:
@@ -489,6 +523,50 @@ jobs:
489523
cache-from: type=registry,ref=ghcr.io/${{ needs.prepare-versions.outputs.repo_name }}:copilot-${{ matrix.image }},mode=max
490524
cache-to: type=inline
491525

526+
# Full airlock + DinD smoke test using the freshly published proxy and
527+
# default app images. This is the unblocker test from issue #20: it spins
528+
# up a real airlock compose project, points the workload at the broker via
529+
# the proxy bridge, and verifies that spawned siblings get the airlock
530+
# NetworkMode injected via Phase 2 body inspection.
531+
#
532+
# Only runs on main because it depends on build-proxy and build-images
533+
# having pushed the canonical sha-tagged images. PRs are covered by the
534+
# cheaper integration-tests-broker job above.
535+
integration-tests-airlock:
536+
name: Integration Tests (Airlock)
537+
needs: [build-proxy, build-images, prepare-versions]
538+
runs-on: ubuntu-24.04
539+
if: github.ref == 'refs/heads/main'
540+
steps:
541+
- name: Checkout repository
542+
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
543+
544+
- name: Setup .NET
545+
uses: actions/setup-dotnet@67a3573c9a986a3f9c594539f4ab511d57bb3ce9 # v4
546+
with:
547+
global-json-file: global.json
548+
549+
- name: Log in to the Container registry
550+
uses: docker/login-action@5e57cd118135c172c3672efd75eb46360885c0ef # v3
551+
with:
552+
registry: ghcr.io
553+
username: ${{ github.actor }}
554+
password: ${{ secrets.GITHUB_TOKEN }}
555+
556+
- name: Pre-pull test images
557+
run: |
558+
docker pull alpine:3.21
559+
docker pull ghcr.io/${{ needs.prepare-versions.outputs.repo_name }}:proxy-sha-${{ github.sha }}
560+
docker pull ghcr.io/${{ needs.prepare-versions.outputs.repo_name }}:copilot-default-sha-${{ github.sha }}
561+
562+
- name: Run integration tests (airlock + dind)
563+
env:
564+
RUN_LIVE_DOCKER_TESTS: "1"
565+
COPILOT_HERE_PROXY_IMAGE: ghcr.io/${{ needs.prepare-versions.outputs.repo_name }}:proxy-sha-${{ github.sha }}
566+
COPILOT_HERE_APP_IMAGE: ghcr.io/${{ needs.prepare-versions.outputs.repo_name }}:copilot-default-sha-${{ github.sha }}
567+
working-directory: tests/CopilotHere.IntegrationTests
568+
run: dotnet run --configuration Release
569+
492570
# Compute version from VERSION file + run number as revision
493571
compute-version:
494572
name: Compute Version
@@ -513,7 +591,7 @@ jobs:
513591
# Publish .NET Tool to NuGet
514592
publish-nuget:
515593
name: Publish NuGet
516-
needs: [test-cli-integration, test-shell-functions, test-airlock, compute-version]
594+
needs: [test-cli-integration, test-shell-functions, test-airlock, integration-tests-broker, compute-version]
517595
runs-on: ubuntu-24.04
518596
if: github.event_name != 'schedule' && github.ref == 'refs/heads/main'
519597
steps:
@@ -538,6 +616,7 @@ jobs:
538616
[
539617
build-images,
540618
build-proxy,
619+
integration-tests-airlock,
541620
release-cli,
542621
publish-nuget,
543622
update-winget,
@@ -593,7 +672,7 @@ jobs:
593672
# Build native CLI binaries for all platforms
594673
build-cli:
595674
name: Build CLI ${{ matrix.rid }}
596-
needs: [test-cli-integration, test-shell-functions, test-airlock, compute-version]
675+
needs: [test-cli-integration, test-shell-functions, test-airlock, integration-tests-broker, compute-version]
597676
runs-on: ${{ matrix.os }}
598677
if: github.event_name != 'schedule' && github.ref == 'refs/heads/main'
599678
strategy:

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5548,3 +5548,5 @@ proxy/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/zeroize-1.8.2/tests/z
55485548
proxy/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/zeroize-1.8.2/tests/zeroize.rs
55495549
test-bin/
55505550
pbi.md
5551+
5552+
.dev-build.env

README.md

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ All functions support switching between Docker image variants using flags:
5656
- **`-h` or `--help`** - Show usage help and examples
5757
- **`--no-cleanup`** - Skip cleanup of unused Docker images
5858
- **`--no-pull`** - Skip pulling the latest image
59+
- **`--dind`** *(beta)* - Enable brokered Docker socket access for Testcontainers and sibling-container workflows. See [Brokered Docker Socket](#-brokered-docker-socket-dind--beta) below.
5960
- **`--mount <path>`** - Mount a directory as read-only (supports `path` or `host:container` format)
6061
- **`--mount-rw <path>`** - Mount a directory as read-write (supports `path` or `host:container` format)
6162
- **`--save-mount <path>`** - Save a mount to local config
@@ -318,6 +319,72 @@ graph LR
318319
Traffic can only leave the Airlock if it explicitly asks the Proxy to carry it. The Proxy inspects the destination against your allow-list and decides whether to let the request pass or block it.
319320

320321

322+
### 🐳 Brokered Docker Socket (DinD) (Beta)
323+
324+
> 🧪 **Beta:** The brokered Docker socket is safer than mounting `/var/run/docker.sock` directly. Every Docker API call passes through a host-owned allowlist. Body-level inspection (rejecting `--privileged` and host bind mounts at request time) is the next phase. See [docs/known-issues.md](docs/known-issues.md#brokered-docker-socket-beta) for what's in scope today.
325+
326+
The `--dind` flag lets the AI inside the container spawn sibling containers on the host's runtime. This unblocks Testcontainers integration tests, on-the-fly image builds, and any workflow where the agent needs `docker run`. The container never sees the real socket: a host-side broker inside the `copilot_here` binary forwards only the requests that match a JSON allowlist.
327+
328+
**Why it's safer than mounting the socket directly:**
329+
330+
- Every Docker API call passes through the host process. The host stays in control of which endpoints reach the daemon.
331+
- Dangerous endpoint families are denied by default: `swarm`, `services`, `tasks`, `nodes`, `secrets`, `configs`, `plugins`, `session`, `distribution`, `auth`, `events`.
332+
- The allowlist is configured per-project (`.copilot_here/docker-broker.json`) or globally (`~/.config/copilot_here/docker-broker.json`).
333+
- `enforce` mode blocks unmatched requests with a 403. `monitor` mode allows everything but logs each call to a JSONL file you can audit.
334+
335+
**Quick start:**
336+
337+
```bash
338+
# One-off session, using the embedded default rules.
339+
copilot_here --dind --dotnet -p "run the integration tests"
340+
341+
# Persist to local project config.
342+
copilot_here --enable-docker-broker
343+
copilot_here --show-docker-broker-rules
344+
345+
# Allow specific images to be spawned (the broker is strict default-deny).
346+
copilot_here --add-docker-broker-image 'mcr.microsoft.com/mssql/server:*'
347+
copilot_here --add-docker-broker-image 'testcontainers/ryuk:*'
348+
349+
# Tweak the rules in $EDITOR.
350+
copilot_here --edit-docker-broker-rules
351+
```
352+
353+
**Setup Commands:**
354+
355+
- **`--enable-docker-broker`** - Enable the broker for current project
356+
- **`--enable-global-docker-broker`** - Enable the broker globally
357+
- **`--disable-docker-broker`** - Disable the broker for current project
358+
- **`--disable-global-docker-broker`** - Disable the broker globally
359+
360+
**Trusted Image Commands:**
361+
362+
- **`--add-docker-broker-image <pattern>`** - Add an image glob to the local trusted list (e.g. `'alpine:*'`)
363+
- **`--add-global-docker-broker-image <pattern>`** - Same, global config
364+
- **`--remove-docker-broker-image <pattern>`** - Remove an image glob from local
365+
- **`--remove-global-docker-broker-image <pattern>`** - Remove from global
366+
- **`--allow-privileged-docker-broker`** / **`--deny-privileged-docker-broker`** - Toggle whether spawned siblings may request `HostConfig.Privileged=true` (default: deny)
367+
- **`--allow-privileged-global-docker-broker`** / **`--deny-privileged-global-docker-broker`** - Same, global config
368+
369+
**Management Commands:**
370+
371+
- **`--show-docker-broker-rules`** - Display defaults plus the active local and global config
372+
- **`--edit-docker-broker-rules`** - Edit local rules in `$EDITOR`
373+
- **`--edit-global-docker-broker-rules`** - Edit global rules in `$EDITOR`
374+
375+
**Modes:**
376+
377+
| Mode | Behavior |
378+
|------|----------|
379+
| `enforce` (default) | Blocks any Docker API call that doesn't match the allowlist with a 403. |
380+
| `monitor` | Allows everything but logs each call to `.copilot_here/logs/docker-broker.jsonl` when `enable_logging: true`. |
381+
382+
**Runtime support:** Docker, OrbStack (uses the standard `/var/run/docker.sock` on macOS, so no extra setup), Podman (rootless and rootful, detected via `podman info`). Set `DOCKER_HOST` if you need to override.
383+
384+
**DinD with airlock:** Allowed, with a loud warning at startup. The broker inspects every `POST /containers/create` body, and in airlock mode it rewrites `HostConfig.NetworkMode` so spawned siblings land on the same internal-only airlock network as the workload. The workload then reaches them by Docker DNS instead of crossing the airlock boundary. This is still beta; check the known issues before relying on it for strict isolation.
385+
386+
**Read this before turning it on:** the broker reads every `POST /containers/create` body and rejects unsafe settings: `Privileged: true`, host network/PID/IPC namespaces, forbidden bind mounts (`/`, `/etc`, `/var`, `/var/run/docker.sock`, and similar), and dangerous Linux capabilities (`SYS_ADMIN`, `SYS_MODULE`, and friends). It also enforces a strict default-deny image allowlist: every image the AI tries to spawn must match an explicit pattern in `body_inspection.allowed_images`, otherwise the call is refused. See [docs/known-issues.md](docs/known-issues.md#brokered-docker-socket-beta) for the remaining limitations and operational caveats.
387+
321388
### Package Managers
322389

323390
**Homebrew (macOS):**

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2026.03.30
1+
2026.04.09
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
using System.CommandLine;
2+
using CopilotHere.Infrastructure;
3+
4+
namespace CopilotHere.Commands.DockerBroker;
5+
6+
public sealed partial class DockerBrokerCommands
7+
{
8+
private static Command SetAddDockerBrokerImageCommand()
9+
{
10+
var command = new Command("--add-docker-broker-image",
11+
"Add an image glob pattern to the local docker broker allowed_images list (e.g. 'mcr.microsoft.com/mssql/server:*')");
12+
13+
var patternArg = new Argument<string>("pattern")
14+
{
15+
Description = "Image glob pattern. Use '*' to match any sequence of characters."
16+
};
17+
command.Add(patternArg);
18+
19+
command.SetAction(parseResult =>
20+
{
21+
var pattern = parseResult.GetValue(patternArg)!;
22+
var paths = AppPaths.Resolve();
23+
var added = DockerBrokerConfigLoader.AddImageLocal(paths, pattern);
24+
if (added)
25+
{
26+
Console.WriteLine($"✅ Added trusted image pattern (local): {pattern}");
27+
Console.WriteLine($" 📁 Rules: {DockerBrokerConfigLoader.GetLocalRulesPath(paths)}");
28+
}
29+
else
30+
{
31+
Console.WriteLine($"ℹ️ Pattern already present (local): {pattern}");
32+
}
33+
return 0;
34+
});
35+
return command;
36+
}
37+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
using System.CommandLine;
2+
using CopilotHere.Infrastructure;
3+
4+
namespace CopilotHere.Commands.DockerBroker;
5+
6+
public sealed partial class DockerBrokerCommands
7+
{
8+
private static Command SetAddGlobalDockerBrokerImageCommand()
9+
{
10+
var command = new Command("--add-global-docker-broker-image",
11+
"Add an image glob pattern to the global docker broker allowed_images list");
12+
13+
var patternArg = new Argument<string>("pattern")
14+
{
15+
Description = "Image glob pattern. Use '*' to match any sequence of characters."
16+
};
17+
command.Add(patternArg);
18+
19+
command.SetAction(parseResult =>
20+
{
21+
var pattern = parseResult.GetValue(patternArg)!;
22+
var paths = AppPaths.Resolve();
23+
var added = DockerBrokerConfigLoader.AddImageGlobal(paths, pattern);
24+
if (added)
25+
{
26+
Console.WriteLine($"✅ Added trusted image pattern (global): {pattern}");
27+
Console.WriteLine($" 📁 Rules: {DockerBrokerConfigLoader.GetGlobalRulesPath(paths)}");
28+
}
29+
else
30+
{
31+
Console.WriteLine($"ℹ️ Pattern already present (global): {pattern}");
32+
}
33+
return 0;
34+
});
35+
return command;
36+
}
37+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using System.CommandLine;
2+
using CopilotHere.Infrastructure;
3+
4+
namespace CopilotHere.Commands.DockerBroker;
5+
6+
public sealed partial class DockerBrokerCommands
7+
{
8+
private static Command SetDisableDockerBrokerCommand()
9+
{
10+
var command = new Command("--disable-docker-broker", "Disable the brokered Docker socket for local config");
11+
command.SetAction(_ =>
12+
{
13+
var paths = AppPaths.Resolve();
14+
DockerBrokerConfigLoader.DisableLocal(paths);
15+
Console.WriteLine("✅ Docker broker disabled (local)");
16+
return 0;
17+
});
18+
return command;
19+
}
20+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using System.CommandLine;
2+
using CopilotHere.Infrastructure;
3+
4+
namespace CopilotHere.Commands.DockerBroker;
5+
6+
public sealed partial class DockerBrokerCommands
7+
{
8+
private static Command SetDisableGlobalDockerBrokerCommand()
9+
{
10+
var command = new Command("--disable-global-docker-broker", "Disable the brokered Docker socket for global config");
11+
command.SetAction(_ =>
12+
{
13+
var paths = AppPaths.Resolve();
14+
DockerBrokerConfigLoader.DisableGlobal(paths);
15+
Console.WriteLine("✅ Docker broker disabled (global)");
16+
return 0;
17+
});
18+
return command;
19+
}
20+
}

0 commit comments

Comments
 (0)