fix: address review feedback on predownload command

- Validate custom agentImage to reject leading dashes and whitespace
- Replace process.exit(1) with thrown error with exitCode for testability
- Update --image-tag help text to clarify it applies to all images
- Add tests for image reference validation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Mossaka

src/cli.ts

@@ -1490,12 +1490,17 @@ export async function handlePredownloadAction(options: {
   enableApiProxy: boolean;
 }): Promise<void> {
   const { predownloadCommand } = await import('./commands/predownload');
-  await predownloadCommand({
-    imageRegistry: options.imageRegistry,
-    imageTag: options.imageTag,
-    agentImage: options.agentImage,
-    enableApiProxy: options.enableApiProxy,
-  });
+  try {
+    await predownloadCommand({
+      imageRegistry: options.imageRegistry,
+      imageTag: options.imageTag,
+      agentImage: options.agentImage,
+      enableApiProxy: options.enableApiProxy,
+    });
+  } catch (error) {
+    const exitCode = (error as Error & { exitCode?: number }).exitCode ?? 1;
+    process.exit(exitCode);
+  }
 }
 
 // Predownload subcommand - pre-pull container images
@@ -1507,7 +1512,7 @@ program
     'Container image registry',
     'ghcr.io/github/gh-aw-firewall'
   )
-  .option('--image-tag <tag>', 'Container image tag', 'latest')
+  .option('--image-tag <tag>', 'Container image tag (applies to squid, agent, and api-proxy images)', 'latest')
   .option(
     '--agent-image <value>',
     'Agent image preset (default, act) or custom image',

src/commands/predownload.test.ts

@@ -62,6 +62,18 @@ describe('predownload', () => {
         'ubuntu:22.04',
       ]);
     });
+
+    it('should reject custom image starting with dash', () => {
+      expect(() => resolveImages({ ...defaults, agentImage: '--help' })).toThrow(
+        'must not start with "-"',
+      );
+    });
+
+    it('should reject custom image containing whitespace', () => {
+      expect(() => resolveImages({ ...defaults, agentImage: 'ubuntu 22.04' })).toThrow(
+        'must not contain whitespace',
+      );
+    });
   });
 
   describe('predownloadCommand', () => {
@@ -104,49 +116,41 @@ describe('predownload', () => {
       );
     });
 
-    it('should exit with code 1 when a pull fails', async () => {
-      const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {
-        throw new Error('process.exit called');
-      });
+    it('should throw with exitCode 1 when a pull fails', async () => {
       execa
         .mockResolvedValueOnce({ stdout: '', stderr: '' })
         .mockRejectedValueOnce(new Error('pull failed'));
 
-      await expect(predownloadCommand(defaults)).rejects.toThrow(
-        'process.exit called',
-      );
-
-      expect(mockExit).toHaveBeenCalledWith(1);
-      mockExit.mockRestore();
+      try {
+        await predownloadCommand(defaults);
+        fail('Expected predownloadCommand to throw');
+      } catch (error) {
+        expect((error as Error).message).toBe('1 of 2 image(s) failed to pull');
+        expect((error as Error & { exitCode?: number }).exitCode).toBe(1);
+      }
     });
 
     it('should continue pulling remaining images after a failure', async () => {
-      const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {
-        throw new Error('process.exit called');
-      });
       execa.mockRejectedValueOnce(new Error('pull failed')).mockResolvedValueOnce({ stdout: '', stderr: '' });
 
       await expect(predownloadCommand(defaults)).rejects.toThrow(
-        'process.exit called',
+        '1 of 2 image(s) failed to pull',
       );
 
       // Both images should have been attempted
       expect(execa).toHaveBeenCalledTimes(2);
-      mockExit.mockRestore();
     });
 
     it('should handle non-Error rejection', async () => {
-      const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {
-        throw new Error('process.exit called');
-      });
       execa.mockRejectedValueOnce('string error').mockResolvedValueOnce({ stdout: '', stderr: '' });
 
-      await expect(predownloadCommand(defaults)).rejects.toThrow(
-        'process.exit called',
-      );
-
-      expect(mockExit).toHaveBeenCalledWith(1);
-      mockExit.mockRestore();
+      try {
+        await predownloadCommand(defaults);
+        fail('Expected predownloadCommand to throw');
+      } catch (error) {
+        expect((error as Error).message).toBe('1 of 2 image(s) failed to pull');
+        expect((error as Error & { exitCode?: number }).exitCode).toBe(1);
+      }
     });
   });
 });

src/commands/predownload.ts

@@ -8,6 +8,19 @@ export interface PredownloadOptions {
   enableApiProxy: boolean;
 }
 
+/**
+ * Validates a custom Docker image reference.
+ * Rejects values that could be interpreted as Docker CLI flags or contain whitespace.
+ */
+function validateImageReference(image: string): void {
+  if (image.startsWith('-')) {
+    throw new Error(`Invalid image reference "${image}": must not start with "-"`);
+  }
+  if (/\s/.test(image)) {
+    throw new Error(`Invalid image reference "${image}": must not contain whitespace`);
+  }
+}
+
 /**
  * Resolves the list of image references to pull based on the given options.
  */
@@ -24,7 +37,8 @@ export function resolveImages(options: PredownloadOptions): string[] {
     const imageName = agentImage === 'act' ? 'agent-act' : 'agent';
     images.push(`${imageRegistry}/${imageName}:${imageTag}`);
   } else {
-    // Custom image - pull as-is
+    // Custom image - validate and pull as-is
+    validateImageReference(agentImage);
     images.push(agentImage);
   }
 
@@ -57,8 +71,11 @@ export async function predownloadCommand(options: PredownloadOptions): Promise<v
   }
 
   if (failed > 0) {
-    logger.error(`${failed} of ${images.length} image(s) failed to pull`);
-    process.exit(1);
+    const message = `${failed} of ${images.length} image(s) failed to pull`;
+    logger.error(message);
+    const error: Error & { exitCode?: number } = new Error(message);
+    error.exitCode = 1;
+    throw error;
   }
 
   logger.info(`All ${images.length} image(s) pre-downloaded successfully`);