From c11d71671d94cf258fa98e2359bdef3a47634998 Mon Sep 17 00:00:00 2001 From: Vishal Punjabi <10079537+vshalpnjabi@users.noreply.github.com> Date: Mon, 2 Nov 2026 22:36:50 +0000 Subject: [PATCH] Reject list_label perPage=0 instead of defaulting to 100 --- pkg/github/labels.go | 10 +++++++--- pkg/github/labels_test.go | 18 ++++++++++++++++++ pkg/github/params.go | 4 ++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/pkg/github/labels.go b/pkg/github/labels.go index c9479e5062..9689d982a1 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -177,9 +177,13 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultError(err.Error()), nil, nil } - perPage, err := OptionalIntParamWithDefault(args, "perPage", listLabelsDefaultPerPage) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + // Distinguish omitted perPage (default 100) from an explicit 0, which is out of range. + perPage := listLabelsDefaultPerPage + if _, present := args["perPage"]; present { + perPage, err = OptionalIntParam(args, "perPage") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } } after, err := OptionalParam[string](args, "after") if err != nil { diff --git a/pkg/github/labels_test.go b/pkg/github/labels_test.go index 43075659bc..9e2aabc872 100644 --- a/pkg/github/labels_test.go +++ b/pkg/github/labels_test.go @@ -217,6 +217,24 @@ func TestListLabels(t *testing.T) { expectToolError: true, expectedToolErrMsg: "perPage value 101 exceeds maximum of 100", }, + { + // Explicit 0 must not be treated as "omitted" and replaced by the default of 100. + // The mock matches first=100 so a silent default would succeed and fail this case. + name: "rejects a page size of 0", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "perPage": float64(0), + }, + mockedClient: githubv4mock.NewMockedHTTPClient(listLabelsMatcher(100, (*githubv4.String)(nil), map[string]any{ + "hasNextPage": githubv4.Boolean(false), + "hasPreviousPage": githubv4.Boolean(false), + "startCursor": githubv4.String("c1"), + "endCursor": githubv4.String("c2"), + })), + expectToolError: true, + expectedToolErrMsg: "perPage value 0 is below minimum of 1", + }, } for _, tc := range tests { diff --git a/pkg/github/params.go b/pkg/github/params.go index 416020abb7..0583c02ecb 100644 --- a/pkg/github/params.go +++ b/pkg/github/params.go @@ -476,8 +476,8 @@ func (p CursorPaginationParams) ToGraphQLParams() (*GraphQLPaginationParams, err if p.PerPage > 100 { return nil, fmt.Errorf("perPage value %d exceeds maximum of 100", p.PerPage) } - if p.PerPage < 0 { - return nil, fmt.Errorf("perPage value %d cannot be negative", p.PerPage) + if p.PerPage < 1 { + return nil, fmt.Errorf("perPage value %d is below minimum of 1", p.PerPage) } first := int32(p.PerPage)