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
24 changes: 23 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, validateAllowHostPorts, 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, parseMemoryLimit, 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 @@ -1539,4 +1539,26 @@
expect(result.error).toBeUndefined();
});
});

describe('parseMemoryLimit', () => {
it('accepts valid memory limits', () => {
expect(parseMemoryLimit('2g')).toEqual({ value: '2g' });
expect(parseMemoryLimit('4g')).toEqual({ value: '4g' });
expect(parseMemoryLimit('512m')).toEqual({ value: '512m' });
expect(parseMemoryLimit('1024k')).toEqual({ value: '1024k' });
expect(parseMemoryLimit('8G')).toEqual({ value: '8g' });
});

it('rejects invalid formats', () => {
expect(parseMemoryLimit('abc')).toHaveProperty('error');
expect(parseMemoryLimit('-1g')).toHaveProperty('error');
expect(parseMemoryLimit('2x')).toHaveProperty('error');
expect(parseMemoryLimit('')).toHaveProperty('error');
expect(parseMemoryLimit('g')).toHaveProperty('error');
});

it('rejects zero', () => {
expect(parseMemoryLimit('0g')).toHaveProperty('error');
});
});
});
30 changes: 30 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,23 @@ export function validateAllowHostPorts(
return { valid: true };
}

/**
* Parses and validates a Docker memory limit string.
* Valid formats: positive integer followed by b, k, m, or g (e.g., "2g", "512m", "4g").
*/
export function parseMemoryLimit(input: string): { value: string; error?: undefined } | { value?: undefined; error: string } {
const pattern = /^(\d+)([bkmg])$/i;
const match = input.match(pattern);
if (!match) {
return { error: `Invalid --memory-limit value "${input}". Expected format: <number><unit> (e.g., 2g, 512m, 4g)` };
}
const num = parseInt(match[1], 10);
if (num <= 0) {
return { error: `Invalid --memory-limit value "${input}". Memory limit must be a positive number.` };
}
return { value: input.toLowerCase() };
}
Comment on lines +429 to +440
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.

parseMemoryLimit matches the input string as-is; unlike other parsers in this file (e.g., DNS parsing), it doesn’t trim whitespace first. This causes values like "2g " (or values copied with trailing whitespace) to be rejected even though they’re otherwise valid. Consider normalizing with input.trim() (and returning the normalized value) before applying the regex.

Copilot uses AI. Check for mistakes.

/**
* 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 @@ -780,6 +797,11 @@ program
'--container-workdir <dir>',
'Working directory inside the container (should match GITHUB_WORKSPACE for path consistency)'
)
.option(
'--memory-limit <limit>',
'Memory limit for the agent container (e.g., 1g, 2g, 4g, 512m). Default: 2g',
'2g'
)
Comment on lines +800 to +804
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.

PR description / issue acceptance criteria mention updating user-facing docs (README.md / AGENTS.md) for the new --memory-limit flag and the new 2g default, but this PR only changes code/tests. Either add the documentation updates or adjust the PR description so it matches what’s actually included.

Copilot uses AI. Check for mistakes.
.option(
'--dns-servers <servers>',
'Comma-separated list of trusted DNS servers. DNS traffic is ONLY allowed to these servers (default: 8.8.8.8,8.8.4.4)',
Expand Down Expand Up @@ -1067,6 +1089,13 @@ program
logger.warn('⚠️ SSL Bump intercepts HTTPS traffic. Only use for trusted workloads.');
}

// Validate memory limit
const memoryLimit = parseMemoryLimit(options.memoryLimit);
if (memoryLimit.error) {
logger.error(memoryLimit.error);
process.exit(1);
}

// Validate agent image option
const agentImageResult = processAgentImageOption(options.agentImage, options.buildLocal);
if (agentImageResult.error) {
Expand Down Expand Up @@ -1096,6 +1125,7 @@ program
volumeMounts,
containerWorkDir: options.containerWorkdir,
dnsServers,
memoryLimit: memoryLimit.value,
proxyLogsDir: options.proxyLogsDir,
enableHostAccess: options.enableHostAccess,
allowHostPorts: options.allowHostPorts,
Expand Down
13 changes: 11 additions & 2 deletions src/docker-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1022,12 +1022,21 @@ describe('docker-manager', () => {
expect(agent.security_opt).toContain('no-new-privileges:true');

// Verify resource limits
expect(agent.mem_limit).toBe('4g');
expect(agent.memswap_limit).toBe('4g');
expect(agent.mem_limit).toBe('2g');
expect(agent.memswap_limit).toBe('2g');
expect(agent.pids_limit).toBe(1000);
expect(agent.cpu_shares).toBe(1024);
});

it('should use custom memory limit when specified', () => {
const customConfig = { ...mockConfig, memoryLimit: '8g' };
const result = generateDockerCompose(customConfig, mockNetworkConfig);
const agent = result.services.agent;

expect(agent.mem_limit).toBe('8g');
expect(agent.memswap_limit).toBe('8g');
});

it('should disable TTY by default to prevent ANSI escape sequences', () => {
const result = generateDockerCompose(mockConfig, mockNetworkConfig);
const agent = result.services.agent;
Expand Down
4 changes: 2 additions & 2 deletions src/docker-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -912,8 +912,8 @@ export function generateDockerCompose(
'apparmor:unconfined',
],
// Resource limits to prevent DoS attacks (conservative defaults)
mem_limit: '4g', // 4GB memory limit
memswap_limit: '4g', // No swap (same as mem_limit)
mem_limit: config.memoryLimit || '2g',
memswap_limit: config.memoryLimit || '2g', // No swap (same as mem_limit)
Comment on lines +915 to +916
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.

mem_limit and memswap_limit both repeat the same fallback expression. To avoid accidental divergence and make the default clearer, consider computing a single local value (e.g., memoryLimit = config.memoryLimit ?? '2g') and reusing it for both fields.

Copilot uses AI. Check for mistakes.
pids_limit: 1000, // Max 1000 processes
cpu_shares: 1024, // Default CPU share
stdin_open: true,
Expand Down
12 changes: 12 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ export interface WrapperConfig {
*/
dnsServers?: string[];

/**
* Memory limit for the agent execution container
*
* Accepts Docker memory format: a positive integer followed by a unit suffix
* (b, k, m, g). Controls the maximum amount of memory the container can use.
*
* @default '2g'
* @example '4g'
* @example '512m'
*/
memoryLimit?: string;

/**
* Custom directory for Squid proxy logs (written directly during runtime)
*
Expand Down
Loading