Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/artifact-permissions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,25 @@ describe('artifact-permissions', () => {
}
});

it('logs stdout when stderr is empty and command writes only to stdout', () => {
const auditDir = makeTempDir();
let warnSpy: jest.SpyInstance | undefined;
try {
getuidSpy = jest.spyOn(process, 'getuid').mockReturnValue(1001);
warnSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
mockExecaSync.mockReturnValue({
stdout: '[entrypoint] Timed out waiting for iptables init container after 30s',
stderr: '',
exitCode: 1,
});
fixArtifactPermissionsForRootless([auditDir], undefined, undefined, undefined, undefined);
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Timed out waiting for iptables init container'));
} finally {
warnSpy?.mockRestore();
fs.rmSync(auditDir, { recursive: true, force: true });
}
});

it('does not warn for benign permission errors on restricted runners', () => {
const auditDir = makeTempDir();
let errorSpy: jest.SpyInstance | undefined;
Expand Down Expand Up @@ -113,9 +132,13 @@ describe('artifact-permissions', () => {
'run',
'--pull',
'never',
'--entrypoint',
'sh',
'-v',
`/host${path.resolve(auditDir)}:/fix:rw`,
'ghcr.io/github/gh-aw-firewall/agent:latest',
'-c',
'chown -R "$TUID:$TGID" /fix 2>/dev/null; chmod -R a+rwX /fix',
]),
expect.objectContaining({ reject: false }),
);
Expand Down
9 changes: 6 additions & 3 deletions src/artifact-permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
}

const existingDirs = dirs.filter(
(dir): dir is string => typeof dir === 'string' && dir.length > 0 && fs.existsSync(dir),

Check warning on line 37 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found existsSync from package "fs" with non literal argument at index 0

Check warning on line 37 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found existsSync from package "fs" with non literal argument at index 0

Check warning on line 37 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found existsSync from package "fs" with non literal argument at index 0
);
if (existingDirs.length === 0) {
return;
Expand Down Expand Up @@ -64,14 +64,15 @@
'DAC_OVERRIDE',
'--cap-add',
'FOWNER',
'--entrypoint',
'sh',
'-e',
`TUID=${uid}`,

Check warning on line 70 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / ESLint

Avoid template literals with expressions in execa arguments. Pass arguments as separate array elements

Check warning on line 70 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Avoid template literals with expressions in execa arguments. Pass arguments as separate array elements

Check warning on line 70 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Avoid template literals with expressions in execa arguments. Pass arguments as separate array elements
'-e',
`TGID=${gid}`,

Check warning on line 72 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / ESLint

Avoid template literals with expressions in execa arguments. Pass arguments as separate array elements

Check warning on line 72 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Avoid template literals with expressions in execa arguments. Pass arguments as separate array elements

Check warning on line 72 in src/artifact-permissions.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Avoid template literals with expressions in execa arguments. Pass arguments as separate array elements
'-v',
mount,
imageRef,
'sh',
'-c',
'chown -R "$TUID:$TGID" /fix 2>/dev/null; chmod -R a+rwX /fix',
],
Expand All @@ -80,6 +81,8 @@

if (typeof result.exitCode === 'number' && result.exitCode !== 0) {
const stderr = result.stderr?.trim();
const stdout = result.stdout?.trim();
const errorDetail = stderr || stdout;
// Ownership/permission repair is best-effort: the agent has already
// finished and its artifacts are still readable by the owning user.
// On rootless or restricted runners (e.g. ARC/DinD with a non-root
Expand All @@ -88,8 +91,8 @@
// 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 =
!!stderr && /(?:^|\n)(?:chown|chmod):.*(?:operation not permitted|permission denied|EPERM|EACCES)/i.test(stderr);
const detail = `for ${dir} (exit ${result.exitCode})` + (stderr ? `: ${stderr}` : '');
!!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) {
logger.debug(
`Rootless artifact permission repair skipped ${detail}. ` +
Expand Down
4 changes: 4 additions & 0 deletions src/artifact-preservation-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,13 @@ describe('artifact-preservation – error paths', () => {
'run',
'--pull',
'never',
'--entrypoint',
'sh',
'-v',
`/host${path.resolve(auditDir)}:/fix:rw`,
'ghcr.io/github/gh-aw-firewall/agent:latest',
'-c',
'chown -R "$TUID:$TGID" /fix 2>/dev/null; chmod -R a+rwX /fix',
]),
expect.objectContaining({ reject: false }),
);
Expand Down
Loading