✨ Add Additional Details to License Check (#2442)

* ✨ Improved Security Policy Check (#2137)

* Examines and awards points for linked content (URLs / Emails)

* Examines and awards points for hints of disclosure and vulnerability practices

* Examines and awards points for hints of elaboration of timelines

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Repaired Security Policy to correctly use linked content length for evaluation

Signed-off-by: Scott Hissam <shissam@gmail.com>

* gofmt'ed changes

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Repaired the case in the evaluation which was too sensitive to content length over the length of the linked content for urls and emails

Signed-off-by: Scott Hissam <shissam@gmail.com>

* added unit test cases for the new content-based Security Policy checks

Signed-off-by: Scott Hissam <shissam@gmail.com>

* reverted the direct (mistaken) change to checks.md and updated the checks.yaml for generate-docs

Signed-off-by: Scott Hissam <shissam@gmail.com>

* ✨ Improved Security Policy Check (#2137) (revisted based on comments)

* replaced reason strings with log.Info & log.Warn (as seen in --show-details)

* internal assertion check for nil (*pinfo) and empty pfile

* internal switched to FileTypeText over FileTypeSource

* internal implement type SecurityPolicyInformationType/SecurityPolicyInformation revised SecurityPolicyData to support only one file

* revised expected unit-test results and revised unit-test to reflect the new SecurityPolicyData type

Signed-off-by: Scott Hissam <shissam@gmail.com>

* revised the score value based on observation of one *or more* url(s) or one email(s) found; unit tests update accordingly

Signed-off-by: Scott Hissam <shissam@gmail.com>

* revised the score value based on observation of one *or more* url(s) or one email(s) found; unit tests update accordingly

Signed-off-by: Scott Hissam <shissam@gmail.com>

* revised the score value based on observation of one *or more* url(s) or one email(s) found; e2e tests update accordingly

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Addressed PR comments; added telemetry for policy hits in security policy file to track hits by line number

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Resolved merge conflict with checks.yaml

Signed-off-by: Scott Hissam <shissam@gmail.com>

* updated raw results to emit all the raw information for the new security policy check

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Resolved merge conflicts and lint errors with json_raw_results.go

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Addressed review comments to reorganize security policy data struct to support the potential for multiple security policy files.

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Added logic to the security policy to process multiple security policy files only after future improvements to aggregating scoring across such files are designed. For now the security policy behaves as originally designed to stop once one of the expected policy files are found in the repo

Signed-off-by: Scott Hissam <shissam@gmail.com>

* added comments regarding the capacity to support multiple policy files and removed unneeded break statements in the code

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Addressed review comments to remove the dependency on the path in the filename from the code and introduced FileSize to checker.File type and removed the SecurityContentLength which was used to hold that information for the new security policy assessment

Signed-off-by: Scott Hissam <shissam@gmail.com>

* restored reporting full security policy path and filename for policies found in the org level repos

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Resolved conflicts in checks.yaml for documentation

Signed-off-by: Scott Hissam <shissam@gmail.com>

* ✨ CLI for scorecard-attestor (#2309)

* Reorganize

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Working commit

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Compile with local scorecard; go mod tidy

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Add signing code

Heavily borrowed from https://github.com/grafeas/kritis/blob/master/cmd/kritis/signer/main.go

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Update deps

* Naming
* Makefile

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Edit license, add lint.yml

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* checks: go mod tidy, license

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Address PR comments

* Split into checker/signer files
* Naming convention

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* License, remove golangci.yml

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Address PR comments

* Use cobra

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Add tests for root command

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Filter out checks that aren't needed for policy evaluation

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Add `make` targets for attestor; submit coverage stats

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Improvements

* Use sclog instead of glog
* Remove unneeded subcommands
* Formatting

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Flags: Make note-name constant and fix messaging

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Remove SupportedRequestTypes

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* go mod tidy

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* go mod tidy, makefile

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

* Fix GH actions run

Signed-off-by: Raghav Kaul <raghavkaul@google.com>

Signed-off-by: Raghav Kaul <raghavkaul@google.com>
Signed-off-by: Scott Hissam <shissam@gmail.com>

* removed whitespace before stanza for Run attestor e2e

Signed-off-by: Scott Hissam <shissam@gmail.com>

* resolved code review and doc review comments

Signed-off-by: Scott Hissam <shissam@gmail.com>

* repaired the link for the maintainer's guide for supporting the coordinated vulnerability disclosure guidelines

Signed-off-by: Scott Hissam <shissam@gmail.com>

* initial implementation of #1369 (comment) to provide more license details

Signed-off-by: Scott Hissam <shissam@gmail.com>

* draft implementation to provide more information on license details

Signed-off-by: Scott Hissam <shissam@gmail.com>

* repaired a misspelling

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Changed to handle http errors with 404 not found as being a non-error for not being able to find a license

Signed-off-by: Scott Hissam <shissam@gmail.com>

* Return an error status similar to other gitlab checks

Signed-off-by: Scott Hissam <shissam@gmail.com>

* add new raw licenses data

Signed-off-by: Scott Hissam <shissam@gmail.com>

* updated e2e test as new license check generates more info and warn as scores change as license file content is not parsed

Signed-off-by: Scott Hissam <shissam@gmail.com>

* added numerous more test filenames and a shouldFail boolean as some filenames will fail that do not meet checks.md rules

Signed-off-by: Scott Hissam <shissam@gmail.com>

* license check now, primarily, uses the GH API for checking licenses

Signed-off-by: Scott Hissam <shissam@gmail.com>

* updated local checker as new license check generates more info and warn as scores change as license file content is not parsed

Signed-off-by: Scott Hissam <shissam@gmail.com>

* added draft license gradation for scoring, add a map to OSI and FSF licenses, added GH API for retrieving repo license, revamp license filename matching when not using a repo API for detecting license files.

Signed-off-by: Scott Hissam <shissam@gmail.com>

* repaired race condition for case insensitive map, improved regex matching, moved licenses to raw, raw now mimics GH API return values for key, name, etc., updated unit tests and raw results accordingly

Signed-off-by: Scott Hissam <shissam@gmail.com>

* completed disambiguation of SPDX Identifiers and filename extensions, reworked some of the code comments, added map generation to TestLicense, added an additional mutex for the regex group identifier index, removed spurious prints, revised unit test accordingly, updated documentation.

Signed-off-by: Scott Hissam <shissam@gmail.com>

* removed repo Key from LicenseInformation as unneeded, changed attribution constants to be more meaningful, update documentation as necessary for changes

Signed-off-by: Scott Hissam <shissam@gmail.com>

Signed-off-by: Scott Hissam <shissam@gmail.com>
Signed-off-by: Raghav Kaul <raghavkaul@google.com>
Co-authored-by: raghavkaul <8695110+raghavkaul@users.noreply.github.com>

checker/raw_result.go

@@ -22,7 +22,7 @@ import (
 
 // RawResults contains results before a policy
 // is applied.
-//nolint
+// nolint
 type RawResults struct {
 	PackagingResults            PackagingData
 	CIIBestPracticesResults     CIIBestPracticesData
@@ -68,7 +68,7 @@ type PackagingData struct {
 }
 
 // Package represents a package.
-//nolint
+// nolint
 type Package struct {
 	// TODO: not supported yet. This needs to be unique across
 	// ecosystems: purl, OSV, CPE, etc.
@@ -125,10 +125,34 @@ type MaintainedData struct {
 	ArchivedStatus       ArchivedStatus
 }
 
+type LicenseAttributionType string
+
+const (
+	// sources of license information used to assert repo's license.
+	LicenseAttributionTypeOther      LicenseAttributionType = "other"
+	LicenseAttributionTypeAPI        LicenseAttributionType = "repositoryAPI"
+	LicenseAttributionTypeHeuristics LicenseAttributionType = "builtinHeuristics"
+)
+
+// license details.
+type License struct {
+	Name        string                 // OSI standardized license name
+	SpdxID      string                 // SPDX standardized identifier
+	Attribution LicenseAttributionType // source of licensing information
+	Approved    bool                   // FSF or OSI Approved License
+}
+
+// one file contains one license.
+type LicenseFile struct {
+	LicenseInformation License
+	File               File
+}
+
 // LicenseData contains the raw results
 // for the License check.
+// Some repos may have more than one license.
 type LicenseData struct {
-	Files []File
+	LicenseFiles []LicenseFile
 }
 
 // CodeReviewData contains the raw results

checks/evaluation/license.go

@@ -19,27 +19,76 @@ import (
 	sce "github.com/ossf/scorecard/v4/errors"
 )
 
+func scoreLicenseCriteria(f *checker.LicenseFile,
+	dl checker.DetailLogger,
+) int {
+	var score int
+	msg := checker.LogMessage{
+		Path:   "",
+		Type:   checker.FileTypeNone,
+		Text:   "",
+		Offset: 1,
+	}
+	msg.Path = f.File.Path
+	msg.Type = checker.FileTypeSource
+	// #1 a license file was found.
+	score += 6
+
+	// #2 the licence was found at the top-level or LICENSE/ folder.
+	switch f.LicenseInformation.Attribution {
+	case checker.LicenseAttributionTypeAPI, checker.LicenseAttributionTypeHeuristics:
+		// both repoAPI and scorecard (not using the API) follow checks.md
+		// for a file to be found it must have been in the correct location
+		// award location points.
+		score += 3
+		msg.Text = "License file found in expected location"
+		dl.Info(&msg)
+		// for repo attribution prepare warning if not an recognized license"
+		msg.Text = "Any licence detected not an FSF or OSI recognized license"
+	case checker.LicenseAttributionTypeOther:
+		// TODO ascertain location found
+		score += 0
+		msg.Text = "License file found in unexpected location"
+		dl.Warn(&msg)
+		// for non repo attribution not the license detection is not supported
+		msg.Text = "Detecting license content not supported"
+	default:
+	}
+
+	// #3 is the license either an FSF or OSI recognized/approved license
+	if f.LicenseInformation.Approved {
+		score += 1
+		msg.Text = "FSF or OSI recognized license"
+		dl.Info(&msg)
+	} else {
+		// message text for this condition set above
+		dl.Warn(&msg)
+	}
+	return score
+}
+
 // License applies the score policy for the License check.
 func License(name string, dl checker.DetailLogger,
 	r *checker.LicenseData,
 ) checker.CheckResult {
+	var score int
 	if r == nil {
 		e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
 		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
 	// Apply the policy evaluation.
-	if r.Files == nil || len(r.Files) == 0 {
+	if r.LicenseFiles == nil || len(r.LicenseFiles) == 0 {
 		return checker.CreateMinScoreResult(name, "license file not detected")
 	}
 
-	for _, f := range r.Files {
-		dl.Info(&checker.LogMessage{
-			Path:   f.Path,
-			Type:   checker.FileTypeSource,
-			Offset: 1,
-		})
+	// TODO: although this a loop, the raw checks will only return one licence file
+	// when more than one license file can be aggregated into a composite
+	// score, that logic can be comprehended here.
+	score = 0
+	for idx := range r.LicenseFiles {
+		score = scoreLicenseCriteria(&r.LicenseFiles[idx], dl)
 	}
 
-	return checker.CreateMaxScoreResult(name, "license file detected")
+	return checker.CreateResultWithScore(name, "license file detected", score)
 }

checks/license.go

@@ -27,7 +27,6 @@ const CheckLicense = "License"
 //nolint:gochecknoinits
 func init() {
 	supportedRequestTypes := []checker.RequestType{
-		checker.FileBased,
 		checker.CommitBased,
 	}
 	if err := registerCheck(CheckLicense, License, supportedRequestTypes); err != nil {

checks/license_test.go

@@ -42,8 +42,9 @@ func TestLicenseFileSubdirectory(t *testing.T) {
 			inputFolder: "testdata/licensedir/withlicense",
 			expected: scut.TestReturn{
 				Error:        nil,
-				Score:        checker.MaxResultScore,
+				Score:        checker.MaxResultScore - 1,
 				NumberOfInfo: 1,
+				NumberOfWarn: 1,
 			},
 			err: nil,
 		},