From 72e6b270263437845c4d9f2ea7c3ab215820275d Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 08:14:13 -0700 Subject: [PATCH 1/6] feat: add durable microvm cleanup Persist identity-validated Cloud Hypervisor cleanup records. Safely reap stale resources after abrupt owner death. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/test-cloud-hypervisor.yml | 11 +- docs/cloud-hypervisor-foundation.md | 25 +- scripts/ci/cloud-hypervisor-live-smoke.sh | 79 +- src/cloud-hypervisor/cleanup-registry.test.ts | 280 +++++ src/cloud-hypervisor/cleanup-registry.ts | 1075 +++++++++++++++++ src/cloud-hypervisor/manager-start.ts | 25 +- src/cloud-hypervisor/manager-stop.ts | 4 + src/cloud-hypervisor/manager-types.ts | 13 +- src/cloud-hypervisor/manager.test.ts | 69 ++ src/cloud-hypervisor/manager.ts | 19 +- src/cloud-hypervisor/virtiofsd.ts | 10 + .../vm-config-builder.test.ts | 1 + src/microvm/network-commands.ts | 35 +- src/microvm/network-manager.ts | 16 +- src/microvm/network-plan.ts | 1 + src/microvm/network-types.ts | 5 + src/microvm/network.test.ts | 43 +- src/microvm/network.ts | 1 + 18 files changed, 1685 insertions(+), 27 deletions(-) create mode 100644 src/cloud-hypervisor/cleanup-registry.test.ts create mode 100644 src/cloud-hypervisor/cleanup-registry.ts diff --git a/.github/workflows/test-cloud-hypervisor.yml b/.github/workflows/test-cloud-hypervisor.yml index 51cf9c0fc..76db2948f 100644 --- a/.github/workflows/test-cloud-hypervisor.yml +++ b/.github/workflows/test-cloud-hypervisor.yml @@ -252,15 +252,16 @@ jobs: if: always() run: | set -euo pipefail - while read -r namespace _; do - case "$namespace" in - awfvm-*) sudo ip netns delete "$namespace" ;; - esac - done < <(sudo ip netns list) if sudo ip netns list | grep -q '^awfvm-'; then + sudo ip netns list >&2 echo "::error::Cloud Hypervisor namespace residue remains after cleanup" exit 1 fi + if sudo iptables -S DOCKER-USER | grep -q -- '--comment awf:awf_vm_'; then + sudo iptables -S DOCKER-USER >&2 + echo "::error::Cloud Hypervisor bridge-rule residue remains after cleanup" + exit 1 + fi # /sys/fs/cgroup/awf-cloud-hypervisor is a parent cgroup that # persists across the whole job; only per-run sub-cgroups are # created one level inside it (see cgroupPath in diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index c797003ec..3f08e87bb 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -86,7 +86,30 @@ AWF performs these steps for each run: and remove network, cgroup, and run-directory resources. Cleanup is idempotent and aggregates errors so one cleanup failure does not -skip later cleanup steps. +skip later cleanup steps. Before the first privileged per-run resource is +created, AWF atomically writes a root-owned mode-`0600` recovery record under +`/run/awf-cloud-hypervisor/pending-cleanup/`, itself a root-owned mode-`0700` +directory. The record contains the owning AWF PID, `/proc` start time, +executable identity, exact resource names, and immutable inode/ifindex +identities captured immediately after each namespace, interface, run directory, +cgroup, VMM, and `virtiofsd` process becomes live. The host bridge-forwarding +rule is tagged with a per-run iptables comment and recorded by its exact tuple, +so concurrent runs do not share an anonymously owned rule. Any staged +virtio-fs bind mounts are recorded by mount ID, device, root, target, filesystem +type, and source; stale recovery revalidates and unmounts them deepest-first +before removing their inode-validated share directory. + +Every subsequent Cloud Hypervisor startup reaps stale records before creating +its own resources. A record whose owner still has the same PID, start time, +executable inode, credentials, and network namespace is active and is skipped, +so concurrent sibling runs cannot reap one another. For an abandoned record, +AWF revalidates every existing resource and process immediately before acting. +It never treats a name or PID alone as ownership evidence: PID reuse, a changed +namespace/interface inode or ifindex, an uncommitted launch identity, malformed +state, or an unsafe record mode stops cleanup, reports an error, and preserves +the record and resources for diagnosis. The record is removed only after normal +teardown succeeds. `--keep-containers` is an explicit diagnostic opt-out: its +record is removed while the requested resources remain preserved. ## Security boundaries diff --git a/scripts/ci/cloud-hypervisor-live-smoke.sh b/scripts/ci/cloud-hypervisor-live-smoke.sh index 7120c08c7..68974d730 100755 --- a/scripts/ci/cloud-hypervisor-live-smoke.sh +++ b/scripts/ci/cloud-hypervisor-live-smoke.sh @@ -7,8 +7,9 @@ set -euo pipefail # This covers allowed/blocked domains, direct # egress, arbitrary TCP, DNS, metadata IP, mandatory API-proxy reflect with # secret-sentinel absence, live workspace sharing incl. symlinks/permissions, -# exit-code propagation, timeout, SIGTERM cancellation, partial-start -# rollback, keep/preserve diagnostics, plus backend-specific live checks: +# exit-code propagation, timeout, SIGTERM cancellation, abrupt process-death +# recovery, partial-start rollback, keep/preserve diagnostics, plus +# backend-specific live checks: # # - device-assumptions: confirms eth0, the sole /dev/vda block disk, and # virtio-fs workspace layout documented in Part 6. @@ -95,6 +96,11 @@ assert_no_residue() { echo "Cloud Hypervisor veth/TAP residue detected" >&2 return 1 fi + if sudo iptables -S DOCKER-USER | grep -q -- '--comment awf:awf_vm_'; then + sudo iptables -S DOCKER-USER >&2 + echo "Cloud Hypervisor per-run bridge rule residue detected" >&2 + return 1 + fi # $CGROUP_ROOT (.../awf-cloud-hypervisor) is a *parent* cgroup that # persists across runs; only per-run sub-cgroups live one level # inside it (see cgroupPath in src/cloud-hypervisor/manager.ts). Any @@ -427,6 +433,65 @@ if [ "$cleanup_ms" -gt "$CLEANUP_CEILING_MS" ]; then exit 1 fi +# SIGKILL cannot run in-process finally/signal cleanup. Kill the root AWF Node +# process itself, prove its VMM/netns/cgroup survive, then let the next ordinary +# invocation recover them through the durable identity-validated registry. +crash_work="$RUN_ROOT/process-death/work" +crash_workspace="$RUN_ROOT/process-death/workspace" +crash_audit="$RUN_ROOT/process-death/audit" +mkdir -p "$crash_work" "$crash_workspace" "$crash_audit" +( + export GITHUB_WORKSPACE="$crash_workspace" + export OPENAI_API_KEY="$SECRET_SENTINEL" + exec sudo -E node "$ROOT/dist/cli.js" \ + "${COMMON[@]}" \ + --work-dir "$crash_work" \ + --audit-dir "$crash_audit" \ + -- 'sleep 300' +) >"$RUN_ROOT/process-death/stdout.log" 2>"$RUN_ROOT/process-death/stderr.log" & +crash_wrapper_pid=$! +crash_node_pid= +for _ in $(seq 1 90); do + for candidate in $(pgrep -f "node $ROOT/dist/cli.js.*--work-dir $crash_work" || true); do + [ "$candidate" = "$crash_wrapper_pid" ] && continue + candidate_exe=$(sudo readlink "/proc/$candidate/exe" 2>/dev/null || true) + case "$candidate_exe" in + */node) crash_node_pid=$candidate; break ;; + esac + done + if [ -n "$crash_node_pid" ] && + sudo ip netns list | grep -q '^awfvm-' && + sudo find /run/awf-cloud-hypervisor/pending-cleanup -maxdepth 1 -name '*.json' | grep -q . && + sudo find "$CGROUP_ROOT" -mindepth 1 -maxdepth 1 -type d | grep -q . && + pgrep -f "$ARTIFACT_DIR/cloud-hypervisor --api-socket" >/dev/null; then + break + fi + sleep 1 +done +[ -n "$crash_node_pid" ] || { + echo "process-death: AWF process did not become live" >&2 + exit 1 +} +sudo kill -KILL "$crash_node_pid" +set +e +wait "$crash_wrapper_pid" +crash_status=$? +set -e +[ "$crash_status" -ne 0 ] || { + echo "process-death: SIGKILL unexpectedly returned success" >&2 + exit 1 +} +sudo ip netns list | grep -q '^awfvm-' || { + echo "process-death: abrupt exit did not leave the expected recovery fixture" >&2 + exit 1 +} +pgrep -f "$ARTIFACT_DIR/cloud-hypervisor --api-socket" >/dev/null || { + echo "process-death: VMM did not survive abrupt owner death" >&2 + exit 1 +} +run_case process-death-reaper 0 'true' +assert_no_residue + keep_work="$RUN_ROOT/keep/work" keep_workspace="$RUN_ROOT/keep/workspace" keep_audit="$RUN_ROOT/keep/audit" @@ -480,6 +545,16 @@ sudo find "$keep_audit/cloud-hypervisor" -type f -size +1048576c -print -quit \ exit 1 } +readarray -t keep_forward_rule < <( + sudo node -e ' + const plan = require(process.argv[1]); + console.log(plan.infrastructureBridge); + console.log(plan.hostForwardRuleComment); + ' "$keep_audit/cloud-hypervisor/network-plan.json" +) +sudo iptables -t filter -D DOCKER-USER \ + -i "${keep_forward_rule[0]}" -o "${keep_forward_rule[0]}" \ + -m comment --comment "${keep_forward_rule[1]}" -j ACCEPT while read -r namespace _; do case "$namespace" in awfvm-*) sudo ip netns delete "$namespace" ;; diff --git a/src/cloud-hypervisor/cleanup-registry.test.ts b/src/cloud-hypervisor/cleanup-registry.test.ts new file mode 100644 index 000000000..1652da826 --- /dev/null +++ b/src/cloud-hypervisor/cleanup-registry.test.ts @@ -0,0 +1,280 @@ +import { promises as fs } from 'fs'; +import type { PathLike } from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { createMicrovmNetworkPlan } from '../microvm/network'; +import { + DurableCloudHypervisorCleanupRegistry, + type CleanupRegistryDependencies, +} from './cleanup-registry'; +import type { CloudHypervisorRunPaths } from './manager-types'; + +function procStat(pid: number, startTime: string): string { + return `${pid} (node) S ${Array(18).fill('0').join(' ')} ${startTime}\n`; +} + +function procStatus(uid = 0, gid = 0): string { + return `Uid:\t${uid}\t${uid}\t${uid}\t${uid}\nGid:\t${gid}\t${gid}\t${gid}\t${gid}\n`; +} + +describe('DurableCloudHypervisorCleanupRegistry', () => { + let temporaryRoot: string; + let ownerStartTime: string; + let daemonNamespace: string; + let mountInfo: string; + + beforeEach(async () => { + temporaryRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'awf-cleanup-registry-')); + ownerStartTime = '1000'; + daemonNamespace = 'net:[5000]'; + mountInfo = ''; + }); + + afterEach(async () => { + await fs.rm(temporaryRoot, { recursive: true, force: true }); + }); + + function dependencies(overrides: CleanupRegistryDependencies = {}): CleanupRegistryDependencies { + const lstat: typeof fs.lstat = (async (filePath: PathLike, options?: unknown) => { + const value = await fs.lstat(filePath, options as never); + if (typeof value.uid === 'bigint') return Object.assign(value, { uid: 0n }); + return Object.assign(value, { uid: 0 }); + }) as typeof fs.lstat; + return { + rootDirectory: temporaryRoot, + effectiveUid: 0, + processId: 4242, + lstat, + readFile: (async (filePath: PathLike, options?: unknown) => { + const name = String(filePath); + if (name === '/proc/4242/stat') return procStat(4242, ownerStartTime); + if (name === '/proc/4242/status') return procStatus(); + if (name === '/proc/4242/cmdline') return 'node\0test\0'; + if (name === '/proc/5000/stat') return procStat(5000, '5000'); + if (name === '/proc/5000/status') return procStatus(); + if (name === '/proc/5000/cmdline') return `${process.execPath}\0--socket-path=/sock\0--shared-dir=/source\0`; + if (name === '/proc/self/mountinfo') return mountInfo; + return fs.readFile(filePath, options as never); + }) as typeof fs.readFile, + readlink: (async (filePath: PathLike) => { + if (String(filePath) === '/proc/4242/ns/net') return 'net:[4026531840]'; + if (String(filePath) === '/proc/5000/ns/net') return daemonNamespace; + return fs.readlink(filePath); + }) as typeof fs.readlink, + realpath: (async (filePath: PathLike) => { + if (String(filePath) === '/proc/4242/exe') return process.execPath; + if (String(filePath) === '/proc/5000/exe') return process.execPath; + return fs.realpath(filePath); + }) as typeof fs.realpath, + run: jest.fn(async (_command: string, args: readonly string[]) => ({ + exitCode: 1, + stdout: '', + stderr: args.includes('netns') + ? 'Cannot open network namespace "missing": No such file or directory' + : 'Device does not exist', + })), + kill: jest.fn(), + sleep: jest.fn().mockResolvedValue(undefined), + ...overrides, + }; + } + + function runPaths(runId: string): CloudHypervisorRunPaths { + const runBaseDir = path.join(temporaryRoot, 'runs'); + const runDirectory = path.join(runBaseDir, 'cloud-hypervisor', runId); + return { + runId, + runBaseDir, + runDirectory, + apiSocketPath: path.join(runDirectory, 'api.socket'), + kernelPath: path.join(runDirectory, 'kernel'), + rootfsPath: path.join(runDirectory, 'rootfs.ext4'), + vsockSocketPath: path.join(runDirectory, 'awf-vsock.socket'), + logPath: path.join(runDirectory, 'cloud-hypervisor.log'), + serialLogPath: path.join(runDirectory, 'serial.log'), + virtiofsdShareDirectory: path.join(runBaseDir, 'virtiofsd', runId), + cgroupPath: path.join(temporaryRoot, 'cgroup', runId), + }; + } + + function networkPlan(runId: string) { + return createMicrovmNetworkPlan(runId, { + infrastructureBridge: 'awfbr0', + enableApiProxy: true, + tapOwnerUid: 1000, + tapOwnerGid: 1000, + }); + } + + it('atomically creates a private record before any resource identity is live', async () => { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const paths = runPaths('recorded-run'); + const plan = networkPlan(paths.runId); + + await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + + const recordPath = path.join(temporaryRoot, 'pending-cleanup', 'recorded-run.json'); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { + owner: { pid: number; startTime: string; executable: string }; + paths: { runDirectory: string; cgroupPath: string }; + network: { namespaceName: string; hostVethName: string; tapName: string }; + identities: Record; + }; + expect(record.owner).toMatchObject({ + pid: 4242, + startTime: '1000', + executable: await fs.realpath(process.execPath), + }); + expect(record.paths).toEqual({ + runDirectory: paths.runDirectory, + cgroupPath: paths.cgroupPath, + virtiofsdShareDirectory: paths.virtiofsdShareDirectory, + }); + expect(record.network).toMatchObject({ + namespaceName: plan.namespaceName, + hostVethName: plan.hostVethName, + tapName: plan.tapName, + }); + expect(record.identities).toEqual({}); + expect((await fs.stat(recordPath)).mode & 0o777).toBe(0o600); + expect((await fs.stat(path.dirname(recordPath))).mode & 0o777).toBe(0o700); + }); + + it('skips a live owner so sibling runs cannot reap each other', async () => { + const deps = dependencies(); + const registry = new DurableCloudHypervisorCleanupRegistry(deps); + const paths = runPaths('active-run'); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).resolves.toBeUndefined(); + + await expect(fs.access( + path.join(temporaryRoot, 'pending-cleanup', 'active-run.json'), + )).resolves.toBeUndefined(); + expect(deps.kill).not.toHaveBeenCalled(); + }); + + it('reaps an abandoned pre-resource record after owner PID reuse', async () => { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const paths = runPaths('stale-run'); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + await expect(fs.access( + path.join(temporaryRoot, 'pending-cleanup', 'stale-run.json'), + )).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('atomically takes over and removes a stale cleanup claim', async () => { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const paths = runPaths('stale-claim'); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', 'stale-claim.json'); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { owner: unknown }; + await fs.writeFile(`${recordPath}.lock`, `${JSON.stringify(record.owner)}\n`, { mode: 0o600 }); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + const names = await fs.readdir(path.dirname(recordPath)); + expect(names.filter((name) => name.startsWith('stale-claim.json'))).toEqual([]); + }); + + it('retains evidence and fails when a live resource lacks a committed identity', async () => { + const base = dependencies(); + const originalLstat = base.lstat as typeof fs.lstat; + const plan = networkPlan('uncertain-run'); + const lstat: typeof fs.lstat = (async (filePath: PathLike, options?: unknown) => { + if (String(filePath) === plan.netnsPath) { + const bigint = Boolean((options as { bigint?: boolean } | undefined)?.bigint); + return { + dev: bigint ? 1n : 1, + ino: bigint ? 2n : 2, + uid: bigint ? 0n : 0, + mode: bigint ? 0o100600n : 0o100600, + isFile: () => true, + isDirectory: () => false, + isSymbolicLink: () => false, + }; + } + return originalLstat(filePath, options as never); + }) as typeof fs.lstat; + const registry = new DurableCloudHypervisorCleanupRegistry({ ...base, lstat }); + const paths = runPaths(plan.runId); + await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /netns exists but its immutable identity was never committed/, + ); + await expect(fs.access( + path.join(temporaryRoot, 'pending-cleanup', 'uncertain-run.json'), + )).resolves.toBeUndefined(); + }); + + it('records the settled private network namespace of sandboxed virtiofsd', async () => { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const paths = runPaths('virtiofsd-netns'); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + + await handle.prepareProcess('virtiofsd-0', process.execPath, '/sock', '/source'); + await handle.captureProcess('virtiofsd-0', 5000); + + const record = JSON.parse(await fs.readFile( + path.join(temporaryRoot, 'pending-cleanup', 'virtiofsd-netns.json'), + 'utf8', + )) as { processes: Record }; + expect(record.processes['virtiofsd-0'].identity.networkNamespace).toBe('net:[5000]'); + }); + + it('revalidates and unmounts recorded virtiofs bind mounts deepest-first', async () => { + const run = jest.fn(async (command: string) => { + if (command === '/usr/bin/umount') { + mountInfo = ''; + return { exitCode: 0, stdout: '', stderr: '' }; + } + return { exitCode: 1, stdout: '', stderr: 'Device does not exist' }; + }); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ run })); + const paths = runPaths('mounted-run'); + const mountPoint = path.join(paths.virtiofsdShareDirectory, '0-workspace'); + await fs.mkdir(mountPoint, { recursive: true }); + mountInfo = `123 1 8:1 /source ${mountPoint} rw - ext4 /dev/sda1 rw\n`; + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.captureVirtiofsdResources(); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + expect(run).toHaveBeenCalledWith('/usr/bin/umount', [mountPoint]); + await expect(fs.access(paths.virtiofsdShareDirectory)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('retries inode-validated cgroup removal while kernel accounting drains', async () => { + const paths = runPaths('cgroup-drain'); + await fs.mkdir(paths.cgroupPath, { recursive: true }); + let attempts = 0; + const rmdir = jest.fn(async (directory: PathLike) => { + attempts += 1; + if (attempts === 1) throw Object.assign(new Error('busy'), { code: 'EBUSY' }); + await fs.rmdir(directory); + }) as typeof fs.rmdir; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ rmdir })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.captureCgroup(); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + expect(rmdir).toHaveBeenCalledTimes(2); + await expect(fs.access(paths.cgroupPath)).rejects.toMatchObject({ code: 'ENOENT' }); + }); +}); diff --git a/src/cloud-hypervisor/cleanup-registry.ts b/src/cloud-hypervisor/cleanup-registry.ts new file mode 100644 index 000000000..1d4fcc458 --- /dev/null +++ b/src/cloud-hypervisor/cleanup-registry.ts @@ -0,0 +1,1075 @@ +import { randomBytes } from 'crypto'; +import { constants, promises as fs } from 'fs'; +import * as path from 'path'; +import execa from 'execa'; +import type { MicrovmNetworkPlan } from '../microvm/network'; +import type { CloudHypervisorRunPaths } from './manager-types'; + +const CLEANUP_DIRECTORY_NAME = 'pending-cleanup'; +const RECORD_VERSION = 1; +const PROCESS_STOP_WAIT_MS = 2_000; +const PROCESS_STOP_INTERVAL_MS = 50; +const PROCESS_IDENTITY_WAIT_MS = 2_000; +const PROCESS_IDENTITY_INTERVAL_MS = 10; +const CGROUP_REMOVAL_WAIT_MS = 5_000; +const CGROUP_REMOVAL_INTERVAL_MS = 100; + +interface FileIdentity { + readonly device: string; + readonly inode: string; +} + +interface ProcessIdentity { + readonly pid: number; + readonly startTime: string; + readonly executable: string; + readonly executableIdentity: FileIdentity; + readonly uid: number; + readonly gid: number; + readonly networkNamespace: string; +} + +interface InterfaceIdentity { + readonly name: string; + readonly namespace?: string; + readonly ifindex: number; +} + +interface MountIdentity { + readonly mountId: number; + readonly device: string; + readonly root: string; + readonly mountPoint: string; + readonly filesystemType: string; + readonly source: string; +} + +interface RecordedProcess { + readonly state: 'pending' | 'live'; + readonly executable: string; + readonly socketPath: string; + readonly sourcePath?: string; + readonly identity?: ProcessIdentity; +} + +interface CleanupRecord { + readonly version: 1; + readonly runId: string; + readonly owner: ProcessIdentity; + readonly cloudHypervisorBinary: string; + readonly paths: { + readonly runDirectory: string; + readonly cgroupPath: string; + readonly virtiofsdShareDirectory: string; + }; + readonly network: { + readonly namespaceName: string; + readonly netnsPath: string; + readonly hostVethName: string; + readonly namespaceVethName: string; + readonly tapName: string; + readonly infrastructureBridge: string; + readonly hostForwardRuleComment: string; + }; + readonly identities: { + runDirectory?: FileIdentity; + cgroup?: FileIdentity; + virtiofsdShareDirectory?: FileIdentity; + netns?: FileIdentity; + hostVeth?: InterfaceIdentity; + namespaceVeth?: InterfaceIdentity; + tap?: InterfaceIdentity; + }; + readonly processes: Record; + mounts: MountIdentity[]; + updatedAt: string; +} + +export type CloudHypervisorNetworkResource = + 'netns' | 'hostVeth' | 'namespaceVeth' | 'tap'; + +export interface CloudHypervisorCleanupHandle { + captureNetworkResource(resource: CloudHypervisorNetworkResource): Promise; + captureRunDirectory(): Promise; + captureCgroup(): Promise; + captureVirtiofsdResources(): Promise; + prepareProcess( + key: string, + executable: string, + socketPath: string, + sourcePath?: string, + ): Promise; + captureProcess(key: string, pid: number): Promise; + complete(): Promise; +} + +export interface CloudHypervisorCleanupRegistry { + reapPending(ipPath: string, umountPath: string): Promise; + create( + paths: CloudHypervisorRunPaths, + plan: MicrovmNetworkPlan, + cloudHypervisorBinary: string, + ipPath: string, + ): Promise; +} + +export interface CleanupRegistryDependencies { + readonly rootDirectory?: string; + readonly effectiveUid?: number; + readonly processId?: number; + readonly readFile?: typeof fs.readFile; + readonly readlink?: typeof fs.readlink; + readonly realpath?: typeof fs.realpath; + readonly lstat?: typeof fs.lstat; + readonly stat?: typeof fs.stat; + readonly mkdir?: typeof fs.mkdir; + readonly readdir?: typeof fs.readdir; + readonly rename?: typeof fs.rename; + readonly link?: typeof fs.link; + readonly unlink?: typeof fs.unlink; + readonly rm?: typeof fs.rm; + readonly rmdir?: typeof fs.rmdir; + readonly open?: typeof fs.open; + readonly kill?: typeof process.kill; + readonly run?: ( + command: string, + args: readonly string[], + ) => Promise<{ exitCode: number; stdout: string; stderr: string }>; + readonly sleep?: (milliseconds: number) => Promise; +} + +interface ResolvedDependencies { + readonly rootDirectory: string; + readonly effectiveUid: number; + readonly processId: number; + readonly readFile: typeof fs.readFile; + readonly readlink: typeof fs.readlink; + readonly realpath: typeof fs.realpath; + readonly lstat: typeof fs.lstat; + readonly stat: typeof fs.stat; + readonly mkdir: typeof fs.mkdir; + readonly readdir: typeof fs.readdir; + readonly rename: typeof fs.rename; + readonly link: typeof fs.link; + readonly unlink: typeof fs.unlink; + readonly rm: typeof fs.rm; + readonly rmdir: typeof fs.rmdir; + readonly open: typeof fs.open; + readonly kill: typeof process.kill; + readonly run: NonNullable; + readonly sleep: NonNullable; +} + +export class DurableCloudHypervisorCleanupRegistry implements CloudHypervisorCleanupRegistry { + private readonly dependencies: ResolvedDependencies; + + constructor(dependencies: CleanupRegistryDependencies = {}) { + const runRoot = dependencies.rootDirectory ?? '/run/awf-cloud-hypervisor'; + this.dependencies = { + rootDirectory: path.join(runRoot, CLEANUP_DIRECTORY_NAME), + effectiveUid: dependencies.effectiveUid ?? process.geteuid?.() ?? -1, + processId: dependencies.processId ?? process.pid, + readFile: dependencies.readFile ?? fs.readFile, + readlink: dependencies.readlink ?? fs.readlink, + realpath: dependencies.realpath ?? fs.realpath, + lstat: dependencies.lstat ?? fs.lstat, + stat: dependencies.stat ?? fs.stat, + mkdir: dependencies.mkdir ?? fs.mkdir, + readdir: dependencies.readdir ?? fs.readdir, + rename: dependencies.rename ?? fs.rename, + link: dependencies.link ?? fs.link, + unlink: dependencies.unlink ?? fs.unlink, + rm: dependencies.rm ?? fs.rm, + rmdir: dependencies.rmdir ?? fs.rmdir, + open: dependencies.open ?? fs.open, + kill: dependencies.kill ?? process.kill, + run: dependencies.run ?? runCommand, + sleep: dependencies.sleep ?? ((milliseconds) => + new Promise((resolve) => setTimeout(resolve, milliseconds))), + }; + } + + async reapPending(ipPath: string, umountPath: string): Promise { + await this.ensureRegistryDirectory(); + const names = await this.dependencies.readdir(this.dependencies.rootDirectory); + const errors: string[] = []; + for (const name of names) { + if (!/^[A-Za-z0-9_.-]+\.json$/.test(name)) continue; + const recordPath = path.join(this.dependencies.rootDirectory, name); + try { + const record = await this.readRecord(recordPath); + if (await this.processMatches(record.owner)) continue; + const release = await this.claim(recordPath); + if (!release) continue; + try { + await this.reapRecord(recordPath, record, ipPath, umountPath); + } finally { + await release(); + } + } catch (error) { + errors.push(`${recordPath}: ${formatError(error)}`); + } + } + if (errors.length > 0) { + throw new Error( + `Cloud Hypervisor stale cleanup is incomplete; retained recovery records: ${errors.join('; ')}`, + ); + } + } + + async create( + paths: CloudHypervisorRunPaths, + plan: MicrovmNetworkPlan, + cloudHypervisorBinary: string, + ipPath: string, + ): Promise { + await this.ensureRegistryDirectory(); + assertSafeRecordPaths(paths, plan); + const recordPath = path.join(this.dependencies.rootDirectory, `${paths.runId}.json`); + const owner = await this.captureProcessIdentity(this.dependencies.processId); + const binary = await this.dependencies.realpath(cloudHypervisorBinary); + const record: CleanupRecord = { + version: RECORD_VERSION, + runId: paths.runId, + owner, + cloudHypervisorBinary: binary, + paths: { + runDirectory: paths.runDirectory, + cgroupPath: paths.cgroupPath, + virtiofsdShareDirectory: paths.virtiofsdShareDirectory, + }, + network: { + namespaceName: plan.namespaceName, + netnsPath: plan.netnsPath, + hostVethName: plan.hostVethName, + namespaceVethName: plan.namespaceVethName, + tapName: plan.tapName, + infrastructureBridge: plan.infrastructureBridge, + hostForwardRuleComment: plan.hostForwardRuleComment, + }, + identities: {}, + processes: {}, + mounts: [], + updatedAt: new Date().toISOString(), + }; + await this.writeRecord(recordPath, record, true); + return this.createHandle(recordPath, record, ipPath); + } + + private createHandle( + recordPath: string, + record: CleanupRecord, + ipPath: string, + ): CloudHypervisorCleanupHandle { + const update = async (): Promise => { + record.updatedAt = new Date().toISOString(); + await this.writeRecord(recordPath, record, false); + }; + return { + captureNetworkResource: async (resource) => { + switch (resource) { + case 'netns': + record.identities.netns = await this.captureFileIdentity(record.network.netnsPath); + break; + case 'hostVeth': + record.identities.hostVeth = await this.captureInterfaceIdentity( + ipPath, record.network.hostVethName, + ); + break; + case 'namespaceVeth': + record.identities.namespaceVeth = await this.captureInterfaceIdentity( + ipPath, record.network.namespaceVethName, record.network.namespaceName, + ); + break; + case 'tap': + record.identities.tap = await this.captureInterfaceIdentity( + ipPath, record.network.tapName, record.network.namespaceName, + ); + break; + } + await update(); + }, + captureRunDirectory: async () => { + record.identities.runDirectory = await this.captureFileIdentity(record.paths.runDirectory); + await update(); + }, + captureCgroup: async () => { + record.identities.cgroup = await this.captureFileIdentity(record.paths.cgroupPath); + await update(); + }, + captureVirtiofsdResources: async () => { + if (await pathExists(record.paths.virtiofsdShareDirectory, this.dependencies.lstat)) { + record.identities.virtiofsdShareDirectory = await this.captureFileIdentity( + record.paths.virtiofsdShareDirectory, + ); + record.mounts = (await this.readMounts()).filter((mount) => + mount.mountPoint === record.paths.virtiofsdShareDirectory || + mount.mountPoint.startsWith(`${record.paths.virtiofsdShareDirectory}${path.sep}`), + ); + } + await update(); + }, + prepareProcess: async (key, executable, socketPath, sourcePath) => { + assertSafeProcessKey(key); + // The key is restricted to a non-prototypal identifier alphabet above. + // eslint-disable-next-line security/detect-object-injection + record.processes[key] = { + state: 'pending', + executable: await this.dependencies.realpath(executable), + socketPath, + ...(sourcePath ? { sourcePath } : {}), + }; + await update(); + }, + captureProcess: async (key, pid) => { + assertSafeProcessKey(key); + // The key is restricted to a non-prototypal identifier alphabet above. + // eslint-disable-next-line security/detect-object-injection + const pending = record.processes[key]; + if (!pending) throw new Error(`Cleanup identity was not prepared for process "${key}"`); + const expectedNetworkNamespace = key === 'vmm' + ? `net:[${record.identities.netns?.inode}]` + : undefined; + const requiresPrivateNetworkNamespace = key.startsWith('virtiofsd-'); + const deadline = Date.now() + PROCESS_IDENTITY_WAIT_MS; + let identity: ProcessIdentity; + for (;;) { + identity = await this.captureProcessIdentity(pid); + if ( + identity.executable === pending.executable && + ( + expectedNetworkNamespace === undefined || + identity.networkNamespace === expectedNetworkNamespace + ) && + ( + !requiresPrivateNetworkNamespace || + identity.networkNamespace !== record.owner.networkNamespace + ) + ) break; + if (Date.now() >= deadline) { + throw new Error(`Process "${key}" did not match its prepared cleanup identity`); + } + // execa returns after fork; allow the trusted ip -> setpriv -> target + // exec chain to finish before requiring the final executable/netns. + await this.dependencies.sleep(PROCESS_IDENTITY_INTERVAL_MS); + } + // eslint-disable-next-line security/detect-object-injection + record.processes[key] = { ...pending, state: 'live', identity }; + await update(); + }, + complete: async () => { + await this.dependencies.unlink(recordPath).catch((error: NodeJS.ErrnoException) => { + if (error.code !== 'ENOENT') throw error; + }); + }, + }; + } + + private async reapRecord( + recordPath: string, + record: CleanupRecord, + ipPath: string, + umountPath: string, + ): Promise { + await this.validateRecordResources(record, ipPath); + for (const [key, recorded] of Object.entries(record.processes)) { + if (recorded.state === 'pending' || !recorded.identity) { + throw new Error(`process "${key}" launch identity was never committed`); + } + if (await this.processMatches(recorded.identity, recorded)) { + await this.stopProcess(recorded.identity, recorded); + } + } + await this.unmountVirtiofsdResources(record, umountPath); + await this.deleteNetwork(record, ipPath); + await removeExactDirectory( + record.paths.cgroupPath, record.identities.cgroup, this.dependencies, false, + ); + await this.assertNoMountsUnder(record.paths.runDirectory); + await removeExactDirectory( + record.paths.runDirectory, record.identities.runDirectory, this.dependencies, true, + ); + await this.assertNoMountsUnder(record.paths.virtiofsdShareDirectory); + await removeExactDirectory( + record.paths.virtiofsdShareDirectory, + record.identities.virtiofsdShareDirectory, + this.dependencies, + true, + ); + await this.dependencies.unlink(recordPath); + } + + private async validateRecordResources(record: CleanupRecord, ipPath: string): Promise { + const netnsExists = await pathExists(record.network.netnsPath, this.dependencies.lstat); + await this.validateFileIfPresent(record.network.netnsPath, record.identities.netns, 'netns'); + await this.validateFileIfPresent( + record.paths.runDirectory, record.identities.runDirectory, 'run directory', + ); + await this.validateFileIfPresent(record.paths.cgroupPath, record.identities.cgroup, 'cgroup'); + await this.validateFileIfPresent( + record.paths.virtiofsdShareDirectory, + record.identities.virtiofsdShareDirectory, + 'virtiofsd share directory', + ); + await this.validateInterfaceIfPresent( + ipPath, record.network.hostVethName, record.identities.hostVeth, undefined, + ); + if (netnsExists) { + await this.validateInterfaceIfPresent( + ipPath, + record.network.namespaceVethName, + record.identities.namespaceVeth, + record.network.namespaceName, + ); + await this.validateInterfaceIfPresent( + ipPath, record.network.tapName, record.identities.tap, record.network.namespaceName, + ); + } + } + + private async deleteNetwork(record: CleanupRecord, ipPath: string): Promise { + if (await this.interfaceExists(ipPath, record.network.hostVethName)) { + await this.validateInterfaceIfPresent( + ipPath, + record.network.hostVethName, + record.identities.hostVeth, + undefined, + ); + await this.runChecked(ipPath, ['link', 'delete', record.network.hostVethName]); + } + if (await pathExists(record.network.netnsPath, this.dependencies.lstat)) { + await this.validateFileIfPresent( + record.network.netnsPath, + record.identities.netns, + 'netns', + ); + await this.runChecked(ipPath, ['netns', 'delete', record.network.namespaceName]); + } + const rule = bridgeForwardRule( + '-C', + record.network.infrastructureBridge, + record.network.hostForwardRuleComment, + ); + const checked = await this.dependencies.run('iptables', rule); + if (checked.exitCode === 0) { + await this.runChecked('iptables', bridgeForwardRule( + '-D', + record.network.infrastructureBridge, + record.network.hostForwardRuleComment, + )); + } else if (checked.exitCode !== 1) { + throw new Error( + `Could not revalidate per-run bridge rule: ${checked.stderr.trim() || checked.stdout.trim()}`, + ); + } + } + + private async stopProcess(identity: ProcessIdentity, recorded: RecordedProcess): Promise { + if (!this.tryKill(identity.pid, 'SIGTERM')) { + if (await this.processMatches(identity, recorded)) { + throw new Error(`process ${identity.pid} still matches after kill reported ESRCH`); + } + return; + } + const deadline = Date.now() + PROCESS_STOP_WAIT_MS; + while (Date.now() < deadline) { + if (!(await this.processMatches(identity, recorded))) return; + await this.dependencies.sleep(PROCESS_STOP_INTERVAL_MS); + } + if (!(await this.processMatches(identity, recorded))) return; + if (!this.tryKill(identity.pid, 'SIGKILL')) { + if (await this.processMatches(identity, recorded)) { + throw new Error(`process ${identity.pid} still matches after kill reported ESRCH`); + } + return; + } + for (let attempt = 0; attempt < PROCESS_STOP_WAIT_MS / PROCESS_STOP_INTERVAL_MS; attempt += 1) { + if (!(await this.processMatches(identity, recorded))) return; + await this.dependencies.sleep(PROCESS_STOP_INTERVAL_MS); + } + throw new Error(`identity-validated process ${identity.pid} did not exit`); + } + + private tryKill(pid: number, signal: NodeJS.Signals): boolean { + try { + this.dependencies.kill(pid, signal); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ESRCH') return false; + throw error; + } + } + + private async unmountVirtiofsdResources( + record: CleanupRecord, + umountPath: string, + ): Promise { + for (const expected of [...record.mounts].sort( + (left, right) => right.mountPoint.length - left.mountPoint.length, + )) { + const current = (await this.readMounts()).find( + (mount) => mount.mountPoint === expected.mountPoint, + ); + if (!current) continue; + if (!sameMountIdentity(current, expected)) { + throw new Error(`mount identity changed: ${expected.mountPoint}`); + } + await this.runChecked(umountPath, [expected.mountPoint]); + } + } + + private async readMounts(): Promise { + const text = await this.dependencies.readFile('/proc/self/mountinfo', 'utf8'); + return text.split(/\r?\n/).filter(Boolean).map(parseMountInfoLine); + } + + private async assertNoMountsUnder(directory: string): Promise { + const remaining = (await this.readMounts()).filter((mount) => + mount.mountPoint === directory || mount.mountPoint.startsWith(`${directory}${path.sep}`), + ); + if (remaining.length > 0) { + throw new Error( + `refusing recursive removal while mounts remain under ${directory}: ` + + remaining.map((mount) => mount.mountPoint).join(', '), + ); + } + } + + private async validateFileIfPresent( + filePath: string, + expected: FileIdentity | undefined, + label: string, + ): Promise { + if (!(await pathExists(filePath, this.dependencies.lstat))) return; + if (!expected) throw new Error(`${label} exists but its immutable identity was never committed`); + const current = await this.captureFileIdentity(filePath); + if (!sameFileIdentity(current, expected)) throw new Error(`${label} identity changed`); + } + + private async validateInterfaceIfPresent( + ipPath: string, + name: string, + expected: InterfaceIdentity | undefined, + namespace: string | undefined, + ): Promise { + const current = await this.tryCaptureInterfaceIdentity(ipPath, name, namespace); + if (!current) return; + if (!expected) throw new Error(`interface "${name}" exists but its identity was never committed`); + if (current.ifindex !== expected.ifindex || current.namespace !== expected.namespace) { + throw new Error(`interface "${name}" identity changed`); + } + } + + private async captureProcessIdentity(pid: number): Promise { + if (!Number.isSafeInteger(pid) || pid <= 1) throw new Error(`Unsafe process id: ${pid}`); + const statText = await this.dependencies.readFile(`/proc/${pid}/stat`, 'utf8'); + const closingParen = statText.lastIndexOf(')'); + if (closingParen < 0) throw new Error(`Malformed /proc/${pid}/stat`); + const fields = statText.slice(closingParen + 2).trim().split(/\s+/); + const startTime = fields[19]; + if (!startTime) throw new Error(`Missing process start time for PID ${pid}`); + const status = await this.dependencies.readFile(`/proc/${pid}/status`, 'utf8'); + const uid = parseStatusIdentity(status, 'Uid'); + const gid = parseStatusIdentity(status, 'Gid'); + const executable = await this.dependencies.realpath(`/proc/${pid}/exe`); + return { + pid, + startTime, + executable, + executableIdentity: await this.captureFileIdentity(executable), + uid, + gid, + networkNamespace: await this.dependencies.readlink(`/proc/${pid}/ns/net`), + }; + } + + private async processMatches( + expected: ProcessIdentity, + recorded?: RecordedProcess, + ): Promise { + try { + const current = await this.captureProcessIdentity(expected.pid); + if ( + current.startTime !== expected.startTime || + current.executable !== expected.executable || + !sameFileIdentity(current.executableIdentity, expected.executableIdentity) || + current.uid !== expected.uid || + current.gid !== expected.gid || + current.networkNamespace !== expected.networkNamespace + ) return false; + if (recorded) { + const cmdline = await this.dependencies.readFile(`/proc/${expected.pid}/cmdline`, 'utf8'); + if ( + !cmdline.includes(recorded.socketPath) || + (recorded.sourcePath !== undefined && !cmdline.includes(recorded.sourcePath)) + ) return false; + } + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return false; + throw error; + } + } + + private async captureFileIdentity(filePath: string): Promise { + const value = await this.dependencies.lstat(filePath, { bigint: true }); + return { device: value.dev.toString(), inode: value.ino.toString() }; + } + + private async captureInterfaceIdentity( + ipPath: string, + name: string, + namespace?: string, + ): Promise { + const identity = await this.tryCaptureInterfaceIdentity(ipPath, name, namespace); + if (!identity) throw new Error(`Could not capture interface identity for "${name}"`); + return identity; + } + + private async tryCaptureInterfaceIdentity( + ipPath: string, + name: string, + namespace?: string, + ): Promise { + const args = namespace + ? ['netns', 'exec', namespace, ipPath, '-json', 'link', 'show', 'dev', name] + : ['-json', 'link', 'show', 'dev', name]; + const result = await this.dependencies.run(ipPath, args); + if (result.exitCode !== 0) { + if (/does not exist|cannot find device/i.test(result.stderr)) return undefined; + throw new Error(`${ipPath} ${args.join(' ')} failed: ${result.stderr.trim()}`); + } + const parsed = JSON.parse(result.stdout) as unknown; + if (!Array.isArray(parsed) || parsed.length !== 1) { + throw new Error(`Unexpected interface inspection for "${name}"`); + } + const item = parsed[0] as { ifindex?: unknown; ifname?: unknown }; + if (item.ifname !== name || !Number.isSafeInteger(item.ifindex)) { + throw new Error(`Invalid interface inspection for "${name}"`); + } + return { name, ...(namespace ? { namespace } : {}), ifindex: item.ifindex as number }; + } + + private async interfaceExists(ipPath: string, name: string): Promise { + return (await this.tryCaptureInterfaceIdentity(ipPath, name)) !== undefined; + } + + private async readRecord(recordPath: string): Promise { + const fileStat = await this.dependencies.lstat(recordPath); + if (!fileStat.isFile() || fileStat.isSymbolicLink() || fileStat.uid !== 0 || + (fileStat.mode & 0o777) !== 0o600) { + throw new Error('cleanup record is not a root-owned mode-0600 regular file'); + } + const parsed = JSON.parse(await this.dependencies.readFile(recordPath, 'utf8')) as CleanupRecord; + validateRecord(parsed, recordPath, this.dependencies.rootDirectory); + return parsed; + } + + private async writeRecord( + recordPath: string, + record: CleanupRecord, + exclusive: boolean, + ): Promise { + const temporaryPath = `${recordPath}.tmp-${this.dependencies.processId}-${randomBytes(6).toString('hex')}`; + const handle = await this.dependencies.open( + temporaryPath, + constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, + 0o600, + ); + try { + await handle.writeFile(`${JSON.stringify(record, null, 2)}\n`); + await handle.sync(); + } finally { + await handle.close(); + } + try { + if (exclusive) { + try { + await this.dependencies.link(temporaryPath, recordPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') { + throw new Error(`Cleanup record already exists for run "${record.runId}"`); + } + throw error; + } + await this.dependencies.unlink(temporaryPath); + } else { + await this.dependencies.rename(temporaryPath, recordPath); + } + const directory = await this.dependencies.open(this.dependencies.rootDirectory, 'r'); + try { await directory.sync(); } finally { await directory.close(); } + } catch (error) { + await this.dependencies.unlink(temporaryPath).catch(() => undefined); + throw error; + } + } + + private async ensureRegistryDirectory(): Promise { + if (this.dependencies.effectiveUid !== 0) { + throw new Error('Cloud Hypervisor cleanup registry requires effective uid 0'); + } + await this.dependencies.mkdir(this.dependencies.rootDirectory, { recursive: true, mode: 0o700 }); + const value = await this.dependencies.lstat(this.dependencies.rootDirectory); + if ( + !value.isDirectory() || + value.isSymbolicLink() || + value.uid !== 0 || + (value.mode & 0o777) !== 0o700 + ) { + throw new Error( + `Cloud Hypervisor cleanup registry has unsafe ownership or mode: ${this.dependencies.rootDirectory}`, + ); + } + } + + private async claim(recordPath: string): Promise<(() => Promise) | undefined> { + const lockPath = `${recordPath}.lock`; + const owner = await this.captureProcessIdentity(this.dependencies.processId); + for (let attempt = 0; attempt < 3; attempt += 1) { + if (await this.hasActiveRenamedClaim(recordPath)) return undefined; + const temporaryPath = `${lockPath}.tmp-${this.dependencies.processId}-${randomBytes(6).toString('hex')}`; + const handle = await this.dependencies.open(temporaryPath, 'wx', 0o600); + try { + await handle.writeFile(`${JSON.stringify(owner)}\n`); + await handle.sync(); + } finally { + await handle.close(); + } + let acquired = false; + try { + await this.dependencies.link(temporaryPath, lockPath); + acquired = true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; + } finally { + await this.dependencies.unlink(temporaryPath).catch(() => undefined); + } + if (acquired) { + if (await this.hasActiveRenamedClaim(recordPath)) { + await this.dependencies.unlink(lockPath).catch(() => undefined); + return undefined; + } + return async () => { + await this.dependencies.unlink(lockPath).catch((error: NodeJS.ErrnoException) => { + if (error.code !== 'ENOENT') throw error; + }); + }; + } + const before = await this.dependencies.lstat(lockPath, { bigint: true }).catch( + (error: NodeJS.ErrnoException) => { + if (error.code === 'ENOENT') return undefined; + throw error; + }, + ); + if (!before) continue; + if ( + !before.isFile() || + before.isSymbolicLink() || + before.uid !== 0n || + (before.mode & 0o777n) !== 0o600n + ) throw new Error(`cleanup claim has unsafe ownership or mode: ${lockPath}`); + let existing: ProcessIdentity; + try { + existing = JSON.parse(await this.dependencies.readFile(lockPath, 'utf8')) as ProcessIdentity; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue; + throw new Error(`cleanup claim is unreadable; refusing to replace it: ${formatError(error)}`); + } + validateProcessIdentity(existing, 'cleanup claim owner'); + if (await this.processMatches(existing)) return undefined; + const claimedPath = `${lockPath}-claimed-owner`; + const claimedTemporaryPath = `${claimedPath}.tmp-${this.dependencies.processId}-${randomBytes(6).toString('hex')}`; + const claimedHandle = await this.dependencies.open(claimedTemporaryPath, 'wx', 0o600); + try { + await claimedHandle.writeFile(`${JSON.stringify(owner)}\n`); + await claimedHandle.sync(); + } finally { + await claimedHandle.close(); + } + try { + await this.dependencies.link(claimedTemporaryPath, claimedPath); + } catch (error) { + await this.dependencies.unlink(claimedTemporaryPath).catch(() => undefined); + if ((error as NodeJS.ErrnoException).code === 'EEXIST') return undefined; + throw error; + } + await this.dependencies.unlink(claimedTemporaryPath); + const current = await this.dependencies.lstat(lockPath, { bigint: true }).catch( + (error: NodeJS.ErrnoException) => { + if (error.code === 'ENOENT') return undefined; + throw error; + }, + ); + if (current && (before.dev !== current.dev || before.ino !== current.ino)) { + await this.dependencies.unlink(claimedPath); + return undefined; + } + if (current) await this.dependencies.unlink(lockPath); + return async () => { + await this.dependencies.unlink(claimedPath).catch((error: NodeJS.ErrnoException) => { + if (error.code !== 'ENOENT') throw error; + }); + }; + } + throw new Error(`could not atomically claim stale cleanup record: ${recordPath}`); + } + + private async hasActiveRenamedClaim(recordPath: string): Promise { + const prefix = `${path.basename(recordPath)}.lock-claimed-`; + for (const name of await this.dependencies.readdir(this.dependencies.rootDirectory)) { + if (!name.startsWith(prefix)) continue; + const claimPath = path.join(this.dependencies.rootDirectory, name); + let owner: ProcessIdentity; + try { + owner = JSON.parse(await this.dependencies.readFile(claimPath, 'utf8')) as ProcessIdentity; + } catch (error) { + throw new Error(`cleanup claim is unreadable; refusing to replace it: ${formatError(error)}`); + } + validateProcessIdentity(owner, 'renamed cleanup claim owner'); + if (await this.processMatches(owner)) return true; + await this.dependencies.unlink(claimPath).catch((error: NodeJS.ErrnoException) => { + if (error.code !== 'ENOENT') throw error; + }); + } + return false; + } + + private async runChecked(command: string, args: readonly string[]): Promise { + const result = await this.dependencies.run(command, args); + if (result.exitCode !== 0) { + throw new Error( + `${command} ${args.join(' ')} failed with code ${result.exitCode}: ` + + `${result.stderr.trim() || result.stdout.trim()}`, + ); + } + } +} + +async function runCommand( + command: string, + args: readonly string[], +): Promise<{ exitCode: number; stdout: string; stderr: string }> { + // `command` is the absolute `ip` path returned by the root-only preflight. + // eslint-disable-next-line local/no-unsafe-execa + const result = await execa(command, [...args], { + reject: false, + stdio: ['ignore', 'pipe', 'pipe'], + env: { PATH: '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin' }, + extendEnv: false, + timeout: 10_000, + }); + return { exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; +} + +async function removeExactDirectory( + directory: string, + expected: FileIdentity | undefined, + dependencies: ResolvedDependencies, + recursive: boolean, +): Promise { + if (!(await pathExists(directory, dependencies.lstat))) return; + if (!expected) throw new Error(`${directory} exists without a committed identity`); + const current = await dependencies.lstat(directory, { bigint: true }); + if ( + current.dev.toString() !== expected.device || + current.ino.toString() !== expected.inode + ) throw new Error(`${directory} identity changed`); + if (recursive) { + await dependencies.rm(directory, { recursive: true, force: false }); + } else { + const deadline = Date.now() + CGROUP_REMOVAL_WAIT_MS; + for (;;) { + try { + await dependencies.rmdir(directory); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if ( + (code !== 'EBUSY' && code !== 'ENOTEMPTY') || + Date.now() >= deadline + ) throw error; + const retryIdentity = await dependencies.lstat(directory, { bigint: true }); + if ( + retryIdentity.dev.toString() !== expected.device || + retryIdentity.ino.toString() !== expected.inode + ) throw new Error(`${directory} identity changed during cgroup drain`); + await dependencies.sleep(CGROUP_REMOVAL_INTERVAL_MS); + } + } + } +} + +function validateRecord( + record: CleanupRecord, + recordPath: string, + registryRoot: string, +): void { + if ( + record?.version !== RECORD_VERSION || + !record.runId || + !/^[A-Za-z0-9_.-]+$/.test(record.runId) || + path.join(registryRoot, `${record.runId}.json`) !== recordPath + ) throw new Error('invalid cleanup record identity'); + if ( + !record.paths?.runDirectory.endsWith(`/${record.runId}`) || + !record.paths?.cgroupPath.endsWith(`/${record.runId}`) || + !record.paths?.virtiofsdShareDirectory.endsWith(`/${record.runId}`) || + record.network?.netnsPath !== `/var/run/netns/${record.network.namespaceName}` + || !/^awf:awf_vm_[0-9a-f]{12}$/.test(record.network.hostForwardRuleComment) + || !/^[A-Za-z0-9_.-]{1,15}$/.test(record.network.infrastructureBridge) + ) throw new Error('cleanup record paths are not run-scoped'); + validateProcessIdentity(record.owner, 'cleanup record owner'); + if ( + typeof record.processes !== 'object' || + record.processes === null || + Array.isArray(record.processes) || + !Array.isArray(record.mounts) + ) throw new Error('cleanup record resource identities are malformed'); + for (const [key, processRecord] of Object.entries(record.processes)) { + assertSafeProcessKey(key); + if ( + (processRecord.state !== 'pending' && processRecord.state !== 'live') || + !path.isAbsolute(processRecord.executable) || + !path.isAbsolute(processRecord.socketPath) || + (processRecord.sourcePath !== undefined && !path.isAbsolute(processRecord.sourcePath)) || + (processRecord.state === 'live' && processRecord.identity === undefined) + ) throw new Error(`cleanup process record is malformed: ${key}`); + if (processRecord.identity) validateProcessIdentity(processRecord.identity, `process "${key}"`); + } + for (const mount of record.mounts) { + if ( + !Number.isSafeInteger(mount.mountId) || + mount.mountId <= 0 || + !mount.device || + !path.isAbsolute(mount.mountPoint) || + ( + mount.mountPoint !== record.paths.virtiofsdShareDirectory && + !mount.mountPoint.startsWith(`${record.paths.virtiofsdShareDirectory}${path.sep}`) + ) || + !mount.filesystemType || + !mount.source + ) throw new Error('cleanup mount identity is malformed'); + } +} + +function assertSafeRecordPaths( + paths: CloudHypervisorRunPaths, + plan: MicrovmNetworkPlan, +): void { + if ( + plan.runId !== paths.runId || + !paths.runDirectory.startsWith(`${paths.runBaseDir}${path.sep}`) || + !paths.runDirectory.endsWith(`${path.sep}${paths.runId}`) || + !paths.cgroupPath.endsWith(`${path.sep}${paths.runId}`) + ) throw new Error('Cloud Hypervisor cleanup resources are not scoped to one run'); +} + +function assertSafeProcessKey(key: string): void { + if ( + !/^[A-Za-z0-9_.-]+$/.test(key) || + key === '__proto__' || + key === 'constructor' || + key === 'prototype' + ) throw new Error(`Unsafe cleanup process key: ${key}`); +} + +function parseStatusIdentity(status: string, name: 'Uid' | 'Gid'): number { + const line = status.split(/\r?\n/).find((candidate) => candidate.startsWith(`${name}:`)); + const identities = line?.slice(name.length + 1).trim().split(/\s+/); + if ( + identities?.length !== 4 || + !identities.every((value) => /^\d+$/.test(value) && value === identities[0]) + ) { + throw new Error(`Process ${name} identities are not stable`); + } + + return Number(identities[0]); +} + +function validateProcessIdentity(identity: ProcessIdentity, label: string): void { + if ( + !identity || + !Number.isSafeInteger(identity.pid) || + identity.pid <= 1 || + !/^\d+$/.test(identity.startTime) || + !path.isAbsolute(identity.executable) || + !/^\d+$/.test(identity.executableIdentity?.device) || + !/^\d+$/.test(identity.executableIdentity?.inode) || + !Number.isSafeInteger(identity.uid) || + identity.uid < 0 || + !Number.isSafeInteger(identity.gid) || + identity.gid < 0 || + !/^net:\[\d+\]$/.test(identity.networkNamespace) + ) throw new Error(`${label} identity is malformed`); +} + +function sameFileIdentity(left: FileIdentity, right: FileIdentity): boolean { + return left.device === right.device && left.inode === right.inode; +} + +function sameMountIdentity(left: MountIdentity, right: MountIdentity): boolean { + return left.mountId === right.mountId && + left.device === right.device && + left.root === right.root && + left.mountPoint === right.mountPoint && + left.filesystemType === right.filesystemType && + left.source === right.source; +} + +function parseMountInfoLine(line: string): MountIdentity { + const fields = line.split(' '); + const separator = fields.indexOf('-'); + const mountId = Number(fields[0]); + if ( + separator < 6 || + !Number.isSafeInteger(mountId) || + !fields[2] || + !fields[3] || + !fields[4] || + !fields[separator + 1] || + !fields[separator + 2] + ) throw new Error('Malformed /proc/self/mountinfo entry'); + return { + mountId, + device: fields[2], + root: decodeMountInfoPath(fields[3]), + mountPoint: decodeMountInfoPath(fields[4]), + filesystemType: fields[separator + 1], + source: decodeMountInfoPath(fields[separator + 2]), + }; +} + +function decodeMountInfoPath(value: string): string { + return value.replace(/\\([0-7]{3})/g, (_match, octal: string) => + String.fromCharCode(Number.parseInt(octal, 8))); +} + +async function pathExists( + filePath: string, + lstat: typeof fs.lstat, +): Promise { + try { + await lstat(filePath); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return false; + throw error; + } +} + +function formatError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function bridgeForwardRule( + operation: '-C' | '-D', + bridge: string, + comment: string, +): string[] { + return [ + '-t', 'filter', operation, 'DOCKER-USER', + '-i', bridge, '-o', bridge, + '-m', 'comment', '--comment', comment, + '-j', 'ACCEPT', + ]; +} diff --git a/src/cloud-hypervisor/manager-start.ts b/src/cloud-hypervisor/manager-start.ts index 2ebf98405..7c4291ad6 100644 --- a/src/cloud-hypervisor/manager-start.ts +++ b/src/cloud-hypervisor/manager-start.ts @@ -30,6 +30,7 @@ import { hasReadOnlyWorkspaceMountPlan } from './filesystem-write-enforcement'; import type { VirtiofsdManager, VirtiofsdDevice } from './virtiofsd'; import { buildCloudHypervisorVmConfig } from './vm-config-builder'; import type { BoundedOutputCapture } from './diagnostics'; +import type { CloudHypervisorCleanupHandle } from './cleanup-registry'; export interface CloudHypervisorStartContext { config: CloudHypervisorOptions; @@ -48,6 +49,7 @@ export interface CloudHypervisorStartContext { setClient(client: CloudHypervisorApiClient | undefined): void; setVirtiofsd(virtiofsd: VirtiofsdManager | undefined): void; setFsDevices(devices: VirtiofsdDevice[]): void; + setCleanupRecord(record: CloudHypervisorCleanupHandle | undefined): void; getFsDevices(): VirtiofsdDevice[]; stop(): Promise; } @@ -67,6 +69,7 @@ export async function startCloudHypervisor( let startupError: unknown; try { const artifacts = await dependencies.preflight(config); + await dependencies.cleanupRegistry.reapPending(artifacts.tools.ip, artifacts.tools.umount); const identity = guestConfig?.identity ?? dependencies.resolveIdentity(); const networkPlan = createMicrovmNetworkPlan(paths.runId, { ...networkConfig, @@ -75,7 +78,16 @@ export async function startCloudHypervisor( tapVnetHdr: true, }); context.setNetworkPlan(networkPlan); - const network = dependencies.createNetwork(networkPlan, artifacts.tools); + const cleanupRecord = await dependencies.cleanupRegistry.create( + paths, + networkPlan, + artifacts.cloudHypervisorBinary, + artifacts.tools.ip, + ); + context.setCleanupRecord(cleanupRecord); + const network = dependencies.createNetwork(networkPlan, artifacts.tools, { + resourceCreated: (resource) => cleanupRecord.captureNetworkResource(resource), + }); context.setNetwork(network); await network.setup(); let rootfsSource = artifacts.rootfsPath; @@ -102,12 +114,14 @@ export async function startCloudHypervisor( } await prepareRunDirectory(dependencies, paths, identity); + await cleanupRecord.captureRunDirectory(); const cgroup = dependencies.createCgroup( paths.cgroupPath, { memoryMib: config.memoryMib, vcpuCount: config.vcpuCount }, ); context.setCgroup(cgroup); await cgroup.setup(); + await cleanupRecord.captureCgroup(); await stageArtifact(dependencies, artifacts.kernelPath, paths.kernelPath, 0o400, identity); await stageArtifact(dependencies, rootfsSource, paths.rootfsPath, 0o600, identity); await stageDiagnosticFile(dependencies, paths.logPath, identity); @@ -122,6 +136,9 @@ export async function startCloudHypervisor( apiSocketPath: paths.apiSocketPath, logFilePath: paths.logPath, }); + await cleanupRecord.prepareProcess( + 'vmm', artifacts.cloudHypervisorBinary, paths.apiSocketPath, + ); const child = dependencies.launch(launchCommand.command, [...launchCommand.args], { reject: false, stdio: ['ignore', 'pipe', 'pipe'], @@ -133,7 +150,9 @@ export async function startCloudHypervisor( context.setProcess(child); child.stdout?.on('data', (chunk: Buffer | string) => context.stdoutCapture.append(chunk)); child.stderr?.on('data', (chunk: Buffer | string) => context.stderrCapture.append(chunk)); - if (child.pid !== undefined) await cgroup.assign(child.pid); + if (child.pid === undefined) throw new Error('Cloud Hypervisor process did not expose a PID'); + await cleanupRecord.captureProcess('vmm', child.pid); + await cgroup.assign(child.pid); await waitForApiSocket(dependencies, paths, config.apiTimeoutMs, child); const client = dependencies.createClient(paths.apiSocketPath, config.apiTimeoutMs); @@ -143,9 +162,11 @@ export async function startCloudHypervisor( const virtiofsd = dependencies.createVirtiofsdManager( artifacts.virtiofsdBinary, paths.runDirectory, paths.virtiofsdShareDirectory, identity, cgroup, { mount: artifacts.tools.mount, umount: artifacts.tools.umount }, + cleanupRecord, ); context.setVirtiofsd(virtiofsd); context.setFsDevices(await virtiofsd.start(guestConfig.exports, guestConfig.mountEnforcement)); + await cleanupRecord.captureVirtiofsdResources(); } await client.vmCreate(buildCloudHypervisorVmConfig({ config, paths, networkPlan, ...(guestConfig ? { guestConfig } : {}), diff --git a/src/cloud-hypervisor/manager-stop.ts b/src/cloud-hypervisor/manager-stop.ts index 0b420c59e..48f55aa4e 100644 --- a/src/cloud-hypervisor/manager-stop.ts +++ b/src/cloud-hypervisor/manager-stop.ts @@ -12,6 +12,7 @@ import type { CloudHypervisorCgroup } from './launcher'; import type { MicrovmRootfsPreparer } from '../microvm/rootfs'; import type { VirtiofsdManager, VirtiofsdDevice } from './virtiofsd'; import type { CloudHypervisorGuestChannel } from './guest-execution'; +import type { CloudHypervisorCleanupHandle } from './cleanup-registry'; const SHUTDOWN_GRACE_MS = 5_000; @@ -28,6 +29,7 @@ export interface CloudHypervisorStopContext { fsDevices: VirtiofsdDevice[]; guest?: CloudHypervisorGuestChannel; cgroup?: CloudHypervisorCgroup; + cleanupRecord?: CloudHypervisorCleanupHandle; instanceStarted: boolean; lastVmInfo?: CloudHypervisorVmInfo; lastVmCounters?: CloudHypervisorVmCounters; @@ -117,6 +119,7 @@ export async function stopCloudHypervisor(context: CloudHypervisorStopContext): if (context.preserve) { try { await context.cgroup?.cleanup(); } catch (error) { errors.push(error); } context.setCgroup(undefined); + if (errors.length === 0) await context.cleanupRecord?.complete(); throwCleanupErrors(errors, 'Cloud Hypervisor preservation failed: '); return; } @@ -132,6 +135,7 @@ export async function stopCloudHypervisor(context: CloudHypervisorStopContext): ); } catch (error) { errors.push(error); } } + if (errors.length === 0) await context.cleanupRecord?.complete(); throwCleanupErrors(errors, 'Cloud Hypervisor cleanup failed: '); } diff --git a/src/cloud-hypervisor/manager-types.ts b/src/cloud-hypervisor/manager-types.ts index 5ebefe032..32b44886f 100644 --- a/src/cloud-hypervisor/manager-types.ts +++ b/src/cloud-hypervisor/manager-types.ts @@ -7,6 +7,7 @@ import { type MicrovmControlPeer, type MicrovmNetworkLifecycle, type MicrovmNetworkPlan, + type MicrovmNetworkResourceObserver, } from '../microvm/network'; import type { MicrovmVsockClient } from '../microvm/vsock-client'; import type { MicrovmRootfsConfig, MicrovmRootfsPreparer } from '../microvm/rootfs'; @@ -15,6 +16,10 @@ import type { CloudHypervisorDirectoryExport } from './exports'; import type { CloudHypervisorCgroup, CloudHypervisorResourceLimits } from './launcher'; import type { CloudHypervisorHostToolPaths, runCloudHypervisorPreflight } from './preflight'; import type { VirtiofsdManager, VirtiofsdMountEnforcement } from './virtiofsd'; +import type { + CloudHypervisorCleanupHandle, + CloudHypervisorCleanupRegistry, +} from './cleanup-registry'; const API_SOCKET_NAME = 'api.socket'; const VSOCK_SOCKET_NAME = 'awf-vsock.socket'; @@ -79,7 +84,12 @@ export interface CloudHypervisorManagerDependencies { rm(directory: string, options: { recursive: true; force: true }): Promise; sleep(milliseconds: number): Promise; createClient(socketPath: string, timeoutMs: number): CloudHypervisorApiClient; - createNetwork(plan: MicrovmNetworkPlan, tools: CloudHypervisorHostToolPaths): MicrovmNetworkLifecycle; + createNetwork( + plan: MicrovmNetworkPlan, + tools: CloudHypervisorHostToolPaths, + observer?: MicrovmNetworkResourceObserver, + ): MicrovmNetworkLifecycle; + cleanupRegistry: CloudHypervisorCleanupRegistry; createRootfsPreparer( config: MicrovmRootfsConfig, tools: CloudHypervisorHostToolPaths, @@ -91,6 +101,7 @@ export interface CloudHypervisorManagerDependencies { identity: { uid: number; gid: number }, cgroup: CloudHypervisorCgroup, tools: Pick, + cleanupRecord?: CloudHypervisorCleanupHandle, ): VirtiofsdManager; createVsockClient(socketPath: string, guestPort: number, timeoutMs: number): MicrovmVsockClient; createCgroup(cgroupPath: string, limits: CloudHypervisorResourceLimits): CloudHypervisorCgroup; diff --git a/src/cloud-hypervisor/manager.test.ts b/src/cloud-hypervisor/manager.test.ts index 7ce0e8cea..b1ed63dfb 100644 --- a/src/cloud-hypervisor/manager.test.ts +++ b/src/cloud-hypervisor/manager.test.ts @@ -20,6 +20,10 @@ import { type CloudHypervisorManagerNetworkConfig, } from './manager'; import type { CloudHypervisorHostToolPaths } from './preflight'; +import type { + CloudHypervisorCleanupHandle, + CloudHypervisorCleanupRegistry, +} from './cleanup-registry'; const hostTools: CloudHypervisorHostToolPaths = { ip: '/usr/bin/ip', @@ -121,6 +125,25 @@ function cgroupMock(): CloudHypervisorCgroup { } as unknown as CloudHypervisorCgroup; } +function cleanupHandleMock(): CloudHypervisorCleanupHandle { + return { + captureNetworkResource: jest.fn().mockResolvedValue(undefined), + captureRunDirectory: jest.fn().mockResolvedValue(undefined), + captureCgroup: jest.fn().mockResolvedValue(undefined), + captureVirtiofsdResources: jest.fn().mockResolvedValue(undefined), + prepareProcess: jest.fn().mockResolvedValue(undefined), + captureProcess: jest.fn().mockResolvedValue(undefined), + complete: jest.fn().mockResolvedValue(undefined), + }; +} + +function cleanupRegistryMock(): CloudHypervisorCleanupRegistry { + return { + reapPending: jest.fn().mockResolvedValue(undefined), + create: jest.fn().mockResolvedValue(cleanupHandleMock()), + }; +} + function dependencies( overrides: Partial = {}, ): CloudHypervisorManagerDependencies { @@ -157,6 +180,7 @@ function dependencies( sleep: jest.fn().mockResolvedValue(undefined), createClient: jest.fn().mockReturnValue(client), createNetwork: jest.fn((plan) => networkLifecycle(plan)), + cleanupRegistry: cleanupRegistryMock(), createRootfsPreparer: jest.fn(() => rootfsPreparerMock()), createVirtiofsdManager: jest.fn(() => virtiofsdManagerMock()), createVsockClient: jest.fn(), @@ -316,6 +340,7 @@ describe('CloudHypervisorManager', () => { tapVnetHdr: true, }), hostTools, + expect.any(Object), ); const lifecycle = (deps.createNetwork as jest.Mock).mock.results[0] .value as MicrovmNetworkLifecycle; @@ -399,6 +424,41 @@ describe('CloudHypervisorManager', () => { expect(order).toEqual(['network', 'cgroup', 'run-directory']); }); + it('commits cleanup intent before resources and removes it only after teardown', async () => { + const order: string[] = []; + const handle = cleanupHandleMock(); + (handle.complete as jest.Mock).mockImplementation(async () => { order.push('record-complete'); }); + const registry: CloudHypervisorCleanupRegistry = { + reapPending: jest.fn(async () => { order.push('reap'); }), + create: jest.fn(async () => { + order.push('record-create'); + return handle; + }), + }; + const deps = dependencies({ + cleanupRegistry: registry, + createNetwork: jest.fn((plan) => ({ + plan, + setup: jest.fn(async () => { + order.push('network-setup'); + return plan; + }), + cleanup: jest.fn(async () => { order.push('network-cleanup'); }), + })), + rm: jest.fn(async () => { order.push('run-directory'); }), + }); + const manager = new CloudHypervisorManager( + config(), '/tmp/awf', deps, 'durable-order', networkConfig(), + ); + + await manager.start(); + await manager.stop(); + + expect(order.indexOf('reap')).toBeLessThan(order.indexOf('record-create')); + expect(order.indexOf('record-create')).toBeLessThan(order.indexOf('network-setup')); + expect(order.slice(-3)).toEqual(['network-cleanup', 'run-directory', 'record-complete']); + }); + it('configures one rootfs disk and virtio-fs devices, then stops daemons after the VMM', async () => { const order: string[] = []; const child = processMock(); @@ -457,6 +517,7 @@ describe('CloudHypervisorManager', () => { ]), }), hostTools, + expect.any(Object), ); expect(client.vmCreate).toHaveBeenCalledWith(expect.objectContaining({ payload: expect.objectContaining({ cmdline: expect.stringContaining('init=/usr/sbin/awf-supervisor') }), @@ -559,8 +620,14 @@ describe('CloudHypervisorManager', () => { it('preserves the cgroup and run directory when virtiofsd cannot be reaped', async () => { const virtiofsd = virtiofsdManagerMock(); (virtiofsd.stop as jest.Mock).mockRejectedValue(new Error('virtiofsd did not exit')); + const handle = cleanupHandleMock(); + const registry: CloudHypervisorCleanupRegistry = { + reapPending: jest.fn().mockResolvedValue(undefined), + create: jest.fn().mockResolvedValue(handle), + }; const deps = dependencies({ createVirtiofsdManager: jest.fn().mockReturnValue(virtiofsd), + cleanupRegistry: registry, }); const manager = new CloudHypervisorManager( config(), @@ -583,6 +650,7 @@ describe('CloudHypervisorManager', () => { expect(lifecycle.cleanup).not.toHaveBeenCalled(); expect(cgroup.cleanup).not.toHaveBeenCalled(); expect(deps.rm).not.toHaveBeenCalled(); + expect(handle.complete).not.toHaveBeenCalled(); }); it('retries the vsock connect on the guest-not-ready-yet boot race, with a fresh client each attempt', async () => { @@ -855,6 +923,7 @@ describe('CloudHypervisorManager', () => { namespaceName: 'ns', netnsPath: '/var/run/netns/ns', nftTableName: 'table', + hostForwardRuleComment: 'awf:awf_vm_0123456789ab', infrastructureBridge: 'awfbr0', hostVethName: 'host', namespaceVethName: 'namespace', diff --git a/src/cloud-hypervisor/manager.ts b/src/cloud-hypervisor/manager.ts index b6d4b0f9b..f05276187 100644 --- a/src/cloud-hypervisor/manager.ts +++ b/src/cloud-hypervisor/manager.ts @@ -47,6 +47,10 @@ import type { CloudHypervisorHostToolPaths } from './preflight'; import { startCloudHypervisor } from './manager-start'; import { stopCloudHypervisor } from './manager-stop'; import { VirtiofsdManager, type VirtiofsdDevice } from './virtiofsd'; +import { + DurableCloudHypervisorCleanupRegistry, + type CloudHypervisorCleanupHandle, +} from './cleanup-registry'; export { CLOUD_HYPERVISOR_GUEST_VSOCK_PORT, @@ -77,10 +81,13 @@ const defaultDependencies: CloudHypervisorManagerDependencies = { rm: fs.rm, sleep: (milliseconds) => new Promise((resolve) => setTimeout(resolve, milliseconds)), createClient: (socketPath, timeoutMs) => new CloudHypervisorApiClient({ socketPath, timeoutMs }), - createNetwork: (plan, tools) => new MicrovmNetworkManager( + createNetwork: (plan, tools, observer) => new MicrovmNetworkManager( plan, new LinuxNetworkCommands(undefined, tools), + undefined, + observer, ), + cleanupRegistry: new DurableCloudHypervisorCleanupRegistry(), createRootfsPreparer: (config, tools) => new MicrovmRootfsPreparer(config, { runTool: async (command, args) => { const tool = tools[command as keyof CloudHypervisorHostToolPaths] ?? command; @@ -93,8 +100,11 @@ const defaultDependencies: CloudHypervisorManagerDependencies = { throw new Error(`${tool} exited with code ${result.exitCode}: ${result.stderr.trim()}`); }, }), - createVirtiofsdManager: (binaryPath, runDirectory, shareDirectory, identity, cgroup, tools) => - new VirtiofsdManager(binaryPath, runDirectory, shareDirectory, identity, cgroup, tools), + createVirtiofsdManager: ( + binaryPath, runDirectory, shareDirectory, identity, cgroup, tools, cleanupRecord, + ) => new VirtiofsdManager( + binaryPath, runDirectory, shareDirectory, identity, cgroup, tools, undefined, cleanupRecord, + ), createVsockClient: (socketPath, guestPort, timeoutMs) => new MicrovmVsockClient({ socketPath, guestPort, @@ -160,6 +170,7 @@ export class CloudHypervisorManager { private fsDevices: VirtiofsdDevice[] = []; private guest: CloudHypervisorGuestChannel | undefined; private cgroup: CloudHypervisorCgroup | undefined; + private cleanupRecord: CloudHypervisorCleanupHandle | undefined; private networkPlan: MicrovmNetworkPlan | undefined; private instanceStarted = false; // Snapshotted in stop(), before any shutdown attempt, since the API @@ -219,6 +230,7 @@ export class CloudHypervisorManager { setClient: (value) => { this.client = value; }, setVirtiofsd: (value) => { this.virtiofsd = value; }, setFsDevices: (value) => { this.fsDevices = value; }, + setCleanupRecord: (value) => { this.cleanupRecord = value; }, getFsDevices: () => this.fsDevices, stop: () => this.stop(), }); @@ -289,6 +301,7 @@ export class CloudHypervisorManager { fsDevices: this.fsDevices, guest: this.guest, cgroup: this.cgroup, + cleanupRecord: this.cleanupRecord, instanceStarted: this.instanceStarted, lastVmInfo: this.lastVmInfo, lastVmCounters: this.lastVmCounters, diff --git a/src/cloud-hypervisor/virtiofsd.ts b/src/cloud-hypervisor/virtiofsd.ts index 8c09c1f18..572f66d92 100644 --- a/src/cloud-hypervisor/virtiofsd.ts +++ b/src/cloud-hypervisor/virtiofsd.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import execa, { type ExecaChildProcess } from 'execa'; import type { CloudHypervisorCgroup } from './launcher'; import type { CloudHypervisorDirectoryExport } from './exports'; +import type { CloudHypervisorCleanupHandle } from './cleanup-registry'; import { StagedHostMountTree, selectMountPlan, @@ -123,6 +124,7 @@ export class VirtiofsdManager { private readonly cgroup: Pick, private readonly tools: { readonly mount: string; readonly umount: string }, private readonly dependencies: VirtiofsdDependencies = defaultDependencies, + private readonly cleanupRecord?: CloudHypervisorCleanupHandle, ) {} /** @@ -301,6 +303,13 @@ export class VirtiofsdManager { const args = buildVirtiofsdArgs(directoryExport, socketPath, sharedDirectory, { announceSubmounts: mountTree !== undefined, }); + const cleanupKey = `virtiofsd-${index}`; + await this.cleanupRecord?.prepareProcess( + cleanupKey, + this.binaryPath, + socketPath, + sharedDirectory, + ); const child = this.dependencies.launch(this.binaryPath, args, { reject: false, stdio: ['ignore', 'pipe', 'pipe'], @@ -326,6 +335,7 @@ export class VirtiofsdManager { if (!child.pid) throw new Error(`virtiofsd for "${directoryExport.tag}" did not expose a PID`); await this.cgroup.assign(child.pid); await this.waitForSocket(daemon); + await this.cleanupRecord?.captureProcess(cleanupKey, child.pid); await this.dependencies.chown(socketPath, this.identity.uid, this.identity.gid); } diff --git a/src/cloud-hypervisor/vm-config-builder.test.ts b/src/cloud-hypervisor/vm-config-builder.test.ts index 611226f64..4c91c227f 100644 --- a/src/cloud-hypervisor/vm-config-builder.test.ts +++ b/src/cloud-hypervisor/vm-config-builder.test.ts @@ -23,6 +23,7 @@ function networkPlan(): MicrovmNetworkPlan { namespaceName: 'ns', netnsPath: '/var/run/netns/ns', nftTableName: 'table', + hostForwardRuleComment: 'awf:awf_vm_0123456789ab', infrastructureBridge: 'awfbr0', hostVethName: 'host', namespaceVethName: 'namespace', diff --git a/src/microvm/network-commands.ts b/src/microvm/network-commands.ts index 60fbc9a9c..08ad9384b 100644 --- a/src/microvm/network-commands.ts +++ b/src/microvm/network-commands.ts @@ -128,27 +128,45 @@ export class LinuxNetworkCommands { * nftables allowlist (which still restricts what the guest can send in * the first place). */ - async ensureBridgeForwardAcceptRule(bridgeName: string): Promise { + async ensureBridgeForwardAcceptRule(bridgeName: string, comment: string): Promise { + assertIptablesComment(comment); + const rule = [ + '-t', 'filter', '-C', 'DOCKER-USER', + '-i', bridgeName, '-o', bridgeName, + '-m', 'comment', '--comment', comment, + '-j', 'ACCEPT', + ]; const checkResult = await this.execute( 'iptables', - ['-t', 'filter', '-C', 'DOCKER-USER', '-i', bridgeName, '-o', bridgeName, '-j', 'ACCEPT'], + rule, { reject: false }, ); const exitCode = (checkResult as { exitCode?: number } | undefined)?.exitCode; if (exitCode === 0) return; await this.execute( 'iptables', - ['-t', 'filter', '-I', 'DOCKER-USER', '1', '-i', bridgeName, '-o', bridgeName, '-j', 'ACCEPT'], + [ + '-t', 'filter', '-I', 'DOCKER-USER', '1', + '-i', bridgeName, '-o', bridgeName, + '-m', 'comment', '--comment', comment, + '-j', 'ACCEPT', + ], { reject: true }, ); } /** Removes the rule `ensureBridgeForwardAcceptRule` installs, if present. Tolerant of it already being gone or the underlying command failing outright. */ - async removeBridgeForwardAcceptRule(bridgeName: string): Promise { + async removeBridgeForwardAcceptRule(bridgeName: string, comment: string): Promise { + assertIptablesComment(comment); try { await this.execute( 'iptables', - ['-t', 'filter', '-D', 'DOCKER-USER', '-i', bridgeName, '-o', bridgeName, '-j', 'ACCEPT'], + [ + '-t', 'filter', '-D', 'DOCKER-USER', + '-i', bridgeName, '-o', bridgeName, + '-m', 'comment', '--comment', comment, + '-j', 'ACCEPT', + ], { reject: false }, ); } catch { @@ -156,6 +174,7 @@ export class LinuxNetworkCommands { // binary itself being unavailable, must not fail the caller's own // cleanup sequence. } + } /** @@ -313,3 +332,9 @@ export class LinuxNetworkCommands { ].join('\n'); } } + +function assertIptablesComment(comment: string): void { + if (!/^awf:awf_vm_[0-9a-f]{12}$/.test(comment)) { + throw new Error(`Unsafe microVM iptables rule comment: ${comment}`); + } +} diff --git a/src/microvm/network-manager.ts b/src/microvm/network-manager.ts index f4573584a..66b981ff9 100644 --- a/src/microvm/network-manager.ts +++ b/src/microvm/network-manager.ts @@ -7,6 +7,7 @@ import type { MicrovmConnectivityProbe, MicrovmNetworkLifecycle, MicrovmNetworkPlan, + MicrovmNetworkResourceObserver, } from './network-types'; export class MicrovmNetworkManager implements MicrovmNetworkLifecycle { @@ -19,6 +20,7 @@ export class MicrovmNetworkManager implements MicrovmNetworkLifecycle { readonly plan: MicrovmNetworkPlan, private readonly commands = new LinuxNetworkCommands(), private readonly probe?: MicrovmConnectivityProbe, + private readonly observer?: MicrovmNetworkResourceObserver, ) {} async setup(): Promise { @@ -31,16 +33,19 @@ export class MicrovmNetworkManager implements MicrovmNetworkLifecycle { try { await this.commands.ip(['netns', 'add', this.plan.namespaceName]); this.namespaceCreated = true; + await this.observer?.resourceCreated('netns'); await this.commands.ip([ 'link', 'add', this.plan.hostVethName, 'type', 'veth', 'peer', 'name', this.plan.namespaceVethName, ]); this.hostVethCreated = true; + await this.observer?.resourceCreated('hostVeth'); await this.commands.ip([ 'link', 'set', this.plan.namespaceVethName, 'netns', this.plan.namespaceName, ]); + await this.observer?.resourceCreated('namespaceVeth'); await this.commands.ip([ 'link', 'set', this.plan.hostVethName, 'master', this.plan.infrastructureBridge, @@ -53,7 +58,10 @@ export class MicrovmNetworkManager implements MicrovmNetworkLifecycle { // default-drop policy. Must happen before any guest traffic can // possibly flow (i.e. before the tap/guest side is even brought // up below). - await this.commands.ensureBridgeForwardAcceptRule(this.plan.infrastructureBridge); + await this.commands.ensureBridgeForwardAcceptRule( + this.plan.infrastructureBridge, + this.plan.hostForwardRuleComment, + ); this.dockerUserRuleInserted = true; await this.commands.ipInNamespace(this.plan.namespaceName, [ @@ -64,6 +72,7 @@ export class MicrovmNetworkManager implements MicrovmNetworkLifecycle { 'group', String(this.plan.tapOwnerGid), ...(this.plan.tapVnetHdr ? ['vnet_hdr'] : []), ]); + await this.observer?.resourceCreated('tap'); await this.commands.ipInNamespace(this.plan.namespaceName, [ 'addr', 'add', `${this.plan.guestGatewayIp}/${this.plan.guestPrefixLength}`, @@ -140,7 +149,10 @@ export class MicrovmNetworkManager implements MicrovmNetworkLifecycle { if (this.dockerUserRuleInserted) { await attempt(async () => { - await this.commands.removeBridgeForwardAcceptRule(this.plan.infrastructureBridge); + await this.commands.removeBridgeForwardAcceptRule( + this.plan.infrastructureBridge, + this.plan.hostForwardRuleComment, + ); this.dockerUserRuleInserted = false; }); } diff --git a/src/microvm/network-plan.ts b/src/microvm/network-plan.ts index eb93a57d8..6db134d15 100644 --- a/src/microvm/network-plan.ts +++ b/src/microvm/network-plan.ts @@ -71,6 +71,7 @@ export function createMicrovmNetworkPlan( namespaceName, netnsPath: `${NETNS_DIRECTORY}/${namespaceName}`, nftTableName, + hostForwardRuleComment: `awf:${nftTableName}`, infrastructureBridge: options.infrastructureBridge, hostVethName, namespaceVethName, diff --git a/src/microvm/network-types.ts b/src/microvm/network-types.ts index be7ea46eb..97ed79bdc 100644 --- a/src/microvm/network-types.ts +++ b/src/microvm/network-types.ts @@ -54,6 +54,7 @@ export interface MicrovmNetworkPlan { readonly namespaceName: string; readonly netnsPath: string; readonly nftTableName: string; + readonly hostForwardRuleComment: string; readonly infrastructureBridge: string; readonly hostVethName: string; readonly namespaceVethName: string; @@ -90,6 +91,10 @@ export interface MicrovmNetworkLifecycle { captureDiagnostics?(): Promise; } +export interface MicrovmNetworkResourceObserver { + resourceCreated(resource: 'netns' | 'hostVeth' | 'namespaceVeth' | 'tap'): Promise; +} + export interface MicrovmNetworkCommandOptions { readonly reject: boolean; readonly input?: string; diff --git a/src/microvm/network.test.ts b/src/microvm/network.test.ts index 095206fe9..3a703c99d 100644 --- a/src/microvm/network.test.ts +++ b/src/microvm/network.test.ts @@ -279,11 +279,13 @@ describe('microVM network lifecycle', () => { expect(calls[5].args).toEqual([ '-t', 'filter', '-C', 'DOCKER-USER', '-i', plan.infrastructureBridge, '-o', plan.infrastructureBridge, + '-m', 'comment', '--comment', plan.hostForwardRuleComment, '-j', 'ACCEPT', ]); expect(calls[6].args).toEqual([ '-t', 'filter', '-I', 'DOCKER-USER', '1', '-i', plan.infrastructureBridge, '-o', plan.infrastructureBridge, + '-m', 'comment', '--comment', plan.hostForwardRuleComment, '-j', 'ACCEPT', ]); expect(calls[7].args).toEqual([ @@ -308,6 +310,21 @@ describe('microVM network lifecycle', () => { expect(probe.verify).toHaveBeenCalledWith(plan); }); + it('publishes each resource identity immediately after creation', async () => { + const plan = createPlan(); + const { commands } = commandHarness(); + const observed: string[] = []; + const observer = { + resourceCreated: jest.fn(async (resource: string) => { + observed.push(resource); + }), + }; + + await new MicrovmNetworkManager(plan, commands, undefined, observer).setup(); + + expect(observed).toEqual(['netns', 'hostVeth', 'namespaceVeth', 'tap']); + }); + it.each([ '172.30.0.0', '172.30.0.0/24/extra', @@ -538,6 +555,7 @@ describe('microVM network lifecycle', () => { expect(insertCall?.args).toEqual([ '-t', 'filter', '-I', 'DOCKER-USER', '1', '-i', plan.infrastructureBridge, '-o', plan.infrastructureBridge, + '-m', 'comment', '--comment', plan.hostForwardRuleComment, '-j', 'ACCEPT', ]); @@ -546,6 +564,7 @@ describe('microVM network lifecycle', () => { expect(deleteCall?.args).toEqual([ '-t', 'filter', '-D', 'DOCKER-USER', '-i', plan.infrastructureBridge, '-o', plan.infrastructureBridge, + '-m', 'comment', '--comment', plan.hostForwardRuleComment, '-j', 'ACCEPT', ]); // Removal must be tolerant of the rule already being gone (e.g. a @@ -581,11 +600,17 @@ describe('LinuxNetworkCommands.ensureBridgeForwardAcceptRule / removeBridgeForwa }), ); - await commands.ensureBridgeForwardAcceptRule('awfbr0'); + await commands.ensureBridgeForwardAcceptRule('awfbr0', 'awf:awf_vm_0123456789ab'); expect(calls).toEqual([ - { args: ['-t', 'filter', '-C', 'DOCKER-USER', '-i', 'awfbr0', '-o', 'awfbr0', '-j', 'ACCEPT'] }, - { args: ['-t', 'filter', '-I', 'DOCKER-USER', '1', '-i', 'awfbr0', '-o', 'awfbr0', '-j', 'ACCEPT'] }, + { args: [ + '-t', 'filter', '-C', 'DOCKER-USER', '-i', 'awfbr0', '-o', 'awfbr0', + '-m', 'comment', '--comment', 'awf:awf_vm_0123456789ab', '-j', 'ACCEPT', + ] }, + { args: [ + '-t', 'filter', '-I', 'DOCKER-USER', '1', '-i', 'awfbr0', '-o', 'awfbr0', + '-m', 'comment', '--comment', 'awf:awf_vm_0123456789ab', '-j', 'ACCEPT', + ] }, ]); }); @@ -598,10 +623,13 @@ describe('LinuxNetworkCommands.ensureBridgeForwardAcceptRule / removeBridgeForwa }), ); - await commands.ensureBridgeForwardAcceptRule('awfbr0'); + await commands.ensureBridgeForwardAcceptRule('awfbr0', 'awf:awf_vm_0123456789ab'); expect(calls).toEqual([ - { args: ['-t', 'filter', '-C', 'DOCKER-USER', '-i', 'awfbr0', '-o', 'awfbr0', '-j', 'ACCEPT'] }, + { args: [ + '-t', 'filter', '-C', 'DOCKER-USER', '-i', 'awfbr0', '-o', 'awfbr0', + '-m', 'comment', '--comment', 'awf:awf_vm_0123456789ab', '-j', 'ACCEPT', + ] }, ]); }); @@ -612,7 +640,10 @@ describe('LinuxNetworkCommands.ensureBridgeForwardAcceptRule / removeBridgeForwa }), ); - await expect(commands.removeBridgeForwardAcceptRule('awfbr0')).resolves.toBeUndefined(); + await expect(commands.removeBridgeForwardAcceptRule( + 'awfbr0', + 'awf:awf_vm_0123456789ab', + )).resolves.toBeUndefined(); }); }); diff --git a/src/microvm/network.ts b/src/microvm/network.ts index b53e790d0..7234acfcb 100644 --- a/src/microvm/network.ts +++ b/src/microvm/network.ts @@ -18,6 +18,7 @@ export type { MicrovmNetworkLifecycle, MicrovmNetworkPlan, MicrovmNetworkPlanOptions, + MicrovmNetworkResourceObserver, MicrovmNetworkRulesetFile, MicrovmTapInterface, } from './network-types'; From 0c2a67f0ec868489d1f38d84133e4d823f594f26 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 08:45:22 -0700 Subject: [PATCH 2/6] fix: harden durable microvm identity cleanup Capture executable inode identity from the live procfs object. Verify firewall rule absence after deletion. Cover cleanup races at base coverage levels. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 92d2c346-d396-40d3-8b74-1f1447a04871 --- src/cloud-hypervisor/cleanup-registry.test.ts | 998 +++++++++++++++++- src/cloud-hypervisor/cleanup-registry.ts | 11 +- src/microvm/network-commands.ts | 46 +- src/microvm/network.test.ts | 36 +- 4 files changed, 1062 insertions(+), 29 deletions(-) diff --git a/src/cloud-hypervisor/cleanup-registry.test.ts b/src/cloud-hypervisor/cleanup-registry.test.ts index 1652da826..3d15bc28d 100644 --- a/src/cloud-hypervisor/cleanup-registry.test.ts +++ b/src/cloud-hypervisor/cleanup-registry.test.ts @@ -20,12 +20,20 @@ function procStatus(uid = 0, gid = 0): string { describe('DurableCloudHypervisorCleanupRegistry', () => { let temporaryRoot: string; let ownerStartTime: string; + let ownerExecutableLink: string; + let daemonExecutableLink: string; + let daemonCmdline: string; + let daemonAlive: boolean; let daemonNamespace: string; let mountInfo: string; beforeEach(async () => { temporaryRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'awf-cleanup-registry-')); ownerStartTime = '1000'; + ownerExecutableLink = process.execPath; + daemonExecutableLink = process.execPath; + daemonCmdline = `${process.execPath}\0--socket-path=/sock\0--shared-dir=/source\0`; + daemonAlive = true; daemonNamespace = 'net:[5000]'; mountInfo = ''; }); @@ -50,17 +58,29 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { if (name === '/proc/4242/stat') return procStat(4242, ownerStartTime); if (name === '/proc/4242/status') return procStatus(); if (name === '/proc/4242/cmdline') return 'node\0test\0'; + if (name.startsWith('/proc/5000/') && !daemonAlive) { + throw Object.assign(new Error('gone'), { code: 'ENOENT' }); + } if (name === '/proc/5000/stat') return procStat(5000, '5000'); if (name === '/proc/5000/status') return procStatus(); - if (name === '/proc/5000/cmdline') return `${process.execPath}\0--socket-path=/sock\0--shared-dir=/source\0`; + if (name === '/proc/5000/cmdline') return daemonCmdline; if (name === '/proc/self/mountinfo') return mountInfo; return fs.readFile(filePath, options as never); }) as typeof fs.readFile, readlink: (async (filePath: PathLike) => { + if (String(filePath) === '/proc/4242/exe') return ownerExecutableLink; + if (String(filePath) === '/proc/5000/exe') return daemonExecutableLink; if (String(filePath) === '/proc/4242/ns/net') return 'net:[4026531840]'; if (String(filePath) === '/proc/5000/ns/net') return daemonNamespace; return fs.readlink(filePath); }) as typeof fs.readlink, + stat: (async (filePath: PathLike, options?: unknown) => { + const name = String(filePath); + if (name === '/proc/4242/exe' || name === '/proc/5000/exe') { + return fs.stat(process.execPath, options as never); + } + return fs.stat(filePath, options as never); + }) as typeof fs.stat, realpath: (async (filePath: PathLike) => { if (String(filePath) === '/proc/4242/exe') return process.execPath; if (String(filePath) === '/proc/5000/exe') return process.execPath; @@ -106,6 +126,16 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { }); } + async function mutateRecord( + runId: string, + mutate: (record: Record) => void, + ): Promise { + const recordPath = path.join(temporaryRoot, 'pending-cleanup', `${runId}.json`); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as Record; + mutate(record); + await fs.writeFile(recordPath, `${JSON.stringify(record)}\n`, { mode: 0o600 }); + } + it('atomically creates a private record before any resource identity is live', async () => { const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); const paths = runPaths('recorded-run'); @@ -125,6 +155,7 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { startTime: '1000', executable: await fs.realpath(process.execPath), }); + expect(record.paths).toEqual({ runDirectory: paths.runDirectory, cgroupPath: paths.cgroupPath, @@ -140,6 +171,106 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { expect((await fs.stat(path.dirname(recordPath))).mode & 0o777).toBe(0o700); }); + it('requires root and refuses to replace an existing run record', async () => { + const paths = runPaths('exclusive-record'); + const plan = networkPlan(paths.runId); + await expect(new DurableCloudHypervisorCleanupRegistry( + dependencies({ effectiveUid: 1000 }), + ).create(paths, plan, process.execPath, '/usr/bin/ip')).rejects.toThrow( + /requires effective uid 0/, + ); + + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + await expect(registry.create(paths, plan, process.execPath, '/usr/bin/ip')).rejects.toThrow( + /Cleanup record already exists/, + ); + }); + + it('preserves the publication error when temporary-file cleanup also races', async () => { + const paths = runPaths('publication-failure'); + const link = jest.fn(async () => { + throw Object.assign(new Error('filesystem denied link'), { code: 'EPERM' }); + }) as typeof fs.link; + const unlink = jest.fn(async () => { + throw Object.assign(new Error('temporary file already gone'), { code: 'ENOENT' }); + }) as typeof fs.unlink; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ link, unlink })); + + await expect(registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + )).rejects.toThrow(/filesystem denied link/); + expect((await fs.readdir(path.join(temporaryRoot, 'pending-cleanup')))).toHaveLength(1); + }); + + it('validates process registration and completes idempotently', async () => { + const paths = runPaths('process-registration'); + const handle = await new DurableCloudHypervisorCleanupRegistry(dependencies()).create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + + await expect(handle.prepareProcess('__proto__', process.execPath, '/sock')).rejects.toThrow( + /Unsafe cleanup process key/, + ); + await expect(handle.captureProcess('missing', 5000)).rejects.toThrow( + /identity was not prepared/, + ); + await handle.complete(); + await expect(handle.complete()).resolves.toBeUndefined(); + }); + + it('times out instead of committing a process whose executable never matches', async () => { + const now = jest.spyOn(Date, 'now') + .mockReturnValueOnce(0) + .mockReturnValueOnce(2_001); + try { + const paths = runPaths('process-mismatch'); + const handle = await new DurableCloudHypervisorCleanupRegistry(dependencies()).create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + daemonExecutableLink = '/usr/bin/not-the-prepared-binary'; + + await expect(handle.captureProcess('worker', 5000)).rejects.toThrow( + /did not match its prepared cleanup identity/, + ); + } finally { + now.mockRestore(); + } + }); + + it('waits for the trusted exec chain to settle before committing process identity', async () => { + const paths = runPaths('process-settles'); + const sleep = jest.fn(async () => { + daemonExecutableLink = process.execPath; + }); + const handle = await new DurableCloudHypervisorCleanupRegistry( + dependencies({ sleep }), + ).create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + await handle.prepareProcess('worker', process.execPath, '/sock'); + daemonExecutableLink = '/usr/bin/setpriv'; + + await expect(handle.captureProcess('worker', 5000)).resolves.toBeUndefined(); + expect(sleep).toHaveBeenCalled(); + }); + + it('retains a stale record when a prepared process identity was never committed', async () => { + const paths = runPaths('pending-process'); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /launch identity was never committed/, + ); + await expect(fs.access( + path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`), + )).resolves.toBeUndefined(); + }); + it('skips a live owner so sibling runs cannot reap each other', async () => { const deps = dependencies(); const registry = new DurableCloudHypervisorCleanupRegistry(deps); @@ -154,6 +285,21 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { expect(deps.kill).not.toHaveBeenCalled(); }); + it('keeps a live owner active when its executable pathname was atomically replaced', async () => { + const deps = dependencies(); + const registry = new DurableCloudHypervisorCleanupRegistry(deps); + const paths = runPaths('upgraded-owner'); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + ownerExecutableLink = `${process.execPath} (deleted)`; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + await expect(fs.access( + path.join(temporaryRoot, 'pending-cleanup', 'upgraded-owner.json'), + )).resolves.toBeUndefined(); + expect(deps.kill).not.toHaveBeenCalled(); + }); + it('reaps an abandoned pre-resource record after owner PID reuse', async () => { const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); const paths = runPaths('stale-run'); @@ -168,10 +314,18 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { }); it('atomically takes over and removes a stale cleanup claim', async () => { - const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); const paths = runPaths('stale-claim'); - await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); const recordPath = path.join(temporaryRoot, 'pending-cleanup', 'stale-claim.json'); + const claimedPath = `${recordPath}.lock-claimed-owner`; + const unlink: typeof fs.unlink = (async (filePath: PathLike) => { + if (String(filePath) === claimedPath) { + await fs.unlink(filePath); + throw Object.assign(new Error('claim already released'), { code: 'ENOENT' }); + } + return fs.unlink(filePath); + }) as typeof fs.unlink; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ unlink })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { owner: unknown }; await fs.writeFile(`${recordPath}.lock`, `${JSON.stringify(record.owner)}\n`, { mode: 0o600 }); ownerStartTime = '2000'; @@ -232,9 +386,12 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { }); it('revalidates and unmounts recorded virtiofs bind mounts deepest-first', async () => { - const run = jest.fn(async (command: string) => { + const run = jest.fn(async (command: string, args: readonly string[]) => { if (command === '/usr/bin/umount') { - mountInfo = ''; + mountInfo = mountInfo + .split('\n') + .filter((line) => line && !line.includes(` ${args[0]} `)) + .join('\n'); return { exitCode: 0, stdout: '', stderr: '' }; } return { exitCode: 1, stdout: '', stderr: 'Device does not exist' }; @@ -242,18 +399,731 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ run })); const paths = runPaths('mounted-run'); const mountPoint = path.join(paths.virtiofsdShareDirectory, '0-workspace'); + const nestedMountPoint = path.join(mountPoint, 'nested'); + await fs.mkdir(nestedMountPoint, { recursive: true }); + mountInfo = [ + `123 1 8:1 /source ${mountPoint} rw - ext4 /dev/sda1 rw`, + `124 123 8:1 /source/nested ${nestedMountPoint} rw - ext4 /dev/sda1 rw`, + '', + ].join('\n'); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.captureVirtiofsdResources(); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + expect(run.mock.calls.filter(([command]) => command === '/usr/bin/umount')).toEqual([ + ['/usr/bin/umount', [nestedMountPoint]], + ['/usr/bin/umount', [mountPoint]], + ]); + await expect(fs.access(paths.virtiofsdShareDirectory)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('captures and identity-validates every live resource during stale-run recovery', async () => { + const paths = runPaths('full-recovery'); + const plan = networkPlan(paths.runId); + const netnsIdentityFile = path.join(temporaryRoot, 'netns-identity'); + await fs.writeFile(netnsIdentityFile, ''); + await fs.mkdir(paths.runDirectory, { recursive: true }); + await fs.mkdir(paths.cgroupPath, { recursive: true }); + await fs.mkdir(paths.virtiofsdShareDirectory, { recursive: true }); + const netnsStat = await fs.lstat(netnsIdentityFile, { bigint: true }); + daemonNamespace = `net:[${netnsStat.ino}]`; + let netnsExists = true; + let firewallRuleExists = true; + const interfaces = new Map([ + [plan.hostVethName, 101], + [plan.namespaceVethName, 102], + [plan.tapName, 103], + ]); + const base = dependencies(); + const baseLstat = base.lstat as typeof fs.lstat; + const lstat: typeof fs.lstat = (async (filePath: PathLike, options?: unknown) => { + if (String(filePath) === plan.netnsPath) { + if (!netnsExists) throw Object.assign(new Error('gone'), { code: 'ENOENT' }); + return fs.lstat(netnsIdentityFile, options as never); + } + return baseLstat(filePath, options as never); + }) as typeof fs.lstat; + const run = jest.fn(async (command: string, args: readonly string[]) => { + if (command === '/usr/bin/ip' && args.includes('-json')) { + const name = args[args.length - 1]; + const ifindex = interfaces.get(name); + return ifindex === undefined + ? { exitCode: 1, stdout: '', stderr: 'Device does not exist' } + : { exitCode: 0, stdout: JSON.stringify([{ ifname: name, ifindex }]), stderr: '' }; + } + if (command === '/usr/bin/ip' && args[0] === 'link' && args[1] === 'delete') { + interfaces.delete(args[2]); + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (command === '/usr/bin/ip' && args[0] === 'netns' && args[1] === 'delete') { + netnsExists = false; + interfaces.delete(plan.namespaceVethName); + interfaces.delete(plan.tapName); + return { exitCode: 0, stdout: '', stderr: '' }; + } + if (command === 'iptables' && args.includes('-C')) { + return { exitCode: firewallRuleExists ? 0 : 1, stdout: '', stderr: '' }; + } + if (command === 'iptables' && args.includes('-D')) { + firewallRuleExists = false; + return { exitCode: 0, stdout: '', stderr: '' }; + } + return { exitCode: 0, stdout: '', stderr: '' }; + }); + + const kill = jest.fn((_pid: number, signal?: NodeJS.Signals | number) => { + if (signal === 'SIGTERM') daemonAlive = false; + return true as const; + }); + const registry = new DurableCloudHypervisorCleanupRegistry( + dependencies({ lstat, run, kill }), + ); + const handle = await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + await handle.captureNetworkResource('netns'); + await handle.captureNetworkResource('hostVeth'); + await handle.captureNetworkResource('namespaceVeth'); + await handle.captureNetworkResource('tap'); + await handle.captureRunDirectory(); + await handle.captureCgroup(); + await handle.captureVirtiofsdResources(); + await handle.prepareProcess('vmm', process.execPath, '/sock'); + await handle.captureProcess('vmm', 5000); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + expect(kill).toHaveBeenCalledWith(5000, 'SIGTERM'); + expect(netnsExists).toBe(false); + expect(interfaces.size).toBe(0); + expect(firewallRuleExists).toBe(false); + await expect(fs.access(paths.runDirectory)).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(fs.access(paths.cgroupPath)).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(fs.access(paths.virtiofsdShareDirectory)).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(fs.access( + path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`), + )).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('escalates an identity-validated live process to SIGKILL', async () => { + const paths = runPaths('kill-escalation'); + const kill = jest.fn((_pid: number, signal?: NodeJS.Signals | number) => { + if (signal === 'SIGKILL') daemonAlive = false; + return true as const; + }); + const now = jest.spyOn(Date, 'now'); + let currentTime = 0; + now.mockImplementation(() => { + currentTime += 1_000; + return currentTime; + }); + try { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ kill })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + await handle.captureProcess('worker', 5000); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + expect(kill).toHaveBeenNthCalledWith(1, 5000, 'SIGTERM'); + expect(kill).toHaveBeenNthCalledWith(2, 5000, 'SIGKILL'); + } finally { + now.mockRestore(); + } + }); + + it('retains evidence when bridge-rule revalidation is uncertain', async () => { + const paths = runPaths('iptables-uncertain'); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ + run: jest.fn(async (command: string) => ( + command === 'iptables' + ? { exitCode: 2, stdout: '', stderr: 'xtables lock busy' } + : { exitCode: 1, stdout: '', stderr: 'Device does not exist' } + )), + })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /Could not revalidate per-run bridge rule: xtables lock busy/, + ); + }); + + it('retains evidence when an identity-validated network deletion command fails', async () => { + const paths = runPaths('network-delete-failure'); + const plan = networkPlan(paths.runId); + const run = jest.fn(async (_command: string, args: readonly string[]) => { + if (args.includes('-json')) { + return { + exitCode: 0, + stdout: JSON.stringify([{ ifname: plan.hostVethName, ifindex: 41 }]), + stderr: '', + }; + } + if (args[0] === 'link' && args[1] === 'delete') { + return { exitCode: 2, stdout: '', stderr: 'device busy' }; + } + return { exitCode: 0, stdout: '', stderr: '' }; + }); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ run })); + const handle = await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + await handle.captureNetworkResource('hostVeth'); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /link delete.*failed with code 2: device busy/, + ); + }); + + it('does not kill a PID whose committed command arguments no longer match', async () => { + const paths = runPaths('changed-process-args'); + const deps = dependencies(); + const registry = new DurableCloudHypervisorCleanupRegistry(deps); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock', '/source'); + await handle.captureProcess('worker', 5000); + daemonCmdline = `${process.execPath}\0--socket-path=/different\0`; + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + expect(deps.kill).not.toHaveBeenCalled(); + }); + + it('fails visibly when an identity-validated process survives SIGKILL', async () => { + const paths = runPaths('unkillable-process'); + const kill = jest.fn(() => true as const); + const now = jest.spyOn(Date, 'now'); + let currentTime = 0; + now.mockImplementation(() => { + currentTime += 1_000; + return currentTime; + }); + try { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ kill })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + await handle.captureProcess('worker', 5000); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /identity-validated process 5000 did not exit/, + ); + expect(kill).toHaveBeenCalledWith(5000, 'SIGKILL'); + } finally { + now.mockRestore(); + } + }); + + it('accepts an ESRCH race only after the recorded process disappears', async () => { + const paths = runPaths('esrch-exited'); + const kill = jest.fn(() => { + daemonAlive = false; + throw Object.assign(new Error('gone'), { code: 'ESRCH' }); + }); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ kill })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + await handle.captureProcess('worker', 5000); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).resolves.toBeUndefined(); + }); + + it('rejects ESRCH and other kill errors while process identity still matches', async () => { + for (const [runId, code, expected] of [ + ['esrch-live', 'ESRCH', /still matches after kill reported ESRCH/], + ['kill-denied', 'EPERM', /operation denied/], + ] as const) { + const kill = jest.fn(() => { + throw Object.assign(new Error('operation denied'), { code }); + }); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ kill })); + const paths = runPaths(runId); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + await handle.captureProcess('worker', 5000); + ownerStartTime = '2000'; + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow(expected); + ownerStartTime = '1000'; + } + }); + + it('accepts an ESRCH race after SIGTERM timeout only when SIGKILL sees process exit', async () => { + const paths = runPaths('sigkill-esrch'); + const kill = jest.fn((_pid: number, signal?: NodeJS.Signals | number) => { + if (signal === 'SIGKILL') { + daemonAlive = false; + throw Object.assign(new Error('gone'), { code: 'ESRCH' }); + } + return true as const; + }); + const now = jest.spyOn(Date, 'now'); + let currentTime = 0; + now.mockImplementation(() => { + currentTime += 1_000; + return currentTime; + }); + try { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ kill })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + await handle.captureProcess('worker', 5000); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).resolves.toBeUndefined(); + expect(kill).toHaveBeenCalledWith(5000, 'SIGKILL'); + } finally { + now.mockRestore(); + } + }); + + it('rejects SIGKILL ESRCH while the recorded process still matches', async () => { + const paths = runPaths('sigkill-esrch-live'); + const kill = jest.fn((_pid: number, signal?: NodeJS.Signals | number) => { + if (signal === 'SIGKILL') throw Object.assign(new Error('gone'), { code: 'ESRCH' }); + return true as const; + }); + const now = jest.spyOn(Date, 'now'); + let currentTime = 0; + now.mockImplementation(() => { + currentTime += 1_000; + return currentTime; + }); + try { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ kill })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.prepareProcess('worker', process.execPath, '/sock'); + await handle.captureProcess('worker', 5000); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /still matches after kill reported ESRCH/, + ); + } finally { + now.mockRestore(); + } + }); + + it('retains evidence when a recorded mount identity changes', async () => { + const paths = runPaths('changed-mount'); + const mountPoint = path.join(paths.virtiofsdShareDirectory, 'workspace'); await fs.mkdir(mountPoint, { recursive: true }); mountInfo = `123 1 8:1 /source ${mountPoint} rw - ext4 /dev/sda1 rw\n`; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); const handle = await registry.create( paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', ); await handle.captureVirtiofsdResources(); + mountInfo = `124 1 8:1 /source ${mountPoint} rw - ext4 /dev/sda1 rw\n`; + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /mount identity changed/, + ); + }); + + it('abandons takeover if the existing claim lock inode changes', async () => { + const paths = runPaths('claim-inode-race'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`); + const lockPath = `${recordPath}.lock`; + const base = dependencies(); + const baseLstat = base.lstat as typeof fs.lstat; + let lockStats = 0; + const lstat: typeof fs.lstat = (async (filePath: PathLike, options?: unknown) => { + const value = await baseLstat(filePath, options as never); + if ( + String(filePath) === lockPath && + (options as { bigint?: boolean } | undefined)?.bigint && + ++lockStats === 2 + ) { + return Object.assign(value, { ino: BigInt(value.ino) + 1n }); + } + return value; + }) as typeof fs.lstat; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ lstat })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { owner: unknown }; + await fs.writeFile(lockPath, JSON.stringify(record.owner), { mode: 0o600 }); ownerStartTime = '2000'; await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); - expect(run).toHaveBeenCalledWith('/usr/bin/umount', [mountPoint]); - await expect(fs.access(paths.virtiofsdShareDirectory)).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(fs.access(recordPath)).resolves.toBeUndefined(); + }); + + it('fails closed on a corrupted renamed cleanup claim', async () => { + const paths = runPaths('corrupt-renamed-claim'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + await fs.writeFile(`${recordPath}.lock-claimed-corrupt`, '{', { mode: 0o600 }); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /cleanup claim is unreadable/, + ); + }); + + it('surfaces failure to release a newly acquired cleanup claim', async () => { + const paths = runPaths('claim-release-failure'); + const lockPath = path.join( + temporaryRoot, + 'pending-cleanup', + `${paths.runId}.json.lock`, + ); + const unlink: typeof fs.unlink = (async (filePath: PathLike) => { + if (String(filePath) === lockPath) { + throw Object.assign(new Error('claim release denied'), { code: 'EPERM' }); + } + return fs.unlink(filePath); + }) as typeof fs.unlink; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ unlink })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /claim release denied/, + ); + }); + + it('surfaces failure to discard a stale renamed cleanup claim', async () => { + const paths = runPaths('stale-renamed-release'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`); + const claimPath = `${recordPath}.lock-claimed-stale`; + const unlink: typeof fs.unlink = (async (filePath: PathLike) => { + if (String(filePath) === claimPath) { + throw Object.assign(new Error('stale claim removal denied'), { code: 'EPERM' }); + } + return fs.unlink(filePath); + }) as typeof fs.unlink; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ unlink })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { + owner: Record; + }; + await fs.writeFile(claimPath, JSON.stringify({ + ...record.owner, + pid: 9999, + startTime: '9999', + }), { mode: 0o600 }); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /stale claim removal denied/, + ); + }); + + it('refuses recursive deletion when an unrecorded mount appears', async () => { + const paths = runPaths('late-mount'); + await fs.mkdir(paths.virtiofsdShareDirectory, { recursive: true }); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.captureVirtiofsdResources(); + mountInfo = `123 1 8:1 / ${paths.virtiofsdShareDirectory} rw - ext4 /dev/sda1 rw\n`; + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /refusing recursive removal while mounts remain/, + ); + }); + + it('fails when the kernel does not release a cgroup before the retry deadline', async () => { + const paths = runPaths('cgroup-timeout'); + await fs.mkdir(paths.cgroupPath, { recursive: true }); + const rmdir = jest.fn(async () => { + throw Object.assign(new Error('busy'), { code: 'EBUSY' }); + }) as typeof fs.rmdir; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ rmdir })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.captureCgroup(); + ownerStartTime = '2000'; + const now = jest.spyOn(Date, 'now') + .mockReturnValueOnce(0) + .mockReturnValueOnce(6_000); + try { + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow('busy'); + } finally { + now.mockRestore(); + } + }); + + it.each([ + ['non-array output', '{}', /Unexpected interface inspection/], + ['wrong interface', '[{"ifname":"other","ifindex":12}]', /Invalid interface inspection/], + ['non-integer index', '[{"ifname":"host","ifindex":"12"}]', /Invalid interface inspection/], + ])('rejects %s from kernel interface inspection', async (_label, stdout, expected) => { + const paths = runPaths('bad-interface'); + const plan = networkPlan(paths.runId); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ + run: jest.fn(async () => ({ exitCode: 0, stdout, stderr: '' })), + })); + const handle = await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + + await expect(handle.captureNetworkResource('hostVeth')).rejects.toThrow(expected); + }); + + it('propagates kernel interface inspection failures', async () => { + const paths = runPaths('interface-inspection-error'); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ + run: jest.fn(async () => ({ exitCode: 2, stdout: '', stderr: 'netlink denied' })), + })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + + await expect(handle.captureNetworkResource('hostVeth')).rejects.toThrow(/netlink denied/); + }); + + it('fails closed when process or resource identity cannot be read', async () => { + const paths = runPaths('identity-unreadable'); + const plan = networkPlan(paths.runId); + const base = dependencies(); + const baseReadFile = base.readFile as typeof fs.readFile; + const readFile: typeof fs.readFile = (async (filePath: PathLike, options?: unknown) => { + if (String(filePath) === '/proc/4242/stat' && ownerStartTime === '2000') { + throw Object.assign(new Error('proc denied'), { code: 'EACCES' }); + } + return baseReadFile(filePath, options as never); + }) as typeof fs.readFile; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ readFile })); + await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + ownerStartTime = '2000'; + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /proc denied/, + ); + + ownerStartTime = '1000'; + const secondPaths = runPaths('resource-unreadable'); + const resourcePlan = networkPlan(secondPaths.runId); + const inaccessibleLstat: typeof fs.lstat = (async ( + filePath: PathLike, + options?: unknown, + ) => { + if (String(filePath) === resourcePlan.netnsPath) { + throw Object.assign(new Error('netns denied'), { code: 'EACCES' }); + } + return (base.lstat as typeof fs.lstat)(filePath, options as never); + }) as typeof fs.lstat; + const second = new DurableCloudHypervisorCleanupRegistry( + dependencies({ lstat: inaccessibleLstat }), + ); + await second.create( + secondPaths, resourcePlan, process.execPath, '/usr/bin/ip', + ); + ownerStartTime = '2000'; + await expect(second.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /netns denied/, + ); + }); + + it('rejects malformed mountinfo and decodes valid escaped mount paths', async () => { + const basePaths = runPaths('mount-parser'); + const paths = { + ...basePaths, + virtiofsdShareDirectory: path.join(temporaryRoot, 'virtiofsd with space', basePaths.runId), + }; + await fs.mkdir(paths.virtiofsdShareDirectory, { recursive: true }); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + mountInfo = 'not mountinfo\n'; + await expect(handle.captureVirtiofsdResources()).rejects.toThrow( + /Malformed \/proc\/self\/mountinfo entry/, + ); + + const escaped = paths.virtiofsdShareDirectory.replace(/ /g, '\\040'); + mountInfo = `123 1 8:1 /source ${escaped} rw - ext4 /dev/sda1 rw\n`; + await expect(handle.captureVirtiofsdResources()).resolves.toBeUndefined(); + }); + + it('rejects unsafe registry and record permissions', async () => { + const unsafeRoot = path.join(temporaryRoot, 'unsafe'); + const unsafeRegistry = path.join(unsafeRoot, 'pending-cleanup'); + await fs.mkdir(unsafeRegistry, { recursive: true, mode: 0o755 }); + await expect(new DurableCloudHypervisorCleanupRegistry( + dependencies({ rootDirectory: unsafeRoot }), + ).reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /unsafe ownership or mode/, + ); + + const paths = runPaths('unsafe-record'); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + await fs.chmod(path.join(temporaryRoot, 'pending-cleanup', 'unsafe-record.json'), 0o644); + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /not a root-owned mode-0600 regular file/, + ); + }); + + it('honors active renamed claims and removes stale renamed claims', async () => { + const paths = runPaths('renamed-claim'); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', 'renamed-claim.json'); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { + owner: Record; + }; + const claimPath = `${recordPath}.lock-claimed-active`; + await fs.writeFile(claimPath, JSON.stringify({ + ...record.owner, + pid: 5000, + startTime: '5000', + networkNamespace: daemonNamespace, + }), { mode: 0o600 }); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + await expect(fs.access(recordPath)).resolves.toBeUndefined(); + + daemonAlive = false; + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + await expect(fs.access(recordPath)).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(fs.access(claimPath)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('rejects unsafe or unreadable cleanup claims', async () => { + for (const [runId, contents, mode, expected] of [ + ['unsafe-claim', '{}', 0o644, /claim has unsafe ownership or mode/], + ['unreadable-claim', '{', 0o600, /claim is unreadable/], + ] as const) { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const paths = runPaths(runId); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + await fs.writeFile( + path.join(temporaryRoot, 'pending-cleanup', `${runId}.json.lock`), + contents, + { mode }, + ); + ownerStartTime = '2000'; + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow(expected); + ownerStartTime = '1000'; + } + }); + + it('leaves a stale record untouched when another reaper wins the takeover marker', async () => { + const paths = runPaths('claim-takeover-race'); + const link: typeof fs.link = (async (source: PathLike, destination: PathLike) => { + if (String(destination).endsWith('.lock-claimed-owner')) { + throw Object.assign(new Error('claimed'), { code: 'EEXIST' }); + } + return fs.link(source, destination); + }) as typeof fs.link; + const unlink: typeof fs.unlink = (async (filePath: PathLike) => { + if (String(filePath).includes('.lock-claimed-owner.tmp-')) { + throw Object.assign(new Error('temporary marker gone'), { code: 'ENOENT' }); + } + return fs.unlink(filePath); + }) as typeof fs.unlink; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ link, unlink })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { owner: unknown }; + await fs.writeFile(`${recordPath}.lock`, JSON.stringify(record.owner), { mode: 0o600 }); + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + await expect(fs.access(recordPath)).resolves.toBeUndefined(); + }); + + it('runs identity-validated deletion through the default argv-only executor', async () => { + const paths = runPaths('default-executor'); + const plan = networkPlan(paths.runId); + const executable = path.join(temporaryRoot, 'fake-ip.js'); + await fs.writeFile(executable, [ + '#!/bin/sh', + 'for name do :; done', + 'case " $* " in *" -json "*) printf \'[{"ifname":"%s","ifindex":42}]\' "$name";; esac', + '', + ].join('\n'), { mode: 0o700 }); + const base = dependencies(); + const registry = new DurableCloudHypervisorCleanupRegistry({ + ...base, + run: undefined, + }); + const handle = await registry.create(paths, plan, process.execPath, executable); + await handle.captureNetworkResource('hostVeth'); + ownerStartTime = '2000'; + + await expect(registry.reapPending(executable, '/usr/bin/umount')).rejects.toThrow( + /stale cleanup is incomplete/, + ); + }); + + it('rejects cross-run setup and unstable process credentials', async () => { + const paths = runPaths('scoped-run'); + await expect(new DurableCloudHypervisorCleanupRegistry(dependencies()).create( + paths, networkPlan('different-run'), process.execPath, '/usr/bin/ip', + )).rejects.toThrow(/resources are not scoped to one run/); + + const unstable = dependencies({ + readFile: (async (filePath: PathLike, options?: unknown) => { + if (String(filePath) === '/proc/4242/stat') return procStat(4242, ownerStartTime); + if (String(filePath) === '/proc/4242/status') { + return 'Uid:\t0\t1\t0\t0\nGid:\t0\t0\t0\t0\n'; + } + return fs.readFile(filePath, options as never); + }) as typeof fs.readFile, + }); + await expect(new DurableCloudHypervisorCleanupRegistry(unstable).create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + )).rejects.toThrow(/Process Uid identities are not stable/); + }); + + it('rejects malformed or cross-run recovery evidence', async () => { + const cases: Array<[string, (record: Record) => void, RegExp]> = [ + ['bad-version', (record) => { record.version = 2; }, /invalid cleanup record identity/], + ['bad-run-path', (record) => { record.paths.runDirectory = '/tmp/other'; }, /not run-scoped/], + ['bad-owner', (record) => { record.owner.pid = 1; }, /owner identity is malformed/], + ['bad-processes', (record) => { record.processes = []; }, /resource identities are malformed/], + ['bad-process', (record) => { + record.processes.worker = { state: 'pending', executable: 'relative', socketPath: '/sock' }; + }, /process record is malformed/], + ['bad-mount', (record) => { + record.mounts = [{ + mountId: 0, + device: '8:1', + root: '/', + mountPoint: record.paths.virtiofsdShareDirectory, + filesystemType: 'ext4', + source: '/dev/sda1', + }]; + }, /mount identity is malformed/], + ]; + + for (const [runId, mutate, expected] of cases) { + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies()); + const paths = runPaths(runId); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + await mutateRecord(runId, mutate); + ownerStartTime = '2000'; + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow(expected); + ownerStartTime = '1000'; + } }); it('retries inode-validated cgroup removal while kernel accounting drains', async () => { @@ -277,4 +1147,118 @@ describe('DurableCloudHypervisorCleanupRegistry', () => { expect(rmdir).toHaveBeenCalledTimes(2); await expect(fs.access(paths.cgroupPath)).rejects.toMatchObject({ code: 'ENOENT' }); }); + + it('fails if a cgroup inode changes while kernel accounting drains', async () => { + const paths = runPaths('changed-cgroup'); + await fs.mkdir(paths.cgroupPath, { recursive: true }); + let cgroupStats = 0; + const base = dependencies(); + const baseLstat = base.lstat as typeof fs.lstat; + const lstat: typeof fs.lstat = (async (filePath: PathLike, options?: unknown) => { + const value = await baseLstat(filePath, options as never); + if ( + String(filePath) === paths.cgroupPath && + (options as { bigint?: boolean } | undefined)?.bigint && + ++cgroupStats >= 4 + ) { + return Object.assign(value, { ino: BigInt(value.ino) + 1n }); + } + return value; + }) as typeof fs.lstat; + const rmdir = jest.fn(async () => { + throw Object.assign(new Error('busy'), { code: 'EBUSY' }); + }) as typeof fs.rmdir; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ lstat, rmdir })); + const handle = await registry.create( + paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip', + ); + await handle.captureCgroup(); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /identity changed during cgroup drain/, + ); + }); + + it('fails when an interface is replaced after its identity is committed', async () => { + const paths = runPaths('changed-interface'); + const plan = networkPlan(paths.runId); + let ifindex = 41; + const run = jest.fn(async (_command: string, args: readonly string[]) => ( + args.includes('-json') + ? { + exitCode: 0, + stdout: JSON.stringify([{ ifname: plan.hostVethName, ifindex }]), + stderr: '', + } + : { exitCode: 0, stdout: '', stderr: '' } + )); + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ run })); + const handle = await registry.create(paths, plan, process.execPath, '/usr/bin/ip'); + await handle.captureNetworkResource('hostVeth'); + ifindex = 42; + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /interface ".*" identity changed/, + ); + }); + + it('backs off when a live renamed claimant appears immediately after lock acquisition', async () => { + const paths = runPaths('late-renamed-claim'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`); + const claim = { owner: undefined as Record | undefined }; + const link: typeof fs.link = (async (source: PathLike, destination: PathLike) => { + await fs.link(source, destination); + if (String(destination) === `${recordPath}.lock` && claim.owner) { + await fs.writeFile( + `${recordPath}.lock-claimed-racer`, + JSON.stringify(claim.owner), + { mode: 0o600 }, + ); + } + }) as typeof fs.link; + const unlink: typeof fs.unlink = (async (filePath: PathLike) => { + if (String(filePath) === `${recordPath}.lock`) { + await fs.unlink(filePath); + throw Object.assign(new Error('lock already removed'), { code: 'ENOENT' }); + } + return fs.unlink(filePath); + }) as typeof fs.unlink; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ link, unlink })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + const record = JSON.parse(await fs.readFile(recordPath, 'utf8')) as { + owner: Record; + }; + claim.owner = { + ...record.owner, + pid: 5000, + startTime: '5000', + networkNamespace: daemonNamespace, + }; + ownerStartTime = '2000'; + + await registry.reapPending('/usr/bin/ip', '/usr/bin/umount'); + + await expect(fs.access(recordPath)).resolves.toBeUndefined(); + await expect(fs.access(`${recordPath}.lock`)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('fails visibly after a contended claim lock repeatedly vanishes', async () => { + const paths = runPaths('vanishing-claim'); + const recordPath = path.join(temporaryRoot, 'pending-cleanup', `${paths.runId}.json`); + const link: typeof fs.link = (async (source: PathLike, destination: PathLike) => { + if (String(destination) === `${recordPath}.lock`) { + throw Object.assign(new Error('contended'), { code: 'EEXIST' }); + } + return fs.link(source, destination); + }) as typeof fs.link; + const registry = new DurableCloudHypervisorCleanupRegistry(dependencies({ link })); + await registry.create(paths, networkPlan(paths.runId), process.execPath, '/usr/bin/ip'); + ownerStartTime = '2000'; + + await expect(registry.reapPending('/usr/bin/ip', '/usr/bin/umount')).rejects.toThrow( + /could not atomically claim stale cleanup record/, + ); + }); }); diff --git a/src/cloud-hypervisor/cleanup-registry.ts b/src/cloud-hypervisor/cleanup-registry.ts index 1d4fcc458..a86761a20 100644 --- a/src/cloud-hypervisor/cleanup-registry.ts +++ b/src/cloud-hypervisor/cleanup-registry.ts @@ -571,12 +571,14 @@ export class DurableCloudHypervisorCleanupRegistry implements CloudHypervisorCle const status = await this.dependencies.readFile(`/proc/${pid}/status`, 'utf8'); const uid = parseStatusIdentity(status, 'Uid'); const gid = parseStatusIdentity(status, 'Gid'); - const executable = await this.dependencies.realpath(`/proc/${pid}/exe`); + const executableLink = `/proc/${pid}/exe`; + const executable = (await this.dependencies.readlink(executableLink)) + .replace(/ \(deleted\)$/, ''); return { pid, startTime, executable, - executableIdentity: await this.captureFileIdentity(executable), + executableIdentity: await this.captureFollowedFileIdentity(executableLink), uid, gid, networkNamespace: await this.dependencies.readlink(`/proc/${pid}/ns/net`), @@ -616,6 +618,11 @@ export class DurableCloudHypervisorCleanupRegistry implements CloudHypervisorCle return { device: value.dev.toString(), inode: value.ino.toString() }; } + private async captureFollowedFileIdentity(filePath: string): Promise { + const value = await this.dependencies.stat(filePath, { bigint: true }); + return { device: value.dev.toString(), inode: value.ino.toString() }; + } + private async captureInterfaceIdentity( ipPath: string, name: string, diff --git a/src/microvm/network-commands.ts b/src/microvm/network-commands.ts index 08ad9384b..2dd6addda 100644 --- a/src/microvm/network-commands.ts +++ b/src/microvm/network-commands.ts @@ -155,26 +155,38 @@ export class LinuxNetworkCommands { ); } - /** Removes the rule `ensureBridgeForwardAcceptRule` installs, if present. Tolerant of it already being gone or the underlying command failing outright. */ + /** Removes the rule `ensureBridgeForwardAcceptRule` installs and verifies it is absent. */ async removeBridgeForwardAcceptRule(bridgeName: string, comment: string): Promise { assertIptablesComment(comment); - try { - await this.execute( - 'iptables', - [ - '-t', 'filter', '-D', 'DOCKER-USER', - '-i', bridgeName, '-o', bridgeName, - '-m', 'comment', '--comment', comment, - '-j', 'ACCEPT', - ], - { reject: false }, - ); - } catch { - // Best-effort cleanup: an already-removed rule, or the iptables - // binary itself being unavailable, must not fail the caller's own - // cleanup sequence. + const rule = [ + '-t', 'filter', '-C', 'DOCKER-USER', + '-i', bridgeName, '-o', bridgeName, + '-m', 'comment', '--comment', comment, + '-j', 'ACCEPT', + ]; + await this.execute( + 'iptables', + rule.map((argument) => argument === '-C' ? '-D' : argument), + { reject: false }, + ); + const checkResult = await this.execute('iptables', rule, { reject: false }); + const exitCode = (checkResult as { exitCode?: number } | undefined)?.exitCode; + const stderr = String( + (checkResult as { stderr?: unknown } | undefined)?.stderr ?? '', + ).trim(); + if ( + exitCode === 1 && + (stderr === '' || /Bad rule \(does a matching rule exist in that chain\?\)/i.test(stderr)) + ) { + return; } - + if (exitCode === 0) { + throw new Error(`iptables rule still exists after deletion: ${comment}`); + } + throw new Error( + `could not verify iptables rule deletion for ${comment}: ` + + `exit ${String(exitCode)}${stderr ? `: ${stderr}` : ''}`, + ); } /** diff --git a/src/microvm/network.test.ts b/src/microvm/network.test.ts index 3a703c99d..0731b5531 100644 --- a/src/microvm/network.test.ts +++ b/src/microvm/network.test.ts @@ -46,6 +46,7 @@ function commandHarness(failAt?: number): { if (options.reject && ++rejectingCall === failAt) { throw new Error(`stage ${failAt} failed`); } + return { exitCode: args.includes('-C') ? 1 : 0, stderr: '' }; }), ); return { calls, commands }; @@ -633,10 +634,14 @@ describe('LinuxNetworkCommands.ensureBridgeForwardAcceptRule / removeBridgeForwa ]); }); - it('removeBridgeForwardAcceptRule never throws even if the rule is already gone', async () => { + it('accepts an explicitly verified already-absent bridge rule', async () => { + const calls: string[][] = []; const commands = new LinuxNetworkCommands( - jest.fn(async () => { - throw new Error('Bad rule (does a matching rule exist in that chain?)'); + jest.fn(async (_command, args) => { + calls.push([...args]); + return args.includes('-C') + ? { exitCode: 1, stderr: '' } + : { exitCode: 1, stderr: 'Bad rule (does a matching rule exist in that chain?)' }; }), ); @@ -644,6 +649,31 @@ describe('LinuxNetworkCommands.ensureBridgeForwardAcceptRule / removeBridgeForwa 'awfbr0', 'awf:awf_vm_0123456789ab', )).resolves.toBeUndefined(); + expect(calls).toHaveLength(2); + }); + + it('fails when a bridge rule remains after deletion', async () => { + const commands = new LinuxNetworkCommands( + jest.fn(async (_command, args) => ( + args.includes('-C') ? { exitCode: 0 } : { exitCode: 2, stderr: 'xtables lock busy' } + )), + ); + + await expect(commands.removeBridgeForwardAcceptRule( + 'awfbr0', + 'awf:awf_vm_0123456789ab', + )).rejects.toThrow(/rule still exists after deletion/); + }); + + it('propagates uncertainty when bridge-rule absence cannot be verified', async () => { + const commands = new LinuxNetworkCommands( + jest.fn(async () => ({ exitCode: 2, stderr: 'xtables lock busy' })), + ); + + await expect(commands.removeBridgeForwardAcceptRule( + 'awfbr0', + 'awf:awf_vm_0123456789ab', + )).rejects.toThrow(/could not verify iptables rule deletion.*xtables lock busy/); }); }); From 939f0eeb5857b785e1ed00fd3a0ea15fc232ea9b Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 09:30:32 -0700 Subject: [PATCH 3/6] test: lock cgroup assignment ordering Ensure VMM cgroup assignment precedes durable process identity capture. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 92d2c346-d396-40d3-8b74-1f1447a04871 --- src/cloud-hypervisor/manager.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cloud-hypervisor/manager.test.ts b/src/cloud-hypervisor/manager.test.ts index c0771433e..1943a7ef9 100644 --- a/src/cloud-hypervisor/manager.test.ts +++ b/src/cloud-hypervisor/manager.test.ts @@ -352,6 +352,10 @@ describe('CloudHypervisorManager', () => { const cgroup = (deps.createCgroup as jest.Mock).mock.results[0].value as CloudHypervisorCgroup; expect(cgroup.setup).toHaveBeenCalledTimes(1); expect(cgroup.assign).toHaveBeenCalledWith(4242); + const cleanupRecord = await (deps.cleanupRegistry.create as jest.Mock).mock.results[0].value as + CloudHypervisorCleanupHandle; + expect((cgroup.assign as jest.Mock).mock.invocationCallOrder[0]) + .toBeLessThan((cleanupRecord.captureProcess as jest.Mock).mock.invocationCallOrder[0]); expect(deps.verifyConfinement).toHaveBeenCalledWith(expect.objectContaining({ pid: 4242, expectedExecutable: '/opt/cloud-hypervisor', From 24d95bb6570e06f28902bec6c58db84071990a76 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 10:19:49 -0700 Subject: [PATCH 4/6] test: count attestation tool lookup Count gh alongside Docker and the eleven Cloud Hypervisor runtime tools. This keeps preflight assertions aligned with manifest attestation verification. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 92d2c346-d396-40d3-8b74-1f1447a04871 --- src/cloud-hypervisor/preflight.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cloud-hypervisor/preflight.test.ts b/src/cloud-hypervisor/preflight.test.ts index 3f77e758a..6b28cfa34 100644 --- a/src/cloud-hypervisor/preflight.test.ts +++ b/src/cloud-hypervisor/preflight.test.ts @@ -268,7 +268,8 @@ describe('Cloud Hypervisor preflight (foundation only)', () => { constants.R_OK | constants.W_OK, ); expect(deps.sha256).toHaveBeenCalledTimes(5); - expect(deps.assertToolAvailable).toHaveBeenCalledTimes(12); + // Docker, the eleven runtime host tools, and gh for attestation verification. + expect(deps.assertToolAvailable).toHaveBeenCalledTimes(13); expect(deps.assertDockerInfrastructure).toHaveBeenCalledWith('/usr/bin/docker'); expect(deps.verifyManifestAttestation).toHaveBeenCalledWith( '/usr/bin/gh', From 7fbc4cfc7a78368d8e8778a7aa3d949885d2c19f Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 10:36:23 -0700 Subject: [PATCH 5/6] fix: reuse verified microVM artifacts Reuse the backend-owned attested snapshot during workflow startup and every boot retry. This prevents duplicate rootfs copies from exhausting the runner's /run filesystem. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 92d2c346-d396-40d3-8b74-1f1447a04871 --- src/cloud-hypervisor-runtime-backend.test.ts | 27 +++++++++++++++++++- src/cloud-hypervisor-runtime-backend.ts | 9 +++++-- src/cloud-hypervisor/manager.test.ts | 25 ++++++++++++++++++ src/cloud-hypervisor/manager.ts | 7 ++++- 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/cloud-hypervisor-runtime-backend.test.ts b/src/cloud-hypervisor-runtime-backend.test.ts index c99903d5c..b4c652c0c 100644 --- a/src/cloud-hypervisor-runtime-backend.test.ts +++ b/src/cloud-hypervisor-runtime-backend.test.ts @@ -243,6 +243,8 @@ describe('Cloud Hypervisor runtime backend', () => { infrastructure(), [{ tag: 'workspace', source: '/workspace', target: '/workspace', mode: 'rw' }], { uid: 1000, gid: 1000 }, + undefined, + preflightResult, )).toBeDefined(); expect(createCloudHypervisorRuntimeBackend(config(), startInfrastructure)) .toEqual(expect.objectContaining({ runtime: 'cloud-hypervisor' })); @@ -302,6 +304,7 @@ describe('Cloud Hypervisor runtime backend', () => { }, ], }, + preflightResult, ); expect(deps.logger.info).toHaveBeenCalledWith( '[cloud-hypervisor] stage=filesystem-write-policy boundary /workspace=ro ' + @@ -325,7 +328,8 @@ describe('Cloud Hypervisor runtime backend', () => { { tag: 'workspace', source: '/workspace-host', target: '/workspace', mode: 'rw' }, ]); expect(call[5]).toBeUndefined(); - expect(call).toHaveLength(6); + expect(call[6]).toBe(preflightResult); + expect(call).toHaveLength(7); expect(deps.logger.info).not.toHaveBeenCalledWith( expect.stringContaining('stage=filesystem-write-policy'), ); @@ -387,6 +391,27 @@ describe('Cloud Hypervisor runtime backend', () => { expect(manager.collectGuestOutputAudit).not.toHaveBeenCalled(); }); + it('reuses the CLI preflight snapshot when workflow startup begins', async () => { + const { deps } = harness(); + const backend = createBackend(config(), deps); + + await backend.preflight(); + await backend.start('/tmp/awf', ['github.com']); + await backend.stop(); + + expect(deps.preflight).toHaveBeenCalledTimes(1); + expect(deps.createManager).toHaveBeenCalledWith( + expect.anything(), + '/tmp/awf', + expect.anything(), + expect.anything(), + expect.anything(), + undefined, + preflightResult, + ); + expect(deps.removeArtifactSnapshot).toHaveBeenCalledWith('/snapshot'); + }); + it('persists bounded raw guest output when an audit directory is configured', async () => { const { manager, deps, stdin } = harness(); const backend = createBackend(config({ auditDir: '/tmp/audit' }), deps); diff --git a/src/cloud-hypervisor-runtime-backend.ts b/src/cloud-hypervisor-runtime-backend.ts index 6335a83bf..e01bf1696 100644 --- a/src/cloud-hypervisor-runtime-backend.ts +++ b/src/cloud-hypervisor-runtime-backend.ts @@ -114,6 +114,7 @@ export interface CloudHypervisorRuntimeBackendDependencies { exports: readonly CloudHypervisorDirectoryExport[], identity: { uid: number; gid: number }, mountEnforcement?: VirtiofsdMountEnforcement, + preflightResult?: CloudHypervisorPreflightResult, ): CloudHypervisorManagerAdapter; resolveExports(mountPolicy: CloudHypervisorOptions['mountPolicy']): Promise; identity(): { uid: number; gid: number }; @@ -133,7 +134,9 @@ function defaultDependencies( preflight: runCloudHypervisorPreflight, resolveInfrastructure: (enableApiProxy, ipPath, topologyPeerNames) => resolveMicrovmInfrastructure(enableApiProxy, undefined, ipPath, topologyPeerNames), - createManager: (config, workDir, infrastructure, exports, identity, mountEnforcement) => + createManager: ( + config, workDir, infrastructure, exports, identity, mountEnforcement, preflightResult, + ) => new CloudHypervisorManager( config, workDir, @@ -156,6 +159,7 @@ function defaultDependencies( supervisorSha256: config.sha256!.supervisor!, identity, }, + preflightResult, ), resolveExports: (mountPolicy) => resolveCloudHypervisorExports( process.env, @@ -248,7 +252,7 @@ class CloudHypervisorRuntimeBackend implements ExternalAgentRuntimeBackend { '[cloud-hypervisor] runtime=cloud-hypervisor maturity=preview fallback=disabled', ); try { - await this.preflight(); + if (!this.preflightResult) await this.preflight(); stage = 'compose-infrastructure'; await this.dependencies.startInfrastructure( workDir, @@ -316,6 +320,7 @@ class CloudHypervisorRuntimeBackend implements ExternalAgentRuntimeBackend { exports, this.identity, mountEnforcement, + this.preflightResult, ); try { stage = 'vmm-configuration'; diff --git a/src/cloud-hypervisor/manager.test.ts b/src/cloud-hypervisor/manager.test.ts index b160a3395..f0fc83924 100644 --- a/src/cloud-hypervisor/manager.test.ts +++ b/src/cloud-hypervisor/manager.test.ts @@ -417,6 +417,31 @@ describe('CloudHypervisorManager', () => { expect(lifecycle.setup).toHaveBeenCalledTimes(1); }); + it('reuses a verified artifact snapshot instead of running preflight again', async () => { + const deps = dependencies(); + const verified = await deps.preflight(config()); + (deps.preflight as jest.Mock).mockReset().mockRejectedValue( + new Error('preflight must not rerun'), + ); + const manager = new CloudHypervisorManager( + config(), + '/tmp/awf', + deps, + 'verified-snapshot', + networkConfig(), + undefined, + verified, + ); + + await expect(manager.start()).resolves.toBeDefined(); + expect(deps.preflight).not.toHaveBeenCalled(); + expect(deps.copyFile).toHaveBeenCalledWith( + verified.rootfsPath, + expect.stringContaining('/rootfs.ext4'), + constants.COPYFILE_EXCL, + ); + }); + it('terminates the partial process and removes its run directory on readiness failure', async () => { const child = processMock(); const missing = Object.assign(new Error('missing'), { code: 'ENOENT' }); diff --git a/src/cloud-hypervisor/manager.ts b/src/cloud-hypervisor/manager.ts index 2336a55f7..6e2040bcd 100644 --- a/src/cloud-hypervisor/manager.ts +++ b/src/cloud-hypervisor/manager.ts @@ -46,6 +46,7 @@ import { } from './manager-types'; import { runCloudHypervisorPreflight } from './preflight'; import type { CloudHypervisorHostToolPaths } from './preflight'; +import type { CloudHypervisorPreflightResult } from './preflight'; import { verifyCloudHypervisorConfinement, type CloudHypervisorConfinementEvidence, @@ -222,15 +223,19 @@ export class CloudHypervisorManager { runId?: string, private readonly networkConfig?: CloudHypervisorManagerNetworkConfig, private readonly guestConfig?: CloudHypervisorManagerGuestConfig, + private readonly preflightResult?: CloudHypervisorPreflightResult, ) { this.paths = createCloudHypervisorRunPaths(config.cloudHypervisorBinary, runId); } async start(): Promise { + const dependencies = this.preflightResult + ? { ...this.dependencies, preflight: async () => this.preflightResult! } + : this.dependencies; return startCloudHypervisor({ config: this.config, workDir: this.workDir, - dependencies: this.dependencies, + dependencies, paths: this.paths, networkConfig: this.networkConfig, guestConfig: this.guestConfig, From 7b3c296f308ef896ac23bf6192cbf3844818d67d Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 31 Aug 2026 10:51:19 -0700 Subject: [PATCH 6/6] fix(cloud-hypervisor): require empty worker capability bound Match the live virtiofsd sandbox state while retaining exact effective and permitted capability checks. Reject capabilities reacquirable across exec. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 92d2c346-d396-40d3-8b74-1f1447a04871 --- docs/cloud-hypervisor-foundation.md | 4 ++-- src/cloud-hypervisor/virtiofsd-sandbox.ts | 10 ++-------- src/cloud-hypervisor/virtiofsd.test.ts | 6 +++--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/docs/cloud-hypervisor-foundation.md b/docs/cloud-hypervisor-foundation.md index dc91c4df8..a2a1b6b06 100644 --- a/docs/cloud-hypervisor-foundation.md +++ b/docs/cloud-hypervisor-foundation.md @@ -229,8 +229,8 @@ configuration, AWF verifies the live parent and worker through `/proc`: - parent and worker UIDs/GIDs match the reviewed root namespace identity; - every parent capability set is empty, while the worker effective and permitted masks equal the pinned minimal virtiofsd set, its inheritable and - ambient sets are empty, and its bounding set contains the capabilities - needed during sandbox setup (rendered non-acquirable after `NoNewPrivs`); + ambient sets are empty, and its bounding set is empty so those setup + capabilities cannot be reacquired across exec; - the worker has `NoNewPrivs: 1` and seccomp filter mode `2`; - the worker mount, PID, and network namespaces differ from the host; - the worker root inode is the inode of the declared export, proving the diff --git a/src/cloud-hypervisor/virtiofsd-sandbox.ts b/src/cloud-hypervisor/virtiofsd-sandbox.ts index ae0b60548..1dd0ae84a 100644 --- a/src/cloud-hypervisor/virtiofsd-sandbox.ts +++ b/src/cloud-hypervisor/virtiofsd-sandbox.ts @@ -3,9 +3,7 @@ import * as path from 'path'; const WORKER_READY_TIMEOUT_MS = 5_000; const WORKER_READY_INTERVAL_MS = 50; const REVIEWED_WORKER_CAPABILITIES = '00000000880000db'; -const REVIEWED_WORKER_CAPABILITY_BITS = BigInt(`0x${REVIEWED_WORKER_CAPABILITIES}`); const ZERO_CAPABILITIES = /^0+$/; -const CAPABILITY_MASK = /^[0-9a-f]{16}$/; const REQUIRED_NAMESPACES = ['mnt', 'pid', 'net'] as const; const CAPABILITY_FIELDS = ['CapInh', 'CapPrm', 'CapEff', 'CapBnd', 'CapAmb'] as const; const CGROUP_ROOT = '/sys/fs/cgroup'; @@ -143,12 +141,8 @@ export async function verifyVirtiofsdSandbox( throw new Error('virtiofsd worker capabilities differ from the reviewed sandbox set'); } const workerBounding = worker.capabilities.CapBnd ?? ''; - if ( - !CAPABILITY_MASK.test(workerBounding) || - (BigInt(`0x${workerBounding}`) & REVIEWED_WORKER_CAPABILITY_BITS) !== - REVIEWED_WORKER_CAPABILITY_BITS - ) { - throw new Error('virtiofsd worker bounding set excludes reviewed runtime capabilities'); + if (!ZERO_CAPABILITIES.test(workerBounding)) { + throw new Error('virtiofsd worker bounding capability set is not empty'); } if (worker.noNewPrivs !== 1 || worker.seccomp !== 2) { throw new Error('virtiofsd worker is missing NoNewPrivs or seccomp filtering'); diff --git a/src/cloud-hypervisor/virtiofsd.test.ts b/src/cloud-hypervisor/virtiofsd.test.ts index 46dd67aa6..1d3931e7d 100644 --- a/src/cloud-hypervisor/virtiofsd.test.ts +++ b/src/cloud-hypervisor/virtiofsd.test.ts @@ -75,7 +75,7 @@ function dependencies( `CapInh:\t${zero}`, `CapPrm:\t${isWorker ? reviewed : zero}`, `CapEff:\t${isWorker ? reviewed : zero}`, - `CapBnd:\t${isWorker ? reviewed : zero}`, + `CapBnd:\t${zero}`, `CapAmb:\t${zero}`, `NoNewPrivs:\t${isWorker ? 1 : 0}`, `Seccomp:\t${isWorker ? 2 : 0}`, @@ -365,11 +365,11 @@ describe('VirtiofsdManager', () => { deps.readFile = jest.fn(async (filePath: string, encoding: BufferEncoding) => { const contents = await readFile(filePath, encoding); return filePath === '/proc/1100/status' - ? contents.replace('CapBnd:\t00000000880000db', 'CapBnd:\t0000000000000000') + ? contents.replace('CapBnd:\t0000000000000000', 'CapBnd:\t00000000880000db') : contents; }); }, - error: /bounding set excludes reviewed runtime capabilities/, + error: /bounding capability set is not empty/, }, { name: 'worker NoNewPrivs',