Skip to content

Commit 5a1c75f

Browse files
mchwangclaude
andcommitted
Own vendor networks on profile failure and bound network cleanup
- Claim the vendor network before any fallible profile work and remove it if creation fails, aggregating cleanup errors. - Reserve cleanup time inside createVendorNetwork's deadline so failure cleanup cannot overrun the caller's timeoutMs. - Compare the live Claude marker exactly, tolerating only one wrapping pair of backticks or quotes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 750fd49 commit 5a1c75f

4 files changed

Lines changed: 51 additions & 20 deletions

File tree

‎agents/container/profile.ts‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -158,22 +158,24 @@ export function createContainerProfile(options: ProfileOptions): ContainerProfil
158158
assertTaskFilesystems(filesystems, invocation.clone);
159159
assertVendorNetwork(options.network, invocation);
160160
if (claimedNetworks.has(options.network)) throw new Error('Vendor network already belongs to another container profile.');
161-
assertPhasePolicy(options.policy, invocation);
162-
const command = assertAgentCommand(options.command, options.policy, invocation.vendor);
163-
const sourceInput = captureInput(options.inputDirectory);
164-
if (invocation.vendor === 'codex' && (!options.codexAuthFile || options.claudeToken))
165-
throw new Error('Codex requires only its auth file.');
166-
if (invocation.vendor === 'claude' && (!options.claudeToken || options.codexAuthFile))
167-
throw new Error('Claude requires only its OAuth token.');
168-
if (options.claudeToken?.includes('\0')) throw new Error('Claude OAuth token is malformed.');
169-
if (!/^codeboost-work-[0-9a-f-]+$/.test(filesystems.workVolume)
170-
|| !/^codeboost-metadata-[0-9a-f-]+$/.test(filesystems.metadataVolume)
171-
|| !/^codeboost-keeper-[0-9a-f-]+$/.test(filesystems.keeper)) throw new Error('Task filesystem identity is invalid.');
172-
// Read through one no-follow descriptor so the path cannot be swapped between check and open.
173-
const sourceAuth = options.codexAuthFile ? readCapturedFile(options.codexAuthFile, 'Codex auth') : undefined;
161+
// Own the network from here on, so any later failure removes it rather than leaking it.
162+
claimedNetworks.add(options.network);
174163
const cleanupDirectories: string[] = [];
175164
let codexAuthFile: string | undefined, authIdentity: FileIdentity | undefined;
176165
try {
166+
assertPhasePolicy(options.policy, invocation);
167+
const command = assertAgentCommand(options.command, options.policy, invocation.vendor);
168+
const sourceInput = captureInput(options.inputDirectory);
169+
if (invocation.vendor === 'codex' && (!options.codexAuthFile || options.claudeToken))
170+
throw new Error('Codex requires only its auth file.');
171+
if (invocation.vendor === 'claude' && (!options.claudeToken || options.codexAuthFile))
172+
throw new Error('Claude requires only its OAuth token.');
173+
if (options.claudeToken?.includes('\0')) throw new Error('Claude OAuth token is malformed.');
174+
if (!/^codeboost-work-[0-9a-f-]+$/.test(filesystems.workVolume)
175+
|| !/^codeboost-metadata-[0-9a-f-]+$/.test(filesystems.metadataVolume)
176+
|| !/^codeboost-keeper-[0-9a-f-]+$/.test(filesystems.keeper)) throw new Error('Task filesystem identity is invalid.');
177+
// Read through one no-follow descriptor so the path cannot be swapped between check and open.
178+
const sourceAuth = options.codexAuthFile ? readCapturedFile(options.codexAuthFile, 'Codex auth') : undefined;
177179
const inputDirectory = mkdtempSync(join(tmpdir(), 'codeboost-input-'));
178180
cleanupDirectories.push(inputDirectory);
179181
writeFileSync(join(inputDirectory, 'schema.json'), sourceInput.content,
@@ -224,11 +226,12 @@ export function createContainerProfile(options: ProfileOptions): ContainerProfil
224226
auth: authIdentity,
225227
cleanupDirectories: Object.freeze([...cleanupDirectories]), filesystems, clone: invocation.clone,
226228
network: options.network, policy: options.policy, invocation }));
227-
claimedNetworks.add(options.network);
228229
return profile;
229230
} catch (error) {
230-
try { removeOwnedDirectories(cleanupDirectories); }
231-
catch (cleanupError) { throw new AggregateError([error, cleanupError], 'Profile creation and cleanup both failed.'); }
231+
const failures: unknown[] = [];
232+
try { removeOwnedDirectories(cleanupDirectories); } catch (cleanupError) { failures.push(cleanupError); }
233+
try { removeVendorNetwork(options.network); } catch (cleanupError) { failures.push(cleanupError); }
234+
if (failures.length) throw new AggregateError([error, ...failures], 'Profile creation and cleanup both failed.');
232235
throw error;
233236
}
234237
}

‎agents/network/network.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,9 @@ export function createVendorNetwork(invocation: InvocationInput, imageId: string
119119
timeoutMs = 60_000): VendorNetwork {
120120
assertBuiltAgentImage(imageId);
121121
const vendor = invocation.vendor;
122-
const remaining = deadline(timeoutMs), allocationId = randomUUID();
122+
// Setup runs inside the caller's budget minus a cleanup reserve, so failure cleanup cannot overrun timeoutMs.
123+
const overall = deadline(timeoutMs), cleanupReserve = Math.min(10_000, Math.floor(timeoutMs / 3));
124+
const remaining = deadline(Math.max(1, timeoutMs - cleanupReserve)), allocationId = randomUUID();
123125
const name = `codeboost-egress-${vendor}-${randomUUID()}`;
124126
const proxyContainer = `codeboost-proxy-${vendor}-${randomUUID()}`;
125127
const subnetSeed = randomUUID().replaceAll('-', '');
@@ -155,9 +157,9 @@ export function createVendorNetwork(invocation: InvocationInput, imageId: string
155157
} catch (error) {
156158
const failures: unknown[] = [];
157159
if (proxyPlanned) try { remove(['rm', '--force', proxyContainer], ['container', 'inspect', proxyContainer],
158-
deadline(30_000), 'vendor proxy', allocationId); } catch (cleanupError) { failures.push(cleanupError); }
160+
overall, 'vendor proxy', allocationId); } catch (cleanupError) { failures.push(cleanupError); }
159161
if (networkPlanned) try { remove(['network', 'rm', name], ['network', 'inspect', name],
160-
deadline(30_000), 'vendor network', allocationId); } catch (cleanupError) { failures.push(cleanupError); }
162+
overall, 'vendor network', allocationId); } catch (cleanupError) { failures.push(cleanupError); }
161163
if (failures.length) throw new AggregateError([error, ...failures], 'Vendor network creation and cleanup failed.');
162164
throw error;
163165
}

‎test/agent-container.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,14 @@ describe('real Docker agent isolation', () => {
317317
expect(result.stderr).toContain('seccomp syscall filter must be enforced');
318318
}, 60_000);
319319

320+
it('removes the claimed vendor network when profile creation fails after the claim', () => {
321+
const data = fixture();
322+
expect(() => profile(data, 'planning', 'noop', { codexAuthFile: join(data.root, 'missing-auth.json') })).toThrow();
323+
const orphan = vendorNetworks.at(-1)!;
324+
expect(spawnSync('docker', ['network', 'inspect', orphan.name], { stdio: 'ignore' }).status).not.toBe(0);
325+
expect(spawnSync('docker', ['container', 'inspect', orphan.proxyContainer], { stdio: 'ignore' }).status).not.toBe(0);
326+
}, 60_000);
327+
320328
it('refuses a Codex auth path that is a link without resolving it', () => {
321329
const data = fixture(), link = join(data.root, 'auth-link.json');
322330
symlinkSync(data.fakeAuth, link);
@@ -452,7 +460,9 @@ describe('real Docker agent isolation', () => {
452460
docker('rm', '--force', authProfile.name); containers.delete(authProfile.name);
453461
const envelope = JSON.parse(output) as { result?: string; is_error?: boolean };
454462
expect(envelope.is_error).not.toBe(true);
455-
expect(envelope.result).toContain('codeboost-schema-marker');
463+
// Tolerate one wrapping pair of backticks or quotes, but nothing else around the value.
464+
const value = envelope.result?.trim().replace(/^(`+|"|')([^]*)\1$/, '$2').trim();
465+
expect(value).toBe('codeboost-schema-marker');
456466
}, 6 * 60_000);
457467
}
458468
});

‎test/agent-network.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { execFileSync, spawnSync } from 'node:child_process';
22
import { randomUUID } from 'node:crypto';
3+
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
4+
import { tmpdir } from 'node:os';
5+
import { join } from 'node:path';
36
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
47
import { buildAgentImage } from '../agents/container/image.ts';
58
import { assertVendorNetwork, createVendorNetwork, removeVendorNetwork, VENDOR_HOSTS,
@@ -27,6 +30,19 @@ beforeAll(() => {
2730
afterAll(() => removeVendorNetwork(network), 60_000);
2831

2932
describe('vendor-only egress', () => {
33+
it('keeps failed allocation and its cleanup inside the caller deadline', () => {
34+
const shim = mkdtempSync(join(tmpdir(), 'docker-shim-'));
35+
const realDocker = execFileSync('sh', ['-c', 'command -v docker'], { encoding: 'utf8' }).trim();
36+
// Every network operation hangs, so both setup and cleanup can only end by deadline.
37+
writeFileSync(join(shim, 'docker'), ['#!/bin/sh', 'if [ "$1" = network ]; then exec sleep 30; fi',
38+
`exec '${realDocker}' "$@"`].join('\n'), { mode: 0o755 });
39+
const path = process.env.PATH, started = performance.now();
40+
process.env.PATH = `${shim}:${path}`;
41+
try { expect(() => createVendorNetwork(invocation, imageId, 3_000)).toThrow(); }
42+
finally { process.env.PATH = path; rmSync(shim, { recursive: true, force: true }); }
43+
expect(performance.now() - started).toBeLessThan(6_000);
44+
}, 60_000);
45+
3046
it('pins the host list with each vendor profile', () => {
3147
expect(VENDOR_HOSTS).toEqual({ claude: ['api.anthropic.com'], codex: ['api.openai.com', 'chatgpt.com'] });
3248
expect(Object.isFrozen(VENDOR_HOSTS.claude)).toBe(true);

0 commit comments

Comments
 (0)