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 initial Maintainers Annotation parsing #3905

Merged
merged 72 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
389e53b
feat: Get maintainers annotation from repo
gabibguti Feb 27, 2024
de0d98b
feat: Hand off maintainers annotation for SARIF
gabibguti Feb 27, 2024
69c5e28
feat: If check is annotated, skip in SARIF output
gabibguti Feb 27, 2024
a345120
feat: Add other annotation reasons
gabibguti Feb 27, 2024
df0427f
feat: Add options to show maintainers annotations in output
gabibguti Feb 28, 2024
b21bdf7
feat: Output maintainers annotations in JSON
gabibguti Feb 28, 2024
2ad4fab
fix: Remove unnecessary maintainers annotation param in SARIF
gabibguti Feb 28, 2024
202005f
feat: Output maintainers annotations in string default result
gabibguti Feb 28, 2024
dbea46b
feat: Ignore annotation if check has max score
gabibguti Feb 28, 2024
3df6791
doc: Add documentation for maintainers annotation
gabibguti Feb 28, 2024
765c63d
refactor: A maintainers annotation obj can verify if a check is exempted
gabibguti Feb 28, 2024
6fe5b31
refactor: Get annotations function can be private
gabibguti Feb 28, 2024
e41d439
refactor: Find scorecard.yml file in the repository's root
gabibguti Mar 4, 2024
e23bf4a
fix: A check should know if it's exempted or not
gabibguti Mar 4, 2024
ae9f55a
fix: Maintainers annotation can only be used in experimental mode
gabibguti Mar 4, 2024
f55bf70
fix: Ignore if scorecard.yml does not exist
gabibguti Mar 4, 2024
946bc1c
fix: Remove unnecessary maintainers annotation param
gabibguti Mar 4, 2024
0104fac
docs: Move complete mantainers annotation doc to feature folder
gabibguti Mar 4, 2024
6033a56
fix: Error logs
gabibguti Mar 5, 2024
b30dbf4
refactor: Rename AnnotationReason to Reason
gabibguti Mar 5, 2024
d3ef155
refactor: Reason documentation
gabibguti Mar 5, 2024
258e896
refactor: Rename ScorecardYml to ScorecardConfig
gabibguti Mar 5, 2024
b2ff1ad
fix: Check name comparison
gabibguti Mar 5, 2024
362c05f
refactor: Rename maintainers annotation folder/file to config
gabibguti Mar 12, 2024
472601e
refactor: Rename and simplify parsing the config
gabibguti Mar 12, 2024
6d60fc3
refactor: Check parses its reasons
gabibguti Mar 12, 2024
84d659d
fix: Is check exempted
gabibguti Mar 12, 2024
cd9266a
refactor: Rename maintainers annotation to annotations
gabibguti Mar 12, 2024
696b76b
refactor: Separate annotations content from config parsing
gabibguti Mar 12, 2024
37d77b2
fix: Omit empty annotations in JSON results
gabibguti Mar 12, 2024
4ae0b28
refactor: Read config file content
gabibguti Mar 19, 2024
6edd500
refactor: JSON2 result options
gabibguti Mar 19, 2024
cbda605
refactor: String result options
gabibguti Mar 19, 2024
54fde87
test: Mock GetFileReader
gabibguti Mar 29, 2024
de8a965
test: Annotation on Binary-Artifacts check
gabibguti Mar 29, 2024
952ca41
feat: Validate annotated checks
gabibguti Mar 29, 2024
e5a6f6c
test: Annotating all checks
gabibguti Mar 29, 2024
bdb4407
feat: Validate annotated reasons
gabibguti Mar 29, 2024
2de2869
test: Annotating all reasons
gabibguti Mar 29, 2024
0e70141
test: Multiple annotations
gabibguti Mar 29, 2024
17e58c3
test: Binary-Artifacts exempted for testing
gabibguti Apr 1, 2024
d9ad34c
test: Binary-Artifacts not exempted
gabibguti Apr 1, 2024
b171872
test: No checks exempted
gabibguti Apr 1, 2024
ceab1dd
test: Exemption is outdated
gabibguti Apr 1, 2024
ea1ca56
test: Improve reasons error comparison
gabibguti Apr 1, 2024
b7a5d3f
test: Multiple exemption reasons in a single annotation
gabibguti Apr 1, 2024
b3950ab
test: Multiple exemption reasons across annotations
gabibguti Apr 1, 2024
8ca4fc7
fix: cmd show annotations flag doc
gabibguti Apr 1, 2024
51b5fd1
test: Add show annotations flag
gabibguti Apr 1, 2024
53ac96f
fix: Remove unnecessary function
gabibguti Apr 1, 2024
9d17753
test: Annotations string format
gabibguti Apr 1, 2024
9006269
test: Annotations json format
gabibguti Apr 1, 2024
05f9aed
fix: Linter fallthrough
gabibguti Apr 1, 2024
fe07f91
fix: Linter imports
gabibguti Apr 1, 2024
1a59b82
fix: Linter unnecessart struct type declaration
gabibguti Apr 1, 2024
5cce247
fix: Linter append combine
gabibguti Apr 1, 2024
a45a4fc
fix: Linter struct memory
gabibguti Apr 1, 2024
443a009
fix: Linter improve error msg in run scorecard
gabibguti Apr 1, 2024
b357f17
fix: Linter dynamic errors
gabibguti Apr 1, 2024
494b6db
docs: Disable security alerts on SARIF output
gabibguti Apr 1, 2024
ad2a9b8
docs: Redirect to configuration doc on main README
gabibguti Apr 1, 2024
2b73436
test: Invalid check in annotations
gabibguti Apr 1, 2024
8234d6d
test: Invalid reason in annotations
gabibguti Apr 1, 2024
997f030
test: Exempt check on SARIF output clears runs
gabibguti Apr 1, 2024
a36843e
test: Add check1 annotations json
gabibguti Apr 1, 2024
05fb5b1
fix: On parse error return empty config file not a "dirty" one
gabibguti Apr 22, 2024
63b480c
fix: On parse config error continue execution
gabibguti Apr 22, 2024
ff88498
Merge branch 'main' into feat/maintainers-annotation
gabibguti Apr 22, 2024
f81871e
fix: Merge conflics importing rules
gabibguti Apr 22, 2024
19f8ae1
fix: Readd is experimental enabled method
gabibguti Apr 22, 2024
e7cd246
feat: Wrap config parse under experimental flag
gabibguti Apr 23, 2024
b28295e
fix unit test by removing unused mock call
spencerschrock Apr 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,13 @@ RESULTS
|---------|------------------------|--------------------------------|--------------------------------|---------------------------------------------------------------------------|
```

##### Showing Maintainers Annotations (Experimental)

To see the maintainers annotations for each check, use the `--show-annotations` option.

For more information on how to configure annotations or what are the available annotations, see [the configuration doc](config/README.md).


##### Using a GitLab Repository

To run Scorecard on a GitLab repository, you must create a [GitLab Access Token](https://gitlab.com/-/profile/personal_access_tokens) with the following permissions:
Expand Down
31 changes: 31 additions & 0 deletions checker/check_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import (
"errors"
"fmt"
"math"
"strings"

"github.com/ossf/scorecard/v5/config"
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/finding"
)
Expand Down Expand Up @@ -254,3 +256,32 @@ func LogFinding(dl DetailLogger, f *finding.Finding, level DetailType) {
dl.Warn(&lm)
}
}

// IsExempted verifies if a given check in the results is exempted in annotations.
func (check *CheckResult) IsExempted(c config.Config) (bool, []string) {
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
// If check has a maximum score, then there it doesn't make sense anymore to reason the check
// This may happen if the check score was once low but then the problem was fixed on Scorecard side
// or on the maintainers side
if check.Score == MaxResultScore {
return false, nil
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
}

// Collect all annotation reasons for this check
var reasons []string

// For all annotations
for _, annotation := range c.Annotations {
for _, checkName := range annotation.Checks {
// If check is in this annotation
if strings.EqualFold(checkName, check.Name) {
// Get all the reasons for this annotation
for _, reasonGroup := range annotation.Reasons {
reasons = append(reasons, reasonGroup.Reason.Doc())
}
}
}
}

// A check is considered exempted if it has annotation reasons
return (len(reasons) > 0), reasons
}
193 changes: 193 additions & 0 deletions checker/check_result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

"github.com/ossf/scorecard/v5/config"
sce "github.com/ossf/scorecard/v5/errors"
)

Expand Down Expand Up @@ -809,3 +810,195 @@ func TestCreateRuntimeErrorResult(t *testing.T) {
})
}
}

func TestIsExempted(t *testing.T) {
t.Parallel()
type args struct {
check CheckResult
config config.Config
}
type want struct {
reasons []config.Reason
isExempted bool
}
tests := []struct {
name string
args args
want want
}{
{
name: "Binary-Artifacts exempted for testing",
args: args{
check: CheckResult{
Name: "Binary-Artifacts",
Score: 0,
},
config: config.Config{
Annotations: []config.Annotation{
{
Checks: []string{"binary-artifacts"},
Reasons: []config.ReasonGroup{
{Reason: "test-data"},
},
},
},
},
},
want: want{
isExempted: true,
reasons: []config.Reason{
config.TestData,
},
},
},
{
name: "Binary-Artifacts not exempted",
args: args{
check: CheckResult{
Name: "Binary-Artifacts",
Score: 0,
},
config: config.Config{
Annotations: []config.Annotation{
{
Checks: []string{"pinned-dependencies"},
Reasons: []config.ReasonGroup{
{Reason: "test-data"},
},
},
{
Checks: []string{"branch-protection"},
Reasons: []config.ReasonGroup{
{Reason: "not-applicable"},
},
},
},
},
},
want: want{
isExempted: false,
},
},
{
name: "No checks exempted",
args: args{
check: CheckResult{
Name: "Binary-Artifacts",
Score: 0,
},
config: config.Config{},
},
want: want{
isExempted: false,
},
},
{
name: "Exemption is outdated",
args: args{
check: CheckResult{
Name: "Binary-Artifacts",
Score: 10,
},
config: config.Config{
Annotations: []config.Annotation{
{
Checks: []string{"binary-artifacts"},
Reasons: []config.ReasonGroup{
{Reason: "test-data"},
},
},
},
},
},
want: want{
isExempted: false,
},
},
{
name: "Multiple exemption reasons in a single annotation",
args: args{
check: CheckResult{
Name: "Binary-Artifacts",
Score: 0,
},
config: config.Config{
Annotations: []config.Annotation{
{
Checks: []string{"binary-artifacts"},
Reasons: []config.ReasonGroup{
{Reason: "test-data"},
{Reason: "remediated"},
},
},
},
},
},
want: want{
isExempted: true,
reasons: []config.Reason{
config.TestData,
config.Remediated,
},
},
},
{
name: "Multiple exemption reasons across annotations",
args: args{
check: CheckResult{
Name: "Binary-Artifacts",
Score: 0,
},
config: config.Config{
Annotations: []config.Annotation{
{
Checks: []string{
"binary-artifacts",
"pinned-dependencies",
},
Reasons: []config.ReasonGroup{
{Reason: "test-data"},
},
},
{
Checks: []string{
"binary-artifacts",
"dangerous-workflow",
},
Reasons: []config.ReasonGroup{
{Reason: "remediated"},
},
},
},
},
},
want: want{
isExempted: true,
reasons: []config.Reason{
config.TestData,
config.Remediated,
},
},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
isExempted, reasons := tt.args.check.IsExempted(tt.args.config)
if isExempted != tt.want.isExempted {
t.Fatalf("IsExempted() = %v, want %v", isExempted, tt.want.isExempted)
}
wantReasons := []string{}
if tt.want.reasons != nil {
for _, r := range tt.want.reasons {
wantReasons = append(wantReasons, r.Doc())
}
} else {
wantReasons = nil
}
if cmp.Equal(reasons, wantReasons) == false {
t.Fatalf("Reasons for IsExempted() = %v, want %v", reasons, wantReasons)
}
})
}
}
6 changes: 5 additions & 1 deletion cmd/internal/scdiff/app/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,9 @@ func JSON(r *pkg.ScorecardResult, w io.Writer) error {
return err
}
Normalize(r)
return r.AsJSON2(details, logLevel, docs, w)
o := pkg.AsJSON2ResultOption{
Details: details,
LogLevel: logLevel,
}
return r.AsJSON2(w, docs, o)
}
2 changes: 1 addition & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import (
const (
scorecardLong = "A program that shows the OpenSSF scorecard for an open source software."
scorecardUse = `./scorecard (--repo=<repo> | --local=<folder> | --{npm,pypi,rubygems,nuget}=<package_name>)
[--checks=check1,...] [--show-details]`
[--checks=check1,...] [--show-details] [--show-annotations]`
scorecardShort = "OpenSSF Scorecard"
)

