From b23c1b56a84f4067f4aee3fa32b4cdcab7f73011 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 13:36:50 +0000 Subject: [PATCH 1/4] Initial plan From 11babe4ca3f92ad2e37032e88b6b1f8fe8d09e36 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 14:02:05 +0000 Subject: [PATCH 2/4] refactor: extract reportBlockedDomains to remove duplicate logic --- src/container-lifecycle.ts | 120 ++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 68 deletions(-) diff --git a/src/container-lifecycle.ts b/src/container-lifecycle.ts index ff38a536..d72068f8 100644 --- a/src/container-lifecycle.ts +++ b/src/container-lifecycle.ts @@ -506,6 +506,56 @@ export async function startContainers(workDir: string, allowedDomains: string[], } } +/** + * Classifies and logs each blocked target, then emits actionable fix suggestions. + * Extracted to avoid duplicating this logic between the startup-error path + * (which uses `logger.error`) and the post-run warning path (which uses `logger.warn`). + * + * @param blockedTargets - Targets that were denied by the firewall + * @param allowedDomains - Domains currently in the allowlist + * @param log - Logging function to use (e.g. `logger.error` or `logger.warn`) + * @returns The categorised lists so callers can decide on further action + */ +function reportBlockedDomains( + blockedTargets: BlockedTarget[], + allowedDomains: string[], + log: (msg: string) => void, +): { missingDomains: string[]; portIssues: BlockedTarget[] } { + const missingDomains: string[] = []; + const portIssues: BlockedTarget[] = []; + + blockedTargets.forEach(blocked => { + const isAllowed = allowedDomains.some(allowed => + blocked.domain === allowed || blocked.domain.endsWith('.' + allowed) + ); + + if (!isAllowed) { + // Domain not in allowlist + log(` - Blocked: ${blocked.target} (domain not in allowlist)`); + missingDomains.push(blocked.domain); + } else if (blocked.port && blocked.port !== '80' && blocked.port !== '443') { + // Domain is allowed but port is not + log(` - Blocked: ${blocked.target} (port ${blocked.port} not allowed, only 80 and 443 are permitted)`); + portIssues.push(blocked); + } else { + // Other reason (shouldn't happen often) + log(` - Blocked: ${blocked.target}`); + } + }); + + log('Allowed domains:'); + allowedDomains.forEach(domain => { log(` - Allowed: ${domain}`); }); + + if (missingDomains.length > 0) { + log(`To fix domain issues: --allow-domains "${[...allowedDomains, ...missingDomains].join(',')}"`); + } + if (portIssues.length > 0) { + log('To fix port issues: Use standard ports 80 (HTTP) or 443 (HTTPS)'); + } + + return { missingDomains, portIssues }; +} + /** * Runs the Squid-log diagnostic check and re-throws with a user-friendly message * when blocked domains are found, or rethrows the original error otherwise. @@ -522,40 +572,7 @@ async function handleHealthcheckError( if (hasDenials) { logger.error('Firewall blocked domains during startup:'); - - const missingDomains: string[] = []; - const portIssues: BlockedTarget[] = []; - - blockedTargets.forEach(blocked => { - const isAllowed = allowedDomains.some(allowed => - blocked.domain === allowed || blocked.domain.endsWith('.' + allowed) - ); - - if (!isAllowed) { - // Domain not in allowlist - logger.error(` - Blocked: ${blocked.target} (domain not in allowlist)`); - missingDomains.push(blocked.domain); - } else if (blocked.port && blocked.port !== '80' && blocked.port !== '443') { - // Domain is allowed but port is not - logger.error(` - Blocked: ${blocked.target} (port ${blocked.port} not allowed, only 80 and 443 are permitted)`); - portIssues.push(blocked); - } else { - // Other reason (shouldn't happen often) - logger.error(` - Blocked: ${blocked.target}`); - } - }); - - logger.error('Allowed domains:'); - allowedDomains.forEach(domain => { - logger.error(` - Allowed: ${domain}`); - }); - - if (missingDomains.length > 0) { - logger.error(`To fix domain issues: --allow-domains "${[...allowedDomains, ...missingDomains].join(',')}"`); - } - if (portIssues.length > 0) { - logger.error('To fix port issues: Use standard ports 80 (HTTP) or 443 (HTTPS)'); - } + reportBlockedDomains(blockedTargets, allowedDomains, msg => logger.error(msg)); // Create a more user-friendly error const blockedList = blockedTargets.map(b => `"${b.target}"`).join(', '); @@ -642,40 +659,7 @@ export async function runAgentCommand(workDir: string, allowedDomains: string[], // If command failed (non-zero exit) and domains were blocked, show a warning if (exitCode !== 0 && hasDenials) { logger.warn('Firewall blocked domains:'); - - const missingDomains: string[] = []; - const portIssues: BlockedTarget[] = []; - - blockedTargets.forEach(blocked => { - const isAllowed = allowedDomains.some(allowed => - blocked.domain === allowed || blocked.domain.endsWith('.' + allowed) - ); - - if (!isAllowed) { - // Domain not in allowlist - logger.warn(` - Blocked: ${blocked.target} (domain not in allowlist)`); - missingDomains.push(blocked.domain); - } else if (blocked.port && blocked.port !== '80' && blocked.port !== '443') { - // Domain is allowed but port is not - logger.warn(` - Blocked: ${blocked.target} (port ${blocked.port} not allowed, only 80 and 443 are permitted)`); - portIssues.push(blocked); - } else { - // Other reason (shouldn't happen often) - logger.warn(` - Blocked: ${blocked.target}`); - } - }); - - logger.warn('Allowed domains:'); - allowedDomains.forEach(domain => { - logger.warn(` - Allowed: ${domain}`); - }); - - if (missingDomains.length > 0) { - logger.warn(`To fix domain issues: --allow-domains "${[...allowedDomains, ...missingDomains].join(',')}"`); - } - if (portIssues.length > 0) { - logger.warn('To fix port issues: Use standard ports 80 (HTTP) or 443 (HTTPS)'); - } + reportBlockedDomains(blockedTargets, allowedDomains, msg => logger.warn(msg)); } return { exitCode, blockedDomains: blockedTargets.map(b => b.domain) }; From 212d465fe57d4fc1860d06fd14bb14742ca2a12a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 14:03:03 +0000 Subject: [PATCH 3/4] refactor: fix spelling categorised -> categorized in JSDoc Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a4ea6ef3-71ec-4c40-821b-80aaf7378c48 --- src/container-lifecycle.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/container-lifecycle.ts b/src/container-lifecycle.ts index d72068f8..304af9e8 100644 --- a/src/container-lifecycle.ts +++ b/src/container-lifecycle.ts @@ -514,7 +514,7 @@ export async function startContainers(workDir: string, allowedDomains: string[], * @param blockedTargets - Targets that were denied by the firewall * @param allowedDomains - Domains currently in the allowlist * @param log - Logging function to use (e.g. `logger.error` or `logger.warn`) - * @returns The categorised lists so callers can decide on further action + * @returns The categorized lists so callers can decide on further action */ function reportBlockedDomains( blockedTargets: BlockedTarget[], From 97239ab800615326d61a00fc4c2a941a4002b57d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 18:46:24 +0000 Subject: [PATCH 4/4] fix: handle wildcards/protocol-prefixes, dedup suggestions, add tests --- src/container-lifecycle.ts | 23 +++++-- src/docker-manager-lifecycle.test.ts | 98 ++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/src/container-lifecycle.ts b/src/container-lifecycle.ts index d9a21de0..3c5d344e 100644 --- a/src/container-lifecycle.ts +++ b/src/container-lifecycle.ts @@ -5,6 +5,7 @@ import execa from 'execa'; import { WrapperConfig, BlockedTarget } from './types'; import { logger } from './logger'; import { generateSquidConfig, generatePolicyManifest } from './squid-config'; +import { parseDomainWithProtocol, isWildcardPattern, wildcardToRegex } from './domain-patterns'; import { generateSessionCa, initSslDb, parseUrlPatterns } from './ssl-bump'; import { SQUID_PORT, @@ -523,18 +524,29 @@ function reportBlockedDomains( allowedDomains: string[], log: (msg: string) => void, ): { missingDomains: string[]; portIssues: BlockedTarget[] } { - const missingDomains: string[] = []; + const uniqueMissingDomains = new Set(); const portIssues: BlockedTarget[] = []; blockedTargets.forEach(blocked => { - const isAllowed = allowedDomains.some(allowed => - blocked.domain === allowed || blocked.domain.endsWith('.' + allowed) - ); + const isAllowed = allowedDomains.some(allowed => { + // Strip any protocol prefix (e.g. "https://github.com" -> "github.com") + const normalizedAllowed = parseDomainWithProtocol(allowed).domain; + if (isWildcardPattern(normalizedAllowed)) { + // Wildcard pattern match (e.g. "*.github.com") + try { + return new RegExp(wildcardToRegex(normalizedAllowed), 'i').test(blocked.domain); + } catch { + return false; + } + } + // Exact match or subdomain match + return blocked.domain === normalizedAllowed || blocked.domain.endsWith('.' + normalizedAllowed); + }); if (!isAllowed) { // Domain not in allowlist log(` - Blocked: ${blocked.target} (domain not in allowlist)`); - missingDomains.push(blocked.domain); + uniqueMissingDomains.add(blocked.domain); } else if (blocked.port && blocked.port !== '80' && blocked.port !== '443') { // Domain is allowed but port is not log(` - Blocked: ${blocked.target} (port ${blocked.port} not allowed, only 80 and 443 are permitted)`); @@ -548,6 +560,7 @@ function reportBlockedDomains( log('Allowed domains:'); allowedDomains.forEach(domain => { log(` - Allowed: ${domain}`); }); + const missingDomains = [...uniqueMissingDomains]; if (missingDomains.length > 0) { log(`To fix domain issues: --allow-domains "${[...allowedDomains, ...missingDomains].join(',')}"`); } diff --git a/src/docker-manager-lifecycle.test.ts b/src/docker-manager-lifecycle.test.ts index 0babaff6..45ab192e 100644 --- a/src/docker-manager-lifecycle.test.ts +++ b/src/docker-manager-lifecycle.test.ts @@ -1,4 +1,5 @@ import { startContainers, stopContainers, fastKillAgentContainer, isAgentExternallyKilled, resetAgentExternallyKilled, AGENT_CONTAINER_NAME, runAgentCommand, setAwfDockerHost } from './docker-manager'; +import { logger } from './logger'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -594,5 +595,102 @@ describe('docker-manager lifecycle', () => { expect(result.exitCode).toBe(143); expect(result.blockedDomains).toEqual([]); }); + + it('should recognize domains matched by a wildcard allowlist entry', async () => { + const squidLogsDir = path.join(testDir, 'squid-logs'); + fs.mkdirSync(squidLogsDir, { recursive: true }); + // api.github.com is blocked on a non-standard port + fs.writeFileSync( + path.join(squidLogsDir, 'access.log'), + '1760994429.358 172.30.0.20:36274 api.github.com:8443 -:- 1.1 CONNECT 403 TCP_DENIED:HIER_NONE api.github.com:8443 "curl/7.81.0"\n' + ); + + mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any); // docker logs -f + mockExecaFn.mockResolvedValueOnce({ stdout: '1', stderr: '', exitCode: 0 } as any); // docker wait + + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + try { + await runAgentCommand(testDir, ['*.github.com']); + // *.github.com covers api.github.com, so the message should report a port issue, not a missing domain + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('port 8443 not allowed')); + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('domain not in allowlist')); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should recognize domains matched by a protocol-prefixed allowlist entry', async () => { + const squidLogsDir = path.join(testDir, 'squid-logs'); + fs.mkdirSync(squidLogsDir, { recursive: true }); + // github.com is listed as https://github.com; a non-standard port block should show as port issue + fs.writeFileSync( + path.join(squidLogsDir, 'access.log'), + '1760994429.358 172.30.0.20:36274 github.com:8080 -:- 1.1 CONNECT 403 TCP_DENIED:HIER_NONE github.com:8080 "curl/7.81.0"\n' + ); + + mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any); // docker logs -f + mockExecaFn.mockResolvedValueOnce({ stdout: '1', stderr: '', exitCode: 0 } as any); // docker wait + + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + try { + await runAgentCommand(testDir, ['https://github.com']); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('port 8080 not allowed')); + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('domain not in allowlist')); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should deduplicate domains in --allow-domains suggestion', async () => { + const squidLogsDir = path.join(testDir, 'squid-logs'); + fs.mkdirSync(squidLogsDir, { recursive: true }); + // Same domain blocked on two different ports — should appear once in the suggestion + fs.writeFileSync( + path.join(squidLogsDir, 'access.log'), + '1760994429.358 172.30.0.20:36274 missing.com:80 -:- 1.1 GET 403 TCP_DENIED:HIER_NONE missing.com:80 "curl/7.81.0"\n' + + '1760994430.000 172.30.0.20:36275 missing.com:443 -:- 1.1 CONNECT 403 TCP_DENIED:HIER_NONE missing.com:443 "curl/7.81.0"\n' + ); + + mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any); // docker logs -f + mockExecaFn.mockResolvedValueOnce({ stdout: '1', stderr: '', exitCode: 0 } as any); // docker wait + + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + try { + await runAgentCommand(testDir, ['github.com']); + const suggestionCalls = warnSpy.mock.calls.filter(([msg]) => + typeof msg === 'string' && msg.includes('--allow-domains') + ); + expect(suggestionCalls).toHaveLength(1); + const suggestion = suggestionCalls[0][0] as string; + // missing.com should appear exactly once in the suggestion + const occurrences = (suggestion.match(/missing\.com/g) ?? []).length; + expect(occurrences).toBe(1); + } finally { + warnSpy.mockRestore(); + } + }); + + it('should use logger.warn (not logger.error) for post-run blocked-domain diagnostics', async () => { + const squidLogsDir = path.join(testDir, 'squid-logs'); + fs.mkdirSync(squidLogsDir, { recursive: true }); + fs.writeFileSync( + path.join(squidLogsDir, 'access.log'), + '1760994429.358 172.30.0.20:36274 blocked.com:443 -:- 1.1 CONNECT 403 TCP_DENIED:HIER_NONE blocked.com:443 "curl/7.81.0"\n' + ); + + mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any); // docker logs -f + mockExecaFn.mockResolvedValueOnce({ stdout: '1', stderr: '', exitCode: 0 } as any); // docker wait + + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + const errorSpy = jest.spyOn(logger, 'error').mockImplementation(() => {}); + try { + await runAgentCommand(testDir, ['github.com']); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('blocked.com')); + expect(errorSpy).not.toHaveBeenCalledWith(expect.stringContaining('blocked.com')); + } finally { + warnSpy.mockRestore(); + errorSpy.mockRestore(); + } + }); }); });