fix: share mcpg network namespace to fix TLS hostname verification by lpcox · Pull Request #1778 · github/gh-aw-firewall

lpcox · 2026-04-08T01:04:37Z

mcpg's GenerateSelfSignedTLS generates certs with SANs for localhost and 127.0.0.1 only (source). The previous architecture had cli-proxy connecting to mcpg via its Docker network IP (172.30.0.51), which fails TLS hostname verification since that IP isn't in the cert's SANs.

Fix: `network_mode: service:cli-proxy-mcpg`

cli-proxy now shares mcpg's network namespace. localhost in cli-proxy resolves to mcpg, matching the cert's SAN.

src/docker-manager.ts
- cli-proxy uses network_mode: 'service:cli-proxy-mcpg' instead of its own networks config
- mcpg binds to 127.0.0.1:18443 (more restrictive than the Docker IP — only reachable from shared namespace)
- Healthcheck uses localhost (runs inside mcpg container, cert matches)
- Agent/iptables AWF_CLI_PROXY_IP and AWF_CLI_PROXY_URL point to mcpg's IP since cli-proxy shares it
- Removed AWF_MCPG_HOST from cli-proxy env (no longer needed)
containers/cli-proxy/entrypoint.sh
- GH_HOST=localhost:${MCPG_PORT} — TLS hostname verification passes
src/types.ts
- DockerService.networks made optional, added network_mode field (mutually exclusive per Docker Compose spec)

# Before: separate IPs, TLS hostname mismatch
cli-proxy-mcpg:
  listen: "172.30.0.51:18443"   # cert only covers localhost
cli-proxy:
  networks: { awf-net: { ipv4_address: 172.30.0.50 } }
  environment:
    GH_HOST: "172.30.0.51:18443"  # ❌ TLS verification fails

# After: shared namespace, localhost end-to-end
cli-proxy-mcpg:
  listen: "127.0.0.1:18443"
cli-proxy:
  network_mode: "service:cli-proxy-mcpg"
  environment:
    GH_HOST: "localhost:18443"    # ✅ matches cert SAN

github-actions · 2026-04-08T01:05:41Z

✅ Coverage Check Passed

Overall Coverage

Metric	Base	PR	Delta
Lines	86.14%	86.31%	📈 +0.17%
Statements	86.02%	86.18%	📈 +0.16%
Functions	87.45%	87.45%	➡️ +0.00%
Branches	78.81%	78.86%	📈 +0.05%

📁 Per-file Coverage Changes (1 files)

