Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 14 additions & 10 deletions src/cli-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,11 @@ program
'Enforce egress via Docker network topology (internal network +\n' +
' dual-homed proxy) instead of iptables. Requires no sudo/NET_ADMIN.\n' +
' Not yet supported with --dns-over-https or --enable-host-access.\n' +
' Enabled by default in --security-mode strict.'
' Enabled by default (strict security).'
)
.option(
'--no-network-isolation',
'Disable network-isolation mode (requires --security-mode compat in strict mode).'
'Disable network-isolation mode (requires --legacy-security).'
)
.option(
'--topology-attach <name>',
Expand Down Expand Up @@ -278,13 +278,18 @@ program
' WARNING: allows firewall bypass via docker run',
false
)
.addOption(
new Option(
'--legacy-security',
'Enable legacy security mode (sudo, host-access, iptables).\n' +
' Default behavior is strict security (network-isolation + api-proxy).',
)
)
.addOption(
new Option(
'--security-mode <mode>',
'Security enforcement mode (default: strict).\n' +
' strict: network-isolation + api-proxy, no sudo/iptables.\n' +
' compat: legacy iptables mode, requires sudo.',
).choices(['strict', 'compat']).default('strict')
'[DEPRECATED] Use --legacy-security instead.',
).choices(['strict', 'compat']).hideHelp()
)
.option(
'--enable-dlp',
Expand All @@ -293,15 +298,14 @@ program
false
)

