Change in Dangerous-Workflow and Token-Permissions scores for repos with no workflows

[spencerschrock]

Describe the bug
GitHub repos without any workflows receive an inconclusive score for Dangerous-Workflow and Token-Permissions.

Reproduction steps
Steps to reproduce the behavior:

1. go run main.go --repo github.com/9fans/plan9port --checks Token-Permissions

Expected behavior

I think there's some debate if this should be inconclusive or a 10.

Additional context
This was introduced in #2821, and it makes sense in the context of GitLab repos. But I don't think the change was intentional for GitHub repos.

[evverx]

Just out of curiosity I wonder if it was caught by the "golden testing" test suite? If so it's really cool.

[raghavkaul]

I feel that an Inconclusive Result makes more sense for GitHub repos, and also that Structured Results may make this clearer: would we have a separate finding for "no GitHub actions being found" ? I don't think that's a relevant finding, for example, for a repo that's very small and has no CI set up all, because there's nothing security-actionable about it.

[spencerschrock]

Just out of curiosity I wonder if it was caught by the "golden testing" test suite? If so it's really cool.

It was done via some manual diffing (awk and diff) of 2 runs across a few hundred repos. But it's nice to have some indication that the process would catch things like this.

I feel that an Inconclusive Result makes more sense for GitHub repos [...] I don't think that's a relevant finding, for example, for a repo that's very small and has no CI set up all, because there's nothing security-actionable about it.

I'm imagining someone running a check then having a policy/attestor trying to confirm: "does the repo have dangerous workflows". For a repo with no workflows I would expect that policy to pass. I'm not sure sure inconclusive would allow us to say that.

From the docs: "The highest score is awarded when all workflows avoid the dangerous code patterns."
Which is vacuously true.

, and also that Structured Results may make this clearer: would we have a separate finding for "no GitHub actions being found"

agreed. But possibly something more generic like OutcomeNotApplicable. Would need fleshed out in #2928

[raghavkaul]

I'm imagining someone running a check then having a policy/attestor trying to confirm: "does the repo have dangerous workflows". For a repo with no workflows I would expect that policy to pass. I'm not sure sure inconclusive would allow us to say that.

The scoring logic is in package evaluation, so raw results as they are could already be used to write such a policy.

[diogoteles08]

An interesting perspective is that having -1s at Dangerous-Workflows and at Token permissions would increase the weight of every other check. Given that Dangerous-Workflows is the single check with "Critical" risk level, and Token-Permissions has a "high", that weight change is probably not irrelevant.

As an example, a project without any GitHub workflows would receive a higher punishment for having Binary Artifacts (just an high risk check as example) than a project that has workflows. It's not clear to me if that makes sense of not.

[diogoteles08]

Thanks for the info, anyone can help me with This?

Do you mean help you to work on this issue? If so, I don't actually think this needs to be worked on.
AFAIU, this issue describes the behaviour that was already introduced, the idea is to discuss if we really want it.

[spencerschrock]

An interesting perspective is that having -1s at Dangerous-Workflows and at Token permissions would increase the weight of every other check. Given that Dangerous-Workflows is the single check with "Critical" risk level, and Token-Permissions has a "high", that weight change is probably not irrelevant.

As an example, a project without any GitHub workflows would receive a higher punishment for having Binary Artifacts (just an high risk check as example) than a project that has workflows. It's not clear to me if that makes sense of not.

+1 good point.
My vote is to maintain -1 for gitlab repos, and revert to 10 for github repos. But I'll throw it on the bi-weekly sync

[david-a-wheeler]

We say the score is "0..10", so -1" doesn't make sense. I'm fine with "-1" as a sentinel, indicating an issue, but if you're actually averaging the -1 in that's a problem.

[evankanderson]

Some folks aren't using GitHub Actions... how do we want to "score" those CI systems? For example, if I'm using always-hacked-free-CI-provider to build my releases, can I end up with a better score than using GitHub actions with some poor implementation choices?

(For example https://github.com/knative/serving uses both actions and Prow from Kubernetes)

We currently support detecting only one pipeline, but it seems like we might want incorporate the GitHub actions risk into the larger CI scoring value as reductions to the CI score, rather than a separate weight.

[leec94]

hi, was there an agreement to this discussion? i'm looking into #2158 if Token-Permissions needs any scoring changes to be completed

[spencerschrock]

We say the score is "0..10", so -1" doesn't make sense. I'm fine with "-1" as a sentinel, indicating an issue, but if you're actually averaging the -1 in that's a problem.

Yes, -1 is a sentinel value for an inconclusive score that doesn't get included in the calculation (which slightly alters the weights of other scores as a byproduct). Some examples of when we use -1:

• Runtime errors, where we don't have the data to make a decision
• Code-Review when all of the PRs are from dependabot, so we don't have any data to make a decision

So I think it boils down to if we have enough data to make a decision. There are no repo files, true, but I think that gives us enough data to say there are no problems with the workflows/tokens since there can't be problems with what doesn't exist.

Some folks aren't using GitHub Actions... how do we want to "score" those CI systems? For example, if I'm using always-hacked-free-CI-provider to build my releases, can I end up with a better score than using GitHub actions with some poor implementation choices?

(For example https://github.com/knative/serving uses both actions and Prow from Kubernetes)

We currently support detecting only one pipeline, but it seems like we might want incorporate the GitHub actions risk into the larger CI scoring value as reductions to the CI score, rather than a separate weight.

Scorecard as a whole focuses on the native CI system, so this is a much bigger question.

hi, was there an agreement to this discussion? i'm looking into #2158 if Token-Permissions needs any scoring changes to be completed

Nothing unanimous yet.

[spencerschrock]

Closing due to lack of consensus combined with the small impact of the decision. We can always revisit if needed.