Skip to content

Commit b62357f

Browse files
shissamraghavkaul
andcommitted
✨ Add Additional Details to License Check (ossf#2442)
* ✨ Improved Security Policy Check (ossf#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 <[email protected]> * Repaired Security Policy to correctly use linked content length for evaluation Signed-off-by: Scott Hissam <[email protected]> * gofmt'ed changes Signed-off-by: Scott Hissam <[email protected]> * 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 <[email protected]> * added unit test cases for the new content-based Security Policy checks Signed-off-by: Scott Hissam <[email protected]> * reverted the direct (mistaken) change to checks.md and updated the checks.yaml for generate-docs Signed-off-by: Scott Hissam <[email protected]> * ✨ Improved Security Policy Check (ossf#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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Addressed PR comments; added telemetry for policy hits in security policy file to track hits by line number Signed-off-by: Scott Hissam <[email protected]> * Resolved merge conflict with checks.yaml Signed-off-by: Scott Hissam <[email protected]> * updated raw results to emit all the raw information for the new security policy check Signed-off-by: Scott Hissam <[email protected]> * Resolved merge conflicts and lint errors with json_raw_results.go Signed-off-by: Scott Hissam <[email protected]> * Addressed review comments to reorganize security policy data struct to support the potential for multiple security policy files. Signed-off-by: Scott Hissam <[email protected]> * 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 <[email protected]> * added comments regarding the capacity to support multiple policy files and removed unneeded break statements in the code Signed-off-by: Scott Hissam <[email protected]> * 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 <[email protected]> * restored reporting full security policy path and filename for policies found in the org level repos Signed-off-by: Scott Hissam <[email protected]> * Resolved conflicts in checks.yaml for documentation Signed-off-by: Scott Hissam <[email protected]> * ✨ CLI for scorecard-attestor (ossf#2309) * Reorganize Signed-off-by: Raghav Kaul <[email protected]> * Working commit Signed-off-by: Raghav Kaul <[email protected]> * Compile with local scorecard; go mod tidy Signed-off-by: Raghav Kaul <[email protected]> * Add signing code Heavily borrowed from https://github.com/grafeas/kritis/blob/master/cmd/kritis/signer/main.go Signed-off-by: Raghav Kaul <[email protected]> * Update deps * Naming * Makefile Signed-off-by: Raghav Kaul <[email protected]> * Edit license, add lint.yml Signed-off-by: Raghav Kaul <[email protected]> * checks: go mod tidy, license Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments * Split into checker/signer files * Naming convention Signed-off-by: Raghav Kaul <[email protected]> * License, remove golangci.yml Signed-off-by: Raghav Kaul <[email protected]> * Address PR comments * Use cobra Signed-off-by: Raghav Kaul <[email protected]> * Add tests for root command Signed-off-by: Raghav Kaul <[email protected]> * Filter out checks that aren't needed for policy evaluation Signed-off-by: Raghav Kaul <[email protected]> * Add `make` targets for attestor; submit coverage stats Signed-off-by: Raghav Kaul <[email protected]> * Improvements * Use sclog instead of glog * Remove unneeded subcommands * Formatting Signed-off-by: Raghav Kaul <[email protected]> * Flags: Make note-name constant and fix messaging Signed-off-by: Raghav Kaul <[email protected]> * Remove SupportedRequestTypes Signed-off-by: Raghav Kaul <[email protected]> * go mod tidy Signed-off-by: Raghav Kaul <[email protected]> * go mod tidy, makefile Signed-off-by: Raghav Kaul <[email protected]> * Fix GH actions run Signed-off-by: Raghav Kaul <[email protected]> Signed-off-by: Raghav Kaul <[email protected]> Signed-off-by: Scott Hissam <[email protected]> * removed whitespace before stanza for Run attestor e2e Signed-off-by: Scott Hissam <[email protected]> * resolved code review and doc review comments Signed-off-by: Scott Hissam <[email protected]> * repaired the link for the maintainer's guide for supporting the coordinated vulnerability disclosure guidelines Signed-off-by: Scott Hissam <[email protected]> * initial implementation of ossf#1369 (comment) to provide more license details Signed-off-by: Scott Hissam <[email protected]> * draft implementation to provide more information on license details Signed-off-by: Scott Hissam <[email protected]> * repaired a misspelling Signed-off-by: Scott Hissam <[email protected]> * 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 <[email protected]> * Return an error status similar to other gitlab checks Signed-off-by: Scott Hissam <[email protected]> * add new raw licenses data Signed-off-by: Scott Hissam <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * license check now, primarily, uses the GH API for checking licenses Signed-off-by: Scott Hissam <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> Signed-off-by: Scott Hissam <[email protected]> Signed-off-by: Raghav Kaul <[email protected]> Co-authored-by: raghavkaul <[email protected]>
1 parent 5ac2f47 commit b62357f

File tree

19 files changed

+1490
-154
lines changed

19 files changed

+1490
-154
lines changed

Diff for: checker/raw_result.go

+27-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
// RawResults contains results before a policy
2424
// is applied.
25-
//nolint
25+
// nolint
2626
type RawResults struct {
2727
PackagingResults PackagingData
2828
CIIBestPracticesResults CIIBestPracticesData
@@ -68,7 +68,7 @@ type PackagingData struct {
6868
}
6969

7070
// Package represents a package.
71-
//nolint
71+
// nolint
7272
type Package struct {
7373
// TODO: not supported yet. This needs to be unique across
7474
// ecosystems: purl, OSV, CPE, etc.
@@ -125,10 +125,34 @@ type MaintainedData struct {
125125
ArchivedStatus ArchivedStatus
126126
}
127127

128+
type LicenseAttributionType string
129+
130+
const (
131+
// sources of license information used to assert repo's license.
132+
LicenseAttributionTypeOther LicenseAttributionType = "other"
133+
LicenseAttributionTypeAPI LicenseAttributionType = "repositoryAPI"
134+
LicenseAttributionTypeHeuristics LicenseAttributionType = "builtinHeuristics"
135+
)
136+
137+
// license details.
138+
type License struct {
139+
Name string // OSI standardized license name
140+
SpdxID string // SPDX standardized identifier
141+
Attribution LicenseAttributionType // source of licensing information
142+
Approved bool // FSF or OSI Approved License
143+
}
144+
145+
// one file contains one license.
146+
type LicenseFile struct {
147+
LicenseInformation License
148+
File File
149+
}
150+
128151
// LicenseData contains the raw results
129152
// for the License check.
153+
// Some repos may have more than one license.
130154
type LicenseData struct {
131-
Files []File
155+
LicenseFiles []LicenseFile
132156
}
133157

134158
// CodeReviewData contains the raw results

Diff for: checks/evaluation/license.go

+57-8
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,76 @@ import (
1919
sce "github.com/ossf/scorecard/v4/errors"
2020
)
2121

22+
func scoreLicenseCriteria(f *checker.LicenseFile,
23+
dl checker.DetailLogger,
24+
) int {
25+
var score int
26+
msg := checker.LogMessage{
27+
Path: "",
28+
Type: checker.FileTypeNone,
29+
Text: "",
30+
Offset: 1,
31+
}
32+
msg.Path = f.File.Path
33+
msg.Type = checker.FileTypeSource
34+
// #1 a license file was found.
35+
score += 6
36+
37+
// #2 the licence was found at the top-level or LICENSE/ folder.
38+
switch f.LicenseInformation.Attribution {
39+
case checker.LicenseAttributionTypeAPI, checker.LicenseAttributionTypeHeuristics:
40+
// both repoAPI and scorecard (not using the API) follow checks.md
41+
// for a file to be found it must have been in the correct location
42+
// award location points.
43+
score += 3
44+
msg.Text = "License file found in expected location"
45+
dl.Info(&msg)
46+
// for repo attribution prepare warning if not an recognized license"
47+
msg.Text = "Any licence detected not an FSF or OSI recognized license"
48+
case checker.LicenseAttributionTypeOther:
49+
// TODO ascertain location found
50+
score += 0
51+
msg.Text = "License file found in unexpected location"
52+
dl.Warn(&msg)
53+
// for non repo attribution not the license detection is not supported
54+
msg.Text = "Detecting license content not supported"
55+
default:
56+
}
57+
58+
// #3 is the license either an FSF or OSI recognized/approved license
59+
if f.LicenseInformation.Approved {
60+
score += 1
61+
msg.Text = "FSF or OSI recognized license"
62+
dl.Info(&msg)
63+
} else {
64+
// message text for this condition set above
65+
dl.Warn(&msg)
66+
}
67+
return score
68+
}
69+
2270
// License applies the score policy for the License check.
2371
func License(name string, dl checker.DetailLogger,
2472
r *checker.LicenseData,
2573
) checker.CheckResult {
74+
var score int
2675
if r == nil {
2776
e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
2877
return checker.CreateRuntimeErrorResult(name, e)
2978
}
3079

3180
// Apply the policy evaluation.
32-
if r.Files == nil || len(r.Files) == 0 {
81+
if r.LicenseFiles == nil || len(r.LicenseFiles) == 0 {
3382
return checker.CreateMinScoreResult(name, "license file not detected")
3483
}
3584

36-
for _, f := range r.Files {
37-
dl.Info(&checker.LogMessage{
38-
Path: f.Path,
39-
Type: checker.FileTypeSource,
40-
Offset: 1,
41-
})
85+
// TODO: although this a loop, the raw checks will only return one licence file
86+
// when more than one license file can be aggregated into a composite
87+
// score, that logic can be comprehended here.
88+
score = 0
89+
for idx := range r.LicenseFiles {
90+
score = scoreLicenseCriteria(&r.LicenseFiles[idx], dl)
4291
}
4392

44-
return checker.CreateMaxScoreResult(name, "license file detected")
93+
return checker.CreateResultWithScore(name, "license file detected", score)
4594
}

Diff for: checks/license.go

-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ const CheckLicense = "License"
2727
//nolint:gochecknoinits
2828
func init() {
2929
supportedRequestTypes := []checker.RequestType{
30-
checker.FileBased,
3130
checker.CommitBased,
3231
}
3332
if err := registerCheck(CheckLicense, License, supportedRequestTypes); err != nil {

Diff for: checks/license_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,9 @@ func TestLicenseFileSubdirectory(t *testing.T) {
4242
inputFolder: "testdata/licensedir/withlicense",
4343
expected: scut.TestReturn{
4444
Error: nil,
45-
Score: checker.MaxResultScore,
45+
Score: checker.MaxResultScore - 1,
4646
NumberOfInfo: 1,
47+
NumberOfWarn: 1,
4748
},
4849
err: nil,
4950
},

0 commit comments

Comments
 (0)