From d5df1d7bfdc081aa24cea7918aa99c94bc6e7be4 Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Sun, 12 Apr 2026 01:40:49 +0530 Subject: [PATCH] fix(referrer): apply default page size when request has no pagination The ReferrerService.DiscoverPrivate and DiscoverPublicShared endpoints were unbounded when clients sent an empty request. The helper referrerPaginationOptsFromProto returned (nil, nil) for a nil pagination field, and the data layer interpreted nil CursorOptions as "return every reference". A container image that had been attested many times would accumulate hundreds of SBOMs/SARIF referrers, all returned in a single response. Apply the existing defaultReferrerPageSize (20) whenever no pagination is supplied or the limit is zero. Clients that already send pagination are unaffected. The CLI already sends a non-nil pagination request with a default limit of 20, so no client-side change is needed. Fixes: chainloop-dev/chainloop#2890 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Vibhav Bobade --- app/controlplane/internal/service/referrer.go | 25 ++++-- .../internal/service/referrer_test.go | 81 +++++++++++++++++++ app/controlplane/pkg/data/referrer.go | 34 ++++---- 3 files changed, 117 insertions(+), 23 deletions(-) create mode 100644 app/controlplane/internal/service/referrer_test.go diff --git a/app/controlplane/internal/service/referrer.go b/app/controlplane/internal/service/referrer.go index 4b0fccea4..0d5cfd61b 100644 --- a/app/controlplane/internal/service/referrer.go +++ b/app/controlplane/internal/service/referrer.go @@ -105,19 +105,28 @@ func (s *ReferrerService) DiscoverPublicShared(ctx context.Context, req *pb.Disc }, nil } +// defaultReferrerPageSize is the page size applied when a referrer Discover* request +// arrives without pagination. It deliberately overrides the package-wide +// pagination.DefaultCursorLimit (10) because referrer responses render nested references +// (SBOMs, SARIF, …) and a slightly larger page is easier to navigate without being +// noticeably slower. Keep ≤ the proto-enforced max of 100. const defaultReferrerPageSize = 20 // referrerPaginationOptsFromProto converts the proto pagination request to cursor options. -// Returns nil when the request has no pagination, preserving backward compatibility (all references returned). +// When the request has no pagination or an unset limit, the default page size is applied +// so that the response is always bounded. A root referrer (e.g. a container image) can +// accumulate an unbounded number of direct references (SBOMs, SARIF reports, ...) as it +// is attested repeatedly — returning all of them in a single response is unsafe. func referrerPaginationOptsFromProto(p *pb.CursorPaginationRequest) (*pagination.CursorOptions, error) { - if p == nil { - return nil, nil - } - limit := int(p.GetLimit()) - if limit == 0 { - limit = defaultReferrerPageSize + limit := defaultReferrerPageSize + var cursor string + if p != nil { + cursor = p.GetCursor() + if l := int(p.GetLimit()); l > 0 { + limit = l + } } - return pagination.NewCursor(p.GetCursor(), limit) + return pagination.NewCursor(cursor, limit) } func bizReferrerToPb(r *biz.StoredReferrer) *pb.ReferrerItem { diff --git a/app/controlplane/internal/service/referrer_test.go b/app/controlplane/internal/service/referrer_test.go new file mode 100644 index 000000000..0efcf93f5 --- /dev/null +++ b/app/controlplane/internal/service/referrer_test.go @@ -0,0 +1,81 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "testing" + + pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReferrerPaginationOptsFromProto(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in *pb.CursorPaginationRequest + wantLimit int + }{ + { + name: "nil request applies default page size", + in: nil, + wantLimit: defaultReferrerPageSize, + }, + { + name: "empty request applies default page size", + in: &pb.CursorPaginationRequest{}, + wantLimit: defaultReferrerPageSize, + }, + { + name: "zero limit applies default page size", + in: &pb.CursorPaginationRequest{Limit: 0}, + wantLimit: defaultReferrerPageSize, + }, + { + name: "explicit limit is honored", + in: &pb.CursorPaginationRequest{Limit: 50}, + wantLimit: 50, + }, + { + name: "limit of 1 is honored", + in: &pb.CursorPaginationRequest{Limit: 1}, + wantLimit: 1, + }, + { + name: "negative limit falls through to default page size", + in: &pb.CursorPaginationRequest{Limit: -5}, + wantLimit: defaultReferrerPageSize, + }, + { + name: "proto max limit of 100 is honored", + in: &pb.CursorPaginationRequest{Limit: 100}, + wantLimit: 100, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + opts, err := referrerPaginationOptsFromProto(tc.in) + require.NoError(t, err) + require.NotNil(t, opts, "pagination options must always be returned so the response is bounded") + assert.Equal(t, tc.wantLimit, opts.Limit) + }) + } +} diff --git a/app/controlplane/pkg/data/referrer.go b/app/controlplane/pkg/data/referrer.go index 6c2b4a78a..f94b4e6cd 100644 --- a/app/controlplane/pkg/data/referrer.go +++ b/app/controlplane/pkg/data/referrer.go @@ -238,23 +238,27 @@ func (r *ReferrerRepo) doGet(ctx context.Context, root *ent.Referrer, allowedOrg // Attach the workflow predicate predicateReferrer = append(predicateReferrer, referrer.HasWorkflowsWith(predicateWF...)) + // Defense-in-depth: if the caller did not supply pagination options, fall back + // to the package-wide default instead of emitting an unbounded query. This + // guarantees the response is bounded even when a future biz-layer caller + // forgets to pass options through — see chainloop-dev/chainloop#2890. + if p == nil { + p = &pagination.CursorOptions{Limit: pagination.DefaultCursorLimit} + } + // Sort references by creation date and ID in descending order for deterministic pagination q := root.QueryReferences().Where(predicateReferrer...).WithWorkflows(). Order(referrer.ByCreatedAt(sql.OrderDesc())). - Order(referrer.ByID(sql.OrderDesc())) - - // Apply pagination: fetch limit+1 to detect next page - if p != nil { - q = q.Limit(p.Limit + 1) - - if p.Cursor != nil { - q = q.Where(func(s *sql.Selector) { - s.Where(sql.CompositeLT( - []string{s.C(referrer.FieldCreatedAt), s.C(referrer.FieldID)}, - p.Cursor.Timestamp, p.Cursor.ID, - )) - }) - } + Order(referrer.ByID(sql.OrderDesc())). + Limit(p.Limit + 1) // fetch limit+1 to detect next page + + if p.Cursor != nil { + q = q.Where(func(s *sql.Selector) { + s.Where(sql.CompositeLT( + []string{s.C(referrer.FieldCreatedAt), s.C(referrer.FieldID)}, + p.Cursor.Timestamp, p.Cursor.ID, + )) + }) } refs, err := q.All(ctx) @@ -264,7 +268,7 @@ func (r *ReferrerRepo) doGet(ctx context.Context, root *ent.Referrer, allowedOrg // Determine if there is a next page and encode the cursor var nextCursor string - if p != nil && len(refs) > p.Limit { + if len(refs) > p.Limit { lastVisible := refs[p.Limit-1] nextCursor = pagination.EncodeCursor(lastVisible.CreatedAt, lastVisible.ID) refs = refs[:p.Limit]