File	Lines (Before → After)	Statements (Before → After)
`src/docker-manager.ts`	86.3% → 86.9% (+0.59%)	85.9% → 86.4% (+0.58%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot

Pull request overview

This PR refactors the CLI proxy sidecar into a two-container setup (mcpg proxy + HTTP exec server) to avoid brittle mcpg binary extraction and to fix the release build failure caused by an incorrect mcpg binary path.

Changes:

Split CLI proxy into cli-proxy-mcpg (runs official gh-aw-mcpg image) + cli-proxy (Node.js server + gh CLI) with a shared named volume for TLS certs.
Update compose generation + tests to model the two-service dependency chain and token isolation.
Update predownload + CLI options to pull/parameterize the mcpg image, plus CI/test cleanup to remove the new container.

Show a summary per file

File	Description
`src/docker-manager.ts`	Generates an additional `cli-proxy-mcpg` service, shared TLS volume, and revised dependency wiring for CLI proxy.
`containers/cli-proxy/entrypoint.sh`	Stops starting mcpg in-process; waits for shared TLS certs and configures `gh` to target mcpg service.
`containers/cli-proxy/Dockerfile`	Removes multi-stage mcpg extraction; leaves HTTP server + `gh` CLI only.
`src/commands/predownload.ts`	Adds mcpg image resolution/pull when CLI proxy is enabled.
`src/cli.ts`	Updates help text and wires mcpg image option through predownload subcommand.
`src/types.ts`	Updates JSDoc to reflect mcpg as a separate container image.
`src/docker-manager.test.ts`	Updates tests to assert two-service architecture and token placement.
`src/commands/predownload.test.ts`	Extends tests to verify mcpg image is included/pulled.
`scripts/ci/cleanup.sh`	Ensures new mcpg container is removed during CI cleanup.
`tests/fixtures/cleanup.ts`	Ensures new mcpg container is removed in test cleanup fixture.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

containers/cli-proxy/entrypoint.sh:41

NODE_EXTRA_CA_CERTS only affects Node.js TLS; the gh CLI is a Go binary and will not use this environment variable. To ensure gh trusts the mcpg proxy’s self-signed CA, also set a CA path that Go honors (e.g., SSL_CERT_FILE/SSL_CERT_DIR) or install the CA into the container’s system trust store before starting the server.

# Configure gh CLI to route through the mcpg proxy (TLS, self-signed CA)
export GH_HOST="${MCPG_HOST}:${MCPG_PORT}"
export NODE_EXTRA_CA_CERTS="/tmp/proxy-tls/ca.crt"
export GH_REPO="${GH_REPO:-$GITHUB_REPOSITORY}"

Files reviewed: 10/10 changed files
Comments generated: 2

Copilot · 2026-04-08T01:10:37Z

containers/cli-proxy/entrypoint.sh


 # Configure gh CLI to route through the mcpg proxy (TLS, self-signed CA)
-export GH_HOST="localhost:18443"
+export GH_HOST="${MCPG_HOST}:${MCPG_PORT}"
 export NODE_EXTRA_CA_CERTS="/tmp/proxy-tls/ca.crt"
 export GH_REPO="${GH_REPO:-$GITHUB_REPOSITORY}"


The mcpg container healthcheck validates https://localhost:${MCPG_PORT} with the generated cert, but the CLI proxy configures GH_HOST to ${MCPG_HOST}:${MCPG_PORT} (typically an IP like 172.30.0.51). If the mcpg server cert is issued only for localhost (common for self-signed startup certs), gh will fail TLS hostname verification when connecting via IP. Consider aligning the hostname used by clients and healthchecks (e.g., use the service DNS name) and/or configuring mcpg to generate a cert whose SAN covers the chosen hostname.

This issue also appears on line 38 of the same file.

src/docker-manager.ts

github-actions · 2026-04-08T01:14:37Z

Security Review — PR #1778

Reviewed the architectural split of cli-proxy into awf-cli-proxy (HTTP server) + awf-cli-proxy-mcpg (DIFC proxy). The refactor has a clear improvement — GH_TOKEN is now isolated to a dedicated, minimal container — but two security regressions need addressing.

Finding 1 — mcpg now listens on all interfaces (was loopback-only)

File: src/docker-manager.ts (new mcpgService block, ~line 1649)

Before (old entrypoint.sh):

mcpg proxy \
  --listen 127.0.0.1:18443 \
  ...

After (new docker-manager.ts):

'--listen', `0.0.0.0:${mcpgPort}`,

Why this matters: In the old architecture, mcpg was a child process inside the cli-proxy container and only bound to loopback — no other container could reach it. In the new architecture, mcpg is a separate container at 172.30.0.51 listening on 0.0.0.0:18443, meaning it is reachable from every container on awf-net, including the agent container at 172.30.0.20.

The host-iptables.ts FW_WRAPPER chain has no explicit rule for 172.30.0.51, and Docker bridge networking typically allows intra-network container-to-container traffic. If the agent container can reach 172.30.0.51:18443 directly, it could attempt to use the mcpg endpoint (which holds GH_TOKEN) without going through the cli-proxy HTTP server's access controls.

The TLS self-signed cert provides a partial barrier (the agent container does not have ca.crt mounted), but it relies on the agent not bypassing TLS verification rather than a network-level control.

Suggested mitigation: Bind mcpg to 172.30.0.51 (its assigned IP) rather than 0.0.0.0, so only containers that already know the IP can reach it while making the intent explicit; and/or add an explicit FW_WRAPPER ACCEPT rule for traffic from 172.30.0.50 → 172.30.0.51:18443 paired with a DROP for all other sources to 172.30.0.51:18443.

Finding 2 — Fail-close GH_TOKEN enforcement removed

File: containers/cli-proxy/entrypoint.sh

Removed in this PR:

if [ -z "$GH_TOKEN" ]; then
  echo "[cli-proxy] ERROR: GH_TOKEN not set - refusing to start without mcpg DIFC enforcement"
  exit 1
fi

Why this matters: This was an explicit fail-close guard: if GH_TOKEN was absent, the entire sidecar refused to start rather than running with potentially unenforced DIFC policies. The guard is now removed with no equivalent replacement.

In docker-manager.ts the token is conditionally included:

...(config.githubToken && { GH_TOKEN: config.githubToken }),

If githubToken is not set, the mcpg container starts without GH_TOKEN. Whether mcpg then refuses to authenticate (fail-closed) or enters a degraded mode without DIFC enforcement is opaque from this code — it depends entirely on the mcpg binary's internal behavior, which can change independently of this repo.

Suggested mitigation: Either add an equivalent fail-close check in the awf-cli-proxy-mcpg container startup (e.g., a wrapper script that exits if GH_TOKEN is empty before exec-ing mcpg), or add a guard in docker-manager.ts that refuses to generate the cli-proxy-mcpg service when githubToken is absent.

No issues found in

Capability dropping: both new containers use cap_drop: ['ALL'] + no-new-privileges:true ✓
Resource limits: mem_limit, pids_limit, cpu_shares all present on mcpg service ✓
Squid routing: mcpg container has HTTP_PROXY/HTTPS_PROXY set to Squid — domain whitelisting enforced ✓
Image reference validation: validateImageReference(mcpgImage) called before pull ✓
GH_TOKEN scope: removed from cli-proxy HTTP server container (improvement) ✓
Guard policy construction: functionally equivalent to old entrypoint logic ✓

Generated by Security Guard for issue #1778 · ● 170.9K · ◷

lpcox · 2026-04-08T01:56:44Z

@copilot apply changes based on the comments in this thread

github-actions · 2026-04-08T02:03:55Z

🏗️ Build Test Suite Results

Ecosystem	Project	Build/Install	Tests	Status
Bun	elysia	✅	1/1 passed	✅ PASS
Bun	hono	✅	1/1 passed	✅ PASS
C++	fmt	✅	N/A	✅ PASS
C++	json	✅	N/A	✅ PASS
Deno	oak	N/A	1/1 passed	✅ PASS
Deno	std	N/A	1/1 passed	✅ PASS
.NET	hello-world	✅	N/A	✅ PASS
.NET	json-parse	✅	N/A	✅ PASS
Go	color	✅	1/1 passed	✅ PASS
Go	env	✅	1/1 passed	✅ PASS
Go	uuid	✅	1/1 passed	✅ PASS
Java	gson	✅	1/1 passed	✅ PASS
Java	caffeine	✅	1/1 passed	✅ PASS
Node.js	clsx	✅	All passed	✅ PASS
Node.js	execa	✅	All passed	✅ PASS
Node.js	p-limit	✅	All passed	✅ PASS
Rust	fd	✅	1/1 passed	✅ PASS
Rust	zoxide	✅	1/1 passed	✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #1778 · ● 556.8K · ◷

Address security review findings from #1778: 1. Bind mcpg to its assigned IP (172.30.0.51) instead of 0.0.0.0 so the agent container cannot reach mcpg directly. Previously mcpg listened on all interfaces, making it reachable from any container on awf-net. 2. Add fail-close guard: generateDockerCompose now throws if enableCliProxy is set but githubToken is absent. mcpg requires a token to enforce DIFC policies — running without one would bypass integrity checks. 3. Use mcpg IP in healthcheck (not localhost) for TLS hostname consistency with how cli-proxy connects via GH_HOST. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-04-08T02:07:51Z

Security Review — PR #1778

Reviewed the split of cli-proxy into separate awf-cli-proxy (Node.js HTTP server) and awf-cli-proxy-mcpg (DIFC proxy) containers.

🔍 Finding: `cli-proxy` HTTP server container no longer routes through Squid

File: src/docker-manager.ts
Severity: Moderate (defense-in-depth reduction)

The old cli-proxy container had HTTP_PROXY/HTTPS_PROXY pointing to Squid, ensuring any outbound HTTP/HTTPS calls were ACL-filtered. The new architecture removes these from cli-proxy and places them only on cli-proxy-mcpg:

 // cli-proxy environment (NEW)
 AWF_MCPG_HOST: mcpgIp,
 AWF_MCPG_PORT: String(mcpgPort),
 GITHUB_REPOSITORY: ...,
 AWF_CLI_PROXY_WRITABLE: ...,
 NO_PROXY: 'localhost,127.0.0.1,::1',
-HTTP_PROXY: `(redacted)   // removed
-HTTPS_PROXY: `(redacted)  // removed
-https_proxy: `(redacted)  // removed

The primary traffic path — gh CLI → GH_HOST → mcpg → Squid → GitHub — is correct and Squid-filtered. However, the cli-proxy container itself (which has gh CLI, curl, bash, and a Node.js runtime) no longer has a Squid backstop.

Why this matters: If the gh CLI in the cli-proxy container ever falls back to a direct API connection (e.g., GH_HOST is unreachable, connection refused), those requests would bypass the Squid domain ACL entirely. Any other HTTP-capable tool running in the container (npm, curl during init scripts, etc.) also lacks filtering.

Suggested fix: Add HTTP_PROXY/HTTPS_PROXY to the cli-proxy service environment as a defense-in-depth measure, even if the primary path goes through mcpg. This matches the firewall's stated defense-in-depth strategy and the pattern used by other sidecars:

// In the cli-proxy service environment block:
HTTP_PROXY: `(redacted)
HTTPS_PROXY: `(redacted)
https_proxy: `(redacted)

✅ Items reviewed and found acceptable

Change	Assessment
`GH_TOKEN` moved from `cli-proxy` to `cli-proxy-mcpg` only	Improvement — tighter credential scoping
`mcpg` listening on `0.0.0.0:18443` (vs. in-process `127.0.0.1:18443`)	Acceptable — agent's filter chain has a default TCP DROP; `172.30.0.51` has no explicit ACCEPT, so agent→mcpg is blocked
`cli-proxy` waits for shared TLS cert volume instead of locally-generated cert	Acceptable — exits with code 1 if cert not present within 30 s
Removal of `GH_TOKEN` check from `cli-proxy` entrypoint	Acceptable — responsibility correctly shifted to `cli-proxy-mcpg`'s own startup enforcement
`cli-proxy-tls` named volume sharing TLS certs between `mcpg` and `cli-proxy`	Acceptable — read-write in mcpg, read-only in cli-proxy (`:ro`)
`MCPG_HOST` fallback to `localhost` in entrypoint	Low risk — fails functionally (no listener), not a bypass
No changes to `setup-iptables.sh`, `src/host-iptables.ts`, `src/squid-config.ts`, seccomp profile, or capability drops	No regression

Bottom line: The architectural direction is sound and the GH_TOKEN isolation improvement is a genuine security win. The one concrete gap worth addressing before merge is restoring HTTP_PROXY/HTTPS_PROXY on the cli-proxy container to preserve the defense-in-depth property that all sidecar containers route outbound traffic through Squid.

Generated by Security Guard for issue #1778 · ● 187.5K · ◷

Copilot · 2026-04-08T02:09:50Z

@copilot apply changes based on the comments in this thread

Applied in 413d75d. The mcpg self-signed TLS cert only covers localhost/127.0.0.1 SANs (verified from mcpg source at internal/proxy/tls.go). The fix uses network_mode: service:cli-proxy-mcpg so cli-proxy shares mcpg's network namespace — localhost resolves to mcpg and TLS hostname verification passes end-to-end.

Key changes:

mcpg healthcheck reverted to localhost (runs inside mcpg container)
cli-proxy uses network_mode: service:cli-proxy-mcpg instead of a separate IP
GH_HOST=localhost:${MCPG_PORT} in entrypoint (cert SAN matches)
Agent/iptables use mcpg's IP (172.30.0.51) to reach cli-proxy

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/b1a5ac57-6103-45c6-b689-67924f7df25b

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/1831b666-eb93-4772-9455-4604a64bfd24 Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

github-actions · 2026-04-08T02:40:55Z

Smoke Test Results

✅ GitHub MCP: fix: add retry logic to apt-get upgrade in agent Dockerfile / perf: optimize build-test workflow token usage (~70-80% reduction)
✅ Playwright: GitHub page title contains "GitHub"
✅ File write: /tmp/gh-aw/agent/smoke-test-claude-24114638692.txt created
✅ Bash: file content verified

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude

github-actions · 2026-04-08T02:41:28Z

🔬 Smoke Test Results

Test	Result
GitHub MCP connectivity	✅ PR #1781 fetched successfully
GitHub.com HTTP	✅ HTTP 200
File write/read	✅ `smoke-test-copilot-24114638723.txt` verified

Overall: PASS

PR by @lpcox · Assignees: none

📰 BREAKING: Report filed by Smoke Copilot

github-actions · 2026-04-08T02:42:30Z

Smoke Test: GitHub Actions Services Connectivity ✅

All checks passed:

Check	Result
Redis PING (`host.docker.internal:6379`)	✅ `PONG`
`pg_isready` (`host.docker.internal:5432`)	✅ accepting connections
`psql SELECT 1` (`smoketest` db, user `postgres`)	✅ returned `1`

Note: redis-cli was unavailable; Redis was tested via raw socket (RESP protocol) confirming +PONG.

🔌 Service connectivity validated by Smoke Services

github-actions · 2026-04-08T02:42:56Z

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.13	Python 3.12.3	❌
Node.js	v24.14.1	v20.20.2	❌
Go	go1.22.12	go1.22.12	✅

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

github-actions · 2026-04-08T12:48:40Z

Smoke test results:
PRs: "fix: share mcpg network namespace to fix TLS hostname verification" | "fix: add retry logic to apt-get upgrade in agent Dockerfile"
GitHub MCP: ✅
safeinputs-gh CLI: ❌
Playwright title check: ❌
Tavily search: ❌
File write/read: ✅
Bash verification: ✅
Discussion interaction: ❌
Build AWF: ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex

Copilot AI review requested due to automatic review settings April 8, 2026 01:04

lpcox requested a review from Mossaka as a code owner April 8, 2026 01:04

Copilot started reviewing on behalf of lpcox April 8, 2026 01:05 View session