From ea3c9214f96a70ff794a4014ffa26fa615f3367d Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 22 Sep 2026 17:26:08 +0200 Subject: [PATCH 1/2] feat(controlplane): enforce workflow contracts when attestations are stored The workflow contract was only enforced by the CLI. crafter.ValidateAttestation had no caller in the control plane, so Store accepted any well-formed, signed attestation regardless of whether it satisfied the contract revision pinned on the run. The bundle is signed by the same client that decides whether to run that check, so a valid signature says nothing about contract compliance. SaveAttestation now loads the contract revision pinned on the workflow run at init time and rejects an attestation whose required materials are missing, before anything is persisted. Choke groups are honoured, and materials the contract does not declare remain allowed. The material presence check is extracted into a validator shared by the CLI and the control plane so both enforce identical semantics. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 513023ae-e4c2-4c52-ba7b-5ef838eda385 --- app/controlplane/cmd/wire_gen.go | 1 + .../internal/service/service_test.go | 10 + .../pkg/biz/testhelpers/wire_gen.go | 1 + app/controlplane/pkg/biz/workflowrun.go | 60 ++++- .../workflowrun_contract_integration_test.go | 231 ++++++++++++++++++ .../v1/crafting_state_validations.go | 28 ++- .../v1/crafting_state_validations_test.go | 111 +++++++++ .../chainloop/materials_roundtrip_test.go | 147 +++++++++++ 8 files changed, 581 insertions(+), 8 deletions(-) create mode 100644 app/controlplane/pkg/biz/workflowrun_contract_integration_test.go create mode 100644 pkg/attestation/renderer/chainloop/materials_roundtrip_test.go diff --git a/app/controlplane/cmd/wire_gen.go b/app/controlplane/cmd/wire_gen.go index a06ddc2e2..c91f4a107 100644 --- a/app/controlplane/cmd/wire_gen.go +++ b/app/controlplane/cmd/wire_gen.go @@ -207,6 +207,7 @@ func wireApp(contextContext context.Context, bootstrap *conf.Bootstrap, readerWr WfrRepo: workflowRunRepo, WfRepo: workflowRepo, OrgRepo: organizationRepo, + ContractRepo: workflowContractRepo, SigningUC: signingUseCase, AuditorUC: auditorUseCase, Logger: logger, diff --git a/app/controlplane/internal/service/service_test.go b/app/controlplane/internal/service/service_test.go index 7c4a73087..0a8e3ec9d 100644 --- a/app/controlplane/internal/service/service_test.go +++ b/app/controlplane/internal/service/service_test.go @@ -96,6 +96,16 @@ func TestHandleUseCaseErr(t *testing.T) { wantCode: codes.InvalidArgument, wantMessage: "validation error: invalid input", }, + { + // An attestation rejected for not satisfying its contract must reach + // the CI job as a client error naming what was missing, not as a + // masked 500 the operator cannot act on. + name: "contract violation reaches the client as a bad request", + err: biz.NewErrValidation(fmt.Errorf("attestation does not satisfy contract revision 4: %w", + errors.New("some materials have not been crafted yet: sbom, sarif"))), + wantCode: codes.InvalidArgument, + wantMessage: "validation error: attestation does not satisfy contract revision 4: some materials have not been crafted yet: sbom, sarif", + }, { name: "unknown error is masked as internal server error", err: errors.New("sensitive details"), diff --git a/app/controlplane/pkg/biz/testhelpers/wire_gen.go b/app/controlplane/pkg/biz/testhelpers/wire_gen.go index c80e591de..447537e40 100644 --- a/app/controlplane/pkg/biz/testhelpers/wire_gen.go +++ b/app/controlplane/pkg/biz/testhelpers/wire_gen.go @@ -107,6 +107,7 @@ func WireTestData(contextContext context.Context, testDatabase *TestDatabase, t WfrRepo: workflowRunRepo, WfRepo: workflowRepo, OrgRepo: organizationRepo, + ContractRepo: workflowContractRepo, SigningUC: signingUseCase, AuditorUC: auditorUseCase, Logger: logger, diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index 619189151..467537e49 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/app/controlplane/pkg/biz/workflowrun.go @@ -28,6 +28,7 @@ import ( "github.com/chainloop-dev/chainloop/app/controlplane/pkg/auditor/events" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/pagination" "github.com/chainloop-dev/chainloop/pkg/attestation" + attestationapi "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/renderer/chainloop" "github.com/chainloop-dev/chainloop/pkg/attestation/verifier" "github.com/chainloop-dev/chainloop/pkg/cache/attestationbundle" @@ -123,11 +124,12 @@ type WorkflowRunRepo interface { } type WorkflowRunUseCase struct { - wfRunRepo WorkflowRunRepo - wfRepo WorkflowRepo - orgRepo OrganizationRepo - logger *log.Helper - auditorUC *AuditorUseCase + wfRunRepo WorkflowRunRepo + wfRepo WorkflowRepo + orgRepo OrganizationRepo + contractRepo WorkflowContractRepo + logger *log.Helper + auditorUC *AuditorUseCase signingUseCase *SigningUseCase bundleCache *attestationbundle.Cache @@ -139,6 +141,7 @@ type WorkflowRunUseCaseOpts struct { WfrRepo WorkflowRunRepo WfRepo WorkflowRepo OrgRepo OrganizationRepo + ContractRepo WorkflowContractRepo SigningUC *SigningUseCase AuditorUC *AuditorUseCase Logger log.Logger @@ -157,6 +160,7 @@ func NewWorkflowRunUseCase(opts *WorkflowRunUseCaseOpts) (*WorkflowRunUseCase, e wfRunRepo: opts.WfrRepo, wfRepo: opts.WfRepo, orgRepo: opts.OrgRepo, + contractRepo: opts.ContractRepo, auditorUC: opts.AuditorUC, signingUseCase: opts.SigningUC, logger: log.NewHelper(logger), @@ -413,6 +417,48 @@ func (uc *WorkflowRunUseCase) orgBlocksReleasedVersions(ctx context.Context, run return org.BlockAttestationsOnReleasedVersions, nil } +// validateAgainstContract rejects an attestation that does not satisfy the +// contract revision pinned on the workflow run when it was initialized. +// +// The CLI runs the same check before pushing, but it signs the bundle with the +// same client that decided whether to run it, so a valid signature says nothing +// about contract compliance. The control plane is the authority here. +func (uc *WorkflowRunUseCase) validateAgainstContract(ctx context.Context, run *WorkflowRun, predicate chainloop.NormalizablePredicate) error { + // Every run created through the attestation init endpoint pins a contract + // revision, so a run without one is an integrity problem rather than a + // reason to skip the check. + if run.ContractVersionID == uuid.Nil { + return NewErrValidation(errors.New("workflow run has no contract revision associated")) + } + + contract, err := uc.contractRepo.FindVersionByID(ctx, run.ContractVersionID) + if err != nil { + return fmt.Errorf("finding contract version: %w", err) + } else if contract == nil || contract.Version == nil || contract.Version.Schema == nil { + return NewErrNotFound("contract version") + } + + // Schema is the v1 form of the contract and is populated for v2 contracts + // too, so this covers both contract formats. A revision we cannot read is an + // error rather than an empty contract: treating it as "nothing required" + // would silently wave the attestation through. + schema := contract.Version.Schema.Schema + if schema == nil { + return NewErrValidation(fmt.Errorf("contract revision %d could not be read", run.ContractRevisionUsed)) + } + + craftedNames := make(map[string]struct{}, len(predicate.GetMaterials())) + for _, m := range predicate.GetMaterials() { + craftedNames[m.Name] = struct{}{} + } + + if err := attestationapi.ValidateMaterialsPresence(schema.GetMaterials(), craftedNames); err != nil { + return NewErrValidation(fmt.Errorf("attestation does not satisfy contract revision %d: %w", run.ContractRevisionUsed, err)) + } + + return nil +} + func (uc *WorkflowRunUseCase) SaveAttestation(ctx context.Context, id string, bundle []byte, opts ...SaveAttestationOption) (*v1.Hash, error) { ctx, span := otelx.Start(ctx, workflowRunTracer, "WorkflowRunUseCase.SaveAttestation") defer span.End() @@ -499,6 +545,10 @@ func (uc *WorkflowRunUseCase) SaveAttestation(ctx context.Context, id string, bu } } + if err := uc.validateAgainstContract(ctx, run, predicate); err != nil { + return nil, err + } + if options.skipBundlePersistence { if err := uc.wfRunRepo.SaveAttestationDigest(ctx, runID, digest.String(), blockReleasedVersions); err != nil { return nil, fmt.Errorf("saving attestation digest: %w", err) diff --git a/app/controlplane/pkg/biz/workflowrun_contract_integration_test.go b/app/controlplane/pkg/biz/workflowrun_contract_integration_test.go new file mode 100644 index 000000000..1ce929196 --- /dev/null +++ b/app/controlplane/pkg/biz/workflowrun_contract_integration_test.go @@ -0,0 +1,231 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package biz_test + +import ( + "context" + "fmt" + "testing" + + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/testhelpers" + "github.com/chainloop-dev/chainloop/pkg/credentials" + creds "github.com/chainloop-dev/chainloop/pkg/credentials/mocks" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +// The testdata attestations carry these contract material names: +// +// full.json -> image, skynet-sbom, skynet2-sbom +// empty.json -> (none) +// with-string.json -> build-ref +const ( + attestationFull = "testdata/attestations/full.json" + attestationEmpty = "testdata/attestations/empty.json" +) + +// The contract is what the organization agreed the run had to produce. It has +// always been checked by the CLI before pushing, but the CLI also signs the +// bundle, so a client that skips the check still produces a valid attestation. +// These tests pin the control plane down as the authority. +type workflowRunContractIntegrationTestSuite struct { + testhelpers.UseCasesEachTestSuite + org *biz.Organization + casBackend *biz.CASBackend +} + +func (s *workflowRunContractIntegrationTestSuite) SetupTest() { + credsWriter := creds.NewReaderWriter(s.T()) + credsWriter.On("SaveCredentials", mock.Anything, mock.Anything, &credentials.OCIKeypair{Repo: "repo", Username: "username", Password: "pass"}). + Return("stored-OCI-secret", nil).Maybe() + + s.TestingUseCases = testhelpers.NewTestingUseCases(s.T(), testhelpers.WithCredsReaderWriter(credsWriter)) + + ctx := context.Background() + var err error + s.org, err = s.Organization.Create(ctx, "contract-enforcement-org") + require.NoError(s.T(), err) + + s.casBackend, err = s.CASBackend.CreateOrUpdate(ctx, s.org.ID, "repo", "username", "pass", backendType, true) + require.NoError(s.T(), err) +} + +// newRunWithContract creates a workflow bound to a contract carrying the given +// materials block and returns a run pinned to that contract revision. +func (s *workflowRunContractIntegrationTestSuite) newRunWithContract(name, materialsYAML string) *biz.WorkflowRun { + s.T().Helper() + ctx := context.Background() + + rawContract := fmt.Sprintf(`apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: %s +spec: +%s`, name, materialsYAML) + + contract, err := s.WorkflowContract.Create(ctx, &biz.WorkflowContractCreateOpts{ + OrgID: s.org.ID, Name: name, RawSchema: []byte(rawContract), + }) + require.NoError(s.T(), err) + + wf, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{ + Name: name, OrgID: s.org.ID, Project: "test-project", ContractID: contract.ID.String(), + }) + require.NoError(s.T(), err) + + contractVersion, err := s.WorkflowContract.Describe(ctx, s.org.ID, wf.ContractID.String(), 0) + require.NoError(s.T(), err) + + run, err := s.WorkflowRun.Create(ctx, &biz.WorkflowRunCreateOpts{ + WorkflowID: wf.ID.String(), ContractRevision: contractVersion, CASBackendID: s.casBackend.ID, + }) + require.NoError(s.T(), err) + + return run +} + +func (s *workflowRunContractIntegrationTestSuite) TestSaveAttestationEnforcesContract() { + testCases := []struct { + name string + // the spec's materials block, already indented under spec: + materials string + attestation string + wantErr string + }{ + { + name: "contract with no materials accepts any attestation", + materials: " materials: []", + attestation: attestationFull, + }, + { + name: "every required material present", + materials: ` materials: + - type: CONTAINER_IMAGE + name: image + - type: SBOM_CYCLONEDX_JSON + name: skynet-sbom`, + attestation: attestationFull, + }, + { + name: "required material missing is rejected", + materials: ` materials: + - type: CONTAINER_IMAGE + name: image + - type: SARIF + name: static-analysis`, + attestation: attestationFull, + wantErr: "some materials have not been crafted yet: static-analysis", + }, + { + name: "attestation with no materials at all is rejected", + materials: ` materials: + - type: CONTAINER_IMAGE + name: image`, + attestation: attestationEmpty, + wantErr: "some materials have not been crafted yet: image", + }, + { + name: "optional material missing is accepted", + materials: ` materials: + - type: SARIF + name: static-analysis + optional: true`, + attestation: attestationEmpty, + }, + { + name: "choke group satisfied by one member", + materials: ` materials: + - type: SBOM_SPDX_JSON + name: spdx-sbom + group: sbom + - type: SBOM_CYCLONEDX_JSON + name: skynet-sbom + group: sbom`, + attestation: attestationFull, + }, + { + name: "choke group with no member crafted is rejected", + materials: ` materials: + - type: SBOM_SPDX_JSON + name: spdx-sbom + group: sbom + - type: SBOM_CYCLONEDX_JSON + name: cyclonedx-sbom + group: sbom`, + attestation: attestationFull, + wantErr: `at least one material from group "sbom" is required: spdx-sbom, cyclonedx-sbom`, + }, + { + // Materials the contract never declared are legitimate: runtime + // evidence and exploded archives both produce undeclared names. + name: "materials beyond the contract are accepted", + materials: ` materials: + - type: CONTAINER_IMAGE + name: image`, + attestation: attestationFull, + }, + } + + for i, tc := range testCases { + s.Run(tc.name, func() { + ctx := context.Background() + run := s.newRunWithContract(fmt.Sprintf("contract-%d", i), tc.materials) + bundleBytes := testhelpers.BundleBytesFromEnvelope(s.T(), tc.attestation) + + digest, err := s.WorkflowRun.SaveAttestation(ctx, run.ID.String(), bundleBytes) + + if tc.wantErr == "" { + require.NoError(s.T(), err) + require.NotNil(s.T(), digest) + return + } + + require.Error(s.T(), err) + s.True(biz.IsErrValidation(err), "expected a validation error, got %T: %v", err, err) + s.ErrorContains(err, tc.wantErr) + // The revision the run was pinned to is named so the operator knows + // which contract the attestation was measured against. + s.ErrorContains(err, fmt.Sprintf("contract revision %d", run.ContractRevisionUsed)) + }) + } +} + +// A rejected attestation must leave no trace: the evidence is not persisted and +// the run stays without an attestation rather than half-recording one. +func (s *workflowRunContractIntegrationTestSuite) TestRejectedAttestationIsNotPersisted() { + ctx := context.Background() + run := s.newRunWithContract("atomic-contract", ` materials: + - type: SARIF + name: static-analysis`) + + bundleBytes := testhelpers.BundleBytesFromEnvelope(s.T(), attestationFull) + _, err := s.WorkflowRun.SaveAttestation(ctx, run.ID.String(), bundleBytes) + require.Error(s.T(), err) + + stored, err := s.WorkflowRun.GetByIDInOrg(ctx, s.org.ID, run.ID.String()) + require.NoError(s.T(), err) + // The run always carries an Attestation holder; what matters is that it was + // left empty, so neither the digest nor the bundle bytes were recorded. + s.Empty(stored.Attestation.Digest, "a rejected attestation must not record a digest on the run") + s.Nil(stored.Attestation.Bundle, "a rejected attestation must not persist its bundle") + s.Nil(stored.Attestation.Envelope, "a rejected attestation must not persist its envelope") +} + +func TestWorkflowRunContractEnforcement(t *testing.T) { + suite.Run(t, new(workflowRunContractIntegrationTestSuite)) +} diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go index 9cf22a389..62cbb0098 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go @@ -21,6 +21,8 @@ import ( "strings" "buf.build/go/protovalidate" + + v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" ) // Custom validations @@ -38,9 +40,29 @@ func (state *CraftingState) ValidateComplete(dryRun bool) error { // Semantic errors // It has values for all the defined, non optional materials - var missing []string - expectedMaterials := state.GetMaterials() craftedMaterials := state.GetAttestation().GetMaterials() + craftedNames := make(map[string]struct{}, len(craftedMaterials)) + for name := range craftedMaterials { + craftedNames[name] = struct{}{} + } + + return ValidateMaterialsPresence(state.GetMaterials(), craftedNames) +} + +// ValidateMaterialsPresence checks that every material the contract requires is +// present in craftedNames. +// +// Materials sharing the same non-empty group form an "at least one of" set, so +// the group is satisfied as soon as one member has been crafted. Materials that +// the contract does not declare are allowed and ignored: runtime-added evidence +// and exploded archives (which emit "-1", "-archive", ...) both +// legitimately produce names the contract never mentions. +// +// It is shared by the CLI, which runs it before pushing, and by the control +// plane, which runs it against the contract revision pinned on the workflow run +// at init time. +func ValidateMaterialsPresence(expectedMaterials []*v1.CraftingSchema_Material, craftedNames map[string]struct{}) error { + var missing []string // Choke groups: materials sharing the same non-empty group form an // "at least one of" set. We track the members of each group and whether @@ -52,7 +74,7 @@ func (state *CraftingState) ValidateComplete(dryRun bool) error { // Iterate on the expected materials for _, m := range expectedMaterials { - _, crafted := craftedMaterials[m.Name] + _, crafted := craftedNames[m.Name] // Grouped materials are enforced at the group level, not individually. if m.Group != "" { diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go index 3b029bfde..17e0ba0a6 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go @@ -166,6 +166,117 @@ func TestCraftingStateValidateComplete(t *testing.T) { } } +func TestValidateMaterialsPresence(t *testing.T) { + art := workflowcontract.CraftingSchema_Material_ARTIFACT + + craftedSet := func(names ...string) map[string]struct{} { + out := make(map[string]struct{}, len(names)) + for _, n := range names { + out[n] = struct{}{} + } + return out + } + + testCases := []struct { + name string + expected []*workflowcontract.CraftingSchema_Material + crafted map[string]struct{} + wantErr string + }{ + { + name: "nil contract and nil crafted set", + expected: nil, + crafted: nil, + }, + { + name: "contract with no materials accepts anything", + expected: []*workflowcontract.CraftingSchema_Material{}, + crafted: craftedSet("something", "else"), + }, + { + name: "all required present", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "sbom"}, + {Type: art, Name: "sarif"}, + }, + crafted: craftedSet("sbom", "sarif"), + }, + { + name: "single required missing", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "sbom"}, + }, + crafted: craftedSet(), + wantErr: "some materials have not been crafted yet: sbom", + }, + { + name: "several missing are reported in contract order", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "sbom"}, + {Type: art, Name: "present"}, + {Type: art, Name: "sarif"}, + }, + crafted: craftedSet("present"), + wantErr: "some materials have not been crafted yet: sbom, sarif", + }, + { + name: "optional missing is allowed", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "sbom", Optional: true}, + }, + crafted: craftedSet(), + }, + { + name: "choke group satisfied by one member", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "cyclonedx", Group: "sbom"}, + {Type: art, Name: "spdx", Group: "sbom"}, + }, + crafted: craftedSet("spdx"), + }, + { + name: "choke group unsatisfied lists every member", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "cyclonedx", Group: "sbom"}, + {Type: art, Name: "spdx", Group: "sbom"}, + }, + crafted: craftedSet(), + wantErr: `at least one material from group "sbom" is required: cyclonedx, spdx`, + }, + { + name: "grouped and ungrouped both unsatisfied are reported together", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "sarif"}, + {Type: art, Name: "cyclonedx", Group: "sbom"}, + {Type: art, Name: "spdx", Group: "sbom"}, + }, + crafted: craftedSet(), + wantErr: `some materials have not been crafted yet: sarif; at least one material from group "sbom" is required: cyclonedx, spdx`, + }, + { + // Materials absent from the contract are legitimate: runtime-added + // evidence and exploded archives both produce undeclared names. + name: "materials not declared in the contract are ignored", + expected: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "scan-report"}, + }, + crafted: craftedSet("scan-report", "scan-report-1", "scan-report-archive", "adhoc"), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := ValidateMaterialsPresence(tc.expected, tc.crafted) + if tc.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Equal(t, tc.wantErr, err.Error()) + }) + } +} + func TestCraftingStateGetEnvAllowList(t *testing.T) { testCases := []struct { name string diff --git a/pkg/attestation/renderer/chainloop/materials_roundtrip_test.go b/pkg/attestation/renderer/chainloop/materials_roundtrip_test.go new file mode 100644 index 000000000..9dcfa19ef --- /dev/null +++ b/pkg/attestation/renderer/chainloop/materials_roundtrip_test.go @@ -0,0 +1,147 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package chainloop + +import ( + "encoding/json" + "testing" + + schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + attestationapi "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" + intoto "github.com/in-toto/attestation/go/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/encoding/protojson" +) + +// The control plane validates an attestation against its contract by matching +// the names that GetMaterials() reports against the material names the contract +// declares. GetMaterials() silently drops any descriptor that normalizeMaterial +// rejects, so a material whose name fails to survive the crafting-state -> +// descriptor -> JSON -> predicate round trip would read as "not crafted" and +// wrongly fail the attestation. +// +// This guards that every material shape keeps its contract name all the way +// through, including over the protojson/json boundary the predicate crosses. +func TestMaterialNameSurvivesPredicateRoundTrip(t *testing.T) { + const sha = "aa5f8b2b0dd7e13d54f5f9a9dd6a4c1efb0d4c3a5e1b8f7a6c9d2e3f4a5b6c7d" + + testCases := []struct { + name string + contractName string + material *attestationapi.Attestation_Material + }{ + { + name: "string material", + contractName: "release-notes", + material: &attestationapi.Attestation_Material{ + MaterialType: schemaapi.CraftingSchema_Material_STRING, + M: &attestationapi.Attestation_Material_String_{ + String_: &attestationapi.Attestation_Material_KeyVal{ + Value: "some value", Digest: "sha256:" + sha, + }, + }, + }, + }, + { + name: "container image", + contractName: "image", + material: &attestationapi.Attestation_Material{ + MaterialType: schemaapi.CraftingSchema_Material_CONTAINER_IMAGE, + M: &attestationapi.Attestation_Material_ContainerImage_{ + ContainerImage: &attestationapi.Attestation_Material_ContainerImage{ + Name: "ghcr.io/chainloop-dev/chainloop/cli", Digest: "sha256:" + sha, + }, + }, + }, + }, + { + name: "artifact uploaded to CAS", + contractName: "binary", + material: &attestationapi.Attestation_Material{ + MaterialType: schemaapi.CraftingSchema_Material_ARTIFACT, + UploadedToCas: true, + M: &attestationapi.Attestation_Material_Artifact_{ + Artifact: &attestationapi.Attestation_Material_Artifact{ + Name: "chainloop-linux-amd64", Digest: "sha256:" + sha, + }, + }, + }, + }, + { + name: "artifact embedded inline", + contractName: "evidence", + material: &attestationapi.Attestation_Material{ + MaterialType: schemaapi.CraftingSchema_Material_EVIDENCE, + InlineCas: true, + M: &attestationapi.Attestation_Material_Artifact_{ + Artifact: &attestationapi.Attestation_Material_Artifact{ + Name: "approval.json", Digest: "sha256:" + sha, Content: []byte(`{"approved":true}`), + }, + }, + }, + }, + { + name: "sarif artifact", + contractName: "static-analysis", + material: &attestationapi.Attestation_Material{ + MaterialType: schemaapi.CraftingSchema_Material_SARIF, + M: &attestationapi.Attestation_Material_Artifact_{ + Artifact: &attestationapi.Attestation_Material_Artifact{ + Name: "report.sarif", Digest: "sha256:" + sha, + }, + }, + }, + }, + { + name: "sbom artifact", + contractName: "sbom", + material: &attestationapi.Attestation_Material{ + MaterialType: schemaapi.CraftingSchema_Material_SBOM_CYCLONEDX_JSON, + M: &attestationapi.Attestation_Material_SbomArtifact{ + SbomArtifact: &attestationapi.Attestation_Material_SBOMArtifact{ + Artifact: &attestationapi.Attestation_Material_Artifact{ + Name: "bom.json", Digest: "sha256:" + sha, + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + descriptor, err := tc.material.CraftingStateToIntotoDescriptor(tc.contractName) + require.NoError(t, err) + + // Mimic the encoding the predicate actually goes through: the + // statement is protojson-encoded into the DSSE payload and decoded + // back into the predicate struct with encoding/json. + raw, err := protojson.Marshal(descriptor) + require.NoError(t, err) + + var decoded intoto.ResourceDescriptor + require.NoError(t, json.Unmarshal(raw, &decoded)) + + predicate := &ProvenancePredicateV02{Materials: []*intoto.ResourceDescriptor{&decoded}} + + materials := predicate.GetMaterials() + require.Len(t, materials, 1, "material was dropped during normalization") + assert.Equal(t, tc.contractName, materials[0].Name) + assert.Equal(t, tc.material.MaterialType.String(), materials[0].Type) + }) + } +} From 837058aa220c2f42e7ee88e20e100d2de9241105 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 22 Sep 2026 19:37:20 +0200 Subject: [PATCH 2/2] fix(controlplane): check the contract before the bundle reaches CAS When skip_db_storage is enabled the bundle is uploaded to the CAS backend before SaveAttestation runs, so an attestation rejected for violating its contract would already have left an orphaned blob behind. Add ValidateAttestationContract, a side-effect-free check the synchronous CAS path runs before uploading. SaveAttestation keeps its own check, which remains the authoritative one since it sits on the path every attestation takes. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 513023ae-e4c2-4c52-ba7b-5ef838eda385 --- .../internal/service/attestation.go | 8 ++++ app/controlplane/pkg/biz/workflowrun.go | 37 ++++++++++++++++ .../workflowrun_contract_integration_test.go | 44 +++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/app/controlplane/internal/service/attestation.go b/app/controlplane/internal/service/attestation.go index 90ce3efd6..53c05ffee 100644 --- a/app/controlplane/internal/service/attestation.go +++ b/app/controlplane/internal/service/attestation.go @@ -336,6 +336,14 @@ func (s *AttestationService) storeAttestation(ctx context.Context, bundle []byte return nil, handleUseCaseErr(hashErr, s.log) } + // On this path the bundle reaches CAS before SaveAttestation gets to + // validate it, so the contract is checked up front. Otherwise an + // attestation rejected for violating its contract would still have left + // a blob behind in the CAS backend. + if err = s.wrUseCase.ValidateAttestationContract(ctx, workflowRunID, bundle); err != nil { + return nil, handleUseCaseErr(err, s.log) + } + if err = s.uploadAttestationToCASWithRetry(ctx, bundle, casBackend, workflowRunID, digestHash); err != nil { return nil, handleUseCaseErr(err, s.log) } diff --git a/app/controlplane/pkg/biz/workflowrun.go b/app/controlplane/pkg/biz/workflowrun.go index 467537e49..bd40c6a83 100644 --- a/app/controlplane/pkg/biz/workflowrun.go +++ b/app/controlplane/pkg/biz/workflowrun.go @@ -417,6 +417,43 @@ func (uc *WorkflowRunUseCase) orgBlocksReleasedVersions(ctx context.Context, run return org.BlockAttestationsOnReleasedVersions, nil } +// ValidateAttestationContract checks a bundle against the contract revision +// pinned on its workflow run without persisting anything. +// +// SaveAttestation runs the same check and is the authoritative one, since it +// sits on the path every attestation takes. This entry point exists for callers +// that push the bundle to a CAS backend before calling SaveAttestation: without +// it, an attestation rejected for violating its contract would already have left +// a blob behind in CAS. +func (uc *WorkflowRunUseCase) ValidateAttestationContract(ctx context.Context, runID string, bundle []byte) error { + ctx, span := otelx.Start(ctx, workflowRunTracer, "WorkflowRunUseCase.ValidateAttestationContract") + defer span.End() + + id, err := uuid.Parse(runID) + if err != nil { + return NewErrInvalidUUID(err) + } + + run, err := uc.wfRunRepo.FindByID(ctx, id) + if err != nil { + return fmt.Errorf("finding workflow run: %w", err) + } else if run == nil { + return NewErrNotFound("workflow run") + } + + dsseEnv, err := attestation.DSSEEnvelopeFromBundleBytes(bundle) + if err != nil { + return fmt.Errorf("extracting DSSE envelope: %w", err) + } + + predicate, err := chainloop.ExtractPredicate(dsseEnv) + if err != nil { + return fmt.Errorf("extracting predicate: %w", err) + } + + return uc.validateAgainstContract(ctx, run, predicate) +} + // validateAgainstContract rejects an attestation that does not satisfy the // contract revision pinned on the workflow run when it was initialized. // diff --git a/app/controlplane/pkg/biz/workflowrun_contract_integration_test.go b/app/controlplane/pkg/biz/workflowrun_contract_integration_test.go index 1ce929196..9308c8c8e 100644 --- a/app/controlplane/pkg/biz/workflowrun_contract_integration_test.go +++ b/app/controlplane/pkg/biz/workflowrun_contract_integration_test.go @@ -24,6 +24,7 @@ import ( "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/testhelpers" "github.com/chainloop-dev/chainloop/pkg/credentials" creds "github.com/chainloop-dev/chainloop/pkg/credentials/mocks" + "github.com/google/uuid" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -226,6 +227,49 @@ func (s *workflowRunContractIntegrationTestSuite) TestRejectedAttestationIsNotPe s.Nil(stored.Attestation.Envelope, "a rejected attestation must not persist its envelope") } +// The synchronous CAS path uploads the bundle before SaveAttestation runs, so it +// needs a check it can run first. This must reach the same verdict as the one +// inside SaveAttestation while leaving the run untouched. +func (s *workflowRunContractIntegrationTestSuite) TestValidateAttestationContractIsSideEffectFree() { + ctx := context.Background() + + s.Run("rejects a violating attestation without touching the run", func() { + run := s.newRunWithContract("preflight-reject", ` materials: + - type: SARIF + name: static-analysis`) + bundleBytes := testhelpers.BundleBytesFromEnvelope(s.T(), attestationFull) + + err := s.WorkflowRun.ValidateAttestationContract(ctx, run.ID.String(), bundleBytes) + require.Error(s.T(), err) + s.True(biz.IsErrValidation(err), "expected a validation error, got %T: %v", err, err) + s.ErrorContains(err, "some materials have not been crafted yet: static-analysis") + + stored, err := s.WorkflowRun.GetByIDInOrg(ctx, s.org.ID, run.ID.String()) + require.NoError(s.T(), err) + s.Empty(stored.Attestation.Digest, "the preflight check must not record anything") + }) + + s.Run("accepts a satisfying attestation and still allows the save", func() { + run := s.newRunWithContract("preflight-accept", ` materials: + - type: CONTAINER_IMAGE + name: image`) + bundleBytes := testhelpers.BundleBytesFromEnvelope(s.T(), attestationFull) + + require.NoError(s.T(), s.WorkflowRun.ValidateAttestationContract(ctx, run.ID.String(), bundleBytes)) + + digest, err := s.WorkflowRun.SaveAttestation(ctx, run.ID.String(), bundleBytes) + require.NoError(s.T(), err) + s.NotNil(digest) + }) + + s.Run("reports an unknown run rather than passing it through", func() { + bundleBytes := testhelpers.BundleBytesFromEnvelope(s.T(), attestationFull) + err := s.WorkflowRun.ValidateAttestationContract(ctx, uuid.NewString(), bundleBytes) + require.Error(s.T(), err) + s.True(biz.IsNotFound(err), "expected a not-found error, got %T: %v", err, err) + }) +} + func TestWorkflowRunContractEnforcement(t *testing.T) { suite.Run(t, new(workflowRunContractIntegrationTestSuite)) }