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
8 changes: 5 additions & 3 deletions docs/network-isolation-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,11 @@ error: --network-isolation requires a reachable Docker daemon, but none was foun
> because it is entangled with the gh-aw §8.2 handshake decisions.

- ✅ **`src/commands/validators/config-assembly.ts`** — `--network-isolation` still rejects
`--dns-over-https` and `--enable-host-access` (genuine host-iptables features), but
`--enable-api-proxy` and `--difc-proxy-host` are accepted (never rejected). Added a guard
so `--topology-attach` requires `--network-isolation`.
`--dns-over-https` (genuine host-iptables feature that needs direct external connectivity),
but now **accepts `--enable-host-access`**: in topology mode host access drives Squid port
ACLs and the `host.docker.internal` hosts-file entry for topology peers rather than
iptables, making the combination valid. Added a guard so `--topology-attach` requires
`--network-isolation`.
- ✅ **`src/cli-workflow.ts`** — added the **late network-attach** step: after
`startContainers` succeeds, when `config.topologyAttach` is non-empty it calls
`connectTopologyContainers('awf-net', names)` (`docker network connect awf-net <container>`,
Expand Down
8 changes: 3 additions & 5 deletions src/commands/validators/config-assembly-flags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,14 @@ describe('config-assembly', () => {
);
});