// -- API Proxy --
// -- API Proxy (always enabled, flags retained for backward compatibility) --
.option(
'--enable-api-proxy',
'Enable API proxy sidecar for secure credential injection.\n' +
' Supports OpenAI (Codex) and Anthropic (Claude) APIs.'
'[DEPRECATED] The API proxy is always enabled. This flag is ignored.'
)
.option(
'--no-enable-api-proxy',
'Disable the API proxy sidecar (requires --security-mode compat in strict mode).'
'[REMOVED] The API proxy cannot be disabled. Passing this flag is an error.'
)
.option(
'--copilot-api-target <host>',
Expand Down
31 changes: 30 additions & 1 deletion src/commands/build-config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
import { WrapperConfig, LogLevel, UpstreamProxyConfig } from '../types';
import { resolveApiCredentials } from './resolve-credentials';
import { logger } from '../logger';

/**
* Resolves the effective `legacySecurity` value from CLI options.
*
* Sources (in priority order):
* 1. `--legacy-security` boolean flag (preferred)
* 2. `--security-mode compat` (deprecated, maps to legacySecurity=true)
*/
function resolveLegacySecurity(options: Record<string, unknown>): boolean | undefined {
// Handle deprecated --security-mode flag
const securityMode = options.securityMode as string | undefined;
if (securityMode === 'compat') {
logger.warn(
'⚠️ --security-mode compat is deprecated. Use --legacy-security instead.',
);
return true;
}
if (securityMode === 'strict') {
logger.warn(
'⚠️ --security-mode is deprecated. Strict security is the default; remove the flag.',
);
return undefined;
}

// Handle --legacy-security boolean
const legacySecurity = options.legacySecurity as boolean | undefined;
return legacySecurity || undefined;
}

/**
* Inputs required to assemble a {@link WrapperConfig}.
Expand Down Expand Up @@ -116,7 +145,7 @@ export function buildConfig(inputs: BuildConfigInputs): WrapperConfig {
sslBump: options.sslBump as boolean,
enableDind: options.enableDind as boolean,
enableDlp: options.enableDlp as boolean,
securityMode: options.securityMode as 'strict' | 'compat' | undefined,
legacySecurity: resolveLegacySecurity(options),
allowedUrls,
enableApiProxy: options.enableApiProxy as boolean | undefined,
modelFallback:
Expand Down
28 changes: 7 additions & 21 deletions src/commands/validate-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const STUB_CONFIG = {
blockedDomains: undefined,
agentCommand: 'echo hi',
logLevel: 'info',
legacySecurity: true,
keepContainers: false,
tty: false,
workDir: '/tmp/workdir',
Expand Down Expand Up @@ -64,7 +65,7 @@ const STUB_CONFIG = {
enableDind: false,
enableDlp: false,
allowedUrls: undefined,
enableApiProxy: false,
enableApiProxy: undefined,
anthropicAutoCache: false,
anthropicCacheTailTtl: undefined,
modelAliases: undefined,
Expand Down Expand Up @@ -514,29 +515,14 @@ describe('validateOptions', () => {
);
});

it('exits when rate limit flags are used without --enable-api-proxy', () => {
mockedOptionParsers.validateRateLimitFlags.mockReturnValue({
valid: false,
error: '--rpm requires --enable-api-proxy',
});
expect(() => validateOptions(validOptions(), 'echo hi')).toThrow('process.exit called');
expect(mockedLogger.error).toHaveBeenCalledWith(
expect.stringContaining('--rpm requires --enable-api-proxy'),
);
});
// Note: "rate limit flags without --enable-api-proxy" is no longer testable
// because applySecurityMode always forces enableApiProxy=true.
// The validateRateLimitFlags check is now dead code for this scenario.
});

describe('feature flag compatibility', () => {
it('exits when --enable-token-steering is used without --enable-api-proxy', () => {
mockedOptionParsers.validateEnableTokenSteeringFlag.mockReturnValue({
valid: false,
error: '--enable-token-steering requires --enable-api-proxy',
});
expect(() => validateOptions(validOptions(), 'echo hi')).toThrow('process.exit called');
expect(mockedLogger.error).toHaveBeenCalledWith(
expect.stringContaining('--enable-token-steering requires --enable-api-proxy'),
);
});
// Note: "--enable-token-steering without --enable-api-proxy" is no longer testable
// because applySecurityMode always forces enableApiProxy=true.

it('exits when --skip-pull and --build-local are combined', () => {
mockedOptionParsers.validateSkipPullWithBuildLocal.mockReturnValue({
Expand Down
20 changes: 2 additions & 18 deletions src/commands/validators/config-assembly-api-proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
logger,
mockBuildConfigOnce,
setupConfigAssemblyTestSuite,
validateRateLimitFlags,

Check failure on line 7 in src/commands/validators/config-assembly-api-proxy.test.ts

View workflow job for this annotation

GitHub Actions / ESLint

'validateRateLimitFlags' is defined but never used

Check failure on line 7 in src/commands/validators/config-assembly-api-proxy.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 22)

'validateRateLimitFlags' is defined but never used

Check failure on line 7 in src/commands/validators/config-assembly-api-proxy.test.ts

View workflow job for this annotation

GitHub Actions / Build and Lint (Node 20)

'validateRateLimitFlags' is defined but never used
} from './config-assembly.test-utils';

describe('config-assembly', () => {
Expand All @@ -29,26 +29,10 @@
);
});

it('should exit if rate limit flags are used without --enable-api-proxy', () => {
(validateRateLimitFlags as jest.Mock).mockReturnValueOnce({
valid: false,
error: 'Rate limit flags require --enable-api-proxy',
});

expect(() => {
callAssembleWith();
}).toThrow('process.exit(1)');

expect(logger.error).toHaveBeenCalledWith(
'Rate limit flags require --enable-api-proxy',
);
});
// Note: "rate limit flags without --enable-api-proxy" test removed —
// API proxy is always enabled, so this scenario cannot occur.

it('should set rate limit config when API proxy is enabled', () => {
mockBuildConfigOnce({
enableApiProxy: true,
});

const mockRateLimitConfig = {
enabled: true,
rpm: 100,
Expand Down
10 changes: 5 additions & 5 deletions src/commands/validators/config-assembly.test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jest.mock('../../option-parsers', () => {
validateSkipPullWithBuildLocal: jest.fn(),
validateAllowHostPorts: jest.fn(),
applyHostServicePortsConfig: jest.fn(),
buildRateLimitConfig: jest.fn(),
buildRateLimitConfig: jest.fn().mockReturnValue({ config: { enabled: false, rpm: 0, rph: 0, bytesPm: 0 } }),
applyAgentTimeout: jest.fn(),
isLoopbackTcpDockerHostUri: actual.isLoopbackTcpDockerHostUri,
};
Expand All @@ -57,8 +57,8 @@ jest.mock('../build-config', () => ({
logLevel: args.logLevel,
allowedDomains: args.allowedDomains,
blockedDomains: args.blockedDomains,
securityMode: 'compat',
enableApiProxy: false,
legacySecurity: true,
enableApiProxy: undefined,
enableTokenSteering: false,
envAll: false,
envFile: undefined,
Expand Down Expand Up @@ -150,8 +150,8 @@ export const createBuildConfigResult = (
logLevel: 'info',
allowedDomains: ['example.com'],
blockedDomains: [],
securityMode: 'compat',
enableApiProxy: false,
legacySecurity: true,
enableApiProxy: undefined,
enableTokenSteering: false,
envAll: false,
envFile: undefined,
Expand Down
76 changes: 36 additions & 40 deletions src/commands/validators/security-mode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ function makeConfig(overrides: Partial<WrapperConfig> = {}): WrapperConfig {
proxyLogsDir: '/tmp/logs',
dnsServers: ['8.8.8.8'],
enableHostAccess: false,
// networkIsolation and enableApiProxy are intentionally left undefined here
// to match the CLI default behaviour — users who do not explicitly pass
// --network-isolation or --enable-api-proxy will have undefined, not false.
enableDind: false,
sslBump: false,
enableDlp: false,
Expand All @@ -50,49 +47,49 @@ describe('applySecurityMode', () => {
(runtimeUsesComposeAgent as jest.Mock).mockReturnValue(true);
});

describe('strict mode (default)', () => {
it('should force networkIsolation on when undefined (not explicitly set)', () => {
const config = makeConfig({ securityMode: 'strict', networkIsolation: undefined });
describe('strict security (default)', () => {
it('should force networkIsolation on when undefined', () => {
const config = makeConfig({ networkIsolation: undefined });
applySecurityMode(config);
expect(config.networkIsolation).toBe(true);
expect(logger.warn).not.toHaveBeenCalledWith(
expect.stringContaining('--no-network-isolation'),
);
});

it('should force networkIsolation on and warn when explicitly disabled', () => {
const config = makeConfig({ securityMode: 'strict', networkIsolation: false });
const config = makeConfig({ networkIsolation: false });
applySecurityMode(config);
expect(config.networkIsolation).toBe(true);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining('--no-network-isolation was ignored'),
);
});

it('should force enableApiProxy on when undefined (not explicitly set)', () => {
const config = makeConfig({ securityMode: 'strict', enableApiProxy: undefined });
it('should always force enableApiProxy on', () => {
const config = makeConfig({ enableApiProxy: undefined });
applySecurityMode(config);
expect(config.enableApiProxy).toBe(true);
expect(logger.warn).not.toHaveBeenCalledWith(
expect.stringContaining('--no-enable-api-proxy'),
});

it('should throw when --no-enable-api-proxy is passed', () => {
const config = makeConfig({ enableApiProxy: false });
expect(() => applySecurityMode(config)).toThrow(
'--no-enable-api-proxy is not allowed',
);
});

it('should force enableApiProxy on and warn when explicitly disabled', () => {
const config = makeConfig({ securityMode: 'strict', enableApiProxy: false });
it('should warn when --enable-api-proxy is explicitly passed', () => {
const config = makeConfig({ enableApiProxy: true });
applySecurityMode(config);
expect(config.enableApiProxy).toBe(true);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining('--no-enable-api-proxy was ignored'),
expect.stringContaining('--enable-api-proxy is deprecated'),
);
expect(config.enableApiProxy).toBe(true);
});

it('should be the default when securityMode is undefined', () => {
const config = makeConfig({ securityMode: undefined });
it('should be the default when legacySecurity is undefined', () => {
const config = makeConfig({ legacySecurity: undefined });
applySecurityMode(config);
expect(config.networkIsolation).toBe(true);
expect(config.enableApiProxy).toBe(true);
expect(logger.warn).not.toHaveBeenCalled();
});

it('should override enableHostAccess with warning', () => {
Expand All @@ -104,7 +101,7 @@ describe('applySecurityMode', () => {
);
});

it('should clear allowHostServicePorts when set (prevents downstream re-enable of host access)', () => {
it('should clear allowHostServicePorts when set', () => {
const config = makeConfig({ allowHostServicePorts: '5432,6379' });
applySecurityMode(config);
expect(config.allowHostServicePorts).toBeUndefined();
Expand All @@ -113,7 +110,7 @@ describe('applySecurityMode', () => {
);
});

it('should clear allowHostServicePorts and allowHostPorts set alongside enableHostAccess', () => {
it('should clear allowHostServicePorts and allowHostPorts alongside enableHostAccess', () => {
const config = makeConfig({
enableHostAccess: true,
allowHostPorts: '3000,8080',
Expand Down Expand Up @@ -143,19 +140,17 @@ describe('applySecurityMode', () => {
);
});

it('should warn that --security-mode compat is required for overridden options', () => {
it('should warn that --legacy-security is required for overridden options', () => {
const config = makeConfig({ enableHostAccess: true, enableDind: true });
applySecurityMode(config);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining('--security-mode compat'),
expect.stringContaining('--legacy-security'),
);
});

it('should not warn when compatible options are already set', () => {
const config = makeConfig({
securityMode: 'strict',
networkIsolation: true,
enableApiProxy: true,
enableHostAccess: false,
enableDind: false,
});
Expand All @@ -169,43 +164,44 @@ describe('applySecurityMode', () => {
});

it('should skip network-isolation enforcement for microVM runtimes', () => {
const config = makeConfig({ securityMode: 'strict', containerRuntime: 'sbx' });
const config = makeConfig({ containerRuntime: 'sbx' });
applySecurityMode(config);
expect(config.networkIsolation).toBeUndefined();
expect(logger.warn).not.toHaveBeenCalledWith(
expect.stringContaining('network-isolation'),
);
});

it('should still enforce api-proxy for microVM runtimes', () => {
const config = makeConfig({ securityMode: 'strict', containerRuntime: 'sbx' });
const config = makeConfig({ containerRuntime: 'sbx' });
applySecurityMode(config);
expect(config.enableApiProxy).toBe(true);
});
});
});

describe('compat mode', () => {
it('should not modify any config values', () => {
describe('legacy security mode', () => {
it('should not override host-access or dind', () => {
const config = makeConfig({
securityMode: 'compat',
legacySecurity: true,
networkIsolation: false,
enableApiProxy: false,
enableHostAccess: true,
enableDind: true,
});
applySecurityMode(config);
expect(config.networkIsolation).toBe(false);
expect(config.enableApiProxy).toBe(false);
expect(config.enableHostAccess).toBe(true);
expect(config.enableDind).toBe(true);
});

it('should log info about compat mode', () => {
const config = makeConfig({ securityMode: 'compat' });
it('should still force api-proxy on in legacy mode', () => {
const config = makeConfig({ legacySecurity: true });
applySecurityMode(config);
expect(config.enableApiProxy).toBe(true);
});

it('should log info about legacy security mode', () => {
const config = makeConfig({ legacySecurity: true });
applySecurityMode(config);
expect(logger.info).toHaveBeenCalledWith(
expect.stringContaining('compat security mode'),
expect.stringContaining('legacy security mode'),
);
});
});
Expand Down
Loading
Loading