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
8 changes: 4 additions & 4 deletions checker/check_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ 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 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
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
42 changes: 35 additions & 7 deletions docs/checks/internal/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@

# This is the source of truth for all check descriptions and remediation steps.
# 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 @@ -48,7 +50,9 @@ checks:
check simply provides insight into the project activity and maintenance
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 All @@ -74,7 +78,6 @@ checks:
a low score on this test. There are many ways to implement dependency updates,
and it is challenging for an automated tool like Scorecard to detect them all. A
low score is therefore not a definitive indication that the project is at risk.

remediation:
- >-
Sign up for automatic dependency updates with one of the previously listed dependency update tools and place
Expand All @@ -87,7 +90,9 @@ checks:
Unlike Dependabot, Renovate bot has support to migrate dockerfiles' dependencies from version pinning to hash pinning
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 @@ -132,13 +137,14 @@ checks:
- Generated documentation in source repositories. Generated documentation is
intended for use by humans (not computers) who can evaluate the context.
Thus, generated documentation doesn't pose the same level of risk.

remediation:
- >-
Remove the generated executable artifacts from the repository.
- >-
Build from source.

Branch-Protection:
id: 4
risk: High
tags: supply-chain, security, source-code, code-reviews
repos: GitHub, GitLab
Expand Down Expand Up @@ -217,14 +223,15 @@ checks:

GitLab Integration Status:
- GitLab associates releases with commits and not with the branch. Releases are ignored in this portion of the scoring.

remediation:
- >-
Enable branch protection settings in your source hosting provider to
avoid force pushes or deletion of your important branches.
- >-
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 @@ -257,7 +264,9 @@ checks:
Integrate those scripts with a CI/CD platform that runs it on every pull
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 @@ -283,7 +292,9 @@ checks:
remediation:
- >-
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 @@ -337,7 +348,6 @@ checks:
notice unintentional vulnerabilities or malicious code, be colluding with a
malicious developer, or even be the same person (using a "[sock
puppet](https://en.wikipedia.org/wiki/Sock_puppet_account)" account).

remediation:
- >-
If the project has only one contributor, or does not have enough
Expand All @@ -356,7 +366,9 @@ 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 @@ -387,7 +399,9 @@ checks:
if they have not already. Otherwise, there is no remediation for this check;
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 @@ -417,7 +431,9 @@ 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 @@ -461,7 +477,9 @@ checks:
remediation:
- 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 @@ -527,7 +545,9 @@ checks:
by the Token-Permissions check.
- >-
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 @@ -560,6 +580,7 @@ checks:
[here](https://github.com/github/codeql-action#usage).

SBOM:
id: 13
risk: Medium
short: Determines if the project maintains a Software Bill of Materials.
repos: GitHub, Gitlab
Expand Down Expand Up @@ -599,6 +620,7 @@ checks:
Alternatively, there are other tools available to generate [CycloneDX](https://cyclonedx.org/tool-center/) and [SPDX](https://spdx.dev/use/tools/) SBOMs.

Security-Policy:
id: 14
risk: Medium
short: Determines if the project has published a security policy.
repos: GitHub
Expand Down Expand Up @@ -635,7 +657,6 @@ checks:
`vuln` and as in "Vulnerability" or "vulnerabilities";
`disclos` as "Disclosure" or "disclose";
and numbers which convey expectations of times, e.g., 30 days or 90 days

remediation:
- >-
Place a security policy file `SECURITY.md` in the root directory of your
Expand All @@ -649,7 +670,9 @@ 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: 15
risk: High
tags: supply-chain, security, releases
repos: GitHub
Expand Down Expand Up @@ -688,7 +711,9 @@ checks:
- >-
If the source is hosted on GitHub, check out the steps
[here](https://wiki.debian.org/Creating%20signed%20GitHub%20releases).

Token-Permissions:
id: 16
risk: High
tags: supply-chain, security, infrastructure
repos: GitHub, local
Expand Down Expand Up @@ -729,7 +754,6 @@ checks:

The check cannot detect if the "read-only" GitHub permission setting is
enabled, as there is no API available.

remediation:
- >-
Set top-level permissions as `read-all` or `contents: read` as described in
Expand All @@ -741,7 +765,9 @@ checks:
To help determine the permissions needed for your workflows, you may use [StepSecurity's online tool](https://app.stepsecurity.io/secureworkflow/) by ticking
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: 17
risk: High
tags: supply-chain, security, vulnerabilities
repos: GitHub
Expand All @@ -767,6 +793,7 @@ checks:
[OSV-Scanner repository](https://github.com/google/osv-scanner#ignore-vulnerabilities-by-id).

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

License:
id: 19
risk: Low
tags: license
repos: GitHub, local
Expand Down Expand Up @@ -839,7 +867,6 @@ checks:
files in a `LICENSES` directory (6/10 points)
- The detected file is at the top-level directory (3/10 points)
- A [FSF or OSI](https://spdx.org/licenses/) license is specified (1/10 points)

remediation:
- >-
Determine [which license](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/licensing-a-repository)
Expand All @@ -858,6 +885,7 @@ checks:
such as `LICENSES/Apache-2.0.txt`.

Webhooks:
id: 20
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
7 changes: 5 additions & 2 deletions pkg/scorecard/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type jsonCheckResultV2 struct {
Name string `json:"name"`
Doc jsonCheckDocumentationV2 `json:"documentation"`
Annotations []string `json:"annotations,omitempty"`
ID uint `json:"id"`
}

type jsonRepoV2 struct {
Expand All @@ -80,12 +81,12 @@ func (s jsonFloatScore) MarshalJSON() ([]byte, error) {
//
//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"`
}

// AsJSON2ResultOption provides configuration options for JSON2 Scorecard results.
Expand Down Expand Up @@ -167,6 +168,7 @@ func (r *Result) AsJSON2(writer io.Writer, checkDocs docs.Doc, opt *AsJSON2Resul
}

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

for _, check := range jsr.Checks {
cr := checker.CheckResult{
ID: check.ID,
Name: check.Name,
Score: check.Score,
Reason: check.Reason,
Expand Down
5 changes: 5 additions & 0 deletions pkg/scorecard/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
Loading