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
46 changes: 45 additions & 1 deletion src/cli.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Command } from 'commander';
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags } from './cli';
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateAllowHostPorts, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags } from './cli';
import { redactSecrets } from './redact-secrets';
import * as fs from 'fs';
import * as path from 'path';
Expand Down Expand Up @@ -48,14 +48,14 @@

afterEach(() => {
// Clean up the test directory
if (fs.existsSync(testDir)) {

Check warning on line 51 in src/cli.test.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 51 in src/cli.test.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 51 in src/cli.test.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
fs.rmSync(testDir, { recursive: true, force: true });
}
});

it('should parse domains from file with one domain per line', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\napi.github.com\nnpmjs.org');

Check warning on line 58 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 58 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 58 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -64,7 +64,7 @@

it('should parse comma-separated domains from file', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com, api.github.com, npmjs.org');

Check warning on line 67 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 67 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 67 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -73,7 +73,7 @@

it('should handle mixed formats (lines and commas)', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\napi.github.com, npmjs.org\nexample.com');

Check warning on line 76 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 76 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 76 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -82,7 +82,7 @@

it('should skip empty lines', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\n\n\napi.github.com\n\nnpmjs.org');

Check warning on line 85 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 85 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 85 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -91,7 +91,7 @@

it('should skip lines with only whitespace', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com\n \n\t\napi.github.com');

Check warning on line 94 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 94 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 94 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -100,7 +100,7 @@

it('should skip comments starting with #', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, '# This is a comment\ngithub.com\n# Another comment\napi.github.com');

Check warning on line 103 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 103 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 103 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -109,7 +109,7 @@

it('should handle inline comments (after domain)', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com # GitHub main domain\napi.github.com # API endpoint');

Check warning on line 112 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 112 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 112 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -118,7 +118,7 @@

it('should handle domains with inline comments in comma-separated format', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, 'github.com, api.github.com # GitHub domains\nnpmjs.org');

Check warning on line 121 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 121 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 121 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand All @@ -133,7 +133,7 @@

it('should return empty array for file with only comments and whitespace', () => {
const filePath = path.join(testDir, 'domains.txt');
fs.writeFileSync(filePath, '# Comment 1\n\n# Comment 2\n \n');

Check warning on line 136 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

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

Check warning on line 136 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

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

Check warning on line 136 in src/cli.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

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

const result = parseDomainsFile(filePath);

Expand Down Expand Up @@ -1495,4 +1495,48 @@
expect(r.valid).toBe(true);
});
});

describe('validateAllowHostPorts', () => {
it('should fail when --allow-host-ports is used without --enable-host-access', () => {
const result = validateAllowHostPorts('3000', undefined);
expect(result.valid).toBe(false);
expect(result.error).toContain('--allow-host-ports requires --enable-host-access');
});

it('should fail when --allow-host-ports is used with enableHostAccess=false', () => {
const result = validateAllowHostPorts('8080', false);
expect(result.valid).toBe(false);
expect(result.error).toContain('--allow-host-ports requires --enable-host-access');
});

it('should pass when --allow-host-ports is used with --enable-host-access', () => {
const result = validateAllowHostPorts('3000', true);
expect(result.valid).toBe(true);
expect(result.error).toBeUndefined();
});

it('should pass when --allow-host-ports is not provided', () => {
const result = validateAllowHostPorts(undefined, undefined);
expect(result.valid).toBe(true);
expect(result.error).toBeUndefined();
});

it('should pass when only --enable-host-access is set without ports', () => {
const result = validateAllowHostPorts(undefined, true);
expect(result.valid).toBe(true);
expect(result.error).toBeUndefined();
});

it('should fail for port ranges without --enable-host-access', () => {
const result = validateAllowHostPorts('3000-3010,8080', undefined);
expect(result.valid).toBe(false);
expect(result.error).toContain('--allow-host-ports requires --enable-host-access');
});

it('should pass for port ranges with --enable-host-access', () => {
const result = validateAllowHostPorts('3000-3010,8000-8090', true);
expect(result.valid).toBe(true);
expect(result.error).toBeUndefined();
});
});
});
26 changes: 23 additions & 3 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,25 @@ export function validateSkipPullWithBuildLocal(
return { valid: true };
}

/**
* Validates that --allow-host-ports is only used with --enable-host-access
* @param allowHostPorts - The --allow-host-ports value (undefined if not provided)
* @param enableHostAccess - Whether --enable-host-access flag was provided
* @returns FlagValidationResult with validation status and error message
*/
export function validateAllowHostPorts(
allowHostPorts: string | undefined,
enableHostAccess: boolean | undefined
): FlagValidationResult {
if (allowHostPorts && !enableHostAccess) {
return {
valid: false,
error: '--allow-host-ports requires --enable-host-access to be set',
};
}
Comment on lines +416 to +421
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateAllowHostPorts uses a truthy check (if (allowHostPorts && ...)). If the flag is provided as an empty string (e.g. --allow-host-ports ""), this bypasses validation even though the option was explicitly set. Consider checking allowHostPorts !== undefined (and optionally allowHostPorts.trim() !== '') instead of relying on truthiness so the constraint is enforced consistently.

Copilot uses AI. Check for mistakes.
return { valid: true };
}

/**
* Parses and validates DNS servers from a comma-separated string
* @param input - Comma-separated DNS server string (e.g., "8.8.8.8,1.1.1.1")
Expand Down Expand Up @@ -1110,9 +1129,10 @@ program
logger.warn(' This may expose sensitive credentials if logs or configs are shared');
}

// Warn if --allow-host-ports is used without --enable-host-access
if (config.allowHostPorts && !config.enableHostAccess) {
logger.error('❌ --allow-host-ports requires --enable-host-access to be set');
// Validate --allow-host-ports requires --enable-host-access
const hostPortsValidation = validateAllowHostPorts(config.allowHostPorts, config.enableHostAccess);
if (!hostPortsValidation.valid) {
logger.error(`❌ ${hostPortsValidation.error}`);
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hostPortsValidation.error is optional in FlagValidationResult, but this log interpolates it without a non-null assertion or fallback. If a future refactor returns { valid: false } without an error message, this would log ❌ undefined. Consider asserting non-null (error!) or providing a default message when valid is false.

Suggested change
logger.error(`❌ ${hostPortsValidation.error}`);
logger.error(`❌ ${hostPortsValidation.error ?? 'Invalid --allow-host-ports configuration'}`);

Copilot uses AI. Check for mistakes.
process.exit(1);
}

Expand Down
Loading