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
2 changes: 1 addition & 1 deletion containers/api-proxy/blocked-request-diagnostics.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ function getDiagStream() {
if (diagStream) return diagStream;
try {
fs.mkdirSync(TOKEN_LOG_DIR, { recursive: true });
diagStream = fs.createWriteStream(DIAG_FILE, { flags: 'a' });
diagStream = fs.createWriteStream(DIAG_FILE, { flags: 'a', mode: 0o644 });
diagStream.on('error', () => { diagStream = null; });
return diagStream;
} catch {
Expand Down
2 changes: 1 addition & 1 deletion containers/api-proxy/otel-exporters.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class FileSpanExporter {
_getStream() {
if (this._stream) return this._stream;
try {
this._stream = fs.createWriteStream(this._filePath, { flags: 'a' });
this._stream = fs.createWriteStream(this._filePath, { flags: 'a', mode: 0o644 });
this._stream.on('error', () => { this._stream = null; });
} catch { return null; }
return this._stream;
Expand Down
4 changes: 2 additions & 2 deletions containers/api-proxy/token-persistence.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function diag(msg, data) {
try {
if (!diagStream) {
fs.mkdirSync(TOKEN_LOG_DIR, { recursive: true });
diagStream = fs.createWriteStream(DIAG_LOG_FILE, { flags: 'a' });
diagStream = fs.createWriteStream(DIAG_LOG_FILE, { flags: 'a', mode: 0o644 });
diagStream.on('error', () => { diagStream = null; });
}
const record = buildTokenDiagRecord(msg, data);
Expand All @@ -81,7 +81,7 @@ function getLogStream() {
try {
// Ensure directory exists
fs.mkdirSync(TOKEN_LOG_DIR, { recursive: true });
logStream = fs.createWriteStream(TOKEN_LOG_FILE, { flags: 'a' });
logStream = fs.createWriteStream(TOKEN_LOG_FILE, { flags: 'a', mode: 0o644 });
logStream.on('error', (err) => {
logRequest('warn', 'token_log_error', { error: err.message });
logStream = null;
Expand Down
2 changes: 1 addition & 1 deletion containers/cli-proxy/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const CLI_PROXY_ACCESS_SCHEMA = `cli-proxy-access/v${AWF_VERSION}`;
let logStream = null;
try {
if (fs.existsSync(LOG_DIR)) {
logStream = fs.createWriteStream(LOG_FILE, { flags: 'a' });
logStream = fs.createWriteStream(LOG_FILE, { flags: 'a', mode: 0o644 });
}
} catch {
// Non-fatal: logging to file is best-effort
Expand Down
75 changes: 75 additions & 0 deletions src/artifact-preservation-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ describe('artifact-preservation – error paths', () => {
// ─── preserveCleanupArtifacts ───────────────────────────────────────────

describe('preserveCleanupArtifacts', () => {
let getuidSpy: jest.SpyInstance<number | undefined, []> | undefined;

afterEach(() => {
getuidSpy?.mockRestore();
getuidSpy = undefined;
});

it('does not throw when agent-logs renameSync fails (line 101)', () => {
const workDir = makeTempDir();
try {
Expand Down Expand Up @@ -160,6 +167,74 @@ describe('artifact-preservation – error paths', () => {
}
});

it('runs rootless permission repair with translated mount paths', () => {
const auditDir = makeTempDir('awf-audit-');
const workDir = makeTempDir();
try {
getuidSpy = jest.spyOn(process, 'getuid').mockReturnValue(1001);
expect(() => preserveCleanupArtifacts(workDir, {
auditDir,
dockerHostPathPrefix: '/host',
imageRegistry: 'ghcr.io/github/gh-aw-firewall',
imageTag: 'latest',
})).not.toThrow();

expect(mockExecaSync).toHaveBeenCalledWith(
'docker',
expect.arrayContaining([
'run',
'--pull',
'never',
'-v',
`/host${path.resolve(auditDir)}:/fix:rw`,
'ghcr.io/github/gh-aw-firewall/agent:latest',
]),
expect.objectContaining({ reject: false }),
);
} finally {
realFs.rmSync(auditDir, { recursive: true, force: true });
realFs.rmSync(workDir, { recursive: true, force: true });
}
});

it('uses agent-act image when agentImage is act', () => {
const auditDir = makeTempDir('awf-audit-');
const workDir = makeTempDir();
try {
getuidSpy = jest.spyOn(process, 'getuid').mockReturnValue(1001);
expect(() => preserveCleanupArtifacts(workDir, {
auditDir,
imageRegistry: 'ghcr.io/github/gh-aw-firewall',
imageTag: 'latest',
agentImage: 'act',
})).not.toThrow();

expect(mockExecaSync).toHaveBeenCalledWith(
'docker',
expect.arrayContaining([
'ghcr.io/github/gh-aw-firewall/agent-act:latest',
]),
expect.objectContaining({ reject: false }),
);
} finally {
realFs.rmSync(auditDir, { recursive: true, force: true });
realFs.rmSync(workDir, { recursive: true, force: true });
}
});

it('skips rootless permission repair when running as root', () => {
const auditDir = makeTempDir('awf-audit-');
const workDir = makeTempDir();
try {
getuidSpy = jest.spyOn(process, 'getuid').mockReturnValue(0);
expect(() => preserveCleanupArtifacts(workDir, { auditDir })).not.toThrow();
expect(mockExecaSync.mock.calls.some(call => call[0] === 'docker')).toBe(false);
} finally {
realFs.rmSync(auditDir, { recursive: true, force: true });
realFs.rmSync(workDir, { recursive: true, force: true });
}
});

it('preserves default audit dir to /tmp when no auditDir arg is provided (lines 170-173)', () => {
const workDir = makeTempDir();
const timestamp = path.basename(workDir).replace('awf-', '');
Expand Down
99 changes: 96 additions & 3 deletions src/artifact-preservation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
import * as path from 'path';
import * as os from 'os';
import execa from 'execa';
import { getSafeHostGid, getSafeHostUid } from './host-identity';
import { buildRuntimeImageRef, parseImageTag } from './image-tag';
import { logger } from './logger';
import { applyHostPathPrefixToVolumes } from './services/host-path-prefix';
import { getLocalDockerEnv } from './docker-host';

/**
* Copies the iptables audit dump from the init-signal volume to the audit directory.
Expand All @@ -12,10 +16,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)) {

Check warning on line 19 in src/artifact-preservation.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 19 in src/artifact-preservation.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 19 in src/artifact-preservation.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

Check warning on line 19 in src/artifact-preservation.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

Check warning on line 19 in src/artifact-preservation.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 19 in src/artifact-preservation.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
try {
fs.copyFileSync(iptablesAuditSrc, path.join(targetAuditDir, 'iptables-audit.txt'));
fs.chmodSync(path.join(targetAuditDir, 'iptables-audit.txt'), 0o644);

Check warning on line 22 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 22 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

Check warning on line 22 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found chmodSync from package "fs" with non literal argument at index 0
logger.debug('Copied iptables audit state to audit directory');
} catch (error) {
logger.debug('Could not copy iptables audit file:', error);
Expand Down Expand Up @@ -54,12 +58,12 @@
}: PreserveDirectoryOptions): void {
if (runtimeDir) {
const targetDir = runtimeSubdir ? path.join(runtimeDir, runtimeSubdir) : runtimeDir;
if (!runtimeDirMustExist || fs.existsSync(targetDir)) {

Check warning on line 61 in src/artifact-preservation.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 61 in src/artifact-preservation.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

Check warning on line 61 in src/artifact-preservation.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
try {
execa.sync('chmod', ['-R', 'a+rX', targetDir]);
logger.info(`${availableLabel} available at: ${targetDir}`);
} catch (error) {
logger.debug(permissionErrorMessage, error);
logger.warn(permissionErrorMessage, error);
}
}
return;
Expand All @@ -67,9 +71,9 @@

const sourceDir = path.join(workDir, workSubdir);
const destinationDir = path.join(os.tmpdir(), `${destinationBaseName}-${timestamp}`);
if (fs.existsSync(sourceDir) && fs.readdirSync(sourceDir).length > 0) {

Check warning on line 74 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 74 in src/artifact-preservation.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 74 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

Check warning on line 74 in src/artifact-preservation.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

Check warning on line 74 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 74 in src/artifact-preservation.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
try {
fs.renameSync(sourceDir, destinationDir);

Check warning on line 76 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / ESLint

Found renameSync from package "fs" with non literal argument at index 0,1

Check warning on line 76 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

Found renameSync from package "fs" with non literal argument at index 0,1

Check warning on line 76 in src/artifact-preservation.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

Found renameSync from package "fs" with non literal argument at index 0,1
if (chmodPreservedDir) {
execa.sync('chmod', ['-R', 'a+rX', destinationDir]);
}
Expand All @@ -84,11 +88,92 @@
proxyLogsDir?: string;
auditDir?: string;
sessionStateDir?: string;
dockerHostPathPrefix?: string;
imageRegistry?: string;
imageTag?: string;
agentImage?: string;
};

function resolvePermFixerImageRef(imageRegistry?: string, imageTag?: string, agentImage?: string): string {
try {
const registry = imageRegistry || 'ghcr.io/github/gh-aw-firewall';
const parsedImageTag = parseImageTag(imageTag || 'latest');
const imageName = agentImage === 'act' ? 'agent-act' : 'agent';
return buildRuntimeImageRef(registry, imageName, parsedImageTag);
} catch {
return 'ghcr.io/github/gh-aw-firewall/agent:latest';
}
}

function fixArtifactPermissionsForRootless(
dirs: Array<string | undefined>,
dockerHostPathPrefix: string | undefined,
imageRegistry: string | undefined,
imageTag: string | undefined,
agentImage: string | undefined,
): void {
const currentUid = process.getuid?.();
if (currentUid === undefined || currentUid === 0) {
return;
}

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

Check warning on line 121 in src/artifact-preservation.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 121 in src/artifact-preservation.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

Check warning on line 121 in src/artifact-preservation.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
);
if (existingDirs.length === 0) {
return;
}

const uid = getSafeHostUid();
const gid = getSafeHostGid();
const imageRef = resolvePermFixerImageRef(imageRegistry, imageTag, agentImage);

for (const dir of existingDirs) {
const mount = applyHostPathPrefixToVolumes([`${path.resolve(dir)}:/fix:rw`], dockerHostPathPrefix)[0];
try {
const result = execa.sync(
'docker',
[
'run',
'--rm',
'--pull',
'never',
'--network',
'none',
'--cap-drop',
'ALL',
'--cap-add',
'CHOWN',
'--cap-add',
'DAC_OVERRIDE',
'--cap-add',
'FOWNER',
'-e',
`TUID=${uid}`,

Check warning on line 152 in src/artifact-preservation.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 152 in src/artifact-preservation.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

Check warning on line 152 in src/artifact-preservation.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
'-e',
`TGID=${gid}`,

Check warning on line 154 in src/artifact-preservation.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 154 in src/artifact-preservation.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

Check warning on line 154 in src/artifact-preservation.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
'-v',
mount,
imageRef,
'sh',
'-c',
'chown -R "$TUID:$TGID" /fix && chmod -R a+rX /fix',
],
{ env: getLocalDockerEnv(), reject: false },
);

if (typeof result.exitCode === 'number' && result.exitCode !== 0) {
logger.warn(`Rootless artifact permission repair failed for ${dir} (exit ${result.exitCode})`);
}
} catch (error) {
logger.warn(`Rootless artifact permission repair failed for ${dir}:`, error);
}
}
}

export function preserveCleanupArtifacts(
workDir: string,
{ proxyLogsDir, auditDir, sessionStateDir }: PreserveCleanupArtifactsOptions = {},
{ proxyLogsDir, auditDir, sessionStateDir, dockerHostPathPrefix, imageRegistry, imageTag, agentImage }: PreserveCleanupArtifactsOptions = {},
): void {
const timestamp = path.basename(workDir).replace('awf-', '');
const agentLogsDestination = path.join(os.tmpdir(), `awf-agent-logs-${timestamp}`);
Expand Down Expand Up @@ -160,7 +245,7 @@
execa.sync('chmod', ['-R', 'a+rX', auditDir]);
logger.info(`Audit artifacts available at: ${auditDir}`);
} catch (error) {
logger.debug('Could not fix audit dir permissions:', error);
logger.warn('Could not fix audit dir permissions as non-root user; rootless repair will be attempted:', error);
}
}
} else {
Expand Down Expand Up @@ -205,6 +290,14 @@
}
}
}

fixArtifactPermissionsForRootless(
[proxyLogsDir, auditDir, sessionStateDir],
dockerHostPathPrefix,
imageRegistry,
imageTag,
agentImage,
);
}

export function removeWorkDirectories(workDir: string): void {
Expand Down
5 changes: 4 additions & 1 deletion src/commands/main-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,10 @@ describe('createMainAction', () => {
false,
STUB_CONFIG.proxyLogsDir,
STUB_CONFIG.auditDir,
STUB_CONFIG.sessionStateDir
STUB_CONFIG.sessionStateDir,
STUB_CONFIG.dockerHostPathPrefix,
STUB_CONFIG.imageRegistry,
STUB_CONFIG.imageTag,
);
expect(mockedHostIptables.cleanupHostIptables).not.toHaveBeenCalled();
expect(processExitSpy).toHaveBeenCalledWith(1);
Expand Down
12 changes: 11 additions & 1 deletion src/commands/main-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,17 @@ export function createMainAction(getOptionValueSource: OptionSourceResolver) {
}

if (!config.keepContainers) {
await cleanup(config.workDir, false, config.proxyLogsDir, config.auditDir, config.sessionStateDir);
await cleanup(
config.workDir,
false,
config.proxyLogsDir,
config.auditDir,
config.sessionStateDir,
config.dockerHostPathPrefix,
config.imageRegistry,
config.imageTag,
config.agentImage,
);
// Note: We don't remove the firewall network here since it can be reused
// across multiple runs. Cleanup script will handle removal if needed.
} else {
Expand Down
14 changes: 13 additions & 1 deletion src/container-cleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export async function cleanup(
proxyLogsDir?: string,
auditDir?: string,
sessionStateDir?: string,
dockerHostPathPrefix?: string,
imageRegistry?: string,
imageTag?: string,
agentImage?: string,
): Promise<void> {
if (keepFiles) {
logger.debug(`Keeping temporary files in: ${workDir}`);
Expand All @@ -29,7 +33,15 @@ export async function cleanup(
return;
}

preserveCleanupArtifacts(workDir, { proxyLogsDir, auditDir, sessionStateDir });
preserveCleanupArtifacts(workDir, {
proxyLogsDir,
auditDir,
sessionStateDir,
dockerHostPathPrefix,
imageRegistry,
imageTag,
agentImage,
});

cleanupSslKeyMaterial(workDir);

Expand Down
4 changes: 3 additions & 1 deletion src/services/api-proxy-service-config.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { generateDockerCompose, WrapperConfig, baseConfig, mockNetworkConfig, useTempWorkDir } from './service-test-setup.test-utils';
import { mockNetworkConfigWithProxy } from './api-proxy-service.test-utils';
import { getSafeHostGid, getSafeHostUid } from '../host-identity';

// Create mock functions (must remain per-file — jest.mock() is hoisted before imports)

Expand Down Expand Up @@ -33,8 +34,9 @@ describe('API proxy sidecar: service configuration', () => {
const configWithProxy = { ...mockConfig, enableApiProxy: true, openaiApiKey: 'sk-test-openai-key' };
const result = generateDockerCompose(configWithProxy, mockNetworkConfigWithProxy);
expect(result.services['api-proxy']).toBeDefined();
const proxy = result.services['api-proxy'];
const proxy = result.services['api-proxy'] as any;
expect(proxy.container_name).toBe('awf-api-proxy');
expect(proxy.user).toBe(`${getSafeHostUid()}:${getSafeHostGid()}`);
expect((proxy.networks as any)['awf-net'].ipv4_address).toBe('172.30.0.30');
});

Expand Down
2 changes: 2 additions & 0 deletions src/services/api-proxy-service-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
} from '../constants';
import { assignImageSource } from '../image-tag';
import { WrapperConfig } from '../types';
import { getSafeHostGid, getSafeHostUid } from '../host-identity';
import { NetworkConfig, ImageBuildConfig } from './squid-service';
import { applyHostPathPrefixToVolumes } from './host-path-prefix';
import { buildContainerSecurityHardening } from './service-security';
Expand All @@ -25,6 +26,7 @@ export function buildApiProxyServiceConfig(params: ApiProxyServiceConfigParams):

const proxyService: any = {
container_name: API_PROXY_CONTAINER_NAME,
user: `${getSafeHostUid()}:${getSafeHostGid()}`,
...buildApiProxyLifecycleConfig(networkConfig),
volumes: applyHostPathPrefixToVolumes(
[
Expand Down
3 changes: 2 additions & 1 deletion src/services/cli-proxy-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ describe('CLI proxy sidecar (external DIFC proxy)', () => {
const configWithCliProxy = { ...mockConfig, difcProxyHost: 'host.docker.internal:18443' };
const result = generateDockerCompose(configWithCliProxy, mockNetworkConfigWithCliProxy);
expect(result.services['cli-proxy']).toBeDefined();
const proxy = result.services['cli-proxy'];
const proxy = result.services['cli-proxy'] as any;
expect(proxy.container_name).toBe('awf-cli-proxy');
expect(proxy.user).toBeUndefined();
// cli-proxy gets its own IP on awf-net (no shared network namespace)
expect((proxy.networks as any)['awf-net'].ipv4_address).toBe('172.30.0.50');
expect(proxy.network_mode).toBeUndefined();
Expand Down
Loading