From 72e697786c7a66ed8967c93557a7c9ead3952a34 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 2 May 2023 10:00:27 -0500 Subject: [PATCH 1/6] :seedling: Bump tj-actions/changed-files from 35.9.1 to 35.9.2 (#2933) Bumps [tj-actions/changed-files](https://github.com/tj-actions/changed-files) from 35.9.1 to 35.9.2. - [Release notes](https://github.com/tj-actions/changed-files/releases) - [Changelog](https://github.com/tj-actions/changed-files/blob/main/HISTORY.md) - [Commits](https://github.com/tj-actions/changed-files/compare/4a0aac0d19aa2838c6741fdf95a5276390418dc2...b2d17f51244a144849c6b37a3a6791b98a51d86f) --- updated-dependencies: - dependency-name: tj-actions/changed-files dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/docker.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 8703ba4d244e..59e5befcebfc 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -41,7 +41,7 @@ jobs: fetch-depth: 2 - id: files name: Get changed files - uses: tj-actions/changed-files@4a0aac0d19aa2838c6741fdf95a5276390418dc2 #v35.9.1 + uses: tj-actions/changed-files@b2d17f51244a144849c6b37a3a6791b98a51d86f #v35.9.2 with: files_ignore: '**.md' - id: docs_only_check From 700faf1d2d6a023ba02bdcce591101f9e22f18c1 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Tue, 2 May 2023 11:32:40 -0700 Subject: [PATCH 2/6] :bug: Add nil check before accessing a step's uses value. (#2935) Signed-off-by: Spencer Schrock --- checks/sast.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks/sast.go b/checks/sast.go index a9f3d0ad2131..423767f842e3 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -262,7 +262,7 @@ var searchGitHubActionWorkflowCodeQL fileparser.DoWhileTrueOnFileContent = func( for _, job := range workflow.Jobs { for _, step := range job.Steps { e, ok := step.Exec.(*actionlint.ExecAction) - if !ok { + if !ok || e == nil || e.Uses == nil { continue } // Parse out repo / SHA. From a4da39a7798ef1391170627e62b9844d9b47bfdc Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Tue, 2 May 2023 17:42:32 -0700 Subject: [PATCH 3/6] =?UTF-8?q?=E2=9C=A8=20[experimental]=20Create=20probe?= =?UTF-8?q?s=20within=20findings=20(#2919)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon * update Signed-off-by: laurentsimon --------- Signed-off-by: laurentsimon --- ...gitHubWorkflowPermissionsStepsNoWrite.yml} | 5 +- ...> gitHubWorkflowPermissionsTopNoWrite.yml} | 7 +- checks/evaluation/permissions/permissions.go | 66 ++++--- checks/permissions_test.go | 6 +- finding/finding.go | 157 ++++++++++++--- finding/finding_test.go | 102 +++++----- finding/probe/probe.go | 183 ++++++++++++++++++ finding/probe/probe_test.go | 109 +++++++++++ finding/probe/testdata/all-fields.yml | 16 ++ .../testdata/invalid-effort.yml} | 5 +- .../testdata/missing-id.yml} | 2 - finding/testdata/effort-high.yml | 3 +- finding/testdata/effort-low.yml | 3 +- finding/testdata/metadata-variables.yml | 7 +- pkg/common.go | 4 +- pkg/sarif.go | 2 +- 16 files changed, 545 insertions(+), 132 deletions(-) rename checks/evaluation/permissions/{GitHubWorkflowPermissionsStepsNoWrite.yml => gitHubWorkflowPermissionsStepsNoWrite.yml} (87%) rename checks/evaluation/permissions/{GitHubWorkflowPermissionsTopNoWrite.yml => gitHubWorkflowPermissionsTopNoWrite.yml} (85%) create mode 100644 finding/probe/probe.go create mode 100644 finding/probe/probe_test.go create mode 100644 finding/probe/testdata/all-fields.yml rename finding/{testdata/risk-high.yml => probe/testdata/invalid-effort.yml} (85%) rename finding/{testdata/risk-low.yml => probe/testdata/missing-id.yml} (90%) diff --git a/checks/evaluation/permissions/GitHubWorkflowPermissionsStepsNoWrite.yml b/checks/evaluation/permissions/gitHubWorkflowPermissionsStepsNoWrite.yml similarity index 87% rename from checks/evaluation/permissions/GitHubWorkflowPermissionsStepsNoWrite.yml rename to checks/evaluation/permissions/gitHubWorkflowPermissionsStepsNoWrite.yml index d4d81e22724c..171f8503fb11 100644 --- a/checks/evaluation/permissions/GitHubWorkflowPermissionsStepsNoWrite.yml +++ b/checks/evaluation/permissions/gitHubWorkflowPermissionsStepsNoWrite.yml @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +id: gitHubWorkflowPermissionsStepsNoWrite short: Checks that GitHub workflows do not have steps with dangerous write permissions -desc: This rule checks that GitHub workflows do not have steps with dangerous write permissions motivation: > Even with permissions default set to read, some scopes having write permissions in their steps brings incurs a risk to the project. By giving write permission to the Actions you call in jobs, an external Action you call could abuse them. Depending on the permissions, @@ -21,10 +21,9 @@ motivation: > For more information about the scopes and the vulnerabilities involved, see https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions. implementation: > - The rule is implemented by checking whether the `permissions` keyword is given non-write permissions for the following + The probe is implemented by checking whether the `permissions` keyword is given non-write permissions for the following scopes: `statuses`, `checks`, `security-events`, `deployments`, `contents`, `packages`, `actions`. Write permissions given to recognized packaging actions or commands are allowed and are considered an acceptable risk. -risk: Medium remediation: effort: High text: diff --git a/checks/evaluation/permissions/GitHubWorkflowPermissionsTopNoWrite.yml b/checks/evaluation/permissions/gitHubWorkflowPermissionsTopNoWrite.yml similarity index 85% rename from checks/evaluation/permissions/GitHubWorkflowPermissionsTopNoWrite.yml rename to checks/evaluation/permissions/gitHubWorkflowPermissionsTopNoWrite.yml index e15475925cf5..65c89173c922 100644 --- a/checks/evaluation/permissions/GitHubWorkflowPermissionsTopNoWrite.yml +++ b/checks/evaluation/permissions/gitHubWorkflowPermissionsTopNoWrite.yml @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +id: gitHubWorkflowPermissionsTopNoWrite short: Checks that GitHub workflows do not have default write permissions -desc: This rule checks that GitHub workflows do not have default write permissions motivation: > If no permissions are declared, a workflow's GitHub token's permissions default to write for all scopes. This include write permissions to push to the repository, to read encrypted secrets, etc. @@ -21,16 +21,15 @@ motivation: > implementation: > The rule is implemented by checking whether the `permissions` keyword is defined at the top of the workflow, and that no write permissions are given. -risk: High remediation: effort: Low text: - - Visit https://app.stepsecurity.io/secureworkflow/${{ repo }}/${{ workflow }}/${{ branch }}?enable=permissions + - Visit https://app.stepsecurity.io/secureworkflow/${{ metadata.repo }}/${{ metadata.workflow }}/${{ metadata.branch }}?enable=permissions - Tick the 'Restrict permissions for GITHUB_TOKEN' - Untick other options - "NOTE: If you want to resolve multiple issues at once, you can visit https://app.stepsecurity.io/securerepo instead." markdown: - - Visit [https://app.stepsecurity.io/secureworkflow](https://app.stepsecurity.io/secureworkflow/${{ repo }}/${{ workflow }}/${{ branch }}?enable=permissions). + - Visit [https://app.stepsecurity.io/secureworkflow](https://app.stepsecurity.io/secureworkflow/${{ metadata.repo }}/${{ metadata.workflow }}/${{ metadata.branch }}?enable=permissions). - Tick the 'Restrict permissions for GITHUB_TOKEN' - Untick other options - "NOTE: If you want to resolve multiple issues at once, you can visit [https://app.stepsecurity.io/securerepo](https://app.stepsecurity.io/securerepo) instead." diff --git a/checks/evaluation/permissions/permissions.go b/checks/evaluation/permissions/permissions.go index a661c372dae5..ed8b0cbc21d8 100644 --- a/checks/evaluation/permissions/permissions.go +++ b/checks/evaluation/permissions/permissions.go @@ -26,13 +26,18 @@ import ( ) //go:embed *.yml -var rules embed.FS +var probes embed.FS type permissions struct { topLevelWritePermissions map[string]bool jobLevelWritePermissions map[string]bool } +var ( + stepsNoWriteID = "gitHubWorkflowPermissionsStepsNoWrite" + topNoWriteID = "gitHubWorkflowPermissionsTopNoWrite" +) + // TokenPermissions applies the score policy for the Token-Permissions check. func TokenPermissions(name string, c *checker.CheckRequest, r *checker.TokenPermissionsData) checker.CheckResult { if r == nil { @@ -67,9 +72,9 @@ func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckReq dl := c.Dlogger //nolint:errcheck remediationMetadata, _ := remediation.New(c) - negativeRuleResults := map[string]bool{ - "GitHubWorkflowPermissionsStepsNoWrite": false, - "GitHubWorkflowPermissionsTopNoWrite": false, + negativeProbeResults := map[string]bool{ + stepsNoWriteID: false, + topNoWriteID: false, } for _, r := range results.TokenPermissions { @@ -77,14 +82,12 @@ func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckReq if r.File != nil { loc = &finding.Location{ Type: r.File.Type, - Value: r.File.Path, + Path: r.File.Path, LineStart: &r.File.Offset, } if r.File.Snippet != "" { loc.Snippet = &r.File.Snippet } - - loc.Value = r.File.Path } text, err := createText(r) @@ -112,7 +115,7 @@ func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckReq // We warn only for top-level. if *r.LocationType == checker.PermissionLocationTop { - warnWithRemediation(dl, msg, remediationMetadata, loc, negativeRuleResults) + warnWithRemediation(dl, msg, remediationMetadata, loc, negativeProbeResults) } else { dl.Debug(msg) } @@ -123,7 +126,7 @@ func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckReq } case checker.PermissionLevelWrite: - warnWithRemediation(dl, msg, remediationMetadata, loc, negativeRuleResults) + warnWithRemediation(dl, msg, remediationMetadata, loc, negativeProbeResults) // Group results by workflow name for score computation. if err := updateWorkflowHashMap(hm, r); err != nil { @@ -132,7 +135,7 @@ func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckReq } } - if err := reportDefaultFindings(results, c.Dlogger, negativeRuleResults); err != nil { + if err := reportDefaultFindings(results, c.Dlogger, negativeProbeResults); err != nil { return checker.InconclusiveResultScore, err } @@ -140,17 +143,18 @@ func applyScorePolicy(results *checker.TokenPermissionsData, c *checker.CheckReq } func reportDefaultFindings(results *checker.TokenPermissionsData, - dl checker.DetailLogger, negativeRuleResults map[string]bool, + dl checker.DetailLogger, negativeProbeResults map[string]bool, ) error { + // TODO(#2928): re-visit the need for NotApplicable outcome. // No workflow files exist. if len(results.TokenPermissions) == 0 { text := "no workflows found in the repository" - if err := reportFinding("GitHubWorkflowPermissionsStepsNoWrite", - text, finding.OutcomeNotApplicable, dl); err != nil { + if err := reportFinding(stepsNoWriteID, + text, finding.OutcomeNotAvailable, dl); err != nil { return err } - if err := reportFinding("GitHubWorkflowPermissionsTopNoWrite", - text, finding.OutcomeNotApplicable, dl); err != nil { + if err := reportFinding(topNoWriteID, + text, finding.OutcomeNotAvailable, dl); err != nil { return err } return nil @@ -158,12 +162,12 @@ func reportDefaultFindings(results *checker.TokenPermissionsData, // Workflow files found, report positive findings if no // negative findings were found. - // NOTE: we don't consider rule `GitHubWorkflowPermissionsTopNoWrite` + // NOTE: we don't consider probe `topNoWriteID` // because positive results are already reported. - found := negativeRuleResults["GitHubWorkflowPermissionsStepsNoWrite"] + found := negativeProbeResults[stepsNoWriteID] if !found { text := fmt.Sprintf("no %s write permissions found", checker.PermissionLocationJob) - if err := reportFinding("GitHubWorkflowPermissionsStepsNoWrite", + if err := reportFinding(stepsNoWriteID, text, finding.OutcomePositive, dl); err != nil { return err } @@ -172,8 +176,12 @@ func reportDefaultFindings(results *checker.TokenPermissionsData, return nil } -func reportFinding(rule, text string, o finding.Outcome, dl checker.DetailLogger) error { - f, err := finding.New(rules, rule) +func reportFinding(probe, text string, o finding.Outcome, dl checker.DetailLogger) error { + content, err := probes.ReadFile(probe + ".yml") + if err != nil { + return fmt.Errorf("%w", err) + } + f, err := finding.FromBytes(content, probe) if err != nil { return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) } @@ -185,11 +193,15 @@ func reportFinding(rule, text string, o finding.Outcome, dl checker.DetailLogger } func createLogMsg(loct *checker.PermissionLocation) (*checker.LogMessage, error) { - Rule := "GitHubWorkflowPermissionsStepsNoWrite" + probe := stepsNoWriteID if loct == nil || *loct == checker.PermissionLocationTop { - Rule = "GitHubWorkflowPermissionsTopNoWrite" + probe = topNoWriteID + } + content, err := probes.ReadFile(probe + ".yml") + if err != nil { + return nil, fmt.Errorf("%w", err) } - f, err := finding.New(rules, Rule) + f, err := finding.FromBytes(content, probe) if err != nil { return nil, sce.WithMessage(sce.ErrScorecardInternal, err.Error()) @@ -201,19 +213,19 @@ func createLogMsg(loct *checker.PermissionLocation) (*checker.LogMessage, error) func warnWithRemediation(logger checker.DetailLogger, msg *checker.LogMessage, rem *remediation.RemediationMetadata, loc *finding.Location, - negativeRuleResults map[string]bool, + negativeProbeResults map[string]bool, ) { - if loc != nil && loc.Value != "" { + if loc != nil && loc.Path != "" { msg.Finding = msg.Finding.WithRemediationMetadata(map[string]string{ "repo": rem.Repo, "branch": rem.Branch, - "workflow": strings.TrimPrefix(loc.Value, ".github/workflows/"), + "workflow": strings.TrimPrefix(loc.Path, ".github/workflows/"), }) } logger.Warn(msg) // Record that we found a negative result. - negativeRuleResults[msg.Finding.Rule] = true + negativeProbeResults[msg.Finding.Probe] = true } func recordPermissionWrite(hm map[string]permissions, path string, diff --git a/checks/permissions_test.go b/checks/permissions_test.go index 066ed36f8187..189156f3c252 100644 --- a/checks/permissions_test.go +++ b/checks/permissions_test.go @@ -319,8 +319,8 @@ func TestGithubTokenPermissions(t *testing.T) { Error: nil, Score: checker.MaxResultScore, NumberOfWarn: 0, - NumberOfInfo: 2, // This is constant. - NumberOfDebug: 8, // This is 4 + (number of actions) + NumberOfInfo: 2, // This is constant. + NumberOfDebug: 8, // This is 4 + (number of actions) }, }, { @@ -488,7 +488,7 @@ func TestGithubTokenPermissionsLineNumber(t *testing.T) { logMessage.Finding.Location != nil && logMessage.Finding.Location.LineStart != nil && *logMessage.Finding.Location.LineStart == expectedLog.lineNumber && - logMessage.Finding.Location.Value == p && + logMessage.Finding.Location.Path == p && logType == checker.DetailWarn } if !scut.ValidateLogMessage(isExpectedLog, &dl) { diff --git a/finding/finding.go b/finding/finding.go index d9c2d72d77dc..c2c23f387fd5 100644 --- a/finding/finding.go +++ b/finding/finding.go @@ -16,10 +16,13 @@ package finding import ( "embed" + "errors" "fmt" "strings" - "github.com/ossf/scorecard/v4/rule" + "gopkg.in/yaml.v3" + + "github.com/ossf/scorecard/v4/finding/probe" ) // FileType is the type of a file. @@ -42,7 +45,7 @@ const ( // nolint: govet type Location struct { Type FileType `json:"type"` - Value string `json:"value"` + Path string `json:"path"` LineStart *uint `json:"lineStart,omitempty"` LineEnd *uint `json:"lineEnd,omitempty"` Snippet *string `json:"snippet,omitempty"` @@ -51,13 +54,33 @@ type Location struct { // Outcome is the result of a finding. type Outcome int +// TODO(#2928): re-visit the finding definitions. const ( + // NOTE: The additional '_' are intended for future use. + // This allows adding outcomes without breaking the values + // of existing outcomes. // OutcomeNegative indicates a negative outcome. OutcomeNegative Outcome = iota + _ + _ + _ + // OutcomeNotAvailable indicates an unavailable outcome, + // typically because an API call did not return an answer. + OutcomeNotAvailable + _ + _ + _ + // OutcomeError indicates an errors while running. + // The results could not be determined. + OutcomeError + _ + _ + _ // OutcomePositive indicates a positive outcome. OutcomePositive - // OutcomeNotApplicable indicates a non-applicable outcome. - OutcomeNotApplicable + _ + _ + _ // OutcomeNotSupported indicates a non-supported outcome. OutcomeNotSupported ) @@ -65,32 +88,92 @@ const ( // Finding represents a finding. // nolint: govet type Finding struct { - Rule string `json:"rule"` - Outcome Outcome `json:"outcome"` - Risk rule.Risk `json:"risk"` - Message string `json:"message"` - Location *Location `json:"location,omitempty"` - Remediation *rule.Remediation `json:"remediation,omitempty"` + Probe string `json:"probe"` + Outcome Outcome `json:"outcome"` + Message string `json:"message"` + Location *Location `json:"location,omitempty"` + Remediation *probe.Remediation `json:"remediation,omitempty"` } -// New creates a new finding. -func New(loc embed.FS, ruleID string) (*Finding, error) { - r, err := rule.New(loc, ruleID) +// AnonymousFinding is a finding without a corerpsonding probe ID. +type AnonymousFinding struct { + Finding + // Remove the probe ID from + // the structure until the probes are GA. + Probe string `json:"probe,omitempty"` +} + +var errInvalid = errors.New("invalid") + +// FromBytes creates a finding for a probe given its config file's content. +func FromBytes(content []byte, probeID string) (*Finding, error) { + p, err := probe.FromBytes(content, probeID) if err != nil { // nolint return nil, err } f := &Finding{ - Rule: ruleID, + Probe: p.ID, + Outcome: OutcomeNegative, + Remediation: p.Remediation, + } + return f, nil +} + +// New creates a new finding. +func New(loc embed.FS, probeID string) (*Finding, error) { + p, err := probe.New(loc, probeID) + if err != nil { + return nil, fmt.Errorf("%w", err) + } + + f := &Finding{ + Probe: p.ID, Outcome: OutcomeNegative, - Remediation: r.Remediation, + Remediation: p.Remediation, } - if r.Remediation != nil { - f.Risk = r.Risk + return f, nil +} + +// NewWith create a finding with the desried location and outcome. +func NewWith(efs embed.FS, probeID, text string, loc *Location, + o Outcome, +) (*Finding, error) { + f, err := New(efs, probeID) + if err != nil { + return nil, fmt.Errorf("finding.New: %w", err) } + + f = f.WithMessage(text).WithOutcome(o).WithLocation(loc) return f, nil } +// NewWith create a negative finding with the desried location. +func NewNegative(efs embed.FS, probeID, text string, loc *Location, +) (*Finding, error) { + return NewWith(efs, probeID, text, loc, OutcomeNegative) +} + +// NewNotAvailable create a finding with a NotAvailable outcome and the desried location. +func NewNotAvailable(efs embed.FS, probeID, text string, loc *Location, +) (*Finding, error) { + return NewWith(efs, probeID, text, loc, OutcomeNotAvailable) +} + +// NewPositive create a positive finding with the desried location. +func NewPositive(efs embed.FS, probeID, text string, loc *Location, +) (*Finding, error) { + return NewWith(efs, probeID, text, loc, OutcomePositive) +} + +// Anonymize removes the probe ID and outcome +// from the finding. It is a temporary solution +// to integrate the code in the details without exposing +// too much information. +func (f *Finding) Anonymize() *AnonymousFinding { + return &AnonymousFinding{Finding: *f} +} + // WithMessage adds a message to an existing finding. // No copy is made. func (f *Finding) WithMessage(text string) *Finding { @@ -102,6 +185,13 @@ func (f *Finding) WithMessage(text string) *Finding { // No copy is made. func (f *Finding) WithLocation(loc *Location) *Finding { f.Location = loc + if f.Remediation != nil && f.Location != nil { + // Replace location data. + f.Remediation.Text = strings.Replace(f.Remediation.Text, + "${{ finding.location.path }}", f.Location.Path, -1) + f.Remediation.Markdown = strings.Replace(f.Remediation.Markdown, + "${{ finding.location.path }}", f.Location.Path, -1) + } return f } @@ -109,6 +199,8 @@ func (f *Finding) WithLocation(loc *Location) *Finding { // No copy is made. func (f *Finding) WithPatch(patch *string) *Finding { f.Remediation.Patch = patch + // NOTE: we will update the remediation section + // using patch information, e.g. ${{ patch.content }}. return f } @@ -131,16 +223,37 @@ func (f *Finding) WithRemediationMetadata(values map[string]string) *Finding { if f.Remediation != nil { // Replace all dynamic values. for k, v := range values { + // Replace metadata. f.Remediation.Text = strings.Replace(f.Remediation.Text, - fmt.Sprintf("${{ %s }}", k), v, -1) + fmt.Sprintf("${{ metadata.%s }}", k), v, -1) f.Remediation.Markdown = strings.Replace(f.Remediation.Markdown, - fmt.Sprintf("${{ %s }}", k), v, -1) + fmt.Sprintf("${{ metadata.%s }}", k), v, -1) } } return f } -// WorseThan compares outcomes. -func (o *Outcome) WorseThan(oo Outcome) bool { - return *o < oo +// UnmarshalYAML is a custom unmarshalling function +// to transform the string into an enum. +func (o *Outcome) UnmarshalYAML(n *yaml.Node) error { + var str string + if err := n.Decode(&str); err != nil { + return fmt.Errorf("decode: %w", err) + } + + switch n.Value { + case "Negative": + *o = OutcomeNegative + case "Positive": + *o = OutcomePositive + case "NotAvailable": + *o = OutcomeNotAvailable + case "NotSupported": + *o = OutcomeNotSupported + case "Error": + *o = OutcomeError + default: + return fmt.Errorf("%w: %q", errInvalid, str) + } + return nil } diff --git a/finding/finding_test.go b/finding/finding_test.go index f45865580e35..bda8e7405a5f 100644 --- a/finding/finding_test.go +++ b/finding/finding_test.go @@ -15,24 +15,21 @@ package finding import ( - "embed" "errors" + "os" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/ossf/scorecard/v4/rule" + "github.com/ossf/scorecard/v4/finding/probe" ) func errCmp(e1, e2 error) bool { return errors.Is(e1, e2) || errors.Is(e2, e1) } -//go:embed testdata/* -var testfs embed.FS - -func Test_New(t *testing.T) { +func Test_FromBytes(t *testing.T) { snippet := "some code snippet" patch := "some patch values" sline := uint(10) @@ -44,106 +41,92 @@ func Test_New(t *testing.T) { tests := []struct { name string id string + path string outcome *Outcome err error metadata map[string]string finding *Finding }{ - { - name: "risk high", - id: "testdata/risk-high", - outcome: &negativeOutcome, - finding: &Finding{ - Rule: "testdata/risk-high", - Outcome: OutcomeNegative, - Risk: rule.RiskHigh, - Remediation: &rule.Remediation{ - Text: "step1\nstep2 https://www.google.com/something", - Markdown: "step1\nstep2 [google.com](https://www.google.com/something)", - Effort: rule.RemediationEffortLow, - }, - }, - }, { name: "effort low", - id: "testdata/effort-low", + id: "effort-low", + path: "testdata/effort-low.yml", outcome: &negativeOutcome, finding: &Finding{ - Rule: "testdata/effort-low", + Probe: "effort-low", Outcome: OutcomeNegative, - Risk: rule.RiskHigh, - Remediation: &rule.Remediation{ + Remediation: &probe.Remediation{ Text: "step1\nstep2 https://www.google.com/something", Markdown: "step1\nstep2 [google.com](https://www.google.com/something)", - Effort: rule.RemediationEffortLow, + Effort: probe.RemediationEffortLow, }, }, }, { name: "effort high", - id: "testdata/effort-high", + id: "effort-high", + path: "testdata/effort-high.yml", outcome: &negativeOutcome, finding: &Finding{ - Rule: "testdata/effort-high", + Probe: "effort-high", Outcome: OutcomeNegative, - Risk: rule.RiskHigh, - Remediation: &rule.Remediation{ + Remediation: &probe.Remediation{ Text: "step1\nstep2 https://www.google.com/something", Markdown: "step1\nstep2 [google.com](https://www.google.com/something)", - Effort: rule.RemediationEffortHigh, + Effort: probe.RemediationEffortHigh, }, }, }, { name: "env variables", - id: "testdata/metadata-variables", + id: "metadata-variables", + path: "testdata/metadata-variables.yml", outcome: &negativeOutcome, metadata: map[string]string{"branch": "master", "repo": "ossf/scorecard"}, finding: &Finding{ - Rule: "testdata/metadata-variables", + Probe: "metadata-variables", Outcome: OutcomeNegative, - Risk: rule.RiskHigh, - Remediation: &rule.Remediation{ + Remediation: &probe.Remediation{ Text: "step1\nstep2 google.com/ossf/scorecard@master", Markdown: "step1\nstep2 [google.com/ossf/scorecard@master](google.com/ossf/scorecard@master)", - Effort: rule.RemediationEffortLow, + Effort: probe.RemediationEffortLow, }, }, }, { name: "patch", - id: "testdata/metadata-variables", + id: "metadata-variables", + path: "testdata/metadata-variables.yml", outcome: &negativeOutcome, metadata: map[string]string{"branch": "master", "repo": "ossf/scorecard"}, finding: &Finding{ - Rule: "testdata/metadata-variables", + Probe: "metadata-variables", Outcome: OutcomeNegative, - Risk: rule.RiskHigh, - Remediation: &rule.Remediation{ + Remediation: &probe.Remediation{ Text: "step1\nstep2 google.com/ossf/scorecard@master", Markdown: "step1\nstep2 [google.com/ossf/scorecard@master](google.com/ossf/scorecard@master)", - Effort: rule.RemediationEffortLow, + Effort: probe.RemediationEffortLow, Patch: &patch, }, }, }, { name: "location", - id: "testdata/metadata-variables", + id: "metadata-variables", + path: "testdata/metadata-variables.yml", outcome: &negativeOutcome, metadata: map[string]string{"branch": "master", "repo": "ossf/scorecard"}, finding: &Finding{ - Rule: "testdata/metadata-variables", + Probe: "metadata-variables", Outcome: OutcomeNegative, - Risk: rule.RiskHigh, - Remediation: &rule.Remediation{ + Remediation: &probe.Remediation{ Text: "step1\nstep2 google.com/ossf/scorecard@master", Markdown: "step1\nstep2 [google.com/ossf/scorecard@master](google.com/ossf/scorecard@master)", - Effort: rule.RemediationEffortLow, + Effort: probe.RemediationEffortLow, }, Location: &Location{ Type: FileTypeSource, - Value: "path/to/file.txt", + Path: "path/to/file.txt", LineStart: &sline, LineEnd: &eline, Snippet: &snippet, @@ -152,29 +135,29 @@ func Test_New(t *testing.T) { }, { name: "text", - id: "testdata/metadata-variables", + id: "metadata-variables", + path: "testdata/metadata-variables.yml", outcome: &negativeOutcome, metadata: map[string]string{"branch": "master", "repo": "ossf/scorecard"}, finding: &Finding{ - Rule: "testdata/metadata-variables", + Probe: "metadata-variables", Outcome: OutcomeNegative, - Risk: rule.RiskHigh, - Remediation: &rule.Remediation{ + Remediation: &probe.Remediation{ Text: "step1\nstep2 google.com/ossf/scorecard@master", Markdown: "step1\nstep2 [google.com/ossf/scorecard@master](google.com/ossf/scorecard@master)", - Effort: rule.RemediationEffortLow, + Effort: probe.RemediationEffortLow, }, Message: "some text", }, }, { - name: "outcome", - id: "testdata/metadata-variables", + name: "positive outcome", + id: "metadata-variables", + path: "testdata/metadata-variables.yml", outcome: &positiveOutcome, finding: &Finding{ - Rule: "testdata/metadata-variables", + Probe: "metadata-variables", Outcome: OutcomePositive, - Risk: rule.RiskHigh, Message: "some text", }, }, @@ -184,7 +167,12 @@ func Test_New(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - r, err := New(testfs, tt.id) + content, err := os.ReadFile(tt.path) + if err != nil { + t.Fatalf(err.Error()) + } + + r, err := FromBytes(content, tt.id) if err != nil || tt.err != nil { if !errCmp(err, tt.err) { t.Fatalf("unexpected error: %v", cmp.Diff(err, tt.err, cmpopts.EquateErrors())) diff --git a/finding/probe/probe.go b/finding/probe/probe.go new file mode 100644 index 000000000000..1db07e8e504a --- /dev/null +++ b/finding/probe/probe.go @@ -0,0 +1,183 @@ +// Copyright 2023 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package probe + +import ( + "embed" + "errors" + "fmt" + "strings" + + "gopkg.in/yaml.v3" +) + +var errInvalid = errors.New("invalid") + +// RemediationEffort indicates the estimated effort necessary to remediate a finding. +type RemediationEffort int + +const ( + // RemediationEffortNone indicates a no remediation effort. + RemediationEffortNone RemediationEffort = iota + // RemediationEffortLow indicates a low remediation effort. + RemediationEffortLow + // RemediationEffortMedium indicates a medium remediation effort. + RemediationEffortMedium + // RemediationEffortHigh indicates a high remediation effort. + RemediationEffortHigh +) + +// Remediation represents the remediation for a finding. +type Remediation struct { + // Patch for machines. + Patch *string `json:"patch,omitempty"` + // Text for humans. + Text string `json:"text"` + // Text in markdown format for humans. + Markdown string `json:"markdown"` + // Effort to remediate. + Effort RemediationEffort `json:"effort"` +} + +// nolint: govet +type yamlRemediation struct { + Text []string `yaml:"text"` + Markdown []string `yaml:"markdown"` + Effort RemediationEffort `yaml:"effort"` +} + +// nolint: govet +type yamlProbe struct { + ID string `yaml:"id"` + Short string `yaml:"short"` + Motivation string `yaml:"motivation"` + Implementation string `yaml:"implementation"` + Remediation yamlRemediation `yaml:"remediation"` +} + +// nolint: govet +type Probe struct { + ID string + Short string + Motivation string + Implementation string + Remediation *Remediation +} + +// FromBytes creates a probe from a file. +func FromBytes(content []byte, probeID string) (*Probe, error) { + r, err := parseFromYAML(content) + if err != nil { + return nil, err + } + + if err := validate(r, probeID); err != nil { + return nil, err + } + + return &Probe{ + ID: r.ID, + Short: r.Short, + Motivation: r.Motivation, + Implementation: r.Implementation, + Remediation: &Remediation{ + Text: strings.Join(r.Remediation.Text, "\n"), + Markdown: strings.Join(r.Remediation.Markdown, "\n"), + Effort: r.Remediation.Effort, + }, + }, nil +} + +// New create a new probe. +func New(loc embed.FS, probeID string) (*Probe, error) { + content, err := loc.ReadFile("def.yml") + if err != nil { + return nil, fmt.Errorf("%w", err) + } + return FromBytes(content, probeID) +} + +func validate(r *yamlProbe, probeID string) error { + if err := validateID(r.ID, probeID); err != nil { + return err + } + if err := validateRemediation(r.Remediation); err != nil { + return err + } + return nil +} + +func validateID(actual, expected string) error { + if actual != expected { + return fmt.Errorf("%w: ID: read '%v', expected '%v'", errInvalid, + actual, expected) + } + return nil +} + +func validateRemediation(r yamlRemediation) error { + switch r.Effort { + case RemediationEffortHigh, RemediationEffortMedium, RemediationEffortLow: + return nil + default: + return fmt.Errorf("%w: %v", errInvalid, fmt.Sprintf("remediation '%v'", r)) + } +} + +func parseFromYAML(content []byte) (*yamlProbe, error) { + r := yamlProbe{} + + err := yaml.Unmarshal(content, &r) + if err != nil { + return nil, fmt.Errorf("%w: %v", errInvalid, err) + } + return &r, nil +} + +// UnmarshalYAML is a custom unmarshalling function +// to transform the string into an enum. +func (r *RemediationEffort) UnmarshalYAML(n *yaml.Node) error { + var str string + if err := n.Decode(&str); err != nil { + return fmt.Errorf("%w: %v", errInvalid, err) + } + + // nolint:goconst + switch n.Value { + case "Low": + *r = RemediationEffortLow + case "Medium": + *r = RemediationEffortMedium + case "High": + *r = RemediationEffortHigh + default: + return fmt.Errorf("%w: effort:%q", errInvalid, str) + } + return nil +} + +// String stringifies the enum. +func (r *RemediationEffort) String() string { + switch *r { + case RemediationEffortLow: + return "Low" + case RemediationEffortMedium: + return "Medium" + case RemediationEffortHigh: + return "High" + default: + return "" + } +} diff --git a/finding/probe/probe_test.go b/finding/probe/probe_test.go new file mode 100644 index 000000000000..2fe1a8a82ad5 --- /dev/null +++ b/finding/probe/probe_test.go @@ -0,0 +1,109 @@ +// Copyright 2023 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package probe + +import ( + "errors" + "os" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" +) + +func errCmp(e1, e2 error) bool { + return errors.Is(e1, e2) || errors.Is(e2, e1) +} + +func Test_FromBytes(t *testing.T) { + t.Parallel() + // nolint: govet + tests := []struct { + name string + id string + path string + err error + probe *Probe + }{ + { + name: "all fields set", + id: "all-fields", + path: "testdata/all-fields.yml", + probe: &Probe{ + ID: "all-fields", + Short: "short description", + Implementation: "impl1 impl2\n", + Motivation: "mot1 mot2\n", + Remediation: &Remediation{ + Text: "step1\nstep2 https://www.google.com/something", + Markdown: "step1\nstep2 [google.com](https://www.google.com/something)", + Effort: RemediationEffortLow, + }, + }, + }, + { + name: "mismatch probe ID", + id: "mismatch-id", + path: "testdata/all-fields.yml", + probe: &Probe{ + ID: "all-fields", + Short: "short description", + Implementation: "impl1 impl2\n", + Motivation: "mot1 mot2\n", + Remediation: &Remediation{ + Text: "step1\nstep2 https://www.google.com/something", + Markdown: "step1\nstep2 [google.com](https://www.google.com/something)", + Effort: RemediationEffortLow, + }, + }, + err: errInvalid, + }, + { + name: "missing id", + id: "missing-id", + path: "testdata/missing-id.yml", + err: errInvalid, + }, + { + name: "invalid effort", + id: "invalid-effort", + path: "testdata/invalid-effort.yml", + err: errInvalid, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + content, err := os.ReadFile(tt.path) + if err != nil { + t.Fatalf(err.Error()) + } + + r, err := FromBytes(content, tt.id) + if err != nil || tt.err != nil { + if !errCmp(err, tt.err) { + t.Fatalf("unexpected error: %v", cmp.Diff(err, tt.err, cmpopts.EquateErrors())) + } + return + } + + if diff := cmp.Diff(*tt.probe, *r); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/finding/probe/testdata/all-fields.yml b/finding/probe/testdata/all-fields.yml new file mode 100644 index 000000000000..c67a3251e3b3 --- /dev/null +++ b/finding/probe/testdata/all-fields.yml @@ -0,0 +1,16 @@ +id: all-fields +short: short description +motivation: > + mot1 + mot2 +implementation: > + impl1 + impl2 +remediation: + effort: Low + text: + - step1 + - step2 https://www.google.com/something + markdown: + - step1 + - step2 [google.com](https://www.google.com/something) diff --git a/finding/testdata/risk-high.yml b/finding/probe/testdata/invalid-effort.yml similarity index 85% rename from finding/testdata/risk-high.yml rename to finding/probe/testdata/invalid-effort.yml index 8f4ac0c957ba..0fc9474478ce 100644 --- a/finding/testdata/risk-high.yml +++ b/finding/probe/testdata/invalid-effort.yml @@ -1,14 +1,13 @@ +id: invalid-effort short: short description -desc: description motivation: > line1 line2 implementation: > line1 line2 -risk: High remediation: - effort: Low + effort: invalid text: - step1 - step2 https://www.google.com/something diff --git a/finding/testdata/risk-low.yml b/finding/probe/testdata/missing-id.yml similarity index 90% rename from finding/testdata/risk-low.yml rename to finding/probe/testdata/missing-id.yml index 1c8e6cfc1309..7fb1325e35dc 100644 --- a/finding/testdata/risk-low.yml +++ b/finding/probe/testdata/missing-id.yml @@ -1,12 +1,10 @@ short: short description -desc: description motivation: > line1 line2 implementation: > line1 line2 -risk: Low remediation: effort: Low text: diff --git a/finding/testdata/effort-high.yml b/finding/testdata/effort-high.yml index 237234d28ed2..57a46402d012 100644 --- a/finding/testdata/effort-high.yml +++ b/finding/testdata/effort-high.yml @@ -1,12 +1,11 @@ +id: effort-high short: short description -desc: description motivation: > line1 line2 implementation: > line1 line2 -risk: High remediation: effort: High text: diff --git a/finding/testdata/effort-low.yml b/finding/testdata/effort-low.yml index 8f4ac0c957ba..7b390ecf0d28 100644 --- a/finding/testdata/effort-low.yml +++ b/finding/testdata/effort-low.yml @@ -1,12 +1,11 @@ +id: effort-low short: short description -desc: description motivation: > line1 line2 implementation: > line1 line2 -risk: High remediation: effort: Low text: diff --git a/finding/testdata/metadata-variables.yml b/finding/testdata/metadata-variables.yml index c13073890520..2fcb7c01f6d9 100644 --- a/finding/testdata/metadata-variables.yml +++ b/finding/testdata/metadata-variables.yml @@ -1,17 +1,16 @@ +id: metadata-variables short: short description -desc: description motivation: > line1 line2 implementation: > line1 line2 -risk: High remediation: effort: Low text: - step1 - - step2 google.com/${{ repo }}@${{ branch }} + - step2 google.com/${{ metadata.repo }}@${{ metadata.branch }} markdown: - step1 - - step2 [google.com/${{ repo }}@${{ branch }}](google.com/${{ repo }}@${{ branch }}) + - step2 [google.com/${{ metadata.repo }}@${{ metadata.branch }}](google.com/${{ metadata.repo }}@${{ metadata.branch }}) diff --git a/pkg/common.go b/pkg/common.go index c3a0ca2ecb4b..bd861d149db9 100644 --- a/pkg/common.go +++ b/pkg/common.go @@ -50,10 +50,10 @@ func nonStructuredResultString(d *checker.CheckDetail) string { func structuredResultString(d *checker.CheckDetail) string { var sb strings.Builder f := d.Msg.Finding - sb.WriteString(fmt.Sprintf("%s: %s severity: %s", typeToString(d.Type), f.Risk.String(), f.Message)) + sb.WriteString(fmt.Sprintf("%s: %s", typeToString(d.Type), f.Message)) if f.Location != nil { - sb.WriteString(fmt.Sprintf(": %s", f.Location.Value)) + sb.WriteString(fmt.Sprintf(": %s", f.Location.Path)) if f.Location.LineStart != nil { sb.WriteString(fmt.Sprintf(":%d", *f.Location.LineStart)) } diff --git a/pkg/sarif.go b/pkg/sarif.go index 8977f6d01c0c..30f43d012940 100644 --- a/pkg/sarif.go +++ b/pkg/sarif.go @@ -209,7 +209,7 @@ func generateDefaultConfig(risk string) string { func getPath(d *checker.CheckDetail) string { f := d.Msg.Finding if f != nil && f.Location != nil { - return f.Location.Value + return f.Location.Path } return d.Msg.Path } From fe3dcf71247a2dad75835b513547c5ec4a9e7c23 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 3 May 2023 09:01:34 +0000 Subject: [PATCH 4/6] :seedling: Bump github.com/onsi/ginkgo/v2 from 2.9.2 to 2.9.3 in /tools Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.9.2 to 2.9.3. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](https://github.com/onsi/ginkgo/compare/v2.9.2...v2.9.3) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- tools/go.mod | 4 ++-- tools/go.sum | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/go.mod b/tools/go.mod index 986497eccf60..a4657fdb9c56 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -9,7 +9,7 @@ require ( github.com/google/ko v0.13.0 github.com/goreleaser/goreleaser v1.17.2 github.com/naveensrinivasan/stunning-tribble v0.4.2 - github.com/onsi/ginkgo/v2 v2.9.2 + github.com/onsi/ginkgo/v2 v2.9.3 google.golang.org/protobuf v1.30.0 ) @@ -143,7 +143,7 @@ require ( github.com/go-git/gcfg v1.5.0 // indirect github.com/go-git/go-billy/v5 v5.3.1 // indirect github.com/go-git/go-git/v5 v5.4.2 // indirect - github.com/go-logr/logr v1.2.3 // indirect + github.com/go-logr/logr v1.2.4 // indirect github.com/go-openapi/analysis v0.21.4 // indirect github.com/go-openapi/errors v0.20.3 // indirect github.com/go-openapi/jsonpointer v0.19.5 // indirect diff --git a/tools/go.sum b/tools/go.sum index fe5d78a51ba5..458b77c559ce 100644 --- a/tools/go.sum +++ b/tools/go.sum @@ -1140,8 +1140,9 @@ github.com/go-logr/logr v0.4.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTg github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.2.1/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ= +github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/stdr v1.2.0/go.mod h1:YkVgnZu1ZjjL7xTxrfm/LLZBfkhTqSR1ydtm6jTKKwI= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-openapi/analysis v0.21.2/go.mod h1:HZwRk4RRisyG8vx2Oe6aqeSQcoxRp47Xkp3+K6q+LdY= @@ -1966,8 +1967,8 @@ github.com/onsi/ginkgo/v2 v2.1.4/go.mod h1:um6tUpWM/cxCK3/FK8BXqEiUMUwRgSM4JXG47 github.com/onsi/ginkgo/v2 v2.1.6/go.mod h1:MEH45j8TBi6u9BMogfbp0stKC5cdGjumZj5Y7AG4VIk= github.com/onsi/ginkgo/v2 v2.3.0/go.mod h1:Eew0uilEqZmIEZr8JrvYlvOM7Rr6xzTmMV8AyFNU9d0= github.com/onsi/ginkgo/v2 v2.4.0/go.mod h1:iHkDK1fKGcBoEHT5W7YBq4RFWaQulw+caOMkAt4OrFo= -github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= -github.com/onsi/ginkgo/v2 v2.9.2/go.mod h1:WHcJJG2dIlcCqVfBAwUCrJxSPFb6v4azBwgxeMeDuts= +github.com/onsi/ginkgo/v2 v2.9.3 h1:5X2vl/isiKqkrOYjiaGgp3JQOcLV59g5o5SuTMqCcxU= +github.com/onsi/ginkgo/v2 v2.9.3/go.mod h1:gCQYp2Q+kSoIj7ykSVb9nskRSsR6PUj4AiLywzIhbKM= github.com/onsi/gomega v0.0.0-20151007035656-2152b45fa28a/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= @@ -1986,7 +1987,7 @@ github.com/onsi/gomega v1.20.1/go.mod h1:DtrZpjmvpn2mPm4YWQa0/ALMDj9v4YxLgojwPeR github.com/onsi/gomega v1.21.1/go.mod h1:iYAIXgPSaDHak0LCMA+AWBpIKBr8WZicMxnE8luStNc= github.com/onsi/gomega v1.22.1/go.mod h1:x6n7VNe4hw0vkyYUM4mjIXx3JbLiPaBPNgB7PRQ1tuM= github.com/onsi/gomega v1.23.0/go.mod h1:Z/NWtiqwBrwUt4/2loMmHL63EDLnYHmVbuBpDr2vQAg= -github.com/onsi/gomega v1.27.4 h1:Z2AnStgsdSayCMDiCU42qIz+HLqEPcgiOCXjAU/w+8E= +github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk= github.com/opencontainers/go-digest v0.0.0-20170106003457-a6d0ee40d420/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v0.0.0-20180430190053-c9281466c8b2/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= From d4624f7fa4f524f8da4f14ce3f5f309323f5feba Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 3 May 2023 14:14:35 +0000 Subject: [PATCH 5/6] :seedling: Bump github.com/onsi/ginkgo/v2 from 2.9.2 to 2.9.3 (#2937) Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.9.2 to 2.9.3. - [Release notes](https://github.com/onsi/ginkgo/releases) - [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md) - [Commits](https://github.com/onsi/ginkgo/compare/v2.9.2...v2.9.3) --- updated-dependencies: - dependency-name: github.com/onsi/ginkgo/v2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index cd612e4505c5..f07829224dab 100644 --- a/go.mod +++ b/go.mod @@ -49,7 +49,7 @@ require ( github.com/gobwas/glob v0.2.3 github.com/google/osv-scanner v1.3.2 github.com/mcuadros/go-jsonschema-generator v0.0.0-20200330054847-ba7a369d4303 - github.com/onsi/ginkgo/v2 v2.9.2 + github.com/onsi/ginkgo/v2 v2.9.3 github.com/otiai10/copy v1.11.0 sigs.k8s.io/release-utils v0.6.0 ) diff --git a/go.sum b/go.sum index d537ff8870b3..e31f9bb7ed6a 100644 --- a/go.sum +++ b/go.sum @@ -1658,8 +1658,8 @@ github.com/onsi/ginkgo/v2 v2.1.4/go.mod h1:um6tUpWM/cxCK3/FK8BXqEiUMUwRgSM4JXG47 github.com/onsi/ginkgo/v2 v2.1.6/go.mod h1:MEH45j8TBi6u9BMogfbp0stKC5cdGjumZj5Y7AG4VIk= github.com/onsi/ginkgo/v2 v2.3.0/go.mod h1:Eew0uilEqZmIEZr8JrvYlvOM7Rr6xzTmMV8AyFNU9d0= github.com/onsi/ginkgo/v2 v2.4.0/go.mod h1:iHkDK1fKGcBoEHT5W7YBq4RFWaQulw+caOMkAt4OrFo= -github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= -github.com/onsi/ginkgo/v2 v2.9.2/go.mod h1:WHcJJG2dIlcCqVfBAwUCrJxSPFb6v4azBwgxeMeDuts= +github.com/onsi/ginkgo/v2 v2.9.3 h1:5X2vl/isiKqkrOYjiaGgp3JQOcLV59g5o5SuTMqCcxU= +github.com/onsi/ginkgo/v2 v2.9.3/go.mod h1:gCQYp2Q+kSoIj7ykSVb9nskRSsR6PUj4AiLywzIhbKM= github.com/onsi/gomega v0.0.0-20151007035656-2152b45fa28a/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= From 0aee65e3061867356ff3d20e8a6fd1866c3e9729 Mon Sep 17 00:00:00 2001 From: Naveen <172697+naveensrinivasan@users.noreply.github.com> Date: Wed, 3 May 2023 11:19:06 -0500 Subject: [PATCH 6/6] :seedling: Unitest for rule/rule.go (#2926) - Unit tests for rule/rule.go - Covered with 88% of code. Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com> --- rule/rule_test.go | 282 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 282 insertions(+) diff --git a/rule/rule_test.go b/rule/rule_test.go index e6484a23f2ee..e84de19d47f7 100644 --- a/rule/rule_test.go +++ b/rule/rule_test.go @@ -17,10 +17,12 @@ package rule import ( "embed" "errors" + "fmt" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "gopkg.in/yaml.v3" ) func errCmp(e1, e2 error) bool { @@ -85,3 +87,283 @@ func Test_New(t *testing.T) { }) } } + +func TestRisk_GreaterThan(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + r Risk + rr Risk + want bool + }{ + { + name: "greater than", + r: RiskHigh, + rr: RiskLow, + want: true, + }, + { + name: "less than", + r: RiskLow, + rr: RiskHigh, + want: false, + }, + { + name: "equal", + r: RiskMedium, + rr: RiskMedium, + want: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if got := tt.r.GreaterThan(tt.rr); got != tt.want { + t.Errorf("Risk.GreaterThan() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRisk_String(t *testing.T) { + t.Parallel() + + tests := []struct { //nolint:govet + name string + r Risk + want string + }{ + { + name: "RiskNone", + r: RiskNone, + want: "None", + }, + { + name: "RiskLow", + r: RiskLow, + want: "Low", + }, + { + name: "RiskMedium", + r: RiskMedium, + want: "Medium", + }, + { + name: "RiskHigh", + r: RiskHigh, + want: "High", + }, + { + name: "RiskCritical", + r: RiskCritical, + want: "Critical", + }, + { + name: "invalid", + r: Risk(100), + want: "", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if got := tt.r.String(); got != tt.want { + t.Errorf("Risk.String() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRemediationEffort_String(t *testing.T) { + t.Parallel() + + tests := []struct { //nolint:govet + name string + effort RemediationEffort + want string + }{ + { + name: "RemediationEffortNone", + effort: RemediationEffortNone, + want: "", + }, + { + name: "RemediationEffortLow", + effort: RemediationEffortLow, + want: "Low", + }, + { + name: "RemediationEffortMedium", + effort: RemediationEffortMedium, + want: "Medium", + }, + { + name: "RemediationEffortHigh", + effort: RemediationEffortHigh, + want: "High", + }, + { + name: "invalid", + effort: RemediationEffort(100), + want: "", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if got := tt.effort.String(); got != tt.want { + t.Errorf("RemediationEffort.String() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRisk_UnmarshalYAML(t *testing.T) { + t.Parallel() + + tests := []struct { //nolint:govet + name string + input string + wantErr error + want Risk + }{ + { + name: "RiskNone", + input: "None", + want: RiskNone, + }, + { + name: "RiskLow", + input: "Low", + want: RiskLow, + }, + { + name: "RiskMedium", + input: "Medium", + want: RiskMedium, + }, + { + name: "RiskHigh", + input: "High", + want: RiskHigh, + }, + { + name: "RiskCritical", + input: "Critical", + want: RiskCritical, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + var r Risk + err := yaml.Unmarshal([]byte(tt.input), &r) + if err != nil { + if tt.wantErr == nil || !errors.Is(err, tt.wantErr) { + t.Errorf("Risk.UnmarshalYAML() error = %v, wantErr %v", err, tt.wantErr) + } + return + } + if r != tt.want { + t.Errorf("Risk.UnmarshalYAML() got = %v, want %v", r, tt.want) + } + }) + } +} + +func TestRemediationEffort_UnmarshalYAML(t *testing.T) { + t.Parallel() + + tests := []struct { //nolint:govet + name string + input string + wantErr error + want RemediationEffort + }{ + { + name: "RemediationEffortLow", + input: "Low", + want: RemediationEffortLow, + }, + { + name: "RemediationEffortMedium", + input: "Medium", + want: RemediationEffortMedium, + }, + { + name: "RemediationEffortHigh", + input: "High", + want: RemediationEffortHigh, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + var r RemediationEffort + err := yaml.Unmarshal([]byte(tt.input), &r) + if err != nil { + if tt.wantErr == nil || !errors.Is(err, tt.wantErr) { + t.Errorf("RemediationEffort.UnmarshalYAML() error = %v, wantErr %v", err, tt.wantErr) + } + return + } + if r != tt.want { + t.Errorf("RemediationEffort.UnmarshalYAML() got = %v, want %v", r, tt.want) + } + }) + } +} + +func Test_validate(t *testing.T) { + t.Parallel() + + tests := []struct { //nolint:govet + name string + rule *jsonRule + wantErr error + }{ + { + name: "valid", + rule: &jsonRule{ + Risk: RiskLow, + Remediation: jsonRemediation{ + Effort: RemediationEffortHigh, + }, + }, + wantErr: nil, + }, + { + name: "invalid risk", + rule: &jsonRule{ + Risk: Risk(100), + Remediation: jsonRemediation{ + Effort: RemediationEffortHigh, + }, + }, + wantErr: fmt.Errorf("%w: invalid: risk '100'", errInvalid), + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + err := validate(tt.rule) + if err != nil { + if tt.wantErr == nil || !cmp.Equal(tt.wantErr.Error(), err.Error()) { + t.Logf("got: %s", err.Error()) + t.Errorf("validate() error = %v, wantErr %v", err, cmp.Diff(tt.wantErr.Error(), err.Error())) + } + return + } + if tt.wantErr != nil { + t.Errorf("validate() error = %v, wantErr %v", err, cmp.Diff(tt.wantErr, err)) + } + }) + } +}