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

* ignore parsing errors

* updates

checks/cii_best_practices.go

@@ -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 {

checks/pinned_dependencies.go

@@ -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"

checks/pinned_dependencies_test.go

@@ -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)
+}

checks/shell_download_validate.go

@@ -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
 }

checks/shell_download_validate_test.go

@@ -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)
 	}
 }