fix(attestation): deduplicate policy evaluations after protojson round-trip

Replace reflect.DeepEqual with maps.Equal for policy evaluation dedup
so nil and empty maps are treated as equivalent. protojson.Marshal omits
empty maps, causing deserialized With fields to be nil instead of empty,
which made the dedup comparison fail and duplicate evaluations appear.

Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>

Author: jiparis

pkg/attestation/crafter/crafter.go

@@ -19,9 +19,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
 	"net/url"
 	"os"
-	"reflect"
 	"slices"
 	"strings"
 	"time"
@@ -752,6 +752,13 @@ func (c *Crafter) addMaterial(ctx context.Context, m *schemaapi.CraftingSchema_M
 	return mt, nil
 }
 
+// policyEvalMatches returns true if two policy evaluations refer to the same policy
+// with the same arguments. It treats nil and empty maps as equivalent to handle
+// protojson round-trip serialization where empty maps are omitted.
+func policyEvalMatches(a, b *api.PolicyEvaluation) bool {
+	return proto.Equal(a.GetPolicyReference(), b.GetPolicyReference()) && maps.Equal(a.GetWith(), b.GetWith())
+}
+
 // EvaluateAttestationPolicies evaluates the attestation-level policies and stores them in the attestation state.
 // The phase parameter controls which policies are evaluated based on their attestation_phases spec field.
 func (c *Crafter) EvaluateAttestationPolicies(ctx context.Context, attestationID string, statement *intoto.Statement, phase policies.EvalPhase) error {
@@ -783,7 +790,7 @@ func (c *Crafter) EvaluateAttestationPolicies(ctx context.Context, attestationID
 	for _, ev := range policyEvaluations {
 		var duplicated bool
 		for _, existing := range filteredPolicyEvaluations {
-			if proto.Equal(existing.PolicyReference, ev.PolicyReference) && reflect.DeepEqual(existing.With, ev.With) {
+			if policyEvalMatches(existing, ev) {
 				duplicated = true
 				break
 			}
@@ -808,7 +815,7 @@ func (c *Crafter) EvaluateAttestationPolicies(ctx context.Context, attestationID
 		// Check if this attestation-level evaluation was re-evaluated in the current phase
 		var reEvaluated bool
 		for _, newEv := range policyEvaluations {
-			if proto.Equal(newEv.PolicyReference, ev.PolicyReference) && reflect.DeepEqual(newEv.With, ev.With) {
+			if policyEvalMatches(newEv, ev) {
 				reEvaluated = true
 				break
 			}

pkg/attestation/crafter/crafter_unit_test.go

@@ -1,5 +1,5 @@
 //
-// Copyright 2023 The Chainloop Authors.
+// Copyright 2023-2026 The Chainloop Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -21,12 +21,12 @@ import (
 	"testing"
 	"time"
 
+	api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1"
 	"github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"github.com/go-git/go-git/v5/config"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -214,6 +214,106 @@ func (s *crafterUnitSuite) TestGitRepoHead() {
 	}
 }
 
+func (s *crafterUnitSuite) TestPolicyEvaluationDedup() {
+	// Simulate the protojson round-trip issue:
+	// - Init phase sets With = map[string]string{} (empty map)
+	// - protojson.Marshal omits empty maps
+	// - protojson.Unmarshal sets With = nil (absent field)
+	// - Push phase produces With = map[string]string{} again
+	// The dedup comparison must treat nil and empty map as equal.
+
+	policyRef := &api.PolicyEvaluation_Reference{
+		Name:   "source-commit",
+		Digest: "sha256:abc123",
+	}
+
+	testCases := []struct {
+		name        string
+		existing    []*api.PolicyEvaluation
+		newEvals    []*api.PolicyEvaluation
+		wantCount   int
+		description string
+	}{
+		{
+			name: "nil vs empty map With are deduplicated",
+			existing: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: nil},
+			},
+			newEvals: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: map[string]string{}},
+			},
+			wantCount:   1,
+			description: "after protojson round-trip, nil With should match empty map With",
+		},
+		{
+			name: "empty map vs empty map With are deduplicated",
+			existing: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: map[string]string{}},
+			},
+			newEvals: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: map[string]string{}},
+			},
+			wantCount:   1,
+			description: "identical empty maps should deduplicate",
+		},
+		{
+			name: "nil vs nil With are deduplicated",
+			existing: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: nil},
+			},
+			newEvals: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: nil},
+			},
+			wantCount:   1,
+			description: "both nil should deduplicate",
+		},
+		{
+			name: "different With args are not deduplicated",
+			existing: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: map[string]string{"key": "val1"}},
+			},
+			newEvals: []*api.PolicyEvaluation{
+				{Name: "source-commit", PolicyReference: policyRef, With: map[string]string{"key": "val2"}},
+			},
+			wantCount:   2,
+			description: "different With values should not deduplicate",
+		},
+		{
+			name: "different policy references are not deduplicated",
+			existing: []*api.PolicyEvaluation{
+				{Name: "policy-a", PolicyReference: &api.PolicyEvaluation_Reference{Name: "policy-a"}, With: nil},
+			},
+			newEvals: []*api.PolicyEvaluation{
+				{Name: "policy-b", PolicyReference: &api.PolicyEvaluation_Reference{Name: "policy-b"}, With: nil},
+			},
+			wantCount:   2,
+			description: "different policies should both be kept",
+		},
+	}
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			all := append(tc.existing, tc.newEvals...)
+
+			var filtered []*api.PolicyEvaluation
+			for _, ev := range all {
+				var duplicated bool
+				for _, existing := range filtered {
+					if policyEvalMatches(existing, ev) {
+						duplicated = true
+						break
+					}
+				}
+				if !duplicated {
+					filtered = append(filtered, ev)
+				}
+			}
+
+			s.Len(filtered, tc.wantCount, tc.description)
+		})
+	}
+}
+
 func TestSuite(t *testing.T) {
 	suite.Run(t, new(crafterUnitSuite))
 }