diff --git a/src/bounded-agent/manager.test.ts b/src/bounded-agent/manager.test.ts index c37a49d0e..def0513ce 100644 --- a/src/bounded-agent/manager.test.ts +++ b/src/bounded-agent/manager.test.ts @@ -493,6 +493,7 @@ describe('prepareBoundedAgents', () => { probe: expect.stringMatching(/^[0-9a-f]{64}$/), }); expect(capabilities.query).not.toBe(capabilities.probe); + expect(fs.statSync(capabilityPath).mode & 0o777).toBe(0o600); expect(probeSbxUnixSocket).toHaveBeenCalledWith('bounded-agent'); }); }); diff --git a/src/bounded-agent/manager.ts b/src/bounded-agent/manager.ts index 4230656ad..2f2b149b9 100644 --- a/src/bounded-agent/manager.ts +++ b/src/bounded-agent/manager.ts @@ -31,6 +31,10 @@ import { resolveBoundedAgentPrimaryBackend, serializeBoundedAgentRuntimeTelemetry, } from './runtime-matrix'; +import { + type SbxIngressCapabilities, + writeSbxIngressCapabilitiesFile, +} from '../bounded-execution/sbx-ingress-capabilities'; /** * Bounded-agent lifecycle orchestration. @@ -163,29 +167,13 @@ export interface PrepareBoundedAgentsDeps { assertPrimaryAvailable?: typeof assertPrimaryRuntimeAvailable; } -interface SbxIngressCapabilities { - version: 1; - query: string; - probe: string; -} - function writeSbxIngressCapabilities(paths: BoundedAgentPaths): void { const capabilities: SbxIngressCapabilities = { version: 1, query: crypto.randomBytes(32).toString('hex'), probe: crypto.randomBytes(32).toString('hex'), }; - const fd = fs.openSync( - paths.capabilityPath, - fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_NOFOLLOW, - 0o600, - ); - try { - fs.writeSync(fd, JSON.stringify(capabilities)); - fs.fchmodSync(fd, 0o600); - } finally { - fs.closeSync(fd); - } + writeSbxIngressCapabilitiesFile(paths.capabilityPath, capabilities); } /** diff --git a/src/bounded-execution/sbx-ingress-capabilities.test.ts b/src/bounded-execution/sbx-ingress-capabilities.test.ts new file mode 100644 index 000000000..8491420b3 --- /dev/null +++ b/src/bounded-execution/sbx-ingress-capabilities.test.ts @@ -0,0 +1,69 @@ +import * as fs from 'fs'; +import { + type SbxIngressCapabilities, + writeSbxIngressCapabilitiesFile, +} from './sbx-ingress-capabilities'; + +const capabilities: SbxIngressCapabilities = { + version: 1, + query: 'a'.repeat(64), + probe: 'b'.repeat(64), +}; + +function createFileOps(overrides: Record = {}) { + return { + open: overrides.open ?? jest.fn(() => 42), + write: overrides.write ?? jest.fn(), + chmod: overrides.chmod ?? jest.fn(), + close: overrides.close ?? jest.fn(), + }; +} + +describe('writeSbxIngressCapabilitiesFile', () => { + it('writes the unchanged payload with exclusive no-follow creation and mode hardening', () => { + const fileOps = createFileOps(); + + writeSbxIngressCapabilitiesFile('/private/capabilities.json', capabilities, fileOps); + + expect(fileOps.open).toHaveBeenCalledWith( + '/private/capabilities.json', + fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_NOFOLLOW, + 0o600, + ); + expect(fileOps.write).toHaveBeenCalledWith(42, JSON.stringify(capabilities)); + expect(fileOps.chmod).toHaveBeenCalledWith(42, 0o600); + expect(fileOps.close).toHaveBeenCalledWith(42); + expect(fileOps.write.mock.invocationCallOrder[0]) + .toBeLessThan(fileOps.chmod.mock.invocationCallOrder[0]); + expect(fileOps.chmod.mock.invocationCallOrder[0]) + .toBeLessThan(fileOps.close.mock.invocationCallOrder[0]); + }); + + it.each(['write', 'chmod'] as const)('closes the descriptor and propagates a %s failure', (operation) => { + const failure = new Error(`${operation} failed`); + const fileOps = createFileOps({ [operation]: jest.fn(() => { throw failure; }) }); + + expect(() => writeSbxIngressCapabilitiesFile('/private/capabilities.json', capabilities, fileOps)) + .toThrow(failure); + expect(fileOps.close).toHaveBeenCalledWith(42); + }); + + it('propagates an open failure without attempting file operations', () => { + const failure = new Error('open failed'); + const fileOps = createFileOps({ open: jest.fn(() => { throw failure; }) }); + + expect(() => writeSbxIngressCapabilitiesFile('/private/capabilities.json', capabilities, fileOps)) + .toThrow(failure); + expect(fileOps.write).not.toHaveBeenCalled(); + expect(fileOps.chmod).not.toHaveBeenCalled(); + expect(fileOps.close).not.toHaveBeenCalled(); + }); + + it('propagates a close failure', () => { + const failure = new Error('close failed'); + const fileOps = createFileOps({ close: jest.fn(() => { throw failure; }) }); + + expect(() => writeSbxIngressCapabilitiesFile('/private/capabilities.json', capabilities, fileOps)) + .toThrow(failure); + }); +}); diff --git a/src/bounded-execution/sbx-ingress-capabilities.ts b/src/bounded-execution/sbx-ingress-capabilities.ts new file mode 100644 index 000000000..3219b36d2 --- /dev/null +++ b/src/bounded-execution/sbx-ingress-capabilities.ts @@ -0,0 +1,45 @@ +import * as fs from 'fs'; + +export interface SbxIngressCapabilities { + version: 1; + query: string; + probe: string; +} + +interface SbxIngressCapabilityFileOps { + open(path: string, flags: number, mode: number): number; + write(fd: number, content: string): void; + chmod(fd: number, mode: number): void; + close(fd: number): void; +} + +const defaultFileOps: SbxIngressCapabilityFileOps = { + open: fs.openSync, + write: fs.writeSync, + chmod: fs.fchmodSync, + close: fs.closeSync, +}; + +/** + * Atomically creates an sbx ingress capability file without following symlinks. + * + * The explicit chmod hardens the final inode mode independently of the process + * umask, and closing in finally ensures write/chmod failures do not leak the fd. + */ +export function writeSbxIngressCapabilitiesFile( + capabilityPath: string, + capabilities: SbxIngressCapabilities, + fileOps: SbxIngressCapabilityFileOps = defaultFileOps, +): void { + const fd = fileOps.open( + capabilityPath, + fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_NOFOLLOW, + 0o600, + ); + try { + fileOps.write(fd, JSON.stringify(capabilities)); + fileOps.chmod(fd, 0o600); + } finally { + fileOps.close(fd); + } +} diff --git a/src/bounded-query/manager.test.ts b/src/bounded-query/manager.test.ts index 9a49196cd..8a3af7220 100644 --- a/src/bounded-query/manager.test.ts +++ b/src/bounded-query/manager.test.ts @@ -127,12 +127,14 @@ describe('prepareBoundedQueries', () => { expect(fs.existsSync(paths.capabilityPath)).toBe(!supported); if (!supported) { const raw = fs.readFileSync(paths.capabilityPath, 'utf8'); + const capabilities = JSON.parse(raw); expect(raw).not.toContain('GH_TOKEN'); - expect(JSON.parse(raw)).toEqual({ + expect(capabilities).toEqual({ version: 1, query: expect.stringMatching(/^[0-9a-f]{64}$/), probe: expect.stringMatching(/^[0-9a-f]{64}$/), }); + expect(capabilities.query).not.toBe(capabilities.probe); expect(fs.statSync(paths.capabilityPath).mode & 0o777).toBe(0o600); } }); diff --git a/src/bounded-query/manager.ts b/src/bounded-query/manager.ts index 6790d41cd..60dafbdbf 100644 --- a/src/bounded-query/manager.ts +++ b/src/bounded-query/manager.ts @@ -31,6 +31,10 @@ import { resolveBoundedQueryPrimaryBackend, serializeBoundedQueryRuntimeTelemetry, } from './runtime-matrix'; +import { + type SbxIngressCapabilities, + writeSbxIngressCapabilitiesFile, +} from '../bounded-execution/sbx-ingress-capabilities'; /** * Bounded-query lifecycle orchestration. @@ -156,29 +160,13 @@ export interface PrepareBoundedQueriesDeps { assertPrimaryAvailable?: typeof assertPrimaryRuntimeAvailable; } -interface SbxIngressCapabilities { - version: 1; - query: string; - probe: string; -} - function writeSbxIngressCapabilities(paths: BoundedQueryPaths): void { const capabilities: SbxIngressCapabilities = { version: 1, query: crypto.randomBytes(32).toString('hex'), probe: crypto.randomBytes(32).toString('hex'), }; - const fd = fs.openSync( - paths.capabilityPath, - fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_NOFOLLOW, - 0o600, - ); - try { - fs.writeSync(fd, JSON.stringify(capabilities)); - fs.fchmodSync(fd, 0o600); - } finally { - fs.closeSync(fd); - } + writeSbxIngressCapabilitiesFile(paths.capabilityPath, capabilities); } /**