Skip to content

Commit 463ed9f

Browse files
mchwangclaude
andcommitted
Close D2 auth swap, unowned cleanup, and restart gaps
- Read Codex auth through one no-follow descriptor instead of lstat, realpath, then open, so the path cannot be swapped to another host file. - Retain staged credentials and fail when the agent container name is held by another invocation, instead of treating it as cleaned up. - Require Docker's default no-restart policy before start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent fe8bd33 commit 463ed9f

3 files changed

Lines changed: 30 additions & 11 deletions

File tree

‎agents/container/profile.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@ const removeOwnedDirectories = (directories: readonly string[]) => {
6060
const readCapturedFile = (path: string, kind: string): { identity: FileIdentity; content: Buffer } => {
6161
let fd: number | undefined;
6262
try {
63-
fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW);
63+
try { fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW); }
64+
catch (error) {
65+
if ((error as NodeJS.ErrnoException).code === 'ELOOP') throw new Error(`${kind} must be a direct regular file, not a link.`);
66+
throw error;
67+
}
6468
const before = fstatSync(fd);
6569
const maximum = 1024 * 1024;
6670
if (!before.isFile() || before.nlink !== 1 || before.size > maximum)
@@ -150,9 +154,8 @@ export function createContainerProfile(options: ProfileOptions): ContainerProfil
150154
if (!/^codeboost-work-[0-9a-f-]+$/.test(filesystems.workVolume)
151155
|| !/^codeboost-metadata-[0-9a-f-]+$/.test(filesystems.metadataVolume)
152156
|| !/^codeboost-keeper-[0-9a-f-]+$/.test(filesystems.keeper)) throw new Error('Task filesystem identity is invalid.');
153-
if (options.codexAuthFile && !lstatSync(options.codexAuthFile).isFile())
154-
throw new Error('Codex auth must be a direct regular file, not a link.');
155-
const sourceAuth = options.codexAuthFile ? readCapturedFile(realpathSync(options.codexAuthFile), 'Codex auth') : undefined;
157+
// Read through one no-follow descriptor so the path cannot be swapped between check and open.
158+
const sourceAuth = options.codexAuthFile ? readCapturedFile(options.codexAuthFile, 'Codex auth') : undefined;
156159
const cleanupDirectories: string[] = [];
157160
let codexAuthFile: string | undefined, authIdentity: FileIdentity | undefined;
158161
try {

‎agents/container/run.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,8 @@ const removeContainerOrThrow = (profile: ContainerProfile) => {
5454
return;
5555
}
5656
const inspected = JSON.parse(before.stdout || '[]')[0] as { Config?: { Labels?: Record<string, string> } } | undefined;
57-
if (inspected?.Config?.Labels?.['io.codeboost.invocation'] !== profile.ownershipId) {
58-
disposeContainerProfile(profile);
59-
return;
60-
}
57+
if (inspected?.Config?.Labels?.['io.codeboost.invocation'] !== profile.ownershipId)
58+
throw new Error('Agent container name is held by another invocation; staged credentials were retained.');
6159
const result = spawnSync('docker', ['rm', '--force', profile.name], {
6260
encoding: 'utf8', timeout: remaining(), env: dockerEnvironment(), stdio: ['ignore', 'pipe', 'pipe'],
6361
});
@@ -87,6 +85,7 @@ type Inspect = {
8785
Ulimits: unknown[] | null; CpuCount: number;
8886
CpuPercent: number; IOMaximumBandwidth: number; IOMaximumIOps: number; DeviceCgroupRules: unknown[] | null;
8987
StorageOpt?: Record<string, string> | null; CgroupParent: string;
88+
RestartPolicy?: { Name?: string; MaximumRetryCount?: number } | null;
9089
Devices: unknown[] | null; DeviceRequests: unknown[] | null; Tmpfs: Record<string, string> | null;
9190
Mounts: Array<{ Type: string; Source: string; Target: string; ReadOnly: boolean }> | null };
9291
Mounts: Array<{ Type: string; Name?: string; Source: string; Destination: string; RW: boolean }>;
@@ -129,7 +128,8 @@ export function validateContainer(container: string, profile: ContainerProfile,
129128
|| host.BlkioWeightDevice?.length || host.BlkioDeviceReadBps?.length || host.BlkioDeviceWriteBps?.length
130129
|| host.BlkioDeviceReadIOps?.length || host.BlkioDeviceWriteIOps?.length || host.Ulimits?.length
131130
|| host.CpuCount !== 0 || host.CpuPercent !== 0 || host.IOMaximumBandwidth !== 0 || host.IOMaximumIOps !== 0
132-
|| host.DeviceCgroupRules !== null || host.StorageOpt != null || host.CgroupParent !== '')
131+
|| host.DeviceCgroupRules !== null || host.StorageOpt != null || host.CgroupParent !== ''
132+
|| !['', 'no'].includes(host.RestartPolicy?.Name ?? '') || (host.RestartPolicy?.MaximumRetryCount ?? 0) !== 0)
133133
throw new Error('Container daemon configuration is missing required lockdown.');
134134
const tmpfs = host.Tmpfs ?? {};
135135
const expectedTmpfs = new Map([

‎test/agent-container.test.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { execFileSync, spawnSync } from 'node:child_process';
22
import { randomBytes, randomUUID } from 'node:crypto';
3-
import { chmodSync, mkdtempSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs';
3+
import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs';
44
import { tmpdir } from 'node:os';
55
import { join } from 'node:path';
66
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
@@ -264,12 +264,28 @@ describe('real Docker agent isolation', () => {
264264
inputDirectory: data.input, command: ['true'], codexAuthFile: data.fakeAuth, imageId });
265265
profiles.push(first, duplicate);
266266
docker(...first.args); containers.add(first.name);
267-
expect(() => createValidatedContainer(duplicate)).toThrow();
267+
expect(() => createValidatedContainer(duplicate)).toThrow('Container creation failed and cleanup did not settle.');
268+
expect(existsSync(duplicate.codexAuthFile!)).toBe(true);
268269
const state = JSON.parse(docker('container', 'inspect', first.name))[0] as { State: { Status: string } };
269270
expect(state.State.Status).toBe('created');
270271
docker('rm', '--force', first.name); containers.delete(first.name);
271272
}, 60_000);
272273

274+
it('refuses a Codex auth path that is a link without resolving it', () => {
275+
const data = fixture(), link = join(data.root, 'auth-link.json');
276+
symlinkSync(data.fakeAuth, link);
277+
expect(() => profile(data, 'planning', ['true'], { codexAuthFile: link })).toThrow('not a link');
278+
}, 60_000);
279+
280+
it('rejects a restart policy that could relaunch the agent after it exits', () => {
281+
const data = fixture(), valid = profile(data, 'planning', ['true']);
282+
const imageIndex = valid.args.indexOf(imageId);
283+
docker(...valid.args.slice(0, imageIndex), '--restart=always', ...valid.args.slice(imageIndex));
284+
containers.add(valid.name);
285+
expect(() => validateContainer(valid.name, valid)).toThrow('lockdown');
286+
docker('rm', '--force', valid.name); containers.delete(valid.name);
287+
}, 60_000);
288+
273289
it('rejects added capabilities and conflicting or duplicate filesystem options', () => {
274290
const data = fixture(), valid = profile(data, 'planning', ['true']);
275291
const imageIndex = valid.args.indexOf(imageId);

0 commit comments

Comments
 (0)