Skip to content

Commit 5b1c63c

Browse files
Mossakaclaude
andauthored
feat(cli): add --skip-pull flag to use pre-downloaded images (#493)
* feat(cli): add --skip-pull flag to use pre-downloaded images Add a new --skip-pull CLI flag that prevents Docker Compose from pulling images from the registry, allowing users to use pre-downloaded or cached images locally. This is useful for: - Air-gapped environments where registry access is unavailable - CI systems with pre-warmed image caches - Local development when images are already cached When --skip-pull is enabled, Docker Compose runs with --pull never. If the required images are not available locally, container startup will fail with a clear error message. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: add tests for skipPull parameter in startContainers Add unit tests to verify: - --pull never is passed when skipPull is true - --pull never is not passed when skipPull is false Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address review comments for --skip-pull flag - Add validation to reject --skip-pull + --build-local combination since building images requires pulling base images - Add security warning when using --skip-pull to inform users about verifying image authenticity - Add documentation for --skip-pull in CLI reference: - Options Summary table entry - Detailed explanation with usage examples - Security caution about image verification - Note about incompatibility with --build-local Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: add tests for validateSkipPullWithBuildLocal function Extract flag validation logic into a testable function and add comprehensive tests to improve coverage on the new --skip-pull validation code. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: use validation function in CLI action handler Simplify the --skip-pull validation by using the extracted validateSkipPullWithBuildLocal function instead of inline checks. This reduces code duplication and improves coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: improve test coverage for docker-manager - Add test for when removing existing containers fails (covers catch block) - Add tests for allowHostPorts option in generateDockerCompose Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: improve coverage for skipPull and related features - Add test for container removal failure handling in startContainers - Add tests for allowHostPorts environment variable - Add tests for GOROOT/CARGO_HOME/JAVA_HOME passthrough in chroot mode These tests improve overall coverage from 82.15% to 82.37%, exceeding the baseline of 82.25%. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b2cff55 commit 5b1c63c

File tree

7 files changed

+243
-7
lines changed

7 files changed

+243
-7
lines changed

docs-site/src/content/docs/reference/cli-reference.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ awf [options] -- <command>
3232
| `--build-local` | flag | `false` | Build containers locally instead of pulling from registry |
3333
| `--image-registry <url>` | string | `ghcr.io/github/gh-aw-firewall` | Container image registry |
3434
| `--image-tag <tag>` | string | `latest` | Container image tag |
35+
| `--skip-pull` | flag | `false` | Use local images without pulling from registry |
3536
| `-e, --env <KEY=VALUE>` | string | `[]` | Environment variable (repeatable) |
3637
| `--env-all` | flag | `false` | Pass all host environment variables |
3738
| `-v, --mount <host:container[:mode]>` | string | `[]` | Volume mount (repeatable) |
@@ -181,6 +182,31 @@ Custom container image registry URL.
181182

182183
Container image tag to use.
183184

185+
### `--skip-pull`
186+
187+
Use local images without pulling from the registry. This is useful for:
188+
189+
- **Air-gapped environments** where registry access is unavailable
190+
- **CI systems with pre-warmed image caches** to avoid unnecessary network calls
191+
- **Local development** when images are already cached
192+
193+
```bash
194+
# Pre-pull images first
195+
docker pull ghcr.io/github/gh-aw-firewall/squid:latest
196+
docker pull ghcr.io/github/gh-aw-firewall/agent:latest
197+
198+
# Use with --skip-pull to avoid re-pulling
199+
sudo awf --skip-pull --allow-domains github.com -- curl https://api.github.com
200+
```
201+
202+
:::caution[Image Verification]
203+
When using `--skip-pull`, you are responsible for verifying image authenticity. The firewall cannot verify that locally cached images haven't been tampered with. See [Image Verification](/gh-aw-firewall/docs/image-verification/) for cosign verification instructions.
204+
:::
205+
206+
:::note[Incompatible with --build-local]
207+
The `--skip-pull` flag cannot be used with `--build-local` since building images requires pulling base images from the registry.
208+
:::
209+
184210
### `-e, --env <KEY=VALUE>`
185211

186212
Pass environment variable to container. Can be specified multiple times.

src/cli-workflow.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ export interface WorkflowDependencies {
44
ensureFirewallNetwork: () => Promise<{ squidIp: string }>;
55
setupHostIptables: (squidIp: string, port: number, dnsServers: string[]) => Promise<void>;
66
writeConfigs: (config: WrapperConfig) => Promise<void>;
7-
startContainers: (workDir: string, allowedDomains: string[], proxyLogsDir?: string) => Promise<void>;
7+
startContainers: (workDir: string, allowedDomains: string[], proxyLogsDir?: string, skipPull?: boolean) => Promise<void>;
88
runAgentCommand: (
99
workDir: string,
1010
allowedDomains: string[],
@@ -51,7 +51,7 @@ export async function runMainWorkflow(
5151
await dependencies.writeConfigs(config);
5252

5353
// Step 2: Start containers
54-
await dependencies.startContainers(config.workDir, config.allowedDomains, config.proxyLogsDir);
54+
await dependencies.startContainers(config.workDir, config.allowedDomains, config.proxyLogsDir, config.skipPull);
5555
onContainersStarted?.();
5656

5757
// Step 3: Wait for agent to complete

src/cli.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Command } from 'commander';
2-
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption } from './cli';
2+
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, validateSkipPullWithBuildLocal } from './cli';
33
import { redactSecrets } from './redact-secrets';
44
import * as fs from 'fs';
55
import * as path from 'path';
@@ -666,6 +666,7 @@ describe('cli', () => {
666666
expect(result.invalidMount).toBe('invalid-mount');
667667
}
668668
});
669+
669670
});
670671

