Skip to content

Commit 9e65405

Browse files
authored
fix: check iptables availability before host firewall setup (#5136)
* fix: check iptables availability * fix: use Record<string, unknown> cast in getErrorStringProperty --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 41cf5ac commit 9e65405

5 files changed

Lines changed: 66 additions & 7 deletions

File tree

src/host-iptables-coverage.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,17 @@ describe('host-iptables branch coverage', () => {
2323
setupHostIptablesTestSuite(iptablesSharedTestHelpers.resetIpv6State);
2424

2525
// -------------------------------------------------------------------------
26-
// Branch 1: host-iptables-rules.ts line 85
27-
// checkPermissionsAndSetupChain – the ternary `? error.stderr : ''` when the
28-
// thrown error object does NOT have a string `stderr` property.
26+
// Branch 1: host-iptables-rules.ts
27+
// checkPermissionsAndSetupChain – the fallback when the thrown error object
28+
// does NOT have a string `stderr` property.
2929
// -------------------------------------------------------------------------
3030
describe('checkPermissionsAndSetupChain – no-stderr error object', () => {
3131
it('treats missing stderr as empty string and proceeds with chain creation', async () => {
3232
// Throw a plain Error (no .stderr property) from the DOCKER-USER list check.
33-
// This forces the ternary on line 85 to evaluate the `''` (else) branch.
33+
// This forces the stderr lookup to use the empty-string fallback.
3434
mockedExeca
3535
.mockResolvedValueOnce(execaResult({ stdout: 'fw-bridge', exitCode: 0 })) // getNetworkBridgeName
36+
.mockResolvedValueOnce(execaResult({ exitCode: 0 })) // iptables --version
3637
.mockRejectedValueOnce(new Error('iptables: table locked')) // DOCKER-USER check – no stderr
3738
.mockResolvedValueOnce(execaResult({ exitCode: 0 })) // iptables -N DOCKER-USER
3839
.mockResolvedValueOnce(execaResult({ exitCode: 1 })); // FW_WRAPPER existence check

src/host-iptables-doh.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ describe('host-iptables (doh)', () => {
1010
mockedExeca
1111
// Mock getNetworkBridgeName
1212
.mockResolvedValueOnce(execaResult({ stdout: 'fw-bridge', stderr: '', exitCode: 0 }))
13+
// Mock iptables --version
14+
.mockResolvedValueOnce(execaResult({ stdout: '', stderr: '', exitCode: 0 }))
1315
// Mock iptables -L DOCKER-USER (permission check)
1416
.mockResolvedValueOnce(execaResult({ stdout: '', stderr: '', exitCode: 0 }))
1517
// Mock chain existence check (doesn't exist)
@@ -49,6 +51,7 @@ describe('host-iptables (doh)', () => {
4951
mockedExeca
5052
.mockResolvedValueOnce(execaResult({ stdout: 'fw-bridge', stderr: '', exitCode: 0 }))
5153
.mockResolvedValueOnce(execaResult({ stdout: '', stderr: '', exitCode: 0 }))
54+
.mockResolvedValueOnce(execaResult({ stdout: '', stderr: '', exitCode: 0 }))
5255
.mockResolvedValueOnce(execaResult({ exitCode: 1 }));
5356

5457
mockedExeca.mockResolvedValue(execaResult({

src/host-iptables-rules.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,21 @@ function isValidPortSpec(spec: string): boolean {
5656
// ts-prune-ignore-next
5757
export const iptablesRulesTestHelpers = { isValidPortSpec };
5858

59+
function getErrorStringProperty(error: unknown, property: string): string {
60+
return typeof error === 'object'
61+
&& error !== null
62+
&& property in error
63+
&& typeof (error as Record<string, unknown>)[property] === 'string'
64+
? (error as Record<string, unknown>)[property] as string
65+
: '';
66+
}
67+
68+
function isMissingIptablesError(error: unknown): boolean {
69+
const code = getErrorStringProperty(error, 'code');
70+
const message = error instanceof Error ? error.message : '';
71+
return code === 'ENOENT' || message.includes('ENOENT') || message.includes('not found');
72+
}
73+
5974
function parseValidPortSpecs(input: string | undefined, label: string): string[] {
6075
if (!input) {
6176
return [];
@@ -78,13 +93,23 @@ function parseValidPortSpecs(input: string | undefined, label: string): string[]
7893
}
7994

8095
async function checkPermissionsAndSetupChain(chain: string): Promise<void> {
96+
try {
97+
await execa('iptables', ['--version'], { timeout: 5000 });
98+
} catch (error: unknown) {
99+
if (isMissingIptablesError(error)) {
100+
throw new Error('iptables is required but was not found. Please install iptables and try again.');
101+
}
102+
throw error;
103+
}
104+
81105
// Check if we have permission to run iptables commands
82106
try {
83107
await execa('iptables', ['-t', 'filter', '-L', 'DOCKER-USER', '-n'], { timeout: 5000 });
84108
} catch (error: unknown) {
85-
const stderr = typeof error === 'object' && error !== null && 'stderr' in error && typeof error.stderr === 'string'
86-
? error.stderr
87-
: '';
109+
if (isMissingIptablesError(error)) {
110+
throw new Error('iptables is required but was not found. Please install iptables and try again.');
111+
}
112+
const stderr = getErrorStringProperty(error, 'stderr');
88113
if (stderr.includes('Permission denied')) {
89114
throw new Error(
90115
'Permission denied: iptables commands require root privileges. ' +

src/host-iptables-setup.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { API_PROXY_PORTS } from './types';
22
import {
33
execaError,
4+
execaMissingCommandError,
45
execaResult,
56
mockedExeca,
67
setupDefaultIptablesMocks,
@@ -28,6 +29,8 @@ describe('host-iptables (setup)', () => {
2829
stderr: '',
2930
exitCode: 0,
3031
}))
32+
// Mock iptables --version
33+
.mockResolvedValueOnce(execaResult())
3134
// Mock iptables -L DOCKER-USER (permission check)
3235
.mockRejectedValueOnce(permissionError);
3336

@@ -36,6 +39,21 @@ describe('host-iptables (setup)', () => {
3639
);
3740
});
3841

42+
it('should throw a clear error if iptables is not installed', async () => {
43+
mockedExeca
44+
// Mock getNetworkBridgeName
45+
.mockResolvedValueOnce(execaResult({ stdout: 'fw-bridge', stderr: '', exitCode: 0 }))
46+
// Mock iptables --version (missing binary)
47+
.mockRejectedValueOnce(execaMissingCommandError());
48+
49+
await expect(setupHostIptables('172.30.0.10', 3128, ['8.8.8.8', '8.8.4.4'])).rejects.toThrow(
50+
'iptables is required but was not found'
51+
);
52+
53+
expect(mockedExeca).not.toHaveBeenCalledWith('iptables', ['-t', 'filter', '-L', 'DOCKER-USER', '-n'], { timeout: 5000 });
54+
expect(mockedExeca).not.toHaveBeenCalledWith('iptables', ['-t', 'filter', '-N', 'DOCKER-USER']);
55+
});
56+
3957
it('should create FW_WRAPPER chain and add rules', async () => {
4058
setupDefaultIptablesMocks({ catchAllStdout: 'Chain DOCKER-USER\nChain FW_WRAPPER' });
4159

@@ -218,6 +236,8 @@ describe('host-iptables (setup)', () => {
218236
mockedExeca
219237
// Mock getNetworkBridgeName
220238
.mockResolvedValueOnce(execaResult({ stdout: 'fw-bridge', stderr: '', exitCode: 0 }))
239+
// Mock iptables --version
240+
.mockResolvedValueOnce(execaResult())
221241
// Mock iptables -L DOCKER-USER (chain doesn't exist)
222242
.mockRejectedValueOnce(noChainError)
223243
// Mock iptables -N DOCKER-USER (create chain)
@@ -475,6 +495,8 @@ describe('host-iptables (setup)', () => {
475495
mockedExeca
476496
// getNetworkBridgeName
477497
.mockResolvedValueOnce(execaResult({ stdout: 'fw-bridge', stderr: '', exitCode: 0 }))
498+
// iptables --version
499+
.mockResolvedValueOnce(execaResult())
478500
// iptables -L DOCKER-USER (chain doesn't exist)
479501
.mockRejectedValueOnce(noChainError)
480502
// iptables -N DOCKER-USER (creation fails)
@@ -512,6 +534,8 @@ describe('host-iptables (setup)', () => {
512534
mockedExeca
513535
// getNetworkBridgeName
514536
.mockResolvedValueOnce(execaResult({ stdout: 'fw-bridge', exitCode: 0 }))
537+
// iptables --version
538+
.mockResolvedValueOnce(execaResult({ exitCode: 0, stdout: '' }))
515539
// iptables -L DOCKER-USER (permission check) — success
516540
.mockResolvedValueOnce(execaResult({ exitCode: 0, stdout: '' }))
517541
// iptables -L FW_WRAPPER (check if chain exists) — exists

src/test-helpers/host-iptables-test-setup.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ export function execaError(message: string, stderr = message): ExecaMockError {
5656
return Object.assign(new Error(message), { stderr });
5757
}
5858

59+
// ts-prune-ignore-next
60+
export function execaMissingCommandError(command = 'iptables'): ExecaMockError & { code: string } {
61+
return Object.assign(new Error(`spawn ${command} ENOENT`), { code: 'ENOENT' });
62+
}
63+
5964
// ts-prune-ignore-next
6065
export function setupHostIptablesTestSuite(resetIpv6State: () => void): void {
6166
beforeEach(() => {
@@ -78,6 +83,7 @@ export function setupDefaultIptablesMocks(
7883
mockedExeca
7984
.mockResolvedValueOnce(execaResult({ stdout: bridgeName, exitCode: 0 }))
8085
.mockResolvedValueOnce(execaResult({ stdout: '', exitCode: 0 }))
86+
.mockResolvedValueOnce(execaResult({ stdout: '', exitCode: 0 }))
8187
.mockResolvedValueOnce(execaResult({ exitCode: chainExists ? 0 : 1 }));
8288
mockedExeca.mockImplementation(((cmd: string, args: readonly string[]) => {
8389
if (cmd === 'iptables' && Array.isArray(args) && args.includes('-C')) {

0 commit comments

Comments
 (0)