refactor: run attestation-level policy evaluations in crafting layer (#1689)

Signed-off-by: Miguel Martinez <[email protected]>

Author: migmartri

app/cli/internal/action/attestation_push.go

@@ -160,6 +160,19 @@ func (action *AttestationPush) Run(ctx context.Context, attestationID string, ru
 		return nil, err
 	}
 
+	// execute policy evaluations
+	// We do not want to evaluate policies here during render since we want to do it in a separate step
+	statement, err := renderer.RenderStatement(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("rendering statement: %w", err)
+	}
+
+	// Add attestation-level policy evaluations
+	if err := crafter.EvaluateAttestationPolicies(ctx, attestationID, statement); err != nil {
+		return nil, fmt.Errorf("evaluating attestation policies: %w", err)
+	}
+
+	// render final attestation with all the evaluated policies inside
 	envelope, err := renderer.Render(ctx)
 	if err != nil {
 		return nil, err

app/cli/internal/action/attestation_status.go

@@ -141,12 +141,12 @@ func (action *AttestationStatus) Run(ctx context.Context, attestationID string,
 		}
 
 		// We do not want to evaluate policies here during render since we want to do it in a separate step
-		statement, err := renderer.RenderStatement(ctx, chainloop.WithSkipPolicyEvaluation(true))
+		statement, err := renderer.RenderStatement(ctx)
 		if err != nil {
 			return nil, fmt.Errorf("rendering statement: %w", err)
 		}
 
-		res.PolicyEvaluations, err = action.getPolicyEvaluations(ctx, c, statement)
+		res.PolicyEvaluations, err = action.getPolicyEvaluations(ctx, c, attestationID, statement)
 		if err != nil {
 			return nil, fmt.Errorf("getting policy evaluations: %w", err)
 		}
@@ -201,30 +201,26 @@ func (action *AttestationStatus) Run(ctx context.Context, attestationID string,
 }
 
 // getPolicyEvaluations retrieves both material-level and attestation-level policy evaluations
-func (action *AttestationStatus) getPolicyEvaluations(ctx context.Context, c *crafter.Crafter, statement *intoto.Statement) (map[string][]*PolicyEvaluation, error) {
+func (action *AttestationStatus) getPolicyEvaluations(ctx context.Context, c *crafter.Crafter, attestationID string, statement *intoto.Statement) (map[string][]*PolicyEvaluation, error) {
 	// grouped by material name
 	evaluations := make(map[string][]*PolicyEvaluation)
 
-	// Add material-level policy evaluations
-	for _, v := range c.CraftingState.Attestation.GetPolicyEvaluations() {
-		if existing, ok := evaluations[v.MaterialName]; ok {
-			evaluations[v.MaterialName] = append(existing, policyEvaluationStateToActionForStatus(v))
-		} else {
-			evaluations[v.MaterialName] = []*PolicyEvaluation{policyEvaluationStateToActionForStatus(v)}
-		}
-	}
-
 	// Add attestation-level policy evaluations
-	attestationEvaluations, err := c.EvaluateAttestationPolicies(ctx, statement)
-	if err != nil {
+	if err := c.EvaluateAttestationPolicies(ctx, attestationID, statement); err != nil {
 		return nil, fmt.Errorf("evaluating attestation policies: %w", err)
 	}
 
-	for _, v := range attestationEvaluations {
-		if existing, ok := evaluations[chainloop.AttPolicyEvaluation]; ok {
-			evaluations[chainloop.AttPolicyEvaluation] = append(existing, policyEvaluationStateToActionForStatus(v))
+	// map evaluations
+	for _, v := range c.CraftingState.Attestation.GetPolicyEvaluations() {
+		keyName := v.MaterialName
+		if keyName == "" {
+			keyName = chainloop.AttPolicyEvaluation
+		}
+
+		if existing, ok := evaluations[keyName]; ok {
+			evaluations[keyName] = append(existing, policyEvaluationStateToActionForStatus(v))
 		} else {
-			evaluations[chainloop.AttPolicyEvaluation] = []*PolicyEvaluation{policyEvaluationStateToActionForStatus(v)}
+			evaluations[keyName] = []*PolicyEvaluation{policyEvaluationStateToActionForStatus(v)}
 		}
 	}
 

pkg/attestation/crafter/crafter.go

@@ -593,23 +593,38 @@ func (c *Crafter) addMaterial(ctx context.Context, m *schemaapi.CraftingSchema_M
 	return nil
 }
 
-func (c *Crafter) EvaluateAttestationPolicies(ctx context.Context, statement *intoto.Statement) ([]*api.PolicyEvaluation, error) {
+// EvaluateAttestationPolicies evaluates the attestation-level policies and stores them in the attestation state
+func (c *Crafter) EvaluateAttestationPolicies(ctx context.Context, attestationID string, statement *intoto.Statement) error {
 	// evaluate attestation-level policies
 	pv := policies.NewPolicyVerifier(c.CraftingState.InputSchema, c.attClient, c.Logger)
-	policyResults, err := pv.VerifyStatement(ctx, statement)
+	policyEvaluations, err := pv.VerifyStatement(ctx, statement)
 	if err != nil {
-		return nil, fmt.Errorf("evaluating policies in statement: %w", err)
+		return fmt.Errorf("evaluating policies in statement: %w", err)
 	}
 
 	pgv := policies.NewPolicyGroupVerifier(c.CraftingState.InputSchema, c.attClient, c.Logger)
 	policyGroupResults, err := pgv.VerifyStatement(ctx, statement)
 	if err != nil {
-		return nil, fmt.Errorf("evaluating policy groups in statement: %w", err)
+		return fmt.Errorf("evaluating policy groups in statement: %w", err)
 	}
 
-	policyResults = append(policyResults, policyGroupResults...)
+	policyEvaluations = append(policyEvaluations, policyGroupResults...)
 
-	return policyResults, nil
+	// Since we are going to override the state, we want to keep the existing material-type policy evaluations
+	for _, ev := range c.CraftingState.Attestation.PolicyEvaluations {
+		// We can not use kind = ATTESTATION since that's a valid material kind
+		if ev.MaterialName != "" {
+			policyEvaluations = append(policyEvaluations, ev)
+		}
+	}
+
+	c.CraftingState.Attestation.PolicyEvaluations = policyEvaluations
+
+	if err := c.stateManager.Write(ctx, attestationID, c.CraftingState); err != nil {
+		return fmt.Errorf("failed to persist crafting state: %w", err)
+	}
+
+	return nil
 }
 
 func (c *Crafter) ValidateAttestation() error {

pkg/attestation/renderer/chainloop/v02.go

@@ -27,7 +27,6 @@ import (
 	pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
 	schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
 	v1 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1"
-	"github.com/chainloop-dev/chainloop/pkg/policies"
 	crv1 "github.com/google/go-containerregistry/pkg/v1"
 	intoto "github.com/in-toto/attestation/go/v1"
 	"github.com/rs/zerolog"
@@ -86,25 +85,7 @@ func NewChainloopRendererV02(att *v1.Attestation, schema *schemaapi.CraftingSche
 	}
 }
 
-type RenderOptions struct {
-	evaluatePolicies bool
-}
-
-type RenderOpt func(*RenderOptions)
-
-func WithSkipPolicyEvaluation(skip bool) RenderOpt {
-	return func(o *RenderOptions) {
-		o.evaluatePolicies = !skip
-	}
-}
-
-func (r *RendererV02) Statement(ctx context.Context, opts ...RenderOpt) (*intoto.Statement, error) {
-	var evaluations []*v1.PolicyEvaluation
-	options := &RenderOptions{evaluatePolicies: true}
-	for _, opt := range opts {
-		opt(options)
-	}
-
+func (r *RendererV02) Statement(_ context.Context) (*intoto.Statement, error) {
 	subject, err := r.subject()
 	if err != nil {
 		return nil, fmt.Errorf("error creating subject: %w", err)
@@ -122,88 +103,9 @@ func (r *RendererV02) Statement(ctx context.Context, opts ...RenderOpt) (*intoto
 		Predicate:     predicate,
 	}
 
-	if options.evaluatePolicies {
-		// Validate policy groups
-		pgv := policies.NewPolicyGroupVerifier(r.schema, r.attClient, r.logger)
-		policyGroupResults, err := pgv.VerifyStatement(ctx, statement)
-		if err != nil {
-			return nil, fmt.Errorf("error applying policy groups to statement: %w", err)
-		}
-		evaluations = append(evaluations, policyGroupResults...)
-
-		// validate attestation-level policies
-		pv := policies.NewPolicyVerifier(r.schema, r.attClient, r.logger)
-		policyResults, err := pv.VerifyStatement(ctx, statement)
-		if err != nil {
-			return nil, fmt.Errorf("applying policies to statement: %w", err)
-		}
-		evaluations = append(evaluations, policyResults...)
-		// log policy violations
-		policies.LogPolicyEvaluations(evaluations, r.logger)
-
-		// insert attestation level policy results into statement
-		if err = addPolicyResults(statement, evaluations); err != nil {
-			return nil, fmt.Errorf("adding policy results to statement: %w", err)
-		}
-	}
-
 	return statement, nil
 }
 
-// addPolicyResults adds policy evaluation results to the statement. It does it by deserializing the predicate from a structpb.Struct,
-// filling PolicyEvaluations, and serializing it again to a structpb.Struct object, using JSON as an intermediate representation.
-// Note that this is needed because intoto predicates are generic structpb.Struct
-func addPolicyResults(statement *intoto.Statement, policyResults []*v1.PolicyEvaluation) error {
-	if len(policyResults) == 0 {
-		return nil
-	}
-
-	predicate := statement.Predicate
-	// marshall to json
-	jsonPredicate, err := protojson.Marshal(predicate)
-	if err != nil {
-		return fmt.Errorf("marshalling predicate: %w", err)
-	}
-
-	// unmarshall to our typed predicate object
-	var p ProvenancePredicateV02
-	err = json.Unmarshal(jsonPredicate, &p)
-	if err != nil {
-		return fmt.Errorf("unmarshalling predicate: %w", err)
-	}
-
-	// insert policy evaluations for attestation
-	if p.PolicyEvaluations == nil {
-		p.PolicyEvaluations = make(map[string][]*PolicyEvaluation)
-	}
-	attEvaluations := make([]*PolicyEvaluation, 0, len(policyResults))
-	for _, ev := range policyResults {
-		renderedEv, err := renderEvaluation(ev)
-		if err != nil {
-			return fmt.Errorf("rendering evaluation: %w", err)
-		}
-		attEvaluations = append(attEvaluations, renderedEv)
-	}
-	p.PolicyEvaluations[AttPolicyEvaluation] = attEvaluations
-
-	// marshall back to JSON
-	jsonPredicate, err = json.Marshal(p)
-	if err != nil {
-		return fmt.Errorf("marshalling predicate: %w", err)
-	}
-
-	// finally unmarshal from JSON to structpb.Struct.
-	var finalPredicate structpb.Struct
-	err = protojson.Unmarshal(jsonPredicate, &finalPredicate)
-	if err != nil {
-		return fmt.Errorf("unmarshalling predicate: %w", err)
-	}
-
-	statement.Predicate = &finalPredicate
-
-	return nil
-}
-
 func commitAnnotations(c *v1.Commit) (*structpb.Struct, error) {
 	annotationsRaw := map[string]interface{}{
 		subjectGitAnnotationWhen:        c.GetDate().AsTime().Format(time.RFC3339),
@@ -285,7 +187,7 @@ func (r *RendererV02) predicate() (*structpb.Struct, error) {
 		return nil, fmt.Errorf("error normalizing materials: %w", err)
 	}
 
-	policies, err := policyEvaluationsFromMaterials(r.att)
+	policies, err := mappedPolicyEvaluations(r.att)
 	if err != nil {
 		return nil, fmt.Errorf("error rendering policy evaluations: %w", err)
 	}
@@ -313,14 +215,19 @@ func (r *RendererV02) predicate() (*structpb.Struct, error) {
 }
 
 // collect all policy evaluations grouped by material
-func policyEvaluationsFromMaterials(att *v1.Attestation) (map[string][]*PolicyEvaluation, error) {
+func mappedPolicyEvaluations(att *v1.Attestation) (map[string][]*PolicyEvaluation, error) {
 	result := map[string][]*PolicyEvaluation{}
 	for _, p := range att.GetPolicyEvaluations() {
+		keyName := p.MaterialName
+		if keyName == "" {
+			keyName = AttPolicyEvaluation
+		}
+
 		ev, err := renderEvaluation(p)
 		if err != nil {
 			return nil, err
 		}
-		result[p.MaterialName] = append(result[p.MaterialName], ev)
+		result[keyName] = append(result[keyName], ev)
 	}
 
 	return result, nil

pkg/attestation/renderer/renderer.go

@@ -54,7 +54,7 @@ type AttestationRenderer struct {
 }
 
 type r interface {
-	Statement(ctx context.Context, opts ...chainloop.RenderOpt) (*intoto.Statement, error)
+	Statement(ctx context.Context) (*intoto.Statement, error)
 }
 
 type Opt func(*AttestationRenderer)
@@ -95,8 +95,8 @@ func NewAttestationRenderer(state *crafter.VersionedCraftingState, attClient pb.
 }
 
 // Render the in-toto statement skipping validations, dsse envelope wrapping nor signing
-func (ab *AttestationRenderer) RenderStatement(ctx context.Context, opts ...chainloop.RenderOpt) (*intoto.Statement, error) {
-	statement, err := ab.renderer.Statement(ctx, opts...)
+func (ab *AttestationRenderer) RenderStatement(ctx context.Context) (*intoto.Statement, error) {
+	statement, err := ab.renderer.Statement(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("generating in-toto statement: %w", err)
 	}