Fix transparent proxy for remote MCP servers behind redirects

Three issues prevented MCPRemoteProxy from connecting to third-party
upstream MCP servers that use HTTP redirects:

1. X-Forwarded-Host leaked the proxy's hostname to the upstream. The
   upstream used it to construct 307 redirect URLs pointing back to
   the proxy, creating a redirect loop. Fix: skip SetXForwarded() for
   remote upstreams (isRemote == true).

2. Go's http.Transport.RoundTrip does not follow redirects, but
   httputil.ReverseProxy uses Transport directly. Upstream 307/308
   redirects (e.g. HTTPS→HTTP scheme changes, path canonicalization)
   were returned to the MCP client which cannot follow them through
   the proxy. Fix: add forwardFollowingRedirects that transparently
   follows up to 10 redirects, preserving method and body for
   307/308 (RFC 7538).

3. When disableUpstreamTokenInjection is true, the client's ToolHive
   JWT was still forwarded to the upstream in the Authorization
   header. Fix: add strip-auth middleware that removes the
   Authorization header before forwarding.

Also adds debug logging for outbound request headers and upstream
response status codes to aid diagnosis of remote proxy issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: aron-muon

pkg/runner/middleware.go

@@ -5,6 +5,7 @@ package runner
 
 import (
 	"fmt"
+	"net/http"
 
 	"github.com/stacklok/toolhive/pkg/audit"
 	"github.com/stacklok/toolhive/pkg/auth"
@@ -37,6 +38,7 @@ func GetSupportedMiddlewareFactories() map[string]types.MiddlewareFactory {
 		audit.MiddlewareType:                  audit.CreateMiddleware,
 		recovery.MiddlewareType:               recovery.CreateMiddleware,
 		headerfwd.HeaderForwardMiddlewareName: headerfwd.CreateMiddleware,
+		stripAuthMiddlewareType:               createStripAuthMiddleware,
 	}
 }
 
@@ -263,9 +265,10 @@ func addUpstreamSwapMiddleware(
 		return middlewares, nil
 	}
 
-	// Skip upstream token injection if explicitly disabled
+	// When upstream token injection is disabled, strip the Authorization header
+	// so the client's ToolHive JWT doesn't leak to the upstream server.
 	if config.EmbeddedAuthServerConfig.DisableUpstreamTokenInjection {
-		return middlewares, nil
+		return addAuthHeaderStripMiddleware(middlewares)
 	}
 
 	// Use provided config or defaults
@@ -287,6 +290,45 @@ func addUpstreamSwapMiddleware(
 	return append(middlewares, *upstreamSwapMwConfig), nil
 }
 
