make marshalling of authorized keys deterministic (#1672)

* make marshalling of authorized keys deterministic

* fix test

Author: jinhoonbang

pkg/types/gateway/workflow_metadata.go

@@ -3,7 +3,11 @@ package gateway
 import (
 	"crypto/sha256"
 	"encoding/hex"
-	"encoding/json"
+	"fmt"
+	"sort"
+
+	jsonv2 "github.com/go-json-experiment/json"
+	"github.com/go-json-experiment/json/jsontext"
 )
 
 const (
@@ -23,32 +27,42 @@ type WorkflowMetadata struct {
 	AuthorizedKeys   []AuthorizedKey
 }
 
+// Digest returns a digest of the workflow metadata. This is used for aggregating metadata
+// across multiple nodes. The digest is a SHA256 hash of the canonical JSON representation,
+// ensuring deterministic output regardless of the order in which authorized keys are reported.
 func (wm *WorkflowMetadata) Digest() (string, error) {
-	data, err := json.Marshal(wm)
+	sortedMetadata := WorkflowMetadata{
+		WorkflowSelector: wm.WorkflowSelector,
+		AuthorizedKeys:   make([]AuthorizedKey, len(wm.AuthorizedKeys)),
+	}
+	copy(sortedMetadata.AuthorizedKeys, wm.AuthorizedKeys)
+	sort.Slice(sortedMetadata.AuthorizedKeys, func(i, j int) bool {
+		if sortedMetadata.AuthorizedKeys[i].KeyType != sortedMetadata.AuthorizedKeys[j].KeyType {
+			return sortedMetadata.AuthorizedKeys[i].KeyType < sortedMetadata.AuthorizedKeys[j].KeyType
+		}
+		return sortedMetadata.AuthorizedKeys[i].PublicKey < sortedMetadata.AuthorizedKeys[j].PublicKey
+	})
+
+	JSONBytes, err := jsonv2.Marshal(sortedMetadata, jsonv2.Deterministic(true))
+	if err != nil {
+		return "", fmt.Errorf("error marshaling JSON: %w", err)
+	}
+
+	canonicalJSONBytes := jsontext.Value(JSONBytes)
+	err = canonicalJSONBytes.Canonicalize()
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("error canonicalizing JSON: %w", err)
 	}
+
 	hasher := sha256.New()
-	hasher.Write(data)
-	digestBytes := hasher.Sum(nil)
+	if _, err := hasher.Write(canonicalJSONBytes); err != nil {
+		return "", fmt.Errorf("error writing to hasher: %w", err)
+	}
 
-	return hex.EncodeToString(digestBytes), nil
+	return hex.EncodeToString(hasher.Sum(nil)), nil
 }
 
 type AuthorizedKey struct {
-	KeyType   KeyType `json:"keyType"`
-	PublicKey string  `json:"publicKey"`
-}
-
-// MarshalJSON implements custom JSON marshalling to ensure alphabetical order of keys for AuthorizedKey,
-// and only includes non-empty fields.
-func (r AuthorizedKey) MarshalJSON() ([]byte, error) {
-	m := make(map[string]any)
-	if r.KeyType != "" {
-		m["keyType"] = r.KeyType
-	}
-	if r.PublicKey != "" {
-		m["publicKey"] = r.PublicKey
-	}
-	return marshalWithSortedKeys(m)
+	KeyType   KeyType `json:"keyType,omitempty"`
+	PublicKey string  `json:"publicKey,omitempty"`
 }

pkg/types/gateway/workflow_metadata_test.go

@@ -0,0 +1,95 @@
+package gateway
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestWorkflowMetadata_Digest(t *testing.T) {
+	t.Run("deterministic - same input produces same digest", func(t *testing.T) {
+		metadata := WorkflowMetadata{
+			WorkflowSelector: WorkflowSelector{
+				WorkflowID:    "workflow-456",
+				WorkflowName:  "deterministic-test",
+				WorkflowOwner: "0xTestOwner",
+			},
+			AuthorizedKeys: []AuthorizedKey{
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xkey1"},
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xkey2"},
+			},
+		}
+
+		digest1, err := metadata.Digest()
+		require.NoError(t, err)
+		require.NotEmpty(t, digest1)
+		require.Len(t, digest1, 64)
+
+		digest2, err := metadata.Digest()
+		require.NoError(t, err)
+
+		require.Equal(t, digest1, digest2, "Multiple calls should produce identical digests")
+	})
+
+	t.Run("array ordering does not affect digest - fixes duplicate workflow ID bug", func(t *testing.T) {
+		metadata1 := WorkflowMetadata{
+			WorkflowSelector: WorkflowSelector{WorkflowID: "workflow-789"},
+			AuthorizedKeys: []AuthorizedKey{
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xAAAA"},
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xBBBB"},
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xCCCC"},
+			},
+		}
+
+		metadata2 := WorkflowMetadata{
+			WorkflowSelector: WorkflowSelector{WorkflowID: "workflow-789"},
+			AuthorizedKeys: []AuthorizedKey{
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xBBBB"},
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xCCCC"},
+				{KeyType: KeyTypeECDSAEVM, PublicKey: "0xAAAA"},
+			},
+		}
+
+		digest1, err := metadata1.Digest()
+		require.NoError(t, err)
+
+		digest2, err := metadata2.Digest()
+		require.NoError(t, err)
+
+		require.Equal(t, digest1, digest2,
+			"Different key order should produce identical digests")
+	})
+
+	t.Run("different metadata produces different digests", func(t *testing.T) {
+		metadata1 := WorkflowMetadata{
+			WorkflowSelector: WorkflowSelector{WorkflowID: "workflow-abc"},
+			AuthorizedKeys:   []AuthorizedKey{{KeyType: KeyTypeECDSAEVM, PublicKey: "0xkey1"}},
+		}
+
+		metadata2 := WorkflowMetadata{
+			WorkflowSelector: WorkflowSelector{WorkflowID: "workflow-xyz"},
+			AuthorizedKeys:   []AuthorizedKey{{KeyType: KeyTypeECDSAEVM, PublicKey: "0xkey1"}},
+		}
+
+		digest1, err := metadata1.Digest()
+		require.NoError(t, err)
+
+		digest2, err := metadata2.Digest()
+		require.NoError(t, err)
+
+		require.NotEqual(t, digest1, digest2,
+			"Different metadata should produce different digests")
+	})
+
+	t.Run("empty authorized keys", func(t *testing.T) {
+		metadata := WorkflowMetadata{
+			WorkflowSelector: WorkflowSelector{WorkflowID: "workflow-empty"},
+			AuthorizedKeys:   []AuthorizedKey{},
+		}
+
+		digest, err := metadata.Digest()
+		require.NoError(t, err)
+		require.NotEmpty(t, digest)
+		require.Len(t, digest, 64)
+	})
+}