Fixes

Author: lee-aaron

cmd/manager.go

@@ -525,7 +525,11 @@ func startManagerGRPCServer(opts ManagerServerOpts, ms *manager.ManagerServer) *
 	if opts.AdminToken != "" {
 		interceptor := manager.NewAdminAuthInterceptor(opts.AdminToken, manager.AdminProtectedMethods())
 		grpcOpts = append(grpcOpts, grpc.ChainUnaryInterceptor(interceptor))
-		logger.Info().Msg("Admin gRPC auth enabled")
+		if opts.CertFile == "" || opts.KeyFile == "" {
+			logger.Warn().Msg("Admin gRPC auth enabled WITHOUT TLS — admin token will be sent in cleartext. Use --cert_file and --key_file in production.")
+		} else {
+			logger.Info().Msg("Admin gRPC auth enabled with TLS")
+		}
 	} else {
 		logger.Warn().Msg("Admin gRPC auth disabled (set --admin_token for production)")
 	}

pkg/iam/crypto.go

@@ -24,12 +24,12 @@ type MasterKey struct {
 	key []byte
 }
 
-// Key returns the raw key bytes for use with external encryption routines.
+// Key returns a copy of the raw key bytes for use with external encryption routines.
 func (m *MasterKey) Key() []byte {
 	if m == nil {
 		return nil
 	}
-	return m.key
+	return append([]byte{}, m.key...)
 }
 
 var (

pkg/manager/admin_auth.go

@@ -5,6 +5,7 @@ package manager
 
 import (
 	"context"
+	"crypto/subtle"
 	"strings"
 
 	"github.com/LeeDigitalWorks/zapfs/pkg/logger"
@@ -64,7 +65,7 @@ func NewAdminAuthInterceptor(token string, protectedMethods []string) grpc.Unary
 		}
 
 		provided := strings.TrimPrefix(authHeader, "Bearer ")
-		if provided != token {
+		if subtle.ConstantTimeCompare([]byte(provided), []byte(token)) != 1 {
 			logger.Warn().Str("method", info.FullMethod).Msg("Admin RPC rejected: invalid token")
 			return nil, status.Error(codes.Unauthenticated, "invalid admin token")
 		}

pkg/manager/raft_fsm.go

@@ -727,16 +727,17 @@ func (ms *ManagerServer) applyRegisterFederation(data json.RawMessage) interface
 
 	// Encrypt federation credentials before storing in Raft state
 	if req.External != nil && len(ms.masterKey) > 0 {
-		if encrypted, err := encryptSecret(ms.masterKey, req.External.AccessKeyId); err == nil {
-			req.External.AccessKeyId = encrypted
-		} else {
-			logger.Error().Err(err).Str("bucket", req.LocalBucket).Msg("Failed to encrypt federation access key")
+		encrypted, err := encryptSecret(ms.masterKey, req.External.AccessKeyId)
+		if err != nil {
+			return fmt.Errorf("encrypt federation access key for %s: %w", req.LocalBucket, err)
 		}
-		if encrypted, err := encryptSecret(ms.masterKey, req.External.SecretAccessKey); err == nil {
-			req.External.SecretAccessKey = encrypted
-		} else {
-			logger.Error().Err(err).Str("bucket", req.LocalBucket).Msg("Failed to encrypt federation secret key")
+		req.External.AccessKeyId = encrypted
+
+		encrypted, err = encryptSecret(ms.masterKey, req.External.SecretAccessKey)
+		if err != nil {
+			return fmt.Errorf("encrypt federation secret key for %s: %w", req.LocalBucket, err)
 		}
+		req.External.SecretAccessKey = encrypted
 	}
 
 	// Store federation config
@@ -923,16 +924,17 @@ func (ms *ManagerServer) applyUpdateFederationCredentials(data json.RawMessage)
 	accessKeyID := req.AccessKeyID
 	secretAccessKey := req.SecretAccessKey
 	if len(ms.masterKey) > 0 {
-		if encrypted, err := encryptSecret(ms.masterKey, accessKeyID); err == nil {
-			accessKeyID = encrypted
-		} else {
-			logger.Error().Err(err).Str("bucket", req.Bucket).Msg("Failed to encrypt federation access key")
+		encrypted, err := encryptSecret(ms.masterKey, accessKeyID)
+		if err != nil {
+			return fmt.Errorf("encrypt federation access key for %s: %w", req.Bucket, err)
 		}
-		if encrypted, err := encryptSecret(ms.masterKey, secretAccessKey); err == nil {
-			secretAccessKey = encrypted
-		} else {
-			logger.Error().Err(err).Str("bucket", req.Bucket).Msg("Failed to encrypt federation secret key")
+		accessKeyID = encrypted
+
+		encrypted, err = encryptSecret(ms.masterKey, secretAccessKey)
+		if err != nil {
+			return fmt.Errorf("encrypt federation secret key for %s: %w", req.Bucket, err)
 		}
+		secretAccessKey = encrypted
 	}
 
 	fedInfo.External.AccessKeyId = accessKeyID

pkg/manager/raft_server.go

@@ -74,11 +74,22 @@ func (t *tlsStreamLayer) Dial(address raft.ServerAddress, timeout time.Duration)
 	if err != nil {
 		return nil, err
 	}
+	// Set deadline for the TLS handshake to prevent a slow/malicious
+	// peer from blocking the Raft transport goroutine indefinitely.
+	if err := conn.SetDeadline(time.Now().Add(timeout)); err != nil {
+		conn.Close()
+		return nil, err
+	}
 	tlsConn := tls.Client(conn, t.tlsConfig)
 	if err := tlsConn.Handshake(); err != nil {
 		conn.Close()
 		return nil, err
 	}
+	// Clear the deadline so the connection can be used normally.
+	if err := conn.SetDeadline(time.Time{}); err != nil {
+		conn.Close()
+		return nil, err
+	}
 	return tlsConn, nil
 }
 

pkg/manager/server.go

@@ -45,6 +45,14 @@ type ClusterCapacity struct {
 // ManagerServer is the main manager server that coordinates the ZapFS cluster.
 // It implements both ManagerServiceServer (cluster management) and IAMServiceServer
 // (credential sync to metadata services).
+//
+// Lock ordering (to prevent deadlocks):
+//
+//	state.Lock() > collectionsRecoveryRequiredMu > clusterCapacityMu
+//
+// The state lock must always be acquired first. The secondary mutexes
+// (collectionsRecoveryRequiredMu, clusterCapacityMu) protect independent
+// caches and may be acquired while state is held, but never the reverse.
 type ManagerServer struct {
 	manager_pb.UnimplementedManagerServiceServer
 	iam_pb.UnimplementedIAMServiceServer
@@ -98,8 +106,11 @@ type ManagerServer struct {
 	// Backup scheduler (enterprise)
 	backupScheduler *BackupScheduler
 
-	// Encryption key for federation secrets in Raft state (nil = no encryption)
-	masterKey []byte
+	// Encryption key for federation secrets in Raft state (nil = no encryption).
+	// Must be set via SetMasterKey before the Raft node processes any federation commands.
+	// Protected by masterKeyOnce to ensure safe publication to the Raft goroutine.
+	masterKey     []byte
+	masterKeyOnce sync.Once
 
 	// Shutdown
 	shutdownCh chan struct{}
@@ -440,15 +451,24 @@ func (ms *ManagerServer) Shutdown() {
 // SetMasterKey sets the encryption key for federation secrets in Raft state.
 // When set, federation credentials (AccessKeyId, SecretAccessKey) are encrypted
 // before being stored in the FSM state and decrypted when read.
+// Must be called before the Raft node processes any federation commands.
+// Can only be called once; subsequent calls are no-ops.
 // Pass nil to disable encryption (credentials stored in plaintext).
 func (ms *ManagerServer) SetMasterKey(key []byte) {
-	ms.masterKey = key
+	ms.masterKeyOnce.Do(func() {
+		ms.masterKey = key
+	})
 }
 
 // DecryptFederationCredentials decrypts the AccessKeyId and SecretAccessKey
 // from a FederationInfo that was stored in encrypted form in Raft state.
 // Returns the decrypted accessKeyID and secretAccessKey.
 // If no masterKey is set or the values are not encrypted, returns them as-is.
+//
+// Note: The metadata service's DB layer (vitess/postgres) has its own independent
+// encryption via iam.EncryptCredentialSecret/DecryptCredentialSecret. This method
+// is for code that reads directly from the Raft FSM state (e.g., manager-side
+// federation operations, diagnostics, or snapshot inspection).
 func (ms *ManagerServer) DecryptFederationCredentials(fedInfo *FederationInfo) (accessKeyID, secretAccessKey string, err error) {
 	if fedInfo == nil || fedInfo.External == nil {
 		return "", "", nil