fix(networking): honor HTTP(S)_PROXY in HttpClientBuilder.Build()

[schonmann]

Problem

• HttpClientBuilder.Build() builds its http.Transport without a Proxy field.
• That means clients it produces ignore HTTP_PROXY / HTTPS_PROXY / NO_PROXY (a zero-value http.Transport has Proxy: nil, unlike http.DefaultTransport).
• The embedded auth server's upstream OAuth/DCR client is built this way (pkg/authserver/upstream/oauth2.go → newHTTPClientForHost → NewHostScopedClientBuilder().Build()).
• Where the upstream is only reachable via an explicit forward proxy, the token-exchange and DCR calls bypass the proxy and fail even with the proxy env vars set.
• The vMCP tool-forwarding path is unaffected, since it uses CloneDefaultTransportWithDialControl, which already sets Proxy: http.ProxyFromEnvironment.

flowchart LR
    Pod["MCPRemoteProxy pod<br/>(HTTPS_PROXY set)"]

    Pod -->|"tool-forwarding client<br/>CloneDefaultTransportWithDialControl"| P1["honors proxy"]
    Pod -->|"auth-server OAuth client<br/>Build → Proxy: nil"| P2["ignores proxy"]

    P1 --> FP["forward proxy"] --> UP1["upstream MCP host<br/>reachable"]
    P2 --> DIRECT["direct egress"] --> UP2["IP-allowlisted token<br/>endpoint → 403"]

    classDef ok fill:#e7f7ec,stroke:#2e7d32,color:#1b3d24;
    classDef bad fill:#fdecea,stroke:#c62828,color:#5b1a15;
    class P1,FP,UP1 ok;
    class P2,DIRECT,UP2 bad;

Loading

Fixes #6671

Fix

• Set Proxy: http.ProxyFromEnvironment on the transport in Build(), matching http.DefaultTransport and the vMCP path.
• Adds a regression test asserting the built transport sets Proxy.

Note

• When the proxy address is itself private, the private-IP dial guard blocks the proxy connection.
• That case is covered by the existing AllowPrivateIPs option.

Type of change

• Bug fix

Test plan

• Unit tests (task test) — added TestHttpClientBuilder_BuildSetsProxyFromEnvironment

Note: the suite was not run on the authoring machine (no Go toolchain available there); relying on CI to execute task test / task lint-fix.

API Compatibility

• This PR does not break the v1beta1 API (it only touches pkg/networking).

Does this introduce a user-facing change?

Yes — HTTP clients built via pkg/networking (including the auth server's upstream OAuth/DCR calls) now honor HTTP(S)_PROXY / NO_PROXY, so deployments behind an explicit forward proxy work as expected.

[Copilot]

Pull request overview

This PR fixes HttpClientBuilder.Build() so HTTP clients created via pkg/networking honor HTTP_PROXY / HTTPS_PROXY / NO_PROXY, aligning behavior with http.DefaultTransport and the existing vMCP transport path; this unblocks proxy-only upstream OAuth/DCR calls from the embedded auth server.

Changes:

• Set http.Transport.Proxy = http.ProxyFromEnvironment in HttpClientBuilder.Build().
• Add a regression test asserting the built transport has a non-nil Proxy function.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/networking/http_client.go	Ensures builder-created transports use proxy env vars by wiring ProxyFromEnvironment.
pkg/networking/http_client_test.go	Adds a regression test to prevent dropping the proxy behavior in future refactors.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[schonmann]

Hi team, quick pull request for your consideration! 👋

In essence -> this will honor the HTTP(S)_PROXY env, making token endpoints from MCPs behind firewall and with a strict source-IP allowlist (i.e. including proxy's IP range) functional.

Thx!

[jhrozek]

Thanks for the patch. Do you think it's safe to always pass on the environment proxy variables?
What about adding a new option along the lines of the existing WithPrivateIPs something like WithProxyFromEnvironment that would enable the proxy environment values only when set?

[schonmann]

Hey, thanks for checking @jhrozek!

Dug into this a bit, and I think always passing is actually a safe choice here. Why:

• From what I can tell, Build() was the only spot in pkg/networking that wasn’t honoring the proxy. CloneDefaultTransportWithDialControl and Go’s own http.DefaultTransport already set Proxy: ProxyFromEnvironment.
• The "enable only when set" behavior kind of comes for free: ProxyFromEnvironment returns a nil URL when no HTTP(S)_PROXY is exported, and the net/http docs say "if Proxy is nil or returns a nil *URL, no proxy is used".
• With no env var it behaves exactly like today, and NO_PROXY handles the opt out.

That's why I'd lean away from a separate WithProxyFromEnvironment flag in this case, specifically. Thoughts?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.24%. Comparing base (2b299c1) to head (494ccbd).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6670      +/-   ##
==========================================
- Coverage   79.31%   79.24%   -0.07%     
==========================================
  Files         800      800              
  Lines       80936    80937       +1     
==========================================
- Hits        64192    64141      -51     
- Misses      16739    16791      +52     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JAORMX]

Thanks for tracking this down! The issue makes sense. We should support deployments where the upstream OAuth/DCR endpoints are only reachable through a forward proxy.

The part I would change is enabling this for every client built here. This builder is also used for registry access, discovery, and JWT/JWKS validation. For deployments that already have proxy variables set, this changes those paths too, even though they previously connected directly. Matching http.DefaultTransport is not enough to establish that all of these clients should share its routing policy.

So, I agree with @jhrozek on making this explicit, along the lines of WithProxyFromEnvironment, and reviewing which callers should enable it rather than changing the shared default.

We also need to sort out the interaction with WithPrivateIPs. Reaching a private proxy and allowing private destinations are different decisions, so I would not recommend AllowPrivateIPs as a general workaround. Some consumers deliberately keep that disabled. An opt-in limits the scope of the change, but we still need to preserve the destination restrictions for the flows that use it. We can work through the security-sensitive details separately.

The new test is useful for catching a missing assignment. Before merging, we also need tests for the supported proxy path, NO_PROXY, and the interaction with the private-IP policy. That is our transport wiring and policy we need to test, not a reimplementation of the Go proxy tests. Please document the chosen behavior alongside the builder option too.

Requesting changes for those policy and compatibility questions. The proxy support itself is worth adding.

Approved the wrong PR .. ooops