671672
describe('IPv4 validation', () => {
@@ -1140,4 +1141,48 @@ describe('cli', () => {
11401141
});
11411142
});
11421143
});
1144+
1145+
describe('validateSkipPullWithBuildLocal', () => {
1146+
it('should return valid when both flags are false', () => {
1147+
const result = validateSkipPullWithBuildLocal(false, false);
1148+
expect(result.valid).toBe(true);
1149+
expect(result.error).toBeUndefined();
1150+
});
1151+
1152+
it('should return valid when both flags are undefined', () => {
1153+
const result = validateSkipPullWithBuildLocal(undefined, undefined);
1154+
expect(result.valid).toBe(true);
1155+
expect(result.error).toBeUndefined();
1156+
});
1157+
1158+
it('should return valid when only skipPull is true', () => {
1159+
const result = validateSkipPullWithBuildLocal(true, false);
1160+
expect(result.valid).toBe(true);
1161+
expect(result.error).toBeUndefined();
1162+
});
1163+
1164+
it('should return valid when only buildLocal is true', () => {
1165+
const result = validateSkipPullWithBuildLocal(false, true);
1166+
expect(result.valid).toBe(true);
1167+
expect(result.error).toBeUndefined();
1168+
});
1169+
1170+
it('should return invalid when both skipPull and buildLocal are true', () => {
1171+
const result = validateSkipPullWithBuildLocal(true, true);
1172+
expect(result.valid).toBe(false);
1173+
expect(result.error).toContain('--skip-pull cannot be used with --build-local');
1174+
});
1175+
1176+
it('should return valid when skipPull is true and buildLocal is undefined', () => {
1177+
const result = validateSkipPullWithBuildLocal(true, undefined);
1178+
expect(result.valid).toBe(true);
1179+
expect(result.error).toBeUndefined();
1180+
});
1181+
1182+
it('should return valid when skipPull is undefined and buildLocal is true', () => {
1183+
const result = validateSkipPullWithBuildLocal(undefined, true);
1184+
expect(result.valid).toBe(true);
1185+
expect(result.error).toBeUndefined();
1186+
});
1187+
});
11431188
});

src/cli.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,35 @@ export function processAgentImageOption(
243243
};
244244
}
245245