+// stripAuthMiddlewareType is the type identifier for the auth header stripping middleware.
+const stripAuthMiddlewareType = "strip-auth"
+
+// addAuthHeaderStripMiddleware adds a middleware that removes the Authorization header
+// before forwarding to the upstream. This prevents the client's ToolHive JWT from
+// leaking to upstream servers that don't expect it.
+func addAuthHeaderStripMiddleware(
+	middlewares []types.MiddlewareConfig,
+) ([]types.MiddlewareConfig, error) {
+	mwConfig, err := types.NewMiddlewareConfig(stripAuthMiddlewareType, struct{}{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create strip-auth middleware config: %w", err)
+	}
+	return append(middlewares, *mwConfig), nil
+}
+
+// createStripAuthMiddleware is the factory function for the auth header stripping middleware.
+func createStripAuthMiddleware(_ *types.MiddlewareConfig, runner types.MiddlewareRunner) error {
+	mw := &stripAuthMiddleware{}
+	runner.AddMiddleware(stripAuthMiddlewareType, mw)
+	return nil
+}
+
+// stripAuthMiddleware removes the Authorization header from requests.
+type stripAuthMiddleware struct{}
+
+// Handler returns the middleware function.
+func (*stripAuthMiddleware) Handler() types.MiddlewareFunction {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			r.Header.Del("Authorization")
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
+// Close cleans up resources.
+func (*stripAuthMiddleware) Close() error { return nil }
+
 // addAWSStsMiddleware adds AWS STS middleware if configured.
 // Returns an error if AWSStsConfig is set but RemoteURL is empty, because
 // SigV4 signing is only meaningful for remote MCP servers.

pkg/runner/middleware_test.go

@@ -253,9 +253,10 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name         string
-		config       *RunConfig
-		wantAppended bool
+		name             string
+		config           *RunConfig
+		wantAppended     bool
+		wantType         string // expected middleware type when appended
 	}{
 		{
 			name:         "nil EmbeddedAuthServerConfig returns input unchanged",
@@ -269,15 +270,17 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) {
 				UpstreamSwapConfig:       nil,
 			},
 			wantAppended: true,
+			wantType:     upstreamswap.MiddlewareType,
 		},
 		{
-			name: "EmbeddedAuthServerConfig with DisableUpstreamTokenInjection skips middleware",
+			name: "DisableUpstreamTokenInjection adds strip-auth middleware instead",
 			config: func() *RunConfig {
 				cfg := createMinimalAuthServerConfig()
 				cfg.DisableUpstreamTokenInjection = true
 				return &RunConfig{EmbeddedAuthServerConfig: cfg}
 			}(),
-			wantAppended: false,
+			wantAppended: true,
+			wantType:     stripAuthMiddlewareType,
 		},
 		{
 			name: "EmbeddedAuthServerConfig set with explicit UpstreamSwapConfig uses provided config",
@@ -288,6 +291,7 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) {
 				},
 			},
 			wantAppended: true,
+			wantType:     upstreamswap.MiddlewareType,
 		},
 		{
 			name: "EmbeddedAuthServerConfig with custom header strategy config",
@@ -299,6 +303,7 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) {
 				},
 			},
 			wantAppended: true,
+			wantType:     upstreamswap.MiddlewareType,
 		},
 	}
 
@@ -318,20 +323,20 @@ func TestAddUpstreamSwapMiddleware(t *testing.T) {
 			// Should have one additional entry.
 			require.Len(t, got, len(initial)+1)
 			added := got[len(got)-1]
-			assert.Equal(t, upstreamswap.MiddlewareType, added.Type)
-
-			// Verify serialized params contain the expected config.
-			var params upstreamswap.MiddlewareParams
-			require.NoError(t, json.Unmarshal(added.Parameters, &params))
+			assert.Equal(t, tt.wantType, added.Type)
 
-			if tt.config.UpstreamSwapConfig != nil {
-				// Should use the provided config
-				require.NotNil(t, params.Config)
-				assert.Equal(t, tt.config.UpstreamSwapConfig.HeaderStrategy, params.Config.HeaderStrategy)
-				assert.Equal(t, tt.config.UpstreamSwapConfig.CustomHeaderName, params.Config.CustomHeaderName)
-			} else {
-				// Should use defaults (empty config is valid)
-				require.NotNil(t, params.Config)
+			// For upstreamswap type, verify serialized params
+			if tt.wantType == upstreamswap.MiddlewareType {
+				var params upstreamswap.MiddlewareParams
+				require.NoError(t, json.Unmarshal(added.Parameters, &params))
+
+				if tt.config.UpstreamSwapConfig != nil {
+					require.NotNil(t, params.Config)
+					assert.Equal(t, tt.config.UpstreamSwapConfig.HeaderStrategy, params.Config.HeaderStrategy)
+					assert.Equal(t, tt.config.UpstreamSwapConfig.CustomHeaderName, params.Config.CustomHeaderName)
+				} else {
+					require.NotNil(t, params.Config)
+				}
 			}
 		})
 	}
