fix: demote benign rootless cleanup errors

src/artifact-permissions.test.ts

@@ -8,7 +8,10 @@ jest.mock('execa', () => {
 import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
-import { fixArtifactPermissionsForRootless } from './artifact-permissions';
+import {
+  fixArtifactPermissionsForRootless,
+  isBenignArtifactPermissionError,
+} from './artifact-permissions';
 import { mockExecaSync } from './test-helpers/mock-execa.test-utils';
 
 function makeTempDir(prefix = 'awf-artifact-perms-'): string {
@@ -115,6 +118,13 @@ describe('artifact-permissions', () => {
     }
   });
 
+  it('treats standalone execa permission codes as benign permission errors', () => {
+    expect(isBenignArtifactPermissionError({
+      code: 'EACCES',
+      message: 'Command failed with EACCES: chmod -R a+rX /tmp/awf-audit',
+    })).toBe(true);
+  });
+
   it('runs rootless permission repair with translated mount paths', () => {
     const auditDir = makeTempDir();
     try {

src/artifact-permissions.ts

@@ -7,6 +7,39 @@
 import { applyHostPathPrefixToVolumes } from './services/host-path-prefix';
 import { getLocalDockerEnv } from './docker-host';
 
+export function isBenignArtifactPermissionError(error: unknown): boolean {
+  const details: string[] = [];
+  if (typeof error === 'string') {
+    details.push(error);
+  } else if (error && typeof error === 'object') {
+    const errorLike = error as {
+      stderr?: unknown;
+      stdout?: unknown;
+      shortMessage?: unknown;
+      message?: unknown;
+      code?: unknown;
+    };
+    for (const value of [
+      errorLike.stderr,
+      errorLike.stdout,
+      errorLike.shortMessage,
+      errorLike.message,
+      errorLike.code,
+    ]) {
+      if (typeof value === 'string') {
+        details.push(value);
+      }
+    }
+  }
+
+  const combinedDetails = details.join('\n');
+  return (
+    /(?:^|\n)(?:chown|chmod):.*(?:operation not permitted|permission denied|\bEPERM\b|\bEACCES\b)/i.test(
+      combinedDetails,
+    ) || /(?:^|\n)\s*(?:EPERM|EACCES)\s*(?:\n|$)/i.test(combinedDetails)
+  );
+}
+
 function resolvePermFixerImageRef(imageRegistry?: string, imageTag?: string, agentImage?: string): string {
   try {
     const registry = imageRegistry || 'ghcr.io/github/gh-aw-firewall';
@@ -34,7 +67,7 @@
   }
 
   const existingDirs = dirs.filter(
     (dir): dir is string => typeof dir === 'string' && dir.length > 0 && fs.existsSync(dir),
   );
   if (existingDirs.length === 0) {
     return;
@@ -67,9 +100,9 @@
           '--entrypoint',
           'sh',
           '-e',
           `TUID=${uid}`,
           '-e',
           `TGID=${gid}`,
           '-v',
           mount,
           imageRef,
@@ -90,10 +123,8 @@
         // producing "Operation not permitted" / "Permission denied". Those are
         // expected and non-fatal, so log them at debug to avoid alarming users
         // who otherwise see a scary WARN for a benign, non-blocking condition.
-        const isBenignPermissionError =
-          !!errorDetail && /(?:^|\n)(?:chown|chmod):.*(?:operation not permitted|permission denied|EPERM|EACCES)/i.test(errorDetail);
         const detail = `for ${dir} (exit ${result.exitCode})` + (errorDetail ? `: ${errorDetail}` : '');
-        if (isBenignPermissionError) {
+        if (isBenignArtifactPermissionError(errorDetail)) {
           logger.debug(
             `Rootless artifact permission repair skipped ${detail}. ` +
               `This is expected on restricted runners and does not affect the run.`,

src/artifact-preservation-errors.test.ts

@@ -41,6 +41,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
 import { fixArtifactPermissionsForRootless } from './artifact-permissions';
+import { logger } from './logger';
 import { mockExecaSync } from './test-helpers/mock-execa.test-utils';
 import {
   preserveIptablesAudit,
@@ -125,21 +126,56 @@ describe('artifact-preservation – error paths', () => {
       }
     });
 
-    it('does not throw when runtimeDir chmod fails (line 62)', () => {
+    it('keeps the primary failure as the last visible diagnostic when runtimeDir chmod is denied', () => {
       // proxyLogsDir squid-logs uses runtimeDirMustExist:false → chmod always called.
       // With no api-proxy-logs or cli-proxy-logs subdirs, squid-logs chmod is first.
       const externalDir = makeTempDir();
       const workDir = makeTempDir();
+      const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
       try {
         const proxyLogsDir = path.join(externalDir, 'proxy-logs');
         realFs.mkdirSync(proxyLogsDir);
 
         mockExecaSync.mockImplementationOnce(() => {
-          throw new Error('chmod: operation not permitted');
+          throw Object.assign(new Error('Command failed with exit code 1: chmod -R a+rX'), {
+            stderr: `chmod: changing permissions of '${proxyLogsDir}': Operation not permitted`,
+            exitCode: 1,
+          });
         });
 
+        logger.error('Fatal error: primary topology startup failure');
         expect(() => preserveCleanupArtifacts(workDir, { proxyLogsDir })).not.toThrow();
+        expect(errorSpy.mock.calls[errorSpy.mock.calls.length - 1]?.[0]).toEqual(
+          expect.stringContaining('[ERROR] Fatal error: primary topology startup failure'),
+        );
+        expect(errorSpy.mock.calls.flat().join('\n')).not.toContain(
+          'Could not fix squid log permissions',
+        );
+      } finally {
+        errorSpy.mockRestore();
+        realFs.rmSync(externalDir, { recursive: true, force: true });
+        realFs.rmSync(workDir, { recursive: true, force: true });
+      }
+    });
+
+    it('warns when runtimeDir chmod fails unexpectedly', () => {
+      const externalDir = makeTempDir();
+      const workDir = makeTempDir();
+      const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+      try {
+        const proxyLogsDir = path.join(externalDir, 'proxy-logs');
+        realFs.mkdirSync(proxyLogsDir);
+        mockExecaSync.mockImplementationOnce(() => {
+          throw new Error('chmod: input/output error');
+        });
+
+        preserveCleanupArtifacts(workDir, { proxyLogsDir });
+
+        expect(errorSpy.mock.calls.flat().join('\n')).toContain(
+          '[WARN] Could not fix squid log permissions:',
+        );
       } finally {
+        errorSpy.mockRestore();
         realFs.rmSync(externalDir, { recursive: true, force: true });
         realFs.rmSync(workDir, { recursive: true, force: true });
       }
@@ -180,6 +216,33 @@ describe('artifact-preservation – error paths', () => {
       }
     });
 
+    it('keeps the primary failure as the last visible diagnostic when auditDir chmod is denied', () => {
+      const auditDir = makeTempDir('awf-audit-');
+      const workDir = makeTempDir();
+      const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+      try {
+        getuidSpy = jest.spyOn(process, 'getuid').mockReturnValue(0);
+        mockExecaSync.mockImplementationOnce(() => {
+          throw Object.assign(new Error('Command failed with EACCES: chmod -R a+rX'), {
+            code: 'EACCES',
+          });
+        });
+
+        logger.error('Fatal error: primary topology startup failure');
+        expect(() => preserveCleanupArtifacts(workDir, { auditDir })).not.toThrow();
+        expect(errorSpy.mock.calls[errorSpy.mock.calls.length - 1]?.[0]).toEqual(
+          expect.stringContaining('[ERROR] Fatal error: primary topology startup failure'),
+        );
+        expect(errorSpy.mock.calls.flat().join('\n')).not.toContain(
+          'Could not fix audit dir permissions as non-root user',
+        );
+      } finally {
+        errorSpy.mockRestore();
+        realFs.rmSync(auditDir, { recursive: true, force: true });
+        realFs.rmSync(workDir, { recursive: true, force: true });
+      }
+    });
+
     it('runs rootless permission repair with translated mount paths', () => {
       const auditDir = makeTempDir('awf-audit-');
       const workDir = makeTempDir();

src/artifact-preservation.ts

@@ -3,7 +3,10 @@
 import * as os from 'os';
 import execa from 'execa';
 import { logger } from './logger';
-import { fixArtifactPermissionsForRootless } from './artifact-permissions';
+import {
+  fixArtifactPermissionsForRootless,
+  isBenignArtifactPermissionError,
+} from './artifact-permissions';
 import { getLocalDockerEnv } from './host-env';
 import { resolveBoundedQueryPaths } from './bounded-query/paths';
 
@@ -21,19 +24,19 @@
   const iptablesAuditSrc = path.join(workDir, 'init-signal', 'iptables-audit.txt');
   const boundedQueryRoot = resolveBoundedQueryPaths(workDir).root;
   const targetAuditDir = auditDir || path.join(workDir, 'audit');
   if (!fs.existsSync(targetAuditDir)) return;
 
   if (fs.existsSync(iptablesAuditSrc)) {
     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);
     }
   }
 
   if (fs.existsSync(boundedQueryRoot)) {
     for (const auditFile of BOUNDED_QUERY_AUDIT_FILES) {
       try {
         const source = `awf-bounded-query-broker:/var/log/awf-bounded-query/${auditFile}`;
@@ -84,12 +87,19 @@
 }: PreserveDirectoryOptions): void {
   if (runtimeDir) {
     const targetDir = runtimeSubdir ? path.join(runtimeDir, runtimeSubdir) : runtimeDir;
     if (fs.existsSync(targetDir)) {
       try {
         execa.sync('chmod', ['-R', 'a+rX', targetDir]);
         logger.info(`${availableLabel} available at: ${targetDir}`);
       } catch (error) {
-        logger.warn(permissionErrorMessage, error);
+        if (isBenignArtifactPermissionError(error)) {
+          logger.debug(
+            `${permissionErrorMessage} Permission repair was denied for ${targetDir}; ` +
+              'this is expected on restricted runners and does not affect the run.',
+          );
+        } else {
+          logger.warn(permissionErrorMessage, error);
+        }
       }
     }
     return;
@@ -97,7 +107,7 @@
 
   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) {
@@ -193,7 +203,14 @@
         execa.sync('chmod', ['-R', 'a+rX', auditDir]);
         logger.info(`Audit artifacts available at: ${auditDir}`);
       } catch (error) {
-        logger.warn('Could not fix audit dir permissions as non-root user; rootless repair will be attempted:', error);
+        if (isBenignArtifactPermissionError(error)) {
+          logger.debug(
+            `Could not fix audit dir permissions as non-root user. Permission repair was denied for ${auditDir}; ` +
+              'this is expected on restricted runners and rootless repair will be attempted.',
+          );
+        } else {
+          logger.warn('Could not fix audit dir permissions as non-root user; rootless repair will be attempted:', error);
+        }
       }
     }
   } else {