Stop trimming binary HMAC secrets before use

pkg/authserver/runner/embeddedauthserver.go

@@ -378,8 +378,12 @@ func loadHMACSecrets(files []string) (*servercrypto.HMACSecrets, error) {
 		return nil, fmt.Errorf("failed to read HMAC secret from %s: %w", files[0], err)
 	}
 
-	// Trim whitespace (Kubernetes Secret mounts may include trailing newlines)
-	current = bytes.TrimSpace(current)
+	// HMAC secrets are raw binary (see MCPExternalAuthConfig.HMACSecretRefs doc) — do not
+	// trim; any byte, including whitespace-valued ones, may be genuine key material.
+	if len(current) < servercrypto.MinSecretLength {
+		return nil, fmt.Errorf("HMAC secret in %s is %d bytes, but must be at least %d bytes",
+			files[0], len(current), servercrypto.MinSecretLength)
+	}
 
 	secrets := &servercrypto.HMACSecrets{
 		Current: current,
@@ -395,7 +399,11 @@ func loadHMACSecrets(files []string) (*servercrypto.HMACSecrets, error) {
 		if err != nil {
 			return nil, fmt.Errorf("failed to read rotated HMAC secret from %s: %w", file, err)
 		}
-		secrets.Rotated = append(secrets.Rotated, bytes.TrimSpace(secret))
+		if len(secret) < servercrypto.MinSecretLength {
+			return nil, fmt.Errorf("rotated HMAC secret in %s is %d bytes, but must be at least %d bytes",
+				file, len(secret), servercrypto.MinSecretLength)
+		}
+		secrets.Rotated = append(secrets.Rotated, secret)
 	}
 
 	return secrets, nil

pkg/authserver/runner/embeddedauthserver_test.go

@@ -167,12 +167,12 @@ func TestLoadHMACSecrets(t *testing.T) {
 		assert.Equal(t, []byte(rotatedSecret), secrets.Rotated[0])
 	})
 
