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

🌱 convert CII Best Practices check to probes #3520

Merged
merged 17 commits into from Nov 28, 2023

Conversation

AdamKorcz
Copy link
Contributor

What kind of change does this PR introduce?

feature

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

This PR converts the CII Best Practices to 6 different probes.

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

Which issue(s) this PR fixes

NONE

Does this PR introduce a user-facing change?

No.


@AdamKorcz AdamKorcz temporarily deployed to gitlab September 27, 2023 15:10 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test September 27, 2023 15:11 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #3520 (8a85ba1) into main (6857320) will decrease coverage by 12.65%.
The diff coverage is 73.25%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3520       +/-   ##
===========================================
- Coverage   76.39%   63.74%   -12.65%     
===========================================
  Files         209      198       -11     
  Lines       14310    13547      -763     
===========================================
- Hits        10932     8636     -2296     
- Misses       2737     4355     +1618     
+ Partials      641      556       -85     

probes/hasBadgeNotFound/def.yml Outdated Show resolved Hide resolved
probes/hasBadgeNotFound/def.yml Outdated Show resolved Hide resolved
probes/hasBadgeNotFound/def.yml Outdated Show resolved Hide resolved
probes/hasBadgeNotFound/def.yml Outdated Show resolved Hide resolved
probes/hasBadgeNotFound/def.yml Outdated Show resolved Hide resolved
probes/hasGoldBadge/def.yml Outdated Show resolved Hide resolved
probes/hasGoldBadge/def.yml Outdated Show resolved Hide resolved
probes/hasGoldBadge/def.yml Outdated Show resolved Hide resolved
probes/hasGoldBadge/impl.go Outdated Show resolved Hide resolved
probes/hasUnknownBadge/def.yml Outdated Show resolved Hide resolved
@AdamKorcz AdamKorcz temporarily deployed to gitlab October 3, 2023 15:01 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 3, 2023 15:01 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to gitlab October 3, 2023 15:04 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 3, 2023 15:05 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to gitlab October 3, 2023 15:11 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 3, 2023 15:11 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to gitlab October 5, 2023 16:12 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 5, 2023 16:12 — with GitHub Actions Inactive
@AdamKorcz
Copy link
Contributor Author

@laurentsimon @spencerschrock PTAL again.

@evverx
Copy link
Contributor

evverx commented Oct 9, 2023

I don't think #3520 (comment) was addressed. That paragraph is still misleading and "that" hasn't been removed.

The OpenSSF Best Practices badge indicates whether or not that the project uses a set of security-focused best development practices for open source software.

@AdamKorcz AdamKorcz temporarily deployed to gitlab October 9, 2023 19:42 — with GitHub Actions Inactive
@AdamKorcz AdamKorcz temporarily deployed to integration-test October 9, 2023 19:43 — with GitHub Actions Inactive
@AdamKorcz
Copy link
Contributor Author

I don't think #3520 (comment) was addressed. That paragraph is still misleading and "that" hasn't been removed.

The OpenSSF Best Practices badge indicates whether or not that the project uses a set of security-focused best development practices for open source software.

Missed that the other probes had the same issue. Fixed now.

@evverx
Copy link
Contributor

evverx commented Oct 9, 2023

To be honest I'm much more interested in getting the wording right :-) It's just that I don't think that badges should be conflated with the best practices. All this check shows is that projects have badges and it doesn't necessarily mean that they actually follow those practices. Projects without badges can follow the practices too.

@laurentsimon
Copy link
Contributor

To be honest I'm much more interested in getting the wording right :-) It's just that I don't think that badges should be conflated with the best practices. All this check shows is that projects have badges and it doesn't necessarily mean that they actually follow those practices. Projects without badges can follow the practices too.

I think it's fine to reword. Any suggestion? I'm not sure which best practices are automatically checked, and which are not (in which case the best practice badge is essentially about education; and that's something we could highlight, eg maintainers have followed an introductory course on ...)

@david-a-wheeler please feel free to suggest

@evverx
Copy link
Contributor

evverx commented Oct 11, 2023

Any suggestion?

To judge from https://github.com/coreinfrastructure/best-practices-badge/blob/main/docs/vetting.md "The best practices badge approach is based on self-certification" with some basic checks so I'd say it's supposed to encourage good practices and let projects self-assess themselves. Personally I think it's too vague to be useful in automated tools like scorecard. I'm not sure why projects with no badges are downgrade either.

that's something we could highlight, eg maintainers have followed an introductory course on

I think it's being discussed in #3534

@david-a-wheeler
Copy link
Contributor

Personally I think it's too vague to be useful in automated tools like scorecard.

They aren't vague at all. They are carefully worded requirements. Most of them can't be directly measured by Scorecard, so the best practices badge provides a way to measure many criteria that otherwise Scorecard cannot measure.

I'm not sure why projects with no badges are downgrade either.

Because a project not pursuing a badge is unlikely to meet its requirements. Once it starts pursuing a badge it's more likely to reach them.

@david-a-wheeler
Copy link
Contributor

All this check shows is that projects have badges and it doesn't necessarily mean that they actually follow those practices

It does in general mean they follow the practices. If it's automated then it's checked. If it's a human making a claim, then it requires a listed human who is asserting that it's met, typically with a justification. The claim is also posted for all to see. We don't get many lies; if someone does lie, the badge is pulled.

@evverx
Copy link
Contributor

evverx commented Oct 11, 2023

It does in general mean they follow the practices

It does not. I often look at projects with badges and they are usually out-of-date because they were received once when companies felt the need to show their commitment to security by participating in that program and have never been updated since then. Basically it's necessary to manually verify all the claims but if I have the resources and the expertise to verify all those claims I'm not exactly sure how the badge is supposed to help me to evaluate anything.

@david-a-wheeler
Copy link
Contributor

It does not. I often look at projects with badges and they are usually out-of-date

If you have specific cases, file an issue & let's fix them. We're planning to improve the automation & rechecking of results, that should also help in some cases.

But ignoring the best practices badge information isn't an improvement.

Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: AdamKorcz <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
Signed-off-by: Adam Korczynski <adam@adalogics.com>
@spencerschrock
Copy link
Contributor

/scdiff generate CII-Best-Practices

Copy link

@spencerschrock spencerschrock merged commit 9b5d762 into ossf:main Nov 28, 2023
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants