diff --git a/app/cli/cmd/workflow_workflow_run_describe.go b/app/cli/cmd/workflow_workflow_run_describe.go index f6bb4f35c..34c7ad5b2 100644 --- a/app/cli/cmd/workflow_workflow_run_describe.go +++ b/app/cli/cmd/workflow_workflow_run_describe.go @@ -299,31 +299,44 @@ func policiesTable(evs []*action.PolicyEvaluation, mt table.Writer, debugMode bo } // appendPolicySection renders the attestation-level policies, either as the -// usual per-policy rows or, when the server declined to inline the -// evaluations, as a notice pointing at the bundle in the CAS backend. +// usual per-policy rows or, when the server declined to inline the evaluations, +// as a notice explaining why. Either way it closes with the bundle in the CAS +// backend, which holds the full set of evaluations the table only summarizes. func appendPolicySection(att *action.WorkflowRunAttestationItem, gt table.Writer, debugMode bool) { - if notice := policyEvaluationsRefNotice(att.PolicyEvaluationsRef, att.PolicyEvaluationStatus); notice != nil { + ref := att.PolicyEvaluationsRef + + // A missing reference means the evaluations, if any, came inline: there is + // no bundle to point at. + if ref == nil || ref.Inlined { + if evs := att.PolicyEvaluations[chainloop.AttPolicyEvaluation]; len(evs) > 0 { + gt.AppendRow(table.Row{"Policies", "------"}) + policiesTable(evs, gt, debugMode) + } + } else { gt.AppendRow(table.Row{"Policies", "------"}) - for _, line := range notice { + for _, line := range policyEvaluationsRefNotice(ref, att.PolicyEvaluationStatus) { gt.AppendRow(table.Row{"", line}) } - - return } - evs := att.PolicyEvaluations[chainloop.AttPolicyEvaluation] - if len(evs) == 0 { + appendPolicyEvaluationsBundleRow(ref, gt) +} + +// appendPolicyEvaluationsBundleRow points at the policy-evaluation bundle +// whenever there is a digest to point at, including when the server could not +// read it: the caller may well have access the control plane lacked. +func appendPolicyEvaluationsBundleRow(ref *action.PolicyEvaluationsRef, gt table.Writer) { + if ref == nil || ref.Digest == "" { return } - gt.AppendRow(table.Row{"Policies", "------"}) - policiesTable(evs, gt, debugMode) + gt.AppendRow(table.Row{"Policy evaluations bundle", downloadPolicyEvaluationsHint(ref)}) } // policyEvaluationsRefNotice renders the lines shown in place of the policy // table when the server returned a reference instead of the evaluations. It // leads with the counters, which stay accurate no matter how large the bundle -// is, and only offers a download when the bundle is known to be there. +// is. func policyEvaluationsRefNotice(ref *action.PolicyEvaluationsRef, status *action.PolicyEvaluationStatus) []string { if ref == nil { return nil @@ -339,12 +352,7 @@ func policyEvaluationsRefNotice(ref *action.PolicyEvaluationsRef, status *action // Without counters there is nothing to summarize, so name the subject instead. if status == nil { - lines := []string{fmt.Sprintf("policy evaluations %s", reason)} - if ref.Reason == action.PolicyEvaluationsRefReasonTooLarge { - lines = append(lines, downloadPolicyEvaluationsHint(ref)) - } - - return lines + return []string{fmt.Sprintf("policy evaluations %s", reason)} } counters := fmt.Sprintf("%d evaluations, %d violations", status.Total, status.Violated) @@ -352,16 +360,11 @@ func policyEvaluationsRefNotice(ref *action.PolicyEvaluationsRef, status *action counters = fmt.Sprintf("%s (%d suppressed)", counters, status.Suppressed) } - lines := []string{fmt.Sprintf("%s - %s", counters, reason)} - if ref.Reason == action.PolicyEvaluationsRefReasonTooLarge { - lines = append(lines, downloadPolicyEvaluationsHint(ref)) - } - - return lines + return []string{fmt.Sprintf("%s - %s", counters, reason)} } func downloadPolicyEvaluationsHint(ref *action.PolicyEvaluationsRef) string { - return fmt.Sprintf("inspect with: chainloop artifact download --digest %s", ref.Digest) + return fmt.Sprintf("chainloop artifact download --digest %s", ref.Digest) } // violationSummary builds a single-line description of a violation using the @@ -512,10 +515,11 @@ func encodeAttestationOutput(run *action.WorkflowRunItemFull, writer io.Writer) if err != nil { return fmt.Errorf("unmarshaling attestation: %w", err) } + return output.EncodeProtoJSON(&bundle) - } else { - return output.EncodeJSON(run.Attestation.Envelope) } + + return output.EncodeJSON(run.Attestation.Envelope) case formatPayloadPAE: return encodePAE(run, writer) default: diff --git a/app/cli/cmd/workflow_workflow_run_describe_test.go b/app/cli/cmd/workflow_workflow_run_describe_test.go index 04c886cfb..59b4c5337 100644 --- a/app/cli/cmd/workflow_workflow_run_describe_test.go +++ b/app/cli/cmd/workflow_workflow_run_describe_test.go @@ -213,8 +213,11 @@ func (s *workflowRunDescribeSuite) TestOutputTypePayload() { const ( testRefDigest = "sha256:abc123" - testRefDownloadHint = "inspect with: chainloop artifact download --digest " + testRefDigest + testRefDownloadHint = "chainloop artifact download --digest " + testRefDigest policiesRowLabel = "Policies" + bundleRowLabel = "Policy evaluations bundle" + testPolicyName = "strong-acl" + testViolationMsg = "weak ACL" ) func TestPolicyEvaluationsRefNotice(t *testing.T) { @@ -237,7 +240,6 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { status: &action.PolicyEvaluationStatus{Total: 12, Violated: 134112, Suppressed: 86321}, want: []string{ "12 evaluations, 134112 violations (86321 suppressed) - too large to include inline (64M)", - testRefDownloadHint, }, }, { @@ -250,7 +252,6 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { status: &action.PolicyEvaluationStatus{Total: 2, Violated: 40}, want: []string{ "2 evaluations, 40 violations - too large to include inline (3M)", - testRefDownloadHint, }, }, { @@ -262,11 +263,10 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { status: &action.PolicyEvaluationStatus{Total: 2, Violated: 40}, want: []string{ "2 evaluations, 40 violations - too large to include inline", - testRefDownloadHint, }, }, { - name: "unavailable bundle does not suggest a download", + name: "unavailable bundle reports why it could not be read", ref: &action.PolicyEvaluationsRef{ Digest: testRefDigest, Reason: action.PolicyEvaluationsRefReasonUnavailable, @@ -285,7 +285,6 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { }, want: []string{ "policy evaluations too large to include inline (1K)", - testRefDownloadHint, }, }, } @@ -298,6 +297,12 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { } func TestAppendPolicySection(t *testing.T) { + inlinedEvaluations := map[string][]*action.PolicyEvaluation{ + chainloop.AttPolicyEvaluation: { + {Name: testPolicyName, Violations: []*action.PolicyViolation{{Message: testViolationMsg}}}, + }, + } + tests := []struct { name string attestation *action.WorkflowRunAttestationItem @@ -305,17 +310,26 @@ func TestAppendPolicySection(t *testing.T) { wantAbsent []string }{ { - name: "inlined evaluations are rendered as policy rows", + name: "inlined evaluations are rendered as policy rows alongside the bundle", attestation: &action.WorkflowRunAttestationItem{ - PolicyEvaluations: map[string][]*action.PolicyEvaluation{ - chainloop.AttPolicyEvaluation: { - {Name: "strong-acl", Violations: []*action.PolicyViolation{{Message: "weak ACL"}}}, - }, + PolicyEvaluations: inlinedEvaluations, + PolicyEvaluationStatus: &action.PolicyEvaluationStatus{Total: 1, Violated: 1}, + PolicyEvaluationsRef: &action.PolicyEvaluationsRef{ + Digest: testRefDigest, + SizeBytes: 2048, + Inlined: true, }, + }, + wantContain: []string{policiesRowLabel, testPolicyName, testViolationMsg, bundleRowLabel, testRefDownloadHint}, + }, + { + name: "evaluations without a bundle render the policy rows alone", + attestation: &action.WorkflowRunAttestationItem{ + PolicyEvaluations: inlinedEvaluations, PolicyEvaluationStatus: &action.PolicyEvaluationStatus{Total: 1, Violated: 1}, }, - wantContain: []string{policiesRowLabel, "strong-acl", "weak ACL"}, - wantAbsent: []string{"artifact download"}, + wantContain: []string{policiesRowLabel, testPolicyName, testViolationMsg}, + wantAbsent: []string{"artifact download", bundleRowLabel}, }, { name: "an oversized bundle is rendered as a notice instead", @@ -327,7 +341,30 @@ func TestAppendPolicySection(t *testing.T) { Reason: action.PolicyEvaluationsRefReasonTooLarge, }, }, - wantContain: []string{policiesRowLabel, "134112 violations", "too large", "artifact download --digest " + testRefDigest}, + wantContain: []string{policiesRowLabel, "134112 violations", "too large", bundleRowLabel, testRefDownloadHint}, + wantAbsent: []string{testPolicyName}, + }, + { + name: "an unavailable bundle still offers the download when the digest is known", + attestation: &action.WorkflowRunAttestationItem{ + PolicyEvaluationStatus: &action.PolicyEvaluationStatus{Total: 3, Violated: 7}, + PolicyEvaluationsRef: &action.PolicyEvaluationsRef{ + Digest: testRefDigest, + Reason: action.PolicyEvaluationsRefReasonUnavailable, + }, + }, + wantContain: []string{policiesRowLabel, "could not be retrieved", bundleRowLabel, testRefDownloadHint}, + }, + { + name: "a reference without a digest cannot offer a download", + attestation: &action.WorkflowRunAttestationItem{ + PolicyEvaluationStatus: &action.PolicyEvaluationStatus{Total: 3, Violated: 7}, + PolicyEvaluationsRef: &action.PolicyEvaluationsRef{ + Reason: action.PolicyEvaluationsRefReasonUnavailable, + }, + }, + wantContain: []string{policiesRowLabel, "could not be retrieved"}, + wantAbsent: []string{"artifact download", bundleRowLabel}, }, { name: "no policies and no reference renders nothing", @@ -336,6 +373,17 @@ func TestAppendPolicySection(t *testing.T) { }, wantAbsent: []string{"Policies"}, }, + { + name: "a bundle with no attestation-level policies still points at it", + attestation: &action.WorkflowRunAttestationItem{ + PolicyEvaluationStatus: &action.PolicyEvaluationStatus{Total: 1}, + PolicyEvaluationsRef: &action.PolicyEvaluationsRef{ + Digest: testRefDigest, + Inlined: true, + }, + }, + wantContain: []string{bundleRowLabel, testRefDownloadHint}, + }, } for _, tc := range tests { diff --git a/app/cli/pkg/action/attestation_push.go b/app/cli/pkg/action/attestation_push.go index f781efe01..ea462f618 100644 --- a/app/cli/pkg/action/attestation_push.go +++ b/app/cli/pkg/action/attestation_push.go @@ -343,9 +343,11 @@ func decodeEnvelope(rawEnvelope []byte) (*dsse.Envelope, error) { } // uploadPolicyEvaluationsBundle serializes policy evaluations as a protobuf bundle, -// uploads to CAS, and returns a ResourceDescriptor referencing the uploaded object. +// uploads to CAS, and returns a reference to the uploaded object. The reference +// records the size of the uploaded bytes so readers can decide whether to fetch +// the bundle without asking the CAS how big it is. // Returns (nil, nil) when there are no evaluations or no uploader. -func uploadPolicyEvaluationsBundle(ctx context.Context, evaluations []*v1.PolicyEvaluation, uploader casclient.Uploader) (*intoto.ResourceDescriptor, error) { +func uploadPolicyEvaluationsBundle(ctx context.Context, evaluations []*v1.PolicyEvaluation, uploader casclient.Uploader) (*crChainloop.PolicyEvaluationsRef, error) { if len(evaluations) == 0 || uploader == nil { return nil, nil } @@ -364,9 +366,12 @@ func uploadPolicyEvaluationsBundle(ctx context.Context, evaluations []*v1.Policy return nil, fmt.Errorf("uploading policy evaluation bundle: %w", err) } - return &intoto.ResourceDescriptor{ - Name: "policy-evaluations", - Digest: map[string]string{"sha256": hexDigest}, - MediaType: crChainloop.PolicyEvaluationsBundleMediaType, + return &crChainloop.PolicyEvaluationsRef{ + ResourceDescriptor: &intoto.ResourceDescriptor{ + Name: "policy-evaluations", + Digest: map[string]string{"sha256": hexDigest}, + MediaType: crChainloop.PolicyEvaluationsBundleMediaType, + }, + SizeBytes: int64(len(data)), }, nil } diff --git a/app/cli/pkg/action/attestation_push_test.go b/app/cli/pkg/action/attestation_push_test.go index 49e7dc713..2225db583 100644 --- a/app/cli/pkg/action/attestation_push_test.go +++ b/app/cli/pkg/action/attestation_push_test.go @@ -117,6 +117,10 @@ func TestUploadPolicyEvaluationsBundle(t *testing.T) { require.NoError(t, err) expectedDigest := fmt.Sprintf("%x", sha256.Sum256(data)) assert.Equal(t, expectedDigest, ref.Digest["sha256"]) + + // The recorded size is what the reader gates the inlining cap on, + // so it has to be the size of the very bytes that were uploaded. + assert.Equal(t, int64(len(data)), ref.GetSizeBytes()) }) } } diff --git a/app/cli/pkg/action/workflow_run_describe.go b/app/cli/pkg/action/workflow_run_describe.go index 8d36c55cb..b2891eef9 100644 --- a/app/cli/pkg/action/workflow_run_describe.go +++ b/app/cli/pkg/action/workflow_run_describe.go @@ -63,9 +63,10 @@ type WorkflowRunAttestationItem struct { PolicyEvaluations map[string][]*PolicyEvaluation `json:"policy_evaluations,omitempty"` // Policy evaluation status PolicyEvaluationStatus *PolicyEvaluationStatus `json:"policy_evaluation_status,omitempty"` - // Set when the evaluations were not inlined above and must be fetched from - // the CAS backend instead. PolicyEvaluations is empty in that case, while - // PolicyEvaluationStatus stays complete. + // Where the policy-evaluation bundle lives in the CAS backend, set whenever + // the attestation carries one. When Inlined is false the evaluations above + // are empty and must be fetched from there instead; PolicyEvaluationStatus + // stays complete either way. PolicyEvaluationsRef *PolicyEvaluationsRef `json:"policy_evaluations_ref,omitempty"` // URL to view the attestation in the UI AttestationViewURL string `json:"attestation_view_url"` @@ -112,7 +113,7 @@ type Annotation struct { } // PolicyEvaluationsRefReason explains why the evaluations were not included -// in the response. +// in the response. Empty when they were. type PolicyEvaluationsRefReason string const ( @@ -123,13 +124,16 @@ const ( ) // PolicyEvaluationsRef points at a policy-evaluation bundle stored in a CAS -// backend, returned in place of the evaluations themselves. +// backend. It is returned both alongside the evaluations and, when they could +// not be inlined, in their place. type PolicyEvaluationsRef struct { Digest string `json:"digest"` // Size of the bundle in bytes, zero when it could not be determined SizeBytes int64 `json:"size_bytes,omitempty"` MediaType string `json:"media_type,omitempty"` - Reason PolicyEvaluationsRefReason `json:"reason"` + Reason PolicyEvaluationsRefReason `json:"reason,omitempty"` + // Whether the evaluations decoded from this bundle are also in the response + Inlined bool `json:"inlined"` } type PolicyEvaluation struct { @@ -339,18 +343,28 @@ func trustedRootPbToVerifier(resp *pb.GetTrustedRootResponse) (*verifier.Trusted return tr, nil } -// pbPolicyEvaluationsRefToAction maps the reference the server returns when it -// declines to inline the evaluations. An unspecified reason is treated as -// unavailable, which is the more conservative rendering: it does not promise -// the caller that a download would succeed. +// pbPolicyEvaluationsRefToAction maps the reference to the policy-evaluation +// bundle. A reason the client does not recognize is reported as unavailable +// unless the server also inlined the evaluations, which is the more +// conservative rendering: it never promises that a download would succeed, and +// never invents a failure the server did not report. func pbPolicyEvaluationsRefToAction(in *pb.PolicyEvaluationsRef) *PolicyEvaluationsRef { if in == nil { return nil } - reason := PolicyEvaluationsRefReasonUnavailable - if in.GetReason() == pb.PolicyEvaluationsRef_REASON_TOO_LARGE { + var reason PolicyEvaluationsRefReason + switch in.GetReason() { + case pb.PolicyEvaluationsRef_REASON_TOO_LARGE: reason = PolicyEvaluationsRefReasonTooLarge + case pb.PolicyEvaluationsRef_REASON_UNAVAILABLE: + reason = PolicyEvaluationsRefReasonUnavailable + default: + // An unrecognized reason next to inlined evaluations is no failure at + // all; on its own it is one we cannot name. + if !in.GetInlined() { + reason = PolicyEvaluationsRefReasonUnavailable + } } return &PolicyEvaluationsRef{ @@ -358,6 +372,7 @@ func pbPolicyEvaluationsRefToAction(in *pb.PolicyEvaluationsRef) *PolicyEvaluati SizeBytes: in.GetSizeBytes(), MediaType: in.GetMediaType(), Reason: reason, + Inlined: in.GetInlined(), } } diff --git a/app/cli/pkg/action/workflow_run_describe_test.go b/app/cli/pkg/action/workflow_run_describe_test.go index 9dd5572e5..ff5266fe1 100644 --- a/app/cli/pkg/action/workflow_run_describe_test.go +++ b/app/cli/pkg/action/workflow_run_describe_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -21,7 +21,9 @@ import ( "os" "testing" + pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" "github.com/secure-systems-lab/go-securesystemslib/dsse" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" ) @@ -73,3 +75,84 @@ func readEnvelope(path string) (*dsse.Envelope, error) { } return &envelope, nil } + +func TestPBPolicyEvaluationsRefToAction(t *testing.T) { + const digest = "sha256:abc123" + + testCases := []struct { + name string + in *pb.PolicyEvaluationsRef + want *PolicyEvaluationsRef + }{ + { + name: "no reference", + }, + { + name: "inlined evaluations carry no reason", + in: &pb.PolicyEvaluationsRef{ + Digest: digest, + SizeBytes: 2048, + MediaType: "application/vnd.chainloop.policy-evaluations.v1+json", + Inlined: true, + }, + want: &PolicyEvaluationsRef{ + Digest: digest, + SizeBytes: 2048, + MediaType: "application/vnd.chainloop.policy-evaluations.v1+json", + Inlined: true, + }, + }, + { + name: "oversized bundle", + in: &pb.PolicyEvaluationsRef{ + Digest: digest, + SizeBytes: 64 * 1024 * 1024, + Reason: pb.PolicyEvaluationsRef_REASON_TOO_LARGE, + }, + want: &PolicyEvaluationsRef{ + Digest: digest, + SizeBytes: 64 * 1024 * 1024, + Reason: PolicyEvaluationsRefReasonTooLarge, + }, + }, + { + name: "unavailable bundle", + in: &pb.PolicyEvaluationsRef{ + Digest: digest, + Reason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + }, + want: &PolicyEvaluationsRef{ + Digest: digest, + Reason: PolicyEvaluationsRefReasonUnavailable, + }, + }, + { + name: "an unspecified reason without inlining stays conservative", + in: &pb.PolicyEvaluationsRef{ + Digest: digest, + }, + want: &PolicyEvaluationsRef{ + Digest: digest, + Reason: PolicyEvaluationsRefReasonUnavailable, + }, + }, + { + name: "an unknown reason alongside inlining does not fabricate a failure", + in: &pb.PolicyEvaluationsRef{ + Digest: digest, + Reason: pb.PolicyEvaluationsRef_Reason(99), + Inlined: true, + }, + want: &PolicyEvaluationsRef{ + Digest: digest, + Inlined: true, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, pbPolicyEvaluationsRefToAction(tc.in)) + }) + } +} diff --git a/app/controlplane/api/controlplane/v1/response_messages.pb.go b/app/controlplane/api/controlplane/v1/response_messages.pb.go index 4bc706ee7..dd0a11e4d 100644 --- a/app/controlplane/api/controlplane/v1/response_messages.pb.go +++ b/app/controlplane/api/controlplane/v1/response_messages.pb.go @@ -1311,9 +1311,9 @@ type AttestationItem struct { PolicyEvaluations map[string]*PolicyEvaluations `protobuf:"bytes,8,rep,name=policy_evaluations,json=policyEvaluations,proto3" json:"policy_evaluations,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` PolicyEvaluationStatus *AttestationItem_PolicyEvaluationStatus `protobuf:"bytes,9,opt,name=policy_evaluation_status,json=policyEvaluationStatus,proto3" json:"policy_evaluation_status,omitempty"` // Reference to the policy-evaluation bundle in the CAS backend. Populated - // only when the evaluations are NOT inlined in policy_evaluations, either - // because the bundle is larger than the server inlines or because it could - // not be resolved. Fetch the bundle by digest to inspect the violations. + // whenever the attestation carries a bundle, whether or not the evaluations + // were also inlined in policy_evaluations: see its "inlined" field. Fetch the + // bundle by digest to inspect the full set of evaluations. // Counters and status in policy_evaluation_status remain complete either way. PolicyEvaluationsRef *PolicyEvaluationsRef `protobuf:"bytes,11,opt,name=policy_evaluations_ref,json=policyEvaluationsRef,proto3" json:"policy_evaluations_ref,omitempty"` unknownFields protoimpl.UnknownFields @@ -1414,8 +1414,9 @@ func (x *AttestationItem) GetPolicyEvaluationsRef() *PolicyEvaluationsRef { return nil } -// Pointer to a policy-evaluation bundle held in a CAS backend, returned in -// place of the evaluations themselves. +// Pointer to a policy-evaluation bundle held in a CAS backend. It is the +// canonical location of the evaluations, returned both alongside them and, when +// they could not be inlined, in their place. type PolicyEvaluationsRef struct { state protoimpl.MessageState `protogen:"open.v1"` // Digest of the bundle as stored in CAS, in "sha256:" form @@ -1424,8 +1425,11 @@ type PolicyEvaluationsRef struct { SizeBytes int64 `protobuf:"varint,2,opt,name=size_bytes,json=sizeBytes,proto3" json:"size_bytes,omitempty"` // Media type of the bundle, i.e application/vnd.chainloop.policy-evaluations.v1+json MediaType string `protobuf:"bytes,3,opt,name=media_type,json=mediaType,proto3" json:"media_type,omitempty"` - // Why the evaluations were not inlined - Reason PolicyEvaluationsRef_Reason `protobuf:"varint,4,opt,name=reason,proto3,enum=controlplane.v1.PolicyEvaluationsRef_Reason" json:"reason,omitempty"` + // Why the evaluations were not inlined. REASON_UNSPECIFIED when they were. + Reason PolicyEvaluationsRef_Reason `protobuf:"varint,4,opt,name=reason,proto3,enum=controlplane.v1.PolicyEvaluationsRef_Reason" json:"reason,omitempty"` + // True when policy_evaluations carries the evaluations decoded from this + // bundle, i.e the reference is informational rather than a fallback. + Inlined bool `protobuf:"varint,5,opt,name=inlined,proto3" json:"inlined,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -1488,6 +1492,13 @@ func (x *PolicyEvaluationsRef) GetReason() PolicyEvaluationsRef_Reason { return PolicyEvaluationsRef_REASON_UNSPECIFIED } +func (x *PolicyEvaluationsRef) GetInlined() bool { + if x != nil { + return x.Inlined + } + return false +} + type PolicyEvaluations struct { state protoimpl.MessageState `protogen:"open.v1"` Evaluations []*PolicyEvaluation `protobuf:"bytes,1,rep,name=evaluations,proto3" json:"evaluations,omitempty"` @@ -3355,14 +3366,15 @@ const file_controlplane_v1_response_messages_proto_rawDesc = "" + " \x01(\fR\brawValue\x1a>\n" + "\x10AnnotationsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\x82\x02\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\x9c\x02\n" + "\x14PolicyEvaluationsRef\x12\x16\n" + "\x06digest\x18\x01 \x01(\tR\x06digest\x12\x1d\n" + "\n" + "size_bytes\x18\x02 \x01(\x03R\tsizeBytes\x12\x1d\n" + "\n" + "media_type\x18\x03 \x01(\tR\tmediaType\x12D\n" + - "\x06reason\x18\x04 \x01(\x0e2,.controlplane.v1.PolicyEvaluationsRef.ReasonR\x06reason\"N\n" + + "\x06reason\x18\x04 \x01(\x0e2,.controlplane.v1.PolicyEvaluationsRef.ReasonR\x06reason\x12\x18\n" + + "\ainlined\x18\x05 \x01(\bR\ainlined\"N\n" + "\x06Reason\x12\x16\n" + "\x12REASON_UNSPECIFIED\x10\x00\x12\x14\n" + "\x10REASON_TOO_LARGE\x10\x01\x12\x16\n" + diff --git a/app/controlplane/api/controlplane/v1/response_messages.proto b/app/controlplane/api/controlplane/v1/response_messages.proto index 68b64c789..4a68911d6 100644 --- a/app/controlplane/api/controlplane/v1/response_messages.proto +++ b/app/controlplane/api/controlplane/v1/response_messages.proto @@ -190,9 +190,9 @@ message AttestationItem { map policy_evaluations = 8; PolicyEvaluationStatus policy_evaluation_status = 9; // Reference to the policy-evaluation bundle in the CAS backend. Populated - // only when the evaluations are NOT inlined in policy_evaluations, either - // because the bundle is larger than the server inlines or because it could - // not be resolved. Fetch the bundle by digest to inspect the violations. + // whenever the attestation carries a bundle, whether or not the evaluations + // were also inlined in policy_evaluations: see its "inlined" field. Fetch the + // bundle by digest to inspect the full set of evaluations. // Counters and status in policy_evaluation_status remain complete either way. PolicyEvaluationsRef policy_evaluations_ref = 11; @@ -242,8 +242,9 @@ message AttestationItem { } } -// Pointer to a policy-evaluation bundle held in a CAS backend, returned in -// place of the evaluations themselves. +// Pointer to a policy-evaluation bundle held in a CAS backend. It is the +// canonical location of the evaluations, returned both alongside them and, when +// they could not be inlined, in their place. message PolicyEvaluationsRef { // Digest of the bundle as stored in CAS, in "sha256:" form string digest = 1; @@ -251,8 +252,11 @@ message PolicyEvaluationsRef { int64 size_bytes = 2; // Media type of the bundle, i.e application/vnd.chainloop.policy-evaluations.v1+json string media_type = 3; - // Why the evaluations were not inlined + // Why the evaluations were not inlined. REASON_UNSPECIFIED when they were. Reason reason = 4; + // True when policy_evaluations carries the evaluations decoded from this + // bundle, i.e the reference is informational rather than a fallback. + bool inlined = 5; enum Reason { REASON_UNSPECIFIED = 0; diff --git a/app/controlplane/api/gen/frontend/controlplane/v1/response_messages.ts b/app/controlplane/api/gen/frontend/controlplane/v1/response_messages.ts index 03b9b80c3..8e51cc490 100644 --- a/app/controlplane/api/gen/frontend/controlplane/v1/response_messages.ts +++ b/app/controlplane/api/gen/frontend/controlplane/v1/response_messages.ts @@ -619,9 +619,9 @@ export interface AttestationItem { policyEvaluationStatus?: AttestationItem_PolicyEvaluationStatus; /** * Reference to the policy-evaluation bundle in the CAS backend. Populated - * only when the evaluations are NOT inlined in policy_evaluations, either - * because the bundle is larger than the server inlines or because it could - * not be resolved. Fetch the bundle by digest to inspect the violations. + * whenever the attestation carries a bundle, whether or not the evaluations + * were also inlined in policy_evaluations: see its "inlined" field. Fetch the + * bundle by digest to inspect the full set of evaluations. * Counters and status in policy_evaluation_status remain complete either way. */ policyEvaluationsRef?: PolicyEvaluationsRef; @@ -704,8 +704,9 @@ export interface AttestationItem_Material_AnnotationsEntry { } /** - * Pointer to a policy-evaluation bundle held in a CAS backend, returned in - * place of the evaluations themselves. + * Pointer to a policy-evaluation bundle held in a CAS backend. It is the + * canonical location of the evaluations, returned both alongside them and, when + * they could not be inlined, in their place. */ export interface PolicyEvaluationsRef { /** Digest of the bundle as stored in CAS, in "sha256:" form */ @@ -714,8 +715,13 @@ export interface PolicyEvaluationsRef { sizeBytes: number; /** Media type of the bundle, i.e application/vnd.chainloop.policy-evaluations.v1+json */ mediaType: string; - /** Why the evaluations were not inlined */ + /** Why the evaluations were not inlined. REASON_UNSPECIFIED when they were. */ reason: PolicyEvaluationsRef_Reason; + /** + * True when policy_evaluations carries the evaluations decoded from this + * bundle, i.e the reference is informational rather than a fallback. + */ + inlined: boolean; } export enum PolicyEvaluationsRef_Reason { @@ -2765,7 +2771,7 @@ export const AttestationItem_Material_AnnotationsEntry = { }; function createBasePolicyEvaluationsRef(): PolicyEvaluationsRef { - return { digest: "", sizeBytes: 0, mediaType: "", reason: 0 }; + return { digest: "", sizeBytes: 0, mediaType: "", reason: 0, inlined: false }; } export const PolicyEvaluationsRef = { @@ -2782,6 +2788,9 @@ export const PolicyEvaluationsRef = { if (message.reason !== 0) { writer.uint32(32).int32(message.reason); } + if (message.inlined === true) { + writer.uint32(40).bool(message.inlined); + } return writer; }, @@ -2820,6 +2829,13 @@ export const PolicyEvaluationsRef = { message.reason = reader.int32() as any; continue; + case 5: + if (tag !== 40) { + break; + } + + message.inlined = reader.bool(); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -2835,6 +2851,7 @@ export const PolicyEvaluationsRef = { sizeBytes: isSet(object.sizeBytes) ? Number(object.sizeBytes) : 0, mediaType: isSet(object.mediaType) ? String(object.mediaType) : "", reason: isSet(object.reason) ? policyEvaluationsRef_ReasonFromJSON(object.reason) : 0, + inlined: isSet(object.inlined) ? Boolean(object.inlined) : false, }; }, @@ -2844,6 +2861,7 @@ export const PolicyEvaluationsRef = { message.sizeBytes !== undefined && (obj.sizeBytes = Math.round(message.sizeBytes)); message.mediaType !== undefined && (obj.mediaType = message.mediaType); message.reason !== undefined && (obj.reason = policyEvaluationsRef_ReasonToJSON(message.reason)); + message.inlined !== undefined && (obj.inlined = message.inlined); return obj; }, @@ -2857,6 +2875,7 @@ export const PolicyEvaluationsRef = { message.sizeBytes = object.sizeBytes ?? 0; message.mediaType = object.mediaType ?? ""; message.reason = object.reason ?? 0; + message.inlined = object.inlined ?? false; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.jsonschema.json index 81ea09c69..fe263d94a 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.jsonschema.json @@ -28,7 +28,7 @@ }, "^(policy_evaluations_ref)$": { "$ref": "controlplane.v1.PolicyEvaluationsRef.jsonschema.json", - "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n only when the evaluations are NOT inlined in policy_evaluations, either\n because the bundle is larger than the server inlines or because it could\n not be resolved. Fetch the bundle by digest to inspect the violations.\n Counters and status in policy_evaluation_status remain complete either way." + "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n whenever the attestation carries a bundle, whether or not the evaluations\n were also inlined in policy_evaluations: see its \"inlined\" field. Fetch the\n bundle by digest to inspect the full set of evaluations.\n Counters and status in policy_evaluation_status remain complete either way." } }, "properties": { @@ -82,7 +82,7 @@ }, "policyEvaluationsRef": { "$ref": "controlplane.v1.PolicyEvaluationsRef.jsonschema.json", - "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n only when the evaluations are NOT inlined in policy_evaluations, either\n because the bundle is larger than the server inlines or because it could\n not be resolved. Fetch the bundle by digest to inspect the violations.\n Counters and status in policy_evaluation_status remain complete either way." + "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n whenever the attestation carries a bundle, whether or not the evaluations\n were also inlined in policy_evaluations: see its \"inlined\" field. Fetch the\n bundle by digest to inspect the full set of evaluations.\n Counters and status in policy_evaluation_status remain complete either way." } }, "title": "Attestation Item", diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.schema.json index 5e5da3a00..bc51593a5 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationItem.schema.json @@ -28,7 +28,7 @@ }, "^(policyEvaluationsRef)$": { "$ref": "controlplane.v1.PolicyEvaluationsRef.schema.json", - "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n only when the evaluations are NOT inlined in policy_evaluations, either\n because the bundle is larger than the server inlines or because it could\n not be resolved. Fetch the bundle by digest to inspect the violations.\n Counters and status in policy_evaluation_status remain complete either way." + "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n whenever the attestation carries a bundle, whether or not the evaluations\n were also inlined in policy_evaluations: see its \"inlined\" field. Fetch the\n bundle by digest to inspect the full set of evaluations.\n Counters and status in policy_evaluation_status remain complete either way." } }, "properties": { @@ -82,7 +82,7 @@ }, "policy_evaluations_ref": { "$ref": "controlplane.v1.PolicyEvaluationsRef.schema.json", - "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n only when the evaluations are NOT inlined in policy_evaluations, either\n because the bundle is larger than the server inlines or because it could\n not be resolved. Fetch the bundle by digest to inspect the violations.\n Counters and status in policy_evaluation_status remain complete either way." + "description": "Reference to the policy-evaluation bundle in the CAS backend. Populated\n whenever the attestation carries a bundle, whether or not the evaluations\n were also inlined in policy_evaluations: see its \"inlined\" field. Fetch the\n bundle by digest to inspect the full set of evaluations.\n Counters and status in policy_evaluation_status remain complete either way." } }, "title": "Attestation Item", diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.jsonschema.json index 6206057e7..2aa299450 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.jsonschema.json @@ -2,7 +2,7 @@ "$id": "controlplane.v1.PolicyEvaluationsRef.jsonschema.json", "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, - "description": "Pointer to a policy-evaluation bundle held in a CAS backend, returned in\n place of the evaluations themselves.", + "description": "Pointer to a policy-evaluation bundle held in a CAS backend. It is the\n canonical location of the evaluations, returned both alongside them and, when\n they could not be inlined, in their place.", "patternProperties": { "^(media_type)$": { "description": "Media type of the bundle, i.e application/vnd.chainloop.policy-evaluations.v1+json", @@ -28,6 +28,10 @@ "description": "Digest of the bundle as stored in CAS, in \"sha256:\u003chex\u003e\" form", "type": "string" }, + "inlined": { + "description": "True when policy_evaluations carries the evaluations decoded from this\n bundle, i.e the reference is informational rather than a fallback.", + "type": "boolean" + }, "mediaType": { "description": "Media type of the bundle, i.e application/vnd.chainloop.policy-evaluations.v1+json", "type": "string" @@ -49,7 +53,7 @@ "type": "integer" } ], - "description": "Why the evaluations were not inlined" + "description": "Why the evaluations were not inlined. REASON_UNSPECIFIED when they were." }, "sizeBytes": { "anyOf": [ diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.schema.json index 6ab18bfbe..360c56934 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyEvaluationsRef.schema.json @@ -2,7 +2,7 @@ "$id": "controlplane.v1.PolicyEvaluationsRef.schema.json", "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, - "description": "Pointer to a policy-evaluation bundle held in a CAS backend, returned in\n place of the evaluations themselves.", + "description": "Pointer to a policy-evaluation bundle held in a CAS backend. It is the\n canonical location of the evaluations, returned both alongside them and, when\n they could not be inlined, in their place.", "patternProperties": { "^(mediaType)$": { "description": "Media type of the bundle, i.e application/vnd.chainloop.policy-evaluations.v1+json", @@ -28,6 +28,10 @@ "description": "Digest of the bundle as stored in CAS, in \"sha256:\u003chex\u003e\" form", "type": "string" }, + "inlined": { + "description": "True when policy_evaluations carries the evaluations decoded from this\n bundle, i.e the reference is informational rather than a fallback.", + "type": "boolean" + }, "media_type": { "description": "Media type of the bundle, i.e application/vnd.chainloop.policy-evaluations.v1+json", "type": "string" @@ -49,7 +53,7 @@ "type": "integer" } ], - "description": "Why the evaluations were not inlined" + "description": "Why the evaluations were not inlined. REASON_UNSPECIFIED when they were." }, "size_bytes": { "anyOf": [ diff --git a/app/controlplane/internal/service/workflowrun.go b/app/controlplane/internal/service/workflowrun.go index 4e8cab18d..344efba3b 100644 --- a/app/controlplane/internal/service/workflowrun.go +++ b/app/controlplane/internal/service/workflowrun.go @@ -32,7 +32,6 @@ import ( "github.com/chainloop-dev/chainloop/pkg/credentials" errors "github.com/go-kratos/kratos/v2/errors" "github.com/google/uuid" - intoto "github.com/in-toto/attestation/go/v1" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -100,9 +99,10 @@ func (p *casResolvedPredicate) GetPolicyEvaluations() map[string][]*chainloop.Po // caller can fetch them directly from the CAS. const defaultPolicyEvaluationsMaxInlineBytes = 10 << 20 // 10MiB -// resolvedPolicyEvaluations carries either the inlined evaluations or, when -// they were deliberately left out, the reference the caller needs to fetch -// them. Exactly one of the two fields is set. +// resolvedPolicyEvaluations carries the reference to the policy-evaluation +// bundle and, when it was small enough to be read, the evaluations decoded from +// it. The reference is always set so callers can point at the bundle whether or +// not it was inlined; evaluations are set only on the inlining path. type resolvedPolicyEvaluations struct { evaluations map[string][]*chainloop.PolicyEvaluation ref *pb.PolicyEvaluationsRef @@ -121,21 +121,27 @@ func (s *WorkflowRunService) policyEvaluationsMaxInlineBytes() int64 { // reference, meaning the caller should keep whatever the predicate itself // holds. // +// The cap is enforced against the size the descriptor records, which travels +// signed with the predicate and costs no round trip. A bundle whose size is not +// recorded is read in full, since sizing it would mean asking the CAS on every +// view -- the very cost the recorded size exists to avoid. +// // Anything that prevents inlining the bundle with confidence -- an oversized -// bundle, an unknown size, an unreachable or undecodable object -- yields a -// reference rather than a download attempt. +// bundle, an unreachable or undecodable object -- yields a reference rather +// than the evaluations. func (s *WorkflowRunService) resolvePolicyEvaluations( ctx context.Context, - descriptor *intoto.ResourceDescriptor, + ref *chainloop.PolicyEvaluationsRef, orgID uuid.UUID, ) *resolvedPolicyEvaluations { - if descriptor == nil { + if ref == nil { return nil } - mediaType := descriptor.GetMediaType() + mediaType := ref.GetMediaType() + bundleSize := ref.GetSizeBytes() - hexDigest, ok := descriptor.GetDigest()["sha256"] + hexDigest, ok := ref.GetDigest()["sha256"] if !ok { s.log.Warnw("msg", "policy evaluations reference has no sha256 digest") return unavailablePolicyEvaluations("", 0, mediaType) @@ -143,104 +149,60 @@ func (s *WorkflowRunService) resolvePolicyEvaluations( digest := fmt.Sprintf("sha256:%s", hexDigest) maxInlineBytes := s.policyEvaluationsMaxInlineBytes() + if bundleSize > maxInlineBytes { + s.log.Infow("msg", "policy evaluations bundle too large to inline", "digest", digest, "size", bundleSize, "max", maxInlineBytes) + return tooLargePolicyEvaluations(digest, bundleSize, mediaType) + } - // A cached bundle costs no CAS round trip. Only under-cap bundles are - // cached below, so the size check here just keeps the cap honest for - // entries written before it existed. + // A cached bundle costs no CAS round trip. if cached, found, err := s.policyEvalCache.Get(ctx, digest); err == nil && found { - if int64(len(cached)) > maxInlineBytes { - return tooLargePolicyEvaluations(digest, int64(len(cached)), mediaType) - } - - return s.decodePolicyEvaluations(cached, digest, int64(len(cached)), mediaType) + return s.decodePolicyEvaluations(cached, digest, mediaType) } mapping, err := s.casMappingUC.FindCASMappingForDownloadByOrg(ctx, digest, []uuid.UUID{orgID}, nil) if err != nil { s.log.Warnw("msg", "finding CAS mapping for policy evaluations", "digest", digest, "err", err) - return unavailablePolicyEvaluations(digest, 0, mediaType) + return unavailablePolicyEvaluations(digest, bundleSize, mediaType) } - // Ask for the size before paying for the transfer. - info, err := s.casClient.Describe(ctx, string(mapping.CASBackend.Provider), mapping.CASBackend.SecretName, mapping.CASBackend.OrganizationID, digest) - if err != nil { - s.log.Warnw("msg", "describing policy evaluations bundle", "digest", digest, "err", err) - return unavailablePolicyEvaluations(digest, 0, mediaType) - } - - if info.Size > maxInlineBytes { - s.log.Infow("msg", "policy evaluations bundle too large to inline", "digest", digest, "size", info.Size, "max", maxInlineBytes) - return tooLargePolicyEvaluations(digest, info.Size, mediaType) - } - - // A size of zero means the backend did not report one, not that the object - // is empty: some backends omit the content length and the proto getter then - // yields zero. Downloading on that basis would be downloading blind. - if info.Size <= 0 { - s.log.Warnw("msg", "policy evaluations bundle has no reported size", "digest", digest) - return unavailablePolicyEvaluations(digest, 0, mediaType) - } - - // The reported size is metadata, so bound the transfer itself as well. - // A backend that under-reports cannot then push us past the cap. - buf := &boundedBuffer{limit: maxInlineBytes} + buf := &bytes.Buffer{} err = s.casClient.Download(ctx, string(mapping.CASBackend.Provider), mapping.CASBackend.SecretName, mapping.CASBackend.OrganizationID, buf, digest) - - // Checked before the error because a writer refusing to grow surfaces as a - // download failure, and because the bound must hold even if an - // implementation swallows the write error. - if buf.exceeded { - s.log.Warnw("msg", "policy evaluations bundle exceeded the cap while downloading", "digest", digest, "reportedSize", info.Size, "max", maxInlineBytes) - // The reported size is known to be wrong, so no size is reported at all. - return tooLargePolicyEvaluations(digest, 0, mediaType) - } - if err != nil { s.log.Warnw("msg", "downloading policy evaluations bundle", "digest", digest, "err", err) - return unavailablePolicyEvaluations(digest, info.Size, mediaType) + return unavailablePolicyEvaluations(digest, bundleSize, mediaType) } data := buf.Bytes() _ = s.policyEvalCache.Set(ctx, digest, data) - return s.decodePolicyEvaluations(data, digest, info.Size, mediaType) -} - -// boundedBuffer accumulates bytes in memory up to a limit and refuses the write -// that would exceed it, recording that it did. It exists so the policy -// evaluations cap is enforced against the bytes actually received rather than -// against the size the CAS backend claims. -type boundedBuffer struct { - buf bytes.Buffer - limit int64 - written int64 - exceeded bool -} - -func (b *boundedBuffer) Write(p []byte) (int, error) { - if b.written+int64(len(p)) > b.limit { - b.exceeded = true - return 0, fmt.Errorf("content exceeds the maximum of %d bytes", b.limit) - } - - n, err := b.buf.Write(p) - b.written += int64(n) - - return n, err -} - -func (b *boundedBuffer) Bytes() []byte { - return b.buf.Bytes() + return s.decodePolicyEvaluations(data, digest, mediaType) } -func (s *WorkflowRunService) decodePolicyEvaluations(data []byte, digest string, size int64, mediaType string) *resolvedPolicyEvaluations { +// decodePolicyEvaluations reports the bytes actually read rather than the size +// the attestation records: the two can disagree, and only the former is known +// to be right. +func (s *WorkflowRunService) decodePolicyEvaluations(data []byte, digest string, mediaType string) *resolvedPolicyEvaluations { evaluations, err := chainloop.PolicyEvaluationsFromBundle(data) if err != nil { s.log.Warnw("msg", "decoding policy evaluations bundle", "digest", digest, "err", err) - return unavailablePolicyEvaluations(digest, size, mediaType) + return unavailablePolicyEvaluations(digest, int64(len(data)), mediaType) } - return &resolvedPolicyEvaluations{evaluations: evaluations} + return &resolvedPolicyEvaluations{ + evaluations: evaluations, + ref: inlinedPolicyEvaluations(digest, int64(len(data)), mediaType), + } +} + +// inlinedPolicyEvaluations references a bundle whose evaluations are being +// returned alongside it. No reason is set: nothing was left out. +func inlinedPolicyEvaluations(digest string, size int64, mediaType string) *pb.PolicyEvaluationsRef { + return &pb.PolicyEvaluationsRef{ + Digest: digest, + SizeBytes: size, + MediaType: mediaType, + Inlined: true, + } } func tooLargePolicyEvaluations(digest string, size int64, mediaType string) *resolvedPolicyEvaluations { @@ -430,11 +392,10 @@ func (s *WorkflowRunService) View(ctx context.Context, req *pb.WorkflowRunServic } if resolved := s.resolvePolicyEvaluations(ctx, predicate.GetPolicyEvaluationsRef(), run.Workflow.OrgID); resolved != nil { - // Either the evaluations are inlined, or the caller is handed the - // reference to fetch them from the CAS itself. - if resolved.ref != nil { - policyEvaluationsRef = resolved.ref - } else { + // The reference always travels back so the caller can fetch the + // bundle from the CAS; the evaluations only when they were inlined. + policyEvaluationsRef = resolved.ref + if resolved.evaluations != nil { predicate = &casResolvedPredicate{NormalizablePredicate: predicate, evals: resolved.evaluations} } } diff --git a/app/controlplane/internal/service/workflowrun_test.go b/app/controlplane/internal/service/workflowrun_test.go index ac2377b52..d7a8f9023 100644 --- a/app/controlplane/internal/service/workflowrun_test.go +++ b/app/controlplane/internal/service/workflowrun_test.go @@ -16,9 +16,7 @@ package service import ( - "bytes" "context" - "errors" "io" "testing" @@ -29,7 +27,6 @@ import ( attestationpb "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/renderer/chainloop" "github.com/chainloop-dev/chainloop/pkg/cache/policyevalbundle" - "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/google/uuid" intoto "github.com/in-toto/attestation/go/v1" "github.com/stretchr/testify/assert" @@ -67,11 +64,17 @@ func policyEvaluationsBundle(t *testing.T) []byte { return data } -func testResourceDescriptor() *intoto.ResourceDescriptor { - return &intoto.ResourceDescriptor{ - Name: "policy-evaluations", - Digest: map[string]string{sha256Alg: testBundleHexDigest}, - MediaType: chainloop.PolicyEvaluationsBundleMediaType, +// testPolicyEvaluationsRef builds the reference an attestation carries. A +// sizeBytes of zero records no size, standing in for an attestation that does +// not report one. +func testPolicyEvaluationsRef(sizeBytes int64) *chainloop.PolicyEvaluationsRef { + return &chainloop.PolicyEvaluationsRef{ + ResourceDescriptor: &intoto.ResourceDescriptor{ + Name: "policy-evaluations", + Digest: map[string]string{sha256Alg: testBundleHexDigest}, + MediaType: chainloop.PolicyEvaluationsBundleMediaType, + }, + SizeBytes: sizeBytes, } } @@ -81,18 +84,17 @@ func TestResolvePolicyEvaluations(t *testing.T) { testCases := []struct { name string - // descriptor defaults to a valid one when nil and useNilDescriptor is false - descriptor *intoto.ResourceDescriptor - useNilDescriptor bool + // ref defaults to a valid one when nil and useNilRef is false + ref *chainloop.PolicyEvaluationsRef + useNilRef bool + // bundleSize is what the reference records; zero means it records none + bundleSize int64 // maxInlineBytes 0 selects the built-in default maxInlineBytes int64 // seedCache pre-populates the bundle cache with these bytes seedCache []byte // mappingErr makes the CAS mapping lookup fail mappingErr error - // describeSize is reported by the CAS when describeErr is nil - describeSize int64 - describeErr error // downloadBody is what the CAS download writes out downloadBody []byte @@ -100,94 +102,91 @@ func TestResolvePolicyEvaluations(t *testing.T) { wantEvaluations bool wantRefReason pb.PolicyEvaluationsRef_Reason wantRefSize int64 - wantDescribeCall bool - wantDownloadCall bool + // the reference carries no digest when the descriptor had no sha256 + wantEmptyRefDigest bool + wantMappingLookup bool + wantDownloadCall bool }{ { - name: "no descriptor resolves to nothing", - useNilDescriptor: true, + name: "no reference resolves to nothing", + useNilRef: true, wantNilResolution: true, }, { - name: "bundle under the cap is inlined", - describeSize: int64(len(bundle)), - downloadBody: bundle, - wantEvaluations: true, - wantDescribeCall: true, - wantDownloadCall: true, - }, - { - name: "bundle over the cap is never downloaded", - maxInlineBytes: 16, - describeSize: 64 * 1024 * 1024, - wantRefReason: pb.PolicyEvaluationsRef_REASON_TOO_LARGE, - wantRefSize: 64 * 1024 * 1024, - wantDescribeCall: true, + name: "a recorded size under the cap is downloaded and inlined", + bundleSize: int64(len(bundle)), + downloadBody: bundle, + wantEvaluations: true, + wantMappingLookup: true, + wantDownloadCall: true, }, { - name: "size exactly at the cap is inlined", - maxInlineBytes: int64(len(bundle)), - describeSize: int64(len(bundle)), - downloadBody: bundle, - wantEvaluations: true, - wantDescribeCall: true, - wantDownloadCall: true, - }, - { - name: "unknown size is not downloaded", - describeErr: errors.New("cas unreachable"), - wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, - wantDescribeCall: true, + name: "a recorded size over the cap never reaches the CAS", + maxInlineBytes: 16, + bundleSize: 64 * 1024 * 1024, + wantRefReason: pb.PolicyEvaluationsRef_REASON_TOO_LARGE, + wantRefSize: 64 * 1024 * 1024, }, { - name: "a size of zero is unknown, not empty, so it is not downloaded", - describeSize: 0, - wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, - wantDescribeCall: true, + name: "a recorded size exactly at the cap is inlined", + maxInlineBytes: int64(len(bundle)), + bundleSize: int64(len(bundle)), + downloadBody: bundle, + wantEvaluations: true, + wantMappingLookup: true, + wantDownloadCall: true, }, { - name: "a body larger than its reported size stops at the cap", - maxInlineBytes: 16, - describeSize: 8, - downloadBody: bytes.Repeat([]byte("x"), 64), - wantRefReason: pb.PolicyEvaluationsRef_REASON_TOO_LARGE, - wantRefSize: 0, - wantDescribeCall: true, - wantDownloadCall: true, + // An unrecorded size leaves nothing to enforce the cap against, and + // sizing the bundle would cost a CAS round trip on every view. + name: "a reference with no recorded size is inlined whatever the cap", + maxInlineBytes: 4, + downloadBody: bundle, + wantEvaluations: true, + wantMappingLookup: true, + wantDownloadCall: true, }, { - name: "missing CAS mapping is not downloaded", - mappingErr: biz.NewErrNotFound("digest"), - wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + name: "missing CAS mapping is not downloaded", + bundleSize: int64(len(bundle)), + mappingErr: biz.NewErrNotFound("digest"), + wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + wantRefSize: int64(len(bundle)), + wantMappingLookup: true, }, { - name: "cached bundle under the cap skips the CAS entirely", + name: "cached bundle skips the CAS entirely", + bundleSize: int64(len(bundle)), seedCache: bundle, wantEvaluations: true, }, { - name: "cached bundle over the cap is discarded", + name: "a cached bundle is still capped by the recorded size", seedCache: bundle, + bundleSize: int64(len(bundle)), maxInlineBytes: 4, wantRefReason: pb.PolicyEvaluationsRef_REASON_TOO_LARGE, wantRefSize: int64(len(bundle)), }, { - name: "undecodable bundle reports unavailable", - describeSize: 16, - downloadBody: []byte("this is not protojson"), - wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, - wantRefSize: 16, - wantDescribeCall: true, - wantDownloadCall: true, + name: "undecodable bundle reports unavailable", + bundleSize: 16, + downloadBody: []byte("this is not protojson"), + wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + wantRefSize: int64(len("this is not protojson")), + wantMappingLookup: true, + wantDownloadCall: true, }, { - name: "descriptor without a sha256 digest reports unavailable", - descriptor: &intoto.ResourceDescriptor{ - Name: "policy-evaluations", - Digest: map[string]string{"sha512": "abc"}, + name: "a reference without a sha256 digest reports unavailable", + ref: &chainloop.PolicyEvaluationsRef{ + ResourceDescriptor: &intoto.ResourceDescriptor{ + Name: "policy-evaluations", + Digest: map[string]string{"sha512": "abc"}, + }, }, - wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + wantEmptyRefDigest: true, }, } @@ -196,36 +195,28 @@ func TestResolvePolicyEvaluations(t *testing.T) { ctx := context.Background() casClient := bizMocks.NewCASClient(t) - if tc.wantDescribeCall { - casClient.On("Describe", mock.Anything, mock.Anything, mock.Anything, orgID, testBundleDigest). - Return(&casclient.ResourceInfo{Digest: testBundleDigest, Size: tc.describeSize}, tc.describeErr) - } if tc.wantDownloadCall { casClient.On("Download", mock.Anything, mock.Anything, mock.Anything, orgID, mock.Anything, testBundleDigest). Run(func(args mock.Arguments) { w, ok := args.Get(4).(io.Writer) require.True(t, ok) - // The error is deliberately ignored: a bounded writer - // rejects a body past the cap and the production code, - // not the test, decides what that means. - _, _ = w.Write(tc.downloadBody) + _, err := w.Write(tc.downloadBody) + require.NoError(t, err) }).Return(nil) } mappingRepo := bizMocks.NewCASMappingRepo(t) - if !tc.useNilDescriptor && tc.descriptor == nil { + if tc.wantMappingLookup { mapping := &biz.CASMapping{CASBackend: &biz.CASBackend{ Provider: "OCI_REPOSITORY", SecretName: "secret-name", OrganizationID: orgID, }} if tc.mappingErr != nil { - mappingRepo.On("FindByDigestInOrgs", mock.Anything, testBundleDigest, mock.Anything, mock.Anything). - Return(nil, tc.mappingErr) - } else if tc.seedCache == nil { - mappingRepo.On("FindByDigestInOrgs", mock.Anything, testBundleDigest, mock.Anything, mock.Anything). - Return(mapping, nil) + mapping = nil } + mappingRepo.On("FindByDigestInOrgs", mock.Anything, testBundleDigest, mock.Anything, mock.Anything). + Return(mapping, tc.mappingErr) } cache, err := policyevalbundle.New(ctx, nil, nil) @@ -245,12 +236,12 @@ func TestResolvePolicyEvaluations(t *testing.T) { }, }) - descriptor := tc.descriptor - if !tc.useNilDescriptor && descriptor == nil { - descriptor = testResourceDescriptor() + ref := tc.ref + if !tc.useNilRef && ref == nil { + ref = testPolicyEvaluationsRef(tc.bundleSize) } - got := svc.resolvePolicyEvaluations(ctx, descriptor, orgID) + got := svc.resolvePolicyEvaluations(ctx, ref, orgID) if tc.wantNilResolution { assert.Nil(t, got) @@ -260,17 +251,34 @@ func TestResolvePolicyEvaluations(t *testing.T) { require.NotNil(t, got) if tc.wantEvaluations { - assert.Nil(t, got.ref, "evaluations were inlined so no ref is expected") require.NotEmpty(t, got.evaluations) assert.Len(t, got.evaluations["registry-report"], 1) + + // The bundle is referenced alongside the evaluations so callers + // always know where the full set lives. + require.NotNil(t, got.ref) + assert.True(t, got.ref.GetInlined()) + assert.Equal(t, pb.PolicyEvaluationsRef_REASON_UNSPECIFIED, got.ref.GetReason()) + assert.Equal(t, testBundleDigest, got.ref.GetDigest()) + assert.Equal(t, chainloop.PolicyEvaluationsBundleMediaType, got.ref.GetMediaType()) + // The decoded byte count, not the size the attestation recorded. + assert.Equal(t, int64(len(bundle)), got.ref.GetSizeBytes()) + return } assert.Empty(t, got.evaluations) require.NotNil(t, got.ref) + assert.False(t, got.ref.GetInlined()) assert.Equal(t, tc.wantRefReason, got.ref.GetReason()) assert.Equal(t, tc.wantRefSize, got.ref.GetSizeBytes()) + wantRefDigest := testBundleDigest + if tc.wantEmptyRefDigest { + wantRefDigest = "" + } + assert.Equal(t, wantRefDigest, got.ref.GetDigest()) + if !tc.wantDownloadCall { casClient.AssertNotCalled(t, "Download", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) } diff --git a/app/controlplane/pkg/biz/casclient.go b/app/controlplane/pkg/biz/casclient.go index ba9800de3..f09a3f0f7 100644 --- a/app/controlplane/pkg/biz/casclient.go +++ b/app/controlplane/pkg/biz/casclient.go @@ -51,10 +51,6 @@ type CASUploader interface { type CASDownloader interface { Download(ctx context.Context, backendType, secretID string, orgID uuid.UUID, w io.Writer, digest string) error - // Describe reports the metadata of a stored resource, including its size in - // bytes, without transferring its content. Callers use it to decide whether - // downloading is worthwhile before paying for the transfer. - Describe(ctx context.Context, backendType, secretID string, orgID uuid.UUID, digest string) (*casclient.ResourceInfo, error) } type CASClient interface { @@ -153,25 +149,6 @@ func (uc *CASClientUseCase) Download(ctx context.Context, backendType, secretID return nil } -func (uc *CASClientUseCase) Describe(ctx context.Context, backendType, secretID string, orgID uuid.UUID, digest string) (*casclient.ResourceInfo, error) { - ctx, span := otelx.Start(ctx, casClientTracer, "CASClientUseCase.Describe") - defer span.End() - - // SourceInternal flags this as the control plane's own traffic so the CAS doesn't emit audit events for it - client, closeFn, err := uc.casAPIClient(&CASCredsOpts{BackendType: backendType, SecretPath: secretID, Role: casJWT.Downloader, OrgID: orgID, SourceInternal: true}) - if err != nil { - return nil, fmt.Errorf("failed to create cas client: %w", err) - } - defer closeFn() - - info, err := client.Describe(ctx, digest) - if err != nil { - return nil, fmt.Errorf("failed to describe content: %w", err) - } - - return info, nil -} - // create a client with a temporary set of credentials for a specific operation func (uc *CASClientUseCase) casAPIClient(backendRef *CASCredsOpts) (casclient.DownloaderUploader, func(), error) { token, err := uc.credsProvider.GenerateTemporaryCredentials(backendRef) diff --git a/app/controlplane/pkg/biz/casclient_test.go b/app/controlplane/pkg/biz/casclient_test.go index 867f915ab..4bc15bc6f 100644 --- a/app/controlplane/pkg/biz/casclient_test.go +++ b/app/controlplane/pkg/biz/casclient_test.go @@ -17,17 +17,14 @@ package biz_test import ( "context" - "errors" "testing" conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/chainloop-dev/chainloop/pkg/casclient/mocks" - "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" ) func TestIsReady(t *testing.T) { @@ -85,71 +82,3 @@ func TestIsReady(t *testing.T) { }) } } - -func TestDescribe(t *testing.T) { - const digest = "sha256:cf4c9c8b7b1b4f4d0b4e3f4a5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d" - - validConf := &conf.Bootstrap_CASServer{ - Grpc: &conf.Server_GRPC{Addr: "localhost:1111"}, - } - - testCases := []struct { - name string - orgID uuid.UUID - casInfo *casclient.ResourceInfo - casErr error - want *casclient.ResourceInfo - wantErr bool - wantCall bool - }{ - { - name: "returns the resource metadata reported by the CAS", - orgID: uuid.New(), - casInfo: &casclient.ResourceInfo{Digest: digest, Filename: "policy-evaluations.json", Size: 2048}, - want: &casclient.ResourceInfo{Digest: digest, Filename: "policy-evaluations.json", Size: 2048}, - wantCall: true, - }, - { - name: "propagates the CAS error", - orgID: uuid.New(), - casErr: errors.New("not found"), - wantErr: true, - wantCall: true, - }, - { - name: "fails without reaching the CAS when the org is missing", - orgID: uuid.Nil, - wantErr: true, - }, - } - - credsProvider, err := biz.NewCASCredentialsUseCase(&conf.Auth{ - CasRobotAccountPrivateKeyPath: "./testdata/test-key.ec.pem", - }) - require.NoError(t, err) - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - c := mocks.NewDownloaderUploader(t) - if tc.wantCall { - c.On("Describe", mock.Anything, digest).Return(tc.casInfo, tc.casErr) - } - - clientProvider := func(_ *conf.Bootstrap_CASServer, _ string) (casclient.DownloaderUploader, func(), error) { - return c, func() {}, nil - } - - uc := biz.NewCASClientUseCase(credsProvider, validConf, nil, biz.WithClientFactory(clientProvider)) - - got, err := uc.Describe(context.Background(), "OCI_REPOSITORY", "secret-name", tc.orgID, digest) - if tc.wantErr { - assert.Error(t, err) - assert.Nil(t, got) - return - } - - assert.NoError(t, err) - assert.Equal(t, tc.want, got) - }) - } -} diff --git a/app/controlplane/pkg/biz/mocks/CASClient.go b/app/controlplane/pkg/biz/mocks/CASClient.go index 0d29cb5d2..7b7373681 100644 --- a/app/controlplane/pkg/biz/mocks/CASClient.go +++ b/app/controlplane/pkg/biz/mocks/CASClient.go @@ -8,7 +8,6 @@ import ( "context" "io" - "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/google/uuid" mock "github.com/stretchr/testify/mock" ) @@ -40,92 +39,6 @@ func (_m *CASClient) EXPECT() *CASClient_Expecter { return &CASClient_Expecter{mock: &_m.Mock} } -// Describe provides a mock function for the type CASClient -func (_mock *CASClient) Describe(ctx context.Context, backendType string, secretID string, orgID uuid.UUID, digest string) (*casclient.ResourceInfo, error) { - ret := _mock.Called(ctx, backendType, secretID, orgID, digest) - - if len(ret) == 0 { - panic("no return value specified for Describe") - } - - var r0 *casclient.ResourceInfo - var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string, uuid.UUID, string) (*casclient.ResourceInfo, error)); ok { - return returnFunc(ctx, backendType, secretID, orgID, digest) - } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, string, uuid.UUID, string) *casclient.ResourceInfo); ok { - r0 = returnFunc(ctx, backendType, secretID, orgID, digest) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*casclient.ResourceInfo) - } - } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, string, uuid.UUID, string) error); ok { - r1 = returnFunc(ctx, backendType, secretID, orgID, digest) - } else { - r1 = ret.Error(1) - } - return r0, r1 -} - -// CASClient_Describe_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Describe' -type CASClient_Describe_Call struct { - *mock.Call -} - -// Describe is a helper method to define mock.On call -// - ctx context.Context -// - backendType string -// - secretID string -// - orgID uuid.UUID -// - digest string -func (_e *CASClient_Expecter) Describe(ctx any, backendType any, secretID any, orgID any, digest any) *CASClient_Describe_Call { - return &CASClient_Describe_Call{Call: _e.mock.On("Describe", ctx, backendType, secretID, orgID, digest)} -} - -func (_c *CASClient_Describe_Call) Run(run func(ctx context.Context, backendType string, secretID string, orgID uuid.UUID, digest string)) *CASClient_Describe_Call { - _c.Call.Run(func(args mock.Arguments) { - var arg0 context.Context - if args[0] != nil { - arg0 = args[0].(context.Context) - } - var arg1 string - if args[1] != nil { - arg1 = args[1].(string) - } - var arg2 string - if args[2] != nil { - arg2 = args[2].(string) - } - var arg3 uuid.UUID - if args[3] != nil { - arg3 = args[3].(uuid.UUID) - } - var arg4 string - if args[4] != nil { - arg4 = args[4].(string) - } - run( - arg0, - arg1, - arg2, - arg3, - arg4, - ) - }) - return _c -} - -func (_c *CASClient_Describe_Call) Return(resourceInfo *casclient.ResourceInfo, err error) *CASClient_Describe_Call { - _c.Call.Return(resourceInfo, err) - return _c -} - -func (_c *CASClient_Describe_Call) RunAndReturn(run func(ctx context.Context, backendType string, secretID string, orgID uuid.UUID, digest string) (*casclient.ResourceInfo, error)) *CASClient_Describe_Call { - _c.Call.Return(run) - return _c -} - // Download provides a mock function for the type CASClient func (_mock *CASClient) Download(ctx context.Context, backendType string, secretID string, orgID uuid.UUID, w io.Writer, digest string) error { ret := _mock.Called(ctx, backendType, secretID, orgID, w, digest) diff --git a/pkg/attestation/renderer/chainloop/chainloop.go b/pkg/attestation/renderer/chainloop/chainloop.go index 9784295fb..77ac9db52 100644 --- a/pkg/attestation/renderer/chainloop/chainloop.go +++ b/pkg/attestation/renderer/chainloop/chainloop.go @@ -42,7 +42,7 @@ type NormalizablePredicate interface { GetRunLink() string GetMetadata() *Metadata GetPolicyEvaluations() map[string][]*PolicyEvaluation - GetPolicyEvaluationsRef() *intoto.ResourceDescriptor + GetPolicyEvaluationsRef() *PolicyEvaluationsRef GetPolicyEvaluationStatus() *PolicyEvaluationStatus } diff --git a/pkg/attestation/renderer/chainloop/v02.go b/pkg/attestation/renderer/chainloop/v02.go index d1a6e8571..e3574ffec 100644 --- a/pkg/attestation/renderer/chainloop/v02.go +++ b/pkg/attestation/renderer/chainloop/v02.go @@ -40,13 +40,37 @@ const PredicateTypeV02 = "chainloop.dev/attestation/v0.2" const AttPolicyEvaluation = "CHAINLOOP.ATTESTATION" const PolicyEvaluationsBundleMediaType = "application/vnd.chainloop.policy-evaluations.v1+json" +// PolicyEvaluationsRef points at the policy-evaluation bundle held in a CAS +// backend. It is an in-toto resource descriptor plus the size in bytes of the +// bundle it points at: the descriptor spec has no field for a size, and readers +// need one to decide whether to fetch the bundle. Embedding keeps the size +// inside the reference in the rendered predicate, next to the digest it +// describes. +type PolicyEvaluationsRef struct { + *intoto.ResourceDescriptor + // Size in bytes of the referenced bundle. Zero when not recorded. + SizeBytes int64 `json:"size,omitempty"` +} + +// GetSizeBytes reports the recorded size, or zero when there is no reference or +// it records none. A negative size is treated as unrecorded: the value is only +// as trustworthy as the attestation it came from, and a reader that trusted it +// would size a bundle it cannot size. +func (r *PolicyEvaluationsRef) GetSizeBytes() int64 { + if r == nil || r.SizeBytes < 0 { + return 0 + } + + return r.SizeBytes +} + type ProvenancePredicateV02 struct { *ProvenancePredicateCommon Materials []*intoto.ResourceDescriptor `json:"materials,omitempty"` // Deprecated: use PolicyEvaluationsRef to fetch full data from CAS. PolicyEvaluations map[string][]*PolicyEvaluation `json:"policyEvaluations,omitempty"` - // Reference to the PolicyEvaluationBundle stored in CAS - PolicyEvaluationsRef *intoto.ResourceDescriptor `json:"policyEvaluationsRef,omitempty"` + // Reference to the PolicyEvaluationBundle stored in CAS, carrying its size + PolicyEvaluationsRef *PolicyEvaluationsRef `json:"policyEvaluationsRef,omitempty"` // Used to read policy evaluations from old attestations PolicyEvaluationsFallback map[string][]*PolicyEvaluation `json:"policy_evaluations,omitempty"` @@ -123,11 +147,11 @@ type RendererV02 struct { *RendererCommon attClient pb.AttestationServiceClient logger *zerolog.Logger - policyEvaluationsRef *intoto.ResourceDescriptor + policyEvaluationsRef *PolicyEvaluationsRef } // SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations bundle. -func (r *RendererV02) SetPolicyEvaluationsRef(ref *intoto.ResourceDescriptor) { +func (r *RendererV02) SetPolicyEvaluationsRef(ref *PolicyEvaluationsRef) { r.policyEvaluationsRef = ref } @@ -533,7 +557,7 @@ func (p *ProvenancePredicateV02) GetPolicyEvaluations() map[string][]*PolicyEval return p.PolicyEvaluations } -func (p *ProvenancePredicateV02) GetPolicyEvaluationsRef() *intoto.ResourceDescriptor { +func (p *ProvenancePredicateV02) GetPolicyEvaluationsRef() *PolicyEvaluationsRef { return p.PolicyEvaluationsRef } diff --git a/pkg/attestation/renderer/chainloop/v02_test.go b/pkg/attestation/renderer/chainloop/v02_test.go index 97690fde9..e74ffc207 100644 --- a/pkg/attestation/renderer/chainloop/v02_test.go +++ b/pkg/attestation/renderer/chainloop/v02_test.go @@ -434,24 +434,41 @@ func mustStructValue(t *testing.T, fields map[string]any) *structpb.Value { return structpb.NewStructValue(s) } +// policyEvaluationsRefName is the descriptor name the CLI gives the bundle. +const policyEvaluationsRefName = "policy-evaluations" + func TestPredicatePolicyEvaluationsRef(t *testing.T) { - testCases := []struct { - name string - ref *intoto.ResourceDescriptor - wantRef bool - }{ - { - name: "ref is present when set", - ref: &intoto.ResourceDescriptor{ - Name: "policy-evaluations", + ref := func(size int64) *PolicyEvaluationsRef { + return &PolicyEvaluationsRef{ + ResourceDescriptor: &intoto.ResourceDescriptor{ + Name: policyEvaluationsRefName, Digest: map[string]string{"sha256": "abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890"}, MediaType: PolicyEvaluationsBundleMediaType, }, - wantRef: true, + SizeBytes: size, + } + } + + testCases := []struct { + name string + ref *PolicyEvaluationsRef + wantRef bool + wantSize int64 + }{ + { + name: "the ref carries the bundle size next to the digest", + ref: ref(4096), + wantRef: true, + wantSize: 4096, + }, + { + name: "a bundle of unrecorded size reports zero", + ref: ref(0), + wantRef: true, + wantSize: 0, }, { name: "ref is nil when not set", - ref: nil, wantRef: false, }, } @@ -490,6 +507,11 @@ func TestPredicatePolicyEvaluationsRef(t *testing.T) { assert.Equal(t, tc.ref.MediaType, predicate.PolicyEvaluationsRef.MediaType) assert.Equal(t, tc.ref.Digest["sha256"], predicate.PolicyEvaluationsRef.Digest["sha256"]) + // The size sits inside the reference, so it survives the round trip + // through the signed statement and readers can decide whether to + // pull the bundle without asking the CAS how big it is. + assert.Equal(t, tc.wantSize, predicate.PolicyEvaluationsRef.GetSizeBytes()) + // With a ref present (CAS offload) the predicate must not also carry // the inline evaluations. assert.Empty(t, predicate.PolicyEvaluations) @@ -497,6 +519,72 @@ func TestPredicatePolicyEvaluationsRef(t *testing.T) { } } +// TestPolicyEvaluationsRefJSON pins the rendered shape: the size is a key of the +// reference object itself, alongside the descriptor's own fields. +func TestPolicyEvaluationsRefJSON(t *testing.T) { + testCases := []struct { + name string + ref *PolicyEvaluationsRef + want string + }{ + { + name: "size is rendered inside the reference", + ref: &PolicyEvaluationsRef{ + ResourceDescriptor: &intoto.ResourceDescriptor{ + Name: policyEvaluationsRefName, + Digest: map[string]string{"sha256": "deadbeef"}, + MediaType: PolicyEvaluationsBundleMediaType, + }, + SizeBytes: 4096, + }, + want: `{"name":"policy-evaluations","digest":{"sha256":"deadbeef"},"media_type":"application/vnd.chainloop.policy-evaluations.v1+json","size":4096}`, + }, + { + name: "an unrecorded size is omitted rather than rendered as zero", + ref: &PolicyEvaluationsRef{ + ResourceDescriptor: &intoto.ResourceDescriptor{ + Name: policyEvaluationsRefName, + Digest: map[string]string{"sha256": "deadbeef"}, + }, + }, + want: `{"name":"policy-evaluations","digest":{"sha256":"deadbeef"}}`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := json.Marshal(tc.ref) + require.NoError(t, err) + assert.JSONEq(t, tc.want, string(got)) + + // and it reads back into the same value + var back PolicyEvaluationsRef + require.NoError(t, json.Unmarshal(got, &back)) + assert.Equal(t, tc.ref.GetSizeBytes(), back.GetSizeBytes()) + assert.Equal(t, tc.ref.GetDigest()["sha256"], back.GetDigest()["sha256"]) + }) + } +} + +func TestPolicyEvaluationsRefGetSizeBytes(t *testing.T) { + testCases := []struct { + name string + ref *PolicyEvaluationsRef + want int64 + }{ + {name: "no reference reports zero"}, + {name: "an unrecorded size reports zero", ref: &PolicyEvaluationsRef{}, want: 0}, + {name: "a recorded size is reported", ref: &PolicyEvaluationsRef{SizeBytes: 128}, want: 128}, + {name: "a negative size is not trusted", ref: &PolicyEvaluationsRef{SizeBytes: -1}, want: 0}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, tc.ref.GetSizeBytes()) + }) + } +} + func TestPolicyEvaluationsFromBundle(t *testing.T) { tests := []struct { name string diff --git a/pkg/attestation/renderer/renderer.go b/pkg/attestation/renderer/renderer.go index 644578010..35175f8d2 100644 --- a/pkg/attestation/renderer/renderer.go +++ b/pkg/attestation/renderer/renderer.go @@ -106,7 +106,7 @@ func (ab *AttestationRenderer) RenderStatement(ctx context.Context) (*intoto.Sta // SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations bundle // on the underlying renderer. This must be called before Render(). -func (ab *AttestationRenderer) SetPolicyEvaluationsRef(ref *intoto.ResourceDescriptor) { +func (ab *AttestationRenderer) SetPolicyEvaluationsRef(ref *chainloop.PolicyEvaluationsRef) { ab.v02Renderer.SetPolicyEvaluationsRef(ref) } diff --git a/pkg/casclient/casclient.go b/pkg/casclient/casclient.go index a28708e2e..8f00f1d94 100644 --- a/pkg/casclient/casclient.go +++ b/pkg/casclient/casclient.go @@ -43,9 +43,6 @@ type Uploader interface { type Downloader interface { Download(ctx context.Context, w io.Writer, digest string) error - // Describe reports the metadata of a stored resource, including its size in - // bytes, without transferring its content - Describe(ctx context.Context, digest string) (*ResourceInfo, error) // Whether the CAS is ready to accept downloads IsReady(ctx context.Context) (bool, error) } diff --git a/pkg/casclient/mocks/Downloader.go b/pkg/casclient/mocks/Downloader.go index 3b553589a..29c5a897b 100644 --- a/pkg/casclient/mocks/Downloader.go +++ b/pkg/casclient/mocks/Downloader.go @@ -8,7 +8,6 @@ import ( "context" "io" - "github.com/chainloop-dev/chainloop/pkg/casclient" mock "github.com/stretchr/testify/mock" ) @@ -39,74 +38,6 @@ func (_m *Downloader) EXPECT() *Downloader_Expecter { return &Downloader_Expecter{mock: &_m.Mock} } -// Describe provides a mock function for the type Downloader -func (_mock *Downloader) Describe(ctx context.Context, digest string) (*casclient.ResourceInfo, error) { - ret := _mock.Called(ctx, digest) - - if len(ret) == 0 { - panic("no return value specified for Describe") - } - - var r0 *casclient.ResourceInfo - var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string) (*casclient.ResourceInfo, error)); ok { - return returnFunc(ctx, digest) - } - if returnFunc, ok := ret.Get(0).(func(context.Context, string) *casclient.ResourceInfo); ok { - r0 = returnFunc(ctx, digest) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*casclient.ResourceInfo) - } - } - if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = returnFunc(ctx, digest) - } else { - r1 = ret.Error(1) - } - return r0, r1 -} - -// Downloader_Describe_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Describe' -type Downloader_Describe_Call struct { - *mock.Call -} - -// Describe is a helper method to define mock.On call -// - ctx context.Context -// - digest string -func (_e *Downloader_Expecter) Describe(ctx any, digest any) *Downloader_Describe_Call { - return &Downloader_Describe_Call{Call: _e.mock.On("Describe", ctx, digest)} -} - -func (_c *Downloader_Describe_Call) Run(run func(ctx context.Context, digest string)) *Downloader_Describe_Call { - _c.Call.Run(func(args mock.Arguments) { - var arg0 context.Context - if args[0] != nil { - arg0 = args[0].(context.Context) - } - var arg1 string - if args[1] != nil { - arg1 = args[1].(string) - } - run( - arg0, - arg1, - ) - }) - return _c -} - -func (_c *Downloader_Describe_Call) Return(resourceInfo *casclient.ResourceInfo, err error) *Downloader_Describe_Call { - _c.Call.Return(resourceInfo, err) - return _c -} - -func (_c *Downloader_Describe_Call) RunAndReturn(run func(ctx context.Context, digest string) (*casclient.ResourceInfo, error)) *Downloader_Describe_Call { - _c.Call.Return(run) - return _c -} - // Download provides a mock function for the type Downloader func (_mock *Downloader) Download(ctx context.Context, w io.Writer, digest string) error { ret := _mock.Called(ctx, w, digest) diff --git a/pkg/casclient/mocks/DownloaderUploader.go b/pkg/casclient/mocks/DownloaderUploader.go index ff112989f..b53e329c3 100644 --- a/pkg/casclient/mocks/DownloaderUploader.go +++ b/pkg/casclient/mocks/DownloaderUploader.go @@ -39,74 +39,6 @@ func (_m *DownloaderUploader) EXPECT() *DownloaderUploader_Expecter { return &DownloaderUploader_Expecter{mock: &_m.Mock} } -// Describe provides a mock function for the type DownloaderUploader -func (_mock *DownloaderUploader) Describe(ctx context.Context, digest string) (*casclient.ResourceInfo, error) { - ret := _mock.Called(ctx, digest) - - if len(ret) == 0 { - panic("no return value specified for Describe") - } - - var r0 *casclient.ResourceInfo - var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string) (*casclient.ResourceInfo, error)); ok { - return returnFunc(ctx, digest) - } - if returnFunc, ok := ret.Get(0).(func(context.Context, string) *casclient.ResourceInfo); ok { - r0 = returnFunc(ctx, digest) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*casclient.ResourceInfo) - } - } - if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = returnFunc(ctx, digest) - } else { - r1 = ret.Error(1) - } - return r0, r1 -} - -// DownloaderUploader_Describe_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Describe' -type DownloaderUploader_Describe_Call struct { - *mock.Call -} - -// Describe is a helper method to define mock.On call -// - ctx context.Context -// - digest string -func (_e *DownloaderUploader_Expecter) Describe(ctx any, digest any) *DownloaderUploader_Describe_Call { - return &DownloaderUploader_Describe_Call{Call: _e.mock.On("Describe", ctx, digest)} -} - -func (_c *DownloaderUploader_Describe_Call) Run(run func(ctx context.Context, digest string)) *DownloaderUploader_Describe_Call { - _c.Call.Run(func(args mock.Arguments) { - var arg0 context.Context - if args[0] != nil { - arg0 = args[0].(context.Context) - } - var arg1 string - if args[1] != nil { - arg1 = args[1].(string) - } - run( - arg0, - arg1, - ) - }) - return _c -} - -func (_c *DownloaderUploader_Describe_Call) Return(resourceInfo *casclient.ResourceInfo, err error) *DownloaderUploader_Describe_Call { - _c.Call.Return(resourceInfo, err) - return _c -} - -func (_c *DownloaderUploader_Describe_Call) RunAndReturn(run func(ctx context.Context, digest string) (*casclient.ResourceInfo, error)) *DownloaderUploader_Describe_Call { - _c.Call.Return(run) - return _c -} - // Download provides a mock function for the type DownloaderUploader func (_mock *DownloaderUploader) Download(ctx context.Context, w io.Writer, digest string) error { ret := _mock.Called(ctx, w, digest)