chore: Add fmt/lint/vet/sec checks

Signed-off-by: Andrew Bayer <[email protected]>

Makefile

@@ -1,6 +1,50 @@
+# Make does not offer a recursive wildcard function, so here's one:
+rwildcard=$(wildcard $1$2) $(foreach d,$(wildcard $1*),$(call rwildcard,$d/,$2))
+
+GO_DEPENDENCIES := $(call rwildcard,pkg/,*.go) $(call rwildcard,scm/,*.go)
+GO := GO111MODULE=on go
+GO_NOMOD := GO111MODULE=off go
+
 build: test
 
 test:
 	go test ./...
 
-linux: build
+linux: build
+
+.PHONY: check
+check: fmt lint sec ## Runs Go format check as well as security checks
+
+get-fmt-deps:
+	$(GO_NOMOD) get golang.org/x/tools/cmd/goimports
+
+.PHONY: importfmt
+importfmt: get-fmt-deps ## Checks the import format of the Go source files
+	@echo "FORMATTING IMPORTS"
+	@goimports -w $(GO_DEPENDENCIES)
+
+.PHONY: fmt ## Checks Go source files are formatted properly
+fmt: importfmt
+	@echo "FORMATTING SOURCE"
+	FORMATTED=`$(GO) fmt ./...`
+	@([[ ! -z "$(FORMATTED)" ]] && printf "Fixed un-formatted files:\n$(FORMATTED)") || true
+
+GOLINT := $(GOPATH)/bin/golint
+$(GOLINT):
+	$(GO_NOMOD) get -u golang.org/x/lint/golint
+
+.PHONY: lint
+lint: $(GOLINT) ## Runs 'go vet' anf 'go lint'
+	@echo "VETTING"
+	$(GO) vet ./...
+	@echo "LINTING"
+	$(GOLINT) -set_exit_status ./...
+
+GOSEC := $(GOPATH)/bin/gosec
+$(GOSEC):
+	$(GO_NOMOD) get -u github.com/securego/gosec/cmd/gosec
+
+.PHONY: sec
+sec: $(GOSEC) ## Runs gosec to check for potential security issues in the Go source
+	@echo "SECURITY SCANNING"
+	$(GOSEC) -quiet -fmt=csv ./...

jenkins-x.yml

@@ -33,5 +33,5 @@ pipelineConfig:
           - name: test
             command: make
             args:
-            - test
+            - check test
             dir: /workspace/source

pkg/hmac/hmac.go

