From ad9d2a24d1ceb68c7e89f20208e1e5ffd3e8cb9a Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Tue, 18 Nov 2025 19:07:17 +0100 Subject: [PATCH 1/8] feat: add skip field to policy group attachments Add support for explicitly disabling specific policies within a policy group by specifying their metadata names in a skip list. This allows users to selectively exclude policies from evaluation without modifying the policy group itself. Changes: - Add skip field to PolicyGroupAttachment protobuf message - Implement getPolicyName() helper to extract policy names from attachments - Filter skipped policies in both material and attestation evaluation paths - Add validateSkipList() to warn about non-existent policy names - Add comprehensive test coverage for skip functionality Example usage: ```yaml policyGroups: - ref: file://groups/sbom-quality.yaml with: bannedLicenses: AGPL-3.0 skip: - sbom-present - license-check ``` Policies are matched by metadata.name. Unknown policy names in the skip list generate warnings but allow execution to continue. Closes #2557 Signed-off-by: Miguel Martinez --- app/cli/pkg/action/testdata/contract_v2.yaml | 6 +- .../workflowcontract/v1/crafting_schema.ts | 21 +- ...t.v1.PolicyGroupAttachment.jsonschema.json | 7 + ...tract.v1.PolicyGroupAttachment.schema.json | 7 + .../workflowcontract/v1/crafting_schema.pb.go | 148 +++++++------- .../workflowcontract/v1/crafting_schema.proto | 2 + pkg/policies/policy_groups.go | 97 ++++++++- pkg/policies/policy_groups_test.go | 191 +++++++++++++++++- 8 files changed, 403 insertions(+), 76 deletions(-) diff --git a/app/cli/pkg/action/testdata/contract_v2.yaml b/app/cli/pkg/action/testdata/contract_v2.yaml index dc10619a7..40b277abe 100644 --- a/app/cli/pkg/action/testdata/contract_v2.yaml +++ b/app/cli/pkg/action/testdata/contract_v2.yaml @@ -33,4 +33,8 @@ spec: attestation: - ref: file://attestation-policy.yaml policyGroups: - - ref: file://testdata/policy_group.yaml \ No newline at end of file + - ref: file://testdata/policy_group.yaml + with: + user_name: "test-user" + skip: + - policy-to-skip \ No newline at end of file diff --git a/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts b/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts index 1dc49f692..dfda40bfe 100644 --- a/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts +++ b/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts @@ -518,6 +518,8 @@ export interface PolicyGroupAttachment { ref: string; /** group arguments */ with: { [key: string]: string }; + /** policy names to skip (matched against metadata.name) */ + skip: string[]; } export interface PolicyGroupAttachment_WithEntry { @@ -2244,7 +2246,7 @@ export const AutoMatch = { }; function createBasePolicyGroupAttachment(): PolicyGroupAttachment { - return { ref: "", with: {} }; + return { ref: "", with: {}, skip: [] }; } export const PolicyGroupAttachment = { @@ -2255,6 +2257,9 @@ export const PolicyGroupAttachment = { Object.entries(message.with).forEach(([key, value]) => { PolicyGroupAttachment_WithEntry.encode({ key: key as any, value }, writer.uint32(18).fork()).ldelim(); }); + for (const v of message.skip) { + writer.uint32(26).string(v!); + } return writer; }, @@ -2282,6 +2287,13 @@ export const PolicyGroupAttachment = { message.with[entry2.key] = entry2.value; } continue; + case 3: + if (tag !== 26) { + break; + } + + message.skip.push(reader.string()); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -2300,6 +2312,7 @@ export const PolicyGroupAttachment = { return acc; }, {}) : {}, + skip: Array.isArray(object?.skip) ? object.skip.map((e: any) => String(e)) : [], }; }, @@ -2312,6 +2325,11 @@ export const PolicyGroupAttachment = { obj.with[k] = v; }); } + if (message.skip) { + obj.skip = message.skip.map((e) => e); + } else { + obj.skip = []; + } return obj; }, @@ -2328,6 +2346,7 @@ export const PolicyGroupAttachment = { } return acc; }, {}); + message.skip = object.skip?.map((e) => e) || []; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.jsonschema.json b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.jsonschema.json index 0503e82ac..dff01af36 100644 --- a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.jsonschema.json @@ -9,6 +9,13 @@ "minLength": 1, "type": "string" }, + "skip": { + "description": "policy names to skip (matched against metadata.name)", + "items": { + "type": "string" + }, + "type": "array" + }, "with": { "additionalProperties": { "type": "string" diff --git a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.schema.json b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.schema.json index feb192779..8bebd1633 100644 --- a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.schema.json +++ b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyGroupAttachment.schema.json @@ -9,6 +9,13 @@ "minLength": 1, "type": "string" }, + "skip": { + "description": "policy names to skip (matched against metadata.name)", + "items": { + "type": "string" + }, + "type": "array" + }, "with": { "additionalProperties": { "type": "string" diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go index c3954c311..9cf508e16 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go @@ -1287,6 +1287,8 @@ type PolicyGroupAttachment struct { Ref string `protobuf:"bytes,1,opt,name=ref,proto3" json:"ref,omitempty"` // group arguments With map[string]string `protobuf:"bytes,2,rep,name=with,proto3" json:"with,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` + // policy names to skip (matched against metadata.name) + Skip []string `protobuf:"bytes,3,rep,name=skip,proto3" json:"skip,omitempty"` } func (x *PolicyGroupAttachment) Reset() { @@ -1335,6 +1337,13 @@ func (x *PolicyGroupAttachment) GetWith() map[string]string { return nil } +func (x *PolicyGroupAttachment) GetSkip() []string { + if x != nil { + return x.Skip + } + return nil +} + // Represents a group or policies type PolicyGroup struct { state protoimpl.MessageState @@ -2102,7 +2111,7 @@ var file_workflowcontract_v1_crafting_schema_proto_rawDesc = []byte{ 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x48, 0x00, 0x52, 0x04, 0x70, 0x61, 0x74, 0x68, 0x12, 0x1c, 0x0a, 0x08, 0x65, 0x6d, 0x62, 0x65, 0x64, 0x64, 0x65, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x48, 0x00, 0x52, 0x08, 0x65, 0x6d, 0x62, 0x65, 0x64, 0x64, 0x65, 0x64, 0x42, 0x0f, 0x0a, 0x06, - 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x12, 0x05, 0xba, 0x48, 0x02, 0x08, 0x01, 0x22, 0xb5, 0x01, + 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x12, 0x05, 0xba, 0x48, 0x02, 0x08, 0x01, 0x22, 0xc9, 0x01, 0x0a, 0x15, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x41, 0x74, 0x74, 0x61, 0x63, 0x68, 0x6d, 0x65, 0x6e, 0x74, 0x12, 0x19, 0x0a, 0x03, 0x72, 0x65, 0x66, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x07, 0xba, 0x48, 0x04, 0x72, 0x02, 0x10, 0x01, 0x52, 0x03, 0x72, @@ -2110,76 +2119,77 @@ var file_workflowcontract_v1_crafting_schema_proto_rawDesc = []byte{ 0x32, 0x34, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x41, 0x74, 0x74, 0x61, 0x63, 0x68, 0x6d, 0x65, 0x6e, 0x74, 0x2e, 0x57, 0x69, 0x74, - 0x68, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x52, 0x04, 0x77, 0x69, 0x74, 0x68, 0x1a, 0x37, 0x0a, 0x09, - 0x57, 0x69, 0x74, 0x68, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, - 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, - 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, - 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, 0xb5, 0x07, 0x0a, 0x0b, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, - 0x47, 0x72, 0x6f, 0x75, 0x70, 0x12, 0x49, 0x0a, 0x0b, 0x61, 0x70, 0x69, 0x5f, 0x76, 0x65, 0x72, - 0x73, 0x69, 0x6f, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x28, 0xba, 0x48, 0x25, 0x72, - 0x23, 0x0a, 0x21, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, - 0x61, 0x63, 0x74, 0x2e, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x6c, 0x6f, 0x6f, 0x70, 0x2e, 0x64, 0x65, - 0x76, 0x2f, 0x76, 0x31, 0x52, 0x0a, 0x61, 0x70, 0x69, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, - 0x12, 0x26, 0x0a, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x42, 0x12, - 0xba, 0x48, 0x0f, 0x72, 0x0d, 0x0a, 0x0b, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, - 0x75, 0x70, 0x52, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0x12, 0x41, 0x0a, 0x08, 0x6d, 0x65, 0x74, 0x61, - 0x64, 0x61, 0x74, 0x61, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1d, 0x2e, 0x77, 0x6f, 0x72, - 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, - 0x2e, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x42, 0x06, 0xba, 0x48, 0x03, 0xc8, 0x01, - 0x01, 0x52, 0x08, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x12, 0x4c, 0x0a, 0x04, 0x73, - 0x70, 0x65, 0x63, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x30, 0x2e, 0x77, 0x6f, 0x72, 0x6b, - 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, - 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x2e, 0x50, 0x6f, 0x6c, 0x69, - 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x53, 0x70, 0x65, 0x63, 0x42, 0x06, 0xba, 0x48, 0x03, - 0xc8, 0x01, 0x01, 0x52, 0x04, 0x73, 0x70, 0x65, 0x63, 0x1a, 0x9d, 0x01, 0x0a, 0x0f, 0x50, 0x6f, - 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x53, 0x70, 0x65, 0x63, 0x12, 0x50, 0x0a, - 0x08, 0x70, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, 0x73, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, - 0x34, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, - 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, - 0x70, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x50, 0x6f, 0x6c, - 0x69, 0x63, 0x69, 0x65, 0x73, 0x52, 0x08, 0x70, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, 0x73, 0x12, - 0x38, 0x0a, 0x06, 0x69, 0x6e, 0x70, 0x75, 0x74, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x0b, 0x32, - 0x20, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, - 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x49, 0x6e, 0x70, 0x75, - 0x74, 0x52, 0x06, 0x69, 0x6e, 0x70, 0x75, 0x74, 0x73, 0x1a, 0xa7, 0x01, 0x0a, 0x13, 0x50, 0x6f, - 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, - 0x73, 0x12, 0x47, 0x0a, 0x09, 0x6d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, 0x73, 0x18, 0x01, - 0x20, 0x03, 0x28, 0x0b, 0x32, 0x29, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, - 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, - 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x2e, 0x4d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, 0x52, - 0x09, 0x6d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, 0x73, 0x12, 0x47, 0x0a, 0x0b, 0x61, 0x74, - 0x74, 0x65, 0x73, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x02, 0x20, 0x03, 0x28, 0x0b, 0x32, + 0x68, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x52, 0x04, 0x77, 0x69, 0x74, 0x68, 0x12, 0x12, 0x0a, 0x04, + 0x73, 0x6b, 0x69, 0x70, 0x18, 0x03, 0x20, 0x03, 0x28, 0x09, 0x52, 0x04, 0x73, 0x6b, 0x69, 0x70, + 0x1a, 0x37, 0x0a, 0x09, 0x57, 0x69, 0x74, 0x68, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, + 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, + 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, + 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, 0xb5, 0x07, 0x0a, 0x0b, 0x50, 0x6f, + 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x12, 0x49, 0x0a, 0x0b, 0x61, 0x70, 0x69, + 0x5f, 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x28, + 0xba, 0x48, 0x25, 0x72, 0x23, 0x0a, 0x21, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, + 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x6c, 0x6f, 0x6f, + 0x70, 0x2e, 0x64, 0x65, 0x76, 0x2f, 0x76, 0x31, 0x52, 0x0a, 0x61, 0x70, 0x69, 0x56, 0x65, 0x72, + 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x26, 0x0a, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0x18, 0x02, 0x20, 0x01, + 0x28, 0x09, 0x42, 0x12, 0xba, 0x48, 0x0f, 0x72, 0x0d, 0x0a, 0x0b, 0x50, 0x6f, 0x6c, 0x69, 0x63, + 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x52, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0x12, 0x41, 0x0a, 0x08, + 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1d, + 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, + 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x42, 0x06, 0xba, + 0x48, 0x03, 0xc8, 0x01, 0x01, 0x52, 0x08, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x12, + 0x4c, 0x0a, 0x04, 0x73, 0x70, 0x65, 0x63, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x30, 0x2e, + 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, + 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x2e, + 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x53, 0x70, 0x65, 0x63, 0x42, + 0x06, 0xba, 0x48, 0x03, 0xc8, 0x01, 0x01, 0x52, 0x04, 0x73, 0x70, 0x65, 0x63, 0x1a, 0x9d, 0x01, + 0x0a, 0x0f, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x53, 0x70, 0x65, + 0x63, 0x12, 0x50, 0x0a, 0x08, 0x70, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, 0x73, 0x18, 0x01, 0x20, + 0x01, 0x28, 0x0b, 0x32, 0x34, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, + 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, + 0x47, 0x72, 0x6f, 0x75, 0x70, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, + 0x70, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, 0x73, 0x52, 0x08, 0x70, 0x6f, 0x6c, 0x69, 0x63, + 0x69, 0x65, 0x73, 0x12, 0x38, 0x0a, 0x06, 0x69, 0x6e, 0x70, 0x75, 0x74, 0x73, 0x18, 0x02, 0x20, + 0x03, 0x28, 0x0b, 0x32, 0x20, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, + 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, + 0x49, 0x6e, 0x70, 0x75, 0x74, 0x52, 0x06, 0x69, 0x6e, 0x70, 0x75, 0x74, 0x73, 0x1a, 0xa7, 0x01, + 0x0a, 0x13, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x50, 0x6f, 0x6c, + 0x69, 0x63, 0x69, 0x65, 0x73, 0x12, 0x47, 0x0a, 0x09, 0x6d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, + 0x6c, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x29, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, + 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, + 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x2e, 0x4d, 0x61, 0x74, 0x65, 0x72, + 0x69, 0x61, 0x6c, 0x52, 0x09, 0x6d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, 0x73, 0x12, 0x47, + 0x0a, 0x0b, 0x61, 0x74, 0x74, 0x65, 0x73, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x02, 0x20, + 0x03, 0x28, 0x0b, 0x32, 0x25, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, + 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, + 0x41, 0x74, 0x74, 0x61, 0x63, 0x68, 0x6d, 0x65, 0x6e, 0x74, 0x52, 0x0b, 0x61, 0x74, 0x74, 0x65, + 0x73, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x1a, 0xd7, 0x02, 0x0a, 0x08, 0x4d, 0x61, 0x74, 0x65, + 0x72, 0x69, 0x61, 0x6c, 0x12, 0x57, 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x18, 0x01, 0x20, 0x01, + 0x28, 0x0e, 0x32, 0x39, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, + 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x43, 0x72, 0x61, 0x66, 0x74, 0x69, 0x6e, + 0x67, 0x53, 0x63, 0x68, 0x65, 0x6d, 0x61, 0x2e, 0x4d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, + 0x2e, 0x4d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, 0x54, 0x79, 0x70, 0x65, 0x42, 0x08, 0xba, + 0x48, 0x05, 0x82, 0x01, 0x02, 0x10, 0x01, 0x52, 0x04, 0x74, 0x79, 0x70, 0x65, 0x12, 0x12, 0x0a, + 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, + 0x65, 0x12, 0x1a, 0x0a, 0x08, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x18, 0x03, 0x20, + 0x01, 0x28, 0x08, 0x52, 0x08, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x12, 0x41, 0x0a, + 0x08, 0x70, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, 0x73, 0x18, 0x06, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x25, 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x41, 0x74, 0x74, 0x61, - 0x63, 0x68, 0x6d, 0x65, 0x6e, 0x74, 0x52, 0x0b, 0x61, 0x74, 0x74, 0x65, 0x73, 0x74, 0x61, 0x74, - 0x69, 0x6f, 0x6e, 0x1a, 0xd7, 0x02, 0x0a, 0x08, 0x4d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, - 0x12, 0x57, 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x39, - 0x2e, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, - 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x43, 0x72, 0x61, 0x66, 0x74, 0x69, 0x6e, 0x67, 0x53, 0x63, 0x68, - 0x65, 0x6d, 0x61, 0x2e, 0x4d, 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, 0x2e, 0x4d, 0x61, 0x74, - 0x65, 0x72, 0x69, 0x61, 0x6c, 0x54, 0x79, 0x70, 0x65, 0x42, 0x08, 0xba, 0x48, 0x05, 0x82, 0x01, - 0x02, 0x10, 0x01, 0x52, 0x04, 0x74, 0x79, 0x70, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x6e, 0x61, 0x6d, - 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x1a, 0x0a, - 0x08, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x18, 0x03, 0x20, 0x01, 0x28, 0x08, 0x52, - 0x08, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x12, 0x41, 0x0a, 0x08, 0x70, 0x6f, 0x6c, - 0x69, 0x63, 0x69, 0x65, 0x73, 0x18, 0x06, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x25, 0x2e, 0x77, 0x6f, - 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2e, 0x76, - 0x31, 0x2e, 0x50, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x41, 0x74, 0x74, 0x61, 0x63, 0x68, 0x6d, 0x65, - 0x6e, 0x74, 0x52, 0x08, 0x70, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, 0x73, 0x3a, 0x7f, 0xba, 0x48, - 0x7c, 0x1a, 0x7a, 0x0a, 0x0e, 0x67, 0x72, 0x6f, 0x75, 0x70, 0x5f, 0x6d, 0x61, 0x74, 0x65, 0x72, - 0x69, 0x61, 0x6c, 0x12, 0x33, 0x69, 0x66, 0x20, 0x6e, 0x61, 0x6d, 0x65, 0x20, 0x69, 0x73, 0x20, - 0x70, 0x72, 0x6f, 0x76, 0x69, 0x64, 0x65, 0x64, 0x2c, 0x20, 0x74, 0x79, 0x70, 0x65, 0x20, 0x73, - 0x68, 0x6f, 0x75, 0x6c, 0x64, 0x20, 0x68, 0x61, 0x76, 0x65, 0x20, 0x61, 0x20, 0x76, 0x61, 0x6c, - 0x69, 0x64, 0x20, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x1a, 0x33, 0x21, 0x68, 0x61, 0x73, 0x28, 0x74, - 0x68, 0x69, 0x73, 0x2e, 0x6e, 0x61, 0x6d, 0x65, 0x29, 0x20, 0x7c, 0x7c, 0x20, 0x68, 0x61, 0x73, - 0x28, 0x74, 0x68, 0x69, 0x73, 0x2e, 0x6e, 0x61, 0x6d, 0x65, 0x29, 0x20, 0x26, 0x26, 0x20, 0x74, - 0x68, 0x69, 0x73, 0x2e, 0x74, 0x79, 0x70, 0x65, 0x20, 0x21, 0x3d, 0x20, 0x30, 0x42, 0x4d, 0x5a, - 0x4b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x68, 0x61, 0x69, - 0x6e, 0x6c, 0x6f, 0x6f, 0x70, 0x2d, 0x64, 0x65, 0x76, 0x2f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x6c, - 0x6f, 0x6f, 0x70, 0x2f, 0x61, 0x70, 0x70, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x70, - 0x6c, 0x61, 0x6e, 0x65, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, - 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2f, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, - 0x6f, 0x74, 0x6f, 0x33, + 0x63, 0x68, 0x6d, 0x65, 0x6e, 0x74, 0x52, 0x08, 0x70, 0x6f, 0x6c, 0x69, 0x63, 0x69, 0x65, 0x73, + 0x3a, 0x7f, 0xba, 0x48, 0x7c, 0x1a, 0x7a, 0x0a, 0x0e, 0x67, 0x72, 0x6f, 0x75, 0x70, 0x5f, 0x6d, + 0x61, 0x74, 0x65, 0x72, 0x69, 0x61, 0x6c, 0x12, 0x33, 0x69, 0x66, 0x20, 0x6e, 0x61, 0x6d, 0x65, + 0x20, 0x69, 0x73, 0x20, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x64, 0x65, 0x64, 0x2c, 0x20, 0x74, 0x79, + 0x70, 0x65, 0x20, 0x73, 0x68, 0x6f, 0x75, 0x6c, 0x64, 0x20, 0x68, 0x61, 0x76, 0x65, 0x20, 0x61, + 0x20, 0x76, 0x61, 0x6c, 0x69, 0x64, 0x20, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x1a, 0x33, 0x21, 0x68, + 0x61, 0x73, 0x28, 0x74, 0x68, 0x69, 0x73, 0x2e, 0x6e, 0x61, 0x6d, 0x65, 0x29, 0x20, 0x7c, 0x7c, + 0x20, 0x68, 0x61, 0x73, 0x28, 0x74, 0x68, 0x69, 0x73, 0x2e, 0x6e, 0x61, 0x6d, 0x65, 0x29, 0x20, + 0x26, 0x26, 0x20, 0x74, 0x68, 0x69, 0x73, 0x2e, 0x74, 0x79, 0x70, 0x65, 0x20, 0x21, 0x3d, 0x20, + 0x30, 0x42, 0x4d, 0x5a, 0x4b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, + 0x63, 0x68, 0x61, 0x69, 0x6e, 0x6c, 0x6f, 0x6f, 0x70, 0x2d, 0x64, 0x65, 0x76, 0x2f, 0x63, 0x68, + 0x61, 0x69, 0x6e, 0x6c, 0x6f, 0x6f, 0x70, 0x2f, 0x61, 0x70, 0x70, 0x2f, 0x63, 0x6f, 0x6e, 0x74, + 0x72, 0x6f, 0x6c, 0x70, 0x6c, 0x61, 0x6e, 0x65, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x77, 0x6f, 0x72, + 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2f, 0x76, 0x31, + 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto index aeaf2e8e8..f42b7f12b 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto @@ -344,6 +344,8 @@ message PolicyGroupAttachment { string ref = 1 [(buf.validate.field).string.min_len = 1]; // group arguments map with = 2; + // policy names to skip (matched against metadata.name) + repeated string skip = 3; } // Represents a group or policies diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index 5d7c241d6..a096942b7 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -67,8 +67,11 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap return nil, NewPolicyError(err) } + // Validate skip list and log warnings for unknown policy names + pgv.validateSkipList(ctx, group, groupAtt) + // gather required policies - policyAtts, err := pgv.requiredPoliciesForMaterial(ctx, material, group, groupArgs) + policyAtts, err := pgv.requiredPoliciesForMaterial(ctx, material, group, groupAtt, groupArgs) if err != nil { return nil, NewPolicyError(err) } @@ -125,7 +128,23 @@ func (pgv *PolicyGroupVerifier) VerifyStatement(ctx context.Context, statement * if err != nil { return nil, NewPolicyError(err) } + + // Validate skip list and log warnings for unknown policy names + pgv.validateSkipList(ctx, group, groupAtt) + for _, attachment := range group.GetSpec().GetPolicies().GetAttestation() { + // Check if policy should be skipped + policyName, err := pgv.getPolicyName(ctx, attachment) + if err != nil { + return nil, NewPolicyError(fmt.Errorf("failed to get policy name: %w", err)) + } + + // Skip if policy name is in the skip list + if slices.Contains(groupAtt.GetSkip(), policyName) { + pgv.logger.Debug().Str("policy", policyName).Msg("skipping attestation policy per skip list") + continue + } + material, err := protojson.Marshal(statement) if err != nil { return nil, NewPolicyError(err) @@ -211,7 +230,7 @@ func getGroupLoader(attachment *v1.PolicyGroupAttachment, opts *LoadPolicyGroupO } // Gets the policies that can be applied to a material within a group -func (pgv *PolicyGroupVerifier) requiredPoliciesForMaterial(ctx context.Context, material *api.Attestation_Material, group *v1.PolicyGroup, groupArgs map[string]string) ([]*v1.PolicyAttachment, error) { +func (pgv *PolicyGroupVerifier) requiredPoliciesForMaterial(ctx context.Context, material *api.Attestation_Material, group *v1.PolicyGroup, groupAtt *v1.PolicyGroupAttachment, groupArgs map[string]string) ([]*v1.PolicyAttachment, error) { result := make([]*v1.PolicyAttachment, 0) // 2. go through all materials in the group and look for the crafted material @@ -234,6 +253,18 @@ func (pgv *PolicyGroupVerifier) requiredPoliciesForMaterial(ctx context.Context, } if apply { + // Check if policy should be skipped + policyName, err := pgv.getPolicyName(ctx, policyAtt) + if err != nil { + return nil, fmt.Errorf("failed to get policy name: %w", err) + } + + // Skip if policy name is in the skip list + if slices.Contains(groupAtt.GetSkip(), policyName) { + pgv.logger.Debug().Str("policy", policyName).Msg("skipping policy per skip list") + continue + } + result = append(result, policyAtt) } } @@ -282,3 +313,65 @@ func (pgv *PolicyGroupVerifier) shouldApplyPolicy(ctx context.Context, policyAtt return false, nil } + +// getPolicyName extracts the metadata.name from a PolicyAttachment +// It handles both embedded and referenced policies by loading the policy spec when needed +func (pgv *PolicyGroupVerifier) getPolicyName(ctx context.Context, attachment *v1.PolicyAttachment) (string, error) { + // Case 1: Embedded policy - direct access + if embedded := attachment.GetEmbedded(); embedded != nil { + return embedded.GetMetadata().GetName(), nil + } + + // Case 2: Referenced policy - must load it + if ref := attachment.GetRef(); ref != "" { + // Load the policy spec using existing loader infrastructure + policy, _, err := pgv.loadPolicySpec(ctx, attachment) + if err != nil { + return "", fmt.Errorf("failed to load policy to get name: %w", err) + } + return policy.GetMetadata().GetName(), nil + } + + // Should never happen due to protobuf validation, but handle defensively + return "", errors.New("policy attachment has neither ref nor embedded policy") +} + +// validateSkipList checks if policy names in the skip list exist in the group +// and logs warnings for any unknown policy names +func (pgv *PolicyGroupVerifier) validateSkipList(ctx context.Context, group *v1.PolicyGroup, groupAtt *v1.PolicyGroupAttachment) { + if len(groupAtt.GetSkip()) == 0 { + return + } + + // Collect all policy names in the group + policyNames := make(map[string]bool) + + // Collect material policy names + for _, groupMaterial := range group.GetSpec().GetPolicies().GetMaterials() { + for _, policyAtt := range groupMaterial.GetPolicies() { + name, err := pgv.getPolicyName(ctx, policyAtt) + if err != nil { + pgv.logger.Warn().Err(err).Msg("failed to get policy name during skip list validation") + continue + } + policyNames[name] = true + } + } + + // Collect attestation policy names + for _, policyAtt := range group.GetSpec().GetPolicies().GetAttestation() { + name, err := pgv.getPolicyName(ctx, policyAtt) + if err != nil { + pgv.logger.Warn().Err(err).Msg("failed to get policy name during skip list validation") + continue + } + policyNames[name] = true + } + + // Check each skip entry against collected policy names + for _, skipName := range groupAtt.GetSkip() { + if !policyNames[skipName] { + pgv.logger.Warn().Str("policy", skipName).Str("group", group.GetMetadata().GetName()).Msg("policy in skip list not found in group") + } + } +} diff --git a/pkg/policies/policy_groups_test.go b/pkg/policies/policy_groups_test.go index 658fdac12..cd34f3c39 100644 --- a/pkg/policies/policy_groups_test.go +++ b/pkg/policies/policy_groups_test.go @@ -21,6 +21,7 @@ import ( v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" + intoto "github.com/in-toto/attestation/go/v1" "github.com/rs/zerolog" "github.com/stretchr/testify/suite" ) @@ -164,11 +165,12 @@ func (s *groupsTestSuite) TestRequiredPoliciesForMaterial() { } v := NewPolicyGroupVerifier(schema.PolicyGroups, nil, nil, &s.logger) - group, _, err := new(FileGroupLoader).Load(context.TODO(), &v1.PolicyGroupAttachment{ + groupAtt := &v1.PolicyGroupAttachment{ Ref: tc.schemaRef, - }) + } + group, _, err := new(FileGroupLoader).Load(context.TODO(), groupAtt) s.Require().NoError(err) - atts, err := v.requiredPoliciesForMaterial(context.TODO(), material, group, nil) + atts, err := v.requiredPoliciesForMaterial(context.TODO(), material, group, groupAtt, nil) s.Require().NoError(err) s.Len(atts, tc.expected) }) @@ -427,3 +429,186 @@ func (s *groupsTestSuite) TestGroupInputs() { }) } } + +func (s *groupsTestSuite) TestSkipPolicies() { + cases := []struct { + name string + policyGroup string + skipPolicies []string + material string + expectedEvaluations int + expectErr bool + }{ + { + name: "no skip list - all policies run", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: nil, + material: `{"specVersion": "1.4"}`, + expectedEvaluations: 1, // policy-result-format policy runs + }, + { + name: "skip material policy", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{"policy-result-format"}, + material: `{"specVersion": "1.4"}`, + expectedEvaluations: 0, // policy-result-format policy skipped + }, + { + name: "skip non-existent policy - warning logged but continues", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{"non-existent-policy"}, + material: `{"specVersion": "1.4"}`, + expectedEvaluations: 1, // policy-result-format policy still runs + }, + { + name: "skip multiple policies including non-existent", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{"policy-result-format", "non-existent"}, + material: `{"specVersion": "1.4"}`, + expectedEvaluations: 0, // policy-result-format skipped + }, + { + name: "empty skip list", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{}, + material: `{"specVersion": "1.4"}`, + expectedEvaluations: 1, + }, + } + + for _, tc := range cases { + s.Run(tc.name, func() { + schema := &v1.CraftingSchema{ + Materials: []*v1.CraftingSchema_Material{ + { + Name: "sbom", + Type: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON, + }, + }, + PolicyGroups: []*v1.PolicyGroupAttachment{ + { + Ref: tc.policyGroup, + Skip: tc.skipPolicies, + }, + }, + } + + material := &api.Attestation_Material{ + M: &api.Attestation_Material_Artifact_{Artifact: &api.Attestation_Material_Artifact{ + Content: []byte(tc.material), + }}, + MaterialType: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON, + InlineCas: true, + } + + verifier := NewPolicyGroupVerifier(schema.GetPolicyGroups(), nil, nil, &s.logger) + evs, err := verifier.VerifyMaterial(context.Background(), material, "") + + if tc.expectErr { + s.Error(err) + return + } + + s.Require().NoError(err) + s.Len(evs, tc.expectedEvaluations) + }) + } +} + +func (s *groupsTestSuite) TestSkipAttestationPolicies() { + cases := []struct { + name string + policyGroup string + skipPolicies []string + expectedEvaluations int + expectErr bool + }{ + { + name: "no skip list - attestation policy runs", + policyGroup: "file://testdata/policy_group.yaml", + skipPolicies: nil, + expectedEvaluations: 1, // workflow policy runs + }, + { + name: "skip attestation policy", + policyGroup: "file://testdata/policy_group.yaml", + skipPolicies: []string{"workflow"}, + expectedEvaluations: 0, // workflow policy skipped + }, + { + name: "skip non-existent attestation policy", + policyGroup: "file://testdata/policy_group.yaml", + skipPolicies: []string{"non-existent"}, + expectedEvaluations: 1, // workflow still runs + }, + } + + for _, tc := range cases { + s.Run(tc.name, func() { + schema := &v1.CraftingSchema{ + PolicyGroups: []*v1.PolicyGroupAttachment{ + { + Ref: tc.policyGroup, + Skip: tc.skipPolicies, + }, + }, + } + + // Create a simple statement + statement := &intoto.Statement{ + Type: "https://in-toto.io/Statement/v0.1", + } + + verifier := NewPolicyGroupVerifier(schema.GetPolicyGroups(), nil, nil, &s.logger) + evs, err := verifier.VerifyStatement(context.Background(), statement) + + if tc.expectErr { + s.Error(err) + return + } + + s.Require().NoError(err) + s.Len(evs, tc.expectedEvaluations) + }) + } +} + +func (s *groupsTestSuite) TestSkipBothMaterialAndAttestationPolicies() { + schema := &v1.CraftingSchema{ + Materials: []*v1.CraftingSchema_Material{ + { + Name: "sbom", + Type: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON, + }, + }, + PolicyGroups: []*v1.PolicyGroupAttachment{ + { + Ref: "file://testdata/policy_group_multikind.yaml", + Skip: []string{"policy-result-format", "workflow"}, + }, + }, + } + + // Test material evaluation - should be skipped + material := &api.Attestation_Material{ + M: &api.Attestation_Material_Artifact_{Artifact: &api.Attestation_Material_Artifact{ + Content: []byte(`{"specVersion": "1.4"}`), + }}, + MaterialType: v1.CraftingSchema_Material_SBOM_CYCLONEDX_JSON, + InlineCas: true, + } + + verifier := NewPolicyGroupVerifier(schema.GetPolicyGroups(), nil, nil, &s.logger) + materialEvs, err := verifier.VerifyMaterial(context.Background(), material, "") + s.Require().NoError(err) + s.Len(materialEvs, 0, "material policy should be skipped") + + // Test attestation evaluation - should be skipped + statement := &intoto.Statement{ + Type: "https://in-toto.io/Statement/v0.1", + } + + attestationEvs, err := verifier.VerifyStatement(context.Background(), statement) + s.Require().NoError(err) + s.Len(attestationEvs, 0, "attestation policy should be skipped") +} From d5c7c3b72b6fbeb40a0ed1eb6871939659844523 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Tue, 18 Nov 2025 19:23:53 +0100 Subject: [PATCH 2/8] refactor: make validateSkipList return error for reusability Change validateSkipList() to return an error instead of logging warnings directly. This allows the function to be reused in contexts where validation errors should block execution, while still supporting the current behavior of logging warnings and continuing. Changes: - Update validateSkipList() to collect unknown policy names and return error - Callers in VerifyMaterial() and VerifyStatement() log errors as warnings - Add comprehensive tests for validateSkipList() error returns - All existing tests continue to pass The current user-facing behavior is unchanged: unknown policy names generate warnings but do not block execution. Signed-off-by: Miguel Martinez --- pkg/policies/policy_groups.go | 26 +++++++--- pkg/policies/policy_groups_test.go | 76 ++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index a096942b7..ee1e04c26 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -68,7 +68,9 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap } // Validate skip list and log warnings for unknown policy names - pgv.validateSkipList(ctx, group, groupAtt) + if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { + pgv.logger.Warn().Err(err).Msg("skip list validation warning") + } // gather required policies policyAtts, err := pgv.requiredPoliciesForMaterial(ctx, material, group, groupAtt, groupArgs) @@ -130,7 +132,9 @@ func (pgv *PolicyGroupVerifier) VerifyStatement(ctx context.Context, statement * } // Validate skip list and log warnings for unknown policy names - pgv.validateSkipList(ctx, group, groupAtt) + if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { + pgv.logger.Warn().Err(err).Msg("skip list validation warning") + } for _, attachment := range group.GetSpec().GetPolicies().GetAttestation() { // Check if policy should be skipped @@ -337,10 +341,10 @@ func (pgv *PolicyGroupVerifier) getPolicyName(ctx context.Context, attachment *v } // validateSkipList checks if policy names in the skip list exist in the group -// and logs warnings for any unknown policy names -func (pgv *PolicyGroupVerifier) validateSkipList(ctx context.Context, group *v1.PolicyGroup, groupAtt *v1.PolicyGroupAttachment) { +// and returns an error if any unknown policy names are found +func (pgv *PolicyGroupVerifier) validateSkipList(ctx context.Context, group *v1.PolicyGroup, groupAtt *v1.PolicyGroupAttachment) error { if len(groupAtt.GetSkip()) == 0 { - return + return nil } // Collect all policy names in the group @@ -368,10 +372,18 @@ func (pgv *PolicyGroupVerifier) validateSkipList(ctx context.Context, group *v1. policyNames[name] = true } - // Check each skip entry against collected policy names + // Check each skip entry against collected policy names and collect unknown ones + var unknownPolicies []string for _, skipName := range groupAtt.GetSkip() { if !policyNames[skipName] { - pgv.logger.Warn().Str("policy", skipName).Str("group", group.GetMetadata().GetName()).Msg("policy in skip list not found in group") + unknownPolicies = append(unknownPolicies, skipName) } } + + // Return error if there are unknown policies + if len(unknownPolicies) > 0 { + return fmt.Errorf("policies in skip list not found in group %q: %v", group.GetMetadata().GetName(), unknownPolicies) + } + + return nil } diff --git a/pkg/policies/policy_groups_test.go b/pkg/policies/policy_groups_test.go index cd34f3c39..7d233ca93 100644 --- a/pkg/policies/policy_groups_test.go +++ b/pkg/policies/policy_groups_test.go @@ -612,3 +612,79 @@ func (s *groupsTestSuite) TestSkipBothMaterialAndAttestationPolicies() { s.Require().NoError(err) s.Len(attestationEvs, 0, "attestation policy should be skipped") } + +func (s *groupsTestSuite) TestValidateSkipList() { + cases := []struct { + name string + policyGroup string + skipPolicies []string + expectError bool + errorContains string + }{ + { + name: "empty skip list returns nil", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{}, + expectError: false, + }, + { + name: "nil skip list returns nil", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: nil, + expectError: false, + }, + { + name: "all valid policy names returns nil", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{"policy-result-format"}, + expectError: false, + }, + { + name: "single unknown policy name returns error", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{"non-existent"}, + expectError: true, + errorContains: "non-existent", + }, + { + name: "multiple unknown policy names returns error with all names", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{"non-existent-1", "non-existent-2"}, + expectError: true, + errorContains: "non-existent-1", + }, + { + name: "mix of valid and invalid returns error with only invalid", + policyGroup: "file://testdata/policy_group_multikind.yaml", + skipPolicies: []string{"policy-result-format", "non-existent"}, + expectError: true, + errorContains: "non-existent", + }, + } + + for _, tc := range cases { + s.Run(tc.name, func() { + groupAtt := &v1.PolicyGroupAttachment{ + Ref: tc.policyGroup, + Skip: tc.skipPolicies, + } + + group, _, err := LoadPolicyGroup(context.Background(), groupAtt, &LoadPolicyGroupOptions{ + Logger: &s.logger, + }) + s.Require().NoError(err) + + verifier := NewPolicyGroupVerifier([]*v1.PolicyGroupAttachment{groupAtt}, nil, nil, &s.logger) + err = verifier.validateSkipList(context.Background(), group, groupAtt) + + if tc.expectError { + s.Error(err) + if tc.errorContains != "" { + s.Contains(err.Error(), tc.errorContains) + } + } else { + s.NoError(err) + } + }) + } +} From 726ab69ae5e9b6d9fc4df7f4a535563a3f45f85a Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Tue, 18 Nov 2025 19:26:53 +0100 Subject: [PATCH 3/8] chore: improve skip list validation warning message Update the warning log message to be more user-friendly. The error object already contains the group name and list of unknown policies, so the message now clearly indicates what the issue is. Changed message from "skip list validation warning" to "some policies in skip list were not found in the policy group". The error details include the specific policy names and group name. Signed-off-by: Miguel Martinez --- pkg/policies/policy_groups.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index ee1e04c26..17da172dd 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -69,7 +69,7 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap // Validate skip list and log warnings for unknown policy names if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { - pgv.logger.Warn().Err(err).Msg("skip list validation warning") + pgv.logger.Warn().Err(err).Msg("some policies in skip list were not found in the policy group") } // gather required policies @@ -133,7 +133,7 @@ func (pgv *PolicyGroupVerifier) VerifyStatement(ctx context.Context, statement * // Validate skip list and log warnings for unknown policy names if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { - pgv.logger.Warn().Err(err).Msg("skip list validation warning") + pgv.logger.Warn().Err(err).Msg("some policies in skip list were not found in the policy group") } for _, attachment := range group.GetSpec().GetPolicies().GetAttestation() { From 29e7644e790a407088bdddbf99b625d9010b7733 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Wed, 19 Nov 2025 23:22:35 +0100 Subject: [PATCH 4/8] chore: upgrade ent golang Signed-off-by: Miguel Martinez --- app/controlplane/configs/config.devel.yaml | 8 ++++---- pkg/policies/policy_groups.go | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/controlplane/configs/config.devel.yaml b/app/controlplane/configs/config.devel.yaml index b52c4d6b4..e7acbe0ec 100644 --- a/app/controlplane/configs/config.devel.yaml +++ b/app/controlplane/configs/config.devel.yaml @@ -98,10 +98,10 @@ prometheus_integration: - org_name: "development" # Policy providers configuration -policy_providers: - - name: chainloop - default: true - url: http://localhost:8002/v1 +# policy_providers: +# - name: chainloop +# default: true +# url: http://localhost:8002/v1 enable_profiler: true # federated_authentication: diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index 17da172dd..b46c5dac2 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "slices" + "strings" v13 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" @@ -69,7 +70,7 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap // Validate skip list and log warnings for unknown policy names if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { - pgv.logger.Warn().Err(err).Msg("some policies in skip list were not found in the policy group") + pgv.logger.Warn().Msg(err.Error()) } // gather required policies @@ -133,7 +134,7 @@ func (pgv *PolicyGroupVerifier) VerifyStatement(ctx context.Context, statement * // Validate skip list and log warnings for unknown policy names if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { - pgv.logger.Warn().Err(err).Msg("some policies in skip list were not found in the policy group") + pgv.logger.Warn().Msg(err.Error()) } for _, attachment := range group.GetSpec().GetPolicies().GetAttestation() { @@ -382,7 +383,7 @@ func (pgv *PolicyGroupVerifier) validateSkipList(ctx context.Context, group *v1. // Return error if there are unknown policies if len(unknownPolicies) > 0 { - return fmt.Errorf("policies in skip list not found in group %q: %v", group.GetMetadata().GetName(), unknownPolicies) + return fmt.Errorf("can't skip policy %q from the group %q. Policy not part of the group", strings.Join(unknownPolicies, ", "), group.GetMetadata().GetName()) } return nil From bf5038d4a531e0b848135f5abe24d66532cfd150 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Thu, 20 Nov 2025 16:31:20 +0100 Subject: [PATCH 5/8] chore: upgrade ent golang Signed-off-by: Miguel Martinez --- app/cli/pkg/action/testdata/contract_v2.yaml | 4 +- .../contracts/contract_with_empty_skip.yaml | 14 +++ .../contracts/contract_with_valid_skip.yaml | 15 +++ .../testdata/policy_group_with_embedded.yaml | 56 ++++++++++ app/controlplane/pkg/biz/workflowcontract.go | 100 ++++++++++++++---- .../biz/workflowcontract_integration_test.go | 40 +++++++ pkg/policies/policy_groups.go | 59 ----------- pkg/policies/policy_groups_test.go | 76 ------------- 8 files changed, 205 insertions(+), 159 deletions(-) create mode 100644 app/controlplane/pkg/biz/testdata/contracts/contract_with_empty_skip.yaml create mode 100644 app/controlplane/pkg/biz/testdata/contracts/contract_with_valid_skip.yaml create mode 100644 app/controlplane/pkg/biz/testdata/policy_group_with_embedded.yaml diff --git a/app/cli/pkg/action/testdata/contract_v2.yaml b/app/cli/pkg/action/testdata/contract_v2.yaml index 40b277abe..b3671cd6b 100644 --- a/app/cli/pkg/action/testdata/contract_v2.yaml +++ b/app/cli/pkg/action/testdata/contract_v2.yaml @@ -33,8 +33,8 @@ spec: attestation: - ref: file://attestation-policy.yaml policyGroups: - - ref: file://testdata/policy_group.yaml + - ref: sbom-quality with: user_name: "test-user" skip: - - policy-to-skip \ No newline at end of file + - policy-to-skip diff --git a/app/controlplane/pkg/biz/testdata/contracts/contract_with_empty_skip.yaml b/app/controlplane/pkg/biz/testdata/contracts/contract_with_empty_skip.yaml new file mode 100644 index 000000000..5fd1ec4f4 --- /dev/null +++ b/app/controlplane/pkg/biz/testdata/contracts/contract_with_empty_skip.yaml @@ -0,0 +1,14 @@ +apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: contract-with-empty-skip + description: Test contract with empty skip list +spec: + materials: + - name: my-sbom + type: SBOM_CYCLONEDX_JSON + runner: + type: GITHUB_ACTION + policyGroups: + - ref: file://../testdata/policy_group_with_embedded.yaml + skip: [] diff --git a/app/controlplane/pkg/biz/testdata/contracts/contract_with_valid_skip.yaml b/app/controlplane/pkg/biz/testdata/contracts/contract_with_valid_skip.yaml new file mode 100644 index 000000000..c6cf36667 --- /dev/null +++ b/app/controlplane/pkg/biz/testdata/contracts/contract_with_valid_skip.yaml @@ -0,0 +1,15 @@ +apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: contract-with-valid-skip + description: Test contract with valid policy skip list +spec: + materials: + - name: my-sbom + type: SBOM_CYCLONEDX_JSON + runner: + type: GITHUB_ACTION + policyGroups: + - ref: file://../testdata/policy_group_with_embedded.yaml + skip: + - sbom-version-policy diff --git a/app/controlplane/pkg/biz/testdata/policy_group_with_embedded.yaml b/app/controlplane/pkg/biz/testdata/policy_group_with_embedded.yaml new file mode 100644 index 000000000..34d5a40bf --- /dev/null +++ b/app/controlplane/pkg/biz/testdata/policy_group_with_embedded.yaml @@ -0,0 +1,56 @@ +apiVersion: workflowcontract.chainloop.dev/v1 +kind: PolicyGroup +metadata: + name: test-policy-group + description: Test policy group with embedded policies for skip list validation +spec: + policies: + materials: + - type: SBOM_CYCLONEDX_JSON + policies: + - embedded: + metadata: + name: sbom-version-policy + description: Check SBOM version + spec: + policies: + - kind: SBOM_CYCLONEDX_JSON + embedded: | + package main + import rego.v1 + result := { + "skipped": false, + "violations": [], + "skip_reason": "", + } + - embedded: + metadata: + name: sbom-quality-policy + description: Check SBOM quality + spec: + policies: + - kind: SBOM_CYCLONEDX_JSON + embedded: | + package main + import rego.v1 + result := { + "skipped": false, + "violations": [], + "skip_reason": "", + } + attestation: + - embedded: + metadata: + name: attestation-policy + description: Validate attestation structure + spec: + policies: + - kind: ATTESTATION + embedded: | + package main + import rego.v1 + result := { + "skipped": false, + "violations": [], + "skip_reason": "", + } diff --git a/app/controlplane/pkg/biz/workflowcontract.go b/app/controlplane/pkg/biz/workflowcontract.go index ce1792fd5..eb693c967 100644 --- a/app/controlplane/pkg/biz/workflowcontract.go +++ b/app/controlplane/pkg/biz/workflowcontract.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/bufbuild/protoyaml-go" @@ -444,7 +445,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(rawSchema []byte, to } } for _, gatt := range schema.GetPolicyGroups() { - if _, err := uc.findPolicyGroup(gatt, token); err != nil { + if _, err := uc.findAndValidatePolicyGroup(gatt, token); err != nil { return NewErrValidation(err) } } @@ -464,7 +465,7 @@ func (uc *WorkflowContractUseCase) ValidateContractPolicies(rawSchema []byte, to } } for _, gatt := range spec.GetPolicyGroups() { - if _, err := uc.findPolicyGroup(gatt, token); err != nil { + if _, err := uc.findAndValidatePolicyGroup(gatt, token); err != nil { return NewErrValidation(err) } } @@ -526,33 +527,88 @@ func (uc *WorkflowContractUseCase) findAndValidatePolicy(att *schemav1.PolicyAtt return policy, nil } -func (uc *WorkflowContractUseCase) findPolicyGroup(att *schemav1.PolicyGroupAttachment, token string) (*schemav1.PolicyGroup, error) { +func (uc *WorkflowContractUseCase) findAndValidatePolicyGroup(att *schemav1.PolicyGroupAttachment, token 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 + return nil, nil + } + // if it should come from a provider, check that it's available // [chainloop://][provider/]name - if loader.IsProviderScheme(att.GetRef()) { - pr := loader.ProviderParts(att.GetRef()) - remoteGroup, err := uc.GetPolicyGroup(pr.Provider, pr.Name, pr.OrgName, "", token) - if err != nil { - return nil, NewErrValidation(fmt.Errorf("failed to get policy group: %w", err)) + pr := loader.ProviderParts(att.GetRef()) + remoteGroup, err := uc.GetPolicyGroup(pr.Provider, pr.Name, pr.OrgName, "", token) + if err != nil { + return nil, NewErrValidation(fmt.Errorf("failed to get policy group: %w", err)) + } + + if remoteGroup.PolicyGroup != nil { + // validate group arguments + with := att.GetWith() + for _, input := range remoteGroup.PolicyGroup.GetSpec().GetInputs() { + _, ok := with[input.GetName()] + if !ok && input.GetRequired() { + return nil, NewErrValidation(fmt.Errorf("missing required input %q for group", input.GetName())) + } + + if input.GetRequired() && input.GetDefault() != "" { + return nil, NewErrValidation(fmt.Errorf("input %s can not be required and have a default at the same time", input.GetName())) + } } - if remoteGroup.PolicyGroup != nil { - // validate group arguments - with := att.GetWith() - for _, input := range remoteGroup.PolicyGroup.GetSpec().GetInputs() { - _, ok := with[input.GetName()] - if !ok && input.GetRequired() { - return nil, NewErrValidation(fmt.Errorf("missing required input %q for group", input.GetName())) - } - if input.GetRequired() && input.GetDefault() != "" { - return nil, NewErrValidation(fmt.Errorf("input %s can not be required and have a default at the same time", input.GetName())) - } + } + + // Validate skip list + if err := uc.validateSkipList(remoteGroup.PolicyGroup, att, token); err != nil { + return nil, fmt.Errorf("failed to validate skip list: %w", err) + } + + return remoteGroup.PolicyGroup, nil +} + +// validateSkipList checks if policy names in the skip list exist in the group +// and returns an error if any unknown policy names are found +func (uc *WorkflowContractUseCase) validateSkipList(group *schemav1.PolicyGroup, groupAtt *schemav1.PolicyGroupAttachment, token string) error { + if len(groupAtt.GetSkip()) == 0 { + return nil + } + + // Collect all policy names in the group + policyNames := make(map[string]bool) + policies := group.GetSpec().GetPolicies() + + // Collect material policy names + for _, groupMaterial := range policies.GetMaterials() { + for _, policyAtt := range groupMaterial.GetPolicies() { + policy, err := uc.findAndValidatePolicy(policyAtt, token) + if err != nil { + return fmt.Errorf("failed to get policy name during skip list validation: %w", err) } + policyNames[policy.GetMetadata().GetName()] = true + } + } + + // Collect attestation policy names + for _, policyAtt := range policies.GetAttestation() { + policy, err := uc.findAndValidatePolicy(policyAtt, token) + if err != nil { + return fmt.Errorf("failed to get policy name during skip list validation: %w", err) } - return remoteGroup.PolicyGroup, nil + policyNames[policy.GetMetadata().GetName()] = true } - // Otherwise, don't return an error, as it might consist of a local policy, not available in this context - return nil, nil + // Check each skip entry against collected policy names and collect unknown ones + var unknownPolicies []string + for _, skipName := range groupAtt.GetSkip() { + if !policyNames[skipName] { + unknownPolicies = append(unknownPolicies, skipName) + } + } + + // Return error if there are unknown policies + if len(unknownPolicies) > 0 { + return fmt.Errorf("policy %q not found in policy group %q", strings.Join(unknownPolicies, ", "), group.GetMetadata().GetName()) + } + + return nil } // Delete soft-deletes the entry diff --git a/app/controlplane/pkg/biz/workflowcontract_integration_test.go b/app/controlplane/pkg/biz/workflowcontract_integration_test.go index 3992ddf51..ffa1077bc 100644 --- a/app/controlplane/pkg/biz/workflowcontract_integration_test.go +++ b/app/controlplane/pkg/biz/workflowcontract_integration_test.go @@ -17,6 +17,7 @@ package biz_test import ( "context" + "fmt" "os" "testing" @@ -217,6 +218,24 @@ func (s *workflowContractIntegrationTestSuite) TestCreate() { }, } + // Add skip list validation test cases + // Note: file:// refs are validated at runtime, not at contract creation time + // These tests verify the skip field is accepted and works correctly + skipListTestCases := []struct { + name string + contractPath string + wantErrMsg string + }{ + { + name: "contract with valid skip list", + contractPath: "testdata/contracts/contract_with_valid_skip.yaml", + }, + { + name: "contract with empty skip list", + contractPath: "testdata/contracts/contract_with_empty_skip.yaml", + }, + } + for _, tc := range testCases { s.Run(tc.name, func() { contract, err := s.WorkflowContract.Create(ctx, tc.input) @@ -238,6 +257,27 @@ func (s *workflowContractIntegrationTestSuite) TestCreate() { } }) } + + // Run skip list validation tests + for i, tc := range skipListTestCases { + s.Run(tc.name, func() { + d, err := os.ReadFile(tc.contractPath) + require.NoError(s.T(), err) + // Generate RFC 1123 compliant contract name + contractName := fmt.Sprintf("skip-test-%d", i) + contract, err := s.WorkflowContract.Create(ctx, &biz.WorkflowContractCreateOpts{ + OrgID: s.org.ID, + Name: contractName, + RawSchema: d, + }) + if tc.wantErrMsg != "" { + s.ErrorContains(err, tc.wantErrMsg) + return + } + require.NoError(s.T(), err) + s.NotEmpty(contract.ID) + }) + } } func (s *workflowContractIntegrationTestSuite) TestList() { diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index b46c5dac2..25d9d9648 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "slices" - "strings" v13 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" @@ -68,11 +67,6 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap return nil, NewPolicyError(err) } - // Validate skip list and log warnings for unknown policy names - if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { - pgv.logger.Warn().Msg(err.Error()) - } - // gather required policies policyAtts, err := pgv.requiredPoliciesForMaterial(ctx, material, group, groupAtt, groupArgs) if err != nil { @@ -132,11 +126,6 @@ func (pgv *PolicyGroupVerifier) VerifyStatement(ctx context.Context, statement * return nil, NewPolicyError(err) } - // Validate skip list and log warnings for unknown policy names - if err := pgv.validateSkipList(ctx, group, groupAtt); err != nil { - pgv.logger.Warn().Msg(err.Error()) - } - for _, attachment := range group.GetSpec().GetPolicies().GetAttestation() { // Check if policy should be skipped policyName, err := pgv.getPolicyName(ctx, attachment) @@ -340,51 +329,3 @@ func (pgv *PolicyGroupVerifier) getPolicyName(ctx context.Context, attachment *v // Should never happen due to protobuf validation, but handle defensively return "", errors.New("policy attachment has neither ref nor embedded policy") } - -// validateSkipList checks if policy names in the skip list exist in the group -// and returns an error if any unknown policy names are found -func (pgv *PolicyGroupVerifier) validateSkipList(ctx context.Context, group *v1.PolicyGroup, groupAtt *v1.PolicyGroupAttachment) error { - if len(groupAtt.GetSkip()) == 0 { - return nil - } - - // Collect all policy names in the group - policyNames := make(map[string]bool) - - // Collect material policy names - for _, groupMaterial := range group.GetSpec().GetPolicies().GetMaterials() { - for _, policyAtt := range groupMaterial.GetPolicies() { - name, err := pgv.getPolicyName(ctx, policyAtt) - if err != nil { - pgv.logger.Warn().Err(err).Msg("failed to get policy name during skip list validation") - continue - } - policyNames[name] = true - } - } - - // Collect attestation policy names - for _, policyAtt := range group.GetSpec().GetPolicies().GetAttestation() { - name, err := pgv.getPolicyName(ctx, policyAtt) - if err != nil { - pgv.logger.Warn().Err(err).Msg("failed to get policy name during skip list validation") - continue - } - policyNames[name] = true - } - - // Check each skip entry against collected policy names and collect unknown ones - var unknownPolicies []string - for _, skipName := range groupAtt.GetSkip() { - if !policyNames[skipName] { - unknownPolicies = append(unknownPolicies, skipName) - } - } - - // Return error if there are unknown policies - if len(unknownPolicies) > 0 { - return fmt.Errorf("can't skip policy %q from the group %q. Policy not part of the group", strings.Join(unknownPolicies, ", "), group.GetMetadata().GetName()) - } - - return nil -} diff --git a/pkg/policies/policy_groups_test.go b/pkg/policies/policy_groups_test.go index 7d233ca93..cd34f3c39 100644 --- a/pkg/policies/policy_groups_test.go +++ b/pkg/policies/policy_groups_test.go @@ -612,79 +612,3 @@ func (s *groupsTestSuite) TestSkipBothMaterialAndAttestationPolicies() { s.Require().NoError(err) s.Len(attestationEvs, 0, "attestation policy should be skipped") } - -func (s *groupsTestSuite) TestValidateSkipList() { - cases := []struct { - name string - policyGroup string - skipPolicies []string - expectError bool - errorContains string - }{ - { - name: "empty skip list returns nil", - policyGroup: "file://testdata/policy_group_multikind.yaml", - skipPolicies: []string{}, - expectError: false, - }, - { - name: "nil skip list returns nil", - policyGroup: "file://testdata/policy_group_multikind.yaml", - skipPolicies: nil, - expectError: false, - }, - { - name: "all valid policy names returns nil", - policyGroup: "file://testdata/policy_group_multikind.yaml", - skipPolicies: []string{"policy-result-format"}, - expectError: false, - }, - { - name: "single unknown policy name returns error", - policyGroup: "file://testdata/policy_group_multikind.yaml", - skipPolicies: []string{"non-existent"}, - expectError: true, - errorContains: "non-existent", - }, - { - name: "multiple unknown policy names returns error with all names", - policyGroup: "file://testdata/policy_group_multikind.yaml", - skipPolicies: []string{"non-existent-1", "non-existent-2"}, - expectError: true, - errorContains: "non-existent-1", - }, - { - name: "mix of valid and invalid returns error with only invalid", - policyGroup: "file://testdata/policy_group_multikind.yaml", - skipPolicies: []string{"policy-result-format", "non-existent"}, - expectError: true, - errorContains: "non-existent", - }, - } - - for _, tc := range cases { - s.Run(tc.name, func() { - groupAtt := &v1.PolicyGroupAttachment{ - Ref: tc.policyGroup, - Skip: tc.skipPolicies, - } - - group, _, err := LoadPolicyGroup(context.Background(), groupAtt, &LoadPolicyGroupOptions{ - Logger: &s.logger, - }) - s.Require().NoError(err) - - verifier := NewPolicyGroupVerifier([]*v1.PolicyGroupAttachment{groupAtt}, nil, nil, &s.logger) - err = verifier.validateSkipList(context.Background(), group, groupAtt) - - if tc.expectError { - s.Error(err) - if tc.errorContains != "" { - s.Contains(err.Error(), tc.errorContains) - } - } else { - s.NoError(err) - } - }) - } -} From fbbb39f92669e6e97fdb5d4a4fc41a4d88663e55 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Thu, 20 Nov 2025 17:43:11 +0100 Subject: [PATCH 6/8] chore: upgrade ent golang Signed-off-by: Miguel Martinez --- pkg/policies/policy_groups.go | 58 ++++++++++++++++------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index 25d9d9648..512381d69 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -74,6 +74,17 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap } for _, policyAtt := range policyAtts { + // Check if policy should be skipped + skip, policyName, err := pgv.shouldSkipPolicy(ctx, policyAtt, groupAtt.GetSkip()) + if err != nil { + return nil, NewPolicyError(fmt.Errorf("failed to check if policy should be skipped: %w", err)) + } + + if skip { + pgv.logger.Debug().Str("policy", policyName).Msg("skipping attestation policy per skip list") + continue + } + // Load material content subject, err := material.GetEvaluableContent(path) if err != nil { @@ -128,13 +139,12 @@ func (pgv *PolicyGroupVerifier) VerifyStatement(ctx context.Context, statement * for _, attachment := range group.GetSpec().GetPolicies().GetAttestation() { // Check if policy should be skipped - policyName, err := pgv.getPolicyName(ctx, attachment) + skip, policyName, err := pgv.shouldSkipPolicy(ctx, attachment, groupAtt.GetSkip()) if err != nil { - return nil, NewPolicyError(fmt.Errorf("failed to get policy name: %w", err)) + return nil, NewPolicyError(fmt.Errorf("failed to check if policy should be skipped: %w", err)) } - // Skip if policy name is in the skip list - if slices.Contains(groupAtt.GetSkip(), policyName) { + if skip { pgv.logger.Debug().Str("policy", policyName).Msg("skipping attestation policy per skip list") continue } @@ -247,18 +257,6 @@ func (pgv *PolicyGroupVerifier) requiredPoliciesForMaterial(ctx context.Context, } if apply { - // Check if policy should be skipped - policyName, err := pgv.getPolicyName(ctx, policyAtt) - if err != nil { - return nil, fmt.Errorf("failed to get policy name: %w", err) - } - - // Skip if policy name is in the skip list - if slices.Contains(groupAtt.GetSkip(), policyName) { - pgv.logger.Debug().Str("policy", policyName).Msg("skipping policy per skip list") - continue - } - result = append(result, policyAtt) } } @@ -308,24 +306,22 @@ func (pgv *PolicyGroupVerifier) shouldApplyPolicy(ctx context.Context, policyAtt return false, nil } -// getPolicyName extracts the metadata.name from a PolicyAttachment +// shouldSkipPolicy checks if a policy should be skipped based on the skip list // It handles both embedded and referenced policies by loading the policy spec when needed -func (pgv *PolicyGroupVerifier) getPolicyName(ctx context.Context, attachment *v1.PolicyAttachment) (string, error) { - // Case 1: Embedded policy - direct access - if embedded := attachment.GetEmbedded(); embedded != nil { - return embedded.GetMetadata().GetName(), nil +func (pgv *PolicyGroupVerifier) shouldSkipPolicy(ctx context.Context, attachment *v1.PolicyAttachment, skipList []string) (bool, string, error) { + if len(skipList) == 0 { + return false, "", nil } - // Case 2: Referenced policy - must load it - if ref := attachment.GetRef(); ref != "" { - // Load the policy spec using existing loader infrastructure - policy, _, err := pgv.loadPolicySpec(ctx, attachment) - if err != nil { - return "", fmt.Errorf("failed to load policy to get name: %w", err) - } - return policy.GetMetadata().GetName(), nil + policy, _, err := pgv.loadPolicySpec(ctx, attachment) + if err != nil { + return false, "", fmt.Errorf("failed to load policy to get name: %w", err) + } + + policyName := policy.GetMetadata().GetName() + if slices.Contains(skipList, policyName) { + return true, policyName, nil } - // Should never happen due to protobuf validation, but handle defensively - return "", errors.New("policy attachment has neither ref nor embedded policy") + return false, "", nil } From f63f3760a8da6bbaf5a0f2afcdaa9f2b09c15287 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Thu, 20 Nov 2025 19:09:15 +0100 Subject: [PATCH 7/8] chore: upgrade ent golang Signed-off-by: Miguel Martinez --- pkg/policies/policy_groups.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index 512381d69..d9a1bb220 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -68,7 +68,7 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap } // gather required policies - policyAtts, err := pgv.requiredPoliciesForMaterial(ctx, material, group, groupAtt, groupArgs) + policyAtts, err := pgv.requiredPoliciesForMaterial(ctx, material, group, groupArgs) if err != nil { return nil, NewPolicyError(err) } @@ -234,7 +234,7 @@ func getGroupLoader(attachment *v1.PolicyGroupAttachment, opts *LoadPolicyGroupO } // Gets the policies that can be applied to a material within a group -func (pgv *PolicyGroupVerifier) requiredPoliciesForMaterial(ctx context.Context, material *api.Attestation_Material, group *v1.PolicyGroup, groupAtt *v1.PolicyGroupAttachment, groupArgs map[string]string) ([]*v1.PolicyAttachment, error) { +func (pgv *PolicyGroupVerifier) requiredPoliciesForMaterial(ctx context.Context, material *api.Attestation_Material, group *v1.PolicyGroup, groupArgs map[string]string) ([]*v1.PolicyAttachment, error) { result := make([]*v1.PolicyAttachment, 0) // 2. go through all materials in the group and look for the crafted material From 0bef66506d8f70b85b0799cae16ada69529da463 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Thu, 20 Nov 2025 19:15:50 +0100 Subject: [PATCH 8/8] chore: upgrade ent golang Signed-off-by: Miguel Martinez --- pkg/policies/policy_groups_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/policies/policy_groups_test.go b/pkg/policies/policy_groups_test.go index cd34f3c39..7f8f2ac20 100644 --- a/pkg/policies/policy_groups_test.go +++ b/pkg/policies/policy_groups_test.go @@ -170,7 +170,7 @@ func (s *groupsTestSuite) TestRequiredPoliciesForMaterial() { } group, _, err := new(FileGroupLoader).Load(context.TODO(), groupAtt) s.Require().NoError(err) - atts, err := v.requiredPoliciesForMaterial(context.TODO(), material, group, groupAtt, nil) + atts, err := v.requiredPoliciesForMaterial(context.TODO(), material, group, nil) s.Require().NoError(err) s.Len(atts, tc.expected) })