🐛 Ignore shell parsing errors when reporting results (#1878)

* ignore parsing errors

* updates
This commit is contained in:
laurentsimon 2022-05-02 10:11:50 -07:00 committed by GitHub
parent e97bf30ef6
commit 875b6f694e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 69 additions and 47 deletions

View File

@ -24,7 +24,6 @@ import (
// CheckCIIBestPractices is the registered name for CIIBestPractices.
const CheckCIIBestPractices = "CII-Best-Practices"
//nolint:gochecknoinits
func init() {
if err := registerCheck(CheckCIIBestPractices, CIIBestPractices, nil); err != nil {

View File

@ -15,6 +15,7 @@
package checks
import (
"errors"
"fmt"
"regexp"
"strings"
@ -235,14 +236,6 @@ func createReturnForIsShellScriptFreeOfInsecureDownloads(r pinnedResult,
dl, err)
}
func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateShellScriptIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsShellScriptFreeOfInsecureDownloads(r, dl, err)
}
var validateShellScriptIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func(
pathfn string,
content []byte,
@ -252,6 +245,7 @@ var validateShellScriptIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileCon
return false, fmt.Errorf(
"validateShellScriptIsFreeOfInsecureDownloads requires exactly 2 arguments: %w", errInvalidArgLength)
}
pdata := dataAsResultPointer(args[1])
dl := dataAsDetailLogger(args[0])
@ -260,10 +254,14 @@ var validateShellScriptIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileCon
addPinnedResult(pdata, true)
return true, nil
}
r, err := validateShellFile(pathfn, 0, 0 /*unknown*/, content, map[string]bool{}, dl)
if err != nil {
return false, err
// Ignore parsing errors.
if errors.Is(err, sce.ErrorShellParsing) {
addPinnedResult(pdata, true)
}
return false, nil
}
addPinnedResult(pdata, r)
@ -288,14 +286,6 @@ func createReturnForIsDockerfileFreeOfInsecureDownloads(r pinnedResult,
dl, err)
}
func testValidateDockerfileIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateDockerfileIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsDockerfileFreeOfInsecureDownloads(r, dl, err)
}
func isDockerfile(pathfn string, content []byte) bool {
if strings.HasSuffix(pathfn, ".go") ||
strings.HasSuffix(pathfn, ".c") ||
@ -368,6 +358,10 @@ var validateDockerfileIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileCont
r, err := validateShellFile(pathfn, uint(child.StartLine)-1, uint(child.EndLine)-1,
bytes, taintedFiles, dl)
if err != nil {
// Ignore parsing errors.
if errors.Is(err, sce.ErrorShellParsing) {
addPinnedResult(pdata, true)
}
return false, err
}
addPinnedResult(pdata, r)
@ -392,12 +386,6 @@ func createReturnForIsDockerfilePinned(r pinnedResult, dl checker.DetailLogger,
dl, err)
}
func testValidateDockerfileIsPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
var r pinnedResult
_, err := validateDockerfileIsPinned(pathfn, content, dl, &r)
return createReturnForIsDockerfilePinned(r, dl, err)
}
var validateDockerfileIsPinned fileparser.DoWhileTrueOnFileContent = func(
pathfn string,
content []byte,
@ -532,14 +520,6 @@ func createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r pinnedResult
dl, err)
}
func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, dl, err)
}
// validateGitHubWorkflowIsFreeOfInsecureDownloads checks if the workflow file downloads dependencies that are unpinned.
// Returns true if the check should continue executing after this file.
var validateGitHubWorkflowIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFileContent = func(
@ -612,6 +592,10 @@ var validateGitHubWorkflowIsFreeOfInsecureDownloads fileparser.DoWhileTrueOnFile
validated, err := validateShellFile(pathfn, uint(execRun.Run.Pos.Line), uint(execRun.Run.Pos.Line),
script, taintedFiles, dl)
if err != nil {
// Ignore parsing errors.
if errors.Is(err, sce.ErrorShellParsing) {
addPinnedResult(pdata, true)
}
return false, err
}
addPinnedResult(pdata, validated)
@ -640,12 +624,6 @@ func createReturnForIsGitHubActionsWorkflowPinned(r worklowPinningResult, dl che
dl, err)
}
func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
var r worklowPinningResult
_, err := validateGitHubActionWorkflow(pathfn, content, dl, &r)
return createReturnForIsGitHubActionsWorkflowPinned(r, dl, err)
}
func generateOwnerToDisplay(gitHubOwned bool) string {
if gitHubOwned {
return "GitHub-owned"

View File

@ -1115,6 +1115,17 @@ func TestShellScriptDownload(t *testing.T) {
NumberOfDebug: 0,
},
},
{
name: "invalid shell script",
filename: "./testdata/script-invalid.sh",
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 1,
},
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
@ -1614,3 +1625,39 @@ func Test_maxScore(t *testing.T) {
})
}
}
func testValidateShellScriptIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateShellScriptIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsShellScriptFreeOfInsecureDownloads(r, dl, err)
}
func testValidateDockerfileIsFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateDockerfileIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsDockerfileFreeOfInsecureDownloads(r, dl, err)
}
func testValidateDockerfileIsPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
var r pinnedResult
_, err := validateDockerfileIsPinned(pathfn, content, dl, &r)
return createReturnForIsDockerfilePinned(r, dl, err)
}
func testValidateGitHubWorkflowScriptFreeOfInsecureDownloads(pathfn string,
content []byte, dl checker.DetailLogger,
) (int, error) {
var r pinnedResult
_, err := validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn, content, dl, &r)
return createReturnForIsGitHubWorkflowScriptFreeOfInsecureDownloads(r, dl, err)
}
func testIsGitHubActionsWorkflowPinned(pathfn string, content []byte, dl checker.DetailLogger) (int, error) {
var r worklowPinningResult
_, err := validateGitHubActionWorkflow(pathfn, content, dl, &r)
return createReturnForIsGitHubActionsWorkflowPinned(r, dl, err)
}

View File

@ -17,7 +17,6 @@ package checks
import (
"bufio"
"bytes"
"errors"
"fmt"
"net/url"
"path"
@ -599,8 +598,8 @@ func isChocoUnpinnedDownload(cmd []string) bool {
str := parts[0]
if strings.EqualFold(str, "--requirechecksum") ||
strings.EqualFold(str, "--requirechecksums") ||
strings.EqualFold(str, "--require-checksums") {
strings.EqualFold(str, "--requirechecksums") ||
strings.EqualFold(str, "--require-checksums") {
return false
}
}
@ -995,12 +994,11 @@ func validateShellFile(pathfn string, startLine, endLine uint,
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.
if err != nil {
// Print this particular error.
dl.Debug(&checker.LogMessage{
Text: err.Error(),
})
err = nil
}
return r, err
}

View File

@ -101,7 +101,7 @@ func TestValidateShellFile(t *testing.T) {
}
dl := scut.TestDetailLogger{}
_, err = validateShellFile(filename, 0, 0, content, map[string]bool{}, &dl)
if err != nil {
t.Errorf("failed to discard shell parsing error: %v", err)
if err == nil {
t.Errorf("failed to detect shell parsing error: %v", err)
}
}