Added tests for vault blob broadcast size and batch request limit in …

core/capabilities/vault/capability.go

@@ -71,6 +71,23 @@ func (s *Capability) Close() error {
 	if lerr := s.MaxRequestBatchSizeLimiter.Close(); lerr != nil {
 		err = errors.Join(err, fmt.Errorf("error closing request batch size limiter: %w", lerr))
 	}
+
+	if lerr := s.MaxCiphertextLengthLimiter.Close(); lerr != nil {
+		err = errors.Join(err, fmt.Errorf("error closing ciphertext size limiter: %w", lerr))
+	}
+
+	if lerr := s.MaxIdentifierKeyLengthLimiter.Close(); lerr != nil {
+		err = errors.Join(err, fmt.Errorf("error closing identifier key length limiter: %w", lerr))
+	}
+
+	if lerr := s.MaxIdentifierOwnerLengthLimiter.Close(); lerr != nil {
+		err = errors.Join(err, fmt.Errorf("error closing identifier owner length limiter: %w", lerr))
+	}
+
+	if lerr := s.MaxIdentifierNamespaceLengthLimiter.Close(); lerr != nil {
+		err = errors.Join(err, fmt.Errorf("error closing identifier namespace length limiter: %w", lerr))
+	}
+
 	if lerr := s.linker.Close(); lerr != nil {
 		err = errors.Join(err, fmt.Errorf("error closing org_id linker: %w", lerr))
 	}
@@ -201,7 +218,7 @@ func (s *Capability) UpdateSecrets(ctx context.Context, request *vaultcommon.Upd
 
 func (s *Capability) DeleteSecrets(ctx context.Context, request *vaultcommon.DeleteSecretsRequest) (*vaulttypes.Response, error) {
 	s.lggr.Debugf("Received Request: %s", request.String())
-	err := s.ValidateDeleteSecretsRequest(request)
+	err := s.ValidateDeleteSecretsRequest(ctx, request)
 	if err != nil {
 		s.lggr.Debugf("Request: [%s] failed validation checks: %s", request.String(), err.Error())
 		return nil, err
@@ -217,7 +234,7 @@ func (s *Capability) DeleteSecrets(ctx context.Context, request *vaultcommon.Del
 
 func (s *Capability) GetSecrets(ctx context.Context, requestID string, request *vaultcommon.GetSecretsRequest) (*vaulttypes.Response, error) {
 	s.lggr.Debugf("Received Request: %s", request.String())
-	if err := s.ValidateGetSecretsRequest(request); err != nil {
+	if err := s.ValidateGetSecretsRequest(ctx, request); err != nil {
 		s.lggr.Debugf("Request: [%s] failed validation checks: %s", request.String(), err.Error())
 		return nil, err
 	}
@@ -228,7 +245,7 @@ func (s *Capability) GetSecrets(ctx context.Context, requestID string, request *
 
 func (s *Capability) ListSecretIdentifiers(ctx context.Context, request *vaultcommon.ListSecretIdentifiersRequest) (*vaulttypes.Response, error) {
 	s.lggr.Debugf("Received Request: %s", request.String())
-	err := s.ValidateListSecretIdentifiersRequest(request)
+	err := s.ValidateListSecretIdentifiersRequest(ctx, request)
 	if err != nil {
 		s.lggr.Debugf("Request: [%s] failed validation checks: %s", request.String(), err.Error())
 		return nil, err
@@ -314,7 +331,7 @@ func NewCapability(
 	orgResolver orgresolver.OrgResolver,
 	limitsFactory limits.Factory,
 ) (*Capability, error) {
-	limiter, err := limits.MakeBoundLimiter(limitsFactory, cresettings.Default.VaultRequestBatchSizeLimit)
+	limiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultRequestBatchSizeLimit)
 	if err != nil {
 		return nil, fmt.Errorf("could not create request batch size limiter: %w", err)
 	}
@@ -326,6 +343,18 @@ func NewCapability(
 	if err != nil {
 		return nil, fmt.Errorf("could not create ciphertext size limiter: %w", err)
 	}
+	idKeyLengthLimiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultIdentifierKeySizeLimit)
+	if err != nil {
+		return nil, fmt.Errorf("could not create identifier key length limiter: %w", err)
+	}
+	idOwnerLengthLimiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultIdentifierOwnerSizeLimit)
+	if err != nil {
+		return nil, fmt.Errorf("could not create identifier owner length limiter: %w", err)
+	}
+	idNamespaceLengthLimiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultIdentifierNamespaceSizeLimit)
+	if err != nil {
+		return nil, fmt.Errorf("could not create identifier namespace length limiter: %w", err)
+	}
 	return &Capability{
 		lggr:                 logger.Named(lggr, "VaultCapability"),
 		clock:                clock,
@@ -334,6 +363,6 @@ func NewCapability(
 		capabilitiesRegistry: capabilitiesRegistry,
 		publicKey:            publicKey,
 		linker:               linker,
-		RequestValidator:     NewRequestValidator(limiter, ciphertextLimiter),
+		RequestValidator:     NewRequestValidator(limiter, ciphertextLimiter, idKeyLengthLimiter, idOwnerLengthLimiter, idNamespaceLengthLimiter),
 	}, nil
 }

core/capabilities/vault/capability_test.go

@@ -847,7 +847,7 @@ func TestCapability_CRUD(t *testing.T) {
 		{
 			name:     "CreateSecrets_Missing_Key",
 			response: nil,
-			error:    "secret ID must have key, namespace and owner set",
+			error:    "key cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.CreateSecretsRequest{
 					RequestId: requestID,
@@ -868,7 +868,7 @@ func TestCapability_CRUD(t *testing.T) {
 		{
 			name:     "CreateSecrets_Missing_Namespace",
 			response: nil,
-			error:    "secret ID must have key, namespace and owner set",
+			error:    "namespace cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.CreateSecretsRequest{
 					RequestId: requestID,
@@ -889,7 +889,7 @@ func TestCapability_CRUD(t *testing.T) {
 		{
 			name:     "CreateSecrets_Missing_Owner",
 			response: nil,
-			error:    "secret ID must have key, namespace and owner set",
+			error:    "owner cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.CreateSecretsRequest{
 					RequestId: requestID,
@@ -1037,7 +1037,7 @@ func TestCapability_CRUD(t *testing.T) {
 				Payload: []byte("hello world"),
 				Format:  "protobuf",
 			},
-			error: "secret ID must have key, namespace and owner set at index",
+			error: "key cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.UpdateSecretsRequest{
 					RequestId: requestID,
@@ -1062,7 +1062,7 @@ func TestCapability_CRUD(t *testing.T) {
 				Payload: []byte("hello world"),
 				Format:  "protobuf",
 			},
-			error: "secret ID must have key, namespace and owner set at index",
+			error: "namespace cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.UpdateSecretsRequest{
 					RequestId: requestID,
@@ -1087,7 +1087,7 @@ func TestCapability_CRUD(t *testing.T) {
 				Payload: []byte("hello world"),
 				Format:  "protobuf",
 			},
-			error: "secret ID must have key, namespace and owner set at index",
+			error: "owner cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.UpdateSecretsRequest{
 					RequestId: requestID,
@@ -1287,7 +1287,7 @@ func TestCapability_CRUD(t *testing.T) {
 		{
 			name:     "DeleteSecrets_Missing_Owner",
 			response: nil,
-			error:    "secret ID must have key, namespace and owner set at index 0",
+			error:    "owner cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.DeleteSecretsRequest{
 					RequestId: requestID,
@@ -1305,7 +1305,7 @@ func TestCapability_CRUD(t *testing.T) {
 		{
 			name:     "DeleteSecrets_Missing_Namespace",
 			response: nil,
-			error:    "secret ID must have key, namespace and owner set at index 0",
+			error:    "namespace cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.DeleteSecretsRequest{
 					RequestId: requestID,
@@ -1323,7 +1323,7 @@ func TestCapability_CRUD(t *testing.T) {
 		{
 			name:     "DeleteSecrets_Missing_Key",
 			response: nil,
-			error:    "secret ID must have key, namespace and owner set at index 0",
+			error:    "key cannot be empty",
 			call: func(t *testing.T, capability *Capability) (*vaulttypes.Response, error) {
 				req := &vault.DeleteSecretsRequest{
 					RequestId: requestID,

core/capabilities/vault/limiter_helpers_test.go

@@ -0,0 +1,34 @@
+package vault
+
+import (
+	"context"
+
+	pkgconfig "github.com/smartcontractkit/chainlink-common/pkg/config"
+	"github.com/smartcontractkit/chainlink-common/pkg/contexts"
+	"github.com/smartcontractkit/chainlink-common/pkg/settings/limits"
+)
+
+// ownerOverrideLimiter is a BoundLimiter that applies different size limits based on the owner in context.
+// This lets tests verify that the validator correctly threads the owner through context to the limiter.
+type ownerOverrideLimiter struct {
+	defaultBound pkgconfig.Size
+	overrides    map[string]pkgconfig.Size
+}
+
+func (o *ownerOverrideLimiter) Close() error { return nil }
+func (o *ownerOverrideLimiter) Limit(ctx context.Context) (pkgconfig.Size, error) {
+	return o.boundFor(ctx), nil
+}
+func (o *ownerOverrideLimiter) Check(ctx context.Context, n pkgconfig.Size) error {
+	bound := o.boundFor(ctx)
+	if n > bound {
+		return limits.ErrorBoundLimited[pkgconfig.Size]{Limit: bound, Amount: n}
+	}
+	return nil
+}
+func (o *ownerOverrideLimiter) boundFor(ctx context.Context) pkgconfig.Size {
+	if override, ok := o.overrides[contexts.CREValue(ctx).Owner]; ok {
+		return override
+	}
+	return o.defaultBound
+}

core/capabilities/vault/validator.go

@@ -5,21 +5,30 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"regexp"
 	"strconv"
 	"strings"
 
 	"github.com/smartcontractkit/tdh2/go/tdh2/tdh2easy"
 
 	vaultcommon "github.com/smartcontractkit/chainlink-common/pkg/capabilities/actions/vault"
 	pkgconfig "github.com/smartcontractkit/chainlink-common/pkg/config"
+	"github.com/smartcontractkit/chainlink-common/pkg/contexts"
 	"github.com/smartcontractkit/chainlink-common/pkg/settings/limits"
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/vault/vaulttypes"
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/vault/vaultutils"
 )
 
+var (
+	isValidIDComponent = regexp.MustCompile(`^[a-zA-Z0-9_]+$`).MatchString
+)
+
 type RequestValidator struct {
-	MaxRequestBatchSizeLimiter limits.BoundLimiter[int]
-	MaxCiphertextLengthLimiter limits.BoundLimiter[pkgconfig.Size]
+	MaxRequestBatchSizeLimiter          limits.BoundLimiter[int]
+	MaxCiphertextLengthLimiter          limits.BoundLimiter[pkgconfig.Size]
+	MaxIdentifierKeyLengthLimiter       limits.BoundLimiter[pkgconfig.Size]
+	MaxIdentifierOwnerLengthLimiter     limits.BoundLimiter[pkgconfig.Size]
+	MaxIdentifierNamespaceLengthLimiter limits.BoundLimiter[pkgconfig.Size]
 }
 
 func (r *RequestValidator) ValidateCreateSecretsRequest(ctx context.Context, publicKey *tdh2easy.PublicKey, request *vaultcommon.CreateSecretsRequest) error {
@@ -56,14 +65,14 @@ func (r *RequestValidator) validateWriteRequest(ctx context.Context, publicKey *
 			return errors.New("secret ID must not be nil at index " + strconv.Itoa(idx))
 		}
 
-		if req.Id.Key == "" || req.Id.Namespace == "" || req.Id.Owner == "" {
-			return errors.New("secret ID must have key, namespace and owner set at index " + strconv.Itoa(idx) + ":" + req.Id.String())
-		}
-
 		if req.EncryptedValue == "" {
 			return errors.New("secret must have encrypted value set at index " + strconv.Itoa(idx) + ":" + req.Id.String())
 		}
-		if err := r.validateCiphertextSize(ctx, req.EncryptedValue); err != nil {
+
+		if err := r.ValidateSecretIdentifier(ctx, req.Id.Key, req.Id.Owner, req.Id.Namespace); err != nil {
+			return fmt.Errorf("invalid secret identifier at index %d: %w", idx, err)
+		}
+		if err := r.ValidateCiphertextSize(ctx, req.Id.Owner, req.EncryptedValue); err != nil {
 			return fmt.Errorf("secret encrypted value at index %d is invalid: %w", idx, err)
 		}
 		expectedWorkflowOwner := workflowOwner
@@ -85,12 +94,13 @@ func (r *RequestValidator) validateWriteRequest(ctx context.Context, publicKey *
 	return nil
 }
 
-func (r *RequestValidator) validateCiphertextSize(ctx context.Context, encryptedValue string) error {
+func (r *RequestValidator) ValidateCiphertextSize(ctx context.Context, owner string, encryptedValue string) error {
 	rawCiphertext, err := hex.DecodeString(encryptedValue)
 	if err != nil {
 		return fmt.Errorf("failed to decode encrypted value: %w", err)
 	}
-	if err := r.MaxCiphertextLengthLimiter.Check(ctx, pkgconfig.Size(len(rawCiphertext))*pkgconfig.Byte); err != nil {
+	innerCtx := contexts.WithCRE(ctx, contexts.CRE{Owner: owner})
+	if err := r.MaxCiphertextLengthLimiter.Check(innerCtx, pkgconfig.Size(len(rawCiphertext))*pkgconfig.Byte); err != nil {
 		var errBoundLimited limits.ErrorBoundLimited[pkgconfig.Size]
 		if errors.As(err, &errBoundLimited) {
 			return fmt.Errorf("ciphertext size exceeds maximum allowed size: %s", errBoundLimited.Limit)
@@ -100,7 +110,50 @@ func (r *RequestValidator) validateCiphertextSize(ctx context.Context, encrypted
 	return nil
 }
 
-func (r *RequestValidator) ValidateGetSecretsRequest(request *vaultcommon.GetSecretsRequest) error {
+func (r *RequestValidator) ValidateSecretIdentifier(ctx context.Context, idKey string, idOwner string, idNamespace string) error {
+	if idKey == "" {
+		return fmt.Errorf("key cannot be empty")
+	}
+	if idOwner == "" {
+		return fmt.Errorf("owner cannot be empty")
+	}
+	if idNamespace == "" {
+		return fmt.Errorf("namespace cannot be empty")
+	}
+
+	if !isValidIDComponent(idKey) || !isValidIDComponent(idOwner) || !isValidIDComponent(idNamespace) {
+		return fmt.Errorf("key, owner and namespace must only contain alphanumeric characters")
+	}
+
+	ctx = contexts.WithCRE(ctx, contexts.CRE{Owner: idOwner})
+	if err := r.MaxIdentifierOwnerLengthLimiter.Check(ctx, pkgconfig.Size(len(idOwner))); err != nil {
+		var errBoundLimited limits.ErrorBoundLimited[pkgconfig.Size]
+		if errors.As(err, &errBoundLimited) {
+			return fmt.Errorf("owner exceeds maximum length of %s", errBoundLimited.Limit)
+		}
+		return fmt.Errorf("failed to check owner length limit: %w", err)
+	}
+
+	if err := r.MaxIdentifierNamespaceLengthLimiter.Check(ctx, pkgconfig.Size(len(idNamespace))); err != nil {
+		var errBoundLimited limits.ErrorBoundLimited[pkgconfig.Size]
+		if errors.As(err, &errBoundLimited) {
+			return fmt.Errorf("namespace exceeds maximum length of %s", errBoundLimited.Limit)
+		}
+		return fmt.Errorf("failed to check namespace length limit: %w", err)
+	}
+
+	if err := r.MaxIdentifierKeyLengthLimiter.Check(ctx, pkgconfig.Size(len(idKey))); err != nil {
+		var errBoundLimited limits.ErrorBoundLimited[pkgconfig.Size]
+		if errors.As(err, &errBoundLimited) {
+			return fmt.Errorf("key exceeds maximum length of %s", errBoundLimited.Limit)
+		}
+		return fmt.Errorf("failed to check key length limit: %w", err)
+	}
+
+	return nil
+}
+
+func (r *RequestValidator) ValidateGetSecretsRequest(ctx context.Context, request *vaultcommon.GetSecretsRequest) error {
 	if len(request.Requests) == 0 {
 		return errors.New("no GetSecret request specified in request")
 	}
@@ -115,19 +168,25 @@ func (r *RequestValidator) ValidateGetSecretsRequest(request *vaultcommon.GetSec
 		if req.Id.Key == "" {
 			return errors.New("secret ID must have key set at index " + strconv.Itoa(idx) + ": " + req.Id.String())
 		}
+		if err := r.ValidateSecretIdentifier(ctx, req.Id.Key, req.Id.Owner, req.Id.Namespace); err != nil {
+			return fmt.Errorf("invalid secret identifier at index %d: %w", idx, err)
+		}
 	}
 
 	return nil
 }
 
-func (r *RequestValidator) ValidateListSecretIdentifiersRequest(request *vaultcommon.ListSecretIdentifiersRequest) error {
+func (r *RequestValidator) ValidateListSecretIdentifiersRequest(ctx context.Context, request *vaultcommon.ListSecretIdentifiersRequest) error {
 	if request.RequestId == "" || request.Owner == "" || request.Namespace == "" {
 		return errors.New("requestID, owner or namespace must not be empty")
 	}
+	if err := r.ValidateSecretIdentifier(ctx, request.Owner, request.Owner, request.Namespace); err != nil {
+		return fmt.Errorf("invalid secret identifier: %w", err)
+	}
 	return nil
 }
 
-func (r *RequestValidator) ValidateDeleteSecretsRequest(request *vaultcommon.DeleteSecretsRequest) error {
+func (r *RequestValidator) ValidateDeleteSecretsRequest(ctx context.Context, request *vaultcommon.DeleteSecretsRequest) error {
 	if request.RequestId == "" {
 		return errors.New("request ID must not be empty")
 	}
@@ -140,8 +199,8 @@ func (r *RequestValidator) ValidateDeleteSecretsRequest(request *vaultcommon.Del
 		if id == nil {
 			return errors.New("secret ID must not be nil at index " + strconv.Itoa(idx))
 		}
-		if id.Key == "" || id.Namespace == "" || id.Owner == "" {
-			return errors.New("secret ID must have key, namespace and owner set at index " + strconv.Itoa(idx) + ": " + id.String())
+		if err := r.ValidateSecretIdentifier(ctx, id.Key, id.Owner, id.Namespace); err != nil {
+			return fmt.Errorf("invalid secret identifier at index %d: %w", idx, err)
 		}
 
 		_, ok := uniqueIDs[vaulttypes.KeyFor(id)]
@@ -157,10 +216,16 @@ func (r *RequestValidator) ValidateDeleteSecretsRequest(request *vaultcommon.Del
 func NewRequestValidator(
 	maxRequestBatchSizeLimiter limits.BoundLimiter[int],
 	maxCiphertextLengthLimiter limits.BoundLimiter[pkgconfig.Size],
+	maxIdentifierKeyLengthLimiter limits.BoundLimiter[pkgconfig.Size],
+	maxIdentifierOwnerLengthLimiter limits.BoundLimiter[pkgconfig.Size],
+	maxIdentifierNamespaceLengthLimiter limits.BoundLimiter[pkgconfig.Size],
 ) *RequestValidator {
 	return &RequestValidator{
-		MaxRequestBatchSizeLimiter: maxRequestBatchSizeLimiter,
-		MaxCiphertextLengthLimiter: maxCiphertextLengthLimiter,
+		MaxRequestBatchSizeLimiter:          maxRequestBatchSizeLimiter,
+		MaxCiphertextLengthLimiter:          maxCiphertextLengthLimiter,
+		MaxIdentifierKeyLengthLimiter:       maxIdentifierKeyLengthLimiter,
+		MaxIdentifierOwnerLengthLimiter:     maxIdentifierOwnerLengthLimiter,
+		MaxIdentifierNamespaceLengthLimiter: maxIdentifierNamespaceLengthLimiter,
 	}
 }