fix: handle EACCES during chroot-home cleanup in rootless Docker

src/artifact-preservation-errors.test.ts

@@ -15,6 +15,15 @@
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 jest.mock('execa', () => require('./test-helpers/mock-execa.test-utils').execaMockFactory());
 
+jest.mock('./artifact-permissions', () => {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  const real = jest.requireActual<typeof import('./artifact-permissions')>('./artifact-permissions');
+  return {
+    ...real,
+    fixArtifactPermissionsForRootless: jest.fn(real.fixArtifactPermissionsForRootless),
+  };
+});
+
 // Wrap the fs methods we need to control with jest.fn() so individual tests
 // can inject one-shot errors without using jest.spyOn (which fails on non-
 // configurable properties in Jest's module sandbox).
@@ -24,6 +33,7 @@ jest.mock('fs', () => {
   return {
     ...real,
     copyFileSync: jest.fn(real.copyFileSync),
+    rmSync: jest.fn(real.rmSync),
     renameSync: jest.fn(real.renameSync),
     mkdirSync: jest.fn(real.mkdirSync),
   };
@@ -32,16 +42,21 @@ jest.mock('fs', () => {
 import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
+import { fixArtifactPermissionsForRootless } from './artifact-permissions';
 import { mockExecaSync } from './test-helpers/mock-execa.test-utils';
 import {
   preserveIptablesAudit,
   preserveCleanupArtifacts,
+  removeWorkDirectories,
 } from './artifact-preservation';
 
 // Cast mocked methods for convenient mockImplementationOnce usage.
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 const realFs = jest.requireActual<typeof import('fs')>('fs');
 const mockCopyFileSync = fs.copyFileSync as jest.MockedFunction<typeof fs.copyFileSync>;
+const mockFixArtifactPermissionsForRootless =
+  fixArtifactPermissionsForRootless as jest.MockedFunction<typeof fixArtifactPermissionsForRootless>;
+const mockRmSync = fs.rmSync as jest.MockedFunction<typeof fs.rmSync>;
 const mockRenameSync = fs.renameSync as jest.MockedFunction<typeof fs.renameSync>;
 const mockMkdirSync = fs.mkdirSync as jest.MockedFunction<typeof fs.mkdirSync>;
 
@@ -56,6 +71,7 @@ describe('artifact-preservation – error paths', () => {
     mockExecaSync.mockReturnValue({ stdout: '', stderr: '', exitCode: 0 });
     // Re-apply real implementations in case a test replaced them with mockImplementation/mockImplementationOnce.
     mockCopyFileSync.mockImplementation(realFs.copyFileSync);
+    mockRmSync.mockImplementation(realFs.rmSync);
     mockRenameSync.mockImplementation(realFs.renameSync);
     mockMkdirSync.mockImplementation(realFs.mkdirSync);
   });
@@ -317,4 +333,48 @@ describe('artifact-preservation – error paths', () => {
       }
     });
   });
+
+  describe('removeWorkDirectories', () => {
+    it('repairs chroot-home permissions and retries removal after EACCES', () => {
+      const workDir = makeTempDir();
+      const chrootHomeDir = `${workDir}-chroot-home`;
+      let chrootHomeRemovalAttempts = 0;
+
+      try {
+        realFs.mkdirSync(chrootHomeDir);
+        realFs.writeFileSync(path.join(chrootHomeDir, '.aws-config'), 'config');
+
+        mockRmSync.mockImplementation((target, options) => {
+          if (target === chrootHomeDir) {
+            chrootHomeRemovalAttempts += 1;
+            if (chrootHomeRemovalAttempts === 1) {
+              const error = Object.assign(new Error('permission denied'), { code: 'EACCES' });
+              throw error;
+            }
+          }
+          return realFs.rmSync(target, options);
+        });
+
+        expect(() => removeWorkDirectories(workDir, {
+          dockerHostPathPrefix: '/host',
+          imageRegistry: 'ghcr.io/github/gh-aw-firewall',
+          imageTag: 'latest',
+          agentImage: 'act',
+        })).not.toThrow();
+
+        expect(mockFixArtifactPermissionsForRootless).toHaveBeenCalledWith(
+          [chrootHomeDir],
+          '/host',
+          'ghcr.io/github/gh-aw-firewall',
+          'latest',
+          'act',
+        );
+        expect(chrootHomeRemovalAttempts).toBe(2);
+        expect(realFs.existsSync(chrootHomeDir)).toBe(false);
+      } finally {
+        realFs.rmSync(workDir, { recursive: true, force: true });
+        realFs.rmSync(chrootHomeDir, { recursive: true, force: true });
+      }
+    });
+  });
 });

