diff --git a/app/cli/pkg/action/attestation_init.go b/app/cli/pkg/action/attestation_init.go index bfb9cc8d1..a15bf9ce0 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) @@ -194,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 @@ -274,12 +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()) - } - // 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 +367,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 +443,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