From a76ce6b28c45444c2d95fdb86b567ab045ef7c07 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 18 Jun 2026 16:10:41 +0200 Subject: [PATCH 1/2] fix(cli): surface policy-group materials during attestation Materials declared by a policy group attached to a contract were not shown during attestation for V2 contracts. Enrichment only mutated the V1 schema, but the crafter stores the V2 schema when present, discarding the enriched V1. Enrich the schema that the crafter actually stores (V2 when present, V1 otherwise) so policy-group materials appear in `attestation status`/`add` and are validated. When the same material is declared in both the contract and a policy group, it is now merged silently when the definitions are compatible, warning only on a genuine type conflict. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: db6acef0-3453-4c3f-ae8d-803dd2a677e7 --- app/cli/pkg/action/attestation_init.go | 88 ++++++++++++++++----- app/cli/pkg/action/attestation_init_test.go | 79 +++++++++++++++++- 2 files changed, 147 insertions(+), 20 deletions(-) diff --git a/app/cli/pkg/action/attestation_init.go b/app/cli/pkg/action/attestation_init.go index bfb9cc8d1..1a901239b 100644 --- a/app/cli/pkg/action/attestation_init.go +++ b/app/cli/pkg/action/attestation_init.go @@ -181,12 +181,6 @@ func (action *AttestationInit) Run(ctx context.Context, opts *AttestationInitRun action.Logger.Debug().Msg("workflow contract and metadata retrieved from the control plane") - // 3. enrich contract with group materials and policies - err = enrichContractMaterials(ctx, contractVersion.GetV1(), client, &action.Logger) - if err != nil { - return "", fmt.Errorf("failed to apply materials from policy groups: %w", err) - } - // Auto discover the runner context and enforce against the one in the contract if needed // nolint:staticcheck discoveredRunner, err := crafter.DiscoverAndEnforceRunner(contractVersion.GetV1().GetRunner().GetType(), action.dryRun, action.AuthTokenRaw, action.Logger) @@ -280,6 +274,20 @@ func (action *AttestationInit) Run(ctx context.Context, opts *AttestationInitRun schemaV2 = parseContractV2(contractVersion.GetRawContract()) } + // Enrich the contract with the materials declared by its attached policy + // groups, so they show up during attestation. See issue #3222. + // Only the schema that the crafter will actually store needs enriching: it + // prefers the V2 schema when present and falls back to V1 otherwise. + if schemaV2 != nil { + err = enrichContractMaterialsV2(ctx, schemaV2, client, &action.Logger) + } else { + //nolint:staticcheck // TODO: Migrate to new contract version API + err = enrichContractMaterials(ctx, contractVersion.GetV1(), client, &action.Logger) + } + if err != nil { + return "", fmt.Errorf("failed to apply materials from policy groups: %w", err) + } + // Initialize the local attestation crafter // NOTE: important to run this initialization here since workflowMeta is populated // with the workflowRunId that comes from the control plane @@ -357,28 +365,61 @@ func (action *AttestationInit) Run(ctx context.Context, opts *AttestationInitRun return attestationID, nil } +// enrichContractMaterials augments a V1 contract schema with the materials +// declared by its attached policy groups. func enrichContractMaterials(ctx context.Context, schema *v1.CraftingSchema, client pb.AttestationServiceClient, logger *zerolog.Logger) error { - contractMaterials := schema.GetMaterials() - for _, pgAtt := range schema.GetPolicyGroups() { + merged, err := mergePolicyGroupMaterials(ctx, schema.GetPolicyGroups(), schema.GetMaterials(), client, logger) + if err != nil { + return err + } + + schema.Materials = merged + + return nil +} + +// enrichContractMaterialsV2 augments a V2 contract schema with the materials +// declared by its attached policy groups. The V2 schema is the one stored in +// the crafting state (and thus surfaced during `attestation status`/`add`) +// whenever it is present, so it must be enriched too. See issue #3222. +func enrichContractMaterialsV2(ctx context.Context, schema *v1.CraftingSchemaV2, client pb.AttestationServiceClient, logger *zerolog.Logger) error { + spec := schema.GetSpec() + if spec == nil { + return nil + } + + merged, err := mergePolicyGroupMaterials(ctx, spec.GetPolicyGroups(), spec.GetMaterials(), client, logger) + if err != nil { + return err + } + + spec.Materials = merged + + return nil +} + +// mergePolicyGroupMaterials returns the contract materials augmented with the +// materials contributed by the attached policy groups. Materials already +// declared in the contract take precedence and are not duplicated. +func mergePolicyGroupMaterials(ctx context.Context, policyGroups []*v1.PolicyGroupAttachment, materials []*v1.CraftingSchema_Material, client pb.AttestationServiceClient, logger *zerolog.Logger) ([]*v1.CraftingSchema_Material, error) { + for _, pgAtt := range policyGroups { group, _, err := policies.LoadPolicyGroup(ctx, pgAtt, &policies.LoadPolicyGroupOptions{ Client: client, Logger: logger, }) if err != nil { - return fmt.Errorf("failed to load policy group: %w", err) + return nil, fmt.Errorf("failed to load policy group: %w", err) } logger.Debug().Msgf("adding materials from policy group '%s'", group.GetMetadata().GetName()) - toAdd, err := getGroupMaterialsToAdd(group, pgAtt, contractMaterials, logger) + toAdd, err := getGroupMaterialsToAdd(group, pgAtt, materials, logger) if err != nil { - return err + return nil, err } - contractMaterials = append(contractMaterials, toAdd...) + materials = append(materials, toAdd...) } - schema.Materials = contractMaterials - - return nil + return materials, nil } // merge existing materials with group ones, taking the contract's one in case of conflict @@ -400,13 +441,22 @@ func getGroupMaterialsToAdd(group *v1.PolicyGroup, pgAtt *v1.PolicyGroupAttachme continue } - // check if material already exists in the contract and skip it in that case + // If the material is already declared in the contract, keep the contract + // definition and don't add a duplicate. Declaring the same material in + // both the contract and a policy group is a legitimate, expected case, so + // it's merged silently when the definitions are compatible; we only warn + // when the types genuinely conflict. ignore := false for _, mat := range fromContract { - if mat.GetName() == csm.GetName() { - logger.Warn().Msgf("material '%s' from policy group '%s' is also present in the contract and will be ignored", mat.GetName(), group.GetMetadata().GetName()) - ignore = true + if mat.GetName() != csm.GetName() { + continue + } + ignore = true + if mat.GetType() != csm.GetType() { + logger.Warn().Msgf("material '%s' is declared in both the contract (%s) and policy group '%s' (%s); using the contract definition", + mat.GetName(), mat.GetType(), group.GetMetadata().GetName(), csm.GetType()) } + break } if !ignore { toAdd = append(toAdd, csm) diff --git a/app/cli/pkg/action/attestation_init_test.go b/app/cli/pkg/action/attestation_init_test.go index 7f9297ef4..898a9ec4c 100644 --- a/app/cli/pkg/action/attestation_init_test.go +++ b/app/cli/pkg/action/attestation_init_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-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. @@ -23,6 +23,7 @@ import ( pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + craftingv1 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -107,6 +108,82 @@ func TestEnrichMaterials(t *testing.T) { } } +// TestEnrichMaterialsV2Schema reproduces issue #3222. +// +// When the contract is in V2 format, materials contributed by an attached +// policy group must end up in the schema that is actually stored in the +// crafting state (the V2 schema), so they are surfaced during +// `attestation status`/`add` and validated as expected. +// +// The crafter stores the V2 schema whenever it is present (see crafter.go: +// SchemaV2 wins over SchemaV1), so the policy-group materials must reach the V2 +// schema to show up in `CraftingState.GetMaterials()`, which is what the status +// command displays. +func TestEnrichMaterialsV2Schema(t *testing.T) { + l := zerolog.Nop() + + // policy_group.yaml contributes the "sbom" and "container" materials. + newV2Schema := func(materials []*v1.CraftingSchema_Material) *v1.CraftingSchemaV2 { + return &v1.CraftingSchemaV2{ + ApiVersion: "chainloop.dev/v1", + Kind: "Contract", + Metadata: &v1.Metadata{Name: "test-contract-v2"}, + Spec: &v1.CraftingSchemaV2Spec{ + Materials: materials, + Runner: &v1.CraftingSchema_Runner{Type: v1.CraftingSchema_Runner_GITHUB_ACTION}, + PolicyGroups: []*v1.PolicyGroupAttachment{ + {Ref: "file://testdata/policy_group.yaml"}, + }, + }, + } + } + + cases := []struct { + name string + materials []*v1.CraftingSchema_Material + wantMaterials []string + }{ + { + name: "policy group materials are surfaced in the stored V2 schema", + materials: []*v1.CraftingSchema_Material{ + {Type: v1.CraftingSchema_Material_ARTIFACT, Name: "rootfs"}, + }, + wantMaterials: []string{"rootfs", "sbom", "container"}, + }, + { + name: "material present in both contract and policy group is merged once", + materials: []*v1.CraftingSchema_Material{ + {Type: v1.CraftingSchema_Material_SBOM_SPDX_JSON, Name: "sbom"}, + }, + wantMaterials: []string{"sbom", "container"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + schemaV2 := newV2Schema(tc.materials) + + // Mirror what Initialize does: enrich the V2 schema, which is the one + // stored in the crafting state. + require.NoError(t, enrichContractMaterialsV2(context.TODO(), schemaV2, nil, &l)) + + // The crafter stores the V2 schema (it takes precedence when present), + // and `attestation status` reads expected materials from the stored + // schema via CraftingState.GetMaterials(). + state := &craftingv1.CraftingState{ + Schema: &craftingv1.CraftingState_SchemaV2{SchemaV2: schemaV2}, + } + + gotNames := make([]string, 0, len(state.GetMaterials())) + for _, m := range state.GetMaterials() { + gotNames = append(gotNames, m.GetName()) + } + + assert.ElementsMatch(t, tc.wantMaterials, gotNames) + }) + } +} + func TestTemplatedGroups(t *testing.T) { cases := []struct { name string From 8b9b7e751e7573c244069d2981eb4077fd3bda36 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 18 Jun 2026 16:23:05 +0200 Subject: [PATCH 2/2] fix(cli): enrich contract materials before control-plane init Move policy-group material enrichment back ahead of the control-plane Init call so a policy-group load failure fails fast, before a workflow run is created, avoiding orphaned/partially initialized attestations. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: db6acef0-3453-4c3f-ae8d-803dd2a677e7 --- app/cli/pkg/action/attestation_init.go | 42 ++++++++++++++------------ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/app/cli/pkg/action/attestation_init.go b/app/cli/pkg/action/attestation_init.go index 1a901239b..a15bf9ce0 100644 --- a/app/cli/pkg/action/attestation_init.go +++ b/app/cli/pkg/action/attestation_init.go @@ -188,6 +188,28 @@ func (action *AttestationInit) Run(ctx context.Context, opts *AttestationInitRun return "", ErrRunnerContextNotFound{err.Error()} } + // Parse the raw contract to get V2 schema if available + var schemaV2 *v1.CraftingSchemaV2 + if contractVersion.GetRawContract() != nil { + schemaV2 = parseContractV2(contractVersion.GetRawContract()) + } + + // Enrich the contract with the materials declared by its attached policy + // groups, so they show up during attestation. See issue #3222. + // Only the schema that the crafter will actually store needs enriching: it + // prefers the V2 schema when present and falls back to V1 otherwise. + // Done before the control-plane Init below so a policy-group load failure + // fails fast, before a workflow run is created. + if schemaV2 != nil { + err = enrichContractMaterialsV2(ctx, schemaV2, client, &action.Logger) + } else { + //nolint:staticcheck // TODO: Migrate to new contract version API + err = enrichContractMaterials(ctx, contractVersion.GetV1(), client, &action.Logger) + } + if err != nil { + return "", fmt.Errorf("failed to apply materials from policy groups: %w", err) + } + var ( // Identifier of this attestation instance attestationID string @@ -268,26 +290,6 @@ func (action *AttestationInit) Run(ctx context.Context, opts *AttestationInitRun } } - // Parse the raw contract to get V2 schema if available - var schemaV2 *v1.CraftingSchemaV2 - if contractVersion.GetRawContract() != nil { - schemaV2 = parseContractV2(contractVersion.GetRawContract()) - } - - // Enrich the contract with the materials declared by its attached policy - // groups, so they show up during attestation. See issue #3222. - // Only the schema that the crafter will actually store needs enriching: it - // prefers the V2 schema when present and falls back to V1 otherwise. - if schemaV2 != nil { - err = enrichContractMaterialsV2(ctx, schemaV2, client, &action.Logger) - } else { - //nolint:staticcheck // TODO: Migrate to new contract version API - err = enrichContractMaterials(ctx, contractVersion.GetV1(), client, &action.Logger) - } - if err != nil { - return "", fmt.Errorf("failed to apply materials from policy groups: %w", err) - } - // Initialize the local attestation crafter // NOTE: important to run this initialization here since workflowMeta is populated // with the workflowRunId that comes from the control plane