Skip to content

fix: add api-proxy hostname to NO_PROXY for Node.js undici compatibility#4003

Merged
lpcox merged 1 commit into
mainfrom
fix/no-proxy-api-proxy-hostname
May 29, 2026
Merged

fix: add api-proxy hostname to NO_PROXY for Node.js undici compatibility#4003
lpcox merged 1 commit into
mainfrom
fix/no-proxy-api-proxy-hostname

Conversation

@lpcox
Copy link
Copy Markdown
Collaborator

@lpcox lpcox commented May 29, 2026

Problem

Node.js undici matches NO_PROXY entries against the request hostname string, not the resolved IP. When code fetches http://api-proxy:10000/reflect, undici checks for api-proxy in NO_PROXY — the existing IP entry (172.30.0.30) doesn't match.

This caused internal api-proxy traffic to be tunneled through Squid via CONNECT api-proxy:10000, which Squid rejects (403) since api-proxy isn't in the domain allowlist.

Affected: Pi provider and any Node.js runtime that fetches internal api-proxy endpoints by hostname.

Fix

Include both the IP and the Docker service hostname api-proxy in NO_PROXY:

environment.NO_PROXY += `,${networkConfig.proxyIp},api-proxy`;

Evidence

From UNDICI debug trace in gh-aw#35381:

UNDICI: connecting to api-proxy:10000 → routed to 172.30.0.10:3128 (Squid) → CONNECT rejected

curl to the same URL succeeds because it resolves hostname to IP before matching NO_PROXY.

Closes #4001

Node.js undici matches NO_PROXY entries against the request hostname
string, not the resolved IP address. When code fetches
http://api-proxy:10000/reflect, undici checks for 'api-proxy' in
NO_PROXY — the existing IP entry (172.30.0.30) doesn't match.

This caused internal api-proxy traffic to be tunneled through Squid,
which rejects it with a 403 since 'api-proxy' isn't in the domain
allowlist.

Fix: include both the IP and the Docker service hostname in NO_PROXY.

Closes #4001

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 29, 2026 12:29
@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.57% 96.62% 📈 +0.05%
Statements 96.45% 96.49% 📈 +0.04%
Functions 98.24% 98.24% ➡️ +0.00%
Branches 90.77% 90.81% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 89.3% → 90.9% (+1.65%) 89.3% → 90.9% (+1.65%)

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

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP (fix: add api-proxy hostname to NO_PROXY for Node.js undici compatibility)
GitHub.com connectivity
File write/read (smoke-test-copilot-26637292121.txt)

Overall: PASS

Author: @lpcox · Reviewer: @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine

  • ✅ GitHub API: 2 PR entries confirmed
  • ✅ GitHub check: playwright_check PASS
  • ✅ File verify: smoke-test-claude-26637292139.txt exists

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ensures internal traffic to the api-proxy sidecar bypasses Squid when Node.js (undici) uses hostname-based NO_PROXY matching, preventing CONNECT api-proxy:10000 requests from being rejected by Squid.

Changes:

  • Append both the api-proxy IP and the Docker Compose service hostname (api-proxy) to NO_PROXY / no_proxy when the api-proxy sidecar is enabled.
  • Update service configuration tests to assert the hostname is included in both NO_PROXY and no_proxy.
Show a summary per file
File Description
src/services/agent-environment/proxy-environment.ts Adds api-proxy hostname to the agent’s NO_PROXY entries when the api-proxy sidecar is enabled.
src/services/api-proxy-service-config.test.ts Extends existing tests to validate NO_PROXY / no_proxy include both api-proxy IP and hostname.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK — PASS ✅

Test Result
GitHub MCP (list PRs) ✅ PR #3998 "feat: support custom API auth headers for internal AI gateways" returned
GitHub.com connectivity
File write/read (smoke-test-copilot-byok-26637292094.txt)
BYOK inference (api-proxy → api.githubcopilot.com)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

PR author: @lpcox · Reviewer: @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.16.0 v22.22.3 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

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

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

  • Redis PING: ❌ (no response/timeout)
  • PostgreSQL pg_isready: ❌ (no response)
  • PostgreSQL SELECT 1: ❌ (timeout)

Overall: FAILhost.docker.internal is not reachable from this runner environment.

🔌 Service connectivity validated by Smoke Services

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

🔮 The ancient spirits stir, and the smoke test agent has passed through the veil.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ 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 1/1 passed ✅ PASS
Deno std 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 1/1 passed ✅ PASS
Node.js execa 1/1 passed ✅ PASS
Node.js p-limit 1/1 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 #4003 · sonnet46 4.7M ·

@lpcox lpcox merged commit d34fc7f into main May 29, 2026
63 of 65 checks passed
@lpcox lpcox deleted the fix/no-proxy-api-proxy-hostname branch May 29, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NO_PROXY missing Docker service hostname 'api-proxy' — Node.js undici routes internal traffic through Squid

2 participants