Treat 401/403 from auth-configured backends as healthy (#4935)

Health probes run as background system checks with no user context, so
they cannot carry the per-user upstream tokens that upstream_inject
backends require. For OAuth-protected backends, a 401/403 challenge is
the expected response to an unauthenticated probe and proves the backend
is reachable, its auth layer works, and the network path is sound.

Map auth errors to BackendHealthy when the backend has an outgoing auth
strategy configured; keep BackendUnauthenticated only as a narrower
misconfiguration signal (backend demands auth, none configured). This
unblocks discovery from excluding OAuth-protected backends from
capability aggregation (resolves the TODO at discovery/middleware.go).

Drop the `|| BackendUnauthenticated` clause from /status since the state
now represents misconfig rather than routability. Legacy PR #4866
routability semantics in monitor.go are left in place with a TODO for a
follow-up revert; they touch operator controllers and warrant a separate
PR.

Closes #4920

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: JAORMX

pkg/vmcp/discovery/middleware.go

@@ -135,17 +135,21 @@ func Middleware(
 // or degraded. Backends that are unhealthy, unknown, or unauthenticated are excluded
 // from capability aggregation to prevent exposing tools from unavailable backends.
 //
-// TODO(#4920): Unauthenticated backends are treated as routable for phase determination
-// but are excluded here because discovery probes cannot carry user tokens. If health
-// probes could authenticate, these backends would be fully healthy and included here.
+// A note on BackendUnauthenticated: a 401/403 from a backend that has an outgoing
+// auth strategy configured is treated as BackendHealthy by the health checker
+// (health probes deliberately do not carry user credentials — the challenge proves
+// reachability). BackendUnauthenticated therefore indicates a misconfiguration:
+// the backend requires authentication but no outgoing auth strategy is configured
+// on the backend target. Excluding such backends from capability aggregation is
+// the correct behavior — their capabilities cannot be safely exposed.
 //
 // Health status filtering:
 //   - healthy: included (fully operational)
 //   - degraded: included (slow but working)
 //   - empty/zero-value: included (assume healthy when health monitoring is disabled)
 //   - unhealthy: excluded (not responding, circuit breaker may be open)
 //   - unknown: excluded (status not yet determined)
-//   - unauthenticated: excluded (authentication failed)
+//   - unauthenticated: excluded (misconfiguration: backend requires auth but none configured)
 //
 // When healthStatusProvider is provided, the current health status from the health
 // monitor is used (respects circuit breaker state). When nil, falls back to the

pkg/vmcp/health/checker.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/stacklok/toolhive/pkg/vmcp"
+	authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types"
 )
 
 // healthChecker implements vmcp.HealthChecker using ListCapabilities as the health check.
@@ -59,7 +60,14 @@ func NewHealthChecker(
 // Health determination logic:
 //   - Success with fast response: Backend is healthy (BackendHealthy)
 //   - Success with slow response (> degradedThreshold): Backend is degraded (BackendDegraded)
-//   - Authentication error: Backend is unauthenticated (BackendUnauthenticated)
+//   - Authentication error (HTTP 401/403) AND backend has an outgoing auth strategy
+//     configured: Backend is healthy (BackendHealthy). Health probes deliberately do
+//     not carry user credentials, so the backend's auth challenge proves reachability
+//     and a working auth layer — that is success for probe purposes.
+//   - Authentication error AND backend has no outgoing auth strategy configured
+//     (AuthConfig nil or StrategyTypeUnauthenticated): Backend is unauthenticated
+//     (BackendUnauthenticated). This signals operator misconfiguration — the backend
+//     requires authentication but none was configured on the backend target.
 //   - Timeout or connection error: Backend is unhealthy (BackendUnhealthy)
 //   - Other errors: Backend is unhealthy (BackendUnhealthy)
 //
@@ -92,8 +100,19 @@ func (h *healthChecker) CheckHealth(ctx context.Context, target *vmcp.BackendTar
 	responseDuration := time.Since(startTime)
 
 	if err != nil {
-		// Categorize the error to determine health status
-		status := categorizeError(err)
+		// Categorize the error to determine health status. The target's outgoing
+		// auth config is consulted: a 401/403 from a backend with an outgoing auth
+		// strategy is the expected response to a no-credential probe and maps to
+		// BackendHealthy. In that case we return a nil error so the monitor records
+		// this as a successful check and does not open the circuit breaker.
+		status := categorizeError(target, err)
+		if status == vmcp.BackendHealthy {
+			slog.Debug("health check received expected auth challenge — treating as healthy",
+				"backend", target.WorkloadName,
+				"error", err,
+				"duration", responseDuration)
+			return vmcp.BackendHealthy, nil
+		}
 		slog.Debug("health check failed for backend",
 			"backend", target.WorkloadName,
 			"error", err,
@@ -115,18 +134,24 @@ func (h *healthChecker) CheckHealth(ctx context.Context, target *vmcp.BackendTar
 	return vmcp.BackendHealthy, nil
 }
 
-// categorizeError determines the appropriate health status based on the error type.
+// categorizeError determines the appropriate health status based on the error type
+// and the backend's outgoing auth configuration.
+//
 // This uses sentinel error checking with errors.Is() for type-safe error categorization.
 // Falls back to string-based detection for backwards compatibility with non-wrapped errors.
-func categorizeError(err error) vmcp.BackendHealthStatus {
+//
+// For auth errors (HTTP 401/403), the target's AuthConfig is consulted to distinguish
+// an expected auth challenge (backend has outgoing auth configured) from a misconfiguration
+// (backend has no outgoing auth strategy). See authErrorStatus for details.
+func categorizeError(target *vmcp.BackendTarget, err error) vmcp.BackendHealthStatus {
 	if err == nil {
 		return vmcp.BackendHealthy
 	}
 
 	// 1. Type-safe detection: Check for sentinel errors using errors.Is()
 	// BackendClient now wraps all errors with appropriate sentinel errors
 	if errors.Is(err, vmcp.ErrAuthenticationFailed) || errors.Is(err, vmcp.ErrAuthorizationFailed) {
-		return vmcp.BackendUnauthenticated
+		return authErrorStatus(target)
 	}
 
 	if errors.Is(err, vmcp.ErrTimeout) || errors.Is(err, vmcp.ErrCancelled) {
@@ -140,7 +165,7 @@ func categorizeError(err error) vmcp.BackendHealthStatus {
 	// 2. String-based detection: Fallback for backwards compatibility
 	// This handles errors from sources that don't wrap with sentinel errors
 	if vmcp.IsAuthenticationError(err) {
-		return vmcp.BackendUnauthenticated
+		return authErrorStatus(target)
 	}
 
 	if vmcp.IsTimeoutError(err) || vmcp.IsConnectionError(err) {
@@ -150,3 +175,24 @@ func categorizeError(err error) vmcp.BackendHealthStatus {
 	// Default to unhealthy for unknown errors
 	return vmcp.BackendUnhealthy
 }
+
+// authErrorStatus maps an authentication error (HTTP 401/403) to a health status
+// using the backend's outgoing auth configuration.
+//
+// Health probes deliberately do not carry user credentials. If the backend is
+// configured with an outgoing auth strategy, a 401/403 from the backend proves
+// that the backend is alive, the auth layer works, and the network+TLS path is
+// healthy — this is the expected response to an unauthenticated probe and is
+// therefore treated as BackendHealthy.
+//
+// If the backend has no outgoing auth strategy configured (AuthConfig nil or
+// StrategyTypeUnauthenticated), a 401/403 indicates operator misconfiguration:
+// the backend requires authentication but none was configured on the backend
+// target. This is reported as BackendUnauthenticated so it surfaces in status.
+func authErrorStatus(target *vmcp.BackendTarget) vmcp.BackendHealthStatus {
+	if target != nil && target.AuthConfig != nil &&
+		target.AuthConfig.Type != authtypes.StrategyTypeUnauthenticated {
+		return vmcp.BackendHealthy
+	}
+	return vmcp.BackendUnauthenticated
+}