mirror of
https://github.com/ossf/scorecard.git
synced 2024-09-11 00:45:36 +03:00
Update ListFiles API to return error (#746)
Co-authored-by: Azeem Shaikh <azeems@google.com>
This commit is contained in:
parent
7c133bc767
commit
9bf1cdc9ce
@ -29,7 +29,7 @@ func init() {
|
||||
|
||||
// AutomaticDependencyUpdate will check the repository if it contains Automatic dependency update.
|
||||
func AutomaticDependencyUpdate(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckIfFileExists2(CheckAutomaticDependencyUpdate, c, fileExists)
|
||||
r, err := CheckIfFileExists(CheckAutomaticDependencyUpdate, c, fileExists)
|
||||
if err != nil {
|
||||
return checker.CreateRuntimeErrorResult(CheckAutomaticDependencyUpdate, err)
|
||||
}
|
||||
|
@ -34,7 +34,7 @@ func init() {
|
||||
|
||||
// BinaryArtifacts will check the repository if it contains binary artifacts.
|
||||
func BinaryArtifacts(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckFilesContent2("*", false, c, checkBinaryFileContent)
|
||||
r, err := CheckFilesContent("*", false, c, checkBinaryFileContent)
|
||||
if err != nil {
|
||||
return checker.CreateRuntimeErrorResult(CheckBinaryArtifacts, err)
|
||||
}
|
||||
|
@ -62,69 +62,31 @@ func isScorecardTestFile(owner, repo, fullpath string) bool {
|
||||
// use "./*".
|
||||
// - A pattern such as "*mypatern*" will match files containing mypattern in *any* directory.
|
||||
//nolint
|
||||
func CheckFilesContent(checkName, shellPathFnPattern string,
|
||||
caseSensitive bool,
|
||||
c *checker.CheckRequest,
|
||||
onFileContent func(path string, content []byte,
|
||||
Logf func(s string, f ...interface{})) (bool, error),
|
||||
) checker.CheckResult {
|
||||
predicate := func(filepath string) bool {
|
||||
// Filter out Scorecard's own test files.
|
||||
if isScorecardTestFile(c.Owner, c.Repo, filepath) {
|
||||
return false
|
||||
}
|
||||
// Filter out files based on path/names using the pattern.
|
||||
b, err := isMatchingPath(shellPathFnPattern, filepath, caseSensitive)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return b
|
||||
}
|
||||
res := true
|
||||
for _, file := range c.RepoClient.ListFiles(predicate) {
|
||||
content, err := c.RepoClient.GetFileContent(file)
|
||||
if err != nil {
|
||||
return checker.MakeRetryResult(checkName, err)
|
||||
}
|
||||
|
||||
rr, err := onFileContent(file, content, c.Logf)
|
||||
if err != nil {
|
||||
return checker.MakeFailResult(checkName, err)
|
||||
}
|
||||
// We don't return rightway to let the onFileContent()
|
||||
// handler log.
|
||||
if !rr {
|
||||
res = false
|
||||
}
|
||||
}
|
||||
if res {
|
||||
return checker.MakePassResult(checkName)
|
||||
}
|
||||
|
||||
return checker.MakeFailResult(checkName, nil)
|
||||
}
|
||||
|
||||
// UPGRADEv2: to rename to CheckFilesContent.
|
||||
func CheckFilesContent2(shellPathFnPattern string,
|
||||
func CheckFilesContent(shellPathFnPattern string,
|
||||
caseSensitive bool,
|
||||
c *checker.CheckRequest,
|
||||
onFileContent func(path string, content []byte,
|
||||
dl checker.DetailLogger) (bool, error),
|
||||
) (bool, error) {
|
||||
predicate := func(filepath string) bool {
|
||||
predicate := func(filepath string) (bool, error) {
|
||||
// Filter out Scorecard's own test files.
|
||||
if isScorecardTestFile(c.Owner, c.Repo, filepath) {
|
||||
return false
|
||||
return false, nil
|
||||
}
|
||||
// Filter out files based on path/names using the pattern.
|
||||
b, err := isMatchingPath(shellPathFnPattern, filepath, caseSensitive)
|
||||
if err != nil {
|
||||
return false
|
||||
return false, err
|
||||
}
|
||||
return b
|
||||
return b, nil
|
||||
}
|
||||
res := true
|
||||
for _, file := range c.RepoClient.ListFiles(predicate) {
|
||||
matchedFiles, err := c.RepoClient.ListFiles(predicate)
|
||||
if err != nil {
|
||||
// nolint: wrapcheck
|
||||
return false, err
|
||||
}
|
||||
for _, file := range matchedFiles {
|
||||
content, err := c.RepoClient.GetFileContent(file)
|
||||
if err != nil {
|
||||
//nolint
|
||||
|
@ -21,33 +21,13 @@ import (
|
||||
// CheckIfFileExists downloads the tar of the repository and calls the onFile() to check
|
||||
// for the occurrence.
|
||||
func CheckIfFileExists(checkName string, c *checker.CheckRequest, onFile func(name string,
|
||||
Logf func(s string, f ...interface{})) (bool, error)) checker.CheckResult {
|
||||
for _, filename := range c.RepoClient.ListFiles(func(string) bool { return true }) {
|
||||
rr, err := onFile(filename, c.Logf)
|
||||
if err != nil {
|
||||
return checker.CheckResult{
|
||||
Name: checkName,
|
||||
Pass: false,
|
||||
Confidence: checker.MaxResultConfidence,
|
||||
Error: err,
|
||||
}
|
||||
}
|
||||
|
||||
if rr {
|
||||
return checker.MakePassResult(checkName)
|
||||
}
|
||||
}
|
||||
const confidence = 5
|
||||
return checker.CheckResult{
|
||||
Name: checkName,
|
||||
Pass: false,
|
||||
Confidence: confidence,
|
||||
}
|
||||
}
|
||||
|
||||
func CheckIfFileExists2(checkName string, c *checker.CheckRequest, onFile func(name string,
|
||||
dl checker.DetailLogger) (bool, error)) (bool, error) {
|
||||
for _, filename := range c.RepoClient.ListFiles(func(string) bool { return true }) {
|
||||
matchedFiles, err := c.RepoClient.ListFiles(func(string) (bool, error) { return true, nil })
|
||||
if err != nil {
|
||||
// nolint: wrapcheck
|
||||
return false, err
|
||||
}
|
||||
for _, filename := range matchedFiles {
|
||||
rr, err := onFile(filename, c.Dlogger)
|
||||
if err != nil {
|
||||
return false, err
|
||||
|
@ -72,7 +72,7 @@ func FrozenDeps(c *checker.CheckRequest) checker.CheckResult {
|
||||
// TODO(laurent): need to support GCB pinning.
|
||||
|
||||
func isShellScriptFreeOfInsecureDownloads(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckFilesContent2("*", false, c, validateShellScriptIsFreeOfInsecureDownloads)
|
||||
r, err := CheckFilesContent("*", false, c, validateShellScriptIsFreeOfInsecureDownloads)
|
||||
return createResultForIsShellScriptFreeOfInsecureDownloads(r, err)
|
||||
}
|
||||
|
||||
@ -105,7 +105,7 @@ func validateShellScriptIsFreeOfInsecureDownloads(pathfn string, content []byte,
|
||||
}
|
||||
|
||||
func isDockerfileFreeOfInsecureDownloads(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckFilesContent2("*Dockerfile*", false, c, validateDockerfileIsFreeOfInsecureDownloads)
|
||||
r, err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsFreeOfInsecureDownloads)
|
||||
return createResultForIsDockerfileFreeOfInsecureDownloads(r, err)
|
||||
}
|
||||
|
||||
@ -168,7 +168,7 @@ func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte,
|
||||
}
|
||||
|
||||
func isDockerfilePinned(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckFilesContent2("*Dockerfile*", false, c, validateDockerfileIsPinned)
|
||||
r, err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsPinned)
|
||||
return createResultForIsDockerfilePinned(r, err)
|
||||
}
|
||||
|
||||
@ -271,7 +271,7 @@ func validateDockerfileIsPinned(pathfn string, content []byte,
|
||||
}
|
||||
|
||||
func isGitHubWorkflowScriptFreeOfInsecureDownloads(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckFilesContent2(".github/workflows/*", false, c, validateGitHubWorkflowIsFreeOfInsecureDownloads)
|
||||
r, err := CheckFilesContent(".github/workflows/*", false, c, validateGitHubWorkflowIsFreeOfInsecureDownloads)
|
||||
return createResultForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, err)
|
||||
}
|
||||
|
||||
@ -355,7 +355,7 @@ func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []by
|
||||
|
||||
// Check pinning of github actions in workflows.
|
||||
func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckFilesContent2(".github/workflows/*", true, c, validateGitHubActionWorkflow)
|
||||
r, err := CheckFilesContent(".github/workflows/*", true, c, validateGitHubActionWorkflow)
|
||||
return createResultForIsGitHubActionsWorkflowPinned(r, err)
|
||||
}
|
||||
|
||||
@ -415,7 +415,7 @@ func validateGitHubActionWorkflow(pathfn string, content []byte, dl checker.Deta
|
||||
|
||||
// Check presence of lock files thru validatePackageManagerFile().
|
||||
func isPackageManagerLockFilePresent(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckIfFileExists2(CheckFrozenDeps, c, validatePackageManagerFile)
|
||||
r, err := CheckIfFileExists(CheckFrozenDeps, c, validatePackageManagerFile)
|
||||
if err != nil {
|
||||
return checker.CreateRuntimeErrorResult(CheckFrozenDeps, err)
|
||||
}
|
||||
|
@ -32,7 +32,7 @@ func init() {
|
||||
}
|
||||
|
||||
func leastPrivilegedTokens(c *checker.CheckRequest) checker.CheckResult {
|
||||
r, err := CheckFilesContent2(".github/workflows/*", false, c, validateGitHubActionTokenPermissions)
|
||||
r, err := CheckFilesContent(".github/workflows/*", false, c, validateGitHubActionTokenPermissions)
|
||||
return createResultForLeastPrivilegeTokens(r, err)
|
||||
}
|
||||
|
||||
|
@ -41,7 +41,7 @@ func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult {
|
||||
}
|
||||
return false, nil
|
||||
}
|
||||
r, err := CheckIfFileExists2(CheckSecurityPolicy, c, onFile)
|
||||
r, err := CheckIfFileExists(CheckSecurityPolicy, c, onFile)
|
||||
if err != nil {
|
||||
return checker.CreateRuntimeErrorResult(CheckSecurityPolicy, err)
|
||||
}
|
||||
@ -67,7 +67,7 @@ func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult {
|
||||
}
|
||||
return false, nil
|
||||
}
|
||||
r, err = CheckIfFileExists2(CheckSecurityPolicy, dotGitHub, onFile)
|
||||
r, err = CheckIfFileExists(CheckSecurityPolicy, dotGitHub, onFile)
|
||||
if err != nil {
|
||||
return checker.CreateRuntimeErrorResult(CheckSecurityPolicy, err)
|
||||
}
|
||||
|
@ -54,7 +54,7 @@ func (client *Client) InitRepo(owner, repoName string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (client *Client) ListFiles(predicate func(string) bool) []string {
|
||||
func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, error) {
|
||||
return client.tarball.listFiles(predicate)
|
||||
}
|
||||
|
||||
|
@ -201,14 +201,18 @@ func (handler *tarballHandler) extractTarball() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (handler *tarballHandler) listFiles(predicate func(string) bool) []string {
|
||||
func (handler *tarballHandler) listFiles(predicate func(string) (bool, error)) ([]string, error) {
|
||||
ret := make([]string, 0)
|
||||
for _, file := range handler.files {
|
||||
if predicate(file) {
|
||||
matches, err := predicate(file)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if matches {
|
||||
ret = append(ret, file)
|
||||
}
|
||||
}
|
||||
return ret
|
||||
return ret, nil
|
||||
}
|
||||
|
||||
func (handler *tarballHandler) getFileContent(filename string) ([]byte, error) {
|
||||
|
@ -28,7 +28,8 @@ import (
|
||||
)
|
||||
|
||||
type listfileTest struct {
|
||||
predicate func(string) bool
|
||||
predicate func(string) (bool, error)
|
||||
err error
|
||||
outcome []string
|
||||
}
|
||||
|
||||
@ -79,17 +80,17 @@ func TestExtractTarball(t *testing.T) {
|
||||
listfileTests: []listfileTest{
|
||||
{
|
||||
// Returns all files in the tarball.
|
||||
predicate: func(string) bool { return true },
|
||||
predicate: func(string) (bool, error) { return true, nil },
|
||||
outcome: []string{"file0", "dir1/file1", "dir1/dir2/file2"},
|
||||
},
|
||||
{
|
||||
// Skips all files inside `dir1/dir2` directory.
|
||||
predicate: func(fn string) bool { return !strings.HasPrefix(fn, "dir1/dir2") },
|
||||
predicate: func(fn string) (bool, error) { return !strings.HasPrefix(fn, "dir1/dir2"), nil },
|
||||
outcome: []string{"file0", "dir1/file1"},
|
||||
},
|
||||
{
|
||||
// Skips all files.
|
||||
predicate: func(fn string) bool { return false },
|
||||
predicate: func(fn string) (bool, error) { return false, nil },
|
||||
outcome: []string{},
|
||||
},
|
||||
},
|
||||
@ -132,10 +133,15 @@ func TestExtractTarball(t *testing.T) {
|
||||
|
||||
// Test ListFiles API.
|
||||
for _, listfiletest := range testcase.listfileTests {
|
||||
matchedFiles, err := handler.listFiles(listfiletest.predicate)
|
||||
if !errors.Is(err, listfiletest.err) {
|
||||
t.Errorf("test failed: expected - %v, got - %v", listfiletest.err, err)
|
||||
continue
|
||||
}
|
||||
if !cmp.Equal(listfiletest.outcome,
|
||||
handler.listFiles(listfiletest.predicate),
|
||||
matchedFiles,
|
||||
cmpopts.SortSlices(isSortedString)) {
|
||||
t.Errorf("test failed: expected - %q, got - %q", listfiletest.outcome, handler.listFiles(listfiletest.predicate))
|
||||
t.Errorf("test failed: expected - %q, got - %q", listfiletest.outcome, matchedFiles)
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -40,7 +40,7 @@ func NewRepoUnavailableError(err error) error {
|
||||
|
||||
type RepoClient interface {
|
||||
InitRepo(owner, repo string) error
|
||||
ListFiles(predicate func(string) bool) []string
|
||||
ListFiles(predicate func(string) (bool, error)) ([]string, error)
|
||||
GetFileContent(filename string) ([]byte, error)
|
||||
ListMergedPRs() ([]PullRequest, error)
|
||||
GetDefaultBranch() (BranchRef, error)
|
||||
|
Loading…
Reference in New Issue
Block a user