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

CII-Best-Practices not detected reliably #110

Closed
Tracked by #97
jonasbb opened this issue Feb 19, 2022 · 6 comments
Closed
Tracked by #97

CII-Best-Practices not detected reliably #110

jonasbb opened this issue Feb 19, 2022 · 6 comments
Projects

Comments

@jonasbb
Copy link
Contributor

jonasbb commented Feb 19, 2022

I am testing this action on one of my repositories (https://github.com/jonasbb/serde_with). One of the problems I noticed is that the detection of the CII Best Practices status is broken and does mostly not detect the status correctly. My project has a CII passing status.

This is the view I have in the security tab.
Screenshot from 2022-02-19 23-53-11

The message even says, that the passing status is detected, but still tells me to inspect the remediation section. The remediation contains this, so I would expect no warning in the security tab.

We give full credit to projects that meet the passing criteria, which is a significant achievement for many projects.

#400 Introduced the action. At that point, the CII passing status was already established. #401 also got merged without affecting the status.
With #402 it got fixed, but already the next PR #406 broke it. Since then, multiple merges happened, but the status did not get updated.

@laurentsimon
Copy link
Contributor

laurentsimon commented Feb 22, 2022

Thanks for the report. I agree this is confusing.
There are 2 ways to fix it:

  1. Improve the message, maybe to the effect of "You have a badge passing, which is not the highest level". Currently, we report the findings if the score is below 10. 10 means you have a gold badge, for other scores see https://github.com/ossf/scorecard/blob/main/checks/cii_best_practices.go#L28-L30
  2. Take the view that a passing badge is good enough and not report results if the badge passes, regardless of the level (silver, gold, etc)

What's your opinion?

cc @david-a-wheeler (author of the badges) can you advise?

@jonasbb
Copy link
Contributor Author

jonasbb commented Feb 22, 2022

Hi @laurentsimon, thanks for the information. I read the part "We give full credit to projects that meet the passing criteria" as a statement that "passing" is the only requirement. I understand if silver and gold badges should be awarded with more points, but then I find the phrasing in the remediation section confusion.

The other thing I don't understand is why this was reported as fixed for PR #402. I assume it would only resolve as being fixed, if it detects a gold score. At least according to my understanding of the code snippet you linked.

@laurentsimon
Copy link
Contributor

laurentsimon commented Feb 22, 2022

Hi @laurentsimon, thanks for the information. I read the part "We give full credit to projects that meet the passing criteria" as a statement that "passing" is the only requirement. I understand if silver and gold badges should be awarded with more points, but then I find the phrasing in the remediation section confusion.

agreed, which is why I proposed solution 1 above, ie change the message. I agree the doc is also out of sync. We'll update it, thanks for pointing this out!

The other thing I don't understand is why this was reported as fixed for PR #402. I assume it would only resolve as being fixed, if it detects a gold score. At least according to my understanding of the code snippet you linked.

You're correct. But, in the scorecard action, we can change the "threshold score" we use to display results. Today we use 10, see https://github.com/ossf/scorecard-action/blob/main/policies/template.yml#L60. If we decide that passing suffices, we can update this score to 5 (passingScore) and the results would not show up in the scanning dashboard.

Let me know if you have further questions.

Let's wait for @david-a-wheeler to also give us his opinion, since he's the author of the best practice badges.

@justaugustus justaugustus added this to Backlog in Scorecard Feb 22, 2022
@laurentsimon
Copy link
Contributor

friendly ping @david-a-wheeler

@laurentsimon
Copy link
Contributor

laurentsimon commented Mar 7, 2022

Let's wait a little longer to David to chime in. If we don't hear from him, I suggest we update the threshold score in the action, so that alerts don't show if the badge is passing (regardless of it actual value)

@jonasbb are you interested in sending us a PR for this? You only need to update this https://github.com/ossf/scorecard-action/blob/main/policies/template.yml#L60

Let me know, otherwise I'll take it.

laurentsimon added a commit that referenced this issue Mar 18, 2022
The remediation section states that only a passing score is required for this check to pass:
> We give full credit to projects that meet the passing criteria, which is a significant achievement for many projects.

A passing score currently equals 5.
https://github.com/ossf/scorecard/blob/e128c3de82607e1b285185da9c76a5262255b180/checks/cii_best_practices.go

See #110 for more details.

Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
@laurentsimon
Copy link
Contributor

closed by #129

Scorecard automation moved this from Backlog to Done Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants