Skip to content

Commit 067c713

Browse files
mchwangclaude
andcommitted
Track the ack-script container by name; time limits on the moved and attached calls (#189)
Review round 50: agent-supervisor's label-less ack-script container is tracked by name before its create and removed by afterAll; runner-workspace's docker helper, which now runs calls this branch moved out of a try, takes CLIENT_LIMIT; the seccomp probe and agent-network's attached helper runs are killed with SIGKILL at their limit, as an attached run forwards SIGTERM; the killed-create test's listing reports its own failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 820c2a2 commit 067c713

4 files changed

Lines changed: 19 additions & 9 deletions

File tree

‎test/agent-container.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -973,8 +973,11 @@ describe('real Docker agent isolation', () => {
973973
].join('\n');
974974
const created = await withDockerShim(['network', 'create'], foreign, () =>
975975
createVendorNetwork(invocation(data.clone, 'planning'), imageId, allocationId).then(() => undefined, error => error));
976-
const left = spawnSync('docker', ['network', 'ls', '--quiet', '--filter', `label=io.codeboost.allocation=${allocationId}`],
977-
{ encoding: 'utf8', timeout: 60_000, killSignal: 'SIGKILL' }).stdout?.trim() ?? '';
976+
const listing = spawnSync('docker', ['network', 'ls', '--quiet', '--filter', `label=io.codeboost.allocation=${allocationId}`],
977+
{ encoding: 'utf8', ...CLIENT_LIMIT });
978+
// A listing that failed fails here with its cause, as main's helper did.
979+
expect(listing.status, listing.error?.message ?? (listing.signal ? `killed by ${listing.signal}` : listing.stderr)).toBe(0);
980+
const left = listing.stdout.trim();
978981
expect(created).toBeInstanceOf(VendorNetworkCreationCleanupError);
979982
const cleanup = (created as VendorNetworkCreationCleanupError).errors[1] as AggregateError;
980983
expect(cleanup.errors.map(error => (error as Error).message)).toEqual(['Refused to remove unowned vendor network.']);
@@ -2639,7 +2642,7 @@ describe('real Docker agent isolation', () => {
26392642
containers.add(name);
26402643
const result = spawnSync('docker', ['run', '--rm', '--name', name, '--read-only', '--user', '10001:10001', '--cap-drop=ALL',
26412644
'--security-opt=no-new-privileges', '--security-opt=seccomp=unconfined', '--network=none', imageId, 'true'],
2642-
{ encoding: 'utf8', timeout: 60_000 });
2645+
{ encoding: 'utf8', ...CLIENT_LIMIT });
26432646
expect(result.status).toBe(78);
26442647
expect(result.stderr).toContain('seccomp syscall filter must be enforced');
26452648
}, 60_000);

‎test/agent-network.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const docker = (...args: string[]) => execFileSync('docker', args, {
2727
const curl = (url: string, direct = false) => spawnSync('docker', ['run', '--rm', ...fileLabel, `--network=${network.name}`,
2828
'--env', `HTTPS_PROXY=${network.proxyUrl}`, ...(direct ? ['--env', 'NO_PROXY=*'] : []),
2929
'--entrypoint', 'curl', imageId, '--silent', '--show-error', '--output', '/dev/null', '--write-out', '%{http_code}',
30-
'--max-time', '15', url], { encoding: 'utf8', timeout: 30_000, stdio: ['ignore', 'pipe', 'pipe'] });
30+
'--max-time', '15', url], { encoding: 'utf8', timeout: 30_000, killSignal: 'SIGKILL', stdio: ['ignore', 'pipe', 'pipe'] });
3131

3232
beforeAll(async () => {
3333
imageId = buildAgentImage();
@@ -316,7 +316,7 @@ describe('vendor-only egress', () => {
316316
"const dns=require('node:dns');dns.setServers(['127.0.0.11']);",
317317
"dns.resolve4('example.com',(error)=>process.exit(error?0:1));",
318318
'setTimeout(()=>process.exit(0),3000);',
319-
].join('')], { encoding: 'utf8', timeout: 10_000, stdio: ['ignore', 'pipe', 'pipe'] });
319+
].join('')], { encoding: 'utf8', timeout: 10_000, killSignal: 'SIGKILL', stdio: ['ignore', 'pipe', 'pipe'] });
320320
expect(result.status).toBe(0);
321321
}, 30_000);
322322

@@ -340,7 +340,7 @@ describe('vendor-only egress', () => {
340340
'--entrypoint', 'node', imageId, '-e', "require('node:net').createServer(()=>{}).listen(4567,'0.0.0.0');setInterval(()=>{},1000)");
341341
const result = spawnSync('docker', ['run', '--rm', ...fileLabel, `--network=${other.name}`, '--entrypoint', 'node', imageId,
342342
'-e', "const s=require('node:net').connect(4567,'codeboost-peer');s.on('connect',()=>process.exit(0));s.on('error',()=>process.exit(1));setTimeout(()=>process.exit(2),3000)"],
343-
{ encoding: 'utf8', timeout: 10_000, stdio: ['ignore', 'pipe', 'pipe'] });
343+
{ encoding: 'utf8', timeout: 10_000, killSignal: 'SIGKILL', stdio: ['ignore', 'pipe', 'pipe'] });
344344
expect(result.status).not.toBe(0);
345345
} finally {
346346
spawnSync('docker', ['rm', '--force', peer], { stdio: 'ignore' });

‎test/agent-supervisor.test.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ const testOwner = (attemptId = 'fixture') => ({ runnerOwner: TEST_RUNNER_OWNER,
3030

3131
const roots: string[] = [], profiles: ContainerProfile[] = [];
3232
const allocations: ReturnType<typeof prepareTaskFilesystems>[] = [];
33+
/** Label-less containers, by name, removed in afterAll should their create land after their test's finally (#189). */
34+
const unlabelled = new Set<string>();
3335
let imageId = '';
3436
const git = fixtureGit;
3537

@@ -70,6 +72,7 @@ async function profile(data: ReturnType<typeof fixture>, probe: IsolationProbe,
7072
beforeAll(async () => { imageId = buildAgentImage(); }, 10 * 60_000);
7173
afterAll(async () => {
7274
try {
75+
for (const name of unlabelled) removeByName('container', name, 'afterAll');
7376
for (const profile of profiles) await disposeContainerProfile(profile);
7477
for (const allocation of allocations.reverse()) removeTaskFilesystems(allocation);
7578
for (const root of roots.reverse()) {
@@ -653,8 +656,11 @@ describe('container invocation supervisor', () => {
653656
it('writes the deferred output acknowledgement idempotently, repairing a partial one', () => {
654657
const token = randomUUID(), file = `/run/codeboost-control/collected-${token}`;
655658
const name = `codeboost-ack-script-${randomUUID()}`;
659+
// Tracked before it is made: it has no labels, and a create that lands after its client was killed is never started,
660+
// so `--rm` does not apply and only removal by name finds it (#189).
661+
unlabelled.add(name);
656662
try {
657-
// Inside the try: the container has no labels, so only the finally below removes it (#189).
663+
// Inside the try: the container has no labels, so the finally below removes it, and afterAll by name (#189).
658664
execFileSync('docker', ['run', '-d', '--rm', '--name', name, '--network', 'none', '--cap-drop=ALL',
659665
'--security-opt=no-new-privileges', '--user', '10001:10001', '--entrypoint', 'sleep',
660666
'--tmpfs', '/run/codeboost-control:rw,nosuid,nodev,noexec,size=65536,nr_inodes=16,uid=0,gid=0,mode=0711',

‎test/runner-workspace.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { attemptRef, openRunnerRepository } from '../runner/runner-repository.ts
1515
import { ReviewService } from '../runner/review.ts';
1616
import { Store } from '../runner/store.ts';
1717
import { createTaskWorkspace } from '../runner/workspace.ts';
18-
import { removeRunnerObjects, requireRealDocker } from './fixtures/docker-sweep.ts';
18+
import { CLIENT_LIMIT, removeRunnerObjects, requireRealDocker } from './fixtures/docker-sweep.ts';
1919
import { fixtureGit as git } from './fixtures/git.ts';
2020

2121
// F2b against the real lane D (#87): each item runs in fresh task storage cloned from the runner-owned repository, an
@@ -43,7 +43,8 @@ afterAll(() => {
4343
removeRunnerObjects(RUNNER_OWNER);
4444
}
4545
}, 7 * 60_000);
46-
const docker = (...args: string[]) => execFileSync('docker', args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim();
46+
const docker = (...args: string[]) => execFileSync('docker', args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'],
47+
...CLIENT_LIMIT }).trim();
4748

4849
async function setup(agent: Record<string, string>) {
4950
const root = mkdtempSync(join(tmpdir(), 'runner-workspace-')); roots.push(root);

0 commit comments

Comments
 (0)