auth: bound metadata header lengths and sanitise resolution errors (#260)

Cap x-subject (≤256 printable ASCII), x-role (≤64), and x-tenant-id
(≤1024 bytes / ≤32 entries) so an oversized header is rejected before
any handler runs. Tenant resolver errors and unknown roles are now
logged server-side at warn and returned to the client as generic
InvalidArgument / PermissionDenied messages, instead of leaking raw
DB error text or echoing the raw role value.

Closes #219.

Author: zeevdr

cmd/server/main.go

@@ -165,7 +165,7 @@ func run() int {
 		authInterceptor = jwtInterceptor
 		logger.InfoContext(ctx, "JWT auth enabled", "jwks_url", cfg.JWTJWKSURL)
 	} else {
-		authInterceptor = auth.NewMetadataInterceptor(tenantResolver(schemaStoreVal))
+		authInterceptor = auth.NewMetadataInterceptor(tenantResolver(schemaStoreVal), auth.WithMetadataLogger(logger))
 		logger.InfoContext(ctx, "metadata auth enabled — pass x-subject, x-role, x-tenant-id headers")
 	}
 

internal/auth/jwt.go

@@ -168,7 +168,8 @@ func (i *Interceptor) authenticate(ctx context.Context) (context.Context, error)
 	switch claims.Role {
 	case RoleSuperAdmin, RoleAdmin, RoleUser:
 	default:
-		return nil, status.Errorf(codes.PermissionDenied, "unknown role: %s", claims.Role)
+		i.logger.WarnContext(ctx, "jwt auth: unknown role", "role", string(claims.Role), "subject", claims.Subject)
+		return nil, status.Error(codes.PermissionDenied, "unknown role")
 	}
 
 	// Non-superadmin must have at least one tenant_id.

internal/auth/jwt_test.go

@@ -1,6 +1,7 @@
 package auth
 
 import (
+	"bytes"
 	"context"
 	"crypto/rand"
 	"crypto/rsa"
@@ -272,16 +273,33 @@ func TestUnaryInterceptor_CorrectIssuer(t *testing.T) {
 }
 
 func TestUnaryInterceptor_UnknownRole(t *testing.T) {
-	interceptor := newTestInterceptor(t, "")
+	var logBuf bytes.Buffer
+	logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelWarn}))
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write(jwksJSON())
+	}))
+	t.Cleanup(srv.Close)
+
+	interceptor, err := NewInterceptor(context.Background(), srv.URL, WithLogger(logger))
+	require.NoError(t, err)
+	t.Cleanup(interceptor.Close)
 	unary := interceptor.UnaryInterceptor()
 
 	claims := validClaims("editor", "tenant-1")
 	ctx := ctxWithBearer(signToken(t, claims))
 
-	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+	_, err = unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
 	require.Error(t, err)
 	assert.Equal(t, codes.PermissionDenied, status.Code(err))
