From 60b6703b4ac29d0413c8e1d49ebe7331078033d9 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 4 Nov 2026 10:45:07 +0100 Subject: [PATCH 1/8] fix(policies): reject material policy attachments that can never apply A policy without material types applies only to the material named by the attachment selector. Attaching it without selector.name was accepted, but the policy never ran and nothing reported it. The control plane now rejects such attachments on contract create and update, and also typed policies whose exact selector names a contract material of an unsupported type. attestation init warns about them for contracts created before this check. The applicability rule is shared between evaluation and validation. Closes #3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c --- app/cli/pkg/action/attestation_init.go | 6 + app/controlplane/pkg/biz/workflowcontract.go | 20 +- .../pkg/biz/workflowcontract_policies_test.go | 151 ++++++++++++++ pkg/attestation/crafter/crafter.go | 11 + .../crafter/policy_attachments_test.go | 84 ++++++++ pkg/policies/attachment_validation.go | 87 ++++++++ pkg/policies/attachment_validation_test.go | 196 ++++++++++++++++++ pkg/policies/policies.go | 22 +- 8 files changed, 563 insertions(+), 14 deletions(-) create mode 100644 app/controlplane/pkg/biz/workflowcontract_policies_test.go create mode 100644 pkg/attestation/crafter/policy_attachments_test.go create mode 100644 pkg/policies/attachment_validation.go create mode 100644 pkg/policies/attachment_validation_test.go diff --git a/app/cli/pkg/action/attestation_init.go b/app/cli/pkg/action/attestation_init.go index f75c5bc82..3c8865336 100644 --- a/app/cli/pkg/action/attestation_init.go +++ b/app/cli/pkg/action/attestation_init.go @@ -333,6 +333,12 @@ func (action *AttestationInit) Run(ctx context.Context, opts *AttestationInitRun return "", err } + // Warn about material policy attachments that can never be evaluated. Contracts created + // before the control plane rejected them, and policies loaded locally, can still have them. + for _, err := range action.c.ValidateMaterialPolicyAttachments(ctx) { + action.Logger.Warn().Msgf("policy will not be evaluated: %v", err) + } + // Load the env variables both the system populated and the user predefined ones if err := action.c.ResolveEnvVars(ctx, attestationID); err != nil { if action.dryRun { diff --git a/app/controlplane/pkg/biz/workflowcontract.go b/app/controlplane/pkg/biz/workflowcontract.go index b8d8d5b69..6ff840a20 100644 --- a/app/controlplane/pkg/biz/workflowcontract.go +++ b/app/controlplane/pkg/biz/workflowcontract.go @@ -526,7 +526,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, } } for _, att := range schema.GetPolicies().GetMaterials() { - if _, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, schema.GetMaterials()); err != nil { return NewErrValidation(err) } } @@ -545,7 +545,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, } } for _, att := range spec.GetPolicies().GetMaterials() { - if _, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, spec.GetMaterials()); err != nil { return NewErrValidation(err) } } @@ -625,6 +625,22 @@ func (uc *WorkflowContractUseCase) findAndValidatePolicy(ctx context.Context, at return policy, nil } +// findAndValidateMaterialPolicy validates a material policy attachment and, when its +// policy can be resolved, checks that the attachment can apply to at least one material. +func (uc *WorkflowContractUseCase) findAndValidateMaterialPolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, materials []*schemav1.CraftingSchema_Material) error { + policy, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames) + if err != nil { + return err + } + + // the policy might not be available in this context (local or batch-local policy) + if policy == nil { + return nil + } + + return loader.ValidateMaterialAttachment(policy, att, materials) +} + func (uc *WorkflowContractUseCase) findAndValidatePolicyGroup(ctx context.Context, att *schemav1.PolicyGroupAttachment, token string, batchPolicyGroupNames []string) (*schemav1.PolicyGroup, error) { if !loader.IsProviderScheme(att.GetRef()) { // Otherwise, don't return an error, as it might consist of a local policy, not available in this context diff --git a/app/controlplane/pkg/biz/workflowcontract_policies_test.go b/app/controlplane/pkg/biz/workflowcontract_policies_test.go new file mode 100644 index 000000000..77fd2b418 --- /dev/null +++ b/app/controlplane/pkg/biz/workflowcontract_policies_test.go @@ -0,0 +1,151 @@ +// +// 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 + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "path" + "testing" + + cpolicies "github.com/chainloop-dev/chainloop/app/controlplane/pkg/policies" + "github.com/go-kratos/kratos/v2/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// remote policies served by the fake policy provider, by name +var providerPolicies = map[string]string{ + "kindless": `apiVersion: chainloop.dev/v1 +kind: Policy +metadata: + name: kindless +spec: + policies: + - embedded: | + package main + import rego.v1 + violations contains msg if { false; msg := "" } +`, + "sbom-only": `apiVersion: chainloop.dev/v1 +kind: Policy +metadata: + name: sbom-only +spec: + policies: + - kind: SBOM_CYCLONEDX_JSON + embedded: | + package main + import rego.v1 + violations contains msg if { false; msg := "" } +`, +} + +func newFakePolicyProvider(t *testing.T) *cpolicies.Registry { + t.Helper() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // the validate endpoint is optional for providers, a 404 skips it + raw, ok := providerPolicies[path.Base(r.URL.Path)] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + + _ = json.NewEncoder(w).Encode(cpolicies.ProviderResponse{ + Digest: "sha256:4a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8f9", + Raw: &cpolicies.RawMessage{Body: []byte(raw), Format: "FORMAT_YAML"}, + }) + })) + t.Cleanup(server.Close) + + registry, err := cpolicies.NewRegistry(log.DefaultLogger, &cpolicies.NewRegistryConfig{Name: "builtin", URL: server.URL, Default: true}) + require.NoError(t, err) + + return registry +} + +func contractWithMaterialPolicy(policyName, selector string) []byte { + return fmt.Appendf(nil, `apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: my-contract +spec: + materials: + - name: my-image + type: CONTAINER_IMAGE + - name: sbom + type: SBOM_CYCLONEDX_JSON + policies: + materials: + - ref: %s + %s +`, policyName, selector) +} + +func TestValidateContractPoliciesMaterialApplicability(t *testing.T) { + uc := NewWorkflowContractUseCase(nil, newFakePolicyProvider(t), nil, log.DefaultLogger) + + testCases := []struct { + name string + contract []byte + wantErr string + }{ + { + name: "kind-less policy without selector", + contract: contractWithMaterialPolicy("kindless", ""), + wantErr: `policy "kindless" applies to any material type and requires a material selector (selector.name)`, + }, + { + name: "kind-less policy with selector", + contract: contractWithMaterialPolicy("kindless", "selector: {name: my-image}"), + }, + { + name: "typed policy with selector of an unsupported material", + contract: contractWithMaterialPolicy("sbom-only", "selector: {name: my-image}"), + wantErr: `policy "sbom-only" does not support material "my-image" of type CONTAINER_IMAGE (supported types: [SBOM_CYCLONEDX_JSON])`, + }, + { + name: "v1 contract with kind-less policy without selector", + contract: []byte(`schemaVersion: v1 +materials: + - name: my-image + type: CONTAINER_IMAGE +policies: + materials: + - ref: kindless +`), + wantErr: `policy "kindless" applies to any material type and requires a material selector (selector.name)`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", nil, nil) + if tc.wantErr == "" { + assert.NoError(t, err) + return + } + + require.Error(t, err) + assert.True(t, IsErrValidation(err)) + assert.ErrorContains(t, err, tc.wantErr) + }) + } +} diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index c2a6345b5..e2425df52 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -1111,6 +1111,17 @@ func policyEvalMatches(a, b *api.PolicyEvaluation) bool { return proto.Equal(a.GetPolicyReference(), b.GetPolicyReference()) && maps.Equal(a.GetWith(), b.GetWith()) } +// ValidateMaterialPolicyAttachments returns one error for each material policy +// attachment of the contract that can never apply to a material, for example a +// policy without material types attached without a material selector. +func (c *Crafter) ValidateMaterialPolicyAttachments(ctx context.Context) []error { + pv := policies.NewPolicyVerifier(c.CraftingState.GetPolicies(), c.attClient, c.Logger, + policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...), + ) + + return pv.ValidateMaterialAttachments(ctx, c.CraftingState.GetMaterials()) +} + // 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 { diff --git a/pkg/attestation/crafter/policy_attachments_test.go b/pkg/attestation/crafter/policy_attachments_test.go new file mode 100644 index 000000000..d4d2822d2 --- /dev/null +++ b/pkg/attestation/crafter/policy_attachments_test.go @@ -0,0 +1,84 @@ +// +// 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 crafter_test + +import ( + "context" + "testing" + + 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" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateMaterialPolicyAttachments(t *testing.T) { + const rego = "package main\nimport rego.v1\nviolations contains msg if { false; msg := \"\" }\n" + + kindless := &schemaapi.Policy{ + ApiVersion: "chainloop.dev/v1", + Kind: "Policy", + Metadata: &schemaapi.Metadata{Name: "kindless"}, + Spec: &schemaapi.PolicySpec{ + Policies: []*schemaapi.PolicySpecV2{{Source: &schemaapi.PolicySpecV2_Embedded{Embedded: rego}}}, + }, + } + + testCases := []struct { + name string + selector *schemaapi.PolicyAttachment_MaterialSelector + wantErrs int + }{ + { + name: "kind-less policy without selector", + wantErrs: 1, + }, + { + name: "kind-less policy with selector", + selector: &schemaapi.PolicyAttachment_MaterialSelector{Name: "image"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + contract := &schemaapi.CraftingSchemaV2{ + ApiVersion: "chainloop.dev/v1", + Kind: "Contract", + Metadata: &schemaapi.Metadata{Name: "contract"}, + Spec: &schemaapi.CraftingSchemaV2Spec{ + Materials: []*schemaapi.CraftingSchema_Material{ + {Name: "image", Type: schemaapi.CraftingSchema_Material_CONTAINER_IMAGE}, + }, + Policies: &schemaapi.Policies{ + Materials: []*schemaapi.PolicyAttachment{ + {Policy: &schemaapi.PolicyAttachment_Embedded{Embedded: kindless}, Selector: tc.selector}, + }, + }, + }, + } + + c, err := newInitializedCrafterV2(t, contract, &v1.WorkflowMetadata{Name: "wf"}, true, "", nil) + require.NoError(t, err) + + errs := c.ValidateMaterialPolicyAttachments(context.Background()) + assert.Len(t, errs, tc.wantErrs) + for _, err := range errs { + assert.ErrorIs(t, err, policies.ErrAttachmentNeverApplies) + } + }) + } +} diff --git a/pkg/policies/attachment_validation.go b/pkg/policies/attachment_validation.go new file mode 100644 index 000000000..03da31c96 --- /dev/null +++ b/pkg/policies/attachment_validation.go @@ -0,0 +1,87 @@ +// +// 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 policies + +import ( + "context" + "errors" + "fmt" + + v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" +) + +// ErrAttachmentNeverApplies is returned when a material policy attachment can +// never be evaluated on any material, so it would silently be a no-op. +var ErrAttachmentNeverApplies = errors.New("policy attachment never applies") + +// ValidateMaterialAttachment checks that a material policy attachment can apply +// to at least one material, using the same rule as the policy evaluation: +// - a policy without material types applies only to the material named by +// the attachment selector, so the selector is required. +// - a typed policy whose selector names, by exact match, a contract material +// of a type the policy does not support never applies. +// +// materials are the materials declared in the contract. A selector that names a +// material not declared in the contract is accepted, because that material can +// be added at attestation time. +func ValidateMaterialAttachment(policy *v1.Policy, att *v1.PolicyAttachment, materials []*v1.CraftingSchema_Material) error { + policyName := policy.GetMetadata().GetName() + if policyName == "" { + policyName = att.GetRef() + } + + specTypes := getPolicyTypes(policy) + selector := att.GetSelector() + + if len(specTypes) == 0 && selector.GetName() == "" { + return fmt.Errorf("%w: policy %q applies to any material type and requires a material selector (selector.name)", ErrAttachmentNeverApplies, policyName) + } + + // a prefix selector can match materials added at attestation time + if selector.GetName() == "" || selector.GetMatchMode() == v1.PolicyAttachment_MaterialSelector_PREFIX { + return nil + } + + for _, m := range materials { + if m.GetName() == selector.GetName() && !attachmentApplies(specTypes, selector, m.GetName(), m.GetType()) { + return fmt.Errorf("%w: policy %q does not support material %q of type %s (supported types: %v)", + ErrAttachmentNeverApplies, policyName, m.GetName(), m.GetType(), specTypes) + } + } + + return nil +} + +// ValidateMaterialAttachments loads the policy of each material attachment and +// returns one error for each attachment that can never apply to a material. +// See ValidateMaterialAttachment. Attachments whose policy can't be loaded are +// skipped, since the load error is reported when the policy is evaluated. +func (pv *PolicyVerifier) ValidateMaterialAttachments(ctx context.Context, materials []*v1.CraftingSchema_Material) []error { + var errs []error + for _, att := range pv.policies.GetMaterials() { + spec, _, err := pv.loadPolicySpec(ctx, att) + if err != nil { + pv.logger.Debug().Err(err).Str("policy", att.GetRef()).Msg("skipping attachment validation: failed to load policy") + continue + } + + if err := ValidateMaterialAttachment(spec, att, materials); err != nil { + errs = append(errs, err) + } + } + + return errs +} diff --git a/pkg/policies/attachment_validation_test.go b/pkg/policies/attachment_validation_test.go new file mode 100644 index 000000000..746e5e2db --- /dev/null +++ b/pkg/policies/attachment_validation_test.go @@ -0,0 +1,196 @@ +// +// 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 policies + +import ( + "context" + "testing" + + v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateMaterialAttachment(t *testing.T) { + kindless := &v1.Policy{ + Metadata: &v1.Metadata{Name: "upstream-workflow-attested"}, + Spec: &v1.PolicySpec{}, + } + kindlessV2 := &v1.Policy{ + Metadata: &v1.Metadata{Name: "kindless-v2"}, + Spec: &v1.PolicySpec{ + Policies: []*v1.PolicySpecV2{{Source: &v1.PolicySpecV2_Path{Path: "p.rego"}}}, + }, + } + sbomPolicy := &v1.Policy{ + Metadata: &v1.Metadata{Name: "sbom-policy"}, + Spec: &v1.PolicySpec{ + Policies: []*v1.PolicySpecV2{ + {Kind: v1.CraftingSchema_Material_SBOM_SPDX_JSON}, + {Kind: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON}, + }, + }, + } + + materials := []*v1.CraftingSchema_Material{ + {Name: "sbom", Type: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON}, + {Name: "image", Type: v1.CraftingSchema_Material_CONTAINER_IMAGE}, + } + + testCases := []struct { + name string + policy *v1.Policy + selector *v1.PolicyAttachment_MaterialSelector + wantErrIs error + wantMsg string + }{ + { + name: "kind-less policy without selector", + policy: kindless, + wantErrIs: ErrAttachmentNeverApplies, + wantMsg: `policy attachment never applies: policy "upstream-workflow-attested" applies to any material type and requires a material selector (selector.name)`, + }, + { + name: "kind-less v2 policy without selector", + policy: kindlessV2, + wantErrIs: ErrAttachmentNeverApplies, + }, + { + name: "kind-less policy with empty selector name", + policy: kindless, + selector: &v1.PolicyAttachment_MaterialSelector{}, + wantErrIs: ErrAttachmentNeverApplies, + }, + { + name: "kind-less policy with selector", + policy: kindless, + selector: &v1.PolicyAttachment_MaterialSelector{Name: "image"}, + }, + { + name: "kind-less policy with selector of a material not in the contract", + policy: kindless, + selector: &v1.PolicyAttachment_MaterialSelector{Name: "added-at-runtime"}, + }, + { + name: "typed policy without selector", + policy: sbomPolicy, + }, + { + name: "typed policy with selector of a supported material", + policy: sbomPolicy, + selector: &v1.PolicyAttachment_MaterialSelector{Name: "sbom"}, + }, + { + name: "typed policy with selector of an unsupported contract material", + policy: sbomPolicy, + selector: &v1.PolicyAttachment_MaterialSelector{Name: "image"}, + wantErrIs: ErrAttachmentNeverApplies, + wantMsg: `policy attachment never applies: policy "sbom-policy" does not support material "image" of type CONTAINER_IMAGE (supported types: [SBOM_SPDX_JSON SBOM_CYCLONEDX_JSON])`, + }, + { + name: "typed policy with explicit exact selector of an unsupported contract material", + policy: sbomPolicy, + selector: &v1.PolicyAttachment_MaterialSelector{Name: "image", MatchMode: v1.PolicyAttachment_MaterialSelector_EXACT}, + wantErrIs: ErrAttachmentNeverApplies, + }, + { + name: "typed policy with prefix selector is not checked against contract materials", + policy: sbomPolicy, + selector: &v1.PolicyAttachment_MaterialSelector{Name: "image", MatchMode: v1.PolicyAttachment_MaterialSelector_PREFIX}, + }, + { + name: "typed policy with selector of a material not in the contract", + policy: sbomPolicy, + selector: &v1.PolicyAttachment_MaterialSelector{Name: "added-at-runtime"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + att := &v1.PolicyAttachment{ + Policy: &v1.PolicyAttachment_Ref{Ref: "chainloop://some-policy"}, + Selector: tc.selector, + } + + err := ValidateMaterialAttachment(tc.policy, att, materials) + if tc.wantErrIs == nil { + assert.NoError(t, err) + return + } + + require.Error(t, err) + assert.ErrorIs(t, err, tc.wantErrIs) + if tc.wantMsg != "" { + assert.EqualError(t, err, tc.wantMsg) + } + }) + } +} + +func TestPolicyVerifierValidateMaterialAttachments(t *testing.T) { + logger := zerolog.Nop() + materials := []*v1.CraftingSchema_Material{ + {Name: "custom-material", Type: v1.CraftingSchema_Material_ARTIFACT}, + } + + testCases := []struct { + name string + attachments []*v1.PolicyAttachment + wantErrs int + }{ + { + name: "valid attachments", + attachments: []*v1.PolicyAttachment{ + {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft.yaml"}}, + { + Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft_not_typed.yaml"}, + Selector: &v1.PolicyAttachment_MaterialSelector{Name: "custom-material"}, + }, + }, + }, + { + name: "kind-less policy without selector", + attachments: []*v1.PolicyAttachment{ + {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft.yaml"}}, + {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft_not_typed.yaml"}}, + }, + wantErrs: 1, + }, + { + name: "several attachments that never apply", + attachments: []*v1.PolicyAttachment{ + {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft_not_typed.yaml"}}, + { + Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft.yaml"}, + Selector: &v1.PolicyAttachment_MaterialSelector{Name: "custom-material"}, + }, + }, + wantErrs: 2, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + pv := NewPolicyVerifier(&v1.Policies{Materials: tc.attachments}, nil, &logger) + errs := pv.ValidateMaterialAttachments(context.TODO(), materials) + assert.Len(t, errs, tc.wantErrs) + for _, err := range errs { + assert.ErrorIs(t, err, ErrAttachmentNeverApplies) + } + }) + } +} diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index f4aff7795..6ecd91415 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -1015,27 +1015,25 @@ func (pv *PolicyVerifier) shouldApplyPolicy(ctx context.Context, policyAtt *v1.P return false, fmt.Errorf("failed to load policy attachment %q: %w", policyAtt.GetRef(), err) } - materialType := material.GetMaterialType() - filteredName := policyAtt.GetSelector().GetName() - specTypes := getPolicyTypes(spec) + return attachmentApplies(getPolicyTypes(spec), policyAtt.GetSelector(), material.GetId(), material.GetMaterialType()), nil +} +// attachmentApplies is the pure applicability rule of shouldApplyPolicy, shared with +// ValidateMaterialAttachment so that contract validation and evaluation can't drift. +func attachmentApplies(specTypes []v1.CraftingSchema_Material_MaterialType, selector *v1.PolicyAttachment_MaterialSelector, materialName string, materialType v1.CraftingSchema_Material_MaterialType) bool { // if spec has a type, and it's different to the material type, skip if len(specTypes) > 0 && !slices.Contains(specTypes, materialType) { - // types don't match, continue - return false, nil + return false } - if filteredName != "" && !nameMatches(filteredName, policyAtt.GetSelector().GetMatchMode(), material.GetId()) { + filteredName := selector.GetName() + if filteredName != "" && !nameMatches(filteredName, selector.GetMatchMode(), materialName) { // a filter exists and doesn't match - return false, nil + return false } // no type nor name to match, we can't guess anything - if len(specTypes) == 0 && filteredName == "" { - return false, nil - } - - return true, nil + return len(specTypes) > 0 || filteredName != "" } // nameMatches reports whether a material name satisfies a selector/group-material From 2064e81e10fdbce6d9b05067729241bda1005590 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 4 Nov 2026 10:52:01 +0100 Subject: [PATCH 2/8] fix(controlplane): only warn about never-applying policies on attestation init attestation init re-sends the contract on every run, so rejecting material policy attachments that can never apply there would break pipelines whose contract was stored before the check. Skip the check in FindOrCreateWorkflow and keep it on contract create and update. The CLI warns at init instead. Refs #3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c --- .../internal/service/attestation.go | 5 +-- app/controlplane/pkg/biz/workflowcontract.go | 35 +++++++++++++++---- .../pkg/biz/workflowcontract_policies_test.go | 14 +++++++- 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/app/controlplane/internal/service/attestation.go b/app/controlplane/internal/service/attestation.go index f76bedea5..2e5b916b2 100644 --- a/app/controlplane/internal/service/attestation.go +++ b/app/controlplane/internal/service/attestation.go @@ -807,9 +807,10 @@ func (s *AttestationService) FindOrCreateWorkflow(ctx context.Context, req *cpAP return nil, err } - // contract validation + // contract validation. The contract is re-sent on every attestation init, so a contract stored + // before the material policy applicability check must keep working. The CLI warns about it instead. if req.GetContractBytes() != nil { - if err = s.workflowContractUseCase.ValidateContractPolicies(ctx, req.GetContractBytes(), token, nil, nil); err != nil { + if err = s.workflowContractUseCase.ValidateContractPolicies(ctx, req.GetContractBytes(), token, nil, nil, biz.WithoutMaterialApplicabilityCheck()); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) } } diff --git a/app/controlplane/pkg/biz/workflowcontract.go b/app/controlplane/pkg/biz/workflowcontract.go index 6ff840a20..693a7aba9 100644 --- a/app/controlplane/pkg/biz/workflowcontract.go +++ b/app/controlplane/pkg/biz/workflowcontract.go @@ -503,12 +503,33 @@ func (uc *WorkflowContractUseCase) RevisionWouldChange(ctx context.Context, orgI return !bytes.Equal(latest.Version.Schema.Raw, incoming.Raw), nil } +// ValidateContractPoliciesOpt configures ValidateContractPolicies +type ValidateContractPoliciesOpt func(*validateContractPoliciesOpts) + +type validateContractPoliciesOpts struct { + skipMaterialApplicability bool +} + +// WithoutMaterialApplicabilityCheck skips the check that rejects material policy attachments +// that can never apply. Use it where an existing contract is re-sent, for example on attestation +// init, so that contracts stored before the check keep working. The CLI warns about them instead. +func WithoutMaterialApplicabilityCheck() ValidateContractPoliciesOpt { + return func(o *validateContractPoliciesOpts) { + o.skipMaterialApplicability = true + } +} + // ValidateContractPolicies checks that policies and policy groups referenced by the contract // exist. batchPolicyNames and batchPolicyGroupNames carry the names of resources being // created/updated in the same batch apply: references to those are treated as known instead of // being resolved against the registry, since they may not be persisted yet (e.g. during // dry-run). Remote references are still validated regardless. -func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, rawSchema []byte, token string, batchPolicyNames, batchPolicyGroupNames []string) error { +func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, rawSchema []byte, token string, batchPolicyNames, batchPolicyGroupNames []string, opts ...ValidateContractPoliciesOpt) error { + var options validateContractPoliciesOpts + for _, opt := range opts { + opt(&options) + } + // Validate that externally provided policies exist c, err := identifyUnMarshalAndValidateRawContract(rawSchema) if err != nil { @@ -526,7 +547,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, } } for _, att := range schema.GetPolicies().GetMaterials() { - if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, schema.GetMaterials()); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, schema.GetMaterials(), !options.skipMaterialApplicability); err != nil { return NewErrValidation(err) } } @@ -545,7 +566,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, } } for _, att := range spec.GetPolicies().GetMaterials() { - if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, spec.GetMaterials()); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, spec.GetMaterials(), !options.skipMaterialApplicability); err != nil { return NewErrValidation(err) } } @@ -625,16 +646,16 @@ func (uc *WorkflowContractUseCase) findAndValidatePolicy(ctx context.Context, at return policy, nil } -// findAndValidateMaterialPolicy validates a material policy attachment and, when its -// policy can be resolved, checks that the attachment can apply to at least one material. -func (uc *WorkflowContractUseCase) findAndValidateMaterialPolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, materials []*schemav1.CraftingSchema_Material) error { +// findAndValidateMaterialPolicy validates a material policy attachment and, when checkApplicability +// is set and its policy can be resolved, checks that the attachment can apply to at least one material. +func (uc *WorkflowContractUseCase) findAndValidateMaterialPolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, materials []*schemav1.CraftingSchema_Material, checkApplicability bool) error { policy, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames) if err != nil { return err } // the policy might not be available in this context (local or batch-local policy) - if policy == nil { + if policy == nil || !checkApplicability { return nil } diff --git a/app/controlplane/pkg/biz/workflowcontract_policies_test.go b/app/controlplane/pkg/biz/workflowcontract_policies_test.go index 77fd2b418..1288355e5 100644 --- a/app/controlplane/pkg/biz/workflowcontract_policies_test.go +++ b/app/controlplane/pkg/biz/workflowcontract_policies_test.go @@ -105,6 +105,7 @@ func TestValidateContractPoliciesMaterialApplicability(t *testing.T) { testCases := []struct { name string contract []byte + opts []ValidateContractPoliciesOpt wantErr string }{ { @@ -133,11 +134,22 @@ policies: `), wantErr: `policy "kindless" applies to any material type and requires a material selector (selector.name)`, }, + { + name: "kind-less policy without selector, applicability check skipped", + contract: contractWithMaterialPolicy("kindless", ""), + opts: []ValidateContractPoliciesOpt{WithoutMaterialApplicabilityCheck()}, + }, + { + name: "missing policy is still rejected when the applicability check is skipped", + contract: contractWithMaterialPolicy("not-found", ""), + opts: []ValidateContractPoliciesOpt{WithoutMaterialApplicabilityCheck()}, + wantErr: `policy "not-found"`, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", nil, nil) + err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", nil, nil, tc.opts...) if tc.wantErr == "" { assert.NoError(t, err) return From 537fdfd71d4d13e023668b29a28f0bc4c9c73b0a Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 4 Nov 2026 11:51:15 +0100 Subject: [PATCH 3/8] test(policies): use constants for repeated test literals Fixes the goconst lint findings in the new attachment validation tests. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c --- pkg/policies/attachment_validation_test.go | 40 +++++++++++++--------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/pkg/policies/attachment_validation_test.go b/pkg/policies/attachment_validation_test.go index 746e5e2db..5d0c2d183 100644 --- a/pkg/policies/attachment_validation_test.go +++ b/pkg/policies/attachment_validation_test.go @@ -25,6 +25,14 @@ import ( "github.com/stretchr/testify/require" ) +const ( + materialSBOM = "sbom" + materialImage = "image" + materialCustom = "custom-material" + refSBOMSyft = "file://testdata/sbom_syft.yaml" + refSBOMSyftNotTyped = "file://testdata/sbom_syft_not_typed.yaml" +) + func TestValidateMaterialAttachment(t *testing.T) { kindless := &v1.Policy{ Metadata: &v1.Metadata{Name: "upstream-workflow-attested"}, @@ -47,8 +55,8 @@ func TestValidateMaterialAttachment(t *testing.T) { } materials := []*v1.CraftingSchema_Material{ - {Name: "sbom", Type: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON}, - {Name: "image", Type: v1.CraftingSchema_Material_CONTAINER_IMAGE}, + {Name: materialSBOM, Type: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON}, + {Name: materialImage, Type: v1.CraftingSchema_Material_CONTAINER_IMAGE}, } testCases := []struct { @@ -78,7 +86,7 @@ func TestValidateMaterialAttachment(t *testing.T) { { name: "kind-less policy with selector", policy: kindless, - selector: &v1.PolicyAttachment_MaterialSelector{Name: "image"}, + selector: &v1.PolicyAttachment_MaterialSelector{Name: materialImage}, }, { name: "kind-less policy with selector of a material not in the contract", @@ -92,25 +100,25 @@ func TestValidateMaterialAttachment(t *testing.T) { { name: "typed policy with selector of a supported material", policy: sbomPolicy, - selector: &v1.PolicyAttachment_MaterialSelector{Name: "sbom"}, + selector: &v1.PolicyAttachment_MaterialSelector{Name: materialSBOM}, }, { name: "typed policy with selector of an unsupported contract material", policy: sbomPolicy, - selector: &v1.PolicyAttachment_MaterialSelector{Name: "image"}, + selector: &v1.PolicyAttachment_MaterialSelector{Name: materialImage}, wantErrIs: ErrAttachmentNeverApplies, wantMsg: `policy attachment never applies: policy "sbom-policy" does not support material "image" of type CONTAINER_IMAGE (supported types: [SBOM_SPDX_JSON SBOM_CYCLONEDX_JSON])`, }, { name: "typed policy with explicit exact selector of an unsupported contract material", policy: sbomPolicy, - selector: &v1.PolicyAttachment_MaterialSelector{Name: "image", MatchMode: v1.PolicyAttachment_MaterialSelector_EXACT}, + selector: &v1.PolicyAttachment_MaterialSelector{Name: materialImage, MatchMode: v1.PolicyAttachment_MaterialSelector_EXACT}, wantErrIs: ErrAttachmentNeverApplies, }, { name: "typed policy with prefix selector is not checked against contract materials", policy: sbomPolicy, - selector: &v1.PolicyAttachment_MaterialSelector{Name: "image", MatchMode: v1.PolicyAttachment_MaterialSelector_PREFIX}, + selector: &v1.PolicyAttachment_MaterialSelector{Name: materialImage, MatchMode: v1.PolicyAttachment_MaterialSelector_PREFIX}, }, { name: "typed policy with selector of a material not in the contract", @@ -144,7 +152,7 @@ func TestValidateMaterialAttachment(t *testing.T) { func TestPolicyVerifierValidateMaterialAttachments(t *testing.T) { logger := zerolog.Nop() materials := []*v1.CraftingSchema_Material{ - {Name: "custom-material", Type: v1.CraftingSchema_Material_ARTIFACT}, + {Name: materialCustom, Type: v1.CraftingSchema_Material_ARTIFACT}, } testCases := []struct { @@ -155,28 +163,28 @@ func TestPolicyVerifierValidateMaterialAttachments(t *testing.T) { { name: "valid attachments", attachments: []*v1.PolicyAttachment{ - {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft.yaml"}}, + {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyft}}, { - Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft_not_typed.yaml"}, - Selector: &v1.PolicyAttachment_MaterialSelector{Name: "custom-material"}, + Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyftNotTyped}, + Selector: &v1.PolicyAttachment_MaterialSelector{Name: materialCustom}, }, }, }, { name: "kind-less policy without selector", attachments: []*v1.PolicyAttachment{ - {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft.yaml"}}, - {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft_not_typed.yaml"}}, + {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyft}}, + {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyftNotTyped}}, }, wantErrs: 1, }, { name: "several attachments that never apply", attachments: []*v1.PolicyAttachment{ - {Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft_not_typed.yaml"}}, + {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyftNotTyped}}, { - Policy: &v1.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft.yaml"}, - Selector: &v1.PolicyAttachment_MaterialSelector{Name: "custom-material"}, + Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyft}, + Selector: &v1.PolicyAttachment_MaterialSelector{Name: materialCustom}, }, }, wantErrs: 2, From 4f648984c0ab2d0ffe13c3d0dbbbffb158c7a51f Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 4 Nov 2026 13:12:07 +0100 Subject: [PATCH 4/8] fix(cli): drop the never-applying policy warning on attestation init The contract can't be fixed at attestation init, so the check only runs when the contract is created or updated in the control plane. Remove the init warning and the verifier helper that only it used. Refs #3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c --- app/cli/pkg/action/attestation_init.go | 6 -- .../internal/service/attestation.go | 4 +- app/controlplane/pkg/biz/workflowcontract.go | 2 +- pkg/attestation/crafter/crafter.go | 11 --- .../crafter/policy_attachments_test.go | 84 ------------------- pkg/policies/attachment_validation.go | 22 ----- pkg/policies/attachment_validation_test.go | 63 +------------- 7 files changed, 5 insertions(+), 187 deletions(-) delete mode 100644 pkg/attestation/crafter/policy_attachments_test.go diff --git a/app/cli/pkg/action/attestation_init.go b/app/cli/pkg/action/attestation_init.go index 3c8865336..f75c5bc82 100644 --- a/app/cli/pkg/action/attestation_init.go +++ b/app/cli/pkg/action/attestation_init.go @@ -333,12 +333,6 @@ func (action *AttestationInit) Run(ctx context.Context, opts *AttestationInitRun return "", err } - // Warn about material policy attachments that can never be evaluated. Contracts created - // before the control plane rejected them, and policies loaded locally, can still have them. - for _, err := range action.c.ValidateMaterialPolicyAttachments(ctx) { - action.Logger.Warn().Msgf("policy will not be evaluated: %v", err) - } - // Load the env variables both the system populated and the user predefined ones if err := action.c.ResolveEnvVars(ctx, attestationID); err != nil { if action.dryRun { diff --git a/app/controlplane/internal/service/attestation.go b/app/controlplane/internal/service/attestation.go index 2e5b916b2..46bba41e8 100644 --- a/app/controlplane/internal/service/attestation.go +++ b/app/controlplane/internal/service/attestation.go @@ -807,8 +807,8 @@ func (s *AttestationService) FindOrCreateWorkflow(ctx context.Context, req *cpAP return nil, err } - // contract validation. The contract is re-sent on every attestation init, so a contract stored - // before the material policy applicability check must keep working. The CLI warns about it instead. + // contract validation. The contract is re-sent on every attestation init, where it can't be fixed, + // so the material policy applicability check only runs when the contract is created or updated. if req.GetContractBytes() != nil { if err = s.workflowContractUseCase.ValidateContractPolicies(ctx, req.GetContractBytes(), token, nil, nil, biz.WithoutMaterialApplicabilityCheck()); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) diff --git a/app/controlplane/pkg/biz/workflowcontract.go b/app/controlplane/pkg/biz/workflowcontract.go index 693a7aba9..e34adc783 100644 --- a/app/controlplane/pkg/biz/workflowcontract.go +++ b/app/controlplane/pkg/biz/workflowcontract.go @@ -512,7 +512,7 @@ type validateContractPoliciesOpts struct { // WithoutMaterialApplicabilityCheck skips the check that rejects material policy attachments // that can never apply. Use it where an existing contract is re-sent, for example on attestation -// init, so that contracts stored before the check keep working. The CLI warns about them instead. +// init, where the contract can't be fixed. The check runs when the contract is created or updated. func WithoutMaterialApplicabilityCheck() ValidateContractPoliciesOpt { return func(o *validateContractPoliciesOpts) { o.skipMaterialApplicability = true diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index e2425df52..c2a6345b5 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -1111,17 +1111,6 @@ func policyEvalMatches(a, b *api.PolicyEvaluation) bool { return proto.Equal(a.GetPolicyReference(), b.GetPolicyReference()) && maps.Equal(a.GetWith(), b.GetWith()) } -// ValidateMaterialPolicyAttachments returns one error for each material policy -// attachment of the contract that can never apply to a material, for example a -// policy without material types attached without a material selector. -func (c *Crafter) ValidateMaterialPolicyAttachments(ctx context.Context) []error { - pv := policies.NewPolicyVerifier(c.CraftingState.GetPolicies(), c.attClient, c.Logger, - policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...), - ) - - return pv.ValidateMaterialAttachments(ctx, c.CraftingState.GetMaterials()) -} - // 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 { diff --git a/pkg/attestation/crafter/policy_attachments_test.go b/pkg/attestation/crafter/policy_attachments_test.go deleted file mode 100644 index d4d2822d2..000000000 --- a/pkg/attestation/crafter/policy_attachments_test.go +++ /dev/null @@ -1,84 +0,0 @@ -// -// 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 crafter_test - -import ( - "context" - "testing" - - 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" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestValidateMaterialPolicyAttachments(t *testing.T) { - const rego = "package main\nimport rego.v1\nviolations contains msg if { false; msg := \"\" }\n" - - kindless := &schemaapi.Policy{ - ApiVersion: "chainloop.dev/v1", - Kind: "Policy", - Metadata: &schemaapi.Metadata{Name: "kindless"}, - Spec: &schemaapi.PolicySpec{ - Policies: []*schemaapi.PolicySpecV2{{Source: &schemaapi.PolicySpecV2_Embedded{Embedded: rego}}}, - }, - } - - testCases := []struct { - name string - selector *schemaapi.PolicyAttachment_MaterialSelector - wantErrs int - }{ - { - name: "kind-less policy without selector", - wantErrs: 1, - }, - { - name: "kind-less policy with selector", - selector: &schemaapi.PolicyAttachment_MaterialSelector{Name: "image"}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - contract := &schemaapi.CraftingSchemaV2{ - ApiVersion: "chainloop.dev/v1", - Kind: "Contract", - Metadata: &schemaapi.Metadata{Name: "contract"}, - Spec: &schemaapi.CraftingSchemaV2Spec{ - Materials: []*schemaapi.CraftingSchema_Material{ - {Name: "image", Type: schemaapi.CraftingSchema_Material_CONTAINER_IMAGE}, - }, - Policies: &schemaapi.Policies{ - Materials: []*schemaapi.PolicyAttachment{ - {Policy: &schemaapi.PolicyAttachment_Embedded{Embedded: kindless}, Selector: tc.selector}, - }, - }, - }, - } - - c, err := newInitializedCrafterV2(t, contract, &v1.WorkflowMetadata{Name: "wf"}, true, "", nil) - require.NoError(t, err) - - errs := c.ValidateMaterialPolicyAttachments(context.Background()) - assert.Len(t, errs, tc.wantErrs) - for _, err := range errs { - assert.ErrorIs(t, err, policies.ErrAttachmentNeverApplies) - } - }) - } -} diff --git a/pkg/policies/attachment_validation.go b/pkg/policies/attachment_validation.go index 03da31c96..49bd97f8b 100644 --- a/pkg/policies/attachment_validation.go +++ b/pkg/policies/attachment_validation.go @@ -16,7 +16,6 @@ package policies import ( - "context" "errors" "fmt" @@ -64,24 +63,3 @@ func ValidateMaterialAttachment(policy *v1.Policy, att *v1.PolicyAttachment, mat return nil } - -// ValidateMaterialAttachments loads the policy of each material attachment and -// returns one error for each attachment that can never apply to a material. -// See ValidateMaterialAttachment. Attachments whose policy can't be loaded are -// skipped, since the load error is reported when the policy is evaluated. -func (pv *PolicyVerifier) ValidateMaterialAttachments(ctx context.Context, materials []*v1.CraftingSchema_Material) []error { - var errs []error - for _, att := range pv.policies.GetMaterials() { - spec, _, err := pv.loadPolicySpec(ctx, att) - if err != nil { - pv.logger.Debug().Err(err).Str("policy", att.GetRef()).Msg("skipping attachment validation: failed to load policy") - continue - } - - if err := ValidateMaterialAttachment(spec, att, materials); err != nil { - errs = append(errs, err) - } - } - - return errs -} diff --git a/pkg/policies/attachment_validation_test.go b/pkg/policies/attachment_validation_test.go index 5d0c2d183..af79b3890 100644 --- a/pkg/policies/attachment_validation_test.go +++ b/pkg/policies/attachment_validation_test.go @@ -16,21 +16,16 @@ package policies import ( - "context" "testing" v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) const ( - materialSBOM = "sbom" - materialImage = "image" - materialCustom = "custom-material" - refSBOMSyft = "file://testdata/sbom_syft.yaml" - refSBOMSyftNotTyped = "file://testdata/sbom_syft_not_typed.yaml" + materialSBOM = "sbom" + materialImage = "image" ) func TestValidateMaterialAttachment(t *testing.T) { @@ -148,57 +143,3 @@ func TestValidateMaterialAttachment(t *testing.T) { }) } } - -func TestPolicyVerifierValidateMaterialAttachments(t *testing.T) { - logger := zerolog.Nop() - materials := []*v1.CraftingSchema_Material{ - {Name: materialCustom, Type: v1.CraftingSchema_Material_ARTIFACT}, - } - - testCases := []struct { - name string - attachments []*v1.PolicyAttachment - wantErrs int - }{ - { - name: "valid attachments", - attachments: []*v1.PolicyAttachment{ - {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyft}}, - { - Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyftNotTyped}, - Selector: &v1.PolicyAttachment_MaterialSelector{Name: materialCustom}, - }, - }, - }, - { - name: "kind-less policy without selector", - attachments: []*v1.PolicyAttachment{ - {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyft}}, - {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyftNotTyped}}, - }, - wantErrs: 1, - }, - { - name: "several attachments that never apply", - attachments: []*v1.PolicyAttachment{ - {Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyftNotTyped}}, - { - Policy: &v1.PolicyAttachment_Ref{Ref: refSBOMSyft}, - Selector: &v1.PolicyAttachment_MaterialSelector{Name: materialCustom}, - }, - }, - wantErrs: 2, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - pv := NewPolicyVerifier(&v1.Policies{Materials: tc.attachments}, nil, &logger) - errs := pv.ValidateMaterialAttachments(context.TODO(), materials) - assert.Len(t, errs, tc.wantErrs) - for _, err := range errs { - assert.ErrorIs(t, err, ErrAttachmentNeverApplies) - } - }) - } -} From d083e12ab6a8da0334d14701f27559289320018f Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 4 Nov 2026 14:16:26 +0100 Subject: [PATCH 5/8] refactor(controlplane): always check material policy applicability FindOrCreateWorkflow also creates or updates the contract when contract bytes are sent, so validate it there too, and drop the WithoutMaterialApplicabilityCheck option. Refs #3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c --- .../internal/service/attestation.go | 5 ++- app/controlplane/pkg/biz/workflowcontract.go | 35 ++++--------------- .../pkg/biz/workflowcontract_policies_test.go | 14 +------- 3 files changed, 10 insertions(+), 44 deletions(-) diff --git a/app/controlplane/internal/service/attestation.go b/app/controlplane/internal/service/attestation.go index 46bba41e8..f76bedea5 100644 --- a/app/controlplane/internal/service/attestation.go +++ b/app/controlplane/internal/service/attestation.go @@ -807,10 +807,9 @@ func (s *AttestationService) FindOrCreateWorkflow(ctx context.Context, req *cpAP return nil, err } - // contract validation. The contract is re-sent on every attestation init, where it can't be fixed, - // so the material policy applicability check only runs when the contract is created or updated. + // contract validation if req.GetContractBytes() != nil { - if err = s.workflowContractUseCase.ValidateContractPolicies(ctx, req.GetContractBytes(), token, nil, nil, biz.WithoutMaterialApplicabilityCheck()); err != nil { + if err = s.workflowContractUseCase.ValidateContractPolicies(ctx, req.GetContractBytes(), token, nil, nil); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) } } diff --git a/app/controlplane/pkg/biz/workflowcontract.go b/app/controlplane/pkg/biz/workflowcontract.go index e34adc783..6ff840a20 100644 --- a/app/controlplane/pkg/biz/workflowcontract.go +++ b/app/controlplane/pkg/biz/workflowcontract.go @@ -503,33 +503,12 @@ func (uc *WorkflowContractUseCase) RevisionWouldChange(ctx context.Context, orgI return !bytes.Equal(latest.Version.Schema.Raw, incoming.Raw), nil } -// ValidateContractPoliciesOpt configures ValidateContractPolicies -type ValidateContractPoliciesOpt func(*validateContractPoliciesOpts) - -type validateContractPoliciesOpts struct { - skipMaterialApplicability bool -} - -// WithoutMaterialApplicabilityCheck skips the check that rejects material policy attachments -// that can never apply. Use it where an existing contract is re-sent, for example on attestation -// init, where the contract can't be fixed. The check runs when the contract is created or updated. -func WithoutMaterialApplicabilityCheck() ValidateContractPoliciesOpt { - return func(o *validateContractPoliciesOpts) { - o.skipMaterialApplicability = true - } -} - // ValidateContractPolicies checks that policies and policy groups referenced by the contract // exist. batchPolicyNames and batchPolicyGroupNames carry the names of resources being // created/updated in the same batch apply: references to those are treated as known instead of // being resolved against the registry, since they may not be persisted yet (e.g. during // dry-run). Remote references are still validated regardless. -func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, rawSchema []byte, token string, batchPolicyNames, batchPolicyGroupNames []string, opts ...ValidateContractPoliciesOpt) error { - var options validateContractPoliciesOpts - for _, opt := range opts { - opt(&options) - } - +func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, rawSchema []byte, token string, batchPolicyNames, batchPolicyGroupNames []string) error { // Validate that externally provided policies exist c, err := identifyUnMarshalAndValidateRawContract(rawSchema) if err != nil { @@ -547,7 +526,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, } } for _, att := range schema.GetPolicies().GetMaterials() { - if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, schema.GetMaterials(), !options.skipMaterialApplicability); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, schema.GetMaterials()); err != nil { return NewErrValidation(err) } } @@ -566,7 +545,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, } } for _, att := range spec.GetPolicies().GetMaterials() { - if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, spec.GetMaterials(), !options.skipMaterialApplicability); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, spec.GetMaterials()); err != nil { return NewErrValidation(err) } } @@ -646,16 +625,16 @@ func (uc *WorkflowContractUseCase) findAndValidatePolicy(ctx context.Context, at return policy, nil } -// findAndValidateMaterialPolicy validates a material policy attachment and, when checkApplicability -// is set and its policy can be resolved, checks that the attachment can apply to at least one material. -func (uc *WorkflowContractUseCase) findAndValidateMaterialPolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, materials []*schemav1.CraftingSchema_Material, checkApplicability bool) error { +// findAndValidateMaterialPolicy validates a material policy attachment and, when its +// policy can be resolved, checks that the attachment can apply to at least one material. +func (uc *WorkflowContractUseCase) findAndValidateMaterialPolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, materials []*schemav1.CraftingSchema_Material) error { policy, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames) if err != nil { return err } // the policy might not be available in this context (local or batch-local policy) - if policy == nil || !checkApplicability { + if policy == nil { return nil } diff --git a/app/controlplane/pkg/biz/workflowcontract_policies_test.go b/app/controlplane/pkg/biz/workflowcontract_policies_test.go index 1288355e5..77fd2b418 100644 --- a/app/controlplane/pkg/biz/workflowcontract_policies_test.go +++ b/app/controlplane/pkg/biz/workflowcontract_policies_test.go @@ -105,7 +105,6 @@ func TestValidateContractPoliciesMaterialApplicability(t *testing.T) { testCases := []struct { name string contract []byte - opts []ValidateContractPoliciesOpt wantErr string }{ { @@ -134,22 +133,11 @@ policies: `), wantErr: `policy "kindless" applies to any material type and requires a material selector (selector.name)`, }, - { - name: "kind-less policy without selector, applicability check skipped", - contract: contractWithMaterialPolicy("kindless", ""), - opts: []ValidateContractPoliciesOpt{WithoutMaterialApplicabilityCheck()}, - }, - { - name: "missing policy is still rejected when the applicability check is skipped", - contract: contractWithMaterialPolicy("not-found", ""), - opts: []ValidateContractPoliciesOpt{WithoutMaterialApplicabilityCheck()}, - wantErr: `policy "not-found"`, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", nil, nil, tc.opts...) + err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", nil, nil) if tc.wantErr == "" { assert.NoError(t, err) return From b9b96020524c9f72c38a40ed385be4338cce15d3 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 4 Nov 2026 14:20:06 +0100 Subject: [PATCH 6/8] refactor(policies): share the material match rules with validation Validation now uses canMatchAnyMaterial and nameMatches, the same helpers as evaluation, instead of repeating their conditions. It only checks contract materials for exact selectors, so a new match mode is never treated as an exact match. Drop duplicate test cases. Refs #3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c --- pkg/policies/attachment_validation.go | 21 +++++++-- pkg/policies/attachment_validation_test.go | 50 ++++++++-------------- pkg/policies/policies.go | 9 +++- 3 files changed, 43 insertions(+), 37 deletions(-) diff --git a/pkg/policies/attachment_validation.go b/pkg/policies/attachment_validation.go index 49bd97f8b..aadf3d4a3 100644 --- a/pkg/policies/attachment_validation.go +++ b/pkg/policies/attachment_validation.go @@ -45,20 +45,33 @@ func ValidateMaterialAttachment(policy *v1.Policy, att *v1.PolicyAttachment, mat specTypes := getPolicyTypes(policy) selector := att.GetSelector() - if len(specTypes) == 0 && selector.GetName() == "" { + if !canMatchAnyMaterial(specTypes, selector) { return fmt.Errorf("%w: policy %q applies to any material type and requires a material selector (selector.name)", ErrAttachmentNeverApplies, policyName) } - // a prefix selector can match materials added at attestation time - if selector.GetName() == "" || selector.GetMatchMode() == v1.PolicyAttachment_MaterialSelector_PREFIX { + // Only an exact selector names a single material. Other match modes, such as PREFIX, + // can match materials added at attestation time. + switch selector.GetMatchMode() { + case v1.PolicyAttachment_MaterialSelector_UNSPECIFIED, v1.PolicyAttachment_MaterialSelector_EXACT: + default: + return nil + } + + if selector.GetName() == "" { return nil } for _, m := range materials { - if m.GetName() == selector.GetName() && !attachmentApplies(specTypes, selector, m.GetName(), m.GetType()) { + if !nameMatches(selector.GetName(), selector.GetMatchMode(), m.GetName()) { + continue + } + + // material names are unique in a contract + if !attachmentApplies(specTypes, selector, m.GetName(), m.GetType()) { return fmt.Errorf("%w: policy %q does not support material %q of type %s (supported types: %v)", ErrAttachmentNeverApplies, policyName, m.GetName(), m.GetType(), specTypes) } + return nil } return nil diff --git a/pkg/policies/attachment_validation_test.go b/pkg/policies/attachment_validation_test.go index af79b3890..9151dd844 100644 --- a/pkg/policies/attachment_validation_test.go +++ b/pkg/policies/attachment_validation_test.go @@ -55,28 +55,22 @@ func TestValidateMaterialAttachment(t *testing.T) { } testCases := []struct { - name string - policy *v1.Policy - selector *v1.PolicyAttachment_MaterialSelector - wantErrIs error - wantMsg string + name string + policy *v1.Policy + selector *v1.PolicyAttachment_MaterialSelector + wantErr bool + wantMsg string }{ { - name: "kind-less policy without selector", - policy: kindless, - wantErrIs: ErrAttachmentNeverApplies, - wantMsg: `policy attachment never applies: policy "upstream-workflow-attested" applies to any material type and requires a material selector (selector.name)`, + name: "kind-less policy without selector", + policy: kindless, + wantErr: true, + wantMsg: `policy attachment never applies: policy "upstream-workflow-attested" applies to any material type and requires a material selector (selector.name)`, }, { - name: "kind-less v2 policy without selector", - policy: kindlessV2, - wantErrIs: ErrAttachmentNeverApplies, - }, - { - name: "kind-less policy with empty selector name", - policy: kindless, - selector: &v1.PolicyAttachment_MaterialSelector{}, - wantErrIs: ErrAttachmentNeverApplies, + name: "kind-less v2 policy without selector", + policy: kindlessV2, + wantErr: true, }, { name: "kind-less policy with selector", @@ -98,17 +92,11 @@ func TestValidateMaterialAttachment(t *testing.T) { selector: &v1.PolicyAttachment_MaterialSelector{Name: materialSBOM}, }, { - name: "typed policy with selector of an unsupported contract material", - policy: sbomPolicy, - selector: &v1.PolicyAttachment_MaterialSelector{Name: materialImage}, - wantErrIs: ErrAttachmentNeverApplies, - wantMsg: `policy attachment never applies: policy "sbom-policy" does not support material "image" of type CONTAINER_IMAGE (supported types: [SBOM_SPDX_JSON SBOM_CYCLONEDX_JSON])`, - }, - { - name: "typed policy with explicit exact selector of an unsupported contract material", - policy: sbomPolicy, - selector: &v1.PolicyAttachment_MaterialSelector{Name: materialImage, MatchMode: v1.PolicyAttachment_MaterialSelector_EXACT}, - wantErrIs: ErrAttachmentNeverApplies, + name: "typed policy with selector of an unsupported contract material", + policy: sbomPolicy, + selector: &v1.PolicyAttachment_MaterialSelector{Name: materialImage}, + wantErr: true, + wantMsg: `policy attachment never applies: policy "sbom-policy" does not support material "image" of type CONTAINER_IMAGE (supported types: [SBOM_SPDX_JSON SBOM_CYCLONEDX_JSON])`, }, { name: "typed policy with prefix selector is not checked against contract materials", @@ -130,13 +118,13 @@ func TestValidateMaterialAttachment(t *testing.T) { } err := ValidateMaterialAttachment(tc.policy, att, materials) - if tc.wantErrIs == nil { + if !tc.wantErr { assert.NoError(t, err) return } require.Error(t, err) - assert.ErrorIs(t, err, tc.wantErrIs) + assert.ErrorIs(t, err, ErrAttachmentNeverApplies) if tc.wantMsg != "" { assert.EqualError(t, err, tc.wantMsg) } diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index 6ecd91415..8b2e3fc79 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -1032,8 +1032,13 @@ func attachmentApplies(specTypes []v1.CraftingSchema_Material_MaterialType, sele return false } - // no type nor name to match, we can't guess anything - return len(specTypes) > 0 || filteredName != "" + return canMatchAnyMaterial(specTypes, selector) +} + +// canMatchAnyMaterial reports whether an attachment has something to match materials on. +// A policy without types and an attachment without a selector name can't match any material. +func canMatchAnyMaterial(specTypes []v1.CraftingSchema_Material_MaterialType, selector *v1.PolicyAttachment_MaterialSelector) bool { + return len(specTypes) > 0 || selector.GetName() != "" } // nameMatches reports whether a material name satisfies a selector/group-material From 48e466a3875712b60e4a1596649f8c57cb2cae06 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 5 Nov 2026 03:18:57 +0100 Subject: [PATCH 7/8] fix(controlplane): validate material policy applicability for batch-local dry runs Accept the raw specs of batch policies in the apply request and use them to run the same applicability and input checks as a real apply. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 2df578c9-f7ba-441c-8f53-410c372821f2 --- .../controlplane/v1/workflow_contract.pb.go | 20 +++++-- .../controlplane/v1/workflow_contract.proto | 4 ++ .../controlplane/v1/workflow_contract.ts | 33 +++++++++++- ...ontractServiceApplyRequest.jsonschema.json | 16 ++++++ ...lowContractServiceApplyRequest.schema.json | 16 ++++++ .../internal/service/attestation.go | 2 +- .../internal/service/workflowcontract.go | 39 ++++++++++++-- app/controlplane/pkg/biz/workflowcontract.go | 52 +++++++++--------- .../pkg/biz/workflowcontract_policies_test.go | 53 ++++++++++++++++++- 9 files changed, 201 insertions(+), 34 deletions(-) diff --git a/app/controlplane/api/controlplane/v1/workflow_contract.pb.go b/app/controlplane/api/controlplane/v1/workflow_contract.pb.go index 7e781bb3a..2ef55df0c 100644 --- a/app/controlplane/api/controlplane/v1/workflow_contract.pb.go +++ b/app/controlplane/api/controlplane/v1/workflow_contract.pb.go @@ -580,8 +580,12 @@ type WorkflowContractServiceApplyRequest struct { BatchPolicyNames []string `protobuf:"bytes,3,rep,name=batch_policy_names,json=batchPolicyNames,proto3" json:"batch_policy_names,omitempty"` // Same as batch_policy_names, for policy groups created/updated in the same batch. BatchPolicyGroupNames []string `protobuf:"bytes,4,rep,name=batch_policy_group_names,json=batchPolicyGroupNames,proto3" json:"batch_policy_group_names,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Raw representation (json or yaml) of the policies created/updated in the same batch apply. + // When present, the specs of batch-local references are used to validate their inputs and + // whether the attachment can apply to a material, as a real apply does. + BatchPolicies [][]byte `protobuf:"bytes,5,rep,name=batch_policies,json=batchPolicies,proto3" json:"batch_policies,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *WorkflowContractServiceApplyRequest) Reset() { @@ -642,6 +646,13 @@ func (x *WorkflowContractServiceApplyRequest) GetBatchPolicyGroupNames() []strin return nil } +func (x *WorkflowContractServiceApplyRequest) GetBatchPolicies() [][]byte { + if x != nil { + return x.BatchPolicies + } + return nil +} + type WorkflowContractServiceApplyResponse struct { state protoimpl.MessageState `protogen:"open.v1"` Result *WorkflowContractItem `protobuf:"bytes,1,opt,name=result,proto3" json:"result,omitempty"` @@ -872,13 +883,14 @@ const file_controlplane_v1_workflow_contract_proto_rawDesc = "" + "$WorkflowContractServiceDeleteRequest\x12\x97\x01\n" + "\x04name\x18\x01 \x01(\tB\x82\x01\xbaH\x7f\xba\x01|\n" + "\rname.dns-1123\x12:must contain only lowercase letters, numbers, and hyphens.\x1a/this.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$')R\x04name\"'\n" + - "%WorkflowContractServiceDeleteResponse\"\xc4\x01\n" + + "%WorkflowContractServiceDeleteResponse\"\xeb\x01\n" + "#WorkflowContractServiceApplyRequest\x12\x1d\n" + "\n" + "raw_schema\x18\x01 \x01(\fR\trawSchema\x12\x17\n" + "\adry_run\x18\x02 \x01(\bR\x06dryRun\x12,\n" + "\x12batch_policy_names\x18\x03 \x03(\tR\x10batchPolicyNames\x127\n" + - "\x18batch_policy_group_names\x18\x04 \x03(\tR\x15batchPolicyGroupNames\"\xa4\x03\n" + + "\x18batch_policy_group_names\x18\x04 \x03(\tR\x15batchPolicyGroupNames\x12%\n" + + "\x0ebatch_policies\x18\x05 \x03(\fR\rbatchPolicies\"\xa4\x03\n" + "$WorkflowContractServiceApplyResponse\x12=\n" + "\x06result\x18\x01 \x01(\v2%.controlplane.v1.WorkflowContractItemR\x06result\x12 \n" + "\tunchanged\x18\x02 \x01(\bB\x02\x18\x01R\tunchanged\x12\x18\n" + diff --git a/app/controlplane/api/controlplane/v1/workflow_contract.proto b/app/controlplane/api/controlplane/v1/workflow_contract.proto index 4169bdd83..546c1fd55 100644 --- a/app/controlplane/api/controlplane/v1/workflow_contract.proto +++ b/app/controlplane/api/controlplane/v1/workflow_contract.proto @@ -127,6 +127,10 @@ message WorkflowContractServiceApplyRequest { repeated string batch_policy_names = 3; // Same as batch_policy_names, for policy groups created/updated in the same batch. repeated string batch_policy_group_names = 4; + // Raw representation (json or yaml) of the policies created/updated in the same batch apply. + // When present, the specs of batch-local references are used to validate their inputs and + // whether the attachment can apply to a material, as a real apply does. + repeated bytes batch_policies = 5; } message WorkflowContractServiceApplyResponse { diff --git a/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts b/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts index c60d110e9..1711ee739 100644 --- a/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts +++ b/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts @@ -79,6 +79,12 @@ export interface WorkflowContractServiceApplyRequest { batchPolicyNames: string[]; /** Same as batch_policy_names, for policy groups created/updated in the same batch. */ batchPolicyGroupNames: string[]; + /** + * Raw representation (json or yaml) of the policies created/updated in the same batch apply. + * When present, the specs of batch-local references are used to validate their inputs and + * whether the attachment can apply to a material, as a real apply does. + */ + batchPolicies: Uint8Array[]; } export interface WorkflowContractServiceApplyResponse { @@ -1009,7 +1015,13 @@ export const WorkflowContractServiceDeleteResponse = { }; function createBaseWorkflowContractServiceApplyRequest(): WorkflowContractServiceApplyRequest { - return { rawSchema: new Uint8Array(0), dryRun: false, batchPolicyNames: [], batchPolicyGroupNames: [] }; + return { + rawSchema: new Uint8Array(0), + dryRun: false, + batchPolicyNames: [], + batchPolicyGroupNames: [], + batchPolicies: [], + }; } export const WorkflowContractServiceApplyRequest = { @@ -1026,6 +1038,9 @@ export const WorkflowContractServiceApplyRequest = { for (const v of message.batchPolicyGroupNames) { writer.uint32(34).string(v!); } + for (const v of message.batchPolicies) { + writer.uint32(42).bytes(v!); + } return writer; }, @@ -1064,6 +1079,13 @@ export const WorkflowContractServiceApplyRequest = { message.batchPolicyGroupNames.push(reader.string()); continue; + case 5: + if (tag !== 42) { + break; + } + + message.batchPolicies.push(reader.bytes()); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -1083,6 +1105,9 @@ export const WorkflowContractServiceApplyRequest = { batchPolicyGroupNames: Array.isArray(object?.batchPolicyGroupNames) ? object.batchPolicyGroupNames.map((e: any) => String(e)) : [], + batchPolicies: Array.isArray(object?.batchPolicies) + ? object.batchPolicies.map((e: any) => bytesFromBase64(e)) + : [], }; }, @@ -1101,6 +1126,11 @@ export const WorkflowContractServiceApplyRequest = { } else { obj.batchPolicyGroupNames = []; } + if (message.batchPolicies) { + obj.batchPolicies = message.batchPolicies.map((e) => base64FromBytes(e !== undefined ? e : new Uint8Array(0))); + } else { + obj.batchPolicies = []; + } return obj; }, @@ -1118,6 +1148,7 @@ export const WorkflowContractServiceApplyRequest = { message.dryRun = object.dryRun ?? false; message.batchPolicyNames = object.batchPolicyNames?.map((e) => e) || []; message.batchPolicyGroupNames = object.batchPolicyGroupNames?.map((e) => e) || []; + message.batchPolicies = object.batchPolicies?.map((e) => e) || []; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json index 1824bcc9e..a02b23375 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json @@ -3,6 +3,14 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, "patternProperties": { + "^(batch_policies)$": { + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "items": { + "pattern": "^[A-Za-z0-9+/]*={0,2}$", + "type": "string" + }, + "type": "array" + }, "^(batch_policy_group_names)$": { "description": "Same as batch_policy_names, for policy groups created/updated in the same batch.", "items": { @@ -28,6 +36,14 @@ } }, "properties": { + "batchPolicies": { + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "items": { + "pattern": "^[A-Za-z0-9+/]*={0,2}$", + "type": "string" + }, + "type": "array" + }, "batchPolicyGroupNames": { "description": "Same as batch_policy_names, for policy groups created/updated in the same batch.", "items": { diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json index 15856b96a..bb19fdd8c 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json @@ -3,6 +3,14 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, "patternProperties": { + "^(batchPolicies)$": { + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "items": { + "pattern": "^[A-Za-z0-9+/]*={0,2}$", + "type": "string" + }, + "type": "array" + }, "^(batchPolicyGroupNames)$": { "description": "Same as batch_policy_names, for policy groups created/updated in the same batch.", "items": { @@ -28,6 +36,14 @@ } }, "properties": { + "batch_policies": { + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "items": { + "pattern": "^[A-Za-z0-9+/]*={0,2}$", + "type": "string" + }, + "type": "array" + }, "batch_policy_group_names": { "description": "Same as batch_policy_names, for policy groups created/updated in the same batch.", "items": { diff --git a/app/controlplane/internal/service/attestation.go b/app/controlplane/internal/service/attestation.go index f76bedea5..c22b643b4 100644 --- a/app/controlplane/internal/service/attestation.go +++ b/app/controlplane/internal/service/attestation.go @@ -809,7 +809,7 @@ func (s *AttestationService) FindOrCreateWorkflow(ctx context.Context, req *cpAP // contract validation if req.GetContractBytes() != nil { - if err = s.workflowContractUseCase.ValidateContractPolicies(ctx, req.GetContractBytes(), token, nil, nil); err != nil { + if err = s.workflowContractUseCase.ValidateContractPolicies(ctx, req.GetContractBytes(), token, nil, nil, nil); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) } } diff --git a/app/controlplane/internal/service/workflowcontract.go b/app/controlplane/internal/service/workflowcontract.go index f1530e56b..29cab6008 100644 --- a/app/controlplane/internal/service/workflowcontract.go +++ b/app/controlplane/internal/service/workflowcontract.go @@ -146,7 +146,7 @@ func (s *WorkflowContractService) Create(ctx context.Context, req *pb.WorkflowCo } if len(req.RawContract) != 0 { - if err = s.contractUseCase.ValidateContractPolicies(ctx, req.RawContract, token, nil, nil); err != nil { + if err = s.contractUseCase.ValidateContractPolicies(ctx, req.RawContract, token, nil, nil, nil); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) } } @@ -214,7 +214,7 @@ func (s *WorkflowContractService) Update(ctx context.Context, req *pb.WorkflowCo // Validate the contract policies if the raw contract is provided if len(req.RawContract) != 0 { - if err = s.contractUseCase.ValidateContractPolicies(ctx, req.RawContract, token, nil, nil); err != nil { + if err = s.contractUseCase.ValidateContractPolicies(ctx, req.RawContract, token, nil, nil, nil); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) } } @@ -259,12 +259,17 @@ func (s *WorkflowContractService) Apply(ctx context.Context, req *pb.WorkflowCon // the resources may not be persisted yet. A real apply persists batch resources before the // contract, so it must always validate fully and never trust the client-supplied batch lists. var batchPolicyNames, batchPolicyGroupNames []string + var batchPolicies map[string]*schemav1.Policy if dryRun { batchPolicyNames = req.GetBatchPolicyNames() batchPolicyGroupNames = req.GetBatchPolicyGroupNames() + + if batchPolicies, err = parseBatchPolicies(req.GetBatchPolicies()); err != nil { + return nil, err + } } - if err = s.contractUseCase.ValidateContractPolicies(ctx, req.RawSchema, token, batchPolicyNames, batchPolicyGroupNames); err != nil { + if err = s.contractUseCase.ValidateContractPolicies(ctx, req.RawSchema, token, batchPolicyNames, batchPolicyGroupNames, batchPolicies); err != nil { return nil, handleUseCaseErr(ctx, err, s.log) } @@ -527,6 +532,34 @@ func validateAndExtractMetadata(rawContract []byte, explicitName, explicitDesc s } // extractMetadata attempts to extract the name from metadata.name in v2 contracts +// parseBatchPolicies parses the raw policies sent with a batch apply, indexed by policy name +func parseBatchPolicies(raws [][]byte) (map[string]*schemav1.Policy, error) { + if len(raws) == 0 { + return nil, nil + } + + parsed := make(map[string]*schemav1.Policy, len(raws)) + for _, raw := range raws { + format, err := unmarshal.IdentifyFormat(raw) + if err != nil { + return nil, errors.BadRequest("invalid", fmt.Sprintf("invalid batch policy: %s", err)) + } + + policy := &schemav1.Policy{} + if err := unmarshal.FromRaw(raw, format, policy, false); err != nil { + return nil, errors.BadRequest("invalid", fmt.Sprintf("invalid batch policy: %s", err)) + } + + if policy.GetMetadata().GetName() == "" { + return nil, errors.BadRequest("invalid", "invalid batch policy: missing metadata name") + } + + parsed[policy.GetMetadata().GetName()] = policy + } + + return parsed, nil +} + func extractMetadata(rawContract []byte) (*schemav1.Metadata, error) { if len(rawContract) == 0 { return nil, nil diff --git a/app/controlplane/pkg/biz/workflowcontract.go b/app/controlplane/pkg/biz/workflowcontract.go index 6ff840a20..3c72aa154 100644 --- a/app/controlplane/pkg/biz/workflowcontract.go +++ b/app/controlplane/pkg/biz/workflowcontract.go @@ -507,8 +507,9 @@ func (uc *WorkflowContractUseCase) RevisionWouldChange(ctx context.Context, orgI // exist. batchPolicyNames and batchPolicyGroupNames carry the names of resources being // created/updated in the same batch apply: references to those are treated as known instead of // being resolved against the registry, since they may not be persisted yet (e.g. during -// dry-run). Remote references are still validated regardless. -func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, rawSchema []byte, token string, batchPolicyNames, batchPolicyGroupNames []string) error { +// dry-run). Remote references are still validated regardless. batchPolicies optionally carries the +// specs of those policies by name, so their inputs and material applicability are validated too. +func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, rawSchema []byte, token string, batchPolicyNames, batchPolicyGroupNames []string, batchPolicies map[string]*schemav1.Policy) error { // Validate that externally provided policies exist c, err := identifyUnMarshalAndValidateRawContract(rawSchema) if err != nil { @@ -521,12 +522,12 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, // DEPRECATED: v1 schema is deprecated, use v2 Contract format instead schema := c.Schema for _, att := range schema.GetPolicies().GetAttestation() { - if _, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames); err != nil { + if _, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames, batchPolicies); err != nil { return NewErrValidation(err) } } for _, att := range schema.GetPolicies().GetMaterials() { - if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, schema.GetMaterials()); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, batchPolicies, schema.GetMaterials()); err != nil { return NewErrValidation(err) } } @@ -540,12 +541,12 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(ctx context.Context, spec := c.Schemav2.GetSpec() if spec.GetPolicies() != nil { for _, att := range spec.GetPolicies().GetAttestation() { - if _, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames); err != nil { + if _, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames, batchPolicies); err != nil { return NewErrValidation(err) } } for _, att := range spec.GetPolicies().GetMaterials() { - if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, spec.GetMaterials()); err != nil { + if err := uc.findAndValidateMaterialPolicy(ctx, att, token, batchPolicyNames, batchPolicies, spec.GetMaterials()); err != nil { return NewErrValidation(err) } } @@ -578,7 +579,7 @@ func (uc *WorkflowContractUseCase) ValidatePolicyAttachment(ctx context.Context, return nil } -func (uc *WorkflowContractUseCase) findAndValidatePolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string) (*schemav1.Policy, error) { +func (uc *WorkflowContractUseCase) findAndValidatePolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, batchPolicies map[string]*schemav1.Policy) (*schemav1.Policy, error) { var policy *schemav1.Policy if att.GetEmbedded() != nil { @@ -593,21 +594,24 @@ func (uc *WorkflowContractUseCase) findAndValidatePolicy(ctx context.Context, at // treat references to them as known instead of resolving against the registry. Only // bare references (no provider/org) can name a batch-local policy; references that // explicitly target a remote provider or org are always validated. - if pr.Provider == "" && pr.OrgName == "" && slices.Contains(batchPolicyNames, pr.Name) { - return nil, nil - } - // Validate attachment - if err := uc.ValidatePolicyAttachment(ctx, pr.Provider, att, token); err != nil { - return nil, err - } + if pr.Provider == "" && pr.OrgName == "" && (slices.Contains(batchPolicyNames, pr.Name) || batchPolicies[pr.Name] != nil) { + // Use the batch spec when the caller sent it, so the same checks as a real apply + // run. Without it the policy is only known and nothing else can be validated. + policy = batchPolicies[pr.Name] + } else { + // Validate attachment + if err := uc.ValidatePolicyAttachment(ctx, pr.Provider, att, token); err != nil { + return nil, err + } - // The control plane is a non-CLI caller, so resolve the true latest revision - // and skip CLI-version compatibility gating. - remotePolicy, err := uc.GetPolicy(ctx, pr.Provider, pr.Name, pr.OrgName, "", token, policies.WithIgnoreCLICompatibility()) - if err != nil { - return nil, err + // The control plane is a non-CLI caller, so resolve the true latest revision + // and skip CLI-version compatibility gating. + remotePolicy, err := uc.GetPolicy(ctx, pr.Provider, pr.Name, pr.OrgName, "", token, policies.WithIgnoreCLICompatibility()) + if err != nil { + return nil, err + } + policy = remotePolicy.Policy } - policy = remotePolicy.Policy } if policy != nil { @@ -627,8 +631,8 @@ func (uc *WorkflowContractUseCase) findAndValidatePolicy(ctx context.Context, at // findAndValidateMaterialPolicy validates a material policy attachment and, when its // policy can be resolved, checks that the attachment can apply to at least one material. -func (uc *WorkflowContractUseCase) findAndValidateMaterialPolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, materials []*schemav1.CraftingSchema_Material) error { - policy, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames) +func (uc *WorkflowContractUseCase) findAndValidateMaterialPolicy(ctx context.Context, att *schemav1.PolicyAttachment, token string, batchPolicyNames []string, batchPolicies map[string]*schemav1.Policy, materials []*schemav1.CraftingSchema_Material) error { + policy, err := uc.findAndValidatePolicy(ctx, att, token, batchPolicyNames, batchPolicies) if err != nil { return err } @@ -702,7 +706,7 @@ func (uc *WorkflowContractUseCase) validateSkipList(ctx context.Context, group * for _, groupMaterial := range policies.GetMaterials() { for _, policyAtt := range groupMaterial.GetPolicies() { // Group member policies come from an already-resolved remote group, never batch-local. - policy, err := uc.findAndValidatePolicy(ctx, policyAtt, token, nil) + policy, err := uc.findAndValidatePolicy(ctx, policyAtt, token, nil, nil) if err != nil { return fmt.Errorf("failed to get policy name during skip list validation: %w", err) } @@ -712,7 +716,7 @@ func (uc *WorkflowContractUseCase) validateSkipList(ctx context.Context, group * // Collect attestation policy names for _, policyAtt := range policies.GetAttestation() { - policy, err := uc.findAndValidatePolicy(ctx, policyAtt, token, nil) + policy, err := uc.findAndValidatePolicy(ctx, policyAtt, token, nil, nil) if err != nil { return fmt.Errorf("failed to get policy name during skip list validation: %w", err) } diff --git a/app/controlplane/pkg/biz/workflowcontract_policies_test.go b/app/controlplane/pkg/biz/workflowcontract_policies_test.go index 77fd2b418..55c17f260 100644 --- a/app/controlplane/pkg/biz/workflowcontract_policies_test.go +++ b/app/controlplane/pkg/biz/workflowcontract_policies_test.go @@ -24,12 +24,15 @@ import ( "path" "testing" + schemav1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" cpolicies "github.com/chainloop-dev/chainloop/app/controlplane/pkg/policies" "github.com/go-kratos/kratos/v2/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +const batchKindless = "batch-kindless" + // remote policies served by the fake policy provider, by name var providerPolicies = map[string]string{ "kindless": `apiVersion: chainloop.dev/v1 @@ -137,7 +140,55 @@ policies: for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", nil, nil) + err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", nil, nil, nil) + if tc.wantErr == "" { + assert.NoError(t, err) + return + } + + require.Error(t, err) + assert.True(t, IsErrValidation(err)) + assert.ErrorContains(t, err, tc.wantErr) + }) + } +} + +func TestValidateContractPoliciesBatchLocalApplicability(t *testing.T) { + uc := NewWorkflowContractUseCase(nil, newFakePolicyProvider(t), nil, log.DefaultLogger) + + kindless := &schemav1.Policy{ + Metadata: &schemav1.Metadata{Name: batchKindless}, + Spec: &schemav1.PolicySpec{ + Policies: []*schemav1.PolicySpecV2{{Source: &schemav1.PolicySpecV2_Embedded{Embedded: "package main"}}}, + }, + } + + testCases := []struct { + name string + contract []byte + specs map[string]*schemav1.Policy + wantErr string + }{ + { + name: "batch-local kind-less policy without selector", + contract: contractWithMaterialPolicy(batchKindless, ""), + specs: map[string]*schemav1.Policy{batchKindless: kindless}, + wantErr: `policy "batch-kindless" applies to any material type and requires a material selector (selector.name)`, + }, + { + name: "batch-local kind-less policy with selector", + contract: contractWithMaterialPolicy(batchKindless, "selector: {name: my-image}"), + specs: map[string]*schemav1.Policy{batchKindless: kindless}, + }, + { + name: "batch-local policy without a spec is only known", + contract: contractWithMaterialPolicy(batchKindless, ""), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := uc.ValidateContractPolicies(context.Background(), tc.contract, "token", []string{batchKindless}, nil, tc.specs) if tc.wantErr == "" { assert.NoError(t, err) return From 420a65b87e56f55f4a1f4bdd3a630cb9110f6ee6 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 5 Nov 2026 03:27:02 +0100 Subject: [PATCH 8/8] fix(controlplane): validate batch policy specs on dry-run apply Validate the raw batch policies while parsing them, and document that batch_policies is only used on dry runs. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 2df578c9-f7ba-441c-8f53-410c372821f2 --- .../controlplane/v1/workflow_contract.pb.go | 1 + .../controlplane/v1/workflow_contract.proto | 1 + .../controlplane/v1/workflow_contract.ts | 1 + ...ontractServiceApplyRequest.jsonschema.json | 4 +- ...lowContractServiceApplyRequest.schema.json | 4 +- .../internal/service/workflowcontract.go | 4 +- .../internal/service/workflowcontract_test.go | 56 +++++++++++++++++++ 7 files changed, 65 insertions(+), 6 deletions(-) diff --git a/app/controlplane/api/controlplane/v1/workflow_contract.pb.go b/app/controlplane/api/controlplane/v1/workflow_contract.pb.go index 2ef55df0c..81dbf5878 100644 --- a/app/controlplane/api/controlplane/v1/workflow_contract.pb.go +++ b/app/controlplane/api/controlplane/v1/workflow_contract.pb.go @@ -581,6 +581,7 @@ type WorkflowContractServiceApplyRequest struct { // Same as batch_policy_names, for policy groups created/updated in the same batch. BatchPolicyGroupNames []string `protobuf:"bytes,4,rep,name=batch_policy_group_names,json=batchPolicyGroupNames,proto3" json:"batch_policy_group_names,omitempty"` // Raw representation (json or yaml) of the policies created/updated in the same batch apply. + // Only used on dry runs, ignored on a real apply (which persists batch resources first). // When present, the specs of batch-local references are used to validate their inputs and // whether the attachment can apply to a material, as a real apply does. BatchPolicies [][]byte `protobuf:"bytes,5,rep,name=batch_policies,json=batchPolicies,proto3" json:"batch_policies,omitempty"` diff --git a/app/controlplane/api/controlplane/v1/workflow_contract.proto b/app/controlplane/api/controlplane/v1/workflow_contract.proto index 546c1fd55..fe31f9508 100644 --- a/app/controlplane/api/controlplane/v1/workflow_contract.proto +++ b/app/controlplane/api/controlplane/v1/workflow_contract.proto @@ -128,6 +128,7 @@ message WorkflowContractServiceApplyRequest { // Same as batch_policy_names, for policy groups created/updated in the same batch. repeated string batch_policy_group_names = 4; // Raw representation (json or yaml) of the policies created/updated in the same batch apply. + // Only used on dry runs, ignored on a real apply (which persists batch resources first). // When present, the specs of batch-local references are used to validate their inputs and // whether the attachment can apply to a material, as a real apply does. repeated bytes batch_policies = 5; diff --git a/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts b/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts index 1711ee739..a55f52e89 100644 --- a/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts +++ b/app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts @@ -81,6 +81,7 @@ export interface WorkflowContractServiceApplyRequest { batchPolicyGroupNames: string[]; /** * Raw representation (json or yaml) of the policies created/updated in the same batch apply. + * Only used on dry runs, ignored on a real apply (which persists batch resources first). * When present, the specs of batch-local references are used to validate their inputs and * whether the attachment can apply to a material, as a real apply does. */ diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json index a02b23375..506ebfd0e 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.jsonschema.json @@ -4,7 +4,7 @@ "additionalProperties": false, "patternProperties": { "^(batch_policies)$": { - "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n Only used on dry runs, ignored on a real apply (which persists batch resources first).\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", "items": { "pattern": "^[A-Za-z0-9+/]*={0,2}$", "type": "string" @@ -37,7 +37,7 @@ }, "properties": { "batchPolicies": { - "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n Only used on dry runs, ignored on a real apply (which persists batch resources first).\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", "items": { "pattern": "^[A-Za-z0-9+/]*={0,2}$", "type": "string" diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json index bb19fdd8c..61b04c3d0 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.WorkflowContractServiceApplyRequest.schema.json @@ -4,7 +4,7 @@ "additionalProperties": false, "patternProperties": { "^(batchPolicies)$": { - "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n Only used on dry runs, ignored on a real apply (which persists batch resources first).\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", "items": { "pattern": "^[A-Za-z0-9+/]*={0,2}$", "type": "string" @@ -37,7 +37,7 @@ }, "properties": { "batch_policies": { - "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", + "description": "Raw representation (json or yaml) of the policies created/updated in the same batch apply.\n Only used on dry runs, ignored on a real apply (which persists batch resources first).\n When present, the specs of batch-local references are used to validate their inputs and\n whether the attachment can apply to a material, as a real apply does.", "items": { "pattern": "^[A-Za-z0-9+/]*={0,2}$", "type": "string" diff --git a/app/controlplane/internal/service/workflowcontract.go b/app/controlplane/internal/service/workflowcontract.go index 29cab6008..2cdc52566 100644 --- a/app/controlplane/internal/service/workflowcontract.go +++ b/app/controlplane/internal/service/workflowcontract.go @@ -531,7 +531,6 @@ func validateAndExtractMetadata(rawContract []byte, explicitName, explicitDesc s return name, description, nil } -// extractMetadata attempts to extract the name from metadata.name in v2 contracts // parseBatchPolicies parses the raw policies sent with a batch apply, indexed by policy name func parseBatchPolicies(raws [][]byte) (map[string]*schemav1.Policy, error) { if len(raws) == 0 { @@ -546,7 +545,7 @@ func parseBatchPolicies(raws [][]byte) (map[string]*schemav1.Policy, error) { } policy := &schemav1.Policy{} - if err := unmarshal.FromRaw(raw, format, policy, false); err != nil { + if err := unmarshal.FromRaw(raw, format, policy, true); err != nil { return nil, errors.BadRequest("invalid", fmt.Sprintf("invalid batch policy: %s", err)) } @@ -560,6 +559,7 @@ func parseBatchPolicies(raws [][]byte) (map[string]*schemav1.Policy, error) { return parsed, nil } +// extractMetadata attempts to extract the name from metadata.name in v2 contracts func extractMetadata(rawContract []byte) (*schemav1.Metadata, error) { if len(rawContract) == 0 { return nil, nil diff --git a/app/controlplane/internal/service/workflowcontract_test.go b/app/controlplane/internal/service/workflowcontract_test.go index 6fcb1ea47..ef3ad3b3c 100644 --- a/app/controlplane/internal/service/workflowcontract_test.go +++ b/app/controlplane/internal/service/workflowcontract_test.go @@ -22,6 +22,62 @@ import ( "github.com/stretchr/testify/require" ) +const batchPolName = "batch-pol" + +func TestParseBatchPolicies(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raws [][]byte + wantNames []string + wantErr bool + }{ + {name: "no policies"}, + { + name: "valid kind-less policy", + raws: [][]byte{[]byte(`apiVersion: chainloop.dev/v1 +kind: Policy +metadata: + name: batch-pol +spec: + policies: + - embedded: | + package main +`)}, + wantNames: []string{batchPolName}, + }, + { + name: "only a metadata name is not a valid policy", + raws: [][]byte{[]byte("metadata:\n name: batch-pol\n")}, + wantErr: true, + }, + { + name: "missing metadata name", + raws: [][]byte{[]byte("apiVersion: chainloop.dev/v1\nkind: Policy\nspec:\n policies:\n - embedded: package main\n")}, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := parseBatchPolicies(tc.raws) + if tc.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Len(t, got, len(tc.wantNames)) + for _, n := range tc.wantNames { + assert.Contains(t, got, n) + } + }) + } +} + func TestExtractMetadata(t *testing.T) { t.Parallel()