Skip to content

Commit 45fa952

Browse files
waveywavesclaude
andauthored
fix(referrer): apply default page size when request has no pagination (#3023)
Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d1b119b commit 45fa952

3 files changed

Lines changed: 117 additions & 23 deletions

File tree

app/controlplane/internal/service/referrer.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,28 @@ func (s *ReferrerService) DiscoverPublicShared(ctx context.Context, req *pb.Disc
105105
}, nil
106106
}
107107

108+
// defaultReferrerPageSize is the page size applied when a referrer Discover* request
109+
// arrives without pagination. It deliberately overrides the package-wide
110+
// pagination.DefaultCursorLimit (10) because referrer responses render nested references
111+
// (SBOMs, SARIF, …) and a slightly larger page is easier to navigate without being
112+
// noticeably slower. Keep ≤ the proto-enforced max of 100.
108113
const defaultReferrerPageSize = 20
109114

110115
// referrerPaginationOptsFromProto converts the proto pagination request to cursor options.
111-
// Returns nil when the request has no pagination, preserving backward compatibility (all references returned).
116+
// When the request has no pagination or an unset limit, the default page size is applied
117+
// so that the response is always bounded. A root referrer (e.g. a container image) can
118+
// accumulate an unbounded number of direct references (SBOMs, SARIF reports, ...) as it
119+
// is attested repeatedly — returning all of them in a single response is unsafe.
112120
func referrerPaginationOptsFromProto(p *pb.CursorPaginationRequest) (*pagination.CursorOptions, error) {
113-
if p == nil {
114-
return nil, nil
115-
}
116-
limit := int(p.GetLimit())
117-
if limit == 0 {
118-
limit = defaultReferrerPageSize
121+
limit := defaultReferrerPageSize
122+
var cursor string
123+
if p != nil {
124+
cursor = p.GetCursor()
125+
if l := int(p.GetLimit()); l > 0 {
126+
limit = l
127+
}
119128
}
120-
return pagination.NewCursor(p.GetCursor(), limit)
129+
return pagination.NewCursor(cursor, limit)
121130
}
122131

123132
func bizReferrerToPb(r *biz.StoredReferrer) *pb.ReferrerItem {
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
//
2+
// Copyright 2026 The Chainloop Authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package service
17+
18+
import (
19+
"testing"
20+
21+
pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
22+
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
func TestReferrerPaginationOptsFromProto(t *testing.T) {
27+
t.Parallel()
28+
29+
tests := []struct {
30+
name string
31+
in *pb.CursorPaginationRequest
32+
wantLimit int
33+
}{
34+
{
35+
name: "nil request applies default page size",
36+
in: nil,
37+
wantLimit: defaultReferrerPageSize,
38+
},
39+
{
40+
name: "empty request applies default page size",
41+
in: &pb.CursorPaginationRequest{},
42+
wantLimit: defaultReferrerPageSize,
43+
},
44+
{
45+
name: "zero limit applies default page size",
46+
in: &pb.CursorPaginationRequest{Limit: 0},
47+
wantLimit: defaultReferrerPageSize,
48+
},
49+
{
50+
name: "explicit limit is honored",
51+
in: &pb.CursorPaginationRequest{Limit: 50},
52+
wantLimit: 50,
53+
},
54+
{
55+
name: "limit of 1 is honored",
56+
in: &pb.CursorPaginationRequest{Limit: 1},
57+
wantLimit: 1,
58+
},
59+
{
60+
name: "negative limit falls through to default page size",
61+
in: &pb.CursorPaginationRequest{Limit: -5},
62+
wantLimit: defaultReferrerPageSize,
63+
},
64+
{
65+
name: "proto max limit of 100 is honored",
66+
in: &pb.CursorPaginationRequest{Limit: 100},
67+
wantLimit: 100,
68+
},
69+
}
70+
71+
for _, tc := range tests {
72+
t.Run(tc.name, func(t *testing.T) {
73+
t.Parallel()
74+
75+
opts, err := referrerPaginationOptsFromProto(tc.in)
76+
require.NoError(t, err)
77+
require.NotNil(t, opts, "pagination options must always be returned so the response is bounded")
78+
assert.Equal(t, tc.wantLimit, opts.Limit)
79+
})
80+
}
81+
}

app/controlplane/pkg/data/referrer.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -238,23 +238,27 @@ func (r *ReferrerRepo) doGet(ctx context.Context, root *ent.Referrer, allowedOrg
238238
// Attach the workflow predicate
239239
predicateReferrer = append(predicateReferrer, referrer.HasWorkflowsWith(predicateWF...))
240240

241+
// Defense-in-depth: if the caller did not supply pagination options, fall back
242+
// to the package-wide default instead of emitting an unbounded query. This
243+
// guarantees the response is bounded even when a future biz-layer caller
244+
// forgets to pass options through — see chainloop-dev/chainloop#2890.
245+
if p == nil {
246+
p = &pagination.CursorOptions{Limit: pagination.DefaultCursorLimit}
247+
}
248+
241249
// Sort references by creation date and ID in descending order for deterministic pagination
242250
q := root.QueryReferences().Where(predicateReferrer...).WithWorkflows().
243251
Order(referrer.ByCreatedAt(sql.OrderDesc())).
244-
Order(referrer.ByID(sql.OrderDesc()))
245-
246-
// Apply pagination: fetch limit+1 to detect next page
247-
if p != nil {
248-
q = q.Limit(p.Limit + 1)
249-
250-
if p.Cursor != nil {
251-
q = q.Where(func(s *sql.Selector) {
252-
s.Where(sql.CompositeLT(
253-
[]string{s.C(referrer.FieldCreatedAt), s.C(referrer.FieldID)},
254-
p.Cursor.Timestamp, p.Cursor.ID,
255-
))
256-
})
257-
}
252+
Order(referrer.ByID(sql.OrderDesc())).
253+
Limit(p.Limit + 1) // fetch limit+1 to detect next page
254+
255+
if p.Cursor != nil {
256+
q = q.Where(func(s *sql.Selector) {
257+
s.Where(sql.CompositeLT(
258+
[]string{s.C(referrer.FieldCreatedAt), s.C(referrer.FieldID)},
259+
p.Cursor.Timestamp, p.Cursor.ID,
260+
))
261+
})
258262
}
259263

260264
refs, err := q.All(ctx)
@@ -264,7 +268,7 @@ func (r *ReferrerRepo) doGet(ctx context.Context, root *ent.Referrer, allowedOrg
264268

265269
// Determine if there is a next page and encode the cursor
266270
var nextCursor string
267-
if p != nil && len(refs) > p.Limit {
271+
if len(refs) > p.Limit {
268272
lastVisible := refs[p.Limit-1]
269273
nextCursor = pagination.EncodeCursor(lastVisible.CreatedAt, lastVisible.ID)
270274
refs = refs[:p.Limit]

0 commit comments

Comments
 (0)