From 4d84e9fdc02fe1653d75bc01e8078bec988ebb73 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 8 Sep 2026 14:59:25 +0200 Subject: [PATCH 1/4] feat(controlplane): size the policy-evaluation bundle from the attestation and always surface it The workflow-run View API sized the policy-evaluation bundle by asking the CAS for its metadata before deciding whether to inline it, and returned a reference to the bundle only when it declined. Callers on the happy path were never told the bundle existed. The attestation predicate now records policyEvaluationsBundleSize alongside the bundle reference, and View reads the size it was handed rather than paying a CAS round trip for it. Bundles above the cap are turned away without touching the CAS at all; bundles whose size the attestation does not record are read in full. AttestationItem.policy_evaluations_ref is populated whenever the attestation carries a bundle, with an "inlined" field saying whether the evaluations travel alongside it. In the CLI, "workflow run describe" renders the artifact download command for the bundle wherever a digest is known, next to the policy table or the notice that stands in for it, and exposes the reference in its JSON output. The CAS Describe plumbing that backed the previous size lookup is removed. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: a101b7eb-b58c-45c3-8fd7-d7f5240ac260 --- app/cli/cmd/workflow_workflow_run_describe.go | 51 +++--- .../workflow_workflow_run_describe_test.go | 72 +++++++-- app/cli/pkg/action/attestation_push.go | 19 +-- app/cli/pkg/action/attestation_push_test.go | 7 +- app/cli/pkg/action/workflow_run_describe.go | 39 +++-- .../pkg/action/workflow_run_describe_test.go | 85 +++++++++- .../controlplane/v1/response_messages.pb.go | 30 ++-- .../controlplane/v1/response_messages.proto | 16 +- .../controlplane/v1/response_messages.ts | 33 +++- ...olplane.v1.AttestationItem.jsonschema.json | 4 +- ...ontrolplane.v1.AttestationItem.schema.json | 4 +- ...ne.v1.PolicyEvaluationsRef.jsonschema.json | 8 +- ...lplane.v1.PolicyEvaluationsRef.schema.json | 8 +- .../internal/service/workflowrun.go | 133 ++++++--------- .../internal/service/workflowrun_test.go | 152 +++++++++--------- app/controlplane/pkg/biz/casclient.go | 23 --- app/controlplane/pkg/biz/casclient_test.go | 71 -------- app/controlplane/pkg/biz/mocks/CASClient.go | 87 ---------- .../renderer/chainloop/chainloop.go | 3 + pkg/attestation/renderer/chainloop/v02.go | 23 ++- .../renderer/chainloop/v02_test.go | 21 ++- pkg/attestation/renderer/renderer.go | 9 +- pkg/casclient/casclient.go | 3 - pkg/casclient/mocks/Downloader.go | 69 -------- pkg/casclient/mocks/DownloaderUploader.go | 68 -------- 25 files changed, 451 insertions(+), 587 deletions(-) diff --git a/app/cli/cmd/workflow_workflow_run_describe.go b/app/cli/cmd/workflow_workflow_run_describe.go index f6bb4f35c..5b39f06bc 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 diff --git a/app/cli/cmd/workflow_workflow_run_describe_test.go b/app/cli/cmd/workflow_workflow_run_describe_test.go index 04c886cfb..d1ab8e222 100644 --- a/app/cli/cmd/workflow_workflow_run_describe_test.go +++ b/app/cli/cmd/workflow_workflow_run_describe_test.go @@ -213,8 +213,9 @@ 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" ) func TestPolicyEvaluationsRefNotice(t *testing.T) { @@ -237,7 +238,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 +250,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 +261,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 +283,6 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { }, want: []string{ "policy evaluations too large to include inline (1K)", - testRefDownloadHint, }, }, } @@ -298,6 +295,12 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { } func TestAppendPolicySection(t *testing.T) { + inlinedEvaluations := map[string][]*action.PolicyEvaluation{ + chainloop.AttPolicyEvaluation: { + {Name: "strong-acl", Violations: []*action.PolicyViolation{{Message: "weak ACL"}}}, + }, + } + tests := []struct { name string attestation *action.WorkflowRunAttestationItem @@ -305,17 +308,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, "strong-acl", "weak ACL", 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"}, + wantAbsent: []string{"artifact download", bundleRowLabel}, }, { name: "an oversized bundle is rendered as a notice instead", @@ -327,7 +339,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{"strong-acl"}, + }, + { + 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 +371,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..cc0740478 100644 --- a/app/cli/pkg/action/attestation_push.go +++ b/app/cli/pkg/action/attestation_push.go @@ -239,12 +239,12 @@ func (action *AttestationPush) Run(ctx context.Context, attestationID string, ru if getCASErr != nil || casBackend.Uploader == nil { action.Logger.Debug().Msg("CAS backend is inline, skipping policy evaluations bundle upload") } else { - ref, uploadErr := uploadPolicyEvaluationsBundle(ctx, evaluations, casBackend.Uploader) + ref, sizeBytes, uploadErr := uploadPolicyEvaluationsBundle(ctx, evaluations, casBackend.Uploader) if uploadErr != nil { return nil, fmt.Errorf("uploading policy evaluations bundle to CAS: %w", uploadErr) } if ref != nil { - renderer.SetPolicyEvaluationsRef(ref) + renderer.SetPolicyEvaluationsRef(ref, sizeBytes) } } } @@ -343,17 +343,18 @@ 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. -// 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) { +// uploads to CAS, and returns a ResourceDescriptor referencing the uploaded +// object along with the size in bytes of what was uploaded. +// Returns (nil, 0, nil) when there are no evaluations or no uploader. +func uploadPolicyEvaluationsBundle(ctx context.Context, evaluations []*v1.PolicyEvaluation, uploader casclient.Uploader) (*intoto.ResourceDescriptor, int64, error) { if len(evaluations) == 0 || uploader == nil { - return nil, nil + return nil, 0, nil } bundle := &v1.PolicyEvaluationBundle{Evaluations: evaluations} data, err := protojson.Marshal(bundle) if err != nil { - return nil, fmt.Errorf("marshaling policy evaluation bundle: %w", err) + return nil, 0, fmt.Errorf("marshaling policy evaluation bundle: %w", err) } sum := sha256.Sum256(data) @@ -361,12 +362,12 @@ func uploadPolicyEvaluationsBundle(ctx context.Context, evaluations []*v1.Policy digest := "sha256:" + hexDigest if _, err := uploader.Upload(ctx, bytes.NewReader(data), "policy-evaluations.json", digest); err != nil { - return nil, fmt.Errorf("uploading policy evaluation bundle: %w", err) + return nil, 0, fmt.Errorf("uploading policy evaluation bundle: %w", err) } return &intoto.ResourceDescriptor{ Name: "policy-evaluations", Digest: map[string]string{"sha256": hexDigest}, MediaType: crChainloop.PolicyEvaluationsBundleMediaType, - }, nil + }, 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..210f38680 100644 --- a/app/cli/pkg/action/attestation_push_test.go +++ b/app/cli/pkg/action/attestation_push_test.go @@ -93,7 +93,7 @@ func TestUploadPolicyEvaluationsBundle(t *testing.T) { uploader = tc.uploader(t) } - ref, err := uploadPolicyEvaluationsBundle(context.Background(), tc.evaluations, uploader) + ref, sizeBytes, err := uploadPolicyEvaluationsBundle(context.Background(), tc.evaluations, uploader) if tc.wantErr { require.Error(t, err) return @@ -103,6 +103,7 @@ func TestUploadPolicyEvaluationsBundle(t *testing.T) { if !tc.wantRef { assert.Nil(t, ref) + assert.Zero(t, sizeBytes) return } @@ -117,6 +118,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)), sizeBytes) }) } } 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..bbded849e 100644 --- a/app/controlplane/internal/service/workflowrun.go +++ b/app/controlplane/internal/service/workflowrun.go @@ -100,9 +100,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,12 +122,19 @@ func (s *WorkflowRunService) policyEvaluationsMaxInlineBytes() int64 { // reference, meaning the caller should keep whatever the predicate itself // holds. // +// bundleSize is the size the attestation records for the bundle, and is what +// the cap is enforced against: it 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, + bundleSize int64, orgID uuid.UUID, ) *resolvedPolicyEvaluations { if descriptor == nil { @@ -143,104 +151,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 { @@ -429,12 +393,11 @@ func (s *WorkflowRunService) View(ctx context.Context, req *pb.WorkflowRunServic return nil, handleUseCaseErr(err, s.log) } - 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 { + if resolved := s.resolvePolicyEvaluations(ctx, predicate.GetPolicyEvaluationsRef(), predicate.GetPolicyEvaluationsBundleSize(), run.Workflow.OrgID); resolved != nil { + // 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..2dad562b0 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" @@ -84,15 +81,14 @@ func TestResolvePolicyEvaluations(t *testing.T) { // descriptor defaults to a valid one when nil and useNilDescriptor is false descriptor *intoto.ResourceDescriptor useNilDescriptor bool + // bundleSize is what the attestation 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,8 +96,10 @@ 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", @@ -109,77 +107,69 @@ func TestResolvePolicyEvaluations(t *testing.T) { wantNilResolution: true, }, { - name: "bundle under the cap is inlined", - describeSize: int64(len(bundle)), - downloadBody: bundle, - wantEvaluations: true, - wantDescribeCall: true, - wantDownloadCall: 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: "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: "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: "an attestation 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", @@ -187,7 +177,8 @@ func TestResolvePolicyEvaluations(t *testing.T) { Name: "policy-evaluations", Digest: map[string]string{"sha512": "abc"}, }, - wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + wantRefReason: pb.PolicyEvaluationsRef_REASON_UNAVAILABLE, + wantEmptyRefDigest: true, }, } @@ -196,36 +187,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) @@ -250,7 +233,7 @@ func TestResolvePolicyEvaluations(t *testing.T) { descriptor = testResourceDescriptor() } - got := svc.resolvePolicyEvaluations(ctx, descriptor, orgID) + got := svc.resolvePolicyEvaluations(ctx, descriptor, tc.bundleSize, orgID) if tc.wantNilResolution { assert.Nil(t, got) @@ -260,17 +243,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..b121de9eb 100644 --- a/pkg/attestation/renderer/chainloop/chainloop.go +++ b/pkg/attestation/renderer/chainloop/chainloop.go @@ -43,6 +43,9 @@ type NormalizablePredicate interface { GetMetadata() *Metadata GetPolicyEvaluations() map[string][]*PolicyEvaluation GetPolicyEvaluationsRef() *intoto.ResourceDescriptor + // GetPolicyEvaluationsBundleSize reports the size in bytes of the bundle the + // reference points at, or zero when the attestation does not record it + GetPolicyEvaluationsBundleSize() int64 GetPolicyEvaluationStatus() *PolicyEvaluationStatus } diff --git a/pkg/attestation/renderer/chainloop/v02.go b/pkg/attestation/renderer/chainloop/v02.go index d1a6e8571..1267b2ebf 100644 --- a/pkg/attestation/renderer/chainloop/v02.go +++ b/pkg/attestation/renderer/chainloop/v02.go @@ -47,6 +47,10 @@ type ProvenancePredicateV02 struct { PolicyEvaluations map[string][]*PolicyEvaluation `json:"policyEvaluations,omitempty"` // Reference to the PolicyEvaluationBundle stored in CAS PolicyEvaluationsRef *intoto.ResourceDescriptor `json:"policyEvaluationsRef,omitempty"` + // Size in bytes of the bundle PolicyEvaluationsRef points at. Recorded here + // so readers can decide whether to pull it without asking the CAS first. + // Zero when unrecorded. + PolicyEvaluationsBundleSize int64 `json:"policyEvaluationsBundleSize,omitempty"` // Used to read policy evaluations from old attestations PolicyEvaluationsFallback map[string][]*PolicyEvaluation `json:"policy_evaluations,omitempty"` @@ -121,14 +125,18 @@ type PolicyViolation struct { type RendererV02 struct { *RendererCommon - attClient pb.AttestationServiceClient - logger *zerolog.Logger - policyEvaluationsRef *intoto.ResourceDescriptor + attClient pb.AttestationServiceClient + logger *zerolog.Logger + policyEvaluationsRef *intoto.ResourceDescriptor + policyEvaluationsBundleSize int64 } -// SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations bundle. -func (r *RendererV02) SetPolicyEvaluationsRef(ref *intoto.ResourceDescriptor) { +// SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations +// bundle, along with its size in bytes. The two are set together so the +// predicate cannot describe a bundle of one size and point at another. +func (r *RendererV02) SetPolicyEvaluationsRef(ref *intoto.ResourceDescriptor, sizeBytes int64) { r.policyEvaluationsRef = ref + r.policyEvaluationsBundleSize = sizeBytes } func NewChainloopRendererV02(att *v1.Attestation, builderVersion, builderDigest string, attClient pb.AttestationServiceClient, logger *zerolog.Logger) *RendererV02 { @@ -276,6 +284,7 @@ func (r *RendererV02) predicate() (*structpb.Struct, error) { Materials: normalizedMaterials, PolicyEvaluations: evalResult.evaluations, PolicyEvaluationsRef: r.policyEvaluationsRef, + PolicyEvaluationsBundleSize: r.policyEvaluationsBundleSize, PolicyHasViolations: evalResult.hasViolations, PolicyEvaluationsCount: evalResult.evaluationsCount, PolicyViolationsCount: evalResult.violationsCount, @@ -537,6 +546,10 @@ func (p *ProvenancePredicateV02) GetPolicyEvaluationsRef() *intoto.ResourceDescr return p.PolicyEvaluationsRef } +func (p *ProvenancePredicateV02) GetPolicyEvaluationsBundleSize() int64 { + return p.PolicyEvaluationsBundleSize +} + func (p *ProvenancePredicateV02) GetPolicyEvaluationStatus() *PolicyEvaluationStatus { skipped, passed, hasGates := p.PolicySkippedCount, p.PolicyPassedCount, p.PolicyHasGates diff --git a/pkg/attestation/renderer/chainloop/v02_test.go b/pkg/attestation/renderer/chainloop/v02_test.go index 97690fde9..79f2c5d51 100644 --- a/pkg/attestation/renderer/chainloop/v02_test.go +++ b/pkg/attestation/renderer/chainloop/v02_test.go @@ -436,18 +436,22 @@ func mustStructValue(t *testing.T, fields map[string]any) *structpb.Value { func TestPredicatePolicyEvaluationsRef(t *testing.T) { testCases := []struct { - name string - ref *intoto.ResourceDescriptor - wantRef bool + name string + ref *intoto.ResourceDescriptor + size int64 + wantRef bool + wantSize int64 }{ { - name: "ref is present when set", + name: "ref and bundle size are present when set", ref: &intoto.ResourceDescriptor{ Name: "policy-evaluations", Digest: map[string]string{"sha256": "abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890"}, MediaType: PolicyEvaluationsBundleMediaType, }, - wantRef: true, + size: 4096, + wantRef: true, + wantSize: 4096, }, { name: "ref is nil when not set", @@ -468,7 +472,7 @@ func TestPredicatePolicyEvaluationsRef(t *testing.T) { renderer := NewChainloopRendererV02(state.Attestation, "dev", "sha256:59e14f1a9de709cdd0e91c36b33e54fcca95f7dba1dc7169a7f81986e02108e5", nil, nil) if tc.ref != nil { - renderer.SetPolicyEvaluationsRef(tc.ref) + renderer.SetPolicyEvaluationsRef(tc.ref, tc.size) } statement, err := renderer.Statement(context.TODO()) @@ -480,6 +484,7 @@ func TestPredicatePolicyEvaluationsRef(t *testing.T) { if !tc.wantRef { assert.Nil(t, predicate.PolicyEvaluationsRef) + assert.Zero(t, predicate.GetPolicyEvaluationsBundleSize()) // Without a ref (no-CAS backend) the evaluations stay inline. assert.NotEmpty(t, predicate.PolicyEvaluations) return @@ -490,6 +495,10 @@ 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 travels with the ref so readers can decide whether to + // pull the bundle without asking the CAS how big it is. + assert.Equal(t, tc.wantSize, predicate.GetPolicyEvaluationsBundleSize()) + // With a ref present (CAS offload) the predicate must not also carry // the inline evaluations. assert.Empty(t, predicate.PolicyEvaluations) diff --git a/pkg/attestation/renderer/renderer.go b/pkg/attestation/renderer/renderer.go index 644578010..4fa7bad85 100644 --- a/pkg/attestation/renderer/renderer.go +++ b/pkg/attestation/renderer/renderer.go @@ -104,10 +104,11 @@ func (ab *AttestationRenderer) RenderStatement(ctx context.Context) (*intoto.Sta return statement, nil } -// 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) { - ab.v02Renderer.SetPolicyEvaluationsRef(ref) +// SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations +// bundle, and its size in bytes, on the underlying renderer. This must be +// called before Render(). +func (ab *AttestationRenderer) SetPolicyEvaluationsRef(ref *intoto.ResourceDescriptor, sizeBytes int64) { + ab.v02Renderer.SetPolicyEvaluationsRef(ref, sizeBytes) } // Attestation (dsee envelope) -> { message: { Statement(in-toto): [subject, predicate] }, signature: "sig" }. 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) From 17d022b2a746285973ce8e5e191a591d27669fea Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 8 Sep 2026 15:09:45 +0200 Subject: [PATCH 2/4] refactor(cli): drop the else after a return in the attestation output switch Behaviour is unchanged; it clears a revive indent-error-flow warning in a file this change already touches. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: a101b7eb-b58c-45c3-8fd7-d7f5240ac260 --- app/cli/cmd/workflow_workflow_run_describe.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/cli/cmd/workflow_workflow_run_describe.go b/app/cli/cmd/workflow_workflow_run_describe.go index 5b39f06bc..34c7ad5b2 100644 --- a/app/cli/cmd/workflow_workflow_run_describe.go +++ b/app/cli/cmd/workflow_workflow_run_describe.go @@ -515,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: From 9c9e7c0f3330cb763d5ce437f5483aaffd3e28bc Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 8 Sep 2026 15:14:34 +0200 Subject: [PATCH 3/4] test(cli): name the repeated policy fixtures in the describe test The added rendering cases pushed two literals past the goconst threshold. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: a101b7eb-b58c-45c3-8fd7-d7f5240ac260 --- app/cli/cmd/workflow_workflow_run_describe_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/cli/cmd/workflow_workflow_run_describe_test.go b/app/cli/cmd/workflow_workflow_run_describe_test.go index d1ab8e222..59b4c5337 100644 --- a/app/cli/cmd/workflow_workflow_run_describe_test.go +++ b/app/cli/cmd/workflow_workflow_run_describe_test.go @@ -216,6 +216,8 @@ const ( testRefDownloadHint = "chainloop artifact download --digest " + testRefDigest policiesRowLabel = "Policies" bundleRowLabel = "Policy evaluations bundle" + testPolicyName = "strong-acl" + testViolationMsg = "weak ACL" ) func TestPolicyEvaluationsRefNotice(t *testing.T) { @@ -297,7 +299,7 @@ func TestPolicyEvaluationsRefNotice(t *testing.T) { func TestAppendPolicySection(t *testing.T) { inlinedEvaluations := map[string][]*action.PolicyEvaluation{ chainloop.AttPolicyEvaluation: { - {Name: "strong-acl", Violations: []*action.PolicyViolation{{Message: "weak ACL"}}}, + {Name: testPolicyName, Violations: []*action.PolicyViolation{{Message: testViolationMsg}}}, }, } @@ -318,7 +320,7 @@ func TestAppendPolicySection(t *testing.T) { Inlined: true, }, }, - wantContain: []string{policiesRowLabel, "strong-acl", "weak ACL", bundleRowLabel, testRefDownloadHint}, + wantContain: []string{policiesRowLabel, testPolicyName, testViolationMsg, bundleRowLabel, testRefDownloadHint}, }, { name: "evaluations without a bundle render the policy rows alone", @@ -326,7 +328,7 @@ func TestAppendPolicySection(t *testing.T) { PolicyEvaluations: inlinedEvaluations, PolicyEvaluationStatus: &action.PolicyEvaluationStatus{Total: 1, Violated: 1}, }, - wantContain: []string{policiesRowLabel, "strong-acl", "weak ACL"}, + wantContain: []string{policiesRowLabel, testPolicyName, testViolationMsg}, wantAbsent: []string{"artifact download", bundleRowLabel}, }, { @@ -340,7 +342,7 @@ func TestAppendPolicySection(t *testing.T) { }, }, wantContain: []string{policiesRowLabel, "134112 violations", "too large", bundleRowLabel, testRefDownloadHint}, - wantAbsent: []string{"strong-acl"}, + wantAbsent: []string{testPolicyName}, }, { name: "an unavailable bundle still offers the download when the digest is known", From 5fe943df0544696cc730d23719ed1ce9217e46ef Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Wed, 9 Sep 2026 06:35:40 +0200 Subject: [PATCH 4/4] refactor: carry the policy-evaluation bundle size inside its reference The size the workflow-run View API gates the inlining cap on was recorded as a predicate field of its own, next to the reference rather than in it. It now sits inside policyEvaluationsRef, alongside the digest and media type it describes, as a "size" key of that object. in-toto resource descriptors have no size field, so the reference becomes a Chainloop type embedding the descriptor and adding the size, which keeps the rendered predicate a single object: "policyEvaluationsRef": { "digest": {"sha256": "4c2497..."}, "media_type": "application/vnd.chainloop.policy-evaluations.v1+json", "name": "policy-evaluations", "size": 325081 } Reading the size from the reference the resolver already receives removes the separate predicate field, the extra interface method and the extra argument threaded through the renderer and the View path. A negative size reads as unrecorded rather than being trusted. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: a101b7eb-b58c-45c3-8fd7-d7f5240ac260 --- app/cli/pkg/action/attestation_push.go | 32 +++--- app/cli/pkg/action/attestation_push_test.go | 5 +- .../internal/service/workflowrun.go | 22 ++-- .../internal/service/workflowrun_test.go | 48 ++++---- .../renderer/chainloop/chainloop.go | 5 +- pkg/attestation/renderer/chainloop/v02.go | 53 +++++---- .../renderer/chainloop/v02_test.go | 107 +++++++++++++++--- pkg/attestation/renderer/renderer.go | 9 +- 8 files changed, 188 insertions(+), 93 deletions(-) diff --git a/app/cli/pkg/action/attestation_push.go b/app/cli/pkg/action/attestation_push.go index cc0740478..ea462f618 100644 --- a/app/cli/pkg/action/attestation_push.go +++ b/app/cli/pkg/action/attestation_push.go @@ -239,12 +239,12 @@ func (action *AttestationPush) Run(ctx context.Context, attestationID string, ru if getCASErr != nil || casBackend.Uploader == nil { action.Logger.Debug().Msg("CAS backend is inline, skipping policy evaluations bundle upload") } else { - ref, sizeBytes, uploadErr := uploadPolicyEvaluationsBundle(ctx, evaluations, casBackend.Uploader) + ref, uploadErr := uploadPolicyEvaluationsBundle(ctx, evaluations, casBackend.Uploader) if uploadErr != nil { return nil, fmt.Errorf("uploading policy evaluations bundle to CAS: %w", uploadErr) } if ref != nil { - renderer.SetPolicyEvaluationsRef(ref, sizeBytes) + renderer.SetPolicyEvaluationsRef(ref) } } } @@ -343,18 +343,19 @@ 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 along with the size in bytes of what was uploaded. -// Returns (nil, 0, nil) when there are no evaluations or no uploader. -func uploadPolicyEvaluationsBundle(ctx context.Context, evaluations []*v1.PolicyEvaluation, uploader casclient.Uploader) (*intoto.ResourceDescriptor, int64, error) { +// 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) (*crChainloop.PolicyEvaluationsRef, error) { if len(evaluations) == 0 || uploader == nil { - return nil, 0, nil + return nil, nil } bundle := &v1.PolicyEvaluationBundle{Evaluations: evaluations} data, err := protojson.Marshal(bundle) if err != nil { - return nil, 0, fmt.Errorf("marshaling policy evaluation bundle: %w", err) + return nil, fmt.Errorf("marshaling policy evaluation bundle: %w", err) } sum := sha256.Sum256(data) @@ -362,12 +363,15 @@ func uploadPolicyEvaluationsBundle(ctx context.Context, evaluations []*v1.Policy digest := "sha256:" + hexDigest if _, err := uploader.Upload(ctx, bytes.NewReader(data), "policy-evaluations.json", digest); err != nil { - return nil, 0, fmt.Errorf("uploading policy evaluation bundle: %w", err) + 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, - }, int64(len(data)), nil + 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 210f38680..2225db583 100644 --- a/app/cli/pkg/action/attestation_push_test.go +++ b/app/cli/pkg/action/attestation_push_test.go @@ -93,7 +93,7 @@ func TestUploadPolicyEvaluationsBundle(t *testing.T) { uploader = tc.uploader(t) } - ref, sizeBytes, err := uploadPolicyEvaluationsBundle(context.Background(), tc.evaluations, uploader) + ref, err := uploadPolicyEvaluationsBundle(context.Background(), tc.evaluations, uploader) if tc.wantErr { require.Error(t, err) return @@ -103,7 +103,6 @@ func TestUploadPolicyEvaluationsBundle(t *testing.T) { if !tc.wantRef { assert.Nil(t, ref) - assert.Zero(t, sizeBytes) return } @@ -121,7 +120,7 @@ func TestUploadPolicyEvaluationsBundle(t *testing.T) { // 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)), sizeBytes) + assert.Equal(t, int64(len(data)), ref.GetSizeBytes()) }) } } diff --git a/app/controlplane/internal/service/workflowrun.go b/app/controlplane/internal/service/workflowrun.go index bbded849e..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" ) @@ -122,28 +121,27 @@ func (s *WorkflowRunService) policyEvaluationsMaxInlineBytes() int64 { // reference, meaning the caller should keep whatever the predicate itself // holds. // -// bundleSize is the size the attestation records for the bundle, and is what -// the cap is enforced against: it 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. +// 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 unreachable or undecodable object -- yields a reference rather // than the evaluations. func (s *WorkflowRunService) resolvePolicyEvaluations( ctx context.Context, - descriptor *intoto.ResourceDescriptor, - bundleSize int64, + 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) @@ -393,7 +391,7 @@ func (s *WorkflowRunService) View(ctx context.Context, req *pb.WorkflowRunServic return nil, handleUseCaseErr(err, s.log) } - if resolved := s.resolvePolicyEvaluations(ctx, predicate.GetPolicyEvaluationsRef(), predicate.GetPolicyEvaluationsBundleSize(), run.Workflow.OrgID); resolved != nil { + if resolved := s.resolvePolicyEvaluations(ctx, predicate.GetPolicyEvaluationsRef(), run.Workflow.OrgID); resolved != nil { // 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 diff --git a/app/controlplane/internal/service/workflowrun_test.go b/app/controlplane/internal/service/workflowrun_test.go index 2dad562b0..d7a8f9023 100644 --- a/app/controlplane/internal/service/workflowrun_test.go +++ b/app/controlplane/internal/service/workflowrun_test.go @@ -64,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, } } @@ -78,10 +84,10 @@ 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 - // bundleSize is what the attestation records; zero means it records none + // 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 @@ -102,8 +108,8 @@ func TestResolvePolicyEvaluations(t *testing.T) { wantDownloadCall bool }{ { - name: "no descriptor resolves to nothing", - useNilDescriptor: true, + name: "no reference resolves to nothing", + useNilRef: true, wantNilResolution: true, }, { @@ -133,7 +139,7 @@ func TestResolvePolicyEvaluations(t *testing.T) { { // An unrecorded size leaves nothing to enforce the cap against, and // sizing the bundle would cost a CAS round trip on every view. - name: "an attestation with no recorded size is inlined whatever the cap", + name: "a reference with no recorded size is inlined whatever the cap", maxInlineBytes: 4, downloadBody: bundle, wantEvaluations: true, @@ -172,10 +178,12 @@ func TestResolvePolicyEvaluations(t *testing.T) { 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, wantEmptyRefDigest: true, @@ -228,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, tc.bundleSize, orgID) + got := svc.resolvePolicyEvaluations(ctx, ref, orgID) if tc.wantNilResolution { assert.Nil(t, got) diff --git a/pkg/attestation/renderer/chainloop/chainloop.go b/pkg/attestation/renderer/chainloop/chainloop.go index b121de9eb..77ac9db52 100644 --- a/pkg/attestation/renderer/chainloop/chainloop.go +++ b/pkg/attestation/renderer/chainloop/chainloop.go @@ -42,10 +42,7 @@ type NormalizablePredicate interface { GetRunLink() string GetMetadata() *Metadata GetPolicyEvaluations() map[string][]*PolicyEvaluation - GetPolicyEvaluationsRef() *intoto.ResourceDescriptor - // GetPolicyEvaluationsBundleSize reports the size in bytes of the bundle the - // reference points at, or zero when the attestation does not record it - GetPolicyEvaluationsBundleSize() int64 + GetPolicyEvaluationsRef() *PolicyEvaluationsRef GetPolicyEvaluationStatus() *PolicyEvaluationStatus } diff --git a/pkg/attestation/renderer/chainloop/v02.go b/pkg/attestation/renderer/chainloop/v02.go index 1267b2ebf..e3574ffec 100644 --- a/pkg/attestation/renderer/chainloop/v02.go +++ b/pkg/attestation/renderer/chainloop/v02.go @@ -40,17 +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"` - // Size in bytes of the bundle PolicyEvaluationsRef points at. Recorded here - // so readers can decide whether to pull it without asking the CAS first. - // Zero when unrecorded. - PolicyEvaluationsBundleSize int64 `json:"policyEvaluationsBundleSize,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"` @@ -125,18 +145,14 @@ type PolicyViolation struct { type RendererV02 struct { *RendererCommon - attClient pb.AttestationServiceClient - logger *zerolog.Logger - policyEvaluationsRef *intoto.ResourceDescriptor - policyEvaluationsBundleSize int64 + attClient pb.AttestationServiceClient + logger *zerolog.Logger + policyEvaluationsRef *PolicyEvaluationsRef } -// SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations -// bundle, along with its size in bytes. The two are set together so the -// predicate cannot describe a bundle of one size and point at another. -func (r *RendererV02) SetPolicyEvaluationsRef(ref *intoto.ResourceDescriptor, sizeBytes int64) { +// SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations bundle. +func (r *RendererV02) SetPolicyEvaluationsRef(ref *PolicyEvaluationsRef) { r.policyEvaluationsRef = ref - r.policyEvaluationsBundleSize = sizeBytes } func NewChainloopRendererV02(att *v1.Attestation, builderVersion, builderDigest string, attClient pb.AttestationServiceClient, logger *zerolog.Logger) *RendererV02 { @@ -284,7 +300,6 @@ func (r *RendererV02) predicate() (*structpb.Struct, error) { Materials: normalizedMaterials, PolicyEvaluations: evalResult.evaluations, PolicyEvaluationsRef: r.policyEvaluationsRef, - PolicyEvaluationsBundleSize: r.policyEvaluationsBundleSize, PolicyHasViolations: evalResult.hasViolations, PolicyEvaluationsCount: evalResult.evaluationsCount, PolicyViolationsCount: evalResult.violationsCount, @@ -542,14 +557,10 @@ func (p *ProvenancePredicateV02) GetPolicyEvaluations() map[string][]*PolicyEval return p.PolicyEvaluations } -func (p *ProvenancePredicateV02) GetPolicyEvaluationsRef() *intoto.ResourceDescriptor { +func (p *ProvenancePredicateV02) GetPolicyEvaluationsRef() *PolicyEvaluationsRef { return p.PolicyEvaluationsRef } -func (p *ProvenancePredicateV02) GetPolicyEvaluationsBundleSize() int64 { - return p.PolicyEvaluationsBundleSize -} - func (p *ProvenancePredicateV02) GetPolicyEvaluationStatus() *PolicyEvaluationStatus { skipped, passed, hasGates := p.PolicySkippedCount, p.PolicyPassedCount, p.PolicyHasGates diff --git a/pkg/attestation/renderer/chainloop/v02_test.go b/pkg/attestation/renderer/chainloop/v02_test.go index 79f2c5d51..e74ffc207 100644 --- a/pkg/attestation/renderer/chainloop/v02_test.go +++ b/pkg/attestation/renderer/chainloop/v02_test.go @@ -434,28 +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) { + ref := func(size int64) *PolicyEvaluationsRef { + return &PolicyEvaluationsRef{ + ResourceDescriptor: &intoto.ResourceDescriptor{ + Name: policyEvaluationsRefName, + Digest: map[string]string{"sha256": "abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890"}, + MediaType: PolicyEvaluationsBundleMediaType, + }, + SizeBytes: size, + } + } + testCases := []struct { name string - ref *intoto.ResourceDescriptor - size int64 + ref *PolicyEvaluationsRef wantRef bool wantSize int64 }{ { - name: "ref and bundle size are present when set", - ref: &intoto.ResourceDescriptor{ - Name: "policy-evaluations", - Digest: map[string]string{"sha256": "abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890"}, - MediaType: PolicyEvaluationsBundleMediaType, - }, - size: 4096, + 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, }, } @@ -472,7 +485,7 @@ func TestPredicatePolicyEvaluationsRef(t *testing.T) { renderer := NewChainloopRendererV02(state.Attestation, "dev", "sha256:59e14f1a9de709cdd0e91c36b33e54fcca95f7dba1dc7169a7f81986e02108e5", nil, nil) if tc.ref != nil { - renderer.SetPolicyEvaluationsRef(tc.ref, tc.size) + renderer.SetPolicyEvaluationsRef(tc.ref) } statement, err := renderer.Statement(context.TODO()) @@ -484,7 +497,6 @@ func TestPredicatePolicyEvaluationsRef(t *testing.T) { if !tc.wantRef { assert.Nil(t, predicate.PolicyEvaluationsRef) - assert.Zero(t, predicate.GetPolicyEvaluationsBundleSize()) // Without a ref (no-CAS backend) the evaluations stay inline. assert.NotEmpty(t, predicate.PolicyEvaluations) return @@ -495,9 +507,10 @@ 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 travels with the ref so readers can decide whether to + // 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.GetPolicyEvaluationsBundleSize()) + assert.Equal(t, tc.wantSize, predicate.PolicyEvaluationsRef.GetSizeBytes()) // With a ref present (CAS offload) the predicate must not also carry // the inline evaluations. @@ -506,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 4fa7bad85..35175f8d2 100644 --- a/pkg/attestation/renderer/renderer.go +++ b/pkg/attestation/renderer/renderer.go @@ -104,11 +104,10 @@ func (ab *AttestationRenderer) RenderStatement(ctx context.Context) (*intoto.Sta return statement, nil } -// SetPolicyEvaluationsRef sets the CAS reference for the policy evaluations -// bundle, and its size in bytes, on the underlying renderer. This must be -// called before Render(). -func (ab *AttestationRenderer) SetPolicyEvaluationsRef(ref *intoto.ResourceDescriptor, sizeBytes int64) { - ab.v02Renderer.SetPolicyEvaluationsRef(ref, sizeBytes) +// 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 *chainloop.PolicyEvaluationsRef) { + ab.v02Renderer.SetPolicyEvaluationsRef(ref) } // Attestation (dsee envelope) -> { message: { Statement(in-toto): [subject, predicate] }, signature: "sig" }.