fix: filter AWF-internal hostnames from blocked-domain reports - #6689
Conversation
Squid logs CONNECT attempts to internal Docker network addresses (e.g. awmgmcpg/awmg-mcpg, 172.30.0.*) as TCP_DENIED when they bypass NO_PROXY or lack a Squid ACL entry. These are container-to-container connections — not missing external dependencies — but the log aggregator and squid-log-reader were surfacing them as blocked external domains in the firewall warning and in awf logs stats/summary output. Add src/logs/internal-domain-filter.ts with isInternalAwfDomain() which returns true for: 1. IPv4 addresses on the AWF Docker network (172.30.0.0/24) 2. Single-label hostnames (no dots, not IPv6, not Squid placeholder) Apply the filter in squid-log-reader.ts and log-aggregator.ts. Closes #6685
There was a problem hiding this comment.
Pull request overview
Filters AWF-internal traffic from blocked-domain warnings and log statistics.
Changes:
- Adds internal IP/hostname detection.
- Applies filtering to runtime warnings and aggregated statistics.
- Adds unit coverage for filtering behavior.
Show a summary per file
| File | Description |
|---|---|
src/logs/internal-domain-filter.ts |
Identifies internal destinations. |
src/logs/internal-domain-filter.test.ts |
Tests destination classification. |
src/squid-log-reader.ts |
Filters runtime blocked targets. |
src/squid-log-reader.test.ts |
Tests warning filtering. |
src/logs/log-aggregator.ts |
Filters denied internal traffic from statistics. |
src/logs/log-aggregator.test.ts |
Tests aggregated results. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/logs/internal-domain-filter.ts:77
- Treating every dot-free name as AWF-internal hides legitimate blocked dependencies. AWF accepts custom DNS servers, and such a server can resolve an exact single-label private service (for example
registry); that denied request will now disappear from bothblockedDomainsand log statistics even though it is not an AWF container. Please restrict this branch to known/resolved topology peer names rather than inferring ownership from the absence of a dot.
// Category 2: Single-label hostname — no dots means a Docker container name,
// never a valid public internet domain.
// Exclusions:
// '-' — Squid's placeholder for an unknown/missing host; not a hostname.
// IPv6 — bracketed ("[::1]") or bare ("::1") addresses contain colons or
// brackets and have no dots, but are not Docker container names.
if (
domain.length > 0 &&
domain !== '-' &&
!domain.includes('.') &&
!domain.includes(':') &&
!domain.startsWith('[')
) {
return true;
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Medium
| * 2. **Single-label hostnames** (no dots): Docker assigns container-scoped DNS | ||
| * names that contain no dots (e.g. `awmgmcpg`, `awmg-mcpg`). Every valid | ||
| * public internet hostname has at least one dot. A denial for a dot-free name | ||
| * is always Docker container-to-container traffic, not an external dependency. |
|
@copilot address review feedback |
Topology peer names are not guaranteed to be single-label (dot-free). For example, `--topology-attach mcp.gateway-01` produces a peer name with a dot, which the existing single-label heuristic cannot detect. Changes: - Add `topologyPeers?: string[]` to `PolicyManifest` so configured peers are written to policy-manifest.json and can be read back at log-analysis time. - Update `generatePolicyManifest()` to include the topology peer list. - Add optional `knownTopologyPeers?: ReadonlySet<string>` param to `isInternalAwfDomain()` (category 2 check). When present, a case- insensitive exact match against the peer set filters dotted names (e.g. `mcp.gateway-01`) that the single-label heuristic misses. - `checkSquidLogs()` now reads topology peers from `<workDir>/audit/policy-manifest.json` and passes them to the filter. - `aggregateLogs()` and `loadAndAggregate()` accept optional topology peers forwarded to `isInternalAwfDomain()`. - `loadLogsWithErrorHandling()` reads the policy manifest before calling `loadAndAggregate()` to pass topology peers at aggregation time. - Tests cover dotted topology peer filtering and case-insensitive matching; existing tests updated for new call signatures.
Fixed in the latest commit. The approach now:
The single-label heuristic is retained as a Category 3 fallback for well-known AWF container names (e.g. |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 99.17% | 99.14% | 📉 -0.03% |
| Statements | 99.07% | 99.02% | 📉 -0.05% |
| Functions | 99.45% | 99.24% | 📉 -0.21% |
| Branches | 95.75% | 95.58% | 📉 -0.17% |
📁 Per-file Coverage Changes (4 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/squid-log-reader.ts |
100.0% → 93.1% (-6.90%) | 100.0% → 91.5% (-8.48%) |
src/commands/logs-command-helpers.ts |
100.0% → 98.4% (-1.62%) | 100.0% → 98.4% (-1.59%) |
src/logs/log-aggregator.ts |
97.3% → 97.4% (+0.10%) | 97.4% → 97.5% (+0.10%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
✨ New Files (1 files)
src/logs/internal-domain-filter.ts: 100.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Claude passed |
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test completed with FAIL status. |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
🚀 Security Guard has started processing this pull request |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
✅ Contribution Check completed successfully! Contribution check complete: no issues found. The PR includes tests for the new behavior, has a clear description with a related issue reference, and keeps new files under the expected src/ layout. |
|
🔌 Smoke Services — All services reachable! ✅ |
|
Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
github-actions
Bot
added
the
smoke-copilot-network-isolation
|
Smoke test: Copilot BYOK (Direct) — PR #6689 (
Running in direct BYOK mode ( Overall: PARTIAL — BYOK inference confirmed; pre-computed step outputs were not injected into prompt.
|
Smoke Test: Claude Engine Validation
Overall result: PASS ✅
|
Smoke Test Results: Gemini
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🔍 Smoke Test Results
Overall: PASS (core engine functional; pre-step template vars unexpanded) Author:
|
Smoke Test: Services Connectivity
Overall: FAIL —
|
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Chroot Version Comparison Results
Overall: ❌ FAILED — Node.js version mismatch (host
|
|
feat(api-proxy): use runtime model metadata and pricing overlays Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
🔥 Smoke Test: Docker Sbx — PASS
Overall: PASS —
|
Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. OTEL integration is fully implemented with proper graceful degradation when not configured.
|
AWF's firewall warning (and
awf logs stats/summary) incorrectly reported AWF-internal Docker container traffic — specifically the MCP Gateway sidecar (awmgmcpg/awmg-mcpg) and172.30.0.xIPs — as blocked external domains. Squid logs everyTCP_DENIEDregardless of whether the destination is internal container-to-container traffic or a genuinely missing external dependency.Changes
New
src/logs/internal-domain-filter.ts:isInternalAwfDomain(domain)returnstruefor two categories of AWF-internal destinations:172.30.0.0/24), checked againstNETWORK_SUBNETfrom the policy-(Squid placeholder) and IPv6 addresses.squid-log-reader.ts: SkipTCP_DENIEDentries matchingisInternalAwfDomainbefore adding toblockedTargets. This fixes both the runtime⚠️ Firewall blocked N domain(s)warning and theblockedDomainsreturn value consumed by gh-aw safe-outputs.log-aggregator.ts: Skip denied entries for internal domains inaggregateLogs(), removing them from request counts and domain stats. Allowed traffic to explicitly whitelisted internal peers is preserved in stats. Time-range tracking includes all entries (consistent with the existingtransaction-end-before-headersfilter).