From e41f8595cbf34a9720bbfca079bd201b763b13a4 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Tue, 22 Feb 2022 15:40:34 -0800 Subject: [PATCH] Generalize CheckFileContent functions (#1670) Co-authored-by: Azeem Shaikh --- checks/dangerous_workflow.go | 28 +++++-- checks/fileparser/listing.go | 103 ++++++------------------ checks/fileparser/listing_test.go | 68 +++------------- checks/fuzzing.go | 12 +-- checks/permissions.go | 106 ++++++++++++++----------- checks/pinned_dependencies.go | 125 +++++++++++++++++++++--------- checks/raw/binary_artifact.go | 19 +++-- 7 files changed, 223 insertions(+), 238 deletions(-) diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go index 4731e72f..bfe83e65 100644 --- a/checks/dangerous_workflow.go +++ b/checks/dangerous_workflow.go @@ -98,23 +98,37 @@ func DangerousWorkflow(c *checker.CheckRequest) checker.CheckResult { data := patternCbData{ workflowPattern: make(map[dangerousResults]bool), } - err := fileparser.CheckFilesContent(".github/workflows/*", false, - c, validateGitHubActionWorkflowPatterns, &data) + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: ".github/workflows/*", + CaseSensitive: false, + }, + validateGitHubActionWorkflowPatterns, c.Dlogger, &data) return createResultForDangerousWorkflowPatterns(data, err) } // Check file content. -func validateGitHubActionWorkflowPatterns(path string, content []byte, dl checker.DetailLogger, - data fileparser.FileCbData) (bool, error) { +var validateGitHubActionWorkflowPatterns fileparser.DoWhileTrueOnFileContent = func(path string, + content []byte, + args ...interface{}) (bool, error) { if !fileparser.IsWorkflowFile(path) { return true, nil } + if len(args) != 2 { + return false, fmt.Errorf( + "validateGitHubActionWorkflowPatterns requires exactly 2 arguments: %w", errInvalidArgLength) + } + // Verify the type of the data. - pdata, ok := data.(*patternCbData) + pdata, ok := args[1].(*patternCbData) if !ok { - // This never happens. - panic("invalid type") + return false, fmt.Errorf( + "validateGitHubActionWorkflowPatterns expects arg[0] of type *patternCbData: %w", errInvalidArgType) + } + dl, ok := args[0].(checker.DetailLogger) + if !ok { + return false, fmt.Errorf( + "validateGitHubActionWorkflowPatterns expects arg[1] of type checker.DetailLogger: %w", errInvalidArgType) } if !fileparser.CheckFileContainsCommands(content, "#") { diff --git a/checks/fileparser/listing.go b/checks/fileparser/listing.go index e141ea4d..44582315 100644 --- a/checks/fileparser/listing.go +++ b/checks/fileparser/listing.go @@ -20,16 +20,16 @@ import ( "path" "strings" - "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/clients" sce "github.com/ossf/scorecard/v4/errors" ) // isMatchingPath uses 'pattern' to shell-match the 'path' and its filename // 'caseSensitive' indicates the match should be case-sensitive. Default: no. -func isMatchingPath(pattern, fullpath string, caseSensitive bool) (bool, error) { - if !caseSensitive { - pattern = strings.ToLower(pattern) +func isMatchingPath(fullpath string, matchPathTo PathMatcher) (bool, error) { + pattern := matchPathTo.Pattern + if !matchPathTo.CaseSensitive { + pattern = strings.ToLower(matchPathTo.Pattern) fullpath = strings.ToLower(fullpath) } @@ -55,87 +55,30 @@ func isTestdataFile(fullpath string) bool { strings.Contains(fullpath, "/testdata/") } -// FileCbData is any data the caller can act upon -// to keep state. -type FileCbData interface{} - -// FileContentCb is the callback. -// The bool returned indicates whether the CheckFilesContent2 -// should continue iterating over files or not. -type FileContentCb func(path string, content []byte, - dl checker.DetailLogger, data FileCbData) (bool, error) - -// CheckFilesContent downloads the tar of the repository and calls the onFileContent() function -// shellPathFnPattern is used for https://golang.org/pkg/path/#Match -// Warning: the pattern is used to match (1) the entire path AND (2) the filename alone. This means: -// - To scope the search to a directory, use "./dirname/*". Example, for the root directory, -// use "./*". -// - A pattern such as "*mypatern*" will match files containing mypattern in *any* directory. -func CheckFilesContent(shellPathFnPattern string, - caseSensitive bool, - c *checker.CheckRequest, - onFileContent FileContentCb, - data FileCbData, -) error { - predicate := func(filepath string) (bool, error) { - // Filter out test files. - if isTestdataFile(filepath) { - return false, nil - } - // Filter out files based on path/names using the pattern. - b, err := isMatchingPath(shellPathFnPattern, filepath, caseSensitive) - if err != nil { - return false, err - } - return b, nil - } - - matchedFiles, err := c.RepoClient.ListFiles(predicate) - if err != nil { - // nolint: wrapcheck - return err - } - - for _, file := range matchedFiles { - content, err := c.RepoClient.GetFileContent(file) - if err != nil { - //nolint - return err - } - - continueIter, err := onFileContent(file, content, c.Dlogger, data) - if err != nil { - return err - } - - if !continueIter { - break - } - } - - return nil +// PathMatcher represents a query for a filepath. +type PathMatcher struct { + Pattern string + CaseSensitive bool } -// FileContentCbV6 is the callback. -// The bool returned indicates whether the CheckFilesContent2 -// should continue iterating over files or not. -type FileContentCbV6 func(path string, content []byte, data FileCbData) (bool, error) +// DoWhileTrueOnFileContent takes a filepath, its content and +// optional variadic args. It returns a boolean indicating whether +// iterating over next files should continue. +type DoWhileTrueOnFileContent func(path string, content []byte, args ...interface{}) (bool, error) -// CheckFilesContentV6 is the same as CheckFilesContent -// but for use with separated check/policy code. -func CheckFilesContentV6(shellPathFnPattern string, - caseSensitive bool, - repoClient clients.RepoClient, - onFileContent FileContentCbV6, - data FileCbData, -) error { +// OnMatchingFileContentDo matches all files listed by `repoClient` against `matchPathTo` +// and on every successful match, runs onFileContent fn on the file's contents. +// 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 { predicate := func(filepath string) (bool, error) { // Filter out test files. if isTestdataFile(filepath) { return false, nil } // Filter out files based on path/names using the pattern. - b, err := isMatchingPath(shellPathFnPattern, filepath, caseSensitive) + b, err := isMatchingPath(filepath, matchPathTo) if err != nil { return false, err } @@ -144,18 +87,16 @@ func CheckFilesContentV6(shellPathFnPattern string, matchedFiles, err := repoClient.ListFiles(predicate) if err != nil { - // nolint: wrapcheck - return err + return fmt.Errorf("error during ListFiles: %w", err) } for _, file := range matchedFiles { content, err := repoClient.GetFileContent(file) if err != nil { - //nolint - return err + return fmt.Errorf("error during GetFileContent: %w", err) } - continueIter, err := onFileContent(file, content, data) + continueIter, err := onFileContent(file, content, args...) if err != nil { return err } diff --git a/checks/fileparser/listing_test.go b/checks/fileparser/listing_test.go index 28867103..71205480 100644 --- a/checks/fileparser/listing_test.go +++ b/checks/fileparser/listing_test.go @@ -20,7 +20,6 @@ import ( "github.com/golang/mock/gomock" - "github.com/ossf/scorecard/v4/checker" mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" ) @@ -316,7 +315,10 @@ func Test_isMatchingPath(t *testing.T) { tt := tt // Re-initializing variable so it is not changed while executing the closure below t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := isMatchingPath(tt.args.pattern, tt.args.fullpath, tt.args.caseSensitive) + got, err := isMatchingPath(tt.args.fullpath, PathMatcher{ + Pattern: tt.args.pattern, + CaseSensitive: tt.args.caseSensitive, + }) if (err != nil) != tt.wantErr { t.Errorf("isMatchingPath() error = %v, wantErr %v", err, tt.wantErr) return @@ -385,8 +387,8 @@ func Test_isTestdataFile(t *testing.T) { } } -// TestCheckFilesContentV6 tests the CheckFilesContentV6 function. -func TestCheckFilesContentV6(t *testing.T) { +// TestOnMatchingFileContentDo tests the OnMatchingFileContent function. +func TestOnMatchingFileContent(t *testing.T) { t.Parallel() //nolint tests := []struct { @@ -448,50 +450,6 @@ func TestCheckFilesContentV6(t *testing.T) { "Dockerfile.template.template.template.template", }, }, - } - - for _, tt := range tests { - tt := tt // Re-initializing variable so it is not changed while executing the closure below - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - x := func(path string, content []byte, data FileCbData) (bool, error) { - if tt.shouldFuncFail { - //nolint - return false, errors.New("test error") - } - if tt.shouldGetPredicateFail { - return false, nil - } - return true, nil - } - - ctrl := gomock.NewController(t) - mockRepo := mockrepo.NewMockRepoClient(ctrl) - mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).Return(nil, nil).AnyTimes() - - result := CheckFilesContentV6(tt.shellPattern, tt.caseSensitive, mockRepo, x, x) - - if tt.wantErr && result == nil { - t.Errorf("CheckFilesContentV6() = %v, want %v test name %v", result, tt.wantErr, tt.name) - } - }) - } -} - -// TestCheckFilesContent tests the CheckFilesContent function. -func TestCheckFilesContent(t *testing.T) { - t.Parallel() - //nolint - tests := []struct { - name string - wantErr bool - shellPattern string - caseSensitive bool - shouldFuncFail bool - shouldGetPredicateFail bool - files []string - }{ { name: "no files", shellPattern: "Dockerfile", @@ -548,8 +506,7 @@ func TestCheckFilesContent(t *testing.T) { tt := tt // Re-initializing variable so it is not changed while executing the closure below t.Run(tt.name, func(t *testing.T) { t.Parallel() - x := func(path string, content []byte, - dl checker.DetailLogger, data FileCbData) (bool, error) { + x := func(path string, content []byte, args ...interface{}) (bool, error) { if tt.shouldFuncFail { //nolint return false, errors.New("test error") @@ -565,14 +522,13 @@ func TestCheckFilesContent(t *testing.T) { mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() mockRepo.EXPECT().GetFileContent(gomock.Any()).Return(nil, nil).AnyTimes() - c := checker.CheckRequest{ - RepoClient: mockRepo, - } - - result := CheckFilesContent(tt.shellPattern, tt.caseSensitive, &c, x, x) + result := OnMatchingFileContentDo(mockRepo, PathMatcher{ + Pattern: tt.shellPattern, + CaseSensitive: tt.caseSensitive, + }, x) if tt.wantErr && result == nil { - t.Errorf("CheckFilesContentV6() = %v, want %v test name %v", result, tt.wantErr, tt.name) + t.Errorf("OnMatchingFileContentDo() = %v, want %v test name %v", result, tt.wantErr, tt.name) } }) } diff --git a/checks/fuzzing.go b/checks/fuzzing.go index ac9c2b7a..2387a40e 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -36,11 +36,13 @@ func init() { func checkCFLite(c *checker.CheckRequest) (bool, error) { result := false - e := fileparser.CheckFilesContent(".clusterfuzzlite/Dockerfile", true, c, - func(path string, content []byte, dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) { - result = fileparser.CheckFileContainsCommands(content, "#") - return false, nil - }, nil) + e := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: ".clusterfuzzlite/Dockerfile", + CaseSensitive: true, + }, func(path string, content []byte, args ...interface{}) (bool, error) { + result = fileparser.CheckFileContainsCommands(content, "#") + return false, nil + }, nil) if e != nil { return result, fmt.Errorf("%w", e) } diff --git a/checks/permissions.go b/checks/permissions.go index 2bcaa944..cce18019 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -82,11 +82,68 @@ func TokenPermissions(c *checker.CheckRequest) checker.CheckResult { data := permissionCbData{ workflows: make(map[string]permissions), } - err := fileparser.CheckFilesContent(".github/workflows/*", false, - c, validateGitHubActionTokenPermissions, &data) + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: ".github/workflows/*", + CaseSensitive: false, + }, validateGitHubActionTokenPermissions, c.Dlogger, &data) return createResultForLeastPrivilegeTokens(data, err) } +// Check file content. +var validateGitHubActionTokenPermissions fileparser.DoWhileTrueOnFileContent = func(path string, + content []byte, + args ...interface{}) (bool, error) { + if !fileparser.IsWorkflowFile(path) { + return true, nil + } + // Verify the type of the data. + if len(args) != 2 { + return false, fmt.Errorf( + "validateGitHubActionTokenPermissions requires exactly 2 arguments: %w", errInvalidArgLength) + } + pdata, ok := args[1].(*permissionCbData) + if !ok { + return false, fmt.Errorf( + "validateGitHubActionTokenPermissions requires arg[0] of type *permissionCbData: %w", errInvalidArgType) + } + dl, ok := args[0].(checker.DetailLogger) + if !ok { + return false, fmt.Errorf( + "validateGitHubActionTokenPermissions requires arg[1] of type checker.DetailLogger: %w", errInvalidArgType) + } + + if !fileparser.CheckFileContainsCommands(content, "#") { + return true, nil + } + + workflow, errs := actionlint.Parse(content) + if len(errs) > 0 && workflow == nil { + return false, fileparser.FormatActionlintError(errs) + } + + // 1. Top-level permission definitions. + //nolint + // https://docs.github.com/en/actions/reference/authentication-in-a-workflow#example-1-passing-the-github_token-as-an-input, + // https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/, + // https://docs.github.com/en/actions/reference/authentication-in-a-workflow#modifying-the-permissions-for-the-github_token. + if err := validateTopLevelPermissions(workflow, path, dl, pdata); err != nil { + return false, err + } + + // 2. Run-level permission definitions, + // see https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idpermissions. + ignoredPermissions := createIgnoredPermissions(workflow, path, dl) + if err := validatejobLevelPermissions(workflow, path, dl, pdata, ignoredPermissions); err != nil { + return false, err + } + + // TODO(laurent): 2. Identify github actions that require write and add checks. + + // TODO(laurent): 3. Read a few runs and ensures they have the same permissions. + + return true, nil +} + func validatePermission(permissionKey permission, permissionValue *actionlint.PermissionScope, permLevel, path string, dl checker.DetailLogger, pPermissions map[permission]bool, ignoredPermissions map[permission]bool) error { @@ -378,51 +435,6 @@ func createResultForLeastPrivilegeTokens(result permissionCbData, err error) che "tokens are read-only in GitHub workflows") } -// Check file content. -func validateGitHubActionTokenPermissions(path string, content []byte, - dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) { - if !fileparser.IsWorkflowFile(path) { - return true, nil - } - // Verify the type of the data. - pdata, ok := data.(*permissionCbData) - if !ok { - // This never happens. - panic("invalid type") - } - - if !fileparser.CheckFileContainsCommands(content, "#") { - return true, nil - } - - workflow, errs := actionlint.Parse(content) - if len(errs) > 0 && workflow == nil { - return false, fileparser.FormatActionlintError(errs) - } - - // 1. Top-level permission definitions. - //nolint - // https://docs.github.com/en/actions/reference/authentication-in-a-workflow#example-1-passing-the-github_token-as-an-input, - // https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/, - // https://docs.github.com/en/actions/reference/authentication-in-a-workflow#modifying-the-permissions-for-the-github_token. - if err := validateTopLevelPermissions(workflow, path, dl, pdata); err != nil { - return false, err - } - - // 2. Run-level permission definitions, - // see https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idpermissions. - ignoredPermissions := createIgnoredPermissions(workflow, path, dl) - if err := validatejobLevelPermissions(workflow, path, dl, pdata, ignoredPermissions); err != nil { - return false, err - } - - // TODO(laurent): 2. Identify github actions that require write and add checks. - - // TODO(laurent): 3. Read a few runs and ensures they have the same permissions. - - return true, nil -} - func createIgnoredPermissions(workflow *actionlint.Workflow, fp string, dl checker.DetailLogger) map[permission]bool { ignoredPermissions := make(map[permission]bool) if requiresPackagesPermissions(workflow, fp, dl) { diff --git a/checks/pinned_dependencies.go b/checks/pinned_dependencies.go index be672e44..493ea6e2 100644 --- a/checks/pinned_dependencies.go +++ b/checks/pinned_dependencies.go @@ -142,7 +142,7 @@ func addPinnedResult(r *pinnedResult, to bool) { } } -func dataAsWorkflowResultPointer(data fileparser.FileCbData) *worklowPinningResult { +func dataAsWorkflowResultPointer(data interface{}) *worklowPinningResult { pdata, ok := data.(*worklowPinningResult) if !ok { // panic if it is not correct type @@ -151,6 +151,24 @@ func dataAsWorkflowResultPointer(data fileparser.FileCbData) *worklowPinningResu return pdata } +func dataAsResultPointer(data interface{}) *pinnedResult { + pdata, ok := data.(*pinnedResult) + if !ok { + // This never happens. + panic("invalid type") + } + return pdata +} + +func dataAsDetailLogger(data interface{}) checker.DetailLogger { + pdata, ok := data.(checker.DetailLogger) + if !ok { + // This never happens. + panic("invalid type") + } + return pdata +} + func createReturnValuesForGitHubActionsWorkflowPinned(r worklowPinningResult, infoMsg string, dl checker.DetailLogger, err error) (int, error) { if err != nil { @@ -180,15 +198,6 @@ func createReturnValuesForGitHubActionsWorkflowPinned(r worklowPinningResult, in return score, nil } -func dataAsResultPointer(data fileparser.FileCbData) *pinnedResult { - pdata, ok := data.(*pinnedResult) - if !ok { - // This never happens. - panic("invalid type") - } - return pdata -} - func createReturnValues(r pinnedResult, infoMsg string, dl checker.DetailLogger, err error) (int, error) { if err != nil { return checker.InconclusiveResultScore, err @@ -210,8 +219,10 @@ func createReturnValues(r pinnedResult, infoMsg string, dl checker.DetailLogger, func isShellScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) { var r pinnedResult - err := fileparser.CheckFilesContent("*", false, - c, validateShellScriptIsFreeOfInsecureDownloads, &r) + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: "*", + CaseSensitive: false, + }, validateShellScriptIsFreeOfInsecureDownloads, c.Dlogger, &r) return createReturnForIsShellScriptFreeOfInsecureDownloads(r, c.Dlogger, err) } @@ -229,9 +240,16 @@ func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string, return createReturnForIsShellScriptFreeOfInsecureDownloads(r, dl, err) } -func validateShellScriptIsFreeOfInsecureDownloads(pathfn string, content []byte, - dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) { - pdata := dataAsResultPointer(data) +var validateShellScriptIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func( + pathfn string, + content []byte, + args ...interface{}) (bool, error) { + if len(args) != 2 { + return false, fmt.Errorf( + "validateShellScriptIsFreeOfInsecureDownloads requires exactly 2 arguments: %w", errInvalidArgLength) + } + pdata := dataAsResultPointer(args[1]) + dl := dataAsDetailLogger(args[0]) // Validate the file type. if !isSupportedShellScriptFile(pathfn, content) { @@ -250,8 +268,10 @@ func validateShellScriptIsFreeOfInsecureDownloads(pathfn string, content []byte, func isDockerfileFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) { var r pinnedResult - err := fileparser.CheckFilesContent("*Dockerfile*", - false, c, validateDockerfileIsFreeOfInsecureDownloads, &r) + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: "*Dockerfile*", + CaseSensitive: false, + }, validateDockerfileIsFreeOfInsecureDownloads, c.Dlogger, &r) return createReturnForIsDockerfileFreeOfInsecureDownloads(r, c.Dlogger, err) } @@ -285,9 +305,16 @@ func isDockerfile(pathfn string, content []byte) bool { return true } -func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte, - dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) { - pdata := dataAsResultPointer(data) +var validateDockerfileIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func( + pathfn string, + content []byte, + args ...interface{}) (bool, error) { + if len(args) != 2 { + return false, fmt.Errorf( + "validateDockerfileIsFreeOfInsecureDownloads requires exactly 2 arguments: %w", errInvalidArgLength) + } + pdata := dataAsResultPointer(args[1]) + dl := dataAsDetailLogger(args[0]) // Return early if this is not a docker file. if !isDockerfile(pathfn, content) { @@ -344,8 +371,10 @@ func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte, func isDockerfilePinned(c *checker.CheckRequest) (int, error) { var r pinnedResult - err := fileparser.CheckFilesContent("*Dockerfile*", false, - c, validateDockerfileIsPinned, &r) + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: "*Dockerfile*", + CaseSensitive: false, + }, validateDockerfileIsPinned, c.Dlogger, &r) return createReturnForIsDockerfilePinned(r, c.Dlogger, err) } @@ -362,12 +391,19 @@ func testValidateDockerfileIsPinned(pathfn string, content []byte, dl checker.De return createReturnForIsDockerfilePinned(r, dl, err) } -func validateDockerfileIsPinned(pathfn string, content []byte, - dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) { +var validateDockerfileIsPinned fileparser.DoWhileTrueOnFileContent = func( + pathfn string, + content []byte, + args ...interface{}) (bool, error) { // Users may use various names, e.g., // Dockerfile.aarch64, Dockerfile.template, Dockerfile_template, dockerfile, Dockerfile-name.template - pdata := dataAsResultPointer(data) + if len(args) != 2 { + return false, fmt.Errorf( + "validateDockerfileIsPinned requires exactly 2 arguments: %w", errInvalidArgLength) + } + pdata := dataAsResultPointer(args[1]) + dl := dataAsDetailLogger(args[0]) // Return early if this is not a dockerfile. if !isDockerfile(pathfn, content) { addPinnedResult(pdata, true) @@ -472,8 +508,10 @@ func validateDockerfileIsPinned(pathfn string, content []byte, func isGitHubWorkflowScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) { var r pinnedResult - err := fileparser.CheckFilesContent(".github/workflows/*", false, - c, validateGitHubWorkflowIsFreeOfInsecureDownloads, &r) + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: ".github/workflows/*", + CaseSensitive: false, + }, validateGitHubWorkflowIsFreeOfInsecureDownloads, c.Dlogger, &r) return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, c.Dlogger, err) } @@ -494,14 +532,20 @@ func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string, // validateGitHubWorkflowIsFreeOfInsecureDownloads checks if the workflow file downloads dependencies that are unpinned. // Returns true if the check should continue executing after this file. -// nolint: gocognit -func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []byte, - dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) { +var validateGitHubWorkflowIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func( + pathfn string, + content []byte, + args ...interface{}) (bool, error) { if !fileparser.IsWorkflowFile(pathfn) { return true, nil } - pdata := dataAsResultPointer(data) + if len(args) != 2 { + return false, fmt.Errorf( + "validateGitHubWorkflowIsFreeOfInsecureDownloads requires exactly 2 arguments: %w", errInvalidArgLength) + } + pdata := dataAsResultPointer(args[1]) + dl := dataAsDetailLogger(args[0]) if !fileparser.CheckFileContainsCommands(content, "#") { addPinnedResult(pdata, true) @@ -569,8 +613,10 @@ func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []by // Check pinning of github actions in workflows. func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) (int, error) { var r worklowPinningResult - err := fileparser.CheckFilesContent(".github/workflows/*", - true, c, validateGitHubActionWorkflow, &r) + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: ".github/workflows/*", + CaseSensitive: true, + }, validateGitHubActionWorkflow, c.Dlogger, &r) return createReturnForIsGitHubActionsWorkflowPinned(r, c.Dlogger, err) } @@ -597,13 +643,20 @@ func generateOwnerToDisplay(gitHubOwned bool) string { // validateGitHubActionWorkflow checks if the workflow file contains unpinned actions. Returns true if the check // should continue executing after this file. -func validateGitHubActionWorkflow(pathfn string, content []byte, - dl checker.DetailLogger, data fileparser.FileCbData) (bool, error) { +var validateGitHubActionWorkflow fileparser.DoWhileTrueOnFileContent = func( + pathfn string, + content []byte, + args ...interface{}) (bool, error) { if !fileparser.IsWorkflowFile(pathfn) { return true, nil } - pdata := dataAsWorkflowResultPointer(data) + if len(args) != 2 { + return false, fmt.Errorf( + "validateGitHubActionWorkflow requires exactly 2 arguments: %w", errInvalidArgLength) + } + pdata := dataAsWorkflowResultPointer(args[1]) + dl := dataAsDetailLogger(args[0]) if !fileparser.CheckFileContainsCommands(content, "#") { addWorkflowPinnedResult(pdata, true, true) diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index f64354c8..2fa1881b 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -31,7 +31,10 @@ import ( // BinaryArtifacts retrieves the raw data for the Binary-Artifacts check. func BinaryArtifacts(c clients.RepoClient) (checker.BinaryArtifactData, error) { files := []checker.File{} - err := fileparser.CheckFilesContentV6("*", false, c, checkBinaryFileContent, &files) + err := fileparser.OnMatchingFileContentDo(c, fileparser.PathMatcher{ + Pattern: "*", + CaseSensitive: false, + }, checkBinaryFileContent, &files) if err != nil { return checker.BinaryArtifactData{}, fmt.Errorf("%w", err) } @@ -40,12 +43,16 @@ func BinaryArtifacts(c clients.RepoClient) (checker.BinaryArtifactData, error) { return checker.BinaryArtifactData{Files: files}, nil } -func checkBinaryFileContent(path string, content []byte, - data fileparser.FileCbData) (bool, error) { - pfiles, ok := data.(*[]checker.File) +var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path string, content []byte, + args ...interface{}) (bool, error) { + if len(args) != 1 { + return false, fmt.Errorf( + "checkBinaryFileContent requires exactly one argument: %w", errInvalidArgLength) + } + pfiles, ok := args[0].(*[]checker.File) if !ok { - // This never happens. - panic("invalid type") + return false, fmt.Errorf( + "checkBinaryFileContent requires argument of type *[]checker.File: %w", errInvalidArgType) } binaryFileTypes := map[string]bool{