Address additional review feedback from PR #5959

- Use proto.Size() check for identifying_properties validation
  (more robust than counting properties, catches large values)
- Remove CustomValidators from EntityCreationOptions
  (no planned use case, reduces complexity)
- Use ReplaceAllProperties instead of SaveAllProperties
  (ensures clean slate for entity creation)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

Author: JAORMX

internal/controlplane/handlers_entity_instances.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/google/uuid"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/protobuf/proto"
 
 	"github.com/mindersec/minder/internal/engine/engcontext"
 	"github.com/mindersec/minder/internal/entities/models"
@@ -245,25 +246,27 @@ func (s *Server) RegisterEntity(
 
 // parseIdentifyingProperties converts proto properties to Properties object
 func parseIdentifyingProperties(req *pb.RegisterEntityRequest) (*properties.Properties, error) {
-	if req.GetIdentifyingProperties() == nil {
+	identifyingProps := req.GetIdentifyingProperties()
+	if identifyingProps == nil {
 		return nil, errors.New("identifying_properties is required")
 	}
 
-	propsMap := req.GetIdentifyingProperties().AsMap()
-
-	// Validate reasonable property count to prevent resource exhaustion
-	const maxPropertyCount = 100
-	if len(propsMap) > maxPropertyCount {
-		return nil, fmt.Errorf("too many identifying properties: got %d, max %d",
-			len(propsMap), maxPropertyCount)
+	// Validate total size to prevent resource exhaustion
+	// Using proto.Size provides a better bound than counting properties,
+	// as it accounts for arbitrarily large values in a Struct
+	const maxProtoSize = 32 * 1024 // 32KB should be plenty for identifying properties
+	if protoSize := proto.Size(identifyingProps); protoSize > maxProtoSize {
+		return nil, fmt.Errorf("identifying_properties too large: %d bytes, max %d bytes",
+			protoSize, maxProtoSize)
 	}
 
-	// Validate property keys are reasonable (alphanumeric, slash, underscore, hyphen)
+	propsMap := identifyingProps.AsMap()
+
+	// Validate property keys are reasonable length
 	for key := range propsMap {
 		if len(key) > 200 {
 			return nil, fmt.Errorf("property key too long: %d characters", len(key))
 		}
-		// Note: Additional key sanitization could be added here if needed
 	}
 
 	return properties.NewProperties(propsMap), nil

internal/controlplane/handlers_entity_instances_test.go

@@ -363,19 +363,22 @@ func TestParseIdentifyingProperties(t *testing.T) {
 			errContains: "identifying_properties is required",
 		},
 		{
-			name: "rejects too many properties",
+			name: "rejects properties that are too large",
 			request: &pb.RegisterEntityRequest{
 				IdentifyingProperties: func() *structpb.Struct {
-					props := make(map[string]any)
-					for i := 0; i < 101; i++ {
-						props[string(rune('a'+i%26))+string(rune(i))] = "value"
+					// Create a value large enough to exceed 32KB limit
+					largeValue := string(make([]byte, 33*1024))
+					for i := range largeValue {
+						largeValue = string(append([]byte(largeValue[:i]), 'x'))
 					}
-					s, _ := structpb.NewStruct(props)
+					s, _ := structpb.NewStruct(map[string]any{
+						"large_key": largeValue,
+					})
 					return s
 				}(),
 			},
 			wantErr:     true,
-			errContains: "too many identifying properties",
+			errContains: "identifying_properties too large",
 		},
 		{
 			name: "rejects property key that is too long",

internal/entities/service/entity_creator.go

@@ -37,9 +37,6 @@ type EntityCreationOptions struct {
 
 	// Whether to publish reconciliation events
 	PublishReconciliationEvent bool
-
-	// Custom validators to run (in addition to default validators)
-	CustomValidators []EntityValidator
 }
 
 // EntityCreator creates entities in a consistent, reusable way
@@ -125,8 +122,8 @@ func (e *entityCreator) CreateEntity(
 		return nil, fmt.Errorf("error fetching properties: %w", err)
 	}
 
-	// 4. Run validators (default + custom)
-	if err := e.runValidators(ctx, entityType, allProps, projectID, opts.CustomValidators); err != nil {
+	// 4. Run validators
+	if err := e.runValidators(ctx, entityType, allProps, projectID); err != nil {
 		return nil, err
 	}
 
@@ -175,8 +172,9 @@ func (e *entityCreator) CreateEntity(
 			return nil, fmt.Errorf("error creating entity: %w", err)
 		}
 
-		// Save properties
-		if err := e.propSvc.SaveAllProperties(ctx, ent.ID, registeredProps,
+		// Replace properties - use Replace to ensure a clean slate
+		// (removes any stale properties from previous failed attempts)
+		if err := e.propSvc.ReplaceAllProperties(ctx, ent.ID, registeredProps,
 			propService.CallBuilder().WithStoreOrTransaction(t)); err != nil {
 			return nil, fmt.Errorf("error saving properties: %w", err)
 		}
@@ -206,10 +204,8 @@ func (e *entityCreator) runValidators(
 	entityType pb.Entity,
 	allProps *properties.Properties,
 	projectID uuid.UUID,
-	customValidators []EntityValidator,
 ) error {
-	allValidators := append(e.validators, customValidators...)
-	for _, validator := range allValidators {
+	for _, validator := range e.validators {
 		if err := validator.Validate(ctx, entityType, allProps, projectID); err != nil {
 			return err
 		}