Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -717,8 +717,8 @@ The following sets of tools are available:
- **list_dependabot_alerts** - List dependabot alerts
- **Required OAuth Scopes**: `security_events`
- **Accepted OAuth Scopes**: `repo`, `security_events`
- `after`: Forward pagination cursor from the previous response's pageInfo.nextCursor. (string, optional)
- `owner`: The owner of the repository. (string, required)
- `page`: Page number for pagination (min 1) (number, optional)
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)
- `repo`: The name of the repository. (string, required)
- `severity`: Filter dependabot alerts by severity (string, optional)
Expand Down
9 changes: 4 additions & 5 deletions pkg/github/__toolsnaps__/list_dependabot_alerts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
"description": "List dependabot alerts in a GitHub repository.",
"inputSchema": {
"properties": {
"after": {
"description": "Forward pagination cursor from the previous response's pageInfo.nextCursor.",
"type": "string"
},
"owner": {
"description": "The owner of the repository.",
"type": "string"
},
"page": {
"description": "Page number for pagination (min 1)",
"minimum": 1,
"type": "number"
},
"perPage": {
"description": "Results per page for pagination (min 1, max 100)",
"maximum": 100,
Expand Down
19 changes: 14 additions & 5 deletions pkg/github/dependabot.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
},
Required: []string{"owner", "repo"},
}
WithPagination(schema)
WithCursorPagination(schema)
// The Dependabot alerts REST endpoint uses cursor pagination via the response's
// Link header (surfaced as pageInfo.nextCursor), not GraphQL. Override the shared
// cursor description, which otherwise refers to a GraphQL PageInfo.
schema.Properties["after"].Description = "Forward pagination cursor from the previous response's pageInfo.nextCursor."
Comment thread
RossTarrant marked this conversation as resolved.
Outdated

return NewTool(
ToolsetMetadataDependabot,
Expand Down Expand Up @@ -152,7 +156,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
return utils.NewToolResultError(err.Error()), nil, nil
}

pagination, err := OptionalPaginationParams(args)
pagination, err := OptionalCursorPaginationParams(args)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
Expand All @@ -165,9 +169,9 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
alerts, resp, err := client.Dependabot.ListRepoAlerts(ctx, owner, repo, &github.ListAlertsOptions{
State: ToStringPtr(state),
Severity: ToStringPtr(severity),
ListOptions: github.ListOptions{
Page: pagination.Page,
ListCursorOptions: github.ListCursorOptions{
PerPage: pagination.PerPage,
After: pagination.After,
},
})
if err != nil {
Expand All @@ -187,7 +191,12 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil
}

r, err := json.Marshal(alerts)
response := map[string]any{
"alerts": alerts,
"pageInfo": buildPageInfo(resp),
}

r, err := json.Marshal(response)
if err != nil {
Comment thread
RossTarrant marked this conversation as resolved.
return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), nil, err
}
Expand Down
61 changes: 45 additions & 16 deletions pkg/github/dependabot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,19 +154,19 @@ func Test_ListDependabotAlerts(t *testing.T) {
}

tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedAlerts []*github.DependabotAlert
expectedErrMsg string
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedAlerts []*github.DependabotAlert
expectedNextCursor string
expectedErrMsg string
}{
{
name: "successful open alerts listing",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
"state": "open",
"page": "1",
"per_page": "30",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert}),
Expand All @@ -185,7 +185,6 @@ func Test_ListDependabotAlerts(t *testing.T) {
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
"severity": "high",
"page": "1",
"per_page": "30",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&highSeverityAlert}),
Expand All @@ -203,7 +202,6 @@ func Test_ListDependabotAlerts(t *testing.T) {
name: "successful all alerts listing",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
"page": "1",
"per_page": "30",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert, &highSeverityAlert}),
Expand All @@ -217,10 +215,10 @@ func Test_ListDependabotAlerts(t *testing.T) {
expectedAlerts: []*github.DependabotAlert{&criticalAlert, &highSeverityAlert},
},
{
name: "successful alerts listing with custom pagination",
name: "successful alerts listing with cursor pagination",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
"page": "3",
"after": "Y3Vyc29yOnYyOpK5",
"per_page": "100",
}).andThen(
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert}),
Expand All @@ -229,12 +227,35 @@ func Test_ListDependabotAlerts(t *testing.T) {
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"page": float64(3),
"after": "Y3Vyc29yOnYyOpK5",
"perPage": float64(100),
},
expectError: false,
expectedAlerts: []*github.DependabotAlert{&criticalAlert},
},
{
name: "successful alerts listing surfaces next page cursor",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
"per_page": "30",
}).andThen(
func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Link", `<https://api.github.com/repos/owner/repo/dependabot/alerts?after=nextcursor123&per_page=30>; rel="next"`)
w.WriteHeader(http.StatusOK)
b, err := json.Marshal([]*github.DependabotAlert{&criticalAlert})
require.NoError(t, err)
_, _ = w.Write(b)
},
),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
},
expectError: false,
expectedAlerts: []*github.DependabotAlert{&criticalAlert},
expectedNextCursor: "nextcursor123",
},
{
name: "alerts listing fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
Expand Down Expand Up @@ -291,11 +312,17 @@ func Test_ListDependabotAlerts(t *testing.T) {
textContent := getTextResult(t, result)

// Unmarshal and verify the result
var returnedAlerts []*github.DependabotAlert
err = json.Unmarshal([]byte(textContent.Text), &returnedAlerts)
var returnedResult struct {
Alerts []*github.DependabotAlert `json:"alerts"`
PageInfo struct {
HasNextPage bool `json:"hasNextPage"`
NextCursor string `json:"nextCursor"`
} `json:"pageInfo"`
}
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
assert.NoError(t, err)
assert.Len(t, returnedAlerts, len(tc.expectedAlerts))
for i, alert := range returnedAlerts {
assert.Len(t, returnedResult.Alerts, len(tc.expectedAlerts))
for i, alert := range returnedResult.Alerts {
assert.Equal(t, *tc.expectedAlerts[i].Number, *alert.Number)
assert.Equal(t, *tc.expectedAlerts[i].HTMLURL, *alert.HTMLURL)
assert.Equal(t, *tc.expectedAlerts[i].State, *alert.State)
Expand All @@ -304,6 +331,8 @@ func Test_ListDependabotAlerts(t *testing.T) {
assert.Equal(t, *tc.expectedAlerts[i].SecurityAdvisory.Severity, *alert.SecurityAdvisory.Severity)
}
}
assert.Equal(t, tc.expectedNextCursor, returnedResult.PageInfo.NextCursor)
assert.Equal(t, tc.expectedNextCursor != "", returnedResult.PageInfo.HasNextPage)
})
}
}