⚠️ Allow probes to specify their own bad outcomes

[spencerschrock]

What kind of change does this PR introduce?

structured result breaking change

• PR title follows the guidelines defined in our pull request documentation

What is the current behavior?

OutcomeNegative which was always assumed to be negative, got changed to OutcomeFalse, which isn't always a bad thing. Some probes detect dangerous behaviors, which would mean the remediation should be applied to OutcomeTrue.

What is the new behavior (if this is a feature change)?**

The github.com/ossf/scorecard/v4/finding/probe package was merged into github.com/ossf/scorecard/v4/finding.

Probes can declare in their YAML which outcome should cause the findings to have a remediation. This is done by specifying a onOutcome field under remediation in the yaml:

remediation:
  # can be True or False depending on how the probe is phrased
  onOutcome: True 

This enables fixing the outcome/probe names for some probes.

1. Several probes were phrased in a way that the conversion from OutcomePositive/Negative to True/False made the returned outcomes wrong because they identified bad behaviors. These were fixed so OutcomeTrue is interpreted negatively.

• hasOSVVulnerabilities
• hasDangerousWorkflowScriptInjection
• hasDangerousWorkflowUntrustedCheckout

2. Other probes were phrased in a way to avoid confusion mentioned in Feature: Rename OutcomePositive/Negative to something more aligned with the security implication of a probe finding #3866. These probes have been chnaged as follows:

• notCreatedRecently -> createdRecently
• freeOfAnyBinaryArtifacts -> hasBinaryArtifacts
• freeOfUnverifiedBinaryArtifacts -> hasUnverifiedBinaryArtifacts
• notArchived -> archived

• Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #3654

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

[spencerschrock]

@laurentsimon this is still a WIP as I need to switch around more probes.

But this commit (eec4665) shows the new field
and this commit (2ba19ac) shows an example of the swap for hasOSVVulnerabilities

Does this generally match what you mentioned in our meeting today?

[laurentsimon]

@laurentsimon this is still a WIP as I need to switch around more probes.

But this commit (eec4665) shows the new field

I was thinking we'd create a new field in the def.yml:

- remediation
  on: true | false
  text: bla 
  ...

and use that to determine when remediation needs to be populated. Having in def.yml also helps with documentation

[laurentsimon]

@laurentsimon this is still a WIP as I need to switch around more probes.
But this commit (eec4665) shows the new field

I was thinking we'd create a new field in the def.yml:

- remediation
  on: true | false
  text: bla 
  ...

and use that to determine when remediation needs to be populated. Having in def.yml also helps with documentation

Sorry, that's basically what you did - I missed the example .yml files and was looking for a def.yml. Yeah We're on the same page. +1 on having badOutcome private!

[spencerschrock]

@laurentsimon this is still a WIP as I need to switch around more probes.
But this commit (eec4665) shows the new field

I was thinking we'd create a new field in the def.yml:

- remediation
  on: true | false
  text: bla 
  ...

and use that to determine when remediation needs to be populated. Having in def.yml also helps with documentation

Sorry, that's basically what you did - I missed the example .yml files and was looking for a def.yml. Yeah We're on the same page. +1 on having badOutcome private!

For the yaml, I like the nested on: field better. Although it pollutes the Remediation struct which is used elsewhere. I'll play around with changes tomorrow.

[spencerschrock]

/scdiff generate Pinned-Dependencies,Contributors,Security-Policy,Vulnerabilities,Branch-Protection,SAST,CII-Best-Practices,Dangerous-Workflow,Code-Review,Token-Permissions,Signed-Releases,Fuzzing,License,CI-Tests,Binary-Artifacts,Dependency-Update-Tool,Maintained,Packaging

[github-actions]

Here's a link to the scdiff run