Add metadata validation to AuthenticateRequest

Call strategy.Validate() before strategy.Authenticate() to catch invalid
or malicious metadata early. This prevents type confusion, injection
attacks, and panics from invalid metadata in strategy implementations.

Changes:
- Add Validate() call in AuthenticateRequest()
- Proper error wrapping with strategy name
- Add test verifying validation is enforced
- Update existing tests to expect Validate() calls

Author: jhrozek

pkg/vmcp/auth/outgoing_authenticator.go

@@ -108,6 +108,7 @@ func (a *DefaultOutgoingAuthenticator) GetStrategy(name string) (Strategy, error
 //
 // Returns an error if:
 //   - The strategy is not found
+//   - The metadata validation fails
 //   - The strategy's Authenticate method fails
 func (a *DefaultOutgoingAuthenticator) AuthenticateRequest(
 	ctx context.Context,
@@ -120,5 +121,10 @@ func (a *DefaultOutgoingAuthenticator) AuthenticateRequest(
 		return err
 	}
 
+	// Validate metadata before using it
+	if err := strategy.Validate(metadata); err != nil {
+		return fmt.Errorf("invalid metadata for strategy %q: %w", strategyName, err)
+	}
+
 	return strategy.Authenticate(ctx, req, metadata)
 }

pkg/vmcp/auth/outgoing_authenticator_test.go

@@ -8,10 +8,11 @@ import (
 	"sync"
 	"testing"
 
-	"github.com/stacklok/toolhive/pkg/vmcp/auth/mocks"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
+
+	"github.com/stacklok/toolhive/pkg/vmcp/auth/mocks"
 )
 
 type testContextKey struct{}
@@ -164,6 +165,7 @@ func TestDefaultOutgoingAuthenticator_AuthenticateRequest(t *testing.T) {
 		auth := NewDefaultOutgoingAuthenticator()
 		strategy := mocks.NewMockStrategy(ctrl)
 		strategy.EXPECT().Name().Return("bearer").AnyTimes()
+		strategy.EXPECT().Validate(gomock.Any()).Return(nil)
 		strategy.EXPECT().Authenticate(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
 			func(_ context.Context, req *http.Request, _ map[string]any) error {
 				// Add a header to verify the request was modified
@@ -201,6 +203,7 @@ func TestDefaultOutgoingAuthenticator_AuthenticateRequest(t *testing.T) {
 		strategyErr := errors.New("authentication failed")
 		strategy := mocks.NewMockStrategy(ctrl)
 		strategy.EXPECT().Name().Return("bearer").AnyTimes()
+		strategy.EXPECT().Validate(gomock.Any()).Return(nil)
 		strategy.EXPECT().Authenticate(gomock.Any(), gomock.Any(), gomock.Any()).Return(strategyErr)
 		require.NoError(t, auth.RegisterStrategy("bearer", strategy))
 
@@ -223,6 +226,7 @@ func TestDefaultOutgoingAuthenticator_AuthenticateRequest(t *testing.T) {
 
 		strategy := mocks.NewMockStrategy(ctrl)
 		strategy.EXPECT().Name().Return("bearer").AnyTimes()
+		strategy.EXPECT().Validate(gomock.Any()).Return(nil)
 		strategy.EXPECT().Authenticate(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
 			func(ctx context.Context, _ *http.Request, metadata map[string]any) error {
 				receivedCtx = ctx
@@ -246,6 +250,35 @@ func TestDefaultOutgoingAuthenticator_AuthenticateRequest(t *testing.T) {
 		assert.Equal(t, "test-value", receivedCtx.Value(testKey))
 		assert.Equal(t, metadata, receivedMetadata)
 	})
+
+	t.Run("validates metadata before authentication", func(t *testing.T) {
+		t.Parallel()
+		ctrl := gomock.NewController(t)
+		t.Cleanup(ctrl.Finish)
+
+		auth := NewDefaultOutgoingAuthenticator()
+		strategy := mocks.NewMockStrategy(ctrl)
+		strategy.EXPECT().Name().Return("test-strategy").AnyTimes()
+
+		require.NoError(t, auth.RegisterStrategy("test-strategy", strategy))
+
+		req := httptest.NewRequest(http.MethodGet, "http://example.com", nil)
+		metadata := map[string]any{"invalid": "data"}
+
+		// Expect Validate to be called and return error
+		strategy.EXPECT().
+			Validate(metadata).
+			Return(errors.New("invalid metadata"))
+
+		// Authenticate should NOT be called if validation fails
+		// (no EXPECT for Authenticate)
+
+		err := auth.AuthenticateRequest(context.Background(), req, "test-strategy", metadata)
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid metadata for strategy")
+		assert.Contains(t, err.Error(), "test-strategy")
+	})
 }
 
 func TestDefaultOutgoingAuthenticator_ConcurrentAccess(t *testing.T) {
@@ -320,6 +353,7 @@ func TestDefaultOutgoingAuthenticator_ConcurrentAccess(t *testing.T) {
 
 		strategy := mocks.NewMockStrategy(ctrl)
 		strategy.EXPECT().Name().Return("bearer").AnyTimes()
+		strategy.EXPECT().Validate(gomock.Any()).Return(nil).AnyTimes()
 		strategy.EXPECT().Authenticate(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
 			func(_ context.Context, req *http.Request, _ map[string]any) error {
 				authMu.Lock()