ks-404 validate ids prior to using as seed for transmission schedule

.changeset/slimy-cars-sparkle.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#internal ks-404 validate ids before using as seed of transmission schedule

core/capabilities/ccip/launcher/test_helpers.go

@@ -24,7 +24,7 @@ var (
 	p2pID1            = getP2PID(1)
 	p2pID2            = getP2PID(2)
 	defaultCapCfgs    = map[string]registrysyncer.CapabilityConfiguration{
-		defaultCapability.ID: registrysyncer.CapabilityConfiguration{},
+		defaultCapability.ID: {},
 	}
 	defaultRegistryDon = registrysyncer.DON{
 		DON:                      getDON(1, []ragep2ptypes.PeerID{p2pID1}, 0),

core/capabilities/remote/target/client.go

@@ -12,6 +12,7 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/remote"
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/remote/target/request"
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/remote/types"
+	"github.com/smartcontractkit/chainlink/v2/core/capabilities/validation"
 	"github.com/smartcontractkit/chainlink/v2/core/logger"
 )
 
@@ -172,8 +173,12 @@ func (c *client) Receive(ctx context.Context, msg *types.MessageBody) {
 }
 
 func GetMessageIDForRequest(req commoncap.CapabilityRequest) (string, error) {
-	if !remote.IsValidWorkflowOrExecutionID(req.Metadata.WorkflowID) || !remote.IsValidWorkflowOrExecutionID(req.Metadata.WorkflowExecutionID) {
-		return "", errors.New("workflow ID and workflow execution ID in request metadata are invalid")
+	if err := validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowID); err != nil {
+		return "", fmt.Errorf("workflow ID is invalid: %w", err)
+	}
+
+	if err := validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowExecutionID); err != nil {
+		return "", fmt.Errorf("workflow execution ID is invalid: %w", err)
 	}
 
 	return req.Metadata.WorkflowID + req.Metadata.WorkflowExecutionID, nil

core/capabilities/remote/trigger_publisher.go

@@ -10,6 +10,7 @@ import (
 	"github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb"
 	"github.com/smartcontractkit/chainlink-common/pkg/services"
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/remote/types"
+	"github.com/smartcontractkit/chainlink/v2/core/capabilities/validation"
 	"github.com/smartcontractkit/chainlink/v2/core/logger"
 	p2ptypes "github.com/smartcontractkit/chainlink/v2/core/services/p2p/types"
 )
@@ -102,8 +103,8 @@ func (p *triggerPublisher) Receive(_ context.Context, msg *types.MessageBody) {
 			p.lggr.Errorw("sender not a member of its workflow DON", "capabilityId", p.capInfo.ID, "callerDonId", msg.CallerDonId, "sender", sender)
 			return
 		}
-		if !IsValidWorkflowOrExecutionID(req.Metadata.WorkflowID) {
-			p.lggr.Errorw("received trigger request with invalid workflow ID", "capabilityId", p.capInfo.ID, "workflowId", SanitizeLogString(req.Metadata.WorkflowID))
+		if err = validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowID); err != nil {
+			p.lggr.Errorw("received trigger request with invalid workflow ID", "capabilityId", p.capInfo.ID, "workflowId", SanitizeLogString(req.Metadata.WorkflowID), "err", err)
 			return
 		}
 		p.lggr.Debugw("received trigger registration", "capabilityId", p.capInfo.ID, "workflowId", req.Metadata.WorkflowID, "sender", sender)

core/capabilities/remote/utils.go

@@ -19,7 +19,6 @@ import (
 
 const (
 	maxLoggedStringLen = 256
-	validWorkflowIDLen = 64
 	maxIDLen           = 128
 )
 
@@ -116,15 +115,6 @@ func SanitizeLogString(s string) string {
 	return s + tooLongSuffix
 }
 
-// Workflow IDs and Execution IDs are 32-byte hex-encoded strings
-func IsValidWorkflowOrExecutionID(id string) bool {
-	if len(id) != validWorkflowIDLen {
-		return false
-	}
-	_, err := hex.DecodeString(id)
-	return err == nil
-}
-
 // Trigger event IDs and message IDs can only contain printable characters and must be non-empty
 func IsValidID(id string) bool {
 	if len(id) == 0 || len(id) > maxIDLen {

core/capabilities/remote/utils_test.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/remote"
 	remotetypes "github.com/smartcontractkit/chainlink/v2/core/capabilities/remote/types"
+	"github.com/smartcontractkit/chainlink/v2/core/capabilities/validation"
 	p2ptypes "github.com/smartcontractkit/chainlink/v2/core/services/p2p/types"
 )
 
@@ -131,9 +132,9 @@ func TestSanitizeLogString(t *testing.T) {
 }
 
 func TestIsValidWorkflowID(t *testing.T) {
-	require.False(t, remote.IsValidWorkflowOrExecutionID("too_short"))
-	require.False(t, remote.IsValidWorkflowOrExecutionID("nothex--95ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0"))
-	require.True(t, remote.IsValidWorkflowOrExecutionID("15c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0"))
+	require.NotNil(t, validation.ValidateWorkflowOrExecutionID("too_short"))
+	require.NotNil(t, validation.ValidateWorkflowOrExecutionID("nothex--95ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0"))
+	require.NoError(t, validation.ValidateWorkflowOrExecutionID("15c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0"))
 }
 
 func TestIsValidTriggerEventID(t *testing.T) {

core/capabilities/transmission/local_target_capability_test.go

@@ -54,8 +54,8 @@ func TestScheduledExecutionStrategy_LocalDON(t *testing.T) {
 			name:     "position 0; oneAtATime",
 			position: 0,
 			schedule: "oneAtATime",
-			low:      300 * time.Millisecond,
-			high:     400 * time.Millisecond,
+			low:      200 * time.Millisecond,
+			high:     300 * time.Millisecond,
 		},
 		{
 			name:     "position 1; oneAtATime",
@@ -68,15 +68,15 @@ func TestScheduledExecutionStrategy_LocalDON(t *testing.T) {
 			name:     "position 2; oneAtATime",
 			position: 2,
 			schedule: "oneAtATime",
-			low:      0 * time.Millisecond,
-			high:     100 * time.Millisecond,
+			low:      300 * time.Millisecond,
+			high:     400 * time.Millisecond,
 		},
 		{
 			name:     "position 3; oneAtATime",
 			position: 3,
 			schedule: "oneAtATime",
-			low:      100 * time.Millisecond,
-			high:     300 * time.Millisecond,
+			low:      0 * time.Millisecond,
+			high:     100 * time.Millisecond,
 		},
 		{
 			name:     "position 0; allAtOnce",
@@ -121,8 +121,8 @@ func TestScheduledExecutionStrategy_LocalDON(t *testing.T) {
 			req := capabilities.CapabilityRequest{
 				Config: m,
 				Metadata: capabilities.RequestMetadata{
-					WorkflowID:          "mock-workflow-id",
-					WorkflowExecutionID: "mock-execution-id-1",
+					WorkflowID:          "15c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0",
+					WorkflowExecutionID: "32c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce1",
 				},
 			}
 

core/capabilities/transmission/transmission.go

@@ -4,10 +4,10 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/smartcontractkit/libocr/permutation"
 
+	"github.com/smartcontractkit/chainlink/v2/core/capabilities/validation"
+
 	"github.com/smartcontractkit/chainlink-common/pkg/capabilities"
 	"github.com/smartcontractkit/chainlink-common/pkg/values"
 	"github.com/smartcontractkit/chainlink/v2/core/services/p2p/types"
@@ -56,8 +56,12 @@ func GetPeerIDToTransmissionDelay(donPeerIDs []types.PeerID, req capabilities.Ca
 		return nil, fmt.Errorf("failed to extract transmission config from request: %w", err)
 	}
 
-	if req.Metadata.WorkflowID == "" || req.Metadata.WorkflowExecutionID == "" {
-		return nil, errors.New("workflow ID and workflow execution ID must be set in request metadata")
+	if err = validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowID); err != nil {
+		return nil, fmt.Errorf("workflow ID is invalid: %w", err)
+	}
+
+	if err = validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowExecutionID); err != nil {
+		return nil, fmt.Errorf("workflow execution ID is invalid: %w", err)
 	}
 
 	transmissionID := req.Metadata.WorkflowID + req.Metadata.WorkflowExecutionID

core/capabilities/transmission/transmission_test.go

@@ -36,38 +36,40 @@ func Test_GetPeerIDToTransmissionDelay(t *testing.T) {
 			"one",
 			"oneAtATime",
 			"100ms",
-			"mock-execution-id",
+			"15c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0",
 			map[string]time.Duration{
 				"one":   300 * time.Millisecond,
-				"two":   100 * time.Millisecond,
-				"three": 0 * time.Millisecond,
+				"two":   0 * time.Millisecond,
+				"three": 100 * time.Millisecond,
 				"four":  200 * time.Millisecond,
 			},
 		},
+
 		{
 			"TestAllAtOnce",
 			"one",
 			"allAtOnce",
 			"100ms",
-			"mock-execution-id",
+			"15c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0",
 			map[string]time.Duration{
 				"one":   0 * time.Millisecond,
 				"two":   0 * time.Millisecond,
 				"three": 0 * time.Millisecond,
 				"four":  0 * time.Millisecond,
 			},
 		},
+
 		{
 			"TestOneAtATimeWithDifferentExecutionID",
 			"one",
 			"oneAtATime",
 			"100ms",
-			"mock-execution-id2",
+			"16c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce1",
 			map[string]time.Duration{
-				"one":   0 * time.Millisecond,
-				"two":   200 * time.Millisecond,
-				"three": 100 * time.Millisecond,
-				"four":  300 * time.Millisecond,
+				"one":   300 * time.Millisecond,
+				"two":   100 * time.Millisecond,
+				"three": 200 * time.Millisecond,
+				"four":  0 * time.Millisecond,
 			},
 		},
 	}
@@ -83,7 +85,7 @@ func Test_GetPeerIDToTransmissionDelay(t *testing.T) {
 			capabilityRequest := capabilities.CapabilityRequest{
 				Config: transmissionCfg,
 				Metadata: capabilities.RequestMetadata{
-					WorkflowID:          "mock-workflow-id",
+					WorkflowID:          "17c631d295ef5e32deb99a10ee6804bc4af13855687559d7ff6552ac6dbb2ce0",
 					WorkflowExecutionID: tc.workflowExecutionID,
 				},
 			}

core/capabilities/validation/validation.go

@@ -0,0 +1,23 @@
+package validation
+
+import (
+	"encoding/hex"
+	"errors"
+)
+
+const (
+	validWorkflowIDLen = 64
+)
+
+// Workflow IDs and Execution IDs are 32-byte hex-encoded strings
+func ValidateWorkflowOrExecutionID(id string) error {
+	if len(id) != validWorkflowIDLen {
+		return errors.New("must be 32 bytes long")
+	}
+	_, err := hex.DecodeString(id)
+	if err != nil {
+		return errors.New("must be a hex-encoded string")
+	}
+
+	return nil
+}