Expand Down
63 changes: 63 additions & 0 deletions config/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Maintainers Annotations

Maintainers Annotations is an experimental feature to let maintainers add annotations to Scorecard checks.

## Showing Maintainers Annotations

To see the maintainers annotations for each check on Scorecard results, use the `--show-annotations` option.

## Adding Annotations

To add annotations to your repository, create a `scorecard.yml` file in the root of your repository.

The file structure is as follows:
```yml
exemptions:
- checks:
- binary-artifacts
annotations:
# the binary files are only used for testing
- annotation: test-data
- checks:
- dangerous-workflow
annotations:
# the workflow is dangerous but only run under maintainers verification and approval
- annotation: remediated
```

You can annotate multiple checks at a time:
```yml
exemptions:
- checks:
- binary-artifacts
- pinned-dependencies
annotations:
# the binary files and files with unpinned dependencies are only used for testing
- annotation: test-data
```

And also provide multiple annotations for checks:
```yml
exemptions:
- checks:
- binary-artifacts
annotations:
# test.exe is only used for testing
- annotation: test-data
# dependency.exe is needed and it's used but the binary signature is verified
- annotation: remediated
```

The available checks are the Scorecard checks in lower case e.g. Binary-Artifacts is `binary-artifacts`.

The annotations are predefined as shown in the table below:

| Annotation | Description | Example |
|------------|-------------|---------|
| test-data | To annotate when a check or probe is targeting a danger in files or code snippets only used for test or example purposes. | The binary files are only used for testing. |
| remediated | To annotate when a check or probe correctly identified a danger and, even though the danger is necessary, a remediation was already applied. | A workflow is dangerous but only run under maintainers verification and approval, or a binary is needed but it is signed or has provenance. |
| not-applicable | To annotate when a check or probe is not applicable for the case. | The dependencies should not be pinned because the project is a library. |
| not-supported | To annotate when the maintainer fulfills a check or probe in a way that is not supported by Scorecard. | Clang-Tidy is used as SAST tool but not identified because its not supported. |
| not-detected | To annotate when the maintainer fulfills a check or probe in a way that is supported by Scorecard but not identified. | Dependabot is configured in the repository settings and not in a file. |

These annotations, when displayed in Scorecard results are parsed to a human-readable format that is similar to the annotation description described in the table above.
Loading
Loading