Fix chroot bind mounts for ARC/DinD split runner-daemon filesystems#2843
Conversation
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 87.37% | 87.28% | 📉 -0.09% |
| Statements | 87.34% | 87.23% | 📉 -0.11% |
| Functions | 82.70% | 82.67% | 📉 -0.03% |
| Branches | 79.43% | 79.11% | 📉 -0.32% |
📁 Per-file Coverage Changes (4 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/services/agent-volumes.ts |
91.6% → 90.5% (-1.11%) | 91.8% → 90.1% (-1.74%) |
src/cli.ts |
28.3% → 27.9% (-0.35%) | 28.3% → 27.9% (-0.35%) |
src/option-parsers.ts |
99.4% → 99.4% (+0.02%) | 99.4% → 99.5% (+0.02%) |
src/container-lifecycle.ts |
87.1% → 88.2% (+1.14%) | 87.5% → 88.6% (+1.11%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
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.
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 adds support for remapping bind-mount source paths to a Docker-daemon-visible filesystem root (e.g. /host) to fix chroot bind mounts in split runner/daemon filesystem setups such as ARC + DinD.
Changes:
- Introduces
--docker-host-path-prefix/container.dockerHostPathPrefixand wires it through config mapping, CLI, and schemas. - Translates agent bind-mount source paths during volume generation (with prefix normalization).
- Adds unit coverage for prefix resolution and mount translation, plus schema/config mapping updates.
Show a summary per file
| File | Description |
|---|---|
| src/types/config.ts | Adds dockerHostPathPrefix to the primary wrapper config type with inline docs. |
| src/services/agent-volumes.ts | Implements prefix normalization + bind-mount host-path translation when generating agent volumes. |
| src/services/agent-volumes.test.ts | Adds compose-level assertions that volume entries are translated under a prefix. |
| src/schema.test.ts | Updates schema sync test fixture to include the new config key. |
| src/option-parsers.ts | Adds resolveDockerHostPathPrefix() to decide explicit vs auto-applied prefix. |
| src/option-parsers.test.ts | Adds unit tests for explicit/auto/none prefix resolution behavior. |
| src/config-file.ts | Extends file config interface and maps container.dockerHostPathPrefix into CLI options. |
| src/config-file.test.ts | Adds schema validation + mapping tests for dockerHostPathPrefix. |
| src/cli.ts | Adds the CLI flag, logging around auto-apply, and plumbs the resolved value into WrapperConfig. |
| src/awf-config-schema.json | Adds JSON schema support for container.dockerHostPathPrefix. |
| docs/awf-config.schema.json | Keeps the docs schema copy in sync with the source schema. |
| docs/awf-config-spec.md | Documents the config-to-CLI flag mapping for the new option. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 12/12 changed files
- Comments generated: 3
| function translateBindMountHostPath(mount: string, dockerHostPathPrefix: string): string { | ||
| const parts = mount.split(':'); | ||
| if (parts.length < 2 || parts.length > 3) { | ||
| return mount; | ||
| } | ||
|
|
||
| const [hostPath, containerPath, mode] = parts; | ||
| if (!hostPath.startsWith('/')) { | ||
| return mount; | ||
| } | ||
|
|
||
| if (dockerHostPathPrefix === '/') { | ||
| return mount; | ||
| } | ||
|
|
||
| const translatedHostPath = hostPath === '/' | ||
| ? dockerHostPathPrefix | ||
| : `${dockerHostPathPrefix}${hostPath}`; | ||
|
|
||
| return mode ? `${translatedHostPath}:${containerPath}:${mode}` : `${translatedHostPath}:${containerPath}`; |
| /** | ||
| * Resolves the effective Docker host path prefix for bind mount translation. | ||
| * | ||
| * If an explicit prefix is provided, it always wins. Otherwise, when AWF detects | ||
| * an external DOCKER_HOST (for example DinD over tcp://), AWF auto-applies | ||
| * '/host' as a compatibility prefix for split runner/daemon filesystems. | ||
| */ | ||
| export function resolveDockerHostPathPrefix( | ||
| dockerHostCheck: { valid: true } | { valid: false; error: string }, | ||
| explicitPrefix: string | undefined | ||
| ): { dockerHostPathPrefix?: string; autoApplied: boolean } { | ||
| const trimmedExplicitPrefix = explicitPrefix?.trim(); | ||
|
|
||
| if (trimmedExplicitPrefix) { | ||
| return { dockerHostPathPrefix: trimmedExplicitPrefix, autoApplied: false }; | ||
| } | ||
|
|
||
| if (!dockerHostCheck.valid) { | ||
| return { dockerHostPathPrefix: '/host', autoApplied: true }; | ||
| } | ||
|
|
||
| return { dockerHostPathPrefix: undefined, autoApplied: false }; | ||
| } |
| expect(volumes).toContain('/daemon-root/usr:/host/usr:ro'); | ||
| expect(volumes).toContain('/daemon-root/etc/passwd:/host/etc/passwd:ro'); | ||
| expect(volumes).toContain('/daemon-root/workspace:/host/workspace:ro'); | ||
| expect(volumes.some((v: string) => v.startsWith(`/daemon-root${mockConfig.workDir}/chroot-`) && v.endsWith(':/host/etc/hosts:ro'))).toBe(true); |
This comment has been minimized.
This comment has been minimized.
|
@copilot address the review feedback |
Addressed in 8b77647.
|
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.
Defense-in-depth against credential leakage via /proc/*/environ: 1. entrypoint.sh: Mount procfs with hidepid=2 so awfuser cannot read PID 1's /proc/1/environ, which may briefly hold auth tokens before unset_sensitive_tokens() clears them. This also blocks indirect access via /dev/fd → /proc/self/fd symlink traversal. 2. agent-volumes.ts: Add /proc to the kernel virtual filesystem passthrough list alongside /dev and /sys. While /proc is not bind-mounted (it uses mount -t proc), this is a defensive guard against accidental exposure through path prefix translation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
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.
# Conflicts: # src/services/agent-volumes.ts
Smoke Test Results✅ File Writing: Created test file successfully Status: PASS (3/3 core tests, auth issue is environment-specific)
|
🔍 Smoke Test Results
Overall: FAIL — GitHub MCP returned 401 and workflow template variables ( cc
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PARTIAL — BYOK inference path works; MCP/pre-step data unavailable due to sandbox credential limitations.
|
|
✅ Merged PRs: #2841 Honor Unix DOCKER_HOST sockets for DinD mounts on ARC runners; #2840 Restore 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 Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL — GitHub Actions service containers are not reachable from this environment via
|
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?
AWF chroot bind mounts assumed runner-visible paths were also visible to the Docker daemon. In ARC + DinD split-filesystem setups, mount sources resolved in the daemon namespace, forcing manual pre-staging of required host paths.
How did you fix it?
CLI + prefix handling
--docker-host-path-prefix <prefix>to remap bind-mount source paths to daemon-visible locations.unix://)DOCKER_HOST, AWF now warns and suggests explicitly setting--docker-host-path-prefixinstead of auto-applying/host.Bind-mount source translation
/dev/nullsource mounts from translation so credential-hiding and docker-socket-hiding overlays remain reliable.Config/schema/docs parity
container.dockerHostPathPrefixto config mapping and both schema copies.awf-config-spec.Behavioral coverage
explicitvsnone)/dev/nullmount-source non-translationdockerHostPathPrefixTesting
/dev/nullbehavior), and config/schema mapping.