-	assert.Contains(t, status.Convert(err).Message(), "unknown role")
+	msg := status.Convert(err).Message()
+	assert.Equal(t, "unknown role", msg)
+	assert.NotContains(t, msg, "editor")
+
+	logged := logBuf.String()
+	assert.Contains(t, logged, "unknown role")
+	assert.Contains(t, logged, "editor")
 }
 
 func TestUnaryInterceptor_NonSuperadminMissingTenantID(t *testing.T) {

internal/auth/metadata.go

@@ -2,6 +2,7 @@ package auth
 
 import (
 	"context"
+	"log/slog"
 	"strings"
 
 	"github.com/golang-jwt/jwt/v5"
@@ -21,6 +22,12 @@ const (
 	headerSubject  = "x-subject"
 	headerRole     = "x-role"
 	headerTenantID = "x-tenant-id"
+
+	// Bound metadata header sizes to keep auth interceptor allocations small.
+	maxHeaderLen  = 1024
+	maxSubjectLen = 256
+	maxTenantIDs  = 32
+	maxRoleLen    = 64
 )
 
 // TenantResolver resolves a tenant identifier (UUID or name slug) to a UUID.
@@ -31,13 +38,31 @@ type TenantResolver func(ctx context.Context, idOrName string) (string, error)
 // instead of JWT tokens. Used when JWT auth is disabled.
 type MetadataInterceptor struct {
 	resolveTenant TenantResolver
+	logger        *slog.Logger
+}
+
+// MetadataInterceptorOption configures a MetadataInterceptor.
+type MetadataInterceptorOption func(*metadataInterceptorOptions)
+
+type metadataInterceptorOptions struct {
+	logger *slog.Logger
+}
+
+// WithMetadataLogger sets the logger used for sanitised server-side errors.
+// Defaults to slog.Default() when unset.
+func WithMetadataLogger(l *slog.Logger) MetadataInterceptorOption {
+	return func(o *metadataInterceptorOptions) { o.logger = l }
 }
 
 // NewMetadataInterceptor creates a new metadata-based auth interceptor.
 // If resolver is non-nil, tenant IDs in x-tenant-id headers are resolved
 // from name slugs to UUIDs before storing in the auth context.
-func NewMetadataInterceptor(resolver TenantResolver) *MetadataInterceptor {
-	return &MetadataInterceptor{resolveTenant: resolver}
+func NewMetadataInterceptor(resolver TenantResolver, opts ...MetadataInterceptorOption) *MetadataInterceptor {
+	o := metadataInterceptorOptions{logger: slog.Default()}
+	for _, opt := range opts {
+		opt(&o)
+	}
+	return &MetadataInterceptor{resolveTenant: resolver, logger: o.logger}
 }
 
 // UnaryInterceptor returns a gRPC unary server interceptor.
@@ -68,35 +93,58 @@ func (m *MetadataInterceptor) StreamInterceptor() grpc.StreamServerInterceptor {
 func (m *MetadataInterceptor) extractClaims(ctx context.Context) (context.Context, error) {
 	md, _ := metadata.FromIncomingContext(ctx)
 
-	subject := firstMetadataValue(md, headerSubject)
+	rawSubject := firstMetadataValue(md, headerSubject)
+	if len(rawSubject) > maxHeaderLen {
+		return nil, status.Errorf(codes.InvalidArgument, "%s header exceeds %d bytes", headerSubject, maxHeaderLen)
+	}
+	subject := rawSubject
 	if subject == "" {
 		return nil, status.Error(codes.Unauthenticated, "x-subject header is required")
 	}
+	if len(subject) > maxSubjectLen {
+		return nil, status.Errorf(codes.InvalidArgument, "%s exceeds %d bytes", headerSubject, maxSubjectLen)
+	}
+	if !isPrintableASCII(subject) {
+		return nil, status.Errorf(codes.InvalidArgument, "%s must be printable ASCII", headerSubject)
+	}
 
-	role := Role(firstMetadataValue(md, headerRole))
+	rawRole := firstMetadataValue(md, headerRole)
+	if len(rawRole) > maxRoleLen {
+		return nil, status.Errorf(codes.InvalidArgument, "%s header exceeds %d bytes", headerRole, maxRoleLen)
+	}
+	role := Role(rawRole)
 	if role == "" {
 		role = RoleSuperAdmin
 	}
 	switch role {
 	case RoleSuperAdmin, RoleAdmin, RoleUser:
 	default:
-		return nil, status.Errorf(codes.PermissionDenied, "unknown role: %s", role)
+		m.logger.WarnContext(ctx, "metadata auth: unknown role", "role", rawRole, "subject", subject)
+		return nil, status.Error(codes.PermissionDenied, "unknown role")
 	}
 
 	// Parse tenant IDs — comma-separated in x-tenant-id header.
 	// If a resolver is configured, slugs are resolved to UUIDs.
-	var tenantIDs []string
 	rawTenantID := firstMetadataValue(md, headerTenantID)
+	if len(rawTenantID) > maxHeaderLen {
+		return nil, status.Errorf(codes.InvalidArgument, "%s header exceeds %d bytes", headerTenantID, maxHeaderLen)
+	}
+	var tenantIDs []string
 	if rawTenantID != "" {
-		for _, id := range strings.Split(rawTenantID, ",") {
+		parts := strings.Split(rawTenantID, ",")
+		if len(parts) > maxTenantIDs {
+			return nil, status.Errorf(codes.InvalidArgument, "%s exceeds %d entries", headerTenantID, maxTenantIDs)
+		}
+		for _, id := range parts {
 			id = strings.TrimSpace(id)
 			if id == "" {
 				continue
 			}
 			if m.resolveTenant != nil {
 				resolved, err := m.resolveTenant(ctx, id)
 				if err != nil {
-					return nil, status.Errorf(codes.InvalidArgument, "failed to resolve tenant %q: %v", id, err)
+					m.logger.WarnContext(ctx, "metadata auth: tenant resolution failed", "tenant", id, "error", err)
+					return nil, status.Error(codes.InvalidArgument, "failed to resolve tenant")
 				}
 				id = resolved
 			}
@@ -126,3 +174,14 @@ func firstMetadataValue(md metadata.MD, key string) string {
 	}
 	return values[0]
 }
+
+// isPrintableASCII reports whether s contains only printable ASCII (0x20–0x7E).
+func isPrintableASCII(s string) bool {
+	for i := 0; i < len(s); i++ {
+		c := s[i]
+		if c < 0x20 || c > 0x7E {
+			return false
+		}
+	}
+	return true
+}

internal/auth/metadata_test.go

@@ -1,7 +1,11 @@
 package auth
 
 import (
+	"bytes"
 	"context"
+	"errors"
+	"log/slog"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -186,6 +190,139 @@ func TestMetadata_ServerServiceBypass(t *testing.T) {
 	assert.Equal(t, "ok", resp)
 }
 
+func TestMetadata_OversizedSubject(t *testing.T) {
+	interceptor := NewMetadataInterceptor(nil)
+	unary := interceptor.UnaryInterceptor()
+
+	ctx := ctxWithMetadata(map[string]string{"x-subject": strings.Repeat("a", maxSubjectLen+1)})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, status.Code(err))
+}
+
+func TestMetadata_HugeSubjectHeader(t *testing.T) {
+	interceptor := NewMetadataInterceptor(nil)
+	unary := interceptor.UnaryInterceptor()
+
+	ctx := ctxWithMetadata(map[string]string{"x-subject": strings.Repeat("a", maxHeaderLen+1)})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, status.Code(err))
+}
+
+func TestMetadata_NonPrintableSubject(t *testing.T) {
+	interceptor := NewMetadataInterceptor(nil)
+	unary := interceptor.UnaryInterceptor()
+
+	ctx := ctxWithMetadata(map[string]string{"x-subject": "user\x00@example.com"})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, status.Code(err))
+	assert.Contains(t, status.Convert(err).Message(), "printable ASCII")
+}
+
+func TestMetadata_OversizedRole(t *testing.T) {
+	interceptor := NewMetadataInterceptor(nil)
+	unary := interceptor.UnaryInterceptor()
+
+	ctx := ctxWithMetadata(map[string]string{
+		"x-subject": "user@example.com",
+		"x-role":    strings.Repeat("a", maxRoleLen+1),
+	})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, status.Code(err))
+}
+
+func TestMetadata_OversizedTenantHeader(t *testing.T) {
+	interceptor := NewMetadataInterceptor(nil)
+	unary := interceptor.UnaryInterceptor()
+
+	ctx := ctxWithMetadata(map[string]string{
+		"x-subject":   "user@example.com",
+		"x-role":      "admin",
+		"x-tenant-id": strings.Repeat("a", maxHeaderLen+1),
+	})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, status.Code(err))
+}
+
+func TestMetadata_TooManyTenantIDs(t *testing.T) {
+	interceptor := NewMetadataInterceptor(nil)
+	unary := interceptor.UnaryInterceptor()
+
+	ids := make([]string, maxTenantIDs+1)
+	for i := range ids {
+		ids[i] = "t"
+	}
+	ctx := ctxWithMetadata(map[string]string{
+		"x-subject":   "admin@example.com",
+		"x-role":      "admin",
+		"x-tenant-id": strings.Join(ids, ","),
+	})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, status.Code(err))
+	assert.Contains(t, status.Convert(err).Message(), "exceeds")
+}
+
+func TestMetadata_TenantResolverErrorSanitised(t *testing.T) {
+	var logBuf bytes.Buffer
+	logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelWarn}))
+
+	resolver := func(_ context.Context, _ string) (string, error) {
+		return "", errors.New("pq: relation \"tenants\" does not exist")
+	}
+	interceptor := NewMetadataInterceptor(resolver, WithMetadataLogger(logger))
+	unary := interceptor.UnaryInterceptor()
+
+	ctx := ctxWithMetadata(map[string]string{
+		"x-subject":   "admin@example.com",
+		"x-role":      "admin",
+		"x-tenant-id": "tenant-123",
+	})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, status.Code(err))
+	msg := status.Convert(err).Message()
+	assert.Equal(t, "failed to resolve tenant", msg)
+	assert.NotContains(t, msg, "pq:")
+	assert.NotContains(t, msg, "tenants")
+
+	logged := logBuf.String()
+	assert.Contains(t, logged, "tenant resolution failed")
+	assert.Contains(t, logged, "pq: relation")
+}
+
+func TestMetadata_UnknownRoleSanitised(t *testing.T) {
+	var logBuf bytes.Buffer
+	logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelWarn}))
+	interceptor := NewMetadataInterceptor(nil, WithMetadataLogger(logger))
+	unary := interceptor.UnaryInterceptor()
+
+	ctx := ctxWithMetadata(map[string]string{
+		"x-subject": "user@example.com",
+		"x-role":    "secretrole",
+	})
+	_, err := unary(ctx, nil, &grpc.UnaryServerInfo{FullMethod: "/test.Service/Method"}, noopHandler)
+
+	require.Error(t, err)
+	assert.Equal(t, codes.PermissionDenied, status.Code(err))
+	assert.Equal(t, "unknown role", status.Convert(err).Message())
+
+	logged := logBuf.String()
+	assert.Contains(t, logged, "unknown role")
+	assert.Contains(t, logged, "secretrole")
+}
+
 func TestMetadata_StreamInterceptor(t *testing.T) {
 	interceptor := NewMetadataInterceptor(nil)
 	stream := interceptor.StreamInterceptor()