fix(vmcp): apply outgoing auth credentials during health checks (#4101) (#4118)

Health check probes were sent unauthenticated to backend MCPServers
because both auth strategies short-circuited on the health-check context
marker before injecting any credentials.

HeaderInjectionStrategy: remove the early return — static headers have no
dependency on user identity and must be injected for all requests,
including probes.

TokenExchangeStrategy: instead of a blanket skip, perform an OAuth2
client_credentials grant when client_id + client_secret are configured,
so the health probe carries a valid Bearer token. When credentials are
not configured the probe is still sent unauthenticated (unchanged
behaviour for that case).

Unit tests are updated / added to cover both the fixed header-injection
path and the two token-exchange health-check branches (with and without
client credentials). E2E tests are added in
test/e2e/thv-operator/virtualmcp for both auth strategies, including a
new DeployMockOAuth2ServerWithNodePort helper that lets the test assert
client_credentials grants were actually made to the token endpoint.

Closes: #4101

Co-authored-by: taskbot <taskbot@users.noreply.github.com>

Author: yrobla

pkg/vmcp/auth/strategies/header_injection.go

@@ -11,7 +11,6 @@ import (
 
 	httpval "github.com/stacklok/toolhive-core/validation/http"
 	authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types"
-	"github.com/stacklok/toolhive/pkg/vmcp/health"
 )
 
 // HeaderInjectionStrategy injects a static header value into request headers.
@@ -50,26 +49,23 @@ func (*HeaderInjectionStrategy) Name() string {
 // Authenticate injects the header value from the strategy config into the request header.
 //
 // This method:
-//  1. Skips authentication if this is a health check request
-//  2. Validates that HeaderName and HeaderValue are present in the strategy config
-//  3. Sets the specified header with the provided value
+//  1. Validates that HeaderName and HeaderValue are present in the strategy config
+//  2. Sets the specified header with the provided value
+//
+// This strategy applies to all requests including health checks, since the header
+// value is static and does not depend on user identity.
 //
 // Parameters:
-//   - ctx: Request context (used to check for health check marker)
+//   - ctx: Request context
 //   - req: The HTTP request to authenticate
 //   - strategy: The backend auth strategy configuration containing HeaderInjection
 //
 // Returns an error if:
 //   - HeaderName is missing or empty
 //   - HeaderValue is missing or empty
 func (*HeaderInjectionStrategy) Authenticate(
-	ctx context.Context, req *http.Request, strategy *authtypes.BackendAuthStrategy,
+	_ context.Context, req *http.Request, strategy *authtypes.BackendAuthStrategy,
 ) error {
-	// Skip authentication for health checks
-	if health.IsHealthCheck(ctx) {
-		return nil
-	}
-
 	if strategy == nil || strategy.HeaderInjection == nil {
 		return fmt.Errorf("header_injection configuration required")
 	}

pkg/vmcp/auth/strategies/header_injection_test.go

@@ -35,7 +35,7 @@ func TestHeaderInjectionStrategy_Authenticate(t *testing.T) {
 		checkHeader   func(t *testing.T, req *http.Request)
 	}{
 		{
-			name: "skips authentication for health checks",
+			name: "injects header for health checks",
 			strategy: &authtypes.BackendAuthStrategy{
 				Type: authtypes.StrategyTypeHeaderInjection,
 				HeaderInjection: &authtypes.HeaderInjectionConfig{
@@ -47,7 +47,7 @@ func TestHeaderInjectionStrategy_Authenticate(t *testing.T) {
 			expectError: false,
 			checkHeader: func(t *testing.T, req *http.Request) {
 				t.Helper()
-				assert.Empty(t, req.Header.Get("X-API-Key"), "X-API-Key header should not be set for health checks")
+				assert.Equal(t, "secret-key-123", req.Header.Get("X-API-Key"), "static header must be injected for health checks")
 			},
 		},
 		{

pkg/vmcp/auth/strategies/tokenexchange.go

@@ -7,10 +7,13 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"sort"
 	"strings"
 	"sync"
 
+	"golang.org/x/oauth2/clientcredentials"
+
 	"github.com/stacklok/toolhive-core/env"
 	"github.com/stacklok/toolhive/pkg/auth"
 	"github.com/stacklok/toolhive/pkg/auth/tokenexchange"
@@ -73,13 +76,12 @@ func (*TokenExchangeStrategy) Name() string {
 // Authenticate exchanges the client's token for a backend token and injects it.
 //
 // This method:
-//  1. Skips authentication if this is a health check request
-//  2. Retrieves the client's identity and token from the context
-//  3. Parses and validates the token exchange configuration from strategy
-//  4. Gets or creates a cached ExchangeConfig for this backend configuration
-//  5. Creates a TokenSource with the current identity token
-//  6. Obtains an access token by performing the exchange
-//  7. Injects the token into the backend request's Authorization header
+//  1. Parses and validates the token exchange configuration from strategy
+//  2. For health check requests: uses a client credentials grant if client_id and
+//     client_secret are configured; otherwise skips authentication
+//  3. For regular requests: retrieves the client's identity and token from the context,
+//     gets or creates a cached ExchangeConfig, performs the token exchange, and injects
+//     the token into the backend request's Authorization header
 //
 // Token caching per user is handled by the upper vMCP TokenCache layer.
 // This strategy only caches the ExchangeConfig template per backend.
@@ -90,15 +92,25 @@ func (*TokenExchangeStrategy) Name() string {
 //   - strategy: Backend auth strategy containing token exchange configuration
 //
 // Returns an error if:
-//   - No identity is found in the context
-//   - The identity has no token
 //   - Strategy configuration is invalid or incomplete
-//   - Token exchange fails
+//   - No identity is found in the context (regular requests only)
+//   - The identity has no token (regular requests only)
+//   - Token exchange or client credentials grant fails
 func (s *TokenExchangeStrategy) Authenticate(
 	ctx context.Context, req *http.Request, strategy *authtypes.BackendAuthStrategy,
 ) error {
-	// Skip authentication for health checks
+	config, err := s.parseTokenExchangeConfig(strategy)
+	if err != nil {
+		return fmt.Errorf("invalid strategy configuration: %w", err)
+	}
+
+	// For health checks there is no user identity to exchange. If client credentials
+	// are configured, use a client credentials grant to authenticate the probe request.
+	// Otherwise skip authentication — the backend will be probed unauthenticated.
 	if health.IsHealthCheck(ctx) {
+		if config.ClientID != "" && config.ClientSecret != "" {
+			return s.authenticateWithClientCredentials(ctx, req, config)
+		}
 		return nil
 	}
 
@@ -111,11 +123,6 @@ func (s *TokenExchangeStrategy) Authenticate(
 		return fmt.Errorf("identity has no token")
 	}
 
-	config, err := s.parseTokenExchangeConfig(strategy)
-	if err != nil {
-		return fmt.Errorf("invalid strategy configuration: %w", err)
-	}
-
 	// Get user-specific exchange config. This creates a fresh config instance
 	// with the current user's token. The underlying server config is cached.
 	exchangeConfig := s.createUserConfig(config, identity.Token)
@@ -131,6 +138,31 @@ func (s *TokenExchangeStrategy) Authenticate(
 	return nil
 }
 
+// authenticateWithClientCredentials performs an OAuth2 client credentials grant and
+// injects the resulting token into the request. Used for health check probes when
+// client_id and client_secret are configured.
+func (*TokenExchangeStrategy) authenticateWithClientCredentials(
+	ctx context.Context, req *http.Request, config *tokenExchangeConfig,
+) error {
+	ccConfig := clientcredentials.Config{
+		ClientID:     config.ClientID,
+		ClientSecret: config.ClientSecret,
+		TokenURL:     config.TokenURL,
+		Scopes:       config.Scopes,
+	}
+	if config.Audience != "" {
+		ccConfig.EndpointParams = url.Values{"audience": {config.Audience}}
+	}
+
+	token, err := ccConfig.Token(ctx)
+	if err != nil {
+		return fmt.Errorf("client credentials grant failed: %w", err)
+	}
+
+	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.AccessToken))
+	return nil
+}
+
 // Validate checks if the required configuration fields are present and valid.
 //
 // This method verifies that:

pkg/vmcp/auth/strategies/tokenexchange_test.go

@@ -96,11 +96,11 @@ func TestTokenExchangeStrategy_Authenticate(t *testing.T) {
 		checkAuthHeader func(t *testing.T, req *http.Request)
 	}{
 		{
-			name:     "skips authentication for health checks",
+			name:     "health check without client credentials skips authentication",
 			setupCtx: func() context.Context { return health.WithHealthCheckMarker(context.Background()) },
 			setupServer: func() *httptest.Server {
 				return httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
-					t.Error("Server should not be called for health checks")
+					t.Error("token endpoint should not be called when no client credentials are configured")
 				}))
 			},
 			strategy: func(server *httptest.Server) *authtypes.BackendAuthStrategy {
@@ -109,7 +109,39 @@ func TestTokenExchangeStrategy_Authenticate(t *testing.T) {
 			expectError: false,
 			checkAuthHeader: func(t *testing.T, req *http.Request) {
 				t.Helper()
-				assert.Empty(t, req.Header.Get("Authorization"), "Authorization header should not be set for health checks")
+				assert.Empty(t, req.Header.Get("Authorization"), "Authorization header should not be set when no client credentials are available")
+			},
+		},
+		{
+			name:     "health check with client credentials uses client credentials grant",
+			setupCtx: func() context.Context { return health.WithHealthCheckMarker(context.Background()) },
+			setupServer: func() *httptest.Server {
+				return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					t.Helper()
+					require.NoError(t, r.ParseForm())
+					assert.Equal(t, "client_credentials", r.Form.Get("grant_type"))
+					clientID, clientSecret, ok := r.BasicAuth()
+					assert.True(t, ok, "expected Basic Auth credentials")
+					assert.Equal(t, "health-client-id", clientID)
+					assert.Equal(t, "health-client-secret", clientSecret)
+
+					w.Header().Set("Content-Type", "application/json")
+					json.NewEncoder(w).Encode(map[string]any{
+						"access_token": "health-check-token",
+						"token_type":   "Bearer",
+					})
+				}))
+			},
+			strategy: func(server *httptest.Server) *authtypes.BackendAuthStrategy {
+				return createTokenExchangeStrategy(server.URL, func(cfg *authtypes.TokenExchangeConfig) {
+					cfg.ClientID = "health-client-id"
+					cfg.ClientSecret = "health-client-secret"
+				})
+			},
+			expectError: false,
+			checkAuthHeader: func(t *testing.T, req *http.Request) {
+				t.Helper()
+				assert.Equal(t, "Bearer health-check-token", req.Header.Get("Authorization"))
 			},
 		},
 		{

pkg/vmcp/health/checker_test.go

@@ -442,6 +442,109 @@ func TestHealthChecker_CheckHealth_Timeout(t *testing.T) {
 	assert.Equal(t, vmcp.BackendUnhealthy, status)
 }
 
+// TestHealthChecker_CheckHealth_ContextCarriesHealthCheckMarker verifies that CheckHealth
+// passes a context with the health check marker to ListCapabilities.
+// This is critical because the auth strategies (header_injection, token_exchange) read
+// this marker to decide how to authenticate probe requests.
+func TestHealthChecker_CheckHealth_ContextCarriesHealthCheckMarker(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	var capturedCtx context.Context
+	mockClient := mocks.NewMockBackendClient(ctrl)
+	mockClient.EXPECT().
+		ListCapabilities(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(ctx context.Context, _ *vmcp.BackendTarget) (*vmcp.CapabilityList, error) {
+			capturedCtx = ctx
+			return &vmcp.CapabilityList{}, nil
+		}).
+		Times(1)
+
+	checker := NewHealthChecker(mockClient, 5*time.Second, 0)
+	target := &vmcp.BackendTarget{
+		WorkloadID:   "backend-1",
+		WorkloadName: "test-backend",
+		BaseURL:      "http://localhost:8080",
+	}
+
+	status, err := checker.CheckHealth(context.Background(), target)
+	require.NoError(t, err)
+	assert.Equal(t, vmcp.BackendHealthy, status)
+
+	// The context passed to ListCapabilities must carry the health check marker so
+	// that auth strategies (header_injection, token_exchange) apply the correct
+	// authentication path for probe requests.
+	require.NotNil(t, capturedCtx, "context must have been captured")
+	assert.True(t, IsHealthCheck(capturedCtx),
+		"ListCapabilities must receive a context with the health check marker; "+
+			"without it, header_injection and token_exchange strategies cannot "+
+			"apply outgoing auth to health check probes")
+}
+
+// TestHealthChecker_CheckHealth_AuthErrorsCategorizesAsUnauthenticated verifies that auth
+// errors from health checks are correctly categorised as BackendUnauthenticated, not
+// BackendUnhealthy. This distinguishes a backend that is reachable-but-needs-auth
+// from one that is down, which is important for health check probes that apply
+// outgoing auth credentials (header_injection or token_exchange).
+func TestHealthChecker_CheckHealth_AuthErrorsCategorizesAsUnauthenticated(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		err  error
+	}{
+		{
+			name: "header injection auth failure - http 401",
+			err:  fmt.Errorf("transport error: http 401"),
+		},
+		{
+			name: "token exchange auth failure - status code 401",
+			err:  fmt.Errorf("backend unavailable: failed to initialize client for backend my-backend: status code 401"),
+		},
+		{
+			name: "sentinel auth error",
+			err:  vmcp.ErrAuthenticationFailed,
+		},
+		{
+			name: "sentinel authz error",
+			err:  vmcp.ErrAuthorizationFailed,
+		},
+		{
+			name: "wrapped sentinel auth error",
+			err:  fmt.Errorf("client credentials grant failed: %w", vmcp.ErrAuthenticationFailed),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockClient := mocks.NewMockBackendClient(ctrl)
+			mockClient.EXPECT().
+				ListCapabilities(gomock.Any(), gomock.Any()).
+				Return(nil, tt.err).
+				Times(1)
+
+			checker := NewHealthChecker(mockClient, 5*time.Second, 0)
+			target := &vmcp.BackendTarget{
+				WorkloadID:   "backend-1",
+				WorkloadName: "test-backend",
+				BaseURL:      "http://localhost:8080",
+			}
+
+			status, err := checker.CheckHealth(context.Background(), target)
+			assert.Error(t, err)
+			assert.Equal(t, vmcp.BackendUnauthenticated, status,
+				"auth failure from a health probe should be BackendUnauthenticated, not BackendUnhealthy")
+		})
+	}
+}
+
 func TestHealthChecker_CheckHealth_MultipleBackends(t *testing.T) {
 	t.Parallel()