-	t.Run("trims whitespace from secrets", func(t *testing.T) {
+	t.Run("preserves leading and trailing whitespace bytes in secrets", func(t *testing.T) {
 		t.Parallel()
 
 		tmpDir := t.TempDir()
 		secretFile := filepath.Join(tmpDir, "hmac-secret")
-		secretValue := "  secret-with-whitespace  \n"
+		secretValue := "  secret-with-whitespace-that-is-at-least-32-bytes  \n"
 
 		err := os.WriteFile(secretFile, []byte(secretValue), 0600)
 		require.NoError(t, err)
@@ -181,7 +181,88 @@ func TestLoadHMACSecrets(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, secrets)
 
-		assert.Equal(t, []byte("secret-with-whitespace"), secrets.Current)
+		assert.Equal(t, []byte(secretValue), secrets.Current)
+	})
+
+	t.Run("does not corrupt binary secrets with whitespace-valued edge bytes", func(t *testing.T) {
+		t.Parallel()
+
+		tmpDir := t.TempDir()
+		secretFile := filepath.Join(tmpDir, "hmac-secret")
+
+		original := make([]byte, 32)
+		for i := range original {
+			original[i] = 0xAB
+		}
+		original[0] = 0x20
+		original[31] = 0x0A
+
+		require.NoError(t, os.WriteFile(secretFile, original, 0600))
+
+		secrets, err := loadHMACSecrets([]string{secretFile})
+		require.NoError(t, err)
+		require.NotNil(t, secrets)
+
+		require.Len(t, secrets.Current, 32)
+		assert.Equal(t, original, secrets.Current)
+	})
+
+	t.Run("does not corrupt rotated binary secrets with whitespace-valued edge bytes", func(t *testing.T) {
+		t.Parallel()
+
+		tmpDir := t.TempDir()
+		currentFile := filepath.Join(tmpDir, "hmac-current")
+		rotatedFile := filepath.Join(tmpDir, "hmac-rotated")
+
+		require.NoError(t, os.WriteFile(currentFile, []byte("current-secret-32-bytes-minimum!"), 0600))
+
+		original := make([]byte, 32)
+		for i := range original {
+			original[i] = 0xAB
+		}
+		original[0] = 0x20
+		original[31] = 0x0A
+
+		require.NoError(t, os.WriteFile(rotatedFile, original, 0600))
+
+		secrets, err := loadHMACSecrets([]string{currentFile, rotatedFile})
+		require.NoError(t, err)
+		require.NotNil(t, secrets)
+
+		require.Len(t, secrets.Rotated, 1)
+		assert.Equal(t, original, secrets.Rotated[0])
+	})
+
+	t.Run("short current secret returns error naming file and byte counts", func(t *testing.T) {
+		t.Parallel()
+
+		tmpDir := t.TempDir()
+		secretFile := filepath.Join(tmpDir, "hmac-secret")
+
+		require.NoError(t, os.WriteFile(secretFile, make([]byte, 31), 0600))
+
+		_, err := loadHMACSecrets([]string{secretFile})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), secretFile)
+		assert.Contains(t, err.Error(), "31")
+		assert.Contains(t, err.Error(), "32")
+	})
+
+	t.Run("short rotated secret returns error naming file and byte counts", func(t *testing.T) {
+		t.Parallel()
+
+		tmpDir := t.TempDir()
+		currentFile := filepath.Join(tmpDir, "hmac-current")
+		rotatedFile := filepath.Join(tmpDir, "hmac-rotated")
+
+		require.NoError(t, os.WriteFile(currentFile, []byte("current-secret-32-bytes-minimum!"), 0600))
+		require.NoError(t, os.WriteFile(rotatedFile, make([]byte, 31), 0600))
+
+		_, err := loadHMACSecrets([]string{currentFile, rotatedFile})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), rotatedFile)
+		assert.Contains(t, err.Error(), "31")
+		assert.Contains(t, err.Error(), "32")
 	})
 
 	t.Run("skips empty paths in rotated files", func(t *testing.T) {

pkg/authserver/server/crypto/keys.go

@@ -26,7 +26,6 @@ import (
 	"encoding/pem"
 	"fmt"
 	"os"
-	"strings"
 
 	"github.com/go-jose/go-jose/v4"
 )
@@ -227,62 +226,3 @@ func DeriveSigningKeyParams(key crypto.Signer, keyID, algorithm string) (*Signin
 
 	return params, nil
 }
-
-// loadHMACSecretFile loads an HMAC secret from a file (internal helper).
-// Returns nil if path is empty (triggers random generation in toInternalConfig).
-// The secret must be at least 32 bytes after trimming whitespace.
-func loadHMACSecretFile(secretPath string) ([]byte, error) {
-	if secretPath == "" {
-		return nil, nil
-	}
-
-	data, err := os.ReadFile(secretPath) // #nosec G304 - secretPath is provided by user via CLI flag or config
-	if err != nil {
-		return nil, fmt.Errorf("failed to read HMAC secret file: %w", err)
-	}
-
-	// Trim whitespace (common in Kubernetes Secret mounts which often add trailing newlines)
-	secret := []byte(strings.TrimSpace(string(data)))
-
-	if len(secret) < MinSecretLength {
-		return nil, fmt.Errorf("HMAC secret must be at least %d bytes", MinSecretLength)
-	}
-
-	return secret, nil
-}
-
-// LoadHMACSecrets loads HMAC secrets from file paths for rotation support.
-// paths[0] is the current (signing) secret; paths[1:] are rotated (verification) secrets.
-// Returns nil if paths is empty (caller should generate random secret).
-func LoadHMACSecrets(paths []string) (*HMACSecrets, error) {
-	if len(paths) == 0 {
-		return nil, nil
-	}
-
-	// Load current secret (required, cannot be empty path)
-	if paths[0] == "" {
-		return nil, fmt.Errorf("current HMAC secret path cannot be empty")
-	}
-	current, err := loadHMACSecretFile(paths[0])
-	if err != nil {
-		return nil, fmt.Errorf("failed to load current HMAC secret: %w", err)
-	}
-
-	// Load rotated secrets (optional, skip empty paths)
-	var rotated [][]byte
-	for i, path := range paths[1:] {
-		if path == "" {
-			continue
-		}
-		secret, err := loadHMACSecretFile(path)
-		if err != nil {
-			return nil, fmt.Errorf("failed to load rotated HMAC secret [%d]: %w", i+1, err)
-		}
-		rotated = append(rotated, secret)
-	}
-
-	return &HMACSecrets{
-		Current: current,
-		Rotated: rotated,
-	}, nil
-}

pkg/authserver/server/crypto/keys_test.go

@@ -25,7 +25,6 @@ import (
 	"encoding/pem"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -286,137 +285,6 @@ func TestDeriveKeyID(t *testing.T) {
 	assert.NotEqual(t, id1, id3, "different keys should produce different IDs")
 }
 
-func TestLoadHMACSecrets(t *testing.T) {
-	t.Parallel()
-
-	validSecret := strings.Repeat("a", 32)
-	validSecret2 := strings.Repeat("b", 32)
-	tooShortSecret := strings.Repeat("a", 31)
-
-	tests := []struct {
-		name        string
-		setup       func(t *testing.T, dir string) []string
-		wantCurrent []byte
-		wantRotated [][]byte
-		wantErr     string
-	}{
-		{
-			name:        "empty paths",
-			setup:       func(_ *testing.T, _ string) []string { return []string{} },
-			wantCurrent: nil,
-			wantRotated: nil,
-		},
-		{
-			name: "single secret",
-			setup: func(_ *testing.T, dir string) []string {
-				return []string{writeFileNamed(t, dir, "current", validSecret)}
-			},
-			wantCurrent: []byte(validSecret),
-			wantRotated: nil,
-		},
-		{
-			name: "with rotated secrets",
-			setup: func(_ *testing.T, dir string) []string {
-				return []string{
-					writeFileNamed(t, dir, "current", validSecret),
-					writeFileNamed(t, dir, "rotated1", validSecret2),
-				}
-			},
-			wantCurrent: []byte(validSecret),
-			wantRotated: [][]byte{[]byte(validSecret2)},
-		},
-		{
-			name: "empty current path",
-			setup: func(_ *testing.T, _ string) []string {
-				return []string{""}
-			},
-			wantErr: "current HMAC secret path cannot be empty",
-		},
-		{
-			name: "invalid current secret file",
-			setup: func(_ *testing.T, _ string) []string {
-				return []string{"/nonexistent/secret"}
-			},
-			wantErr: "failed to load current",
-		},
-		{
-			name: "invalid rotated secret",
-			setup: func(_ *testing.T, dir string) []string {
-				return []string{
-					writeFileNamed(t, dir, "current", validSecret),
-					"/nonexistent/rotated",
-				}
-			},
-			wantErr: "failed to load rotated HMAC secret [1]",
-		},
-		{
-			name: "skip empty rotated paths",
-			setup: func(_ *testing.T, dir string) []string {
-				return []string{
-					writeFileNamed(t, dir, "current", validSecret),
-					"",
-					writeFileNamed(t, dir, "rotated2", validSecret2),
-				}
-			},
-			wantCurrent: []byte(validSecret),
-			wantRotated: [][]byte{[]byte(validSecret2)},
-		},
-		{
-			name: "whitespace trimmed",
-			setup: func(_ *testing.T, dir string) []string {
-				return []string{
-					writeFileNamed(t, dir, "current", "  "+validSecret+"  \n\n"),
-					writeFileNamed(t, dir, "rotated", "\t"+validSecret2+"\n"),
-				}
-			},
-			wantCurrent: []byte(validSecret),
-			wantRotated: [][]byte{[]byte(validSecret2)},
-		},
-		{
-			name: "current too short",
-			setup: func(_ *testing.T, dir string) []string {
-				return []string{writeFileNamed(t, dir, "current", tooShortSecret)}
-			},
-			wantErr: "HMAC secret must be at least",
-		},
-		{
-			name: "rotated too short",
-			setup: func(_ *testing.T, dir string) []string {
-				return []string{
-					writeFileNamed(t, dir, "current", validSecret),
-					writeFileNamed(t, dir, "rotated", tooShortSecret),
-				}
-			},
-			wantErr: "failed to load rotated HMAC secret [1]",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			dir := t.TempDir()
-			paths := tt.setup(t, dir)
-
-			secrets, err := LoadHMACSecrets(paths)
-
-			if tt.wantErr != "" {
-				require.Error(t, err)
-				assert.Contains(t, err.Error(), tt.wantErr)
-				assert.Nil(t, secrets)
-			} else {
-				require.NoError(t, err)
-				if tt.wantCurrent == nil {
-					assert.Nil(t, secrets)
-				} else {
-					require.NotNil(t, secrets)
-					assert.Equal(t, tt.wantCurrent, secrets.Current)
-					assert.Equal(t, tt.wantRotated, secrets.Rotated)
-				}
-			}
-		})
-	}
-}
-
 // Helpers
 
 func writePEM(t *testing.T, dir, pemType string, der []byte) string {
@@ -426,10 +294,3 @@ func writePEM(t *testing.T, dir, pemType string, der []byte) string {
 	require.NoError(t, os.WriteFile(path, data, 0600))
 	return path
 }
-
-func writeFileNamed(t *testing.T, dir, name, content string) string {
-	t.Helper()
-	path := filepath.Join(dir, name)
-	require.NoError(t, os.WriteFile(path, []byte(content), 0600))
-	return path
-}