@@ -6,7 +6,7 @@ package hmac
 
 import (
 	"crypto/hmac"
-	"crypto/sha1"
+	"crypto/sha1" // #nosec
 	"crypto/sha256"
 	"encoding/hex"
 	"hash"
@@ -42,7 +42,7 @@ func ValidatePrefix(message, key []byte, signature string) bool {
 
 func validate(h func() hash.Hash, message, key, signature []byte) bool {
 	mac := hmac.New(h, key)
-	mac.Write(message)
+	mac.Write(message) // #nosec
 	sum := mac.Sum(nil)
 	return hmac.Equal(signature, sum)
 }

scm/app.go

@@ -6,6 +6,7 @@ import (
 )
 
 type (
+	// InstallationToken is the token used for interacting with the app
 	InstallationToken struct {
 		Token     string
 		ExpiresAt *time.Time

scm/client.go

@@ -174,9 +174,9 @@ func (c *Client) Do(ctx context.Context, in *Request) (*Response, error) {
 
 	// dumps the response for debugging purposes.
 	if c.DumpResponse != nil {
-		c.DumpResponse(res, true)
+		_, err = c.DumpResponse(res, true)
 	}
-	return newResponse(res), nil
+	return newResponse(res), err
 }
 
 // newResponse creates a new Response for the provided

scm/const.go

@@ -71,10 +71,12 @@ func ToState(s string) State {
 	}
 }
 
+// MarshalJSON marshals State to JSON
 func (s State) MarshalJSON() ([]byte, error) {
 	return []byte(fmt.Sprintf(`"%s"`, s.String())), nil
 }
 
+// UnmarshalJSON unmarshals JSON to State
 func (s *State) UnmarshalJSON(b []byte) error {
 	*s = ToState(strings.Trim(string(b), `"`))
 	return nil

scm/driver/bitbucket/bitbucket.go

@@ -16,6 +16,7 @@ import (
 	"github.com/jenkins-x/go-scm/scm"
 )
 
+// NewWebHookService creates a new instance of the webhook service without the rest of the client
 func NewWebHookService() scm.WebhookService {
 	return &webhookService{nil}
 }
@@ -70,7 +71,7 @@ func (c *wrapper) do(ctx context.Context, method, path string, in, out interface
 	// write it to the body of the request.
 	if in != nil {
 		buf := new(bytes.Buffer)
-		json.NewEncoder(buf).Encode(in)
+		json.NewEncoder(buf).Encode(in) // #nosec
 		req.Header = map[string][]string{
 			"Content-Type": {"application/json"},
 		}
@@ -90,7 +91,7 @@ func (c *wrapper) do(ctx context.Context, method, path string, in, out interface
 		return res, scm.ErrNotAuthorized
 	} else if res.Status > 300 {
 		err := new(Error)
-		json.NewDecoder(res.Body).Decode(err)
+		json.NewDecoder(res.Body).Decode(err) // #nosec
 		return res, err
 	}
 
@@ -101,8 +102,8 @@ func (c *wrapper) do(ctx context.Context, method, path string, in, out interface
 	// if raw output is expected, copy to the provided
 	// buffer and exit.
 	if w, ok := out.(io.Writer); ok {
-		io.Copy(w, res.Body)
-		return res, nil
+		_, err := io.Copy(w, res.Body)
+		return res, err
 	}
 
 	// if a json response is expected, parse and return

scm/driver/bitbucket/git.go

@@ -84,31 +84,43 @@ func (s *gitService) ListBranches(ctx context.Context, repo string, opts scm.Lis
 	path := fmt.Sprintf("2.0/repositories/%s/refs/branches?%s", repo, encodeListOptions(opts))
 	out := new(branches)
 	res, err := s.client.do(ctx, "GET", path, nil, out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertBranchList(out), res, err
 }
 
 func (s *gitService) ListCommits(ctx context.Context, repo string, opts scm.CommitListOptions) ([]*scm.Commit, *scm.Response, error) {
 	path := fmt.Sprintf("2.0/repositories/%s/commits/%s?%s", repo, opts.Ref, encodeCommitListOptions(opts))
 	out := new(commits)
 	res, err := s.client.do(ctx, "GET", path, nil, out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertCommitList(out), res, err
 }
 
 func (s *gitService) ListTags(ctx context.Context, repo string, opts scm.ListOptions) ([]*scm.Reference, *scm.Response, error) {
 	path := fmt.Sprintf("2.0/repositories/%s/refs/tags?%s", repo, encodeListOptions(opts))
 	out := new(branches)
 	res, err := s.client.do(ctx, "GET", path, nil, &out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertTagList(out), res, err
 }
 
 func (s *gitService) ListChanges(ctx context.Context, repo, ref string, opts scm.ListOptions) ([]*scm.Change, *scm.Response, error) {
 	path := fmt.Sprintf("2.0/repositories/%s/diffstat/%s?%s", repo, ref, encodeListOptions(opts))
 	out := new(diffstats)
 	res, err := s.client.do(ctx, "GET", path, nil, &out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertDiffstats(out), res, err
 }
 

scm/driver/bitbucket/org.go

@@ -54,7 +54,10 @@ func (s *organizationService) List(ctx context.Context, opts scm.ListOptions) ([
 	path := fmt.Sprintf("2.0/teams?%s", encodeListRoleOptions(opts))
 	out := new(organizationList)
 	res, err := s.client.do(ctx, "GET", path, nil, out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertOrganizationList(out), res, err
 }
 

scm/driver/bitbucket/pr.go

@@ -37,15 +37,21 @@ func (s *pullService) List(ctx context.Context, repo string, opts scm.PullReques
 		return nil, res, err
 	}
 	res, err := s.client.do(ctx, "GET", path, nil, out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertPullRequests(out), res, err
 }
 
 func (s *pullService) ListChanges(ctx context.Context, repo string, number int, opts scm.ListOptions) ([]*scm.Change, *scm.Response, error) {
 	path := fmt.Sprintf("2.0/repositories/%s/pullrequests/%d/diffstat?%s", repo, number, encodeListOptions(opts))
 	out := new(diffstats)
 	res, err := s.client.do(ctx, "GET", path, nil, out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertDiffstats(out), res, err
 }
 

scm/driver/bitbucket/repo.go

@@ -127,7 +127,10 @@ func (s *repositoryService) List(ctx context.Context, opts scm.ListOptions) ([]*
 	}
 	out := new(repositories)
 	res, err := s.client.do(ctx, "GET", path, nil, &out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertRepositoryList(out), res, err
 }
 
@@ -144,7 +147,10 @@ func (s *repositoryService) ListHooks(ctx context.Context, repo string, opts scm
 	path := fmt.Sprintf("2.0/repositories/%s/hooks?%s", repo, encodeListOptions(opts))
 	out := new(hooks)
 	res, err := s.client.do(ctx, "GET", path, nil, out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertHookList(out), res, err
 }
 
@@ -153,7 +159,10 @@ func (s *repositoryService) ListStatus(ctx context.Context, repo, ref string, op
 	path := fmt.Sprintf("2.0/repositories/%s/commit/%s/statuses?%s", repo, ref, encodeListOptions(opts))
 	out := new(statuses)
 	res, err := s.client.do(ctx, "GET", path, nil, out)
-	copyPagination(out.pagination, res)
+	if err != nil {
+		return nil, res, err
+	}
+	err = copyPagination(out.pagination, res)
 	return convertStatusList(out), res, err
 }
 

scm/driver/fake/content.go

@@ -32,7 +32,7 @@ func (c contentService) Find(_ context.Context, repo, path, ref string) (*scm.Co
 			Status: 404,
 		}, errors.Wrapf(err, "file %s does not exist", f)
 	}
-	data, err := ioutil.ReadFile(f)
+	data, err := ioutil.ReadFile(f) // #nosec
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "failed to read file %s", f)
 	}
@@ -129,7 +129,7 @@ func (c contentService) path(repo string, path string, ref string) (string, erro
 	return filepath.Join(repoDir, path), nil
 }
 
-/// DirExists checks if path exists and is a directory
+// DirExists checks if path exists and is a directory
 func DirExists(path string) (bool, error) {
 	info, err := os.Stat(path)
 	if err == nil {

scm/driver/fake/data.go

@@ -2,6 +2,7 @@ package fake
 
 import "github.com/jenkins-x/go-scm/scm"
 
+// Data is used to store/represent test data for the fake client
 type Data struct {
 	Issues                     map[int][]*scm.Issue
 	OrgMembers                 map[string][]string
@@ -74,6 +75,7 @@ type Data struct {
 	ContentDir string
 }
 
+// DeletedRef represents a ref that has been deleted
 type DeletedRef struct {
 	Org, Repo, Ref string
 }

scm/driver/fake/org.go

@@ -62,9 +62,9 @@ func (s *organizationService) List(context.Context, scm.ListOptions) ([]*scm.Org
 				Name:   fmt.Sprintf("organisation%d", i),
 				Avatar: fmt.Sprintf("https://github.com/organisation%d.png", i),
 				Permissions: scm.Permissions{
-					true,
-					true,
-					true,
+					MembersCreatePrivate:  true,
+					MembersCreatePublic:   true,
+					MembersCreateInternal: true,
 				},
 			}
 			orgs = append(orgs, &org)

scm/driver/fake/repo.go

@@ -175,6 +175,7 @@ func (s *repositoryService) ListHooks(ctx context.Context, fullName string, opts
 }
 
 func (s *repositoryService) CreateHook(ctx context.Context, fullName string, input *scm.HookInput) (*scm.Hook, *scm.Response, error) {
+	/* #nosec */
 	hook := &scm.Hook{
 		ID:     fmt.Sprintf("%d", rand.Int()),
 		Name:   input.Name,

scm/driver/fake/user.go

@@ -19,16 +19,16 @@ func (s *userService) DeleteToken(context.Context, int64) (*scm.Response, error)
 	return nil, scm.ErrNotSupported
 }
 
-func (u *userService) Find(ctx context.Context) (*scm.User, *scm.Response, error) {
-	return &u.data.CurrentUser, nil, nil
+func (s *userService) Find(ctx context.Context) (*scm.User, *scm.Response, error) {
+	return &s.data.CurrentUser, nil, nil
 }
 
-func (u *userService) FindEmail(ctx context.Context) (string, *scm.Response, error) {
-	return u.data.CurrentUser.Email, nil, nil
+func (s *userService) FindEmail(ctx context.Context) (string, *scm.Response, error) {
+	return s.data.CurrentUser.Email, nil, nil
 }
 
-func (u *userService) FindLogin(ctx context.Context, login string) (*scm.User, *scm.Response, error) {
-	for _, user := range u.data.Users {
+func (s *userService) FindLogin(ctx context.Context, login string) (*scm.User, *scm.Response, error) {
+	for _, user := range s.data.Users {
 		if user.Login == login {
 			return user, nil, nil
 		}

scm/driver/gitea/gitea.go

@@ -19,6 +19,7 @@ import (
 	"github.com/jenkins-x/go-scm/scm"
 )
 
+// NewWebHookService creates a new instance of the webhook service without the rest of the client
 func NewWebHookService() scm.WebhookService {
 	return &webhookService{nil}
 }