Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Code Scanning API #1545

Merged
merged 7 commits into from Jun 17, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions github/authorizations.go
Expand Up @@ -41,6 +41,7 @@ const (
ScopeReadGPGKey Scope = "read:gpg_key"
ScopeWriteGPGKey Scope = "write:gpg_key"
ScopeAdminGPGKey Scope = "admin:gpg_key"
ScopeSecurityEvents Scope = "security_events"
)

// AuthorizationsService handles communication with the authorization related
Expand Down
117 changes: 117 additions & 0 deletions github/code-scanning.go
@@ -0,0 +1,117 @@
// Copyright 2020 The go-github AUTHORS. All rights reserved.
//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package github

import (
"context"
"fmt"
"strconv"
"strings"
)

// CodeScanningService handles communication with the code scanning related
// methods of the GitHub API.
//
// GitHub API docs: https://developer.github.com/v3/code-scanning/
type CodeScanningService service

type Alert struct {
RuleID *string `json:"rule_id,omitempty"`
RuleSeverity *string `json:"rule_severity,omitempty"`
RuleDescription *string `json:"rule_description,omitempty"`
Tool *string `json:"tool,omitempty"`
CreatedAt *Timestamp `json:"created_at,omitempty"`
Open *bool `json:"open,omitempty"`
ClosedBy *string `json:"closed_by,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might ClosedBy be a *User ? Maybe you could ask support@github.com to find out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's the same as ClosedBy for issues, yes. Will confirm with GitHub support.

ClosedAt *Timestamp `json:"closed_at,omitempty"`
URL *string `json:"url,omitempty"`
HTMLURL *string `json:"html_url,omitempty"`
}

// ID() returns the ID associated with an alert. It is the number at the end of the security alert's URL.
func (a *Alert) ID() int64 {
if a == nil {
return 0
}

s := a.GetHTMLURL()

// Check for an ID to parse at the end of the url
if i := strings.LastIndex(s, "/"); i >= 0 {
s = s[i+1:]
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to use URL as a fallback. In my experience, HTMLURL is always populated. That will simplify this method significantly.

Let's go ahead and delete lines 52-63.


// Return the alert ID as a 64-bit integer. Unable to convert or out of range returns 0.
id, err := strconv.ParseInt(s, 10, 64)
if err != nil {
return 0
}

return id
}

// AlertListOptions specifies optional parameters to the CodeScanningService.ListAlerts
// method.
type AlertListOptions struct {
// State of the code scanning alerts to list. Set to closed to list only closed code scanning alerts. Default: open
State string `url:"state,omitempty"`

// Return code scanning alerts for a specific branch reference. The ref must be formatted as heads/<branch name>.
Ref string `url:"ref,omitempty"`
}

// ListAlertsForRepo lists code scanning alerts for a repository.
//
// Lists all open code scanning alerts for the default branch (usually master) and protected branches in a repository.
// You must use an access token with the security_events scope to use this endpoint. GitHub Apps must have the security_events
// read permission to use this endpoint.
//
// GitHub API docs: https://developer.github.com/v3/code-scanning/#list-code-scanning-alerts-for-a-repository
func (s *CodeScanningService) ListAlertsForRepo(ctx context.Context, owner, repo string, opts *AlertListOptions) ([]*Alert, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/code-scanning/alerts", owner, repo)
u, err := addOptions(u, opts)
if err != nil {
return nil, nil, err
}

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
}

var alerts []*Alert
resp, err := s.client.Do(ctx, req, &alerts)
if err != nil {
return nil, resp, err
}

return alerts, resp, nil
}

// GetAlert gets a single code scanning alert for a repository.
//
// You must use an access token with the security_events scope to use this endpoint.
// GitHub Apps must have the security_events read permission to use this endpoint.
//
// The security alert_id is the number at the end of the security alert's URL.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to make this a little more accessible from ListAlertsForRepo so that users of the library don't have to parse it out themselves?

I'm thinking we could add a helper method like:

func (a *Alert) ID() int64 {
...
}

that would then do the parsing for the user of this repo? (If you agree, then this comment can be updated.)
Thoughts?

(Note that we want this to return 0 if a==nil or if parsing fails, and not panic. 😄 )
(Alternatively, we could change the signature to return (int64, error), but I prefer the above since they could parse things themselves if they want.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. I added a helper function that checks Alert.HTMLURL first for something resembling a url with an ID at the end (split at last forward slash, if no forward slash then tries to split at the last trailing slash in Alert.URL), then converts whatever was found to an int64. Checking HTMLURL first was because the example given for getting the alert id used HTMLURL.

Do you think it's worth checking both HTMLURL and URL, or that there should be a more extensive check to see if the URL is valid before trying to convert whatever was after the last slash to an int64?

//
// GitHub API docs: https://developer.github.com/v3/code-scanning/#get-a-code-scanning-alert
func (s *CodeScanningService) GetAlert(ctx context.Context, owner, repo string, id int64) (*Alert, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/code-scanning/alerts/%v", owner, repo, id)

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
}

a := new(Alert)
resp, err := s.client.Do(ctx, req, a)
if err != nil {
return nil, resp, err
}

return a, resp, nil
}
165 changes: 165 additions & 0 deletions github/code-scanning_test.go
@@ -0,0 +1,165 @@
// Copyright 2020 The go-github AUTHORS. All rights reserved.
//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package github

import (
"context"
"fmt"
"net/http"
"reflect"
"testing"
"time"
)

func TestActionsService_Alert_ID(t *testing.T) {
// Test: nil Alert ID == 0
var a *Alert
id := a.ID()
var want int64
if id != want {
t.Errorf("Alert.ID error returned %+v, want %+v", id, want)
}

// Test: Valid HTMLURL
a = &Alert{
HTMLURL: String("https://github.com/o/r/security/code-scanning/88"),
}
id = a.ID()
want = 88
if !reflect.DeepEqual(id, want) {
t.Errorf("Alert.ID error returned %+v, want %+v", id, want)
}

// Test: HTMLURL is nil
a = &Alert{}
id = a.ID()
want = 0
if !reflect.DeepEqual(id, want) {
t.Errorf("Alert.ID error returned %+v, want %+v", id, want)
}

// Test: ID can't be parsed as an int
a = &Alert{
HTMLURL: String("https://github.com/o/r/security/code-scanning/bad88"),
}
id = a.ID()
want = 0
if !reflect.DeepEqual(id, want) {
t.Errorf("Alert.ID error returned %+v, want %+v", id, want)
}
}

func TestActionsService_ListAlertsForRepo(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/repos/o/r/code-scanning/alerts", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testFormValues(t, r, values{"state": "open", "ref": "heads/master"})
fmt.Fprint(w, `[{
"rule_id":"js/trivial-conditional",
"rule_severity":"warning",
"rule_description":"Useless conditional",
"tool":"CodeQL",
"created_at":"2020-05-06T12:00:00Z",
"open":true,
"closed_by":null,
"closed_at":null,
"url":"https://api.github.com/repos/o/r/code-scanning/alerts/25",
"html_url":"https://github.com/o/r/security/code-scanning/25"
},
{
"rule_id":"js/useless-expression",
"rule_severity":"warning",
"rule_description":"Expression has no effect",
"tool":"CodeQL",
"created_at":"2020-05-06T12:00:00Z",
"open":true,
"closed_by":null,
"closed_at":null,
"url":"https://api.github.com/repos/o/r/code-scanning/alerts/88",
"html_url":"https://github.com/o/r/security/code-scanning/88"
}]`)
})

opts := &AlertListOptions{State: "open", Ref: "heads/master"}
alerts, _, err := client.CodeScanning.ListAlertsForRepo(context.Background(), "o", "r", opts)
if err != nil {
t.Errorf("CodeScanning.ListAlertsForRepo returned error: %v", err)
}

date := Timestamp{time.Date(2020, time.May, 06, 12, 00, 00, 0, time.UTC)}
want := []*Alert{
{
RuleID: String("js/trivial-conditional"),
RuleSeverity: String("warning"),
RuleDescription: String("Useless conditional"),
Tool: String("CodeQL"),
CreatedAt: &date,
Open: Bool(true),
ClosedBy: nil,
ClosedAt: nil,
URL: String("https://api.github.com/repos/o/r/code-scanning/alerts/25"),
HTMLURL: String("https://github.com/o/r/security/code-scanning/25"),
},
{
RuleID: String("js/useless-expression"),
RuleSeverity: String("warning"),
RuleDescription: String("Expression has no effect"),
Tool: String("CodeQL"),
CreatedAt: &date,
Open: Bool(true),
ClosedBy: nil,
ClosedAt: nil,
URL: String("https://api.github.com/repos/o/r/code-scanning/alerts/88"),
HTMLURL: String("https://github.com/o/r/security/code-scanning/88"),
},
}
if !reflect.DeepEqual(alerts, want) {
t.Errorf("CodeScanning.ListAlertsForRepo returned %+v, want %+v", alerts, want)
}
}

func TestActionsService_GetAlert(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/repos/o/r/code-scanning/alerts/88", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, `{"rule_id":"js/useless-expression",
"rule_severity":"warning",
"rule_description":"Expression has no effect",
"tool":"CodeQL",
"created_at":"2019-01-02T15:04:05Z",
"open":true,
"closed_by":null,
"closed_at":null,
"url":"https://api.github.com/repos/o/r/code-scanning/alerts/88",
"html_url":"https://github.com/o/r/security/code-scanning/88"}`)
})

alert, _, err := client.CodeScanning.GetAlert(context.Background(), "o", "r", 88)
if err != nil {
t.Errorf("CodeScanning.GetAlert returned error: %v", err)
}

date := Timestamp{time.Date(2019, time.January, 02, 15, 04, 05, 0, time.UTC)}
want := &Alert{
RuleID: String("js/useless-expression"),
RuleSeverity: String("warning"),
RuleDescription: String("Expression has no effect"),
Tool: String("CodeQL"),
CreatedAt: &date,
Open: Bool(true),
ClosedBy: nil,
ClosedAt: nil,
URL: String("https://api.github.com/repos/o/r/code-scanning/alerts/88"),
HTMLURL: String("https://github.com/o/r/security/code-scanning/88"),
}
if !reflect.DeepEqual(alert, want) {
t.Errorf("CodeScanning.GetAlert returned %+v, want %+v", alert, want)
}
}
80 changes: 80 additions & 0 deletions github/github-accessors.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions github/github.go
Expand Up @@ -165,6 +165,7 @@ type Client struct {
Apps *AppsService
Authorizations *AuthorizationsService
Checks *ChecksService
CodeScanning *CodeScanningService
Gists *GistsService
Git *GitService
Gitignores *GitignoresService
Expand Down Expand Up @@ -271,6 +272,7 @@ func NewClient(httpClient *http.Client) *Client {
c.Apps = (*AppsService)(&c.common)
c.Authorizations = (*AuthorizationsService)(&c.common)
c.Checks = (*ChecksService)(&c.common)
c.CodeScanning = (*CodeScanningService)(&c.common)
c.Gists = (*GistsService)(&c.common)
c.Git = (*GitService)(&c.common)
c.Gitignores = (*GitignoresService)(&c.common)
Expand Down