change signature of FileIfExist and FileContent (#787)

* draft

* add pinning

* remove functions

* typo

* commment

* name
This commit is contained in:
laurentsimon 2021-07-30 08:09:52 -07:00 committed by GitHub
parent b35cbdcdcf
commit 29594d4294
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 163 additions and 139 deletions

View File

@ -29,30 +29,38 @@ func init() {
// AutomaticDependencyUpdate will check the repository if it contains Automatic dependency update.
func AutomaticDependencyUpdate(c *checker.CheckRequest) checker.CheckResult {
r, err := CheckIfFileExists(CheckAutomaticDependencyUpdate, c, fileExists)
var r bool
err := CheckIfFileExists(CheckAutomaticDependencyUpdate, c, fileExists, &r)
if err != nil {
return checker.CreateRuntimeErrorResult(CheckAutomaticDependencyUpdate, err)
}
if !r {
return checker.CreateMinScoreResult(CheckAutomaticDependencyUpdate, "no tool detected [dependabot|renovabot]")
c.Dlogger.Warn("dependabot not detected")
c.Dlogger.Warn("renovatebot not detected")
return checker.CreateMinScoreResult(CheckAutomaticDependencyUpdate, "no update tool detected")
}
// High score result.
return checker.CreateMaxScoreResult(CheckAutomaticDependencyUpdate, "tool detected")
return checker.CreateMaxScoreResult(CheckAutomaticDependencyUpdate, "update tool detected")
}
// fileExists will validate the if frozen dependencies file name exists.
func fileExists(name string, dl checker.DetailLogger) (bool, error) {
func fileExists(name string, dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := FileGetCbDataAsBoolPointer(data)
switch strings.ToLower(name) {
case ".github/dependabot.yml":
dl.Info("dependabot detected : %s", name)
return true, nil
// https://docs.renovatebot.com/configuration-options/
case ".github/renovate.json", ".github/renovate.json5", ".renovaterc.json", "renovate.json",
"renovate.json5", ".renovaterc":
dl.Info("renovate detected: %s", name)
return true, nil
default:
return false, nil
// Continue iterating.
return true, nil
}
*pdata = true
// We found the file, no need to continue iterating.
return false, nil
}

View File

@ -17,6 +17,7 @@ package checks
import (
"fmt"
"path/filepath"
"strings"
"github.com/h2non/filetype"
"github.com/h2non/filetype/types"
@ -34,11 +35,12 @@ func init() {
// BinaryArtifacts will check the repository if it contains binary artifacts.
func BinaryArtifacts(c *checker.CheckRequest) checker.CheckResult {
r, err := CheckFilesContent("*", false, c, checkBinaryFileContent)
var binFound bool
err := CheckFilesContent("*", false, c, checkBinaryFileContent, &binFound)
if err != nil {
return checker.CreateRuntimeErrorResult(CheckBinaryArtifacts, err)
}
if !r {
if binFound {
return checker.CreateMinScoreResult(CheckBinaryArtifacts, "binaries present in source code")
}
@ -46,7 +48,8 @@ func BinaryArtifacts(c *checker.CheckRequest) checker.CheckResult {
}
func checkBinaryFileContent(path string, content []byte,
dl checker.DetailLogger) (bool, error) {
dl checker.DetailLogger, data FileCbData) (bool, error) {
pfound := FileGetCbDataAsBoolPointer(data)
binaryFileTypes := map[string]bool{
"crx": true,
"deb": true,
@ -92,12 +95,14 @@ func checkBinaryFileContent(path string, content []byte,
}
if _, ok := binaryFileTypes[t.Extension]; ok {
dl.Warn("binary found: %s", path)
return false, nil
} else if _, ok := binaryFileTypes[filepath.Ext(path)]; ok {
dl.Warn("binary detected: %s", path)
*pfound = true
return true, nil
} else if _, ok := binaryFileTypes[strings.ReplaceAll(filepath.Ext(path), ".", "")]; ok {
// Falling back to file based extension.
dl.Warn("binary found: %s", path)
return false, nil
dl.Warn("binary detected: %s", path)
*pfound = true
return true, nil
}
return true, nil

View File

@ -24,29 +24,6 @@ import (
sce "github.com/ossf/scorecard/v2/errors"
)
// 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,
dl checker.DetailLogger) (bool, error)) (bool, error) {
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
}
if rr {
return true, nil
}
}
return false, nil
}
// 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) {
@ -79,58 +56,6 @@ func isScorecardTestFile(owner, repo, fullpath string) bool {
strings.Contains(fullpath, "/testdata/"))
}
// 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.
//nolint
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, error) {
// Filter out Scorecard's own test files.
if isScorecardTestFile(c.Owner, c.Repo, 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
}
res := true
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
return false, err
}
rr, err := onFileContent(file, content, c.Dlogger)
if err != nil {
return false, err
}
// We don't return rightway to let the onFileContent()
// handler log.
if !rr {
res = false
}
}
return res, nil
}
// FileCbData is any data the caller can act upon
// to keep state.
type FileCbData interface{}
@ -141,7 +66,13 @@ type FileCbData interface{}
type FileContentCb func(path string, content []byte,
dl checker.DetailLogger, data FileCbData) (bool, error)
func CheckFilesContent2(shellPathFnPattern string,
// 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,
@ -186,6 +117,40 @@ func CheckFilesContent2(shellPathFnPattern string,
return nil
}
type FileCb func(path string,
dl checker.DetailLogger, data FileCbData) (bool, error)
// CheckIfFileExists downloads the tar of the repository and calls the onFile() to check
// for the occurrence.
func CheckIfFileExists(checkName string, c *checker.CheckRequest, onFile FileCb, data FileCbData) error {
matchedFiles, err := c.RepoClient.ListFiles(func(string) (bool, error) { return true, nil })
if err != nil {
// nolint: wrapcheck
return err
}
for _, filename := range matchedFiles {
continueIter, err := onFile(filename, c.Dlogger, data)
if err != nil {
return err
}
if !continueIter {
break
}
}
return nil
}
func FileGetCbDataAsBoolPointer(data FileCbData) *bool {
pdata, ok := data.(*bool)
if !ok {
// This never happens.
panic("invalid type")
}
return pdata
}
// CheckFileContainsCommands checks if the file content contains commands or not.
// `comment` is the string or character that indicates a comment:
// for example for Dockerfiles, it would be `#`.

View File

@ -40,7 +40,7 @@ type permissionCbData struct {
func TokenPermissions(c *checker.CheckRequest) checker.CheckResult {
data := permissionCbData{writePermissions: make(map[string]bool)}
err := CheckFilesContent2(".github/workflows/*", false,
err := CheckFilesContent(".github/workflows/*", false,
c, validateGitHubActionTokenPermissions, &data)
return createResultForLeastPrivilegeTokens(data, err)
}

View File

@ -134,7 +134,8 @@ func createReturnValues(r bool, infoMsg string, dl checker.DetailLogger, err err
}
func isShellScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) {
r, err := CheckFilesContent("*", false, c, validateShellScriptIsFreeOfInsecureDownloads)
var r bool
err := CheckFilesContent("*", false, c, validateShellScriptIsFreeOfInsecureDownloads, &r)
return createReturnForIsShellScriptFreeOfInsecureDownloads(r, c.Dlogger, err)
}
@ -146,21 +147,32 @@ func createReturnForIsShellScriptFreeOfInsecureDownloads(r bool, dl checker.Deta
func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) (int, error) {
r, err := validateShellScriptIsFreeOfInsecureDownloads(pathfn, content, dl)
var r bool
_, err := validateShellScriptIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsShellScriptFreeOfInsecureDownloads(r, dl, err)
}
func validateShellScriptIsFreeOfInsecureDownloads(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := FileGetCbDataAsBoolPointer(data)
// Validate the file type.
if !isShellScriptFile(pathfn, content) {
*pdata = true
return true, nil
}
return validateShellFile(pathfn, content, dl)
r, err := validateShellFile(pathfn, content, dl)
if err != nil {
return false, err
}
*pdata = r
return true, nil
}
func isDockerfileFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) {
r, err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsFreeOfInsecureDownloads)
var r bool
err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsFreeOfInsecureDownloads, &r)
return createReturnForIsDockerfileFreeOfInsecureDownloads(r, c.Dlogger, err)
}
@ -173,18 +185,23 @@ func createReturnForIsDockerfileFreeOfInsecureDownloads(r bool, dl checker.Detai
func testValidateDockerfileIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) (int, error) {
r, err := validateDockerfileIsFreeOfInsecureDownloads(pathfn, content, dl)
var r bool
_, err := validateDockerfileIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsDockerfileFreeOfInsecureDownloads(r, dl, err)
}
func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := FileGetCbDataAsBoolPointer(data)
// Return early if this is a script, e.g. script_dockerfile_something.sh
if isShellScriptFile(pathfn, content) {
*pdata = true
return true, nil
}
if !CheckFileContainsCommands(content, "#") {
*pdata = true
return true, nil
}
@ -221,11 +238,19 @@ func validateDockerfileIsFreeOfInsecureDownloads(pathfn string, content []byte,
bytes = append(bytes, cmd...)
bytes = append(bytes, '\n')
}
return validateShellFile(pathfn, bytes, dl)
r, err := validateShellFile(pathfn, bytes, dl)
if err != nil {
return false, err
}
*pdata = r
return true, nil
}
func isDockerfilePinned(c *checker.CheckRequest) (int, error) {
r, err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsPinned)
var r bool
err := CheckFilesContent("*Dockerfile*", false, c, validateDockerfileIsPinned, &r)
return createReturnForIsDockerfilePinned(r, c.Dlogger, err)
}
@ -237,22 +262,26 @@ func createReturnForIsDockerfilePinned(r bool, dl checker.DetailLogger, err erro
}
func testValidateDockerfileIsPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
r, err := validateDockerfileIsPinned(pathfn, content, dl)
var r bool
_, err := validateDockerfileIsPinned(pathfn, content, dl, &r)
return createReturnForIsDockerfilePinned(r, dl, err)
}
func validateDockerfileIsPinned(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
dl checker.DetailLogger, data FileCbData) (bool, error) {
// Users may use various names, e.g.,
// Dockerfile.aarch64, Dockerfile.template, Dockerfile_template, dockerfile, Dockerfile-name.template
// Templates may trigger false positives, e.g. FROM { NAME }.
pdata := FileGetCbDataAsBoolPointer(data)
// Return early if this is a script, e.g. script_dockerfile_something.sh
if isShellScriptFile(pathfn, content) {
*pdata = true
return true, nil
}
if !CheckFileContainsCommands(content, "#") {
*pdata = true
return true, nil
}
@ -322,11 +351,13 @@ func validateDockerfileIsPinned(pathfn string, content []byte,
// The file need not have a FROM statement,
// https://github.com/tensorflow/tensorflow/blob/master/tensorflow/tools/dockerfiles/partials/jupyter.partial.Dockerfile.
return ret, nil
*pdata = ret
return true, nil
}
func isGitHubWorkflowScriptFreeOfInsecureDownloads(c *checker.CheckRequest) (int, error) {
r, err := CheckFilesContent(".github/workflows/*", false, c, validateGitHubWorkflowIsFreeOfInsecureDownloads)
var r bool
err := CheckFilesContent(".github/workflows/*", false, c, validateGitHubWorkflowIsFreeOfInsecureDownloads, &r)
return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, c.Dlogger, err)
}
@ -340,13 +371,17 @@ func createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r bool,
func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger) (int, error) {
r, err := validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn, content, dl)
var r bool
_, err := validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, dl, err)
}
func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []byte,
dl checker.DetailLogger) (bool, error) {
dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := FileGetCbDataAsBoolPointer(data)
if !CheckFileContainsCommands(content, "#") {
*pdata = true
return true, nil
}
@ -398,12 +433,14 @@ func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []by
}
}
return validated, nil
*pdata = validated
return true, nil
}
// Check pinning of github actions in workflows.
func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) (int, error) {
r, err := CheckFilesContent(".github/workflows/*", true, c, validateGitHubActionWorkflow)
var r bool
err := CheckFilesContent(".github/workflows/*", true, c, validateGitHubActionWorkflow, &r)
return createReturnForIsGitHubActionsWorkflowPinned(r, c.Dlogger, err)
}
@ -415,13 +452,18 @@ func createReturnForIsGitHubActionsWorkflowPinned(r bool, dl checker.DetailLogge
}
func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
r, err := validateGitHubActionWorkflow(pathfn, content, dl)
var r bool
_, err := validateGitHubActionWorkflow(pathfn, content, dl, &r)
return createReturnForIsGitHubActionsWorkflowPinned(r, dl, err)
}
// Check file content.
func validateGitHubActionWorkflow(pathfn string, content []byte, dl checker.DetailLogger) (bool, error) {
func validateGitHubActionWorkflow(pathfn string, content []byte,
dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := FileGetCbDataAsBoolPointer(data)
if !CheckFileContainsCommands(content, "#") {
*pdata = true
return true, nil
}
@ -452,12 +494,14 @@ func validateGitHubActionWorkflow(pathfn string, content []byte, dl checker.Deta
}
}
return ret, nil
*pdata = ret
return true, nil
}
// Check presence of lock files thru validatePackageManagerFile().
func isPackageManagerLockFilePresent(c *checker.CheckRequest) (int, error) {
r, err := CheckIfFileExists(CheckPinnedDependencies, c, validatePackageManagerFile)
var r bool
err := CheckIfFileExists(CheckPinnedDependencies, c, validatePackageManagerFile, &r)
if err != nil {
return checker.InconclusiveResultScore, err
}
@ -472,37 +516,32 @@ func isPackageManagerLockFilePresent(c *checker.CheckRequest) (int, error) {
// validatePackageManagerFile will validate the if frozen dependecies file name exists.
// TODO(laurent): need to differentiate between libraries and programs.
// TODO(laurent): handle multi-language repos.
func validatePackageManagerFile(name string, dl checker.DetailLogger) (bool, error) {
func validatePackageManagerFile(name string, dl checker.DetailLogger, data FileCbData) (bool, error) {
switch strings.ToLower(name) {
// TODO(laurent): "go.mod" is for libraries
default:
return true, nil
case "go.sum":
dl.Info("go lock file detected: %s", name)
return true, nil
case "vendor/", "third_party/", "third-party/":
dl.Info("vendoring detected in: %s", name)
return true, nil
case "package-lock.json", "npm-shrinkwrap.json":
dl.Info("javascript lock file detected: %s", name)
return true, nil
// TODO(laurent): add check for hashbased pinning in requirements.txt - https://davidwalsh.name/hashin
// Note: because requirements.txt does not handle transitive dependencies, we consider it
// not a lock file, until we have remediation steps for pip-build.
case "pipfile.lock":
dl.Info("python lock file detected: %s", name)
return true, nil
case "gemfile.lock":
dl.Info("ruby lock file detected: %s", name)
return true, nil
case "cargo.lock":
dl.Info("rust lock file detected: %s", name)
return true, nil
case "yarn.lock":
dl.Info("yarn lock file detected: %s", name)
return true, nil
case "composer.lock":
dl.Info("composer lock file detected: %s", name)
return true, nil
default:
return false, nil
}
pdata := FileGetCbDataAsBoolPointer(data)
*pdata = true
return true, nil
}

View File

@ -34,18 +34,22 @@ func init() {
}
func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult {
// check repository for repository-specific policy
onFile := func(name string, dl checker.DetailLogger) (bool, error) {
var r bool
// Check repository for repository-specific policy.
onFile := func(name string, dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := FileGetCbDataAsBoolPointer(data)
if strings.EqualFold(name, "security.md") {
c.Dlogger.Info("security policy detected: %s", name)
return true, nil
*pdata = true
return false, nil
} else if isSecurityRstFound(name) {
c.Dlogger.Info("security policy detected: %s", name)
return true, nil
*pdata = true
return false, nil
}
return false, nil
return true, nil
}
r, err := CheckIfFileExists(CheckSecurityPolicy, c, onFile)
err := CheckIfFileExists(CheckSecurityPolicy, c, onFile, &r)
if err != nil {
return checker.CreateRuntimeErrorResult(CheckSecurityPolicy, err)
}
@ -64,14 +68,16 @@ func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult {
case err == nil:
defer dotGitHubClient.Close()
dotGitHub.RepoClient = dotGitHubClient
onFile = func(name string, dl checker.DetailLogger) (bool, error) {
onFile = func(name string, dl checker.DetailLogger, data FileCbData) (bool, error) {
pdata := FileGetCbDataAsBoolPointer(data)
if strings.EqualFold(name, "security.md") {
dl.Info("security policy detected in .github folder: %s", name)
return true, nil
*pdata = true
return false, nil
}
return false, nil
return true, nil
}
r, err = CheckIfFileExists(CheckSecurityPolicy, dotGitHub, onFile)
err = CheckIfFileExists(CheckSecurityPolicy, dotGitHub, onFile, &r)
if err != nil {
return checker.CreateRuntimeErrorResult(CheckSecurityPolicy, err)
}

View File

@ -29,7 +29,7 @@ import (
// TODO: use dedicated repo that don't change.
// TODO: need negative results.
var _ = Describe("E2E TEST:Binary-Artifacts", func() {
var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
Context("E2E TEST:Binary artifacts are not present in source code", func() {
It("Should return not binary artifacts in source code", func() {
dl := scut.TestDetailLogger{}
@ -65,22 +65,23 @@ var _ = Describe("E2E TEST:Binary-Artifacts", func() {
It("Should return binary artifacts present in source code", func() {
dl := scut.TestDetailLogger{}
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient, graphClient)
err := repoClient.InitRepo("a1ive", "grub2-filemanager")
err := repoClient.InitRepo("ossf-tests", "scorecard-check-binary-artifacts-e2e")
Expect(err).Should(BeNil())
req := checker.CheckRequest{
Ctx: context.Background(),
Client: ghClient,
RepoClient: repoClient,
Owner: "a1ive",
Repo: "grub2-filemanager",
Owner: "ossf-tests",
Repo: "scorecard-check-binary-artifacts-e2e",
GraphClient: graphClient,
Dlogger: &dl,
}
// TODO: upload real binaries to the repo as well.
expected := scut.TestReturn{
Errors: nil,
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfWarn: 35,
NumberOfInfo: 0,
NumberOfDebug: 0,
}