Skip to content

Commit

Permalink
Merge branch 'main' into feat/rmdefaulttok
Browse files Browse the repository at this point in the history
  • Loading branch information
laurentsimon authored Mar 23, 2022
2 parents 83337c5 + 66b3d8c commit 96a9790
Show file tree
Hide file tree
Showing 26 changed files with 160 additions and 91 deletions.
12 changes: 11 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,23 @@ run:
concurrency: 6
deadline: 5m
issues:
new-from-rev: ""
include:
# revive `package-comments` and `exported` rules.
- EXC0012
- EXC0013
- EXC0014
- EXC0015
# Maximum issues count per one linter.
# Set to 0 to disable.
# Default: 50
max-issues-per-linter: 0
# Maximum count of issues with the same text.
# Set to 0 to disable.
# Default: 3
max-same-issues: 0
new-from-rev: ""
# Fix found issues (if it's supported by the linter).
fix: true
skip-files:
- cron/data/request.pb.go # autogenerated
linters:
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ GINKGO := ginkgo
GIT_HASH := $(shell git rev-parse HEAD)
GIT_VERSION ?= $(shell git describe --tags --always --dirty)
SOURCE_DATE_EPOCH=$(shell git log --date=iso8601-strict -1 --pretty=%ct)
GOLANGGCI_LINT := golangci-lint
GOLANGCI_LINT := golangci-lint
PROTOC_GEN_GO := protoc-gen-go
MOCKGEN := mockgen
PROTOC := $(shell which protoc)
Expand Down Expand Up @@ -58,9 +58,9 @@ update-dependencies: ## Update go dependencies for all modules
cd tools
go mod tidy && go mod verify

$(GOLANGGCI_LINT): install
$(GOLANGCI_LINT): install
check-linter: ## Install and run golang linter
check-linter: $(GOLANGGCI_LINT)
check-linter: $(GOLANGCI_LINT)
# Run golangci-lint linter
golangci-lint run -c .golangci.yml