src/artifact-preservation.ts

@@ -13,10 +13,10 @@
 export function preserveIptablesAudit(workDir: string, auditDir?: string): void {
   const iptablesAuditSrc = path.join(workDir, 'init-signal', 'iptables-audit.txt');
   const targetAuditDir = auditDir || path.join(workDir, 'audit');
   if (fs.existsSync(iptablesAuditSrc) && fs.existsSync(targetAuditDir)) {
     try {
       fs.copyFileSync(iptablesAuditSrc, path.join(targetAuditDir, 'iptables-audit.txt'));
       fs.chmodSync(path.join(targetAuditDir, 'iptables-audit.txt'), 0o644);
       logger.debug('Copied iptables audit state to audit directory');
     } catch (error) {
       logger.debug('Could not copy iptables audit file:', error);
@@ -55,7 +55,7 @@
 }: PreserveDirectoryOptions): void {
   if (runtimeDir) {
     const targetDir = runtimeSubdir ? path.join(runtimeDir, runtimeSubdir) : runtimeDir;
     if (!runtimeDirMustExist || fs.existsSync(targetDir)) {
       try {
         execa.sync('chmod', ['-R', 'a+rX', targetDir]);
         logger.info(`${availableLabel} available at: ${targetDir}`);
@@ -68,9 +68,9 @@
 
   const sourceDir = path.join(workDir, workSubdir);
   const destinationDir = path.join(os.tmpdir(), `${destinationBaseName}-${timestamp}`);
   if (fs.existsSync(sourceDir) && fs.readdirSync(sourceDir).length > 0) {
     try {
       fs.renameSync(sourceDir, destinationDir);
       if (chmodPreservedDir) {
         execa.sync('chmod', ['-R', 'a+rX', destinationDir]);
       }
@@ -220,11 +220,39 @@
   );
 }
 
-export function removeWorkDirectories(workDir: string): void {
+type RemoveWorkDirectoriesOptions = Pick<
+  PreserveCleanupArtifactsOptions,
+  'dockerHostPathPrefix' | 'imageRegistry' | 'imageTag' | 'agentImage'
+>;
+
+export function removeWorkDirectories(workDir: string, options: RemoveWorkDirectoriesOptions = {}): void {
   fs.rmSync(workDir, { recursive: true, force: true });
 
   const chrootHomeDir = `${workDir}-chroot-home`;
   if (fs.existsSync(chrootHomeDir)) {
-    fs.rmSync(chrootHomeDir, { recursive: true, force: true });
+    try {
+      fs.rmSync(chrootHomeDir, { recursive: true, force: true });
+    } catch (error: unknown) {
+      // In rootless Docker, files created inside the container may be owned by
+      // remapped UIDs that the host process cannot delete. Fix permissions via
+      // a privileged container, then retry removal.
+      if (error && typeof error === 'object' && 'code' in error && error.code === 'EACCES') {
+        logger.debug('Chroot home removal failed with EACCES; attempting rootless permission repair');
+        fixArtifactPermissionsForRootless(
+          [chrootHomeDir],
+          options.dockerHostPathPrefix,
+          options.imageRegistry,
+          options.imageTag,
+          options.agentImage,
+        );
+        try {
+          fs.rmSync(chrootHomeDir, { recursive: true, force: true });
+        } catch (retryError) {
+          logger.warn('Failed to remove chroot home directory after permission repair:', retryError);
+        }
+      } else {
+        logger.warn('Failed to remove chroot home directory:', error);
+      }
+    }
   }
 }

src/container-cleanup.ts

@@ -50,7 +50,12 @@ export async function cleanup(
       await unmountSslTmpfs(sslDir);
     }
 
-    removeWorkDirectories(workDir);
+    removeWorkDirectories(workDir, {
+      dockerHostPathPrefix,
+      imageRegistry,
+      imageTag,
+      agentImage,
+    });
     logger.debug('Temporary files cleaned up');
   } catch (error) {
     logger.warn('Failed to clean up temporary files:', error);