Skip to content

Commit bf63094

Browse files
committed
Block agent DNS and harden proxy validation
1 parent 9c0655f commit bf63094

6 files changed

Lines changed: 44 additions & 14 deletions

File tree

‎agents/container/profile.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ export function createContainerProfile(options: ProfileOptions): ContainerProfil
187187
const args = ['create', '--name', name, '--read-only', '--user', '10001:10001', '--cap-drop=ALL',
188188
'--security-opt=no-new-privileges', '--pids-limit=128', '--memory=512m', '--memory-swap=512m',
189189
'--cpus=1', '--shm-size=16m', '--ipc=private', '--cgroupns=private',
190-
`--network=${options.network.name}`, '--env', 'HOME=/home/codeboost', '--env', `CODEBOOST_PHASE=${invocation.phase}`,
190+
`--network=${options.network.name}`, '--dns=127.0.0.1', '--env', 'HOME=/home/codeboost',
191+
'--env', `CODEBOOST_PHASE=${invocation.phase}`,
191192
'--label', `io.codeboost.invocation=${ownershipId}`,
192193
'--env', `CODEBOOST_VENDOR=${invocation.vendor}`, '--env', 'npm_config_cache=/tmp/npm-cache',
193194
'--env', `HTTPS_PROXY=${options.network.proxyUrl}`, '--env', `HTTP_PROXY=${options.network.proxyUrl}`,

‎agents/container/run.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ type Inspect = {
8888
CpuPercent: number; IOMaximumBandwidth: number; IOMaximumIOps: number; DeviceCgroupRules: unknown[] | null;
8989
StorageOpt?: Record<string, string> | null; CgroupParent: string;
9090
Devices: unknown[] | null; DeviceRequests: unknown[] | null; Tmpfs: Record<string, string> | null;
91-
Mounts: Array<{ Type: string; Source: string; Target: string; ReadOnly: boolean }> | null };
91+
Mounts: Array<{ Type: string; Source: string; Target: string; ReadOnly: boolean }> | null; Dns: string[] };
9292
Mounts: Array<{ Type: string; Name?: string; Source: string; Destination: string; RW: boolean }>;
9393
NetworkSettings: { Networks: Record<string, unknown> };
9494
};
@@ -132,6 +132,8 @@ export function validateContainer(container: string, profile: ContainerProfile,
132132
|| host.CpuCount !== 0 || host.CpuPercent !== 0 || host.IOMaximumBandwidth !== 0 || host.IOMaximumIOps !== 0
133133
|| host.DeviceCgroupRules !== null || host.StorageOpt != null || host.CgroupParent !== '')
134134
throw new Error('Container daemon configuration is missing required lockdown.');
135+
if (JSON.stringify(host.Dns) !== JSON.stringify(['127.0.0.1']))
136+
throw new Error('Container DNS configuration changed.');
135137
if (JSON.stringify(Object.keys(inspect.NetworkSettings.Networks)) !== JSON.stringify([profile.network.name]))
136138
throw new Error('Container network attachment changed.');
137139
const tmpfs = host.Tmpfs ?? {};

