mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
fix: upstream transport must not inherit process-level proxy env vars
PMG's upstream http.Transport used http.ProxyFromEnvironment, which reads HTTPS_PROXY/HTTP_PROXY from the PMG process's own environment at request time. Engineers with corporate proxy env vars in their shell caused PMG to route its own outbound registry connections through that external proxy. When the proxy drops or resets the connection, npm/pip see ECONNRESET / "socket hang up" — reproduced on both npm.pkg.github.com and registry.npmjs.org (e.g. istanbul-reports-3.2.0.tgz). Fix: set Proxy: nil on the upstream transport so PMG always connects directly to the registry, ignoring any ambient proxy env vars. Add TestUpstreamTransportDoesNotInheritProcessProxyEnv which mimics a corporate proxy locally via httptest.Server + t.Setenv, and asserts that zero connections reach the fake proxy regardless of env var state. https://claude.ai/code/session_01EJKTARXXGsVxScKyAfovrv
This commit is contained in:
+10
-1
@@ -139,8 +139,17 @@ func newUpstreamTransport(config *ProxyConfig) *http.Transport {
|
||||
|
||||
// Keep transport behavior close to goproxy defaults and only harden TLS:
|
||||
// enforce server certificate verification and require TLS 1.2+.
|
||||
//
|
||||
// Proxy is explicitly nil (not http.ProxyFromEnvironment) so that PMG's own
|
||||
// upstream connections always go direct to the registry. Using
|
||||
// http.ProxyFromEnvironment would cause the transport to inherit any
|
||||
// HTTPS_PROXY/HTTP_PROXY env vars present in the user's shell (e.g. a
|
||||
// corporate proxy), routing PMG's outbound traffic through that external
|
||||
// proxy. When that proxy drops or resets the connection, npm/pip see an
|
||||
// ECONNRESET / "socket hang up" error, which is the root cause of the
|
||||
// reported npm proxy failures.
|
||||
return &http.Transport{
|
||||
Proxy: http.ProxyFromEnvironment,
|
||||
Proxy: nil,
|
||||
DialContext: dialer.DialContext,
|
||||
TLSHandshakeTimeout: config.ConnectTimeout,
|
||||
TLSClientConfig: &tls.Config{
|
||||
|
||||
@@ -4,10 +4,12 @@ import (
|
||||
"crypto/tls"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestNewProxyServerSecuresUpstreamTLSConfig(t *testing.T) {
|
||||
@@ -27,6 +29,52 @@ func TestNewProxyServerSecuresUpstreamTLSConfig(t *testing.T) {
|
||||
assert.GreaterOrEqual(t, internalProxy.proxy.Tr.TLSClientConfig.MinVersion, uint16(tls.VersionTLS12), "minimum TLS version should be 1.2+")
|
||||
}
|
||||
|
||||
// TestUpstreamTransportDoesNotInheritProcessProxyEnv reproduces the ECONNRESET bug
|
||||
// where engineers with HTTPS_PROXY set in their shell (e.g. corporate proxy) caused
|
||||
// PMG's upstream transport to route its own outbound connections through that proxy,
|
||||
// leading to socket hang-ups and ECONNRESET on both npm.pkg.github.com and
|
||||
// registry.npmjs.org (e.g. istanbul-reports-3.2.0.tgz).
|
||||
//
|
||||
// Technique: a local httptest.Server acts as the "corporate proxy". t.Setenv injects
|
||||
// HTTPS_PROXY/HTTP_PROXY pointing to it, which http.ProxyFromEnvironment would pick up.
|
||||
// We count how many connections reach the fake proxy — the upstream transport must
|
||||
// produce zero (it should always connect directly to the registry, ignoring env proxy vars).
|
||||
func TestUpstreamTransportDoesNotInheritProcessProxyEnv(t *testing.T) {
|
||||
var proxyConnections atomic.Int32
|
||||
|
||||
fakeProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
proxyConnections.Add(1)
|
||||
// Return 502 so the transport doesn't hang waiting for a tunnel.
|
||||
http.Error(w, "fake corporate proxy — should never be reached by PMG", http.StatusBadGateway)
|
||||
}))
|
||||
defer fakeProxy.Close()
|
||||
|
||||
// Mimic "corporate proxy" env vars present in the shell before pmg is invoked.
|
||||
// t.Setenv restores the originals automatically after the test.
|
||||
t.Setenv("HTTPS_PROXY", fakeProxy.URL)
|
||||
t.Setenv("HTTP_PROXY", fakeProxy.URL)
|
||||
t.Setenv("https_proxy", fakeProxy.URL)
|
||||
t.Setenv("http_proxy", fakeProxy.URL)
|
||||
|
||||
tr := newUpstreamTransport(&ProxyConfig{
|
||||
ConnectTimeout: 2 * time.Second,
|
||||
RequestTimeout: 2 * time.Second,
|
||||
})
|
||||
|
||||
req, err := http.NewRequest(http.MethodGet, "https://registry.npmjs.org/", nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
//nolint:errcheck // the request may fail for network reasons; we only care about WHERE it was routed
|
||||
tr.RoundTrip(req)
|
||||
|
||||
// If proxyConnections > 0 the transport incorrectly forwarded PMG's own upstream
|
||||
// traffic through the user's HTTPS_PROXY, which is the root cause of the ECONNRESET.
|
||||
assert.Equal(t, int32(0), proxyConnections.Load(),
|
||||
"upstream transport must not inherit HTTPS_PROXY/HTTP_PROXY from the process env; "+
|
||||
"got %d connection(s) to the fake corporate proxy",
|
||||
proxyConnections.Load())
|
||||
}
|
||||
|
||||
func TestNewProxyServerRejectsUntrustedUpstreamCertByDefault(t *testing.T) {
|
||||
target := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
|
||||
Reference in New Issue
Block a user