From 314b6e4eb129b7974f24935da294789d92d7e6f9 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sat, 8 Aug 2026 18:53:34 -0700 Subject: [PATCH 1/4] feat: enforce Firecracker isolated networking Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/firecracker-runtime-backend.ts | 6 +- src/firecracker/api-client.test.ts | 14 + src/firecracker/manager.test.ts | 125 +++++- src/firecracker/manager.ts | 64 ++- src/firecracker/network.test.ts | 301 ++++++++++++++ src/firecracker/network.ts | 616 +++++++++++++++++++++++++++++ src/types/runtime-options.ts | 4 +- 7 files changed, 1111 insertions(+), 19 deletions(-) create mode 100644 src/firecracker/network.test.ts create mode 100644 src/firecracker/network.ts diff --git a/src/firecracker-runtime-backend.ts b/src/firecracker-runtime-backend.ts index 4173dc777..88a142d2b 100644 --- a/src/firecracker-runtime-backend.ts +++ b/src/firecracker-runtime-backend.ts @@ -5,7 +5,7 @@ import type { WrapperConfig } from './types'; export const FIRECRACKER_INCOMPLETE_CAPABILITY_ERROR = 'Firecracker runtime workload execution is unavailable in this preview: ' + - 'networking and guest agent/vsock execution are not implemented'; + 'workspace image and guest command execution are not implemented'; export interface FirecrackerRuntimeBackendDependencies { startInfrastructure: WorkflowDependencies['startContainers']; @@ -16,7 +16,9 @@ export interface FirecrackerRuntimeBackendDependencies { * Fail-closed backend boundary for the Firecracker control-plane preview. * * The manager primitives are intentionally not dispatched by the main workflow - * until networking and guest command execution land in later stack layers. + * until workspace preparation and guest command execution land in later stack + * layers. FirecrackerManager separately refuses to launch without host-side + * network enforcement. */ export class FirecrackerRuntimeBackend implements ExternalAgentRuntimeBackend { readonly runtime = 'firecracker'; diff --git a/src/firecracker/api-client.test.ts b/src/firecracker/api-client.test.ts index feda29e1c..76c2015c3 100644 --- a/src/firecracker/api-client.test.ts +++ b/src/firecracker/api-client.test.ts @@ -59,6 +59,11 @@ describe('FirecrackerApiClient', () => { is_root_device: true, is_read_only: false, }); + await client.putNetworkInterface({ + iface_id: 'primary interface', + host_dev_name: 'fct123456789012', + guest_mac: '02:00:00:00:00:01', + }); await client.instanceStart(); expect(received).toEqual([ @@ -77,6 +82,15 @@ describe('FirecrackerApiClient', () => { is_read_only: false, }), }, + { + method: 'PUT', + url: '/network-interfaces/primary%20interface', + body: JSON.stringify({ + iface_id: 'primary interface', + host_dev_name: 'fct123456789012', + guest_mac: '02:00:00:00:00:01', + }), + }, { method: 'PUT', url: '/actions', diff --git a/src/firecracker/manager.test.ts b/src/firecracker/manager.test.ts index cd8e59af0..f78ad29c3 100644 --- a/src/firecracker/manager.test.ts +++ b/src/firecracker/manager.test.ts @@ -5,7 +5,12 @@ import { FirecrackerManager, createFirecrackerRunPaths, type FirecrackerManagerDependencies, + type FirecrackerManagerNetworkConfig, } from './manager'; +import type { + FirecrackerNetworkLifecycle, + FirecrackerNetworkPlan, +} from './network'; function config(overrides: Partial = {}): FirecrackerOptions { return { @@ -34,6 +39,24 @@ function processMock(): ExecaChildProcess { return child; } +function networkConfig( + overrides: Partial = {}, +): FirecrackerManagerNetworkConfig { + return { + infrastructureBridge: 'awfbr0', + enableApiProxy: true, + ...overrides, + }; +} + +function networkLifecycle(plan: FirecrackerNetworkPlan): FirecrackerNetworkLifecycle { + return { + plan, + setup: jest.fn().mockResolvedValue(plan), + cleanup: jest.fn().mockResolvedValue(undefined), + }; +} + function dependencies( overrides: Partial = {}, ): FirecrackerManagerDependencies { @@ -41,6 +64,7 @@ function dependencies( putMachineConfig: jest.fn().mockResolvedValue(undefined), putBootSource: jest.fn().mockResolvedValue(undefined), putDrive: jest.fn().mockResolvedValue(undefined), + putNetworkInterface: jest.fn().mockResolvedValue(undefined), instanceStart: jest.fn().mockResolvedValue(undefined), } as unknown as FirecrackerApiClient; return { @@ -60,6 +84,7 @@ function dependencies( rm: jest.fn().mockResolvedValue(undefined), sleep: jest.fn().mockResolvedValue(undefined), createClient: jest.fn().mockReturnValue(client), + createNetwork: jest.fn((plan) => networkLifecycle(plan)), resolveIdentity: jest.fn().mockReturnValue({ uid: 1000, gid: 1000 }), ...overrides, }; @@ -90,7 +115,13 @@ describe('FirecrackerManager', () => { it('launches jailer and configures machine, kernel, and root drive', async () => { const deps = dependencies(); - const manager = new FirecrackerManager(config(), '/tmp/awf', deps, 'run-1'); + const manager = new FirecrackerManager( + config(), + '/tmp/awf', + deps, + 'run-1', + networkConfig(), + ); const client = await manager.start(); expect(deps.launch).toHaveBeenCalledWith( @@ -98,6 +129,7 @@ describe('FirecrackerManager', () => { expect.arrayContaining([ '--id', 'run-1', '--exec-file', '/opt/firecracker', + '--netns', expect.stringMatching(/^\/var\/run\/netns\/awffc-/), '--api-sock', '/run/firecracker.socket', ]), expect.objectContaining({ reject: false }), @@ -114,6 +146,23 @@ describe('FirecrackerManager', () => { path_on_host: '/rootfs', is_root_device: true, })); + expect(client.putNetworkInterface).toHaveBeenCalledWith({ + iface_id: 'eth0', + host_dev_name: expect.stringMatching(/^fct[0-9a-f]{12}$/), + guest_mac: expect.any(String), + }); + const configuredNetwork = (client.putNetworkInterface as jest.Mock) + .mock.calls[0][0] as { guest_mac: string }; + expect(configuredNetwork.guest_mac.split(':')).toHaveLength(6); + expect(configuredNetwork.guest_mac.startsWith('02:')).toBe(true); + expect(deps.createNetwork).toHaveBeenCalledWith(expect.objectContaining({ + infrastructureBridge: 'awfbr0', + jailerUid: 1000, + jailerGid: 1000, + })); + const lifecycle = (deps.createNetwork as jest.Mock).mock.results[0] + .value as FirecrackerNetworkLifecycle; + expect(lifecycle.setup).toHaveBeenCalledTimes(1); }); it('terminates the partial process and removes its jail on readiness failure', async () => { @@ -124,7 +173,13 @@ describe('FirecrackerManager', () => { access: jest.fn().mockRejectedValue(missing), sleep: jest.fn(async () => new Promise((resolve) => setTimeout(resolve, 2))), }); - const manager = new FirecrackerManager(config(), '/tmp/awf', deps, 'partial'); + const manager = new FirecrackerManager( + config(), + '/tmp/awf', + deps, + 'partial', + networkConfig(), + ); await expect(manager.start()).rejects.toThrow(/API socket was not ready/); expect(child.kill).toHaveBeenCalledWith( @@ -135,6 +190,72 @@ describe('FirecrackerManager', () => { '/tmp/awf/firecracker-jailer/firecracker/partial', { recursive: true, force: true }, ); + const lifecycle = (deps.createNetwork as jest.Mock).mock.results[0] + .value as FirecrackerNetworkLifecycle; + expect(lifecycle.cleanup).toHaveBeenCalledTimes(1); + }); + + it('refuses to launch without host-side network enforcement', async () => { + const deps = dependencies(); + const manager = new FirecrackerManager(config(), '/tmp/awf', deps, 'unsafe'); + + await expect(manager.start()).rejects.toThrow(/unfiltered microVM/); + expect(deps.preflight).not.toHaveBeenCalled(); + expect(deps.launch).not.toHaveBeenCalled(); + }); + + it('cleans up the network before removing the jail', async () => { + const order: string[] = []; + const deps = dependencies({ + createNetwork: jest.fn((plan) => ({ + plan, + setup: jest.fn().mockResolvedValue(plan), + cleanup: jest.fn(async () => { + order.push('network'); + }), + })), + rm: jest.fn(async () => { + order.push('jail'); + }), + }); + const manager = new FirecrackerManager( + config(), + '/tmp/awf', + deps, + 'cleanup', + networkConfig(), + ); + + await manager.start(); + await manager.stop(); + + expect(order).toEqual(['network', 'jail']); + }); + + it('rolls back the network when typed NIC configuration fails', async () => { + const client = { + putMachineConfig: jest.fn().mockResolvedValue(undefined), + putBootSource: jest.fn().mockResolvedValue(undefined), + putDrive: jest.fn().mockResolvedValue(undefined), + putNetworkInterface: jest.fn().mockRejectedValue(new Error('invalid NIC')), + } as unknown as FirecrackerApiClient; + const deps = dependencies({ + createClient: jest.fn().mockReturnValue(client), + }); + const manager = new FirecrackerManager( + config(), + '/tmp/awf', + deps, + 'nic-failure', + networkConfig(), + ); + + await expect(manager.start()).rejects.toThrow('invalid NIC'); + + const lifecycle = (deps.createNetwork as jest.Mock).mock.results[0] + .value as FirecrackerNetworkLifecycle; + expect(lifecycle.cleanup).toHaveBeenCalledTimes(1); + expect(deps.rm).toHaveBeenCalled(); }); it('fails fast when jailer exits by signal before API readiness', async () => { diff --git a/src/firecracker/manager.ts b/src/firecracker/manager.ts index 365e9bb7b..bc4e8c365 100644 --- a/src/firecracker/manager.ts +++ b/src/firecracker/manager.ts @@ -4,6 +4,14 @@ import * as path from 'path'; import execa, { type ExecaChildProcess } from 'execa'; import type { FirecrackerOptions } from '../types/runtime-options'; import { FirecrackerApiClient } from './api-client'; +import { + FirecrackerNetworkManager, + assertSafeFirecrackerRunId, + createFirecrackerNetworkPlan, + type FirecrackerControlPeer, + type FirecrackerNetworkLifecycle, + type FirecrackerNetworkPlan, +} from './network'; import { runFirecrackerPreflight } from './preflight'; const API_SOCKET_NAME = 'firecracker.socket'; @@ -38,9 +46,16 @@ export interface FirecrackerManagerDependencies { rm(directory: string, options: { recursive: true; force: true }): Promise; sleep(milliseconds: number): Promise; createClient(socketPath: string, timeoutMs: number): FirecrackerApiClient; + createNetwork(plan: FirecrackerNetworkPlan): FirecrackerNetworkLifecycle; resolveIdentity(): { uid: number; gid: number }; } +export interface FirecrackerManagerNetworkConfig { + infrastructureBridge: string; + enableApiProxy: boolean; + controlPeer?: FirecrackerControlPeer; +} + const defaultDependencies: FirecrackerManagerDependencies = { preflight: runFirecrackerPreflight, launch: (command, args, options) => execa(command, args, options), @@ -52,6 +67,7 @@ const defaultDependencies: FirecrackerManagerDependencies = { rm: fs.rm, sleep: (milliseconds) => new Promise((resolve) => setTimeout(resolve, milliseconds)), createClient: (socketPath, timeoutMs) => new FirecrackerApiClient({ socketPath, timeoutMs }), + createNetwork: (plan) => new FirecrackerNetworkManager(plan), resolveIdentity: resolveJailerIdentity, }; @@ -76,9 +92,7 @@ export function createFirecrackerRunPaths( firecrackerBinary: string, runId = `awf-${process.pid}-${randomBytes(6).toString('hex')}`, ): FirecrackerRunPaths { - if (!/^[A-Za-z0-9-]{1,64}$/.test(runId)) { - throw new Error(`Unsafe Firecracker run id: ${runId}`); - } + assertSafeFirecrackerRunId(runId); const chrootBaseDir = path.join(workDir, 'firecracker-jailer'); const jailRoot = path.join( chrootBaseDir, @@ -103,21 +117,36 @@ export class FirecrackerManager { readonly paths: FirecrackerRunPaths; private process: ExecaChildProcess | undefined; private client: FirecrackerApiClient | undefined; + private network: FirecrackerNetworkLifecycle | undefined; constructor( private readonly config: FirecrackerOptions, workDir: string, private readonly dependencies: FirecrackerManagerDependencies = defaultDependencies, runId?: string, + private readonly networkConfig?: FirecrackerManagerNetworkConfig, ) { this.paths = createFirecrackerRunPaths(workDir, config.firecrackerBinary, runId); } async start(): Promise { + if (!this.networkConfig) { + throw new Error( + 'Firecracker network configuration is required; refusing to launch an unfiltered microVM', + ); + } + let startupError: unknown; try { const artifacts = await this.dependencies.preflight(this.config); const identity = this.dependencies.resolveIdentity(); + const networkPlan = createFirecrackerNetworkPlan(this.paths.runId, { + ...this.networkConfig, + jailerUid: identity.uid, + jailerGid: identity.gid, + }); + this.network = this.dependencies.createNetwork(networkPlan); + await this.network.setup(); await this.dependencies.mkdir(this.paths.chrootBaseDir, { recursive: true, mode: 0o700, @@ -131,6 +160,7 @@ export class FirecrackerManager { '--uid', String(identity.uid), '--gid', String(identity.gid), '--chroot-base-dir', this.paths.chrootBaseDir, + '--netns', networkPlan.netnsPath, '--', '--api-sock', `/run/${API_SOCKET_NAME}`, ], @@ -162,6 +192,7 @@ export class FirecrackerManager { is_root_device: true, is_read_only: false, }); + await this.client.putNetworkInterface(networkPlan.networkInterface); return this.client; } catch (error) { startupError = error; @@ -184,7 +215,7 @@ export class FirecrackerManager { } async stop(): Promise { - let processError: unknown; + const errors: unknown[] = []; if (this.process && this.process.exitCode === null && !this.process.killed) { const child = this.process; try { @@ -194,12 +225,19 @@ export class FirecrackerManager { throw new Error('Firecracker process termination was not confirmed'); } } catch (error) { - processError = error; + errors.push(error); } } this.process = undefined; this.client = undefined; + try { + await this.network?.cleanup(); + } catch (error) { + errors.push(error); + } + this.network = undefined; + try { await this.dependencies.rm( path.join( @@ -210,15 +248,15 @@ export class FirecrackerManager { { recursive: true, force: true }, ); } catch (error) { - if (processError) { - throw new Error( - `Failed to terminate Firecracker: ${formatError(processError)}; ` + - `failed to remove jail: ${formatError(error)}`, - ); - } - throw error; + errors.push(error); + } + + if (errors.length === 1) throw errors[0]; + if (errors.length > 1) { + throw new Error( + `Firecracker cleanup failed: ${errors.map(formatError).join('; ')}`, + ); } - if (processError) throw processError; } private async waitForApiSocket(): Promise { diff --git a/src/firecracker/network.test.ts b/src/firecracker/network.test.ts new file mode 100644 index 000000000..731582478 --- /dev/null +++ b/src/firecracker/network.test.ts @@ -0,0 +1,301 @@ +import { + FirecrackerLinuxNetworkCommands, + FirecrackerNetworkManager, + createFirecrackerNetworkPlan, + generateFirecrackerNftRuleset, + type FirecrackerConnectivityProbe, + type FirecrackerNetworkCommandOptions, + type FirecrackerNetworkPlan, +} from './network'; + +interface CommandCall { + command: string; + args: readonly string[]; + options: FirecrackerNetworkCommandOptions; +} + +function createPlan( + runId = 'run-123', + overrides: Partial[1]> = {}, +): FirecrackerNetworkPlan { + return createFirecrackerNetworkPlan(runId, { + infrastructureBridge: 'awfbr0', + enableApiProxy: true, + jailerUid: 1000, + jailerGid: 1000, + ...overrides, + }); +} + +function commandHarness(failAt?: number): { + calls: CommandCall[]; + commands: FirecrackerLinuxNetworkCommands; +} { + const calls: CommandCall[] = []; + let rejectingCall = 0; + const commands = new FirecrackerLinuxNetworkCommands( + jest.fn(async (command, args, options) => { + calls.push({ command, args, options }); + if (options.reject && ++rejectingCall === failAt) { + throw new Error(`stage ${failAt} failed`); + } + }), + ); + return { calls, commands }; +} + +describe('Firecracker network planning', () => { + it('allocates deterministic, disjoint per-run guest addressing and bounded names', () => { + const first = createPlan('run-123'); + const same = createPlan('run-123'); + const second = createPlan('run-456'); + + expect(first).toEqual(same); + expect(second.guestSubnet).not.toBe(first.guestSubnet); + expect(second.guestMac).not.toBe(first.guestMac); + expect(first.guestSubnet).toMatch(/^100\.(?:6[4-9]|[78]\d|9\d|1[01]\d|12[0-7])\.\d+\.\d+\/30$/); + expect(first.guestGatewayIp).not.toBe(first.guestIp); + expect(first.infrastructureIp).toBe('172.30.0.20'); + expect(first.infrastructureCidr).toBe('172.30.0.0/24'); + expect(first.netnsPath).toBe(`/var/run/netns/${first.namespaceName}`); + expect(first.networkInterface).toEqual({ + iface_id: 'eth0', + host_dev_name: first.tapName, + guest_mac: first.guestMac, + }); + for (const name of [ + first.tapName, + first.hostVethName, + first.namespaceVethName, + first.infrastructureBridge, + ]) { + expect(name.length).toBeLessThanOrEqual(15); + expect(name).toMatch(/^[A-Za-z0-9_.-]+$/); + } + }); + + it('derives exact service endpoints from centralized proxy policy', () => { + const enabled = createPlan(); + const disabled = createPlan('without-api', { enableApiProxy: false }); + const withControl = createPlan('control-peer', { + controlPeer: { ip: '172.30.0.60', ports: [8443, 8444] }, + }); + + expect(enabled.allowedEndpoints).toEqual([ + { name: 'squid', ip: '172.30.0.10', port: 3128 }, + { name: 'api-proxy-openai', ip: '172.30.0.30', port: 10000 }, + { name: 'api-proxy-anthropic', ip: '172.30.0.30', port: 10001 }, + { name: 'api-proxy-copilot', ip: '172.30.0.30', port: 10002 }, + { name: 'api-proxy-gemini', ip: '172.30.0.30', port: 10003 }, + { name: 'api-proxy-vertex', ip: '172.30.0.30', port: 10004 }, + ]); + expect(disabled.allowedEndpoints).toEqual([ + { name: 'squid', ip: '172.30.0.10', port: 3128 }, + ]); + expect(withControl.allowedEndpoints).toEqual(expect.arrayContaining([ + { name: 'control-peer', ip: '172.30.0.60', port: 8443 }, + { name: 'control-peer', ip: '172.30.0.60', port: 8444 }, + ])); + }); + + it('rejects unsafe names, identities, peers, and direct DNS before execution', () => { + expect(() => createPlan('../escape')).toThrow(/run id/); + expect(() => createPlan('underscore_is_not_valid')).toThrow(/run id/); + expect(() => createPlan('a'.repeat(65))).toThrow(/run id/); + expect(() => createPlan('bad-bridge', { + infrastructureBridge: 'bridge-name-is-too-long', + })).toThrow(/IFNAMSIZ/); + expect(() => createPlan('root-owner', { jailerUid: 0 })).toThrow(/uid/); + expect(() => createPlan('public-peer', { + controlPeer: { ip: '8.8.8.8', ports: [443] }, + })).toThrow(/RFC1918/); + expect(() => createPlan('metadata-peer', { + controlPeer: { ip: '169.254.169.254', ports: [443] }, + })).toThrow(/RFC1918/); + expect(() => createPlan('dns-peer', { + controlPeer: { ip: '172.30.0.60', ports: [53] }, + })).toThrow(/direct DNS/); + expect(() => createPlan('off-topology-peer', { + controlPeer: { ip: '10.20.30.40', ports: [8443] }, + })).toThrow(/outside 172\.30\.0\.0\/24/); + }); + + it('rejects a future centralized infrastructure policy that overlaps the guest link', () => { + const plan = createPlan('overlap-defense'); + + expect(() => generateFirecrackerNftRuleset({ + ...plan, + infrastructureCidr: plan.guestSubnet, + infrastructureIp: plan.guestIp, + })).toThrow(/guest subnet overlaps infrastructure/); + }); +}); + +describe('Firecracker nftables policy', () => { + it('installs default-drop policy with exact endpoint, identity, and return rules', () => { + const plan = createPlan(); + const ruleset = generateFirecrackerNftRuleset(plan); + + expect(ruleset).toContain(`table inet ${plan.nftTableName}`); + expect(ruleset.match(/policy drop;/g)).toHaveLength(3); + expect(ruleset).toContain( + `iifname "${plan.tapName}" ether saddr != ${plan.guestMac} drop`, + ); + expect(ruleset).toContain( + `iifname "${plan.tapName}" ip saddr != ${plan.guestIp} drop`, + ); + expect(ruleset).toContain('ip daddr 169.254.0.0/16 drop'); + expect(ruleset).toContain('ip daddr 224.0.0.0/4 drop'); + expect(ruleset).toContain('ip daddr 172.30.0.1 drop'); + expect(ruleset).toContain('udp dport 53 drop'); + expect(ruleset).toContain('tcp dport 53 drop'); + expect(ruleset).toContain('ct state established,related accept'); + expect(ruleset).toContain('ip daddr 172.30.0.10 tcp dport 3128'); + for (let port = 10000; port <= 10004; port += 1) { + expect(ruleset).toContain(`ip daddr 172.30.0.30 tcp dport ${port}`); + } + expect(ruleset).not.toContain('masquerade'); + expect(ruleset).not.toContain('flush ruleset'); + expect(ruleset).not.toMatch(/ip daddr 0\.0\.0\.0\/0.*accept/); + }); + + it('emits SNAT only for the same exact allowed destination pairs', () => { + const plan = createPlan('narrow-snat', { enableApiProxy: false }); + const ruleset = generateFirecrackerNftRuleset(plan); + const snatLines = ruleset.split('\n').filter((line) => line.includes('snat to')); + + expect(snatLines).toEqual([ + expect.stringContaining( + `ip daddr 172.30.0.10 tcp dport 3128 snat to ${plan.infrastructureIp}`, + ), + ]); + }); +}); + +describe('Firecracker network lifecycle', () => { + it('creates the namespace, veth, TAP, forwarding, and atomic policy in order', async () => { + const plan = createPlan(); + const { calls, commands } = commandHarness(); + const probe: FirecrackerConnectivityProbe = { + verify: jest.fn().mockResolvedValue(undefined), + }; + const manager = new FirecrackerNetworkManager(plan, commands, probe); + + await expect(manager.setup()).resolves.toBe(plan); + + expect(calls[0]).toEqual({ + command: 'ip', + args: ['netns', 'add', plan.namespaceName], + options: { reject: true }, + }); + expect(calls[1].args).toEqual([ + 'link', 'add', plan.hostVethName, + 'type', 'veth', + 'peer', 'name', plan.namespaceVethName, + ]); + expect(calls[2].args).toEqual([ + 'link', 'set', plan.namespaceVethName, + 'netns', plan.namespaceName, + ]); + expect(calls[3].args).toEqual([ + 'link', 'set', plan.hostVethName, + 'master', plan.infrastructureBridge, + ]); + expect(calls[5].args).toEqual([ + 'netns', 'exec', plan.namespaceName, 'ip', + 'tuntap', 'add', + 'dev', plan.tapName, + 'mode', 'tap', + 'user', '1000', + 'group', '1000', + ]); + expect(calls[11].args).toContain('net.ipv4.ip_forward=1'); + expect(calls[12].args).toContain('net.ipv6.conf.all.disable_ipv6=1'); + expect(calls[13].args).toContain('net.ipv6.conf.default.disable_ipv6=1'); + expect(calls[14]).toEqual({ + command: 'ip', + args: ['netns', 'exec', plan.namespaceName, 'nft', '-f', '-'], + options: { + reject: true, + input: generateFirecrackerNftRuleset(plan), + }, + }); + expect(probe.verify).toHaveBeenCalledWith(plan); + }); + + it('rolls back every partial setup stage with run-specific cleanup', async () => { + const plan = createPlan('rollback-all'); + const setupStageCount = 15; + + for (let failAt = 1; failAt <= setupStageCount; failAt += 1) { + const { calls, commands } = commandHarness(failAt); + const manager = new FirecrackerNetworkManager(plan, commands); + + await expect(manager.setup()).rejects.toThrow(`stage ${failAt} failed`); + const cleanupCalls = calls.filter((call) => call.options.reject === false); + if (failAt === 1) { + expect(cleanupCalls).toEqual([]); + } else if (failAt === 2) { + expect(cleanupCalls).toEqual([{ + command: 'ip', + args: ['netns', 'delete', plan.namespaceName], + options: { reject: false }, + }]); + } else { + expect(cleanupCalls).toEqual([ + { + command: 'ip', + args: ['link', 'delete', plan.hostVethName], + options: { reject: false }, + }, + { + command: 'ip', + args: ['netns', 'delete', plan.namespaceName], + options: { reject: false }, + }, + ]); + } + } + }); + + it('treats a supplied connectivity probe failure as setup failure', async () => { + const plan = createPlan('probe-failure'); + const { calls, commands } = commandHarness(); + const probe: FirecrackerConnectivityProbe = { + verify: jest.fn().mockRejectedValue(new Error('proxy unreachable')), + }; + const manager = new FirecrackerNetworkManager(plan, commands, probe); + + await expect(manager.setup()).rejects.toThrow('proxy unreachable'); + expect(calls.slice(-1)[0].args).toEqual([ + 'netns', 'delete', plan.namespaceName, + ]); + }); + + it('cleanup is idempotent and never targets unrelated nftables objects', async () => { + const plan = createPlan('cleanup-twice'); + const { calls, commands } = commandHarness(); + const manager = new FirecrackerNetworkManager(plan, commands); + + await manager.setup(); + await manager.cleanup(); + const callsAfterFirstCleanup = calls.length; + await manager.cleanup(); + + expect(calls).toHaveLength(callsAfterFirstCleanup); + const cleanupCalls = calls.filter((call) => call.options.reject === false); + expect(cleanupCalls).toHaveLength(3); + expect(cleanupCalls.filter((call) => call.args.includes('delete'))).toEqual([ + expect.objectContaining({ + args: expect.arrayContaining([plan.nftTableName]), + }), + expect.objectContaining({ + args: expect.arrayContaining([plan.hostVethName]), + }), + expect.objectContaining({ + args: expect.arrayContaining([plan.namespaceName]), + }), + ]); + expect(calls.some((call) => call.args.includes('flush'))).toBe(false); + }); +}); diff --git a/src/firecracker/network.ts b/src/firecracker/network.ts new file mode 100644 index 000000000..1c4e41c19 --- /dev/null +++ b/src/firecracker/network.ts @@ -0,0 +1,616 @@ +import { createHash } from 'crypto'; +import execa from 'execa'; +import { + AGENT_IP, + API_PROXY_IP, + HOST_GATEWAY, + NETWORK_SUBNET, + SQUID_IP, + SQUID_PORT, + apiProxyPorts, +} from '../config/network-policy'; +import type { FirecrackerNetworkInterface } from './api-client'; + +const LINUX_INTERFACE_NAME_MAX_LENGTH = 15; +const FIRECRACKER_GUEST_NETWORK_BASE = ipv4ToInteger('100.64.0.0'); +const FIRECRACKER_GUEST_SUBNET_COUNT = 1 << 20; +const FIRECRACKER_GUEST_PREFIX_LENGTH = 30; +const NETNS_DIRECTORY = '/var/run/netns'; +const BLOCKED_LINK_LOCAL_CIDR = '169.254.0.0/16'; +const BLOCKED_MULTICAST_CIDR = '224.0.0.0/4'; + +export interface FirecrackerAllowedEndpoint { + readonly name: string; + readonly ip: string; + readonly port: number; +} + +export interface FirecrackerControlPeer { + readonly ip: string; + readonly ports: readonly number[]; +} + +export interface FirecrackerNetworkPlanOptions { + readonly infrastructureBridge: string; + readonly enableApiProxy: boolean; + readonly jailerUid: number; + readonly jailerGid: number; + readonly controlPeer?: FirecrackerControlPeer; +} + +export interface FirecrackerNetworkPlan { + readonly runId: string; + readonly namespaceName: string; + readonly netnsPath: string; + readonly nftTableName: string; + readonly infrastructureBridge: string; + readonly hostVethName: string; + readonly namespaceVethName: string; + readonly tapName: string; + readonly infrastructureIp: string; + readonly infrastructureCidr: string; + readonly hostGatewayIp: string; + readonly guestSubnet: string; + readonly guestIp: string; + readonly guestGatewayIp: string; + readonly guestPrefixLength: number; + readonly guestMac: string; + readonly jailerUid: number; + readonly jailerGid: number; + readonly allowedEndpoints: readonly FirecrackerAllowedEndpoint[]; + readonly networkInterface: FirecrackerNetworkInterface; +} + +export interface FirecrackerConnectivityProbe { + verify(plan: FirecrackerNetworkPlan): Promise; +} + +export interface FirecrackerNetworkCommandOptions { + readonly reject: boolean; + readonly input?: string; +} + +export type FirecrackerNetworkCommandExecutor = ( + command: string, + args: readonly string[], + options: FirecrackerNetworkCommandOptions, +) => Promise; + +const defaultCommandExecutor: FirecrackerNetworkCommandExecutor = async ( + command, + args, + options, +) => { + if (command !== 'ip') { + throw new Error(`Unsupported Firecracker network command: ${command}`); + } + await execa('ip', [...args], options); +}; + +/** + * Dependency-injected argv-only Linux networking operations. + */ +export class FirecrackerLinuxNetworkCommands { + constructor(private readonly execute: FirecrackerNetworkCommandExecutor = defaultCommandExecutor) {} + + ip(args: readonly string[], reject = true): Promise { + return this.execute('ip', args, { reject }); + } + + ipInNamespace( + namespaceName: string, + args: readonly string[], + reject = true, + ): Promise { + return this.execute('ip', ['netns', 'exec', namespaceName, 'ip', ...args], { reject }); + } + + sysctlInNamespace( + namespaceName: string, + setting: string, + reject = true, + ): Promise { + return this.execute( + 'ip', + ['netns', 'exec', namespaceName, 'sysctl', '-q', '-w', setting], + { reject }, + ); + } + + nftInNamespace( + namespaceName: string, + args: readonly string[], + input?: string, + reject = true, + ): Promise { + return this.execute( + 'ip', + ['netns', 'exec', namespaceName, 'nft', ...args], + { reject, ...(input === undefined ? {} : { input }) }, + ); + } +} + +export interface FirecrackerNetworkLifecycle { + readonly plan: FirecrackerNetworkPlan; + setup(): Promise; + cleanup(): Promise; +} + +/** + * Owns the host-side network resources for exactly one Firecracker run. + */ +export class FirecrackerNetworkManager implements FirecrackerNetworkLifecycle { + private setupComplete = false; + private namespaceCreated = false; + private hostVethCreated = false; + private nftTableCreated = false; + + constructor( + readonly plan: FirecrackerNetworkPlan, + private readonly commands = new FirecrackerLinuxNetworkCommands(), + private readonly probe?: FirecrackerConnectivityProbe, + ) {} + + async setup(): Promise { + if (this.setupComplete) return this.plan; + + try { + await this.commands.ip(['netns', 'add', this.plan.namespaceName]); + this.namespaceCreated = true; + await this.commands.ip([ + 'link', 'add', this.plan.hostVethName, + 'type', 'veth', + 'peer', 'name', this.plan.namespaceVethName, + ]); + this.hostVethCreated = true; + await this.commands.ip([ + 'link', 'set', this.plan.namespaceVethName, + 'netns', this.plan.namespaceName, + ]); + await this.commands.ip([ + 'link', 'set', this.plan.hostVethName, + 'master', this.plan.infrastructureBridge, + ]); + await this.commands.ip(['link', 'set', this.plan.hostVethName, 'up']); + + await this.commands.ipInNamespace(this.plan.namespaceName, [ + 'tuntap', 'add', + 'dev', this.plan.tapName, + 'mode', 'tap', + 'user', String(this.plan.jailerUid), + 'group', String(this.plan.jailerGid), + ]); + await this.commands.ipInNamespace(this.plan.namespaceName, [ + 'addr', 'add', + `${this.plan.guestGatewayIp}/${this.plan.guestPrefixLength}`, + 'dev', this.plan.tapName, + ]); + await this.commands.ipInNamespace( + this.plan.namespaceName, + ['link', 'set', this.plan.tapName, 'up'], + ); + await this.commands.ipInNamespace(this.plan.namespaceName, [ + 'addr', 'add', + `${this.plan.infrastructureIp}/${prefixLength(this.plan.infrastructureCidr)}`, + 'dev', this.plan.namespaceVethName, + ]); + await this.commands.ipInNamespace( + this.plan.namespaceName, + ['link', 'set', this.plan.namespaceVethName, 'up'], + ); + await this.commands.ipInNamespace( + this.plan.namespaceName, + ['link', 'set', 'lo', 'up'], + ); + await this.commands.sysctlInNamespace( + this.plan.namespaceName, + 'net.ipv4.ip_forward=1', + ); + await this.commands.sysctlInNamespace( + this.plan.namespaceName, + 'net.ipv6.conf.all.disable_ipv6=1', + ); + await this.commands.sysctlInNamespace( + this.plan.namespaceName, + 'net.ipv6.conf.default.disable_ipv6=1', + ); + await this.commands.nftInNamespace( + this.plan.namespaceName, + ['-f', '-'], + generateFirecrackerNftRuleset(this.plan), + ); + this.nftTableCreated = true; + await this.probe?.verify(this.plan); + this.setupComplete = true; + return this.plan; + } catch (error) { + try { + await this.cleanup(); + } catch (cleanupError) { + throw new Error( + `Firecracker network setup failed: ${formatError(error)}; ` + + `rollback also failed: ${formatError(cleanupError)}`, + ); + } + throw error; + } + } + + async cleanup(): Promise { + const errors: unknown[] = []; + const attempt = async (operation: () => Promise): Promise => { + try { + await operation(); + } catch (error) { + errors.push(error); + } + }; + + if (this.nftTableCreated) { + await attempt(async () => { + await this.commands.nftInNamespace( + this.plan.namespaceName, + ['delete', 'table', 'inet', this.plan.nftTableName], + undefined, + false, + ); + this.nftTableCreated = false; + }); + } + if (this.hostVethCreated) { + await attempt(async () => { + await this.commands.ip( + ['link', 'delete', this.plan.hostVethName], + false, + ); + this.hostVethCreated = false; + }); + } + if (this.namespaceCreated) { + await attempt(async () => { + await this.commands.ip( + ['netns', 'delete', this.plan.namespaceName], + false, + ); + this.namespaceCreated = false; + this.hostVethCreated = false; + this.nftTableCreated = false; + }); + } + this.setupComplete = false; + + if (errors.length > 0) { + throw new Error( + `Failed to clean up Firecracker network: ${errors.map(formatError).join('; ')}`, + ); + } + } +} + +export function createFirecrackerNetworkPlan( + runId: string, + options: FirecrackerNetworkPlanOptions, +): FirecrackerNetworkPlan { + assertSafeFirecrackerRunId(runId); + assertInterfaceName(options.infrastructureBridge, 'infrastructure bridge'); + assertPositiveIdentity(options.jailerUid, 'jailer uid'); + assertPositiveIdentity(options.jailerGid, 'jailer gid'); + + const digest = createHash('sha256').update(runId).digest(); + const token = digest.toString('hex').slice(0, 12); + const subnetIndex = digest.readUInt32BE(0) & (FIRECRACKER_GUEST_SUBNET_COUNT - 1); + const subnetBase = FIRECRACKER_GUEST_NETWORK_BASE + subnetIndex * 4; + const guestGatewayIp = integerToIpv4(subnetBase + 1); + const guestIp = integerToIpv4(subnetBase + 2); + const guestMac = [ + 0x02, + digest[4], + digest[5], + digest[6], + digest[7], + digest[8], + ].map((byte) => byte.toString(16).padStart(2, '0')).join(':'); + + const namespaceName = `awffc-${token}`; + const tapName = `fct${token}`; + const hostVethName = `fch${token}`; + const namespaceVethName = `fcn${token}`; + const nftTableName = `awf_fc_${token}`; + for (const [label, name] of [ + ['TAP', tapName], + ['host veth', hostVethName], + ['namespace veth', namespaceVethName], + ] as const) { + assertInterfaceName(name, label); + } + + const allowedEndpoints = createAllowedEndpoints( + options.enableApiProxy, + options.controlPeer, + ); + const plan: FirecrackerNetworkPlan = { + runId, + namespaceName, + netnsPath: `${NETNS_DIRECTORY}/${namespaceName}`, + nftTableName, + infrastructureBridge: options.infrastructureBridge, + hostVethName, + namespaceVethName, + tapName, + infrastructureIp: AGENT_IP, + infrastructureCidr: NETWORK_SUBNET, + hostGatewayIp: HOST_GATEWAY, + guestSubnet: `${integerToIpv4(subnetBase)}/${FIRECRACKER_GUEST_PREFIX_LENGTH}`, + guestIp, + guestGatewayIp, + guestPrefixLength: FIRECRACKER_GUEST_PREFIX_LENGTH, + guestMac, + jailerUid: options.jailerUid, + jailerGid: options.jailerGid, + allowedEndpoints, + networkInterface: { + iface_id: 'eth0', + host_dev_name: tapName, + guest_mac: guestMac, + }, + }; + validatePlan(plan); + return plan; +} + +export function generateFirecrackerNftRuleset(plan: FirecrackerNetworkPlan): string { + validatePlan(plan); + const allowRules = plan.allowedEndpoints.flatMap((endpoint) => [ + ` iifname "${plan.tapName}" oifname "${plan.namespaceVethName}" ` + + `ether saddr ${plan.guestMac} ip saddr ${plan.guestIp} ` + + `ip daddr ${endpoint.ip} tcp dport ${endpoint.port} ` + + 'ct state new,established accept', + ]); + const snatRules = plan.allowedEndpoints.map((endpoint) => + ` iifname "${plan.tapName}" oifname "${plan.namespaceVethName}" ` + + `ip saddr ${plan.guestIp} ip daddr ${endpoint.ip} tcp dport ${endpoint.port} ` + + `snat to ${plan.infrastructureIp}`, + ); + + return [ + `table inet ${plan.nftTableName} {`, + ' chain input {', + ' type filter hook input priority filter; policy drop;', + ' iifname "lo" accept', + ' ct state established,related accept', + ' }', + ' chain output {', + ' type filter hook output priority filter; policy drop;', + ' oifname "lo" accept', + ' ct state established,related accept', + ' }', + ' chain forward {', + ' type filter hook forward priority filter; policy drop;', + ' ct state invalid drop', + ` iifname "${plan.tapName}" ether saddr != ${plan.guestMac} drop`, + ` iifname "${plan.tapName}" ip saddr != ${plan.guestIp} drop`, + ` iifname "${plan.tapName}" ip daddr ${BLOCKED_LINK_LOCAL_CIDR} drop`, + ` iifname "${plan.tapName}" ip daddr ${BLOCKED_MULTICAST_CIDR} drop`, + ` iifname "${plan.tapName}" ip daddr ${plan.hostGatewayIp} drop`, + ` iifname "${plan.tapName}" ip daddr ${plan.infrastructureIp} drop`, + ` iifname "${plan.tapName}" udp dport 53 drop`, + ` iifname "${plan.tapName}" tcp dport 53 drop`, + ` iifname "${plan.namespaceVethName}" oifname "${plan.tapName}" ` + + `ether daddr ${plan.guestMac} ip daddr ${plan.guestIp} ` + + 'ct state established,related accept', + ...allowRules, + ' }', + ' chain postrouting {', + ' type nat hook postrouting priority srcnat; policy accept;', + ...snatRules, + ' }', + '}', + '', + ].join('\n'); +} + +function createAllowedEndpoints( + enableApiProxy: boolean, + controlPeer?: FirecrackerControlPeer, +): readonly FirecrackerAllowedEndpoint[] { + const endpoints: FirecrackerAllowedEndpoint[] = [{ + name: 'squid', + ip: SQUID_IP, + port: SQUID_PORT, + }]; + if (enableApiProxy) { + for (const [provider, port] of Object.entries(apiProxyPorts())) { + endpoints.push({ + name: `api-proxy-${provider}`, + ip: API_PROXY_IP, + port, + }); + } + } + if (controlPeer) { + assertPrivateIpv4(controlPeer.ip, 'control peer IP'); + if ( + !isInCidr(controlPeer.ip, NETWORK_SUBNET) || + controlPeer.ip === HOST_GATEWAY || + controlPeer.ip === AGENT_IP || + isInCidr(controlPeer.ip, BLOCKED_LINK_LOCAL_CIDR) || + isInCidr(controlPeer.ip, BLOCKED_MULTICAST_CIDR) + ) { + throw new Error( + `Unsafe Firecracker control peer IP outside ${NETWORK_SUBNET}: ${controlPeer.ip}`, + ); + } + if (controlPeer.ports.length === 0) { + throw new Error('Firecracker control peer must specify at least one TCP port'); + } + for (const port of controlPeer.ports) { + assertPort(port, 'control peer port'); + if (port === 53) { + throw new Error('Firecracker control peer cannot enable direct DNS'); + } + endpoints.push({ name: 'control-peer', ip: controlPeer.ip, port }); + } + } + + const seen = new Set(); + return endpoints.filter((endpoint) => { + const key = `${endpoint.ip}:${endpoint.port}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +} + +function validatePlan(plan: FirecrackerNetworkPlan): void { + assertSafeFirecrackerRunId(plan.runId); + assertSafeObjectName(plan.namespaceName, 'network namespace'); + assertSafeObjectName(plan.nftTableName, 'nftables table'); + assertInterfaceName(plan.infrastructureBridge, 'infrastructure bridge'); + assertInterfaceName(plan.hostVethName, 'host veth'); + assertInterfaceName(plan.namespaceVethName, 'namespace veth'); + assertInterfaceName(plan.tapName, 'TAP'); + assertIpv4(plan.infrastructureIp, 'infrastructure IP'); + assertCidr(plan.infrastructureCidr, 'infrastructure CIDR'); + assertIpv4(plan.hostGatewayIp, 'host gateway IP'); + assertCidr(plan.guestSubnet, 'guest subnet'); + assertIpv4(plan.guestIp, 'guest IP'); + assertIpv4(plan.guestGatewayIp, 'guest gateway IP'); + const guestNetworkIp = plan.guestSubnet.split('/')[0]; + const infrastructureNetworkIp = plan.infrastructureCidr.split('/')[0]; + if ( + isInCidr(guestNetworkIp, plan.infrastructureCidr) || + isInCidr(infrastructureNetworkIp, plan.guestSubnet) + ) { + throw new Error( + `Firecracker guest subnet overlaps infrastructure: ` + + `${plan.guestSubnet} and ${plan.infrastructureCidr}`, + ); + } + const macOctets = plan.guestMac.split(':'); + if ( + macOctets.length !== 6 || + macOctets[0] !== '02' || + macOctets.some((octet) => ( + octet.length !== 2 || + [...octet].some((character) => ( + !'0123456789abcdef'.includes(character) + )) + )) + ) { + throw new Error(`Unsafe Firecracker guest MAC: ${plan.guestMac}`); + } + for (const endpoint of plan.allowedEndpoints) { + assertSafeObjectName(endpoint.name, 'endpoint name'); + assertIpv4(endpoint.ip, 'endpoint IP'); + assertPort(endpoint.port, 'endpoint port'); + if (isInCidr(endpoint.ip, plan.guestSubnet)) { + throw new Error( + `Firecracker endpoint ${endpoint.ip}:${endpoint.port} overlaps the guest subnet`, + ); + } + } +} + +export function assertSafeFirecrackerRunId(runId: string): void { + if (runId.length < 1 || runId.length > 64 || !/^[A-Za-z0-9-]+$/.test(runId)) { + throw new Error(`Unsafe Firecracker run id: ${runId}`); + } +} + +function assertSafeObjectName(value: string, label: string): void { + if (!/^[A-Za-z0-9_.-]+$/.test(value)) { + throw new Error(`Unsafe Firecracker ${label}: ${value}`); + } +} + +function assertInterfaceName(value: string, label: string): void { + assertSafeObjectName(value, label); + if (value.length > LINUX_INTERFACE_NAME_MAX_LENGTH) { + throw new Error( + `Firecracker ${label} exceeds Linux IFNAMSIZ: ${value}`, + ); + } +} + +function assertPositiveIdentity(value: number, label: string): void { + if (!Number.isSafeInteger(value) || value <= 0) { + throw new Error(`Firecracker ${label} must be a positive integer`); + } +} + +function assertPort(value: number, label: string): void { + if (!Number.isInteger(value) || value < 1 || value > 65_535) { + throw new Error(`Firecracker ${label} must be an integer in 1-65535`); + } +} + +function assertPrivateIpv4(value: string, label: string): void { + assertIpv4(value, label); + if ( + !isInCidr(value, '10.0.0.0/8') && + !isInCidr(value, '172.16.0.0/12') && + !isInCidr(value, '192.168.0.0/16') + ) { + throw new Error(`Firecracker ${label} must be an RFC1918 address: ${value}`); + } +} + +function assertIpv4(value: string, label: string): void { + const rawOctets = value.split('.'); + if ( + rawOctets.length !== 4 || + rawOctets.some((octet) => ( + octet.length < 1 || + octet.length > 3 || + [...octet].some((character) => ( + character < '0' || character > '9' + )) || + Number(octet) > 255 + )) + ) { + throw new Error(`Invalid Firecracker ${label}: ${value}`); + } +} + +function assertCidr(value: string, label: string): void { + const [address, rawPrefix, extra] = value.split('/'); + assertIpv4(address, label); + const prefix = Number(rawPrefix); + if (extra !== undefined || !Number.isInteger(prefix) || prefix < 0 || prefix > 32) { + throw new Error(`Invalid Firecracker ${label}: ${value}`); + } +} + +function prefixLength(cidr: string): number { + assertCidr(cidr, 'CIDR'); + return Number(cidr.split('/')[1]); +} + +function isInCidr(ip: string, cidr: string): boolean { + const [network, rawPrefix] = cidr.split('/'); + const prefix = Number(rawPrefix); + const mask = prefix === 0 ? 0 : (0xffffffff << (32 - prefix)) >>> 0; + return (ipv4ToInteger(ip) & mask) === (ipv4ToInteger(network) & mask); +} + +function ipv4ToInteger(ip: string): number { + assertIpv4(ip, 'IPv4 address'); + return ip.split('.').reduce((value, octet) => ( + ((value << 8) | Number(octet)) >>> 0 + ), 0); +} + +function integerToIpv4(value: number): string { + const normalized = value >>> 0; + return [ + normalized >>> 24, + (normalized >>> 16) & 0xff, + (normalized >>> 8) & 0xff, + normalized & 0xff, + ].join('.'); +} + +function formatError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/types/runtime-options.ts b/src/types/runtime-options.ts index 34f9bd2ef..99968ec07 100644 --- a/src/types/runtime-options.ts +++ b/src/types/runtime-options.ts @@ -21,8 +21,8 @@ export interface FirecrackerArtifactDigests { /** * Preview control-plane configuration for the Firecracker microVM runtime. * - * Networking and guest command execution are intentionally not part of this - * configuration surface yet. + * Host-side network enforcement is supplied directly to FirecrackerManager + * after infrastructure discovery; guest command execution is not available yet. */ export interface FirecrackerOptions { previewEnabled: boolean; From fd0d9aeeee1b0e31e62bfac67eadad3ba4be3243 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Sun, 9 Aug 2026 12:00:01 -0700 Subject: [PATCH 2/4] test: preserve Firecracker signal readiness coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/firecracker/manager.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/firecracker/manager.test.ts b/src/firecracker/manager.test.ts index f78ad29c3..39cd8bbb0 100644 --- a/src/firecracker/manager.test.ts +++ b/src/firecracker/manager.test.ts @@ -267,7 +267,13 @@ describe('FirecrackerManager', () => { access: jest.fn().mockRejectedValue(missing), sleep: jest.fn().mockResolvedValue(undefined), }); - const manager = new FirecrackerManager(config({ apiTimeoutMs: 2000 }), '/tmp/awf', deps, 'signal'); + const manager = new FirecrackerManager( + config({ apiTimeoutMs: 2000 }), + '/tmp/awf', + deps, + 'signal', + networkConfig(), + ); await expect(manager.start()).rejects.toThrow( /exited before API readiness with code null and signal SIGKILL/, From 5ba54326fba39003c9a9efb95aad4faf53092c42 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:30:40 +0000 Subject: [PATCH 3/4] fix: retain Firecracker network policy on teardown failure Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- src/firecracker/manager.test.ts | 26 ++++++++++++++++ src/firecracker/manager.ts | 2 +- src/firecracker/network.test.ts | 53 ++++++++++++++++++++++++++------- src/firecracker/network.ts | 27 ++--------------- 4 files changed, 73 insertions(+), 35 deletions(-) diff --git a/src/firecracker/manager.test.ts b/src/firecracker/manager.test.ts index 39cd8bbb0..31c07a685 100644 --- a/src/firecracker/manager.test.ts +++ b/src/firecracker/manager.test.ts @@ -232,6 +232,32 @@ describe('FirecrackerManager', () => { expect(order).toEqual(['network', 'jail']); }); + it('retains failed network cleanup for a later stop retry', async () => { + const cleanup = jest.fn() + .mockRejectedValueOnce(new Error('network cleanup failed')) + .mockResolvedValue(undefined); + const deps = dependencies({ + createNetwork: jest.fn((plan) => ({ + plan, + setup: jest.fn().mockResolvedValue(plan), + cleanup, + })), + }); + const manager = new FirecrackerManager( + config(), + '/tmp/awf', + deps, + 'cleanup-retry', + networkConfig(), + ); + + await manager.start(); + await expect(manager.stop()).rejects.toThrow('network cleanup failed'); + await expect(manager.stop()).resolves.toBeUndefined(); + + expect(cleanup).toHaveBeenCalledTimes(2); + }); + it('rolls back the network when typed NIC configuration fails', async () => { const client = { putMachineConfig: jest.fn().mockResolvedValue(undefined), diff --git a/src/firecracker/manager.ts b/src/firecracker/manager.ts index bc4e8c365..f18f24bfe 100644 --- a/src/firecracker/manager.ts +++ b/src/firecracker/manager.ts @@ -233,10 +233,10 @@ export class FirecrackerManager { try { await this.network?.cleanup(); + this.network = undefined; } catch (error) { errors.push(error); } - this.network = undefined; try { await this.dependencies.rm( diff --git a/src/firecracker/network.test.ts b/src/firecracker/network.test.ts index 731582478..8d855ba2b 100644 --- a/src/firecracker/network.test.ts +++ b/src/firecracker/network.test.ts @@ -232,26 +232,26 @@ describe('Firecracker network lifecycle', () => { const manager = new FirecrackerNetworkManager(plan, commands); await expect(manager.setup()).rejects.toThrow(`stage ${failAt} failed`); - const cleanupCalls = calls.filter((call) => call.options.reject === false); + const cleanupCalls = calls.filter((call) => call.args.includes('delete')); if (failAt === 1) { expect(cleanupCalls).toEqual([]); } else if (failAt === 2) { expect(cleanupCalls).toEqual([{ command: 'ip', args: ['netns', 'delete', plan.namespaceName], - options: { reject: false }, + options: { reject: true }, }]); } else { expect(cleanupCalls).toEqual([ { command: 'ip', args: ['link', 'delete', plan.hostVethName], - options: { reject: false }, + options: { reject: true }, }, { command: 'ip', args: ['netns', 'delete', plan.namespaceName], - options: { reject: false }, + options: { reject: true }, }, ]); } @@ -272,7 +272,7 @@ describe('Firecracker network lifecycle', () => { ]); }); - it('cleanup is idempotent and never targets unrelated nftables objects', async () => { + it('disconnects the host veth before deleting the namespace and its nft policy', async () => { const plan = createPlan('cleanup-twice'); const { calls, commands } = commandHarness(); const manager = new FirecrackerNetworkManager(plan, commands); @@ -283,12 +283,9 @@ describe('Firecracker network lifecycle', () => { await manager.cleanup(); expect(calls).toHaveLength(callsAfterFirstCleanup); - const cleanupCalls = calls.filter((call) => call.options.reject === false); - expect(cleanupCalls).toHaveLength(3); + const cleanupCalls = calls.filter((call) => call.args.includes('delete')); + expect(cleanupCalls).toHaveLength(2); expect(cleanupCalls.filter((call) => call.args.includes('delete'))).toEqual([ - expect.objectContaining({ - args: expect.arrayContaining([plan.nftTableName]), - }), expect.objectContaining({ args: expect.arrayContaining([plan.hostVethName]), }), @@ -296,6 +293,42 @@ describe('Firecracker network lifecycle', () => { args: expect.arrayContaining([plan.namespaceName]), }), ]); + expect(cleanupCalls.every((call) => call.options.reject)).toBe(true); expect(calls.some((call) => call.args.includes('flush'))).toBe(false); }); + + it('retains the namespace and nft policy for a retry when host veth deletion fails', async () => { + const plan = createPlan('cleanup-retry'); + let hostVethDeleteFailed = false; + const { calls, commands } = commandHarness(); + const originalIp = commands.ip.bind(commands); + jest.spyOn(commands, 'ip').mockImplementation(async (args, reject = true) => { + if ( + !hostVethDeleteFailed + && args[0] === 'link' + && args[1] === 'delete' + && args[2] === plan.hostVethName + ) { + hostVethDeleteFailed = true; + throw new Error('host veth deletion failed'); + } + return originalIp(args, reject); + }); + const manager = new FirecrackerNetworkManager(plan, commands); + + await manager.setup(); + await expect(manager.cleanup()).rejects.toThrow('host veth deletion failed'); + expect(calls.some((call) => ( + call.args[0] === 'netns' + && call.args[1] === 'delete' + && call.args[2] === plan.namespaceName + ))).toBe(false); + + await expect(manager.cleanup()).resolves.toBeUndefined(); + expect(calls.filter((call) => ( + call.args[0] === 'netns' + && call.args[1] === 'delete' + && call.args[2] === plan.namespaceName + ))).toHaveLength(1); + }); }); diff --git a/src/firecracker/network.ts b/src/firecracker/network.ts index 1c4e41c19..5e2d61975 100644 --- a/src/firecracker/network.ts +++ b/src/firecracker/network.ts @@ -144,7 +144,6 @@ export class FirecrackerNetworkManager implements FirecrackerNetworkLifecycle { private setupComplete = false; private namespaceCreated = false; private hostVethCreated = false; - private nftTableCreated = false; constructor( readonly plan: FirecrackerNetworkPlan, @@ -220,7 +219,6 @@ export class FirecrackerNetworkManager implements FirecrackerNetworkLifecycle { ['-f', '-'], generateFirecrackerNftRuleset(this.plan), ); - this.nftTableCreated = true; await this.probe?.verify(this.plan); this.setupComplete = true; return this.plan; @@ -247,35 +245,16 @@ export class FirecrackerNetworkManager implements FirecrackerNetworkLifecycle { } }; - if (this.nftTableCreated) { - await attempt(async () => { - await this.commands.nftInNamespace( - this.plan.namespaceName, - ['delete', 'table', 'inet', this.plan.nftTableName], - undefined, - false, - ); - this.nftTableCreated = false; - }); - } if (this.hostVethCreated) { await attempt(async () => { - await this.commands.ip( - ['link', 'delete', this.plan.hostVethName], - false, - ); + await this.commands.ip(['link', 'delete', this.plan.hostVethName]); this.hostVethCreated = false; }); } - if (this.namespaceCreated) { + if (this.namespaceCreated && !this.hostVethCreated) { await attempt(async () => { - await this.commands.ip( - ['netns', 'delete', this.plan.namespaceName], - false, - ); + await this.commands.ip(['netns', 'delete', this.plan.namespaceName]); this.namespaceCreated = false; - this.hostVethCreated = false; - this.nftTableCreated = false; }); } this.setupComplete = false; From 4c3b4f1a9ee6469885bf1fa1a6452a2323a00a0c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:32:20 +0000 Subject: [PATCH 4/4] test: cover Firecracker namespace cleanup retry Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- src/firecracker/network.test.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/firecracker/network.test.ts b/src/firecracker/network.test.ts index 8d855ba2b..49f666707 100644 --- a/src/firecracker/network.test.ts +++ b/src/firecracker/network.test.ts @@ -331,4 +331,34 @@ describe('Firecracker network lifecycle', () => { && call.args[2] === plan.namespaceName ))).toHaveLength(1); }); + + it('retains the namespace for a retry when namespace deletion fails', async () => { + const plan = createPlan('namespace-retry'); + let namespaceDeleteFailed = false; + const { calls, commands } = commandHarness(); + const originalIp = commands.ip.bind(commands); + jest.spyOn(commands, 'ip').mockImplementation(async (args, reject = true) => { + if ( + !namespaceDeleteFailed + && args[0] === 'netns' + && args[1] === 'delete' + && args[2] === plan.namespaceName + ) { + namespaceDeleteFailed = true; + throw new Error('namespace deletion failed'); + } + return originalIp(args, reject); + }); + const manager = new FirecrackerNetworkManager(plan, commands); + + await manager.setup(); + await expect(manager.cleanup()).rejects.toThrow('namespace deletion failed'); + await expect(manager.cleanup()).resolves.toBeUndefined(); + + expect(calls.filter((call) => ( + call.args[0] === 'netns' + && call.args[1] === 'delete' + && call.args[2] === plan.namespaceName + ))).toHaveLength(1); + }); });