it('should exit if --network-isolation is combined with --enable-host-access', () => {
it('should accept --enable-host-access combined with --network-isolation', () => {
mockBuildConfigOnce({ networkIsolation: true, enableHostAccess: true });

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

expect(logger.error).toHaveBeenCalledWith(
expect.stringContaining('--network-isolation is not supported with --enable-host-access'),
);
expect(getMockExit()).not.toHaveBeenCalled();
});

it('should exit if --topology-attach is used without --network-isolation', () => {
Expand Down
12 changes: 7 additions & 5 deletions src/commands/validators/infrastructure-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,13 @@ export function validateFeatureFlagCompatibility(config: WrapperConfig): void {
logger.error(' The DoH proxy needs direct external connectivity, which the internal network does not provide.');
process.exit(1);
}
if (config.enableHostAccess) {
logger.error('❌ --network-isolation is not supported with --enable-host-access.');
logger.error(' Host access relies on host-level iptables, which network-isolation mode does not configure.');
process.exit(1);
}
// --enable-host-access is intentionally allowed with --network-isolation:
// in topology mode the agent is on an internal Docker network with no direct
// host route, so no host-level iptables are configured for host access.
// Instead, trusted services are reachable via topology peers
// (--topology-attach) attached to awf-net, and --enable-host-access drives
// Squid port ACLs and the hosts-file entry for host.docker.internal that
// make those peers discoverable.
} else if (config.topologyAttach && config.topologyAttach.length > 0) {
logger.error('❌ --topology-attach requires --network-isolation.');
logger.error(' Trusted containers can only be attached to the internal topology network in network-isolation mode.');
Expand Down
33 changes: 27 additions & 6 deletions src/commands/validators/security-mode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,15 @@ describe('applySecurityMode', () => {
expect(config.enableApiProxy).toBe(true);
});

it('should override enableHostAccess with warning', () => {
it('should preserve enableHostAccess in network-isolation (topology) mode', () => {
// In strict mode, networkIsolation is forced to true before the
// enableHostAccess check runs. Host access is valid in topology mode
// because the agent reaches services via topology peers on awf-net,
// not via host-level iptables.
const config = makeConfig({ enableHostAccess: true });
applySecurityMode(config);
expect(config.enableHostAccess).toBe(false);
expect(logger.warn).toHaveBeenCalledWith(
expect(config.enableHostAccess).toBe(true);
expect(logger.warn).not.toHaveBeenCalledWith(
expect.stringContaining('--enable-host-access was ignored'),
);
});
Expand All @@ -120,15 +124,19 @@ describe('applySecurityMode', () => {
);
});

it('should clear allowHostServicePorts and allowHostPorts alongside enableHostAccess', () => {
it('should preserve enableHostAccess and allowHostPorts but still clear allowHostServicePorts', () => {
// enableHostAccess and allowHostPorts are topology-compatible (drive Squid
// port ACLs / hosts-file), so they are preserved in strict+topology mode.
// allowHostServicePorts is iptables-based (GitHub Actions services) and
// is still suppressed.
const config = makeConfig({
enableHostAccess: true,
allowHostPorts: '3000,8080',
allowHostServicePorts: '5432',
});
applySecurityMode(config);
expect(config.enableHostAccess).toBe(false);
expect(config.allowHostPorts).toBeUndefined();
expect(config.enableHostAccess).toBe(true);
expect(config.allowHostPorts).toBe('3000,8080');
expect(config.allowHostServicePorts).toBeUndefined();
});

Expand Down Expand Up @@ -184,6 +192,19 @@ describe('applySecurityMode', () => {
applySecurityMode(config);
expect(config.enableApiProxy).toBe(true);
});

it('should suppress enableHostAccess for microVM runtimes even when networkIsolation is true', () => {
const config = makeConfig({
containerRuntime: 'sbx',
networkIsolation: true,
enableHostAccess: true,
});
applySecurityMode(config);
expect(config.enableHostAccess).toBe(false);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining('--enable-host-access was ignored'),
);
});
});
});

Expand Down
25 changes: 22 additions & 3 deletions src/commands/validators/security-mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,23 @@ export function applySecurityMode(config: WrapperConfig): void {
// Force api-proxy on (always, regardless of flags).
config.enableApiProxy = true;

// Override incompatible options
if (config.enableHostAccess) {
// Override host access options that depend on host-level iptables.
//
// In network-isolation (topology) mode, the agent is on an internal Docker
// network with no direct host route, so no iptables-based host access is
// configured. Instead, trusted services are reached via topology peers
// (--topology-attach) attached to awf-net. --enable-host-access in that
// mode drives Squid port ACLs and the hosts-file entry for
// host.docker.internal — both of which are compatible with strict security.
//
// NOTE: at this point in the pipeline, networkIsolation has already been
// forced to true above (for non-microVM runtimes), so
// !config.networkIsolation is false for standard Docker-compose runs.
//
// For microVM runtimes, networkIsolation does not imply topology routing
// support (the compose agent is not used), so host access remains
// incompatible and is still suppressed in strict mode.
if (config.enableHostAccess && (isMicroVmRuntime || !config.networkIsolation)) {
logger.warn(
'⚠️ --enable-host-access was ignored (incompatible with strict security, the default).\n' +
' Pass --legacy-security to enable host access.',
Expand All @@ -74,7 +89,11 @@ export function applySecurityMode(config: WrapperConfig): void {
}

// Similarly, allowHostServicePorts alone (without enableHostAccess) would
// auto-enable host access downstream — suppress it in strict mode.
// auto-enable host access downstream via iptables — suppress it in strict
// mode. This applies even in network-isolation mode because
// allowHostServicePorts is specifically for GitHub Actions services
// containers accessed through host-gateway iptables rules, not topology
// peers.
if (config.allowHostServicePorts) {
logger.warn(
'⚠️ --allow-host-service-ports was ignored (incompatible with strict security, the default).\n' +
Expand Down
12 changes: 12 additions & 0 deletions src/compose-generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,18 @@ describe('generateDockerCompose', () => {
expect(result.services.agent.dns).toEqual(['127.0.0.11']);
});

it('keeps host gateway off the agent proxy bypass list in topology mode', () => {
const result = generateDockerCompose(
{ ...mockConfig, networkIsolation: true, enableHostAccess: true },
mockNetworkConfig,
);

const noProxy = String(result.services.agent.environment?.NO_PROXY ?? '').split(',');
expect(noProxy).not.toContain('host.docker.internal');
expect(noProxy).not.toContain('172.30.0.1');
expect(result.services.agent.extra_hosts?.['host.docker.internal']).toBeUndefined();
});

it('should still build the iptables-init service in default (iptables) mode', () => {
const result = generateDockerCompose(mockConfig, mockNetworkConfig);

Expand Down
10 changes: 10 additions & 0 deletions src/services/agent-environment/proxy-environment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ describe('buildProxyEnvironment', () => {
expect(env.NO_PROXY.split(',')).toContain('host.docker.internal');
});

it('does not add host gateway entries to NO_PROXY in topology mode', () => {
const env = run({
...baseConfig,
networkIsolation: true,
enableHostAccess: true,
});
expect(env.NO_PROXY.split(',')).not.toContain('172.30.0.1');
expect(env.NO_PROXY.split(',')).not.toContain('host.docker.internal');
});

describe('topology-attached peers', () => {
it('exempts topology peers from proxy routing for a compose agent in isolation mode', () => {
const env = run({
Expand Down
6 changes: 5 additions & 1 deletion src/services/agent-environment/proxy-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ export function buildProxyEnvironment(params: ProxyEnvironmentParams): void {
// whose netstack can't use host-netns iptables (e.g. gVisor) have no such
// bypass, so add the gateway to NO_PROXY so proxy-aware clients (rmcp) connect
// to it directly instead of being routed through Squid and rejected.
const gatewayNeedsNoProxy = config.enableHostAccess || !runtimeUsesIptables(config.containerRuntime);
const gatewayNeedsNoProxy = (
!config.networkIsolation &&
config.enableHostAccess &&
runtimeUsesComposeAgent(config.containerRuntime)
) || !runtimeUsesIptables(config.containerRuntime);
if (gatewayNeedsNoProxy) {
const subnetBase = networkConfig.subnet.split('/')[0];
const parts = subnetBase.split('.');
Expand Down
6 changes: 4 additions & 2 deletions src/services/agent-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '../constants';
import { ACT_PRESET_BASE_IMAGE, getSafeHostUid, getSafeHostGid } from '../host-identity';
import { buildRuntimeImageRef } from '../image-tag';
import { resolveDockerRuntime, runtimeNeedsStaticDns } from '../container-runtime';
import { resolveDockerRuntime, runtimeNeedsStaticDns, runtimeUsesComposeAgent } from '../container-runtime';
import { buildInternalServiceHosts } from './internal-service-hosts';
import { logger } from '../logger';
import { WrapperConfig } from '../types';
Expand Down Expand Up @@ -155,7 +155,9 @@ export function buildAgentService(params: AgentServiceParams): any {
}

// Enable host.docker.internal for agent when --enable-host-access is set
if (config.enableHostAccess) {
const shouldInjectHostGateway = config.enableHostAccess &&
!(config.networkIsolation && runtimeUsesComposeAgent(config.containerRuntime));
if (shouldInjectHostGateway) {
agentService.extra_hosts = { 'host.docker.internal': 'host-gateway' };
environment.AWF_ENABLE_HOST_ACCESS = '1';
}
Expand Down
20 changes: 20 additions & 0 deletions src/services/agent-volumes/hosts-file-branches.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,26 @@ describe('generateHostsFileMount – localhostDetected branch', () => {
// localhost entry should not have been replaced with gateway IP
expect(content).not.toContain(`${gatewayIp}\tlocalhost`);
});

it('does not inject host gateway mappings in topology mode', () => {
const gatewayIp = '172.17.0.1';
mockExecaSync.mockReturnValue({ stdout: gatewayIp, stderr: '' });

const config = makeConfig({
workDir: getTmpDir(),
allowedDomains: [],
enableHostAccess: true,
localhostDetected: true,
networkIsolation: true,
});

const mount = generateHostsFileMount(config);
const hostsPath = mount.split(':')[0];
const content = fs.readFileSync(hostsPath, 'utf8');

expect(content).not.toContain(`${gatewayIp}\thost.docker.internal`);
expect(content).not.toContain(`${gatewayIp}\tlocalhost`);
});
});

describe('pruneStaleChrootStageDirs – error handling', () => {
Expand Down
5 changes: 4 additions & 1 deletion src/services/agent-volumes/hosts-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as path from 'path';
import execa from 'execa';
import { logger } from '../../logger';
import { WrapperConfig } from '../../types';
import { runtimeUsesComposeAgent } from '../../container-runtime';
import { getDockerHostStageRoot, shouldUseDockerHostStaging } from './docker-host-staging';

const STALE_CHROOT_STAGE_MAX_AGE_MS = 24 * 60 * 60 * 1000;
Expand Down Expand Up @@ -38,7 +39,9 @@ export function generateHostsFileMount(config: WrapperConfig): string {
}
}

if (config.enableHostAccess) {
const shouldInjectHostGateway = config.enableHostAccess &&
!(config.networkIsolation && runtimeUsesComposeAgent(config.containerRuntime));
if (shouldInjectHostGateway) {
try {
const { stdout } = execa.sync('docker', [
'network', 'inspect', 'bridge',
Expand Down
Loading