diff --git a/src/artifact-permissions.test.ts b/src/artifact-permissions.test.ts index 26b8ca373..7474f806b 100644 --- a/src/artifact-permissions.test.ts +++ b/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 { diff --git a/src/artifact-permissions.ts b/src/artifact-permissions.ts index 4b0aea4c4..a544a987a 100644 --- a/src/artifact-permissions.ts +++ b/src/artifact-permissions.ts @@ -7,6 +7,39 @@ import { logger } from './logger'; 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'; @@ -90,10 +123,8 @@ export function fixArtifactPermissionsForRootless( // 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.`, diff --git a/src/artifact-preservation-errors.test.ts b/src/artifact-preservation-errors.test.ts index 4587b3d67..5c360d298 100644 --- a/src/artifact-preservation-errors.test.ts +++ b/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(); diff --git a/src/artifact-preservation.ts b/src/artifact-preservation.ts index 72d67ff63..91fccd12f 100644 --- a/src/artifact-preservation.ts +++ b/src/artifact-preservation.ts @@ -3,7 +3,10 @@ import * as path from 'path'; 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'; @@ -89,7 +92,14 @@ function preserveDirectory({ 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; @@ -193,7 +203,14 @@ export function preserveCleanupArtifacts( 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 {