fix: compose concurrent VMM device ACL cleanup

src/cloud-hypervisor/cleanup-handle.ts

@@ -105,6 +105,7 @@ export function createCleanupHandle(options: CleanupHandleFactoryOptions): Cloud
     prepareVmmAcl: async (
       aclPath: CloudHypervisorDevicePath,
       baseline: readonly string[],
+      temporaryUids: readonly number[] = [],
     ) => {
       if (!record.vmmIdentity || record.vmmIdentity.state !== 'live') {
         throw new Error('VMM cleanup identity is not committed before ACL grant');
@@ -117,10 +118,14 @@ export function createCleanupHandle(options: CleanupHandleFactoryOptions): Cloud
         record.vmmIdentity.aclPaths.push(aclPath);
         record.vmmIdentity.aclBaselines ??= {};
         record.vmmIdentity.aclBaselines[aclPath] = validatedBaseline;
+        record.vmmIdentity.aclTemporaryUids ??= {};
+        record.vmmIdentity.aclTemporaryUids[aclPath] = [...temporaryUids];
         await update();
       } else if (
         JSON.stringify(record.vmmIdentity.aclBaselines?.[aclPath]) !==
-        JSON.stringify(validatedBaseline)
+        JSON.stringify(validatedBaseline) ||
+        JSON.stringify(record.vmmIdentity.aclTemporaryUids?.[aclPath] ?? []) !==
+        JSON.stringify(temporaryUids)
       ) {
         throw new Error(`VMM ACL cleanup baseline changed for ${aclPath}`);
       }
@@ -135,6 +140,9 @@ export function createCleanupHandle(options: CleanupHandleFactoryOptions): Cloud
       if (record.vmmIdentity.aclBaselines) {
         delete record.vmmIdentity.aclBaselines[aclPath];
       }
+      if (record.vmmIdentity.aclTemporaryUids) {
+        delete record.vmmIdentity.aclTemporaryUids[aclPath];
+      }
       await update();
     },
     captureNetworkResource: async (resource: CloudHypervisorNetworkResource) => {

src/cloud-hypervisor/cleanup-identity.ts

@@ -100,6 +100,7 @@ export interface CleanupRecord {
     gid?: number;
     aclPaths: string[];
     aclBaselines?: Partial<Record<CloudHypervisorDevicePath, string[]>>;
+    aclTemporaryUids?: Partial<Record<CloudHypervisorDevicePath, number[]>>;
   };
   mounts: MountIdentity[];
   updatedAt: string;
@@ -204,6 +205,21 @@ export function validateRecord(
           }
         })
       )) ||
