Skip to content

Commit c375743

Browse files
Mossakaclaude
andcommitted
fix: use mkdtempSync for chroot-hosts to address CodeQL CWE-377
Build complete chroot-hosts content in memory, then write atomically to a securely-created temp directory (fs.mkdtempSync). This satisfies CodeQL's js/insecure-temporary-file rule by using the recognized sanitizer for temp file creation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fdd5713 commit c375743

2 files changed

Lines changed: 34 additions & 19 deletions

File tree

src/docker-manager.test.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -568,10 +568,10 @@ describe('docker-manager', () => {
568568
expect(volumes).toContain('/etc/ca-certificates:/host/etc/ca-certificates:ro');
569569
expect(volumes).toContain('/etc/alternatives:/host/etc/alternatives:ro');
570570
expect(volumes).toContain('/etc/ld.so.cache:/host/etc/ld.so.cache:ro');
571-
// /etc/hosts is now always a custom chroot-hosts file in chroot mode (for pre-resolved domains)
571+
// /etc/hosts is always a custom hosts file in a secure chroot temp dir (for pre-resolved domains)
572572
const hostsVolume = volumes.find((v: string) => v.includes('/host/etc/hosts'));
573573
expect(hostsVolume).toBeDefined();
574-
expect(hostsVolume).toContain('chroot-hosts:/host/etc/hosts:ro');
574+
expect(hostsVolume).toMatch(/chroot-.*\/hosts:\/host\/etc\/hosts:ro/);
575575

576576
// Should still include essential mounts
577577
expect(volumes).toContain('/tmp:/tmp:rw');
@@ -799,7 +799,7 @@ describe('docker-manager', () => {
799799
// Should mount a read-only copy of /etc/hosts with host.docker.internal pre-injected
800800
const hostsVolume = volumes.find((v: string) => v.includes('/host/etc/hosts'));
801801
expect(hostsVolume).toBeDefined();
802-
expect(hostsVolume).toContain('chroot-hosts:/host/etc/hosts:ro');
802+
expect(hostsVolume).toMatch(/chroot-.*\/hosts:\/host\/etc\/hosts:ro/);
803803
});
804804

805805
it('should inject host.docker.internal into chroot-hosts file', () => {
@@ -810,8 +810,10 @@ describe('docker-manager', () => {
810810
};
811811
generateDockerCompose(config, mockNetworkConfig);
812812

813-
// The chroot-hosts file should exist and contain host.docker.internal
814-
const chrootHostsPath = `${mockConfig.workDir}/chroot-hosts`;
813+
// Find the chroot hosts file (mkdtempSync creates chroot-XXXXXX directory)
814+
const chrootDir = fs.readdirSync(mockConfig.workDir).find(d => d.startsWith('chroot-'));
815+
expect(chrootDir).toBeDefined();
816+
const chrootHostsPath = `${mockConfig.workDir}/${chrootDir}/hosts`;
815817
expect(fs.existsSync(chrootHostsPath)).toBe(true);
816818
const content = fs.readFileSync(chrootHostsPath, 'utf8');
817819
// Docker bridge gateway resolution may succeed or fail in test env,
@@ -829,10 +831,10 @@ describe('docker-manager', () => {
829831
const agent = result.services.agent;
830832
const volumes = agent.volumes as string[];
831833

832-
// Should mount a custom chroot-hosts file (for pre-resolved domains)
834+
// Should mount a custom hosts file in a secure chroot temp dir (for pre-resolved domains)
833835
const hostsVolume = volumes.find((v: string) => v.includes('/host/etc/hosts'));
834836
expect(hostsVolume).toBeDefined();
835-
expect(hostsVolume).toContain('chroot-hosts:/host/etc/hosts:ro');
837+
expect(hostsVolume).toMatch(/chroot-.*\/hosts:\/host\/etc\/hosts:ro/);
836838
});
837839

838840
it('should pre-resolve allowed domains into chroot-hosts file', () => {
@@ -859,7 +861,10 @@ describe('docker-manager', () => {
859861
};
860862
generateDockerCompose(config, mockNetworkConfig);
861863

862-
const chrootHostsPath = `${mockConfig.workDir}/chroot-hosts`;
864+
// Find the chroot hosts file (mkdtempSync creates chroot-XXXXXX directory)
865+
const chrootDir = fs.readdirSync(mockConfig.workDir).find(d => d.startsWith('chroot-'));
866+
expect(chrootDir).toBeDefined();
867+
const chrootHostsPath = `${mockConfig.workDir}/${chrootDir}/hosts`;
863868
expect(fs.existsSync(chrootHostsPath)).toBe(true);
864869
const content = fs.readFileSync(chrootHostsPath, 'utf8');
865870

@@ -887,7 +892,10 @@ describe('docker-manager', () => {
887892
// Should not throw even if resolution fails
888893
generateDockerCompose(config, mockNetworkConfig);
889894

890-
const chrootHostsPath = `${mockConfig.workDir}/chroot-hosts`;
895+
// Find the chroot hosts file (mkdtempSync creates chroot-XXXXXX directory)
896+
const chrootDir = fs.readdirSync(mockConfig.workDir).find(d => d.startsWith('chroot-'));
897+
expect(chrootDir).toBeDefined();
898+
const chrootHostsPath = `${mockConfig.workDir}/${chrootDir}/hosts`;
891899
expect(fs.existsSync(chrootHostsPath)).toBe(true);
892900
const content = fs.readFileSync(chrootHostsPath, 'utf8');
893901

@@ -916,7 +924,10 @@ describe('docker-manager', () => {
916924
};
917925
generateDockerCompose(config, mockNetworkConfig);
918926

919-
const chrootHostsPath = `${mockConfig.workDir}/chroot-hosts`;
927+
// Find the chroot hosts file (mkdtempSync creates chroot-XXXXXX directory)
928+
const chrootDir = fs.readdirSync(mockConfig.workDir).find(d => d.startsWith('chroot-'));
929+
expect(chrootDir).toBeDefined();
930+
const chrootHostsPath = `${mockConfig.workDir}/${chrootDir}/hosts`;
920931
const content = fs.readFileSync(chrootHostsPath, 'utf8');
921932

922933
// Count occurrences of 'localhost' - should only be the original entries, not duplicated

src/docker-manager.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -500,18 +500,19 @@ export function generateDockerCompose(
500500
// 1. Pre-resolve allowed domains using the host's DNS stack (supports Tailscale MagicDNS,
501501
// split DNS, and other custom resolvers not available inside the container)
502502
// 2. Inject host.docker.internal when --enable-host-access is set
503-
const chrootHostsPath = path.join(config.workDir, 'chroot-hosts');
503+
// Build complete chroot hosts file content in memory, then write atomically
504+
// to a securely-created temp directory (mkdtempSync) to satisfy CWE-377.
505+
let hostsContent = '127.0.0.1 localhost\n';
504506
try {
505-
fs.copyFileSync('/etc/hosts', chrootHostsPath);
507+
hostsContent = fs.readFileSync('/etc/hosts', 'utf-8');
506508
} catch {
507-
fs.writeFileSync(chrootHostsPath, '127.0.0.1 localhost\n');
509+
// /etc/hosts not readable, use minimal fallback
508510
}
509511

510-
// Pre-resolve allowed domains on the host and inject into /etc/hosts
512+
// Pre-resolve allowed domains on the host and append to hosts content.
511513
// This is critical for domains that rely on custom DNS (e.g., Tailscale MagicDNS
512514
// at 100.100.100.100) which is unreachable from inside the Docker container's
513515
// network namespace. Resolution runs on the host where all DNS resolvers are available.
514-
const hostsContent = fs.readFileSync(chrootHostsPath, 'utf-8');
515516
for (const domain of config.allowedDomains) {
516517
// Skip patterns that aren't resolvable hostnames
517518
if (domain.startsWith('*.') || domain.startsWith('.') || domain.includes('*')) continue;
@@ -523,7 +524,7 @@ export function generateDockerCompose(
523524
const parts = stdout.trim().split(/\s+/);
524525
const ip = parts[0];
525526
if (ip) {
526-
fs.appendFileSync(chrootHostsPath, `${ip}\t${domain}\n`);
527+
hostsContent += `${ip}\t${domain}\n`;
527528
logger.debug(`Pre-resolved ${domain} -> ${ip} for chroot /etc/hosts`);
528529
}
529530
} catch {
@@ -532,7 +533,7 @@ export function generateDockerCompose(
532533
}
533534
}
534535

535-
// Add host.docker.internal when host access is enabled
536+
// Add host.docker.internal when host access is enabled.
536537
// Docker only adds this to the container's /etc/hosts via extra_hosts, but the
537538
// chroot uses the host's /etc/hosts which lacks this entry. MCP servers need it
538539
// to connect to the MCP gateway running on the host.
@@ -544,15 +545,18 @@ export function generateDockerCompose(
544545
]);
545546
const hostGatewayIp = stdout.trim();
546547
if (hostGatewayIp) {
547-
fs.appendFileSync(chrootHostsPath, `${hostGatewayIp}\thost.docker.internal\n`);
548+
hostsContent += `${hostGatewayIp}\thost.docker.internal\n`;
548549
logger.debug(`Added host.docker.internal (${hostGatewayIp}) to chroot-hosts`);
549550
}
550551
} catch (err) {
551552
logger.debug(`Could not resolve Docker bridge gateway: ${err}`);
552553
}
553554
}
554555

555-
fs.chmodSync(chrootHostsPath, 0o644);
556+
// Write to a securely-created directory (mkdtempSync satisfies CWE-377)
557+
const chrootHostsDir = fs.mkdtempSync(path.join(config.workDir, 'chroot-'));
558+
const chrootHostsPath = path.join(chrootHostsDir, 'hosts');
559+
fs.writeFileSync(chrootHostsPath, hostsContent, { mode: 0o644 });
556560
agentVolumes.push(`${chrootHostsPath}:/host/etc/hosts:ro`);
557561

558562
// SECURITY: Hide Docker socket to prevent firewall bypass via 'docker run'

0 commit comments

Comments
 (0)