@@ -344,6 +349,7 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) {
 		name               string
 		config             *RunConfig
 		wantUpstreamSwap   bool
+		wantStripAuth      bool
 		wantHeaderStrategy string
 	}{
 		{
@@ -357,13 +363,14 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) {
 			wantUpstreamSwap: false,
 		},
 		{
-			name: "DisableUpstreamTokenInjection omits upstream-swap",
+			name: "DisableUpstreamTokenInjection adds strip-auth instead of upstream-swap",
 			config: func() *RunConfig {
 				cfg := createMinimalAuthServerConfig()
 				cfg.DisableUpstreamTokenInjection = true
 				return &RunConfig{EmbeddedAuthServerConfig: cfg}
 			}(),
 			wantUpstreamSwap: false,
+			wantStripAuth:    true,
 		},
 		{
 			name: "explicit UpstreamSwapConfig is used",
@@ -385,20 +392,25 @@ func TestPopulateMiddlewareConfigs_UpstreamSwap(t *testing.T) {
 			err := PopulateMiddlewareConfigs(tt.config)
 			require.NoError(t, err)
 
-			var found bool
+			var foundSwap bool
+			var foundStrip bool
 			var foundConfig *types.MiddlewareConfig
 			for i, mw := range tt.config.MiddlewareConfigs {
 				if mw.Type == upstreamswap.MiddlewareType {
-					found = true
+					foundSwap = true
 					foundConfig = &tt.config.MiddlewareConfigs[i]
-					break
+				}
+				if mw.Type == stripAuthMiddlewareType {
+					foundStrip = true
 				}
 			}
-			assert.Equal(t, tt.wantUpstreamSwap, found,
+			assert.Equal(t, tt.wantUpstreamSwap, foundSwap,
 				"upstream-swap middleware presence mismatch")
+			assert.Equal(t, tt.wantStripAuth, foundStrip,
+				"strip-auth middleware presence mismatch")
 
 			// Verify config values if we expect the middleware and have specific expectations
-			if found && tt.wantHeaderStrategy != "" {
+			if foundSwap && tt.wantHeaderStrategy != "" {
 				var params upstreamswap.MiddlewareParams
 				require.NoError(t, json.Unmarshal(foundConfig.Parameters, &params))
 				require.NotNil(t, params.Config)

pkg/transport/proxy/transparent/transparent_proxy.go

@@ -352,6 +352,10 @@ func (p *TransparentProxy) serverInitialized() bool {
 	return p.isServerInitialized.Load()
 }
 
+// maxRedirects is the maximum number of HTTP redirects the transport follows
+// before returning an error. Matches http.Client's default of 10.
+const maxRedirects = 10
+
 func (t *tracingTransport) forward(req *http.Request) (*http.Response, error) {
 	tr := t.base
 	if tr == nil {
@@ -360,6 +364,87 @@ func (t *tracingTransport) forward(req *http.Request) (*http.Response, error) {
 	return tr.RoundTrip(req)
 }
 
+// forwardFollowingRedirects sends req and transparently follows 3xx redirects.
+//
+// Go's http.Transport.RoundTrip does not follow redirects (that is
+// http.Client's job), but httputil.ReverseProxy uses Transport directly.
+// Without this, upstream redirects (e.g. HTTPS→HTTP scheme changes, path
+// canonicalization, or API gateway routing) are returned to the MCP client
+// which cannot follow them through the proxy.
+//
+// 307/308 preserve method and body (RFC 7538). 301/302/303 change to GET.
+func (t *tracingTransport) forwardFollowingRedirects(req *http.Request, body []byte) (*http.Response, error) {
+	for range maxRedirects {
+		resp, err := t.forward(req)
+		if err != nil {
+			return nil, err
+		}
+
+		if !isRedirectStatus(resp.StatusCode) {
+			return resp, nil
+		}
+
+		location := resp.Header.Get("Location")
+		if location == "" {
+			return resp, nil
+		}
+
+		redirectURL, err := req.URL.Parse(location)
+		if err != nil {
+			slog.Warn("unparsable redirect Location, returning raw redirect response",
+				"location", location, "error", err)
+			return resp, nil
+		}
+
+		slog.Warn("following upstream redirect",
+			"status", resp.StatusCode,
+			"from", req.URL.String(),
+			"to", redirectURL.String(),
+		)
+
+		// Drain and close the body so the TCP connection can be reused.
+		_, _ = io.Copy(io.Discard, resp.Body)
+		resp.Body.Close()
+
+		newReq, err := http.NewRequestWithContext(req.Context(), req.Method, redirectURL.String(), nil)
+		if err != nil {
+			return nil, fmt.Errorf("failed to build redirect request: %w", err)
+		}
+		newReq.Header = req.Header.Clone()
+		newReq.Host = redirectURL.Host
+
+		switch resp.StatusCode {
+		case http.StatusMovedPermanently, http.StatusFound, http.StatusSeeOther:
+			// 301/302/303 → change to GET, drop body (RFC 7231)
+			newReq.Method = http.MethodGet
+			newReq.ContentLength = 0
+		default:
+			// 307/308 → preserve method and body (RFC 7538)
+			if len(body) > 0 {
+				newReq.Body = io.NopCloser(bytes.NewReader(body))
+				newReq.ContentLength = int64(len(body))
+			}
+		}
+
+		req = newReq
+	}
+
+	return nil, fmt.Errorf("upstream exceeded %d redirects", maxRedirects)
+}
+
+// isRedirectStatus returns true for HTTP 3xx codes that carry a Location header.
+func isRedirectStatus(code int) bool {
+	switch code {
+	case http.StatusMovedPermanently,    // 301
+		http.StatusFound,             // 302
+		http.StatusSeeOther,          // 303
+		http.StatusTemporaryRedirect, // 307
+		http.StatusPermanentRedirect: // 308
+		return true
+	}
+	return false
+}
+
 // nolint:gocyclo // This function handles multiple request types and is complex by design
 func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	// Always rewrite Host header to match the target URL to avoid "Invalid Host" errors
@@ -371,6 +456,14 @@ func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error)
 		req.Host = req.URL.Host
 	}
 
+	slog.Debug("outbound request to upstream",
+		"method", req.Method,
+		"url", req.URL.String(),
+		"host", req.Host,
+		"accept", req.Header.Get("Accept"),
+		"content_type", req.Header.Get("Content-Type"),
+	)
+
 	reqBody := readRequestBody(req)
 
 	// thv proxy does not provide the transport type, so we need to detect it from the request
@@ -385,7 +478,7 @@ func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error)
 		sawInitialize = t.detectInitialize(reqBody)
 	}
 
-	resp, err := t.forward(req)
+	resp, err := t.forwardFollowingRedirects(req, reqBody)
 	if err != nil {
 		if errors.Is(err, context.Canceled) {
 			// Expected during shutdown or client disconnect—silently ignore
@@ -395,6 +488,13 @@ func (t *tracingTransport) RoundTrip(req *http.Request) (*http.Response, error)
 		return nil, err
 	}
 
+	slog.Debug("upstream response received",
+		"status", resp.StatusCode,
+		"url", req.URL.String(),
+		"content_type", resp.Header.Get("Content-Type"),
+		"mcp_session_id", resp.Header.Get("Mcp-Session-Id"),
+	)
+
 	// Check for 401 Unauthorized response (bearer token authentication failure)
 	if resp.StatusCode == http.StatusUnauthorized {
 		//nolint:gosec // G706: logging target URI from config
@@ -504,7 +604,15 @@ func (p *TransparentProxy) Start(ctx context.Context) error {
 		FlushInterval: -1,
 		Rewrite: func(pr *httputil.ProxyRequest) {
 			pr.SetURL(targetURL)
-			pr.SetXForwarded()
+
+			// Only set X-Forwarded-* headers for local backends.
+			// For remote upstreams, these headers leak the proxy's hostname
+			// (X-Forwarded-Host) to third-party servers, which can cause
+			// 307 redirect loops when the upstream uses that header to
+			// construct redirect URLs pointing back to the proxy.
+			if !p.isRemote {
+				pr.SetXForwarded()
+			}
 
 			// Stash the original inbound request in the outbound request's
 			// context so that ModifyResponse (SSE response processor) can