‎agents/network/network.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export interface VendorNetwork {
1515
readonly vendor: InvocationInput['vendor'];
1616
}
1717
interface NetworkIdentity { readonly allocationId: string; readonly imageId: string; readonly invocation: InvocationInput;
18-
readonly subnet: string }
18+
readonly subnet: string; readonly proxyIp: string }
1919
const identities = new WeakMap<VendorNetwork, NetworkIdentity>();
2020
const environment = () => ({ PATH: process.env.PATH, DOCKER_HOST: process.env.DOCKER_HOST });
2121
const deadline = (timeoutMs: number) => {
@@ -66,13 +66,17 @@ const validateVendorNetwork = (network: VendorNetwork, invocation: InvocationInp
6666
SecurityOpt?: string[]; Memory?: number; MemorySwap?: number; NanoCpus?: number; PidsLimit?: number;
6767
NetworkMode?: string; PidMode?: string; IpcMode?: string; UTSMode?: string; UsernsMode?: string;
6868
CgroupnsMode?: string; Devices?: unknown[] | null; DeviceRequests?: unknown[] | null };
69-
NetworkSettings?: { Networks?: Record<string, unknown> }; Mounts?: unknown[] } | undefined;
69+
NetworkSettings?: { Networks?: Record<string, { IPAddress?: string }> }; Mounts?: unknown[] } | undefined;
70+
const image = JSON.parse(docker(['image', 'inspect', identity.imageId], remaining()))[0] as
71+
{ Config?: { Env?: string[] } } | undefined;
7072
const inspectedNetwork = JSON.parse(docker(['network', 'inspect', network.name], remaining()))[0] as
7173
{ Internal?: boolean; Driver?: string; Labels?: Record<string, string>; IPAM?: { Config?: Array<{ Subnet?: string }> };
7274
Containers?: Record<string, { Name?: string }> } | undefined;
7375
const networks = Object.keys(inspect?.NetworkSettings?.Networks ?? {}).sort();
7476
const endpoints = Object.values(inspectedNetwork?.Containers ?? {}).map(value => value.Name).sort();
7577
const allowedEndpoints = [network.proxyContainer, ...(agentName ? [agentName] : [])];
78+
const expectedEnvironment = [...(image?.Config?.Env ?? []),
79+
`CODEBOOST_ALLOWED_HOSTS=${VENDOR_HOSTS[network.vendor].join(',')}`].sort();
7680
if (!inspect?.State?.Running || inspect.Config?.Image !== identity.imageId || inspect.Config?.User !== '10001:10001'
7781
|| inspect.Config?.Labels?.['io.codeboost.egress'] !== identity.allocationId || !inspect.HostConfig?.ReadonlyRootfs
7882
|| inspect.HostConfig.Privileged || !inspect.HostConfig.CapDrop?.map(value => value.toUpperCase()).includes('ALL')
@@ -85,10 +89,10 @@ const validateVendorNetwork = (network: VendorNetwork, invocation: InvocationInp
8589
|| inspect.HostConfig.UsernsMode !== '' || inspect.HostConfig.CgroupnsMode !== 'private'
8690
|| (inspect.HostConfig.Devices?.length ?? 0) !== 0 || (inspect.HostConfig.DeviceRequests?.length ?? 0) !== 0
8791
|| JSON.stringify(networks) !== JSON.stringify(['bridge', network.name].sort()) || inspect.Mounts?.length
88-
|| inspect.Config?.Entrypoint?.[0] !== 'node'
92+
|| JSON.stringify(inspect.Config?.Entrypoint) !== JSON.stringify(['node'])
8993
|| JSON.stringify(inspect.Config?.Cmd) !== JSON.stringify(['/usr/local/lib/codeboost-egress-proxy.mjs'])
90-
|| inspect.Config.Env?.filter(value => value.startsWith('CODEBOOST_ALLOWED_HOSTS=')).length !== 1
91-
|| !inspect.Config.Env?.includes(`CODEBOOST_ALLOWED_HOSTS=${VENDOR_HOSTS[network.vendor].join(',')}`)
94+
|| JSON.stringify([...(inspect.Config.Env ?? [])].sort()) !== JSON.stringify(expectedEnvironment)
95+
|| inspect.NetworkSettings?.Networks?.[network.name]?.IPAddress !== identity.proxyIp
9296
|| !inspectedNetwork?.Internal || inspectedNetwork.Driver !== 'bridge'
9397
|| inspectedNetwork.Labels?.['io.codeboost.egress'] !== identity.allocationId
9498
|| inspectedNetwork.IPAM?.Config?.length !== 1 || inspectedNetwork.IPAM.Config[0]?.Subnet !== identity.subnet
@@ -129,8 +133,13 @@ export function createVendorNetwork(invocation: InvocationInput, imageId: string
129133
"socket.once('connect',()=>{socket.destroy();process.exit(0)});",
130134
"socket.once('error',()=>{socket.destroy();if(++attempts===50)process.exit(1);setTimeout(check,20)})};check();",
131135
].join('')], remaining());
132-
const network = Object.freeze({ name, proxyContainer, proxyUrl: 'http://codeboost-proxy:3128', vendor });
133-
identities.set(network, Object.freeze({ allocationId, imageId, invocation, subnet }));
136+
const proxyInspect = JSON.parse(docker(['container', 'inspect', proxyContainer], remaining()))[0] as
137+
{ NetworkSettings?: { Networks?: Record<string, { IPAddress?: string }> } } | undefined;
138+
const proxyIp = proxyInspect?.NetworkSettings?.Networks?.[name]?.IPAddress;
139+
if (!proxyIp || !/^10\.254\.\d{1,3}\.\d{1,3}$/.test(proxyIp))
140+
throw new Error('Vendor proxy did not receive its expected internal address.');
141+
const network = Object.freeze({ name, proxyContainer, proxyUrl: `http://${proxyIp}:3128`, vendor });
142+
identities.set(network, Object.freeze({ allocationId, imageId, invocation, subnet, proxyIp }));
134143
validateVendorNetwork(network, invocation, undefined, remaining);
135144
remaining();
136145
return network;

