diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e8d7c3628..984f57925 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -56,6 +56,7 @@ jobs: token: ${{ secrets.buf_api_token }} breaking: true pr_comment: false + exclude_imports: true lint-dagger-module: runs-on: ubuntu-latest diff --git a/app/cli/cmd/attestation_push.go b/app/cli/cmd/attestation_push.go index 9b9972d8d..4efa77909 100644 --- a/app/cli/cmd/attestation_push.go +++ b/app/cli/cmd/attestation_push.go @@ -64,7 +64,7 @@ func newAttestationPushCmd() *cobra.Command { Annotations: map[string]string{ useAPIToken: "true", }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { info, err := executableInfo() if err != nil { return fmt.Errorf("getting executable information: %w", err) @@ -194,17 +194,11 @@ func validatePolicyEnforcement(status *action.AttestationStatusResult, bypassPol } } - // Do a final check in case the operator has configured the attestation - // to be blocked on any policy violation. - if status.MustBlockOnPolicyViolations { - if bypassPolicyCheck { - logger.Warn().Msg(exceptionBypassPolicyCheck) - return nil - } - - if status.HasPolicyViolations { - return ErrBlockedByPolicyViolation - } + // Block on any policy violation only when configured (bypass handled above). + // When we have policy evaluations, gate semantics are already enforced in the loop above. + if status.MustBlockOnPolicyViolations && bypassPolicyCheck { + logger.Warn().Msg(exceptionBypassPolicyCheck) + return nil } return nil diff --git a/app/cli/cmd/attestation_push_test.go b/app/cli/cmd/attestation_push_test.go index 101662a66..0100e805a 100644 --- a/app/cli/cmd/attestation_push_test.go +++ b/app/cli/cmd/attestation_push_test.go @@ -67,4 +67,25 @@ func TestValidatePolicyEnforcement(t *testing.T) { require.ErrorAs(t, err, &gateErr) require.Equal(t, "cdx-fresh", gateErr.PolicyName) }) + + t.Run("does not block when strategy is enforced and policy is explicitly not gated", func(t *testing.T) { + status := &action.AttestationStatusResult{ + PolicyEvaluations: map[string][]*action.PolicyEvaluation{ + "materials": { + { + Name: "cdx-fresh", + Gate: false, + Violations: []*action.PolicyViolation{ + {Message: "policy violation"}, + }, + }, + }, + }, + HasPolicyViolations: true, + MustBlockOnPolicyViolations: true, + } + + err := validatePolicyEnforcement(status, false) + require.NoError(t, err) + }) } 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 da3e00ec3..6c6eb8aab 100644 --- a/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts +++ b/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts @@ -487,8 +487,13 @@ export interface PolicyAttachment { with: { [key: string]: string }; /** List of requirements this policy contributes to satisfy */ requirements: string[]; - /** If true, the policy will act as a gate, returning an error code if the policy fails */ - gate: boolean; + /** + * Controls whether policy violations act as a gate. + * - true: policy violations are blocking for this policy + * - false: policy violations are non-blocking for this policy + * - unset: inherit organization-level default behavior + */ + gate?: boolean | undefined; } export interface PolicyAttachment_WithEntry { @@ -1414,7 +1419,7 @@ function createBasePolicyAttachment(): PolicyAttachment { disabled: false, with: {}, requirements: [], - gate: false, + gate: undefined, }; } @@ -1438,7 +1443,7 @@ export const PolicyAttachment = { for (const v of message.requirements) { writer.uint32(50).string(v!); } - if (message.gate === true) { + if (message.gate !== undefined) { writer.uint32(56).bool(message.gate); } return writer; @@ -1525,7 +1530,7 @@ export const PolicyAttachment = { }, {}) : {}, requirements: Array.isArray(object?.requirements) ? object.requirements.map((e: any) => String(e)) : [], - gate: isSet(object.gate) ? Boolean(object.gate) : false, + gate: isSet(object.gate) ? Boolean(object.gate) : undefined, }; }, @@ -1572,7 +1577,7 @@ export const PolicyAttachment = { return acc; }, {}); message.requirements = object.requirements?.map((e) => e) || []; - message.gate = object.gate ?? false; + message.gate = object.gate ?? undefined; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.jsonschema.json b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.jsonschema.json index f3b99559c..1ab915cdb 100644 --- a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.jsonschema.json @@ -13,7 +13,7 @@ "description": "meant to be used to embed the policy in the contract" }, "gate": { - "description": "If true, the policy will act as a gate, returning an error code if the policy fails", + "description": "Controls whether policy violations act as a gate.\n - true: policy violations are blocking for this policy\n - false: policy violations are non-blocking for this policy\n - unset: inherit organization-level default behavior", "type": "boolean" }, "ref": { diff --git a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.schema.json b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.schema.json index bfcf6c43e..a7764cefe 100644 --- a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.schema.json +++ b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.PolicyAttachment.schema.json @@ -13,7 +13,7 @@ "description": "meant to be used to embed the policy in the contract" }, "gate": { - "description": "If true, the policy will act as a gate, returning an error code if the policy fails", + "description": "Controls whether policy violations act as a gate.\n - true: policy violations are blocking for this policy\n - false: policy violations are non-blocking for this policy\n - unset: inherit organization-level default behavior", "type": "boolean" }, "ref": { diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go index 6f7a1d2e3..06cfb540b 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go @@ -713,8 +713,11 @@ type PolicyAttachment struct { With map[string]string `protobuf:"bytes,5,rep,name=with,proto3" json:"with,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` // List of requirements this policy contributes to satisfy Requirements []string `protobuf:"bytes,6,rep,name=requirements,proto3" json:"requirements,omitempty"` - // If true, the policy will act as a gate, returning an error code if the policy fails - Gate bool `protobuf:"varint,7,opt,name=gate,proto3" json:"gate,omitempty"` + // Controls whether policy violations act as a gate. + // - true: policy violations are blocking for this policy + // - false: policy violations are non-blocking for this policy + // - unset: inherit organization-level default behavior + Gate *bool `protobuf:"varint,7,opt,name=gate,proto3,oneof" json:"gate,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -803,8 +806,8 @@ func (x *PolicyAttachment) GetRequirements() []string { } func (x *PolicyAttachment) GetGate() bool { - if x != nil { - return x.Gate + if x != nil && x.Gate != nil { + return *x.Gate } return false } @@ -1987,21 +1990,22 @@ const file_workflowcontract_v1_crafting_schema_proto_rawDesc = "" + "\x05value\x18\x02 \x01(\tR\x05value\"\x98\x01\n" + "\bPolicies\x12C\n" + "\tmaterials\x18\x01 \x03(\v2%.workflowcontract.v1.PolicyAttachmentR\tmaterials\x12G\n" + - "\vattestation\x18\x02 \x03(\v2%.workflowcontract.v1.PolicyAttachmentR\vattestation\"\x8a\x04\n" + + "\vattestation\x18\x02 \x03(\v2%.workflowcontract.v1.PolicyAttachmentR\vattestation\"\x98\x04\n" + "\x10PolicyAttachment\x12\x1b\n" + "\x03ref\x18\x01 \x01(\tB\a\xbaH\x04r\x02\x10\x01H\x00R\x03ref\x129\n" + "\bembedded\x18\x02 \x01(\v2\x1b.workflowcontract.v1.PolicyH\x00R\bembedded\x12R\n" + "\bselector\x18\x03 \x01(\v26.workflowcontract.v1.PolicyAttachment.MaterialSelectorR\bselector\x12\x1a\n" + "\bdisabled\x18\x04 \x01(\bR\bdisabled\x12C\n" + "\x04with\x18\x05 \x03(\v2/.workflowcontract.v1.PolicyAttachment.WithEntryR\x04with\x12c\n" + - "\frequirements\x18\x06 \x03(\tB?\xbaH<\x92\x019\"7r523^([a-z0-9-]+\\/)?([^\\s\\/]+\\/)([^\\s@\\/]+)(@[^\\s@]+)?$R\frequirements\x12\x12\n" + - "\x04gate\x18\a \x01(\bR\x04gate\x1a7\n" + + "\frequirements\x18\x06 \x03(\tB?\xbaH<\x92\x019\"7r523^([a-z0-9-]+\\/)?([^\\s\\/]+\\/)([^\\s@\\/]+)(@[^\\s@]+)?$R\frequirements\x12\x17\n" + + "\x04gate\x18\a \x01(\bH\x01R\x04gate\x88\x01\x01\x1a7\n" + "\tWithEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\x1a&\n" + "\x10MaterialSelector\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04nameB\x0f\n" + - "\x06policy\x12\x05\xbaH\x02\b\x01\"\xf6\x01\n" + + "\x06policy\x12\x05\xbaH\x02\b\x01B\a\n" + + "\x05_gate\"\xf6\x01\n" + "\x06Policy\x12I\n" + "\vapi_version\x18\x01 \x01(\tB(\xbaH%r#\n" + "!workflowcontract.chainloop.dev/v1R\n" + diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto index 9d9185345..6e2e1f25e 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto @@ -236,8 +236,11 @@ message PolicyAttachment { } }]; - // If true, the policy will act as a gate, returning an error code if the policy fails - bool gate = 7; + // Controls whether policy violations act as a gate. + // - true: policy violations are blocking for this policy + // - false: policy violations are non-blocking for this policy + // - unset: inherit organization-level default behavior + optional bool gate = 7; message MaterialSelector { // material name diff --git a/buf.yaml b/buf.yaml index 7a9c5041b..a12692e91 100644 --- a/buf.yaml +++ b/buf.yaml @@ -49,6 +49,7 @@ modules: except: - EXTENSION_NO_DELETE - FIELD_SAME_DEFAULT + - FIELD_SAME_CARDINALITY - path: app/controlplane/internal/conf lint: use: diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index 99f3f8928..5918e0648 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -728,7 +728,14 @@ func (c *Crafter) addMaterial(ctx context.Context, m *schemaapi.CraftingSchema_M return i.MaterialName == m.Name }) - pgv := policies.NewPolicyGroupVerifier(c.CraftingState.GetPolicyGroups(), c.CraftingState.GetPolicies(), c.attClient, c.Logger, policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...)) + pgv := policies.NewPolicyGroupVerifier( + c.CraftingState.GetPolicyGroups(), + c.CraftingState.GetPolicies(), + c.attClient, + c.Logger, + policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...), + policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()), + ) policyGroupResults, err := pgv.VerifyMaterial(ctx, mt, value) if err != nil { return nil, fmt.Errorf("error applying policy groups to material: %w", err) @@ -739,7 +746,13 @@ func (c *Crafter) addMaterial(ctx context.Context, m *schemaapi.CraftingSchema_M policies.LogPolicyEvaluations(policyGroupResults, c.Logger) // Validate policies - pv := policies.NewPolicyVerifier(c.CraftingState.GetPolicies(), c.attClient, c.Logger, policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...)) + pv := policies.NewPolicyVerifier( + c.CraftingState.GetPolicies(), + c.attClient, + c.Logger, + policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...), + policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()), + ) policyResults, err := pv.VerifyMaterial(ctx, mt, value) if err != nil { return nil, fmt.Errorf("error applying policies to material: %w", err) @@ -772,6 +785,7 @@ func (c *Crafter) EvaluateAttestationPolicies(ctx context.Context, attestationID // evaluate attestation-level policies pv := policies.NewPolicyVerifier(c.CraftingState.GetPolicies(), c.attClient, c.Logger, policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...), + policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()), policies.WithEvalPhase(phase), ) policyEvaluations, err := pv.VerifyStatement(ctx, statement) @@ -781,6 +795,7 @@ func (c *Crafter) EvaluateAttestationPolicies(ctx context.Context, attestationID pgv := policies.NewPolicyGroupVerifier(c.CraftingState.GetPolicyGroups(), c.CraftingState.GetPolicies(), c.attClient, c.Logger, policies.WithAllowedHostnames(c.CraftingState.Attestation.PoliciesAllowedHostnames...), + policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()), policies.WithEvalPhase(phase), ) policyGroupResults, err := pgv.VerifyStatement(ctx, statement) diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index 625101765..486cf58ee 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -78,6 +78,7 @@ type PolicyVerifier struct { client v13.AttestationServiceClient grpcConn *grpc.ClientConn allowedHostnames []string + defaultGate bool includeRawData bool enablePrint bool evalPhase EvalPhase @@ -87,6 +88,7 @@ var _ Verifier = (*PolicyVerifier)(nil) type PolicyVerifierOptions struct { AllowedHostnames []string + DefaultGate bool IncludeRawData bool EnablePrint bool GRPCConn *grpc.ClientConn @@ -101,6 +103,12 @@ func WithAllowedHostnames(hostnames ...string) PolicyVerifierOption { } } +func WithDefaultGate(defaultGate bool) PolicyVerifierOption { + return func(o *PolicyVerifierOptions) { + o.DefaultGate = defaultGate + } +} + func WithIncludeRawData(include bool) PolicyVerifierOption { return func(o *PolicyVerifierOptions) { o.IncludeRawData = include @@ -137,6 +145,7 @@ func NewPolicyVerifier(policies *v1.Policies, client v13.AttestationServiceClien logger: logger, grpcConn: options.GRPCConn, allowedHostnames: options.AllowedHostnames, + defaultGate: options.DefaultGate, includeRawData: options.IncludeRawData, enablePrint: options.EnablePrint, evalPhase: options.EvalPhase, @@ -336,10 +345,22 @@ func (pv *PolicyVerifier) evaluatePolicyAttachment(ctx context.Context, attachme SkipReasons: reasons, Requirements: attachment.Requirements, RawResults: engineRawResultsToAPIRawResults(rawResults), - Gate: attachment.GetGate(), + Gate: policyAttachmentGate(attachment, pv.defaultGate), }, nil } +func policyAttachmentGate(attachment *v1.PolicyAttachment, defaultGate bool) bool { + if attachment == nil { + return defaultGate + } + + if attachment.Gate != nil { + return attachment.GetGate() + } + + return defaultGate +} + // ComputeArguments takes a list of arguments, and matches it against the expected inputs. It also applies a set of interpolations if needed. func ComputeArguments(name string, inputs []*v1.PolicyInput, args map[string]string, bindings map[string]string, logger *zerolog.Logger) (map[string]string, error) { result := make(map[string]string) diff --git a/pkg/policies/policies_test.go b/pkg/policies/policies_test.go index 4e165cd42..b0b8a046a 100644 --- a/pkg/policies/policies_test.go +++ b/pkg/policies/policies_test.go @@ -1434,3 +1434,57 @@ func (s *testSuite) TestIsURLPath() { }) } } + +func (s *testSuite) TestPolicyAttachmentGate() { + trueGate := true + falseGate := false + + cases := []struct { + name string + attachment *v12.PolicyAttachment + defaultGate bool + expectedGate bool + }{ + { + name: "nil attachment falls back to default true", + attachment: nil, + defaultGate: true, + expectedGate: true, + }, + { + name: "unset gate inherits default false", + attachment: &v12.PolicyAttachment{}, + defaultGate: false, + expectedGate: false, + }, + { + name: "unset gate inherits default true", + attachment: &v12.PolicyAttachment{}, + defaultGate: true, + expectedGate: true, + }, + { + name: "explicit gate false overrides default true", + attachment: &v12.PolicyAttachment{ + Gate: &falseGate, + }, + defaultGate: true, + expectedGate: false, + }, + { + name: "explicit gate true overrides default false", + attachment: &v12.PolicyAttachment{ + Gate: &trueGate, + }, + defaultGate: false, + expectedGate: true, + }, + } + + for _, tc := range cases { + s.Run(tc.name, func() { + got := policyAttachmentGate(tc.attachment, tc.defaultGate) + s.Equal(tc.expectedGate, got) + }) + } +}