Skip to content

Commit d964ac2

Browse files
Mossakaclaude
andauthored
fix: use docker cp instead of file bind mounts for DinD compatibility (#1079)
* fix: use env var injection for squid config to support DinD environments 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> * fix: redirect docker compose stdout to stderr to prevent test pollution 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> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 30d1f1d commit d964ac2

File tree

2 files changed

+67
-12
lines changed

2 files changed

+67
-12
lines changed

src/docker-manager.test.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -477,12 +477,29 @@ describe('docker-manager', () => {
477477
const squid = result.services['squid-proxy'];
478478

479479
expect(squid.container_name).toBe('awf-squid');
480-
expect(squid.volumes).toContain('/tmp/awf-test/squid.conf:/etc/squid/squid.conf:ro');
480+
// squid.conf is NOT bind-mounted; it's injected via AWF_SQUID_CONFIG_B64 env var
481+
expect(squid.volumes).not.toContainEqual(expect.stringContaining('squid.conf'));
481482
expect(squid.volumes).toContain('/tmp/awf-test/squid-logs:/var/log/squid:rw');
482483
expect(squid.healthcheck).toBeDefined();
483484
expect(squid.ports).toContain('3128:3128');
484485
});
485486

487+
it('should inject squid config via base64 env var when content is provided', () => {
488+
const squidConfig = 'http_port 3128\nacl allowed_domains dstdomain .github.com\n';
489+
const result = generateDockerCompose(mockConfig, mockNetworkConfig, undefined, squidConfig);
490+
const squid = result.services['squid-proxy'] as any;
491+
492+
// Should have AWF_SQUID_CONFIG_B64 env var with base64-encoded config
493+
expect(squid.environment.AWF_SQUID_CONFIG_B64).toBe(
494+
Buffer.from(squidConfig).toString('base64')
495+
);
496+
497+
// Should override entrypoint to decode config before starting squid
498+
expect(squid.entrypoint).toBeDefined();
499+
expect(squid.entrypoint[2]).toContain('base64 -d > /etc/squid/squid.conf');
500+
expect(squid.entrypoint[2]).toContain('entrypoint.sh');
501+
});
502+
486503
it('should configure agent container with proxy settings', () => {
487504
const result = generateDockerCompose(mockConfig, mockNetworkConfig);
488505
const agent = result.services.agent;
@@ -2267,7 +2284,7 @@ describe('docker-manager', () => {
22672284
expect(mockExecaFn).toHaveBeenCalledWith(
22682285
'docker',
22692286
['compose', 'up', '-d'],
2270-
{ cwd: testDir, stdio: 'inherit' }
2287+
{ cwd: testDir, stdout: process.stderr, stderr: 'inherit' }
22712288
);
22722289
});
22732290

@@ -2280,7 +2297,7 @@ describe('docker-manager', () => {
22802297
expect(mockExecaFn).toHaveBeenCalledWith(
22812298
'docker',
22822299
['compose', 'up', '-d'],
2283-
{ cwd: testDir, stdio: 'inherit' }
2300+
{ cwd: testDir, stdout: process.stderr, stderr: 'inherit' }
22842301
);
22852302
});
22862303

@@ -2293,7 +2310,7 @@ describe('docker-manager', () => {
22932310
expect(mockExecaFn).toHaveBeenCalledWith(
22942311
'docker',
22952312
['compose', 'up', '-d', '--pull', 'never'],
2296-
{ cwd: testDir, stdio: 'inherit' }
2313+
{ cwd: testDir, stdout: process.stderr, stderr: 'inherit' }
22972314
);
22982315
});
22992316

@@ -2306,7 +2323,7 @@ describe('docker-manager', () => {
23062323
expect(mockExecaFn).toHaveBeenCalledWith(
23072324
'docker',
23082325
['compose', 'up', '-d'],
2309-
{ cwd: testDir, stdio: 'inherit' }
2326+
{ cwd: testDir, stdout: process.stderr, stderr: 'inherit' }
23102327
);
23112328
});
23122329

@@ -2361,7 +2378,7 @@ describe('docker-manager', () => {
23612378
expect(mockExecaFn).toHaveBeenCalledWith(
23622379
'docker',
23632380
['compose', 'down', '-v'],
2364-
{ cwd: testDir, stdio: 'inherit' }
2381+
{ cwd: testDir, stdout: process.stderr, stderr: 'inherit' }
23652382
);
23662383
});
23672384

src/docker-manager.ts

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ export interface SslConfig {
230230
export function generateDockerCompose(
231231
config: WrapperConfig,
232232
networkConfig: { subnet: string; squidIp: string; agentIp: string; proxyIp?: string },
233-
sslConfig?: SslConfig
233+
sslConfig?: SslConfig,
234+
squidConfigContent?: string
234235
): DockerComposeConfig {
235236
const projectRoot = path.join(__dirname, '..');
236237

@@ -249,8 +250,12 @@ export function generateDockerCompose(
249250
: path.join(config.workDir, 'api-proxy-logs');
250251

251252
// Build Squid volumes list
253+
// Note: squid.conf is NOT bind-mounted. Instead, it's passed as a base64-encoded
254+
// environment variable (AWF_SQUID_CONFIG_B64) and decoded by the entrypoint override.
255+
// This supports Docker-in-Docker (DinD) environments where the Docker daemon runs
256+
// in a separate container and cannot access files on the host filesystem.
257+
// See: https://github.com/github/gh-aw/issues/18385
252258
const squidVolumes = [
253-
`${config.workDir}/squid.conf:/etc/squid/squid.conf:ro`,
254259
`${squidLogsPath}:/var/log/squid:rw`,
255260
];
256261

@@ -292,6 +297,29 @@ export function generateDockerCompose(
292297
],
293298
};
294299

300+
// Inject squid.conf via environment variable instead of bind mount.
301+
// In Docker-in-Docker (DinD) environments, the Docker daemon runs in a separate
302+
// container and cannot access files on the host filesystem. Bind-mounting
303+
// squid.conf fails because the daemon creates a directory at the missing path.
304+
// Passing the config as a base64-encoded env var works universally because
305+
// env vars are part of the container spec sent via the Docker API.
306+
if (squidConfigContent) {
307+
const configB64 = Buffer.from(squidConfigContent).toString('base64');
308+
squidService.environment = {
309+
...squidService.environment,
310+
AWF_SQUID_CONFIG_B64: configB64,
311+
};
312+
// Override entrypoint to decode the config before starting squid.
313+
// The original entrypoint (/usr/local/bin/entrypoint.sh) is called after decoding.
314+
// Use $$ to escape $ for Docker Compose variable interpolation.
315+
// Docker Compose interprets $VAR as variable substitution in YAML values;
316+
// $$ produces a literal $ that the shell inside the container will expand.
317+
squidService.entrypoint = [
318+
'/bin/bash', '-c',
319+
'echo "$$AWF_SQUID_CONFIG_B64" | base64 -d > /etc/squid/squid.conf && exec /usr/local/bin/entrypoint.sh',
320+
];
321+
}
322+
295323
// Only enable host.docker.internal when explicitly requested via --enable-host-access
296324
// This allows containers to reach services on the host machine (e.g., MCP gateways)
297325
// Security note: When combined with allowing host.docker.internal domain,
@@ -1294,9 +1322,11 @@ export async function writeConfigs(config: WrapperConfig): Promise<void> {
12941322
// Write Docker Compose config
12951323
// Uses mode 0o600 (owner-only read/write) because this file contains sensitive
12961324
// environment variables (tokens, API keys) in plaintext
1297-
const dockerCompose = generateDockerCompose(config, networkConfig, sslConfig);
1325+
const dockerCompose = generateDockerCompose(config, networkConfig, sslConfig, squidConfig);
12981326
const dockerComposePath = path.join(config.workDir, 'docker-compose.yml');
1299-
fs.writeFileSync(dockerComposePath, yaml.dump(dockerCompose), { mode: 0o600 });
1327+
// lineWidth: -1 disables line wrapping to prevent base64-encoded values
1328+
// (like AWF_SQUID_CONFIG_B64) from being split across multiple lines
1329+
fs.writeFileSync(dockerComposePath, yaml.dump(dockerCompose, { lineWidth: -1 }), { mode: 0o600 });
13001330
logger.debug(`Docker Compose config written to: ${dockerComposePath}`);
13011331
}
13021332

@@ -1395,9 +1425,16 @@ export async function startContainers(workDir: string, allowedDomains: string[],
13951425
composeArgs.push('--pull', 'never');
13961426
logger.debug('Using --pull never (skip-pull mode)');
13971427
}
1428+
// Redirect Docker Compose stdout to stderr so it doesn't pollute the
1429+
// agent command's stdout. Docker Compose outputs build progress and
1430+
// container creation status to stdout, which would be captured by test
1431+
// runners and break assertions that check for agent command output.
1432+
// All AWF informational output goes to stderr (via logger), so this
1433+
// keeps the output consistent. Users still see progress in their terminal.
13981434
await execa('docker', composeArgs, {
13991435
cwd: workDir,
1400-
stdio: 'inherit',
1436+
stdout: process.stderr,
1437+
stderr: 'inherit',
14011438
});
14021439
logger.success('Containers started successfully');
14031440
} catch (error) {
@@ -1551,7 +1588,8 @@ export async function stopContainers(workDir: string, keepContainers: boolean):
15511588
try {
15521589
await execa('docker', ['compose', 'down', '-v'], {
15531590
cwd: workDir,
1554-
stdio: 'inherit',
1591+
stdout: process.stderr,
1592+
stderr: 'inherit',
15551593
});
15561594
logger.success('Containers stopped successfully');
15571595
} catch (error) {

0 commit comments

Comments
 (0)