+      (identity.aclTemporaryUids !== undefined && (
+        typeof identity.aclTemporaryUids !== 'object' ||
+        identity.aclTemporaryUids === null ||
+        Array.isArray(identity.aclTemporaryUids) ||
+        Object.keys(identity.aclTemporaryUids).some((aclPath) =>
+          aclPath !== '/dev/kvm' && aclPath !== '/dev/net/tun') ||
+        Object.entries(identity.aclTemporaryUids).some(([aclPath, uids]) =>
+          !identity.aclPaths.includes(aclPath) ||
+          !Array.isArray(uids) ||
+          uids.some((uid) => !Number.isSafeInteger(uid) || uid <= 0) ||
+          new Set(uids).size !== uids.length ||
+          !uids.every((uid) =>
+            identity.aclBaselines?.[aclPath as CloudHypervisorDevicePath]
+              ?.includes(`user:${uid}:rw-`)))
+      )) ||
       (identity.state === 'live' && (() => {
         const uid = identity.uid;
         const gid = identity.gid;

src/cloud-hypervisor/cleanup-registry.ts

@@ -49,7 +49,11 @@ export interface CloudHypervisorCleanupHandle {
   captureArtifactSnapshot(directory: string): Promise<void>;
   prepareVmmAccount(name: string): Promise<void>;
   captureVmmIdentity(identity: import('./vmm-identity').CloudHypervisorVmmIdentity): Promise<void>;
-  prepareVmmAcl(path: CloudHypervisorDevicePath, baseline: readonly string[]): Promise<void>;
+  prepareVmmAcl(
+    path: CloudHypervisorDevicePath,
+    baseline: readonly string[],
+    temporaryUids?: readonly number[],
+  ): Promise<void>;
   releaseVmmAcl(path: string): Promise<void>;
   captureNetworkResource(resource: CloudHypervisorNetworkResource): Promise<void>;
   captureRunDirectory(): Promise<void>;

src/cloud-hypervisor/cleanup-vmm-identity.test.ts

@@ -24,10 +24,20 @@ const baseline = [
   'other::rw-',
 ];
 
-function cleanupRecord(aclBaselines?: Record<'/dev/kvm', string[]>): CleanupRecord {
+function cleanupRecord(
+  aclBaselines?: Record<'/dev/kvm', string[]>,
+  options: {
+    runId?: string;
+    name?: string;
+    uid?: number;
+    gid?: number;
+    aclTemporaryUids?: Record<'/dev/kvm', number[]>;
+  } = {},
+): CleanupRecord {
+  const runId = options.runId ?? 'acl-recovery';
   return {
     version: 1,
-    runId: 'acl-recovery',
+    runId,
     owner: {
       pid: process.pid,
       startTime: '1',
@@ -39,19 +49,22 @@ function cleanupRecord(aclBaselines?: Record<'/dev/kvm', string[]>): CleanupReco
     },
     cloudHypervisorBinary: process.execPath,
     paths: {
-      runDirectory: '/run/awf-cloud-hypervisor/cloud-hypervisor/acl-recovery',
-      cgroupPath: '/sys/fs/cgroup/awf-cloud-hypervisor/acl-recovery',
-      virtiofsdShareDirectory: '/run/awf-cloud-hypervisor/virtiofsd/acl-recovery',
+      runDirectory: `/run/awf-cloud-hypervisor/cloud-hypervisor/${runId}`,
+      cgroupPath: `/sys/fs/cgroup/awf-cloud-hypervisor/${runId}`,
+      virtiofsdShareDirectory: `/run/awf-cloud-hypervisor/virtiofsd/${runId}`,
     },
     identities: {},
     processes: {},
     vmmIdentity: {
       state: 'live',
-      name: 'awfvmm-0123456789abcdef0123',
-      uid: 23001,
-      gid: 23002,
+      name: options.name ?? 'awfvmm-0123456789abcdef0123',
+      uid: options.uid ?? 23001,
+      gid: options.gid ?? 23002,
       aclPaths: ['/dev/kvm'],
       ...(aclBaselines ? { aclBaselines } : {}),
+      ...(options.aclTemporaryUids ? {
+        aclTemporaryUids: options.aclTemporaryUids,
+      } : {}),
     },
     mounts: [],
     updatedAt: new Date(0).toISOString(),
@@ -116,6 +129,9 @@ describe('Cloud Hypervisor durable VMM identity cleanup', () => {
         return { exitCode: 0, stdout: `${currentAcl.join('\n')}\n`, stderr: '' };
       }
       if (command === tools.getent && args[0] === 'passwd' && !accountDeleted) {
+        if (/^\d+$/.test(args[1]) && args[1] !== '23001') {
+          return { exitCode: 2, stdout: '', stderr: '' };
+        }
         return {
           exitCode: 0,
           stdout:
@@ -222,4 +238,131 @@ describe('Cloud Hypervisor durable VMM identity cleanup', () => {
       ['--remove', 'user:23001', '/dev/kvm'],
     );
   });
+
+  it.each([
+    ['earlier identity first', [999, 997]],
+    ['later identity first', [997, 999]],
+  ] as const)(
+    'preserves the exact runner baseline for two overlapping identities: %s',
+    async (_name, cleanupOrder) => {
+      const runnerUid = 1001;
+      const identities = new Map([
+        [999, {
+          runId: 'first-run',
+          name: 'awfvmm-11111111111111111111',
+          gid: 1999,
+          baseline: [
+            'user::rw-', `user:${runnerUid}:rw-`,
+            'group::rw-', 'mask::rw-', 'other::---',
+          ],
+          temporaryUids: [],
+        }],
+        [997, {
+          runId: 'second-run',
+          name: 'awfvmm-22222222222222222222',
+          gid: 1997,
+          baseline: [
+            'user::rw-', 'user:999:rw-', `user:${runnerUid}:rw-`,
+            'group::rw-', 'mask::rw-', 'other::---',
+          ],
+          temporaryUids: [999],
+        }],
+      ]);
+      const accountExists = new Map([[999, true], [997, true]]);
+      const groupExists = new Map([[999, true], [997, true]]);
+      let currentAcl = [
+        'user::rw-', 'user:997:rw-', 'user:999:rw-', `user:${runnerUid}:rw-`,
+        'group::rw-', 'mask::rw-', 'other::---',
+      ];
+      const run = jest.fn(async (command: string, args: readonly string[]) => {
+        if (command === tools.getfacl) {
+          return { exitCode: 0, stdout: `${currentAcl.join('\n')}\n`, stderr: '' };
+        }
+        if (command === tools.setfacl && args[0].startsWith('--set=')) {
+          currentAcl = args[0].slice('--set='.length).split(',');
+          return { exitCode: 0, stdout: '', stderr: '' };
+        }
+        if (command === tools.getent && args[0] === 'passwd') {
+          const query = args[1];
+          const entry = [...identities.entries()].find(([uid, identity]) =>
+            String(uid) === query || identity.name === query);
+          if (!entry || !accountExists.get(entry[0])) {
+            return { exitCode: 2, stdout: '', stderr: '' };
+          }
+          const [uid, identity] = entry;
+          return {
+            exitCode: 0,
+            stdout: `${identity.name}:x:${uid}:${identity.gid}:` +
+              `AWF Cloud Hypervisor ${identity.runId}:/nonexistent:/usr/sbin/nologin\n`,
+            stderr: '',
+          };
+        }
+        if (command === tools.getent && args[0] === 'group') {
+          const entry = [...identities.entries()].find(([, identity]) =>
+            identity.name === args[1]);
+          if (!entry || !groupExists.get(entry[0])) {
+            return { exitCode: 2, stdout: '', stderr: '' };
+          }
+          return {
+            exitCode: 0,
+            stdout: `${entry[1].name}:x:${entry[1].gid}:\n`,
+            stderr: '',
+          };
+        }
+        if (command === tools.id) {
+          const entry = [...identities.entries()].find(([, identity]) =>
+            identity.name === args[1]);
+          if (!entry || !accountExists.get(entry[0])) {
+            return { exitCode: 1, stdout: '', stderr: '' };
+          }
+          const [uid, identity] = entry;
+          return {
+            exitCode: 0,
+            stdout: `${args[0] === '-u' ? uid : identity.gid}\n`,
+            stderr: '',
+          };
+        }
+        if (command === tools.userdel) {
+          const entry = [...identities.entries()].find(([, identity]) =>
+            identity.name === args[0])!;
+          accountExists.set(entry[0], false);
+          currentAcl = ['user::rw-', 'group::rw-', 'mask::rw-', 'other::---'];
+          return { exitCode: 0, stdout: '', stderr: '' };
+        }
+        if (command === tools.groupdel) {
+          const entry = [...identities.entries()].find(([, identity]) =>
+            identity.name === args[0])!;
+          groupExists.set(entry[0], false);
+          return { exitCode: 0, stdout: '', stderr: '' };
+        }
+        throw new Error(`Unexpected command: ${command} ${args.join(' ')}`);
+      });
+      const resolved = dependencies(root, run);
+
+      for (const uid of cleanupOrder) {
+        const identity = identities.get(uid)!;
+        await deleteVmmIdentity(
+          resolved,
+          cleanupRecord(
+            { '/dev/kvm': identity.baseline },
+            {
+              runId: identity.runId,
+              name: identity.name,
+              uid,
+              gid: identity.gid,
+              aclTemporaryUids: { '/dev/kvm': identity.temporaryUids },
+            },
+          ),
+          tools,
+        );
+      }
+
+      expect(currentAcl).toEqual([
+        'user::rw-', `user:${runnerUid}:rw-`,
+        'group::rw-', 'mask::rw-', 'other::---',
+      ]);
+      expect(currentAcl).not.toContain('user:999:rw-');
+      expect(currentAcl).not.toContain('user:997:rw-');
+    },
+  );
 });

src/cloud-hypervisor/cleanup-vmm-identity.ts

@@ -1,12 +1,14 @@
 import type { CleanupRecord } from './cleanup-identity';
 import * as path from 'path';
 import {
+  resolveTemporaryVmmAclUids,
   withCloudHypervisorDeviceAclLock,
   type CloudHypervisorVmmIdentityDependencies,
   type CloudHypervisorVmmIdentityToolPaths,
 } from './vmm-identity';
 import { runChecked, type ResolvedCleanupDependencies } from './cleanup-dependencies';
 import {
+  composeDeviceAcl,
   describeDeviceAclShape,
   deviceAclArgument,
   parseDeviceAcl,
@@ -29,10 +31,10 @@ export async function deleteVmmIdentity(
   }
   if (identity.state === 'live' && identity.aclPaths.length > 0) {
     await withVmmAclLock(dependencies, tools, async () => {
-      await restoreVmmAclsLocked(dependencies, identity, tools);
+      const restoredTargets = await restoreVmmAclsLocked(dependencies, identity, tools);
       const expectedGid = await deleteVmmAccount(dependencies, record, identity, tools);
       await deleteVmmGroup(dependencies, identity, tools, expectedGid);
-      await restoreVmmAclsLocked(dependencies, identity, tools);
+      await restoreVmmAclsLocked(dependencies, identity, tools, restoredTargets);
     });
     return;
   }
@@ -67,12 +69,42 @@ async function restoreVmmAclsLocked(
   dependencies: ResolvedCleanupDependencies,
   identity: RecordedVmmIdentity,
   tools: CloudHypervisorVmmIdentityToolPaths,
-): Promise<void> {
+  retainedTargets?: ReadonlyMap<CloudHypervisorDevicePath, readonly string[]>,
+): Promise<Map<CloudHypervisorDevicePath, string[]>> {
+  const restoredTargets = new Map<CloudHypervisorDevicePath, string[]>();
   for (const aclPath of [...identity.aclPaths].reverse()) {
     const baseline = identity.aclBaselines?.[aclPath as CloudHypervisorDevicePath];
     if (baseline) {
+      const retainedTarget = retainedTargets?.get(aclPath as CloudHypervisorDevicePath);
+      let restoredTarget: string[];
+      if (retainedTarget) {
+        restoredTarget = [...retainedTarget];
+      } else {
+        const before = await dependencies.run(tools.getfacl, [
+          '--absolute-names', '--numeric', aclPath,
+        ]);
+        if (before.exitCode !== 0) {
+          throw new Error(
+            `VMM ACL restoration validation failed for ${aclPath}: ${before.stderr.trim()}`,
+          );
+        }
+        const current = parseDeviceAcl(before.stdout);
+        const liveTemporaryUids = await resolveTemporaryVmmAclUids(
+          current,
+          tools,
+          dependencies.run,
+        );
+        restoredTarget = composeDeviceAcl(
+          baseline,
+          current,
+          identity.uid!,
+          identity.aclTemporaryUids?.[aclPath as CloudHypervisorDevicePath] ?? [],
+          liveTemporaryUids,
+        );
+      }
+      restoredTargets.set(aclPath as CloudHypervisorDevicePath, restoredTarget);
       await runChecked(dependencies.run, tools.setfacl, [
-        `--set=${deviceAclArgument(baseline)}`, aclPath,
+        `--set=${deviceAclArgument(restoredTarget)}`, aclPath,
       ]);
       const restored = await dependencies.run(tools.getfacl, [
         '--absolute-names', '--numeric', aclPath,
@@ -83,10 +115,10 @@ async function restoreVmmAclsLocked(
         );
       }
       const restoredAcl = parseDeviceAcl(restored.stdout);
-      if (!sameDeviceAcl(restoredAcl, baseline)) {
+      if (!sameDeviceAcl(restoredAcl, restoredTarget)) {
         throw new Error(
           `VMM ACL restoration validation failed for ${aclPath}: ` +
-          `before=${describeDeviceAclShape(baseline)} ` +
+          `before=${describeDeviceAclShape(restoredTarget)} ` +
           `after=${describeDeviceAclShape(restoredAcl)}`,
         );
       }
@@ -109,6 +141,7 @@ async function restoreVmmAclsLocked(
       ) throw new Error(`VMM ACL removal validation failed for ${aclPath}`);
     }
   }
+  return restoredTargets;
 }
 
 /**