Merge branch 'main' into bugfix-codereview-implicit

.github/workflows/codeql-analysis.yml

@@ -61,7 +61,7 @@ jobs:
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@18fe527fa8b29f134bb91f32f1a5dc5abb15ed7f # v1
+      uses: github/codeql-action/init@c3b6fce4ee2ca25bc1066aa3bf73962fda0e8898 # v1
       with:
         languages: ${{ matrix.language }}
         queries: +security-extended
@@ -73,7 +73,7 @@ jobs:
     # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
     # If this step fails, then you should remove it and run the build manually (see below)
     - name: Autobuild
-      uses: github/codeql-action/autobuild@18fe527fa8b29f134bb91f32f1a5dc5abb15ed7f # v1
+      uses: github/codeql-action/autobuild@c3b6fce4ee2ca25bc1066aa3bf73962fda0e8898 # v1
 
     # ℹ️ Command-line programs to run using the OS shell.
     # 📚 https://git.io/JvXDl
@@ -87,4 +87,4 @@ jobs:
     #   make release
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@18fe527fa8b29f134bb91f32f1a5dc5abb15ed7f # v1
+      uses: github/codeql-action/analyze@c3b6fce4ee2ca25bc1066aa3bf73962fda0e8898 # v1

.github/workflows/integration.yml

@@ -76,7 +76,7 @@ jobs:
           retry_on: error
           timeout_minutes: 30
           command: make e2e-pat
-      
+
       - name: Run attestor e2e  #using retry because the GitHub token is being throttled.
         uses: nick-invision/retry@3e91a01664abd3c5cd539100d10d33b9c5b68482
         env:

.github/workflows/scorecard-analysis.yml

@@ -48,6 +48,6 @@ jobs:
           retention-days: 5
 
       - name: "Upload SARIF results"
-        uses: github/codeql-action/upload-sarif@18fe527fa8b29f134bb91f32f1a5dc5abb15ed7f # v1
+        uses: github/codeql-action/upload-sarif@c3b6fce4ee2ca25bc1066aa3bf73962fda0e8898 # v1
         with:
           sarif_file: results.sarif

checker/raw_result.go

@@ -165,11 +165,37 @@ type VulnerabilitiesData struct {
 	Vulnerabilities []clients.Vulnerability
 }
 
+type SecurityPolicyInformationType string
+
+const (
+	// forms of security policy hints being evaluated.
+	SecurityPolicyInformationTypeEmail SecurityPolicyInformationType = "emailAddress"
+	SecurityPolicyInformationTypeLink  SecurityPolicyInformationType = "httpLink"
+	SecurityPolicyInformationTypeText  SecurityPolicyInformationType = "vulnDisclosureText"
+)
+
+type SecurityPolicyValueType struct {
+	Match      string // Snippet of match
+	LineNumber uint   // Line number in policy file of match
+	Offset     uint   // Offset in the line of the match
+}
+
+type SecurityPolicyInformation struct {
+	InformationType  SecurityPolicyInformationType
+	InformationValue SecurityPolicyValueType
+}
+
+type SecurityPolicyFile struct {
+	// security policy information found in repo or org
+	Information []SecurityPolicyInformation
+	// file that contains the security policy information
+	File File
+}
+
 // SecurityPolicyData contains the raw results
 // for the Security-Policy check.
 type SecurityPolicyData struct {
-	// Files contains a list of files.
-	Files []File
+	PolicyFiles []SecurityPolicyFile
 }
 
 // BinaryArtifactData contains the raw results
@@ -238,6 +264,7 @@ type File struct {
 	Snippet   string   // Snippet of code
 	Offset    uint     // Offset in the file of Path (line for source/text files).
 	EndOffset uint     // End of offset in the file, e.g. if the command spans multiple lines.
+	FileSize  uint     // Total size of file.
 	Type      FileType // Type of file.
 	// TODO: add hash.
 }

checks/evaluation/security_policy.go

@@ -19,6 +19,104 @@ import (
 	sce "github.com/ossf/scorecard/v4/errors"
 )
 
