Skip to content

fix(referrer): apply default page size when request has no pagination#3023

Draft
waveywaves wants to merge 1 commit intochainloop-dev:mainfrom
waveywaves:fix/referrer-unbounded-references
Draft

fix(referrer): apply default page size when request has no pagination#3023
waveywaves wants to merge 1 commit intochainloop-dev:mainfrom
waveywaves:fix/referrer-unbounded-references

Conversation

@waveywaves
Copy link
Copy Markdown
Contributor

Summary

Fixes #2890.

The ReferrerService.DiscoverPrivate and DiscoverPublicShared endpoints return an unbounded number of direct references (SBOMs, SARIF reports, …) when a client sends an empty pagination field. A container image that is attested repeatedly accumulates these references over time, so a single call can return hundreds of items.

Root cause

referrerPaginationOptsFromProto in app/controlplane/internal/service/referrer.go returned (nil, nil) whenever the request's pagination was nil, and the data layer treated nil *CursorOptions as "skip pagination entirely":

```go
// app/controlplane/pkg/data/referrer.go (unchanged)
if p != nil {
q = q.Limit(p.Limit + 1)
// …
}
```

This was intentional "backward compatibility" per the helper's comment, but it is the bug the issue describes.

Fix

Always return *pagination.CursorOptions with the existing defaultReferrerPageSize = 20:

  • nil request → default page size
  • empty request → default page size
  • Limit: 0 → default page size
  • explicit Limit: N → honored as-is (still capped at 100 by the proto validation rule)

The biz-layer traversal is still bounded by maxTraverseLevels = 1 (app/controlplane/pkg/data/referrer.go:196), so level-0 refs are now bounded and nothing deeper needs a change.

Client impact

  • CLI: unaffected. app/cli/cmd/referrer_discover.go already sends a non-nil pagination request with DefaultLimit: 20.
  • Raw gRPC/REST clients: the default response now contains at most 20 direct references. Clients that want more must pass pagination.limit explicitly (up to 100) or page through with pagination.cursor. This is a behavior change but it is the fix the issue asks for.

Test plan

  • go test ./internal/service/... -run TestReferrerPaginationOptsFromProto -v — 5 subtests covering nil / empty / zero / explicit / boundary limit
  • go test ./internal/service/... — full service package passes
  • go vet ./internal/service/... clean
  • gofmt -l clean
  • Commits GPG-signed and DCO signed-off

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#2890

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Referrer discover API returns unbounded references

1 participant