246+
/**
247+
* Result of validating flag combinations
248+
*/
249+
export interface FlagValidationResult {
250+
/** Whether the validation passed */
251+
valid: boolean;
252+
/** Error message if validation failed */
253+
error?: string;
254+
}
255+
256+
/**
257+
* Validates that --skip-pull is not used with --build-local
258+
* @param skipPull - Whether --skip-pull flag was provided
259+
* @param buildLocal - Whether --build-local flag was provided
260+
* @returns FlagValidationResult with validation status and error message
261+
*/
262+
export function validateSkipPullWithBuildLocal(
263+
skipPull: boolean | undefined,
264+
buildLocal: boolean | undefined
265+
): FlagValidationResult {
266+
if (skipPull && buildLocal) {
267+
return {
268+
valid: false,
269+
error: '--skip-pull cannot be used with --build-local. Building images requires pulling base images from the registry.',
270+
};
271+
}
272+
return { valid: true };
273+
}
274+
246275
/**
247276
* Parses and validates DNS servers from a comma-separated string
248277
* @param input - Comma-separated DNS server string (e.g., "8.8.8.8,1.1.1.1")
@@ -507,6 +536,11 @@ program
507536
'Container image tag',
508537
'latest'
509538
)
539+
.option(
540+
'--skip-pull',
541+
'Use local images without pulling from registry (requires images to be pre-downloaded)',
542+
false
543+
)
510544
.option(
511545
'-e, --env <KEY=VALUE>',
512546
'Additional environment variables to pass to container (can be specified multiple times)',
@@ -788,6 +822,7 @@ program
788822
tty: options.tty || false,
789823
workDir: options.workDir,
790824
buildLocal: options.buildLocal,
825+
skipPull: options.skipPull,
791826
agentImage,
792827
imageRegistry: options.imageRegistry,
793828
imageTag: options.imageTag,
@@ -816,6 +851,13 @@ program
816851
process.exit(1);
817852
}
818853

854+
// Error if --skip-pull is used with --build-local (incompatible flags)
855+
const skipPullValidation = validateSkipPullWithBuildLocal(config.skipPull, config.buildLocal);
856+
if (!skipPullValidation.valid) {
857+
logger.error(`❌ ${skipPullValidation.error}`);
858+
process.exit(1);
859+
}
860+
819861
// Warn if --enable-host-access is used with host.docker.internal in allowed domains
820862
if (config.enableHostAccess) {
821863
const hasHostDomain = allowedDomains.some(d =>

src/docker-manager.test.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,47 @@ describe('docker-manager', () => {
623623
expect(environment.AWF_CHROOT_ENABLED).toBe('true');
624624
});
625625

626+
it('should pass GOROOT, CARGO_HOME, JAVA_HOME to container when enableChroot is true and env vars are set', () => {
627+
const originalGoroot = process.env.GOROOT;
628+
const originalCargoHome = process.env.CARGO_HOME;
629+
const originalJavaHome = process.env.JAVA_HOME;
630+
631+
process.env.GOROOT = '/usr/local/go';
632+
process.env.CARGO_HOME = '/home/user/.cargo';
633+
process.env.JAVA_HOME = '/usr/lib/jvm/java-17';
634+
635+
try {
636+
const configWithChroot = {
637+
...mockConfig,
638+
enableChroot: true
639+
};
640+
const result = generateDockerCompose(configWithChroot, mockNetworkConfig);
641+
const agent = result.services.agent;
642+
const environment = agent.environment as Record<string, string>;
643+
644+
expect(environment.AWF_GOROOT).toBe('/usr/local/go');
645+
expect(environment.AWF_CARGO_HOME).toBe('/home/user/.cargo');
646+
expect(environment.AWF_JAVA_HOME).toBe('/usr/lib/jvm/java-17');
647+
} finally {
648+
// Restore original values
649+
if (originalGoroot !== undefined) {
650+
process.env.GOROOT = originalGoroot;
651+
} else {
652+
delete process.env.GOROOT;
653+
}
654+
if (originalCargoHome !== undefined) {
655+
process.env.CARGO_HOME = originalCargoHome;
656+
} else {
657+
delete process.env.CARGO_HOME;
658+
}
659+
if (originalJavaHome !== undefined) {
660+
process.env.JAVA_HOME = originalJavaHome;
661+
} else {
662+
delete process.env.JAVA_HOME;
663+
}
664+
}
665+
});
666+
626667
it('should not set AWF_CHROOT_ENABLED when enableChroot is false', () => {
627668
const result = generateDockerCompose(mockConfig, mockNetworkConfig);
628669
const agent = result.services.agent;
@@ -932,6 +973,24 @@ describe('docker-manager', () => {
932973
});
933974
});
934975

976+
describe('allowHostPorts option', () => {
977+
it('should set AWF_ALLOW_HOST_PORTS when allowHostPorts is specified', () => {
978+
const config = { ...mockConfig, enableHostAccess: true, allowHostPorts: '8080,3000' };
979+
const result = generateDockerCompose(config, mockNetworkConfig);
980+
const env = result.services.agent.environment as Record<string, string>;
981+
982+
expect(env.AWF_ALLOW_HOST_PORTS).toBe('8080,3000');
983+
});
984+
985+
it('should NOT set AWF_ALLOW_HOST_PORTS when allowHostPorts is undefined', () => {
986+
const config = { ...mockConfig, enableHostAccess: true };
987+
const result = generateDockerCompose(config, mockNetworkConfig);
988+
const env = result.services.agent.environment as Record<string, string>;
989+
990+
expect(env.AWF_ALLOW_HOST_PORTS).toBeUndefined();
991+
});
992+
});
993+
935994
it('should override environment variables with additionalEnv', () => {
936995
const originalEnv = process.env.GITHUB_TOKEN;
937996
process.env.GITHUB_TOKEN = 'original_token';
@@ -1246,6 +1305,22 @@ describe('docker-manager', () => {
12461305
);
12471306
});
12481307

1308+
it('should continue when removing existing containers fails', async () => {
1309+
// First call (docker rm) throws an error, but we should continue
1310+
mockExecaFn.mockRejectedValueOnce(new Error('No such container'));
1311+
// Second call (docker compose up) succeeds
1312+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
1313+
1314+
await startContainers(testDir, ['github.com']);
1315+
1316+
// Should still call docker compose up even if rm failed
1317+
expect(mockExecaFn).toHaveBeenCalledWith(
1318+
'docker',
1319+
['compose', 'up', '-d'],
1320+
{ cwd: testDir, stdio: 'inherit' }
1321+
);
1322+
});
1323+
12491324
it('should run docker compose up', async () => {
12501325
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
12511326
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
@@ -1259,6 +1334,32 @@ describe('docker-manager', () => {
12591334
);
12601335
});
12611336

1337+
it('should run docker compose up with --pull never when skipPull is true', async () => {
1338+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
1339+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
1340+
1341+
await startContainers(testDir, ['github.com'], undefined, true);
1342+
1343+
expect(mockExecaFn).toHaveBeenCalledWith(
1344+
'docker',
1345+
['compose', 'up', '-d', '--pull', 'never'],
1346+
{ cwd: testDir, stdio: 'inherit' }
1347+
);
1348+
});
1349+
1350+
it('should run docker compose up without --pull never when skipPull is false', async () => {
1351+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
1352+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
1353+
1354+
await startContainers(testDir, ['github.com'], undefined, false);
1355+
1356+
expect(mockExecaFn).toHaveBeenCalledWith(
1357+
'docker',
1358+
['compose', 'up', '-d'],
1359+
{ cwd: testDir, stdio: 'inherit' }
1360+
);
1361+
});
1362+
12621363
it('should handle docker compose failure', async () => {
12631364
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
12641365
mockExecaFn.mockRejectedValueOnce(new Error('Docker compose failed'));

src/docker-manager.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -800,8 +800,12 @@ async function checkSquidLogs(workDir: string, proxyLogsDir?: string): Promise<{
800800

801801
/**
802802
* Starts Docker Compose services
803+
* @param workDir - Working directory containing Docker Compose config
804+
* @param allowedDomains - List of allowed domains for error reporting
805+
* @param proxyLogsDir - Optional custom directory for proxy logs
806+
* @param skipPull - If true, use local images without pulling from registry
803807
*/
804-
export async function startContainers(workDir: string, allowedDomains: string[], proxyLogsDir?: string): Promise<void> {
808+
export async function startContainers(workDir: string, allowedDomains: string[], proxyLogsDir?: string, skipPull?: boolean): Promise<void> {
805809
logger.info('Starting containers...');
806810

807811
// Force remove any existing containers with these names to avoid conflicts
@@ -817,7 +821,12 @@ export async function startContainers(workDir: string, allowedDomains: string[],
817821
}
818822

819823
try {
820-
await execa('docker', ['compose', 'up', '-d'], {
824+
const composeArgs = ['compose', 'up', '-d'];
825+
if (skipPull) {
826+
composeArgs.push('--pull', 'never');
827+
logger.debug('Using --pull never (skip-pull mode)');
828+
}
829+
await execa('docker', composeArgs, {
821830
cwd: workDir,
822831
stdio: 'inherit',
823832
});

src/types.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,28 @@ export interface WrapperConfig {
138138

139139
/**
140140
* Whether to build container images locally instead of pulling from registry
141-
*
141+
*
142142
* When true, Docker images are built from local Dockerfiles in containers/squid
143143
* and containers/agent directories. When false (default), images are pulled
144144
* from the configured registry.
145-
*
145+
*
146146
* @default false
147147
*/
148148
buildLocal?: boolean;
149149

150+
/**
151+
* Whether to skip pulling images from the registry
152+
*
153+
* When true, Docker Compose will use locally available images without
154+
* attempting to pull from the registry. This is useful when images are
155+
* pre-downloaded or in air-gapped environments.
156+
*
157+
* If the required images are not available locally, container startup will fail.
158+
*
159+
* @default false
160+
*/
161+
skipPull?: boolean;
162+
150163
/**
151164
* Agent container image preset or custom base image
152165
*

0 commit comments

Comments
 (0)