+func scoreSecurityCriteria(f checker.File,
+	info []checker.SecurityPolicyInformation,
+	dl checker.DetailLogger,
+) int {
+	var urls, emails, discvuls, linkedContentLen, score int
+
+	emails = countSecInfo(info, checker.SecurityPolicyInformationTypeEmail, true)
+	urls = countSecInfo(info, checker.SecurityPolicyInformationTypeLink, true)
+	discvuls = countSecInfo(info, checker.SecurityPolicyInformationTypeText, false)
+
+	for _, i := range findSecInfo(info, checker.SecurityPolicyInformationTypeEmail, true) {
+		linkedContentLen += len(i.InformationValue.Match)
+	}
+	for _, i := range findSecInfo(info, checker.SecurityPolicyInformationTypeLink, true) {
+		linkedContentLen += len(i.InformationValue.Match)
+	}
+
+	msg := checker.LogMessage{
+		Path: f.Path,
+		Type: f.Type,
+		Text: "",
+	}
+
+	// #1: linked content found (email/http): score += 6
+	if (urls + emails) > 0 {
+		score += 6
+		msg.Text = "Found linked content in security policy"
+		dl.Info(&msg)
+	} else {
+		msg.Text = "no email or URL found in security policy"
+		dl.Warn(&msg)
+	}
+
+	// #2: more bytes than the sum of the length of all the linked content found: score += 3
+	//     rationale: there appears to be information and context around those links
+	//     no credit if there is just a link to a site or an email address (those given above)
+	//     the test here is that each piece of linked content will likely contain a space
+	//     before and after the content (hence the two multiplier)
+	if f.FileSize > 1 && (f.FileSize > uint(linkedContentLen+((urls+emails)*2))) {
+		score += 3
+		msg.Text = "Found text in security policy"
+		dl.Info(&msg)
+	} else {
+		msg.Text = "No text (beyond any linked content) found in security policy"
+		dl.Warn(&msg)
+	}
+
+	// #3: found whole number(s) and or match(es) to "Disclos" and or "Vuln": score += 1
+	//     rationale: works towards the intent of the security policy file
+	//     regarding whom to contact about vuls and disclosures and timing
+	//     e.g., we'll disclose, report a vulnerabily, 30 days, etc.
+	//     looking for at least 2 hits
+	if discvuls > 1 {
+		score += 1
+		msg.Text = "Found disclosure, vulnerability, and/or timelines in security policy"
+		dl.Info(&msg)
+	} else {
+		msg.Text = "One or no descriptive hints of disclosure, vulnerability, and/or timelines in security policy"
+		dl.Warn(&msg)
+	}
+
+	return score
+}
+
+func countSecInfo(secInfo []checker.SecurityPolicyInformation,
+	infoType checker.SecurityPolicyInformationType,
+	unique bool,
+) int {
+	keys := make(map[string]bool)
+	count := 0
+	for _, entry := range secInfo {
+		if _, present := keys[entry.InformationValue.Match]; !present && entry.InformationType == infoType {
+			keys[entry.InformationValue.Match] = true
+			count += 1
+		} else if !unique && entry.InformationType == infoType {
+			count += 1
+		}
+	}
+	return count
+}
+
+func findSecInfo(secInfo []checker.SecurityPolicyInformation,
+	infoType checker.SecurityPolicyInformationType,
+	unique bool,
+) []checker.SecurityPolicyInformation {
+	keys := make(map[string]bool)
+	var secList []checker.SecurityPolicyInformation
+	for _, entry := range secInfo {
+		if _, present := keys[entry.InformationValue.Match]; !present && entry.InformationType == infoType {
+			keys[entry.InformationValue.Match] = true
+			secList = append(secList, entry)
+		} else if !unique && entry.InformationType == infoType {
+			secList = append(secList, entry)
+		}
+	}
+	return secList
+}
+
 // SecurityPolicy applies the score policy for the Security-Policy check.
 func SecurityPolicy(name string, dl checker.DetailLogger, r *checker.SecurityPolicyData) checker.CheckResult {
 	if r == nil {
@@ -27,23 +125,31 @@ func SecurityPolicy(name string, dl checker.DetailLogger, r *checker.SecurityPol
 	}
 
 	// Apply the policy evaluation.
-	if r.Files == nil || len(r.Files) == 0 {
-		// If the file is null or has zero lengths, directly return as not detected.
+	if len(r.PolicyFiles) == 0 {
+		// If the file is unset, directly return as not detected.
 		return checker.CreateMinScoreResult(name, "security policy file not detected")
 	}
 
-	for _, f := range r.Files {
+	// TODO: although this a loop, the raw checks will only return one security policy
+	// when more than one security policy file can be aggregated into a composite
+	// score, that logic can be comprehended here.
+	score := 0
+	for _, spd := range r.PolicyFiles {
+		score = scoreSecurityCriteria(spd.File,
+			spd.Information, dl)
+
 		msg := checker.LogMessage{
-			Path:   f.Path,
-			Type:   f.Type,
-			Offset: f.Offset,
+			Path: spd.File.Path,
+			Type: spd.File.Type,
 		}
 		if msg.Type == checker.FileTypeURL {
 			msg.Text = "security policy detected in org repo"
 		} else {
 			msg.Text = "security policy detected in current repo"
 		}
+
 		dl.Info(&msg)
 	}
-	return checker.CreateMaxScoreResult(name, "security policy file detected")
+
+	return checker.CreateResultWithScore(name, "security policy file detected", score)
 }

checks/evaluation/security_policy_test.go

@@ -59,32 +59,34 @@ func TestSecurityPolicy(t *testing.T) {
 			args: args{
 				name: "test_security_policy_3",
 				r: &checker.SecurityPolicyData{
-					Files: []checker.File{
-						{
+					PolicyFiles: []checker.SecurityPolicyFile{{
+						File:                  checker.File{
 							Path: "/etc/security/pam_env.conf",
 							Type: checker.FileTypeURL,
 						},
+						Information:           make([]checker.SecurityPolicyInformation, 0),
 					},
-				},
+				}},
 			},
 			want: checker.CheckResult{
-				Score: 10,
+				Score: 0,
 			},
 		},
 		{
 			name: "test_security_policy_4",
 			args: args{
 				name: "test_security_policy_4",
 				r: &checker.SecurityPolicyData{
-					Files: []checker.File{
-						{
+					PolicyFiles: []checker.SecurityPolicyFile{{
+						File:                  checker.File{
 							Path: "/etc/security/pam_env.conf",
 						},
+						Information:           make([]checker.SecurityPolicyInformation, 0),
 					},
-				},
+				}},
 			},
 			want: checker.CheckResult{
-				Score: 10,
+				Score: 0,
 			},
 		},
 	}

checks/raw/binary_artifact.go

@@ -16,12 +16,9 @@ package raw
 
 import (
 	"fmt"
-	"log"
-	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
-	"unicode"
 	"unicode/utf8"
 
 	semver "github.com/Masterminds/semver/v3"
@@ -158,12 +155,7 @@ var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path strin
 	}
 
 	exists2 := binaryFileTypes[strings.ReplaceAll(filepath.Ext(path), ".", "")]
-	// TODO remove the comparison after testing in release-test worker
-	isTextFile := isText(content)
-	if _, enabled := os.LookupEnv("SCORECARD_COMPARE_ISTEXT"); enabled && isTextFile != isText2(content) {
-		log.Printf("isText implementations differ (raw.isText: %t) for file: %s", isTextFile, path)
-	}
-	if !isTextFile && exists2 {
+	if !isText(content) && exists2 {
 		*pfiles = append(*pfiles, checker.File{
 			Path:   path,
 			Type:   checker.FileTypeBinary,
@@ -174,22 +166,11 @@ var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path strin
 	return true, nil
 }
 
-// TODO: refine this function.
-func isText(content []byte) bool {
-	for _, c := range string(content) {
-		if c == '\t' || c == '\n' || c == '\r' {
-			continue
-		}
-		if !unicode.IsPrint(c) {
-			return false
-		}
-	}
-	return true
-}
-
-// TODO decine between isText and isText2 after testing in release-test worker
-// modified version of golang.org/x/tools/godoc/util.
-func isText2(s []byte) bool {
+// determines if the first 1024 bytes are text
+//
+//	A version of golang.org/x/tools/godoc/util modified to allow carriage returns
+//	and utf8.RuneError (0xFFFD), as the file may not be utf8 encoded.
+func isText(s []byte) bool {
 	const max = 1024 // at least utf8.UTFMax
 	if len(s) > max {
 		s = s[0:max]
@@ -199,8 +180,8 @@ func isText2(s []byte) bool {
 			// last char may be incomplete - ignore
 			break
 		}
-		if c == 0xFFFD || c < ' ' && c != '\n' && c != '\t' && c != '\r' {
-			// decoding error or control character - not a text file
+		if c < ' ' && c != '\n' && c != '\t' && c != '\r' {
+			// control character - not a text file
 			return false
 		}
 	}