Expand Down
3 changes: 2 additions & 1 deletion checker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge
clients.RepoClient, // ossFuzzClient
clients.CIIBestPracticesClient, // ciiClient
clients.VulnerabilitiesClient, // vulnClient
error) {
error,
) {
var githubRepo clients.Repo
if localURI != "" {
localRepo, errLocal := localdir.MakeLocalDirRepo(localURI)
Expand Down
3 changes: 2 additions & 1 deletion checks/evaluation/binary_artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import (

// BinaryArtifacts applies the score policy for the Binary-Artifacts check.
func BinaryArtifacts(name string, dl checker.DetailLogger,
r *checker.BinaryArtifactData) checker.CheckResult {
r *checker.BinaryArtifactData,
) checker.CheckResult {
if r == nil {
e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
return checker.CreateRuntimeErrorResult(name, e)
Expand Down
3 changes: 2 additions & 1 deletion checks/evaluation/branch_protection.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ type levelScore struct {

// BranchProtection runs Branch-Protection check.
func BranchProtection(name string, dl checker.DetailLogger,
r *checker.BranchProtectionsData) checker.CheckResult {
r *checker.BranchProtectionsData,
) checker.CheckResult {
var scores []levelScore

// Check protections on all the branches.
Expand Down
3 changes: 2 additions & 1 deletion checks/evaluation/code_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ var (

// CodeReview applies the score policy for the Code-Review check.
func CodeReview(name string, dl checker.DetailLogger,
r *checker.CodeReviewData) checker.CheckResult {
r *checker.CodeReviewData,
) checker.CheckResult {
if r == nil {
e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
return checker.CreateRuntimeErrorResult(name, e)
Expand Down
3 changes: 2 additions & 1 deletion checks/evaluation/dependency_update_tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import (

// DependencyUpdateTool applies the score policy for the Dependency-Update-Tool check.
func DependencyUpdateTool(name string, dl checker.DetailLogger,
r *checker.DependencyUpdateToolData) checker.CheckResult {
r *checker.DependencyUpdateToolData,
) checker.CheckResult {
if r == nil {
e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
return checker.CreateRuntimeErrorResult(name, e)
Expand Down
3 changes: 2 additions & 1 deletion checks/evaluation/vulnerabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import (

// Vulnerabilities applies the score policy for the Vulnerabilities check.
func Vulnerabilities(name string, dl checker.DetailLogger,
r *checker.VulnerabilitiesData) checker.CheckResult {
r *checker.VulnerabilitiesData,
) checker.CheckResult {
if r == nil {
e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
return checker.CreateRuntimeErrorResult(name, e)
Expand Down
3 changes: 2 additions & 1 deletion checks/fileparser/github_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ type JobMatcherStep struct {

// AnyJobsMatch returns true if any of the jobs have a match in the given workflow.
func AnyJobsMatch(workflow *actionlint.Workflow, jobMatchers []JobMatcher, fp string, dl checker.DetailLogger,
logMsgNoMatch string) bool {
logMsgNoMatch string,
) bool {
for _, job := range workflow.Jobs {
for _, matcher := range jobMatchers {
if !matcher.matches(job) {
Expand Down
3 changes: 2 additions & 1 deletion checks/fileparser/listing.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ type DoWhileTrueOnFileContent func(path string, content []byte, args ...interfac
// Continues iterating along the matched files until onFileContent returns
// either a false value or an error.
func OnMatchingFileContentDo(repoClient clients.RepoClient, matchPathTo PathMatcher,
onFileContent DoWhileTrueOnFileContent, args ...interface{}) error {
onFileContent DoWhileTrueOnFileContent, args ...interface{},
) error {
predicate := func(filepath string) (bool, error) {
// Filter out test files.
if isTestdataFile(filepath) {
Expand Down
18 changes: 12 additions & 6 deletions checks/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ func TokenPermissions(c *checker.CheckRequest) checker.CheckResult {
// Check file content.
var validateGitHubActionTokenPermissions fileparser.DoWhileTrueOnFileContent = func(path string,
content []byte,
args ...interface{}) (bool, error) {
args ...interface{},
) (bool, error) {
if !fileparser.IsWorkflowFile(path) {
return true, nil
}
Expand Down Expand Up @@ -146,7 +147,8 @@ var validateGitHubActionTokenPermissions fileparser.DoWhileTrueOnFileContent = f

func validatePermission(permissionKey permission, permissionValue *actionlint.PermissionScope,
permLevel, path string, dl checker.DetailLogger, pPermissions map[permission]bool,
ignoredPermissions map[permission]bool) error {
ignoredPermissions map[permission]bool,
) error {
if permissionValue.Value == nil {
return sce.WithMessage(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error())
}
Expand Down Expand Up @@ -188,7 +190,8 @@ func validatePermission(permissionKey permission, permissionValue *actionlint.Pe

func validateMapPermissions(scopes map[string]*actionlint.PermissionScope, permLevel, path string,
dl checker.DetailLogger, pPermissions map[permission]bool,
ignoredPermissions map[permission]bool) error {
ignoredPermissions map[permission]bool,
) error {
for key, v := range scopes {
if err := validatePermission(permission(key), v, permLevel, path, dl, pPermissions, ignoredPermissions); err != nil {
return err
Expand Down Expand Up @@ -223,7 +226,8 @@ func recordAllPermissionsWrite(p *permissionCbData, permLevel, path string) {

func validatePermissions(permissions *actionlint.Permissions, permLevel, path string,
dl checker.DetailLogger, pdata *permissionCbData,
ignoredPermissions map[permission]bool) error {
ignoredPermissions map[permission]bool,
) error {
allIsSet := permissions != nil && permissions.All != nil && permissions.All.Value != ""
scopeIsSet := permissions != nil && len(permissions.Scopes) > 0
if permissions == nil || (!allIsSet && !scopeIsSet) {
Expand Down Expand Up @@ -264,7 +268,8 @@ func validatePermissions(permissions *actionlint.Permissions, permLevel, path st
}

func validateTopLevelPermissions(workflow *actionlint.Workflow, path string,
dl checker.DetailLogger, pdata *permissionCbData) error {
dl checker.DetailLogger, pdata *permissionCbData,
) error {
// Check if permissions are set explicitly.
if workflow.Permissions == nil {
dl.Warn(&checker.LogMessage{
Expand All @@ -283,7 +288,8 @@ func validateTopLevelPermissions(workflow *actionlint.Workflow, path string,

func validatejobLevelPermissions(workflow *actionlint.Workflow, path string,
dl checker.DetailLogger, pdata *permissionCbData,
ignoredPermissions map[permission]bool) error {
ignoredPermissions map[permission]bool,
) error {
for _, job := range workflow.Jobs {
// Run-level permissions may be left undefined.
// For most workflows, no write permissions are needed,
Expand Down
3 changes: 2 additions & 1 deletion checks/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ type file struct {
}

func testValidateGitHubActionTokenPermissions(files []file,
dl checker.DetailLogger) checker.CheckResult {
dl checker.DetailLogger,
) checker.CheckResult {
data := permissionCbData{
workflows: make(map[string]permissions),
}
Expand Down
39 changes: 26 additions & 13 deletions checks/pinned_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ func dataAsDetailLogger(data interface{}) checker.DetailLogger {
}

func createReturnValuesForGitHubActionsWorkflowPinned(r worklowPinningResult, infoMsg string,
dl checker.DetailLogger, err error) (int, error) {
dl checker.DetailLogger, err error,
) (int, error) {
if err != nil {
return checker.InconclusiveResultScore, err
}
Expand Down Expand Up @@ -227,14 +228,16 @@ func isShellScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error)
}

func createReturnForIsShellScriptFreeOfInsecureDownloads(r pinnedResult,
dl checker.DetailLogger, err error) (int, error) {
dl checker.DetailLogger, err error,
) (int, error) {
return createReturnValues(r,
"no insecure (not pinned by hash) dependency downloads found in shell scripts",
dl, err)
}

func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) (int, error) {
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateShellScriptIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsShellScriptFreeOfInsecureDownloads(r, dl, err)
Expand All @@ -243,7 +246,8 @@ func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string,
var validateShellScriptIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func(
pathfn string,
content []byte,
args ...interface{}) (bool, error) {
args ...interface{},
) (bool, error) {
if len(args) != 2 {
return false, fmt.Errorf(
"validateShellScriptIsFreeOfInsecureDownloads requires exactly 2 arguments: %w", errInvalidArgLength)
Expand Down Expand Up @@ -277,14 +281,16 @@ func isDockerfileFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) {

// Create the result.
func createReturnForIsDockerfileFreeOfInsecureDownloads(r pinnedResult,
dl checker.DetailLogger, err error) (int, error) {
dl checker.DetailLogger, err error,
) (int, error) {
return createReturnValues(r,
"no insecure (not pinned by hash) dependency downloads found in Dockerfiles",
dl, err)
}

func testValidateDockerfileIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) (int, error) {
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateDockerfileIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsDockerfileFreeOfInsecureDownloads(r, dl, err)
Expand All @@ -308,7 +314,8 @@ func isDockerfile(pathfn string, content []byte) bool {
var validateDockerfileIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func(
pathfn string,
content []byte,
args ...interface{}) (bool, error) {
args ...interface{},
) (bool, error) {
if len(args) != 2 {
return false, fmt.Errorf(
"validateDockerfileIsFreeOfInsecureDownloads requires exactly 2 arguments: %w", errInvalidArgLength)
Expand Down Expand Up @@ -394,7 +401,8 @@ func testValidateDockerfileIsPinned(pathfn string, content []byte, dl checker.De
var validateDockerfileIsPinned fileparser.DoWhileTrueOnFileContent = func(
pathfn string,
content []byte,
args ...interface{}) (bool, error) {
args ...interface{},
) (bool, error) {
// Users may use various names, e.g.,
// Dockerfile.aarch64, Dockerfile.template, Dockerfile_template, dockerfile, Dockerfile-name.template

Expand Down Expand Up @@ -517,14 +525,16 @@ func isGitHubWorkflowScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int

// Create the result.
func createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r pinnedResult,
dl checker.DetailLogger, err error) (int, error) {
dl checker.DetailLogger, err error,
) (int, error) {
return createReturnValues(r,
"no insecure (not pinned by hash) dependency downloads found in GitHub workflows",
dl, err)
}

func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) (int, error) {
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, dl, err)
Expand All @@ -535,7 +545,8 @@ func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string,
var validateGitHubWorkflowIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func(
pathfn string,
content []byte,
args ...interface{}) (bool, error) {
args ...interface{},
) (bool, error) {
if !fileparser.IsWorkflowFile(pathfn) {
return true, nil
}
Expand Down Expand Up @@ -622,7 +633,8 @@ func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) (int, error) {

// Create the result.
func createReturnForIsGitHubActionsWorkflowPinned(r worklowPinningResult, dl checker.DetailLogger,
err error) (int, error) {
err error,
) (int, error) {
return createReturnValuesForGitHubActionsWorkflowPinned(r,
"actions are pinned",
dl, err)
Expand All @@ -646,7 +658,8 @@ func generateOwnerToDisplay(gitHubOwned bool) string {
var validateGitHubActionWorkflow fileparser.DoWhileTrueOnFileContent = func(
pathfn string,
content []byte,
args ...interface{}) (bool, error) {
args ...interface{},
) (bool, error) {
if !fileparser.IsWorkflowFile(pathfn) {
return true, nil
}
Expand Down
3 changes: 2 additions & 1 deletion checks/raw/binary_artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ func BinaryArtifacts(c clients.RepoClient) (checker.BinaryArtifactData, error) {
}

var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path string, content []byte,
args ...interface{}) (bool, error) {
args ...interface{},
) (bool, error) {
if len(args) != 1 {
return false, fmt.Errorf(
"checkBinaryFileContent requires exactly one argument: %w", errInvalidArgLength)
Expand Down
18 changes: 12 additions & 6 deletions checks/shell_download_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ func getLine(startLine, endLine uint, node syntax.Node) (uint, uint) {
}

func isFetchPipeExecute(startLine, endLine uint, node syntax.Node, cmd, pathfn string,
dl checker.DetailLogger) bool {
dl checker.DetailLogger,
) bool {
// BinaryCmd {Op=|, X=CallExpr{Args={curl, -s, url}}, Y=CallExpr{Args={bash,}}}.
bc, ok := node.(*syntax.BinaryCmd)
if !ok {
Expand Down Expand Up @@ -357,7 +358,8 @@ func getRedirectFile(red []*syntax.Redirect) (string, bool) {
}

func isExecuteFiles(startLine, endLine uint, node syntax.Node, cmd, pathfn string, files map[string]bool,
dl checker.DetailLogger) bool {
dl checker.DetailLogger,
) bool {
ce, ok := node.(*syntax.CallExpr)
if !ok {
return false
Expand Down Expand Up @@ -574,7 +576,8 @@ func isPipUnpinnedDownload(cmd []string) bool {
}

func isUnpinnedPakageManagerDownload(startLine, endLine uint, node syntax.Node,
cmd, pathfn string, dl checker.DetailLogger) bool {
cmd, pathfn string, dl checker.DetailLogger,
) bool {
ce, ok := node.(*syntax.CallExpr)
if !ok {
return false
Expand Down Expand Up @@ -655,7 +658,8 @@ func recordFetchFileFromNode(node syntax.Node) (pathfn string, ok bool, err erro
}

func isFetchProcSubsExecute(startLine, endLine uint, node syntax.Node, cmd, pathfn string,
dl checker.DetailLogger) bool {
dl checker.DetailLogger,
) bool {
ce, ok := node.(*syntax.CallExpr)
if !ok {
return false
Expand Down Expand Up @@ -792,7 +796,8 @@ func nodeToString(p *syntax.Printer, node syntax.Node) (string, error) {
}

func validateShellFileAndRecord(pathfn string, startLine, endLine uint, content []byte, files map[string]bool,
dl checker.DetailLogger) (bool, error) {
dl checker.DetailLogger,
) (bool, error) {
in := strings.NewReader(string(content))
f, err := syntax.NewParser().Parse(in, pathfn)
if err != nil {
Expand Down Expand Up @@ -942,7 +947,8 @@ func isMatchingShellScriptFile(pathfn string, content []byte, shellsToMatch []st
}

func validateShellFile(pathfn string, startLine, endLine uint,
content []byte, taintedFiles map[string]bool, dl checker.DetailLogger) (bool, error) {
content []byte, taintedFiles map[string]bool, dl checker.DetailLogger,
) (bool, error) {
r, err := validateShellFileAndRecord(pathfn, startLine, endLine, content, taintedFiles, dl)
if err != nil && errors.Is(err, sce.ErrorShellParsing) {
// Discard and print this particular error for now.
Expand Down
Loading

0 comments on commit 96a9790

Please sign in to comment.