‎test/agent-container.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,11 @@ describe('real Docker agent isolation', () => {
296296
expect(state.State.Status).toBe('created');
297297
docker('rm', '--force', valid.name); containers.delete(valid.name);
298298

299+
const dnsArgs = valid.args.map(value => value === '--dns=127.0.0.1' ? '--dns=8.8.8.8' : value);
300+
docker(...dnsArgs); containers.add(valid.name);
301+
expect(() => validateContainer(valid.name, valid)).toThrow('DNS configuration');
302+
docker('rm', '--force', valid.name); containers.delete(valid.name);
303+
299304
const imageIndex = valid.args.indexOf(imageId);
300305
const namespaceArgs = [...valid.args.slice(0, imageIndex), '--uts=host', ...valid.args.slice(imageIndex)];
301306
docker(...namespaceArgs); containers.add(valid.name);
@@ -336,7 +341,7 @@ describe('real Docker agent isolation', () => {
336341
expect(() => prepareTaskFilesystems({ ...data.clone }, {
337342
workBytes: 1024, workInodes: 16, metadataBytes: 1024, metadataInodes: 16,
338343
}, imageId)).toThrow('trusted clone builder');
339-
});
344+
}, 60_000);
340345

341346
if (process.env.CODEBOOST_RUN_AUTH_PROBES === '1') {
342347
it('runs the authenticated Codex startup path with isolated writable state', () => {

‎test/agent-network.test.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ describe('vendor-only egress', () => {
4949
expect(direct.stdout).toBe('000');
5050
}, 60_000);
5151

52+
it('does not forward arbitrary DNS even when the embedded resolver is addressed directly', () => {
53+
const result = spawnSync('docker', ['run', '--rm', `--network=${network.name}`, '--dns=127.0.0.1',
54+
'--entrypoint', 'node', imageId, '-e', [
55+
"const dns=require('node:dns');dns.setServers(['127.0.0.11']);",
56+
"dns.resolve4('example.com',(error)=>process.exit(error?0:1));",
57+
'setTimeout(()=>process.exit(0),3000);',
58+
].join('')], { encoding: 'utf8', timeout: 10_000, stdio: ['ignore', 'pipe', 'pipe'] });
59+
expect(result.status).toBe(0);
60+
}, 30_000);
61+
5262
it('rejects a copied network capability', () => {
5363
expect(() => createVendorNetwork(invocation, imageId, 0)).toThrow('positive integer');
5464
expect(() => assertVendorNetwork(network, invocation, undefined, 0)).toThrow('positive integer');
@@ -75,7 +85,10 @@ describe('vendor-only egress', () => {
7585
}
7686
}, 60_000);
7787

78-
it('rejects a proxy replaced with a host namespace before launch', () => {
88+
it.each([
89+
['host namespace', ['--pid=host']],
90+
['extra Node environment', ['--env', 'NODE_OPTIONS=--trace-warnings']],
91+
] as const)('rejects a proxy replaced with %s before launch', (_label, extra) => {
7992
const replacementInvocation = captureInvocation({ ...invocation, attemptId: 'mutated-proxy-probe',
8093
deadline: Date.now() + 60_000 });
8194
const replacement = createVendorNetwork(replacementInvocation, imageId);
@@ -86,7 +99,7 @@ describe('vendor-only egress', () => {
8699
docker('rm', '--force', replacement.proxyContainer);
87100
docker('run', '--detach', '--name', replacement.proxyContainer, '--read-only', '--user', '10001:10001',
88101
'--cap-drop=ALL', '--security-opt=no-new-privileges', '--pids-limit=64', '--memory=64m', '--memory-swap=64m',
89-
'--cpus=.25', '--pid=host', '--network', replacement.name, '--network-alias', 'codeboost-proxy',
102+
'--cpus=.25', ...extra, '--network', replacement.name, '--network-alias', 'codeboost-proxy',
90103
'--label', `io.codeboost.egress=${allocation}`, '--env', `CODEBOOST_ALLOWED_HOSTS=${VENDOR_HOSTS.claude.join(',')}`,
91104
'--entrypoint', 'node', imageId, '/usr/local/lib/codeboost-egress-proxy.mjs');
92105
docker('network', 'connect', 'bridge', replacement.proxyContainer);

‎test/questions.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import { ReviewService } from '../runner/review.ts';
77
import { Questions } from '../runner/questions.ts';
88
import { choiceKeys } from '../core/approvals.ts';
99
import { agentArguments } from '../runner/question-agent.ts';
10-
// Real-Git context reads match the existing review integration suite budget.
11-
vi.setConfig({testTimeout:15000});
10+
// Real-Git context reads can overlap the Docker-backed isolation suite in a full run.
11+
vi.setConfig({testTimeout:30000});
1212
const roots:string[]=[], services:ReviewService[]=[], managers:Questions[]=[];
1313
afterEach(async()=>{for(const manager of managers.splice(0))await manager.close();services.splice(0).forEach(s=>s.close());roots.splice(0).forEach(root=>rmSync(root,{recursive:true,force:true}));vi.restoreAllMocks();});
1414
function waitForAbort(_prompt:string,signal:AbortSignal):Promise<string>{return new Promise((_,reject)=>signal.addEventListener('abort',()=>reject(signal.reason),{once:true}));}

0 commit comments

Comments
 (0)