mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(enterprise/aibridgeproxyd): stop injecting default port into forwarded Host header (#26656)
## Problem PR #23109 introduced port normalization for the private IP blocking feature, which mutated `CoderAccessURL.Host` to always include the default port (e.g. `coder.example.com:443`). This leaked into the `Host` header of every request forwarded to the Coder server. When `CODER_REDIRECT_TO_ACCESS_URL=true`, the `redirectToAccessURL` middleware compared the `Host` header literally against the access URL (`coder.example.com`), saw a mismatch, and returned a 307 redirect to the Coder dashboard HTML page. Copilot then received HTML instead of JSON: ``` Failed to start MCP client: Streamable HTTP error: Unexpected content type: text/html; charset=utf-8 Failed to load custom agents: SyntaxError: Unexpected token '<', "<!doctype "... is not valid JSON ``` ## Changes - Stop mutating `coderAccessURL.Host`; store the resolved port in a separate field for `isBlockedIP` - Update existing tests that asserted the old (mutated) `.Port()` behavior - Add test cases verifying the Host is preserved with and without an explicit port > Generated with the assistance of Coder Agents on behalf of @ssncferreira
This commit is contained in:
@@ -127,6 +127,8 @@ type Server struct {
|
||||
listener net.Listener
|
||||
tlsEnabled bool
|
||||
coderAccessURL *url.URL
|
||||
// coderAccessPort is the resolved port for the Coder access URL.
|
||||
coderAccessPort string
|
||||
// refreshProviders fetches the live provider snapshot on Reload.
|
||||
// Nil disables hot-reload.
|
||||
refreshProviders RefreshProvidersFunc
|
||||
@@ -265,7 +267,6 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
|
||||
coderAccessPort = "80"
|
||||
}
|
||||
}
|
||||
coderAccessURL.Host = net.JoinHostPort(coderAccessURL.Hostname(), coderAccessPort)
|
||||
|
||||
// MITM cert and key are required to intercept and decrypt HTTPS traffic.
|
||||
if opts.MITMCertFile == "" || opts.MITMKeyFile == "" {
|
||||
@@ -325,6 +326,7 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
|
||||
proxy: proxy,
|
||||
tlsEnabled: opts.TLSCertFile != "",
|
||||
coderAccessURL: coderAccessURL,
|
||||
coderAccessPort: coderAccessPort,
|
||||
refreshProviders: opts.RefreshProviders,
|
||||
allowedPorts: allowedPorts,
|
||||
caCert: certPEM,
|
||||
@@ -801,7 +803,7 @@ func (s *Server) isBlockedIP(ip net.IP, hostname string, port string) bool {
|
||||
// block connections to its own deployment. Hostname-based (not IP-based)
|
||||
// to handle dynamic IPs (DNS changes, load balancers, k8s rescheduling).
|
||||
// The port is normalized at startup to handle URLs without explicit ports.
|
||||
if strings.EqualFold(hostname, s.coderAccessURL.Hostname()) && port == s.coderAccessURL.Port() {
|
||||
if strings.EqualFold(hostname, s.coderAccessURL.Hostname()) && port == s.coderAccessPort {
|
||||
return false
|
||||
}
|
||||
|
||||
|
||||
@@ -622,8 +622,7 @@ func TestNew(t *testing.T) {
|
||||
MITMKeyFile: mitmKeyFile,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "localhost", srv.CoderAccessURL().Hostname())
|
||||
require.Equal(t, "80", srv.CoderAccessURL().Port())
|
||||
require.Equal(t, "localhost", srv.CoderAccessURL().Host)
|
||||
})
|
||||
|
||||
t.Run("CoderAccessURLDefaultHTTPSPort", func(t *testing.T) {
|
||||
@@ -639,8 +638,7 @@ func TestNew(t *testing.T) {
|
||||
MITMKeyFile: mitmKeyFile,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "localhost", srv.CoderAccessURL().Hostname())
|
||||
require.Equal(t, "443", srv.CoderAccessURL().Port())
|
||||
require.Equal(t, "localhost", srv.CoderAccessURL().Host)
|
||||
})
|
||||
|
||||
t.Run("CoderAccessURLExplicitPort", func(t *testing.T) {
|
||||
@@ -949,6 +947,45 @@ func TestNew(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, srv)
|
||||
})
|
||||
|
||||
t.Run("CoderAccessURLHostPreserved", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mitmCertFile, mitmKeyFile := getSharedTestMITMCert(t)
|
||||
logger := slogtest.Make(t, nil)
|
||||
|
||||
srv, err := aibridgeproxyd.New(t.Context(), logger, aibridgeproxyd.Options{
|
||||
ListenAddr: "127.0.0.1:0",
|
||||
CoderAccessURL: "https://coder.example.com",
|
||||
MITMCertFile: mitmCertFile,
|
||||
MITMKeyFile: mitmKeyFile,
|
||||
AllowedPrivateCIDRs: []string{"127.0.0.1/32"},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = srv.Close() })
|
||||
|
||||
require.Equal(t, "coder.example.com", srv.CoderAccessURL().Host,
|
||||
"Host must not have :443 appended")
|
||||
})
|
||||
|
||||
t.Run("CoderAccessURLExplicitPortPreserved", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mitmCertFile, mitmKeyFile := getSharedTestMITMCert(t)
|
||||
logger := slogtest.Make(t, nil)
|
||||
|
||||
srv, err := aibridgeproxyd.New(t.Context(), logger, aibridgeproxyd.Options{
|
||||
ListenAddr: "127.0.0.1:0",
|
||||
CoderAccessURL: "https://coder.example.com:8443",
|
||||
MITMCertFile: mitmCertFile,
|
||||
MITMKeyFile: mitmKeyFile,
|
||||
AllowedPrivateCIDRs: []string{"127.0.0.1/32"},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = srv.Close() })
|
||||
|
||||
require.Equal(t, "coder.example.com:8443", srv.CoderAccessURL().Host)
|
||||
})
|
||||
}
|
||||
|
||||
func TestClose(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user