Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add check ID #4021

Closed
wants to merge 12 commits into from
11 changes: 6 additions & 5 deletions checker/check_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,16 @@ var errSuccessTotal = errors.New("unexpected number of success is higher than to
//
//nolint:govet
type CheckResult struct {
Name string
Comment on lines 63 to -64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've expected the scdiff invocation above to fail. Going to poke around a little before merge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this patch before merge?

diff --git a/cmd/internal/scdiff/app/compare/compare.go b/cmd/internal/scdiff/app/compare/compare.go
index 51dfe62f..7155a97c 100644
--- a/cmd/internal/scdiff/app/compare/compare.go
+++ b/cmd/internal/scdiff/app/compare/compare.go
@@ -49,6 +49,9 @@ func compareChecks(r1, r2 *scorecard.Result) bool {
                if r1.Checks[i].Name != r2.Checks[i].Name {
                        return false
                }
+               if r1.Checks[i].ID != r2.Checks[i].ID {
+                       return false
+               }
                if r1.Checks[i].Score != r2.Checks[i].Score {
                        return false
                }

Version int
Error error
Score int
Name string
Reason string
Details []CheckDetail

// Findings from the check's probes.
Findings []finding.Finding
ID uint
Version int
Score int
}

// CheckDetail contains information for each detail.
Expand All @@ -86,13 +87,13 @@ type LogMessage struct {
Finding *finding.Finding

// Non-structured results.
Remediation *probe.Remediation // Remediation information, if any.
Text string // A short string explaining why the detail was recorded/logged.
Path string // Fullpath to the file.
Snippet string // Snippet of code
Type finding.FileType // Type of file.
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.
Snippet string // Snippet of code
Remediation *probe.Remediation // Remediation information, if any.
}

// ProportionalScoreWeighted is a structure that contains
Expand Down
2 changes: 1 addition & 1 deletion cmd/internal/scdiff/app/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func generate(scRunner scorecardRunner, repos io.Reader, output io.Writer) error
if err != nil {
return fmt.Errorf("running scorecard on %s: %w", scanner.Text(), err)
}
// TODO(https://github.com/ossf/scorecard/issues/3360) pretty print?
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved

err = format.JSON(&results, output)
if err != nil {
return fmt.Errorf("formatting results: %w", err)
Expand Down
16 changes: 9 additions & 7 deletions cron/internal/format/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
type jsonCheckResult struct {
Name string
Details []string
ID uint
ashearin marked this conversation as resolved.
Show resolved Hide resolved
Confidence int
Pass bool
}
Expand All @@ -46,13 +47,13 @@ type jsonCheckDocumentationV2 struct {
// Can be extended if needed.
}

//nolint:govet
type jsonCheckResultV2 struct {
Details []string `json:"details"`
Score int `json:"score"`
Doc jsonCheckDocumentationV2 `json:"documentation"`
Reason string `json:"reason"`
Name string `json:"name"`
Doc jsonCheckDocumentationV2 `json:"documentation"`
Details []string `json:"details"`
ID uint `json:"id"`
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
Score int `json:"score"`
}

type jsonRepoV2 struct {
Expand All @@ -72,14 +73,13 @@ func (s jsonFloatScore) MarshalJSON() ([]byte, error) {
return []byte(fmt.Sprintf("%.1f", s)), nil
}

//nolint:govet
type jsonScorecardResultV2 struct {
Date string `json:"date"`
Repo jsonRepoV2 `json:"repo"`
Scorecard jsonScorecardV2 `json:"scorecard"`
AggregateScore jsonFloatScore `json:"score"`
Date string `json:"date"`
Checks []jsonCheckResultV2 `json:"checks"`
Metadata []string `json:"metadata"`
AggregateScore jsonFloatScore `json:"score"`
}

// AsJSON exports results as JSON for new detail format.
Expand All @@ -94,6 +94,7 @@ func AsJSON(r *pkg.ScorecardResult, showDetails bool, logLevel log.Level, writer

for _, checkResult := range r.Checks {
tmpResult := jsonCheckResult{
ID: checkResult.ID,
Name: checkResult.Name,
}
if showDetails {
Expand Down Expand Up @@ -148,6 +149,7 @@ func AsJSON2(r *pkg.ScorecardResult, showDetails bool,
}

tmpResult := jsonCheckResultV2{
ID: doc.GetID(),
Name: checkResult.Name,
Doc: jsonCheckDocumentationV2{
URL: doc.GetDocumentationURL(r.Scorecard.CommitSHA),
Expand Down
5 changes: 5 additions & 0 deletions cron/internal/format/mock_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ import (
type mockCheck struct {
name, risk, short, description, url string
tags, remediation, repos []string
ID uint
}

func (c *mockCheck) GetID() uint {
return c.ID
}

func (c *mockCheck) GetName() string {
Expand Down
1 change: 1 addition & 0 deletions docs/checks/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type Doc interface {

// CheckDoc defines the documentation interface for a check.
type CheckDoc interface {
GetID() uint
GetName() string
GetRisk() string
GetShort() string
Expand Down
5 changes: 5 additions & 0 deletions docs/checks/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ type CheckDocImpl struct {
internalCheck internal.Check
}

// GetID returns the ID of the check.
func (c *CheckDocImpl) GetID() uint {
return c.internalCheck.ID
}

// GetName returns the name of the check.
func (c *CheckDocImpl) GetName() string {
return c.internalCheck.Name
Expand Down
19 changes: 19 additions & 0 deletions docs/checks/internal/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# Run `cd checks/main && go run /main` to generate `checks.json` and `checks.md`.
checks:
Maintained:
id: 1
risk: High
tags: supply-chain, security
repos: GitHub
Expand Down Expand Up @@ -49,6 +50,7 @@ checks:
commitment. External users should determine whether the software is the type
that would not normally need active maintenance.
Dependency-Update-Tool:
id: 2
risk: High
tags: supply-chain, security, dependencies
repos: GitHub, local
Expand Down Expand Up @@ -88,6 +90,7 @@ checks:
via the [pinDigests setting](https://docs.renovatebot.com/configuration-options/#pindigests) without
additional manual effort.
Binary-Artifacts:
id: 3
risk: High
tags: supply-chain, security, dependencies
repos: GitHub, GitLab, local
Expand Down Expand Up @@ -139,6 +142,7 @@ checks:
- >-
Build from source.
Branch-Protection:
id: 4
risk: High
tags: supply-chain, security, source-code, code-reviews
repos: GitHub, GitLab
Expand Down Expand Up @@ -225,6 +229,7 @@ checks:
- >-
For GitHub, check out the steps [here](https://docs.github.com/en/github/administering-a-repository/managing-a-branch-protection-rule).
CI-Tests:
id: 5
risk: Low
tags: supply-chain, testing
repos: GitHub, GitLab
Expand Down Expand Up @@ -258,6 +263,7 @@ checks:
request (e.g. if hosted on GitHub, [GitHub Actions](https://docs.github.com/en/actions/learn-github-actions/introduction-to-github-actions),
[Prow](https://github.com/kubernetes/test-infra/tree/master/prow), etc).
CII-Best-Practices:
id: 6
risk: Low
tags: security-awareness, security-training, security
repos: GitHub
Expand All @@ -284,6 +290,7 @@ checks:
- >-
Sign up for the [OpenSSF Best Practices program](https://www.bestpractices.dev/).
Code-Review:
id: 7
risk: High
tags: supply-chain, security, source-code, code-reviews
repos: GitHub
Expand Down Expand Up @@ -357,6 +364,7 @@ checks:
Enforce the rule for administrators / code owners as well.
([Instructions for GitHub.](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#include-administrators))
Contributors:
id: 8
risk: Low
tags: source-code
repos: GitHub
Expand Down Expand Up @@ -388,6 +396,7 @@ checks:
it simply provides insight into which organizations have contributed so that
you can make a trust-based decision based on that information.
Fuzzing:
id: 9
risk: Medium
tags: supply-chain, security, testing
repos: GitHub
Expand Down Expand Up @@ -418,6 +427,7 @@ checks:
Integrate the project with OSS-Fuzz by following the instructions
[here](https://google.github.io/oss-fuzz/).
Packaging:
id: 10
risk: Medium
tags: supply-chain, security, releases
repos: GitHub
Expand Down Expand Up @@ -462,6 +472,7 @@ checks:
- Publish your project as a downloadable package, e.g., if hosted on GitHub, use [GitHub's mechanisms for publishing a package](https://docs.github.com/en/packages/learn-github-packages/publishing-a-package).
- If hosted on GitHub, use a GitHub action to release your package to language-specific hubs.
Pinned-Dependencies:
id: 11
risk: Medium
tags: supply-chain, security, dependencies
repos: GitHub, local
Expand Down Expand Up @@ -528,6 +539,7 @@ checks:
- >-
To help update your dependencies after pinning them, use tools such as those listed for the dependency update tool check.
SAST:
id: 12
risk: Medium
tags: supply-chain, security, testing
repos: GitHub
Expand Down Expand Up @@ -559,6 +571,7 @@ checks:
Run CodeQL checks in your CI/CD by following the instructions
[here](https://github.com/github/codeql-action#usage).
Security-Policy:
id: 13
risk: Medium
short: Determines if the project has published a security policy.
repos: GitHub
Expand Down Expand Up @@ -610,6 +623,7 @@ checks:
For GitHub, see more information
[here](https://docs.github.com/en/code-security/getting-started/adding-a-security-policy-to-your-repository).
Signed-Releases:
id: 14
risk: High
tags: supply-chain, security, releases
repos: GitHub
Expand Down Expand Up @@ -649,6 +663,7 @@ checks:
If the source is hosted on GitHub, check out the steps
[here](https://wiki.debian.org/Creating%20signed%20GitHub%20releases).
Token-Permissions:
id: 15
risk: High
tags: supply-chain, security, infrastructure
repos: GitHub, local
Expand Down Expand Up @@ -702,6 +717,7 @@ checks:
the "Restrict permissions for GITHUB_TOKEN". You may also tick the "Pin actions to a full length commit SHA" to fix issues found
by the Pinned-dependencies check.
Vulnerabilities:
id: 16
risk: High
tags: supply-chain, security, vulnerabilities
repos: GitHub
Expand All @@ -727,6 +743,7 @@ checks:
[OSV-Scanner repository](https://github.com/google/osv-scanner#ignore-vulnerabilities-by-id).

Dangerous-Workflow:
id: 17
risk: Critical
tags: supply-chain, security, infrastructure
repos: GitHub, local
Expand Down Expand Up @@ -766,6 +783,7 @@ checks:
for information on avoiding and mitigating the risk of script injections.

License:
id: 18
risk: Low
tags: license
repos: GitHub, local
Expand Down Expand Up @@ -817,6 +835,7 @@ checks:
such as `LICENSES/Apache-2.0.txt`.

Webhooks:
id: 19
risk: Critical
tags: security, infrastructure
repos: GitHub
Expand Down
3 changes: 2 additions & 1 deletion docs/checks/internal/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ type Check struct {
Description string `yaml:"description"`
Tags string `yaml:"tags"`
Repos string `yaml:"repos"`
Remediation []string `yaml:"remediation"`
Name string `yaml:"-"`
URL string `yaml:"-"`
Remediation []string `yaml:"remediation"`
ID uint `yaml:"id"`
}

// Doc stores the documentation for all checks.
Expand Down
14 changes: 13 additions & 1 deletion docs/checks/internal/validate/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
var allowedRisks = map[string]bool{"Critical": true, "High": true, "Medium": true, "Low": true}

func main() {
CheckIDMap := make(map[uint]string)

m, err := docs.Read()
if err != nil {
panic(fmt.Sprintf("docs.Read: %v", err))
Expand All @@ -35,10 +37,20 @@ func main() {
if e != nil {
panic(fmt.Sprintf("GetCheck: %v: %s", e, check))
}

if check != c.GetName() {
panic(fmt.Sprintf("invalid checkName: %s != %s", check, c.GetName()))
}
id := c.GetID()
if id == 0 {
panic(fmt.Sprintf("invalid ID for %s Check: %d", check, id))
}
// check if id is already used with another check
dupe, ok := CheckIDMap[id]
if ok {
panic(fmt.Sprintf("Duplicate ID (%d) for Checks: %s and %s", id, check, dupe))
}
CheckIDMap[id] = check

if c.GetDescription() == "" {
panic(fmt.Sprintf("description for checkName: %s is empty", check))
}
Expand Down
19 changes: 11 additions & 8 deletions pkg/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
type jsonCheckResult struct {
Name string
Details []string
ID uint
Confidence int
Pass bool
}
ashearin marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -46,14 +47,13 @@ type jsonCheckDocumentationV2 struct {
Short string `json:"short"`
// Can be extended if needed.
}

//nolint:govet
type jsonCheckResultV2 struct {
Details []string `json:"details"`
Score int `json:"score"`
Doc jsonCheckDocumentationV2 `json:"documentation"`
Reason string `json:"reason"`
Name string `json:"name"`
Doc jsonCheckDocumentationV2 `json:"documentation"`
Details []string `json:"details"`
ID uint `json:"id"`
Score int `json:"score"`
}

type jsonRepoV2 struct {
Expand All @@ -77,14 +77,14 @@ func (s jsonFloatScore) MarshalJSON() ([]byte, error) {

// JSONScorecardResultV2 exports results as JSON for new detail format.
//
//nolint:govet

type JSONScorecardResultV2 struct {
ashearin marked this conversation as resolved.
Show resolved Hide resolved
Date string `json:"date"`
Repo jsonRepoV2 `json:"repo"`
Scorecard jsonScorecardV2 `json:"scorecard"`
AggregateScore jsonFloatScore `json:"score"`
Date string `json:"date"`
Checks []jsonCheckResultV2 `json:"checks"`
Metadata []string `json:"metadata"`
AggregateScore jsonFloatScore `json:"score"`
}

// AsJSON exports results as JSON for new detail format.
Expand All @@ -99,6 +99,7 @@ func (r *ScorecardResult) AsJSON(showDetails bool, logLevel log.Level, writer io

for _, checkResult := range r.Checks {
tmpResult := jsonCheckResult{
ID: checkResult.ID,
Name: checkResult.Name,
}
if showDetails {
Expand Down Expand Up @@ -154,6 +155,7 @@ func (r *ScorecardResult) AsJSON2(showDetails bool,
}

tmpResult := jsonCheckResultV2{
ID: doc.GetID(),
Name: checkResult.Name,
Doc: jsonCheckDocumentationV2{
URL: doc.GetDocumentationURL(r.Scorecard.CommitSHA),
Expand Down Expand Up @@ -216,6 +218,7 @@ func ExperimentalFromJSON2(r io.Reader) (result ScorecardResult, score float64,

for _, check := range jsr.Checks {
cr := checker.CheckResult{
ID: check.ID,
Name: check.Name,
Score: check.Score,
Reason: check.Reason,
Expand Down
Loading