Recognize mcp-go authorization-required sentinels as auth

pkg/vmcp/client/client.go

@@ -429,6 +429,17 @@ func wrapBackendError(err error, backendID string, operation string) error {
 		return fmt.Errorf("%w: failed to %s for backend %s: %v",
 			vmcp.ErrAuthenticationFailed, operation, backendID, err)
 	}
+	// transport.ErrAuthorizationRequired is returned (wrapped in *transport.Error
+	// and *transport.AuthorizationRequiredError) for 401 responses with a
+	// WWW-Authenticate header. transport.ErrOAuthAuthorizationRequired is the
+	// companion sentinel from the OAuth-handler path. Both must map to
+	// ErrAuthenticationFailed so health monitoring engages the auth-aware
+	// branch (#4935) instead of treating the probe as unhealthy (#5223).
+	if errors.Is(err, transport.ErrAuthorizationRequired) ||
+		errors.Is(err, transport.ErrOAuthAuthorizationRequired) {
+		return fmt.Errorf("%w: failed to %s for backend %s: %v",
+			vmcp.ErrAuthenticationFailed, operation, backendID, err)
+	}
 	// ErrLegacySSEServer is returned for any 4xx (except 401) on initialize POST.
 	// This includes 403 (auth rejection) and 404/405 (endpoint not found/method not allowed).
 	// We cannot distinguish auth failures from routing errors without the raw status code,

pkg/vmcp/client/client_test.go

@@ -1001,6 +1001,34 @@ func TestWrapBackendError(t *testing.T) {
 			err:          context.DeadlineExceeded,
 			wantSentinel: vmcp.ErrTimeout,
 		},
+		{
+			// mcp-go v0.49.0 added ErrAuthorizationRequired for 401 responses
+			// with a WWW-Authenticate header. Issue #5223: probe of GitHub
+			// Copilot's MCP returns this; without recognizing the sentinel here
+			// the error falls through to ErrBackendUnavailable and the health
+			// monitor never engages the auth-aware branch (#4935).
+			name:         "ErrAuthorizationRequired maps to ErrAuthenticationFailed",
+			err:          transport.ErrAuthorizationRequired,
+			wantSentinel: vmcp.ErrAuthenticationFailed,
+		},
+		{
+			// Production chain mcp-go produces: transport.Error wrapping
+			// *AuthorizationRequiredError. Error() formats as
+			// "transport error: authorization required" — byte-for-byte
+			// the string seen in North's WARN logs (issue #5223). Both
+			// layers Unwrap() to the sentinel so errors.Is must succeed.
+			name:         "wrapped AuthorizationRequiredError (production chain) maps to ErrAuthenticationFailed",
+			err:          transport.NewError(&transport.AuthorizationRequiredError{}),
+			wantSentinel: vmcp.ErrAuthenticationFailed,
+		},
+		{
+			// Companion sentinel for the OAuth-handler path. Less hot in
+			// practice but belongs in the same matcher block; the existing
+			// transport.ErrUnauthorized check would not catch it.
+			name:         "ErrOAuthAuthorizationRequired maps to ErrAuthenticationFailed",
+			err:          transport.ErrOAuthAuthorizationRequired,
+			wantSentinel: vmcp.ErrAuthenticationFailed,
+		},
 	}
 
 	for _, tt := range tests {

pkg/vmcp/errors.go

@@ -119,6 +119,14 @@ func IsAuthenticationError(err error) bool {
 		return true
 	}
 
+	// mcp-go ErrAuthorizationRequired ("authorization required") and
+	// ErrOAuthAuthorizationRequired ("no valid token available, authorization
+	// required") string forms. Used as a fallback when the typed sentinel is
+	// not preserved through the error chain (issue #5223).
+	if strings.Contains(errLower, "authorization required") {
+		return true
+	}
+
 	return false
 }
 

pkg/vmcp/health/checker_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/mark3labs/mcp-go/client/transport"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
@@ -450,6 +451,14 @@ func TestIsAuthenticationError(t *testing.T) {
 		{name: "404 not found", err: errors.New("404 not found"), expectErr: false},
 		{name: "500 internal server error", err: errors.New("500 internal server error"), expectErr: false},
 		{name: "hostname with 401", err: errors.New("http://backend401.example.com"), expectErr: false},
+		// Pin against accidental loosening of the "authorization required"
+		// substring matcher: a validation message of the form "field
+		// 'authorization' required" must not be misclassified as an auth
+		// failure. The current matcher uses the contiguous substring
+		// "authorization required" (one space, no punctuation), so this
+		// string does not match — but a future loosening (e.g. allowing
+		// any whitespace) would silently regress.
+		{name: "validation message containing 'authorization' and 'required'", err: errors.New("field 'authorization' required"), expectErr: false},
 		{name: "nil error", err: nil, expectErr: false},
 	}
 
@@ -633,6 +642,16 @@ func TestHealthChecker_CheckHealth_AuthErrorsCategorizesAsUnauthenticated(t *tes
 			name: "mcp-go ErrUnauthorized wrapped as ErrAuthenticationFailed by wrapBackendError",
 			err:  fmt.Errorf("%w: failed to initialize for backend my-backend: unauthorized (401)", vmcp.ErrAuthenticationFailed),
 		},
+		{
+			// Issue #5223: mcp-go v0.49.0+ returns *AuthorizationRequiredError for
+			// 401 with WWW-Authenticate, wrapped in *transport.Error. Both layers
+			// Unwrap() to transport.ErrAuthorizationRequired. The string fallback
+			// must recognize "authorization required" so the probe error reaches
+			// health monitoring as BackendUnauthenticated rather than falling
+			// through to BackendUnhealthy and producing WARN spam.
+			name: "transport.Error wrapping AuthorizationRequiredError",
+			err:  transport.NewError(&transport.AuthorizationRequiredError{}),
+		},
 	}
 
 	for _, tt := range tests {
@@ -697,6 +716,31 @@ func TestHealthChecker_CheckHealth_AuthErrorWithOutgoingAuthIsHealthy(t *testing
 			authConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject},
 			err:        fmt.Errorf("%w: unauthorized (401)", vmcp.ErrAuthenticationFailed),
 		},
+		{
+			// Issue #5223 exact reproducer: North's github-copilot-entry backend
+			// behind an upstreamInject auth strategy. Probe carries no user creds;
+			// mcp-go returns the typed authorization-required chain. Once correctly
+			// classified as auth, #4935's logic must take over and report
+			// BackendHealthy with nil err so the monitor records a successful
+			// check, the circuit breaker stays closed, and the WARN spam stops.
+			name:       "upstream_inject + transport.Error wrapping AuthorizationRequiredError",
+			authConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject},
+			err:        transport.NewError(&transport.AuthorizationRequiredError{}),
+		},
+		{
+			// Same mcp-go chain as the upstream_inject row above; this row pins
+			// that token_exchange flows through the same authErrorStatus branch.
+			name:       "token_exchange + transport.Error wrapping AuthorizationRequiredError",
+			authConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeTokenExchange},
+			err:        transport.NewError(&transport.AuthorizationRequiredError{}),
+		},
+		{
+			// Same mcp-go chain as the upstream_inject row above; this row pins
+			// that header_injection flows through the same authErrorStatus branch.
+			name:       "header_injection + transport.Error wrapping AuthorizationRequiredError",
+			authConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeHeaderInjection},
+			err:        transport.NewError(&transport.AuthorizationRequiredError{}),
+		},
 	}
 
 	for _, tt := range tests {