Skip to content

fix: use docker cp instead of file bind mounts for DinD compatibility#1079

Open
Mossaka wants to merge 4 commits intomainfrom
fix/arc-dind-squid-mount
Open

fix: use docker cp instead of file bind mounts for DinD compatibility#1079
Mossaka wants to merge 4 commits intomainfrom
fix/arc-dind-squid-mount

Conversation

@Mossaka
Copy link
Collaborator

@Mossaka Mossaka commented Feb 27, 2026

Summary

Fixes #18385 (github/gh-aw#18385)

Three fixes in this PR:

  1. DinD compatibility: Replace squid.conf file bind mount with base64-encoded environment variable injection
  2. Dependency audit: Fix npm audit vulnerabilities (ajv ReDoS, minimatch ReDoS)
  3. Test reliability: Redirect Docker Compose stdout to stderr to prevent build output from polluting test assertions

Fix 1: DinD squid.conf injection

In DinD, the Docker daemon runs in a separate dind sidecar container. File bind mounts fail because the daemon can't access files on the runner's filesystem. Instead of bind-mounting squid.conf, the config is:

  1. Base64-encoded and passed as AWF_SQUID_CONFIG_B64 env var
  2. Decoded by an entrypoint override before squid starts

Fix 2: Dependency vulnerabilities

npm audit fix resolves:

  • ajv <6.14.0: moderate severity ReDoS with $data option
  • minimatch 10.0.0-10.2.2: high severity ReDoS via GLOBSTAR segments

Fix 3: Docker Compose stdout isolation

Docker Compose outputs build progress and container status to stdout during docker compose up -d. When integration tests capture AWF's stdout, this build output gets mixed in, breaking test assertions. Fixed by redirecting Docker Compose stdout to process.stderr, keeping it visible to users while isolating agent command output for tests.

Changes

  • src/docker-manager.ts: env var injection, entrypoint override, YAML lineWidth fix, stdout→stderr redirect
  • src/docker-manager.test.ts: updated test expectations
  • package-lock.json: npm audit fix

Test plan

  • All 822 unit tests pass
  • Build succeeds (npm run build)
  • Lint passes (npm run lint — 0 errors)
  • npm audit — 0 vulnerabilities
  • CI integration tests (monitoring)

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings February 27, 2026 01:45
@github-actions
Copy link
Contributor

github-actions bot commented Feb 27, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 82.03% 82.21% 📈 +0.18%
Statements 82.01% 82.18% 📈 +0.17%
Functions 82.50% 82.50% ➡️ +0.00%
Branches 74.20% 74.33% 📈 +0.13%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/docker-manager.ts 83.1% → 83.8% (+0.68%) 82.4% → 83.1% (+0.66%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions
Copy link
Contributor

Bun Build Test Results

Project Install Tests Status
elysia 1/1 PASS
hono 1/1 PASS

Overall: ✅ PASS

  • Bun version: 1.3.10
  • Both projects installed with no packages (clean lockfiles)
  • All tests passed successfully

Generated by Build Test Bun for issue #1079

@github-actions
Copy link
Contributor

🤖 Smoke test results for @Mossaka's PR:

Test Result
GitHub MCP (last 2 merged PRs) #1078 fix: add explicit execute directive to smoke-codex to prevent noop, #1069 fix(deps): resolve high-severity rollup vulnerability in docs-site
Playwright (github.com title) ✅ "GitHub · Change is constant. GitHub keeps you ahead. · GitHub"
File write /tmp/gh-aw/agent/smoke-test-copilot-22469225061.txt created
Bash verification cat confirmed file contents

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1079

@github-actions
Copy link
Contributor

🦕 Deno Build Test Results

Project Tests Status
oak 1/1 ✅ PASS
std 1/1 ✅ PASS

Overall: ✅ PASS

Generated by Build Test Deno for issue #1079

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates AWF’s Docker Compose startup flow to work in Docker-in-Docker (DinD) environments by avoiding file bind mounts for generated configuration artifacts and injecting them via the Docker API.

Changes:

  • Removes squid.conf (and SSL cert/key) file bind mounts and injects them with docker cp.
  • Splits container startup into create (up --no-start) → inject → start (up -d --no-recreate) phases.
  • Updates unit tests to assert the new 3-phase startup behavior and SSL file injection paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/docker-manager.ts Switches from bind-mounting config files to docker cp injection and implements a 3-phase compose startup.
src/docker-manager.test.ts Updates tests to reflect the new compose create/inject/start flow and SSL injection expectations.
Comments suppressed due to low confidence (1)

src/docker-manager.ts:270

  • The comments here suggest directory bind mounts are safe in DinD because Docker will create missing directories, but for mounts where AWF expects pre-populated host content (e.g., ssl_db initialized by initSslDb, or host-side log inspection via checkSquidLogs), creating an empty directory on the daemon side is not equivalent. Consider tightening this comment to avoid implying DinD compatibility for all directory mounts, or document which mounts still require a shared host/daemon filesystem.
  // Note: cert and key FILES are injected via `docker cp` in startContainers()
  // to support DinD environments. Only the ssl_db DIRECTORY is bind-mounted here
  // since directory mounts work in DinD (Docker creates them as dirs by default).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 267 to 274
// Add SSL-related volumes if SSL Bump is enabled
// Note: cert and key FILES are injected via `docker cp` in startContainers()
// to support DinD environments. Only the ssl_db DIRECTORY is bind-mounted here
// since directory mounts work in DinD (Docker creates them as dirs by default).
if (sslConfig) {
squidVolumes.push(`${sslConfig.caFiles.certPath}:${sslConfig.caFiles.certPath}:ro`);
squidVolumes.push(`${sslConfig.caFiles.keyPath}:${sslConfig.caFiles.keyPath}:ro`);
// Mount SSL database at /var/spool/squid_ssl_db (Squid's expected location)
squidVolumes.push(`${sslConfig.sslDbPath}:/var/spool/squid_ssl_db:rw`);
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ssl_db directory is still bind-mounted from a host path (sslConfig.sslDbPath) into /var/spool/squid_ssl_db. In DinD/remote-daemon setups, that host path may not be visible to the Docker daemon, so Docker will mount an empty directory created on the daemon side; this breaks SSL Bump because the DB is expected to be pre-initialized (initSslDb) and security_file_certgen is invoked without creating it. Consider injecting the initialized ssl_db directory via docker cp, or switching to a named volume and initializing the DB inside the squid container before starting it.

This issue also appears on line 268 of the same file.

See below for a potential fix:

  // to support DinD/remote-daemon environments. The ssl_db DIRECTORY is provided
  // via a Docker-managed named volume so it does not rely on host path visibility.
  if (sslConfig) {
    // Mount SSL database at /var/spool/squid_ssl_db (Squid's expected location)
    // using a named volume instead of a host bind mount.
    squidVolumes.push('awf-squid-ssl-db:/var/spool/squid_ssl_db:rw');

Copilot uses AI. Check for mistakes.
Comment on lines +1400 to +1412
// Inject SSL cert/key files if SSL Bump is enabled
const sslCertPath = path.join(workDir, 'ssl', 'ca-cert.pem');
const sslKeyPath = path.join(workDir, 'ssl', 'ca-key.pem');
if (fs.existsSync(sslCertPath)) {
// Inject cert and key at fixed container paths that squid.conf references
// (SQUID_CONTAINER_SSL_CERT and SQUID_CONTAINER_SSL_KEY constants).
// Parent directory /etc/squid/ already exists in the squid image.
await execa('docker', ['cp', sslCertPath, `awf-squid:${SQUID_CONTAINER_SSL_CERT}`], {
stdio: 'inherit',
});
await execa('docker', ['cp', sslKeyPath, `awf-squid:${SQUID_CONTAINER_SSL_KEY}`], {
stdio: 'inherit',
});
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSL cert/key injection is gated only on the cert file existing, but the code unconditionally attempts to docker cp the key as well. If the cert exists without the key (partial/failed previous run, manual setup), this will fail with a confusing docker error. Gate on both files existing and emit a clear error when only one is present.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

Go Build Test Results

Project Download Tests Status
color PASS
env PASS
uuid PASS

Overall: ✅ PASS

Generated by Build Test Go for issue #1079

@github-actions
Copy link
Contributor

C++ Build Test Results

Project CMake Build Status
fmt PASS
json PASS

Overall: PASS

All C++ projects configured and built successfully with GCC 13.3.0.

Generated by Build Test C++ for issue #1079

@github-actions
Copy link
Contributor

fix: use docker cp instead of file bind mounts for DinD compatibility
chore(deps): bump the all-github-actions group across 1 directory with 15 updates
Tests: GitHub MCP merged PRs ✅; safeinputs-gh ✅; Playwright ✅
Tests: Tavily ❌ (tool unavailable); File write ✅; Bash cat ✅
Tests: Discussion query+comment ✅; Build ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1079

@github-actions
Copy link
Contributor

🦀 Rust Build Test Results

Project Build Tests Status
fd 1/1 PASS
zoxide 1/1 PASS

Overall: ✅ PASS

Generated by Build Test Rust for issue #1079

@github-actions
Copy link
Contributor

.NET Build Test Results

Project Restore Build Run Status
hello-world PASS
json-parse PASS

Overall: PASS

Run output

hello-world:

Hello, World!
```

**json-parse:**
```
{
  "Name": "AWF Test",
  "Version": 1,
  "Success": true
}
Name: AWF Test, Success: True

Generated by Build Test .NET for issue #1079

@github-actions
Copy link
Contributor

🟢 Node.js Build Test Results

Project Install Tests Status
clsx PASS ✅ PASS
execa PASS ✅ PASS
p-limit PASS ✅ PASS

Overall: ✅ PASS

Generated by Build Test Node.js for issue #1079

@github-actions
Copy link
Contributor

Java Build Test Results

Project Compile Tests Status
gson 1/1 PASS
caffeine 1/1 PASS

Overall: PASS

Generated by Build Test Java for issue #1079

@github-actions
Copy link
Contributor

Smoke Test Results

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1079

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.13.1 v20.20.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ Not all versions match — Go matches, but Python and Node.js differ between host and chroot environments.

Tested by Smoke Chroot for issue #1079

@Mossaka Mossaka force-pushed the fix/arc-dind-squid-mount branch 2 times, most recently from 293d857 to a7b0fb6 Compare February 27, 2026 02:28
Replace the squid.conf file bind mount with a base64-encoded environment
variable (AWF_SQUID_CONFIG_B64) to support Docker-in-Docker environments
like ARC self-hosted runners.

In DinD, the Docker daemon runs in a separate container and cannot access
files on the host filesystem. File bind mounts fail with "not a directory"
errors because the daemon creates a directory at the non-existent path.

Instead of bind-mounting squid.conf, the config is:
1. Base64-encoded and passed as AWF_SQUID_CONFIG_B64 env var
2. Decoded by an entrypoint override before squid starts

This works universally because env vars are part of the container spec
sent via the Docker API, bypassing filesystem sharing entirely.

The startup flow (docker compose up -d) is unchanged, so all existing
integration tests and behavior remain compatible.

Fixes github/gh-aw#18385

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Contributor

C++ Build Test Results

Project CMake Build Status
fmt PASS
json PASS

Overall: PASS 🎉

Generated by Build Test C++ for issue #1079

@github-actions
Copy link
Contributor

.NET Build Test Results

Project Restore Build Run Status
hello-world PASS
json-parse PASS

Overall: PASS

Run output

hello-world:

Hello, World!
```

**json-parse:**
```
{
  "Name": "AWF Test",
  "Version": 1,
  "Success": true
}
Name: AWF Test, Success: True

Generated by Build Test .NET for issue #1079

@github-actions
Copy link
Contributor

🦀 Rust Build Test Results

Project Build Tests Status
fd 1/1 PASS
zoxide 1/1 PASS

Overall: ✅ PASS

Test Details

fd:

test tests::it_works ... ok
test result: ok. 1 passed; 0 failed; 0 ignored
````

**zoxide:**
````
test tests::test_multiply ... ok
test result: ok. 1 passed; 0 failed; 0 ignored

Generated by Build Test Rust for issue #1079

@github-actions
Copy link
Contributor

Smoke Test Results

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1079

@github-actions
Copy link
Contributor

☕ Java Build Test Results

Project Compile Tests Status
gson 1/1 PASS
caffeine 1/1 PASS

Overall: ✅ PASS

Both projects compiled and all tests passed successfully.

Generated by Build Test Java for issue #1079

@github-actions
Copy link
Contributor

Node.js Build Test Results ✅

Project Install Tests Status
clsx PASS ✅ PASS
execa PASS ✅ PASS
p-limit PASS ✅ PASS

Overall: ✅ PASS

All 3 Node.js projects installed and tested successfully.

Generated by Build Test Node.js for issue #1079

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3
Node.js v24.13.1 v20.20.0
Go go1.22.12 go1.22.12

Result: ❌ Not all versions match — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1079

Mossaka and others added 2 commits March 3, 2026 19:28
Run npm audit fix to resolve:
- ajv <6.14.0: moderate severity ReDoS with $data option
- minimatch 10.0.0-10.2.2: high severity ReDoS via GLOBSTAR segments

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Docker Compose outputs build progress and container status to stdout
during `docker compose up -d` and `docker compose down -v`. When tests
capture AWF's stdout to check agent command output, the Docker Compose
output gets mixed in, breaking assertions.

Redirect Docker Compose stdout to process.stderr so:
- Users still see progress output in their terminal (stderr is visible)
- Test runners only capture agent command output on stdout
- AWF's stderr-based logging remains consistent

The `docker logs -f` command for agent output streaming retains
stdio: inherit since that output IS the agent's actual stdout.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Node.js Build Test Results ✅

Project Install Tests Status
clsx All passed PASS
execa All passed PASS
p-limit All passed PASS

Overall: PASS

Generated by Build Test Node.js for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

.NET Build Test Results

Project Restore Build Run Status
hello-world PASS
json-parse PASS

Overall: PASS

Run output

hello-world:

Hello, World!
```

**json-parse:**
```
{
  "Name": "AWF Test",
  "Version": 1,
  "Success": true
}
Name: AWF Test, Success: True

Generated by Build Test .NET for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3
Node.js v24.13.1 v20.20.0
Go go1.22.12 go1.22.12

Result: Some versions do not match. Go matches, but Python and Node.js differ between host and chroot environments.

Tested by Smoke Chroot for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Java Build Test Results

Project Compile Tests Status
gson 1/1 PASS
caffeine 1/1 PASS

Overall: ✅ PASS

All projects compiled and tests passed successfully via Maven with Squid proxy.

Generated by Build Test Java for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Smoke test results for run 22639188137:

✅ GitHub MCP — Last 2 merged PRs: #1078 "fix: add explicit execute directive to smoke-codex to prevent noop", #1070 "chore: investigate issue duplication detector workflow failure"
✅ Playwright — github.com title contains "GitHub"
✅ File write — /tmp/gh-aw/agent/smoke-test-copilot-22639188137.txt created
✅ Bash — file read back successfully

Overall: PASS | PR author: @Mossaka

📰 BREAKING: Report filed by Smoke Copilot for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Smoke test results

✅ GitHub MCP: #1003 chore(deps): bump all-npm-dependencies, #1078 fix: add explicit execute directive to smoke-codex
✅ Playwright: github.com title contains "GitHub"
✅ File write: /tmp/gh-aw/agent/smoke-test-claude-22639188101.txt created
✅ Bash verify: file contents confirmed

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Rust Build Test Results

Project Build Tests Status
fd 1/1 PASS
zoxide 1/1 PASS

Overall: PASS

Generated by Build Test Rust for issue #1079

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Go Build Test Results ✅

Project Download Tests Status
color PASS ✅ PASS
env PASS ✅ PASS
uuid PASS ✅ PASS

Overall: PASS

Generated by Build Test Go for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

🧪 Bun Build Test Results

Project Install Tests Status
elysia 1/1 PASS
hono 1/1 PASS

Overall: ✅ PASS

Bun version: 1.3.10

Generated by Build Test Bun for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Build Test: Node.js ✅

Project Install Tests Status
clsx PASS PASS
execa PASS PASS
p-limit PASS PASS

Overall: PASS

Generated by Build Test Node.js for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Smoke Test Results ✅ PASS

Test Result
GitHub MCP (last 2 merged PRs) #1078: fix: add explicit execute directive to smoke-codex to prevent noop
Playwright (github.com title) ✅ "GitHub · Change is constant. GitHub keeps you ahead."
File write /tmp/gh-aw/agent/smoke-test-copilot-22640303986.txt created
Bash verify ✅ File read back successfully

PR author: @Mossaka | No assignees

📰 BREAKING: Report filed by Smoke Copilot for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

C++ Build Test Results

Project CMake Build Status
fmt PASS
json PASS

Overall: PASS

Generated by Build Test C++ for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

.NET Build Test Results

Project Restore Build Run Status
hello-world PASS
json-parse PASS

Overall: PASS

Run output

hello-world: Hello, World!

json-parse:

{
  "Name": "AWF Test",
  "Version": 1,
  "Success": true
}
Name: AWF Test, Success: True

Generated by Build Test .NET for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Smoke Test Results

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Rust Build Test Results

Project Build Tests Status
fd 1/1 PASS
zoxide 1/1 PASS

Overall: ✅ PASS

Generated by Build Test Rust for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Java Build Test Results

Project Compile Tests Status
gson 1/1 PASS
caffeine 1/1 PASS

Overall: PASS

Generated by Build Test Java for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Deno Build Test Results

Project Tests Status
oak 1/1 ✅ PASS
std 1/1 ✅ PASS

Overall: ✅ PASS

Generated by Build Test Deno for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.13.1 v20.20.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Result: FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1079

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Overall: FAIL
GitHub MCP (last 2 merged PRs) ✅
safeinputs-gh PR list ✅
Playwright title ✅
Tavily search ❌
File write+cat ✅
Discussion comment ✅
Build (npm ci && npm run build) ✅
PR titles: fix: add explicit execute directive to smoke-codex to prevent noop; fix(deps): resolve high-severity rollup vulnerability in docs-site; fix: correct tmpfs healthcheck paths and smoke test build command; chore(deps): bump the all-npm-dependencies group across 1 directory with 13 updates

🔮 The oracle has spoken through Smoke Codex for issue #1079

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants