Honor Unix DOCKER_HOST sockets for DinD mounts on ARC runners#2841
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (3 files)
Coverage comparison generated by |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR fixes Docker-in-Docker (DinD) socket mounting for ARC/Kubernetes runners by deriving the bind-mounted Unix socket from configured Docker host settings instead of always assuming /var/run/docker.sock, improving compatibility with runners that expose Docker via a non-standard shared socket path.
Changes:
- Added Unix-socket resolution logic for DinD mounts (with fallback + warnings for malformed
unix://values). - Updated agent volume mounting to only include
/run/docker.sockwhen using the default socket path. - Added unit tests for custom Unix
DOCKER_HOST, override precedence, and malformed Unix socket fallback; documented the ARC DinD sidecar pattern.
Show a summary per file
| File | Description |
|---|---|
| src/services/agent-volumes.ts | Adds resolveDockerSocketPath() and updates DinD volume mounts to honor unix:// socket paths (with fallback behavior). |
| src/services/agent-volumes.test.ts | Adds tests covering custom Unix socket mounts, precedence, and invalid Unix socket fallback behavior. |
| docs/environment.md | Documents ARC/Kubernetes runner pattern using a shared Unix Docker socket with --enable-dind. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/services/agent-volumes.ts:36
socketPathis used verbatim in a docker volume spec (${dockerSocketPath}:/host${dockerSocketPath}:rw). IfDOCKER_HOSTcontains trailing whitespace or unexpected characters like:or newlines, this can produce an invalid/misparsed mount string (and:could be interpreted as additional mount fields). It would be safer totrim()the extracted path and reject paths containing:/control characters before returning it.
const socketPath = dockerHost.slice('unix://'.length);
if (socketPath.startsWith('/') && socketPath !== '/' && socketPath.trim() !== '') {
return socketPath;
}
- Files reviewed: 3/3 changed files
- Comments generated: 3
| const DEFAULT_DOCKER_SOCKET_PATH = '/var/run/docker.sock'; | ||
|
|
||
| function resolveDockerSocketPath(config: WrapperConfig): string { | ||
| const dockerHost = config.awfDockerHost ?? process.env.DOCKER_HOST; |
| it('should prefer awfDockerHost over DOCKER_HOST when enableDind is true', () => { | ||
| const originalDockerHost = process.env.DOCKER_HOST; | ||
| process.env.DOCKER_HOST = 'unix:///tmp/arc/docker.sock'; | ||
|
|
||
| try { | ||
| const dindConfig = { | ||
| ...mockConfig, | ||
| enableDind: true, | ||
| awfDockerHost: 'unix:///run/user/1000/docker.sock', | ||
| }; | ||
| const result = generateDockerCompose(dindConfig, mockNetworkConfig); | ||
| const volumes = result.services.agent.volumes as string[]; | ||
|
|
||
| expect(volumes).toContain('/run/user/1000/docker.sock:/host/run/user/1000/docker.sock:rw'); | ||
| expect(volumes).not.toContain('/tmp/arc/docker.sock:/host/tmp/arc/docker.sock:rw'); | ||
| } finally { |
| On ARC self-hosted runners that expose Docker via a shared Unix socket volume instead of a TCP listener, set `DOCKER_HOST` to that Unix socket and enable DinD passthrough: | ||
|
|
||
| ```yaml | ||
| env: | ||
| DOCKER_HOST: unix:///var/run/docker.sock | ||
| steps: | ||
| - name: Run agent with AWF | ||
| run: | | ||
| awf --enable-dind --allow-domains github.com -- docker ps | ||
| ``` | ||
|
|
||
| When `DOCKER_HOST` points to a Unix socket, AWF now uses that socket path for DinD exposure instead of assuming `/var/run/docker.sock`. If your runner uses a different socket path, AWF will honor it automatically; `--docker-host unix:///path/to/docker.sock` remains the explicit override for AWF's own container operations. | ||
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address the review feedback |
lpcox
left a comment
There was a problem hiding this comment.
Review: PR #2841 — Honor Unix DOCKER_HOST sockets for DinD mounts
Verdict: ✅ Approve — clean, self-contained, no security concerns.
What I checked
-
resolveDockerSocketPath()validation — Correctly rejects TCPDOCKER_HOSTvalues and falls back to/var/run/docker.sock. Only Unix socket paths are honored. Relative and empty paths are rejected with a warning. -
/dev/nulloverlays untouched — Theelseblock (DinD disabled) that mounts/dev/nullover Docker sockets is completely unchanged. Security model preserved. -
Fallback behavior — When
DOCKER_HOSTis missing, invalid, or TCP, defaults to/var/run/docker.sock. The/run/docker.socksymlink mount is correctly made conditional on default path only. -
awfDockerHostprecedence —--docker-hostflag correctly takes priority overDOCKER_HOSTenv var. -
Test coverage — Good: Unix socket exposure, flag precedence, invalid paths, agent
DOCKER_HOSToverride. Minor gap: no explicit test for TCPDOCKER_HOST+enableDind(should fall back to default), but the logic is straightforward.
Staging note
This is the smallest and most self-contained of the three ARC/DinD PRs. Recommend merging first before #2843 and #2839 to minimize rebase conflicts.
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PASS (MCP auth failure is environment-level, not AWF/BYOK issue)
|
Smoke Test Results❌ GitHub MCP Testing - gh CLI auth failed Overall: FAIL (1/4 tests failed)
|
Smoke Test Results
Overall: FAIL — GitHub MCP returned 401 Bad credentials. cc
|
Smoke Test: CodexRestore Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Smoke Test Results
Result: ❌ Not all versions matched — Go matches, but Python and Node.js differ between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL —
|
The --docker-host-path-prefix flag from PR #2843 fully supersedes the --arc-dind feature. This merge brings in main (which includes #2841 and #2843) and removes the now-redundant arc-dind module and inline sourcePath() wrappers. Changes: - Delete src/arc-dind.ts and src/arc-dind.test.ts (orphaned) - Remove --arc-dind CLI option, config, schema, and docs - Remove sourcePath() wrappers from all service builders - Keep --docker-host-path-prefix with centralized translation in buildAgentVolumes() and kernel VFS passthrough (/dev, /sys, /proc) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bug Fix
What was the bug?
ARC DinD runners can expose Docker through a shared Unix socket without using the default hardcoded path. AWF was still assuming
/var/run/docker.sockfor DinD socket exposure, so MCP gateway startup could fail even whenDOCKER_HOSTalready pointed at a valid Unix socket.How did you fix it?
Socket path resolution
awfDockerHostfirst, thenDOCKER_HOST, when either uses aunix://URI./var/run/docker.sockonly when no valid Unix socket is configured./run/docker.sockmounted only for the standard default socket case.Agent Docker client alignment
--enable-dindis used with--docker-host unix:///path/to/docker.sock, set the agent container'sDOCKER_HOSTto that same Unix socket so in-agentdockercommands target the mounted socket by default.DOCKER_HOSTset, avoiding a mismatch between the mounted DinD socket and the agent's Docker client default.Invalid config handling
DOCKER_HOSTcontains a malformed Unix socket URI.DOCKER_HOSTvalues for DinD socket mounting instead of treating them as bind-mount paths.Coverage and docs
--docker-host+--enable-dindcase when hostDOCKER_HOSTis unset.--docker-hostalso controls the DinD socket exposed to the agent when--enable-dindis enabled, and that AWF sets the agent'sDOCKER_HOSTto match.