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

Bring CLOMonitor Score to 100% #9769

Closed
19 of 21 tasks
eddie-knight opened this issue Oct 7, 2022 · 9 comments · Fixed by #12032
Closed
19 of 21 tasks

Bring CLOMonitor Score to 100% #9769

eddie-knight opened this issue Oct 7, 2022 · 9 comments · Fixed by #12032
Assignees
Labels
area/build Build or GithubAction/CI issues area/docs Incorrect, missing, or mistakes in docs type/feature Feature request type/security Security related

Comments

@eddie-knight
Copy link
Contributor

eddie-knight commented Oct 7, 2022

Per discussion in slack, this issue is to track the efforts necessary to bring argo-workflow CLOMonitor score to 100% as part of the CNCF security slam.

Below is a checklist of action items. I will add comments if there is anything that can't be addressed or if I am unsure of how to address it. It is possible to exclude checks with a written justification if that becomes necssary.

CLOMonitor report

Summary

Repository: argo-workflows
URL: https://github.com/argoproj/argo-workflows
Checks sets: CODE
Score: 88

Checks passed per category

Category Score
Documentation 100%
License 75%
Best Practices 100%
Security 80%
Legal n/a

Checks

Documentation [100%]

License [75%]

  • Apache-2.0 (docs)
  • Approved license (docs)
  • License scanning (docs)

Best Practices [100%]

Security [80%]

For more information about the checks sets available and how each of the checks work, please see the CLOMonitor's documentation.

@eddie-knight
Copy link
Contributor Author

The Token-Permissions check is actively being refined right now, so we shouldn't put too much thought into that until this issue closes: ossf/scorecard#2338

@agilgur5 agilgur5 added area/docs Incorrect, missing, or mistakes in docs area/build Build or GithubAction/CI issues labels Sep 22, 2023
@agilgur5 agilgur5 self-assigned this Sep 22, 2023
@agilgur5 agilgur5 added the type/security Security related label Sep 22, 2023
@agilgur5
Copy link
Contributor

agilgur5 commented Sep 22, 2023

  • Signed releases (docs)

This was completed in #9837

  • License scanning (docs)

And there's a new one now:

Assigning to myself right now as I may be working on the related SLSA Level 3, Level 4 checks soon. But anyone can feel free to add as well!

@agilgur5
Copy link
Contributor

agilgur5 commented Oct 18, 2023

I believe Snyk's security scans do license checks as well, so I think this might just need a badge on the README to satisfy.

Welp, never mind, apparently Snyk does not support this for Go deps (see also the CLOMonitor issue: cncf/clomonitor#50 (comment)). So I integrated FOSSA same as Argo CD (see also #12023 that was made automatically).

So am gonna make a PR for that badge for CLOMonitor and that should bring our non-security scores at 100%. EDIT: See #12032

I'm going to open a separate issue for the security scores / OpenSSF checks so that we can close this one out and that one can be more focused. There's also a few other newer OpenSSF checks as well, and Token-Permissions seems to be refined now. EDIT: See #12031

@agilgur5
Copy link
Contributor

agilgur5 commented Oct 20, 2023

The Token-Permissions check is actively being refined right now, so we shouldn't put too much thought into that until this issue closes: ossf/scorecard#2338

@eddie-knight curious if the SECURITY-INSIGHTS.yml checks ("Dependencies policy", "Security insights", and "Self-Assessment) are going through similar refinement this time around? They aren't listed under OpenSSF Scorecard's checks right now.

I also haven't seen any project (including k8s itself) adopt a SECURITY-INSIGHTS.yml other than some of the OpenSSF related repos.

@eddie-knight
Copy link
Contributor Author

eddie-knight commented Oct 20, 2023

Definitely @agilgur5. The SI was adopted by CLOMonitor (not via Scorecard) at the beginning of this month in order to streamline the hygiene checks you listed there.

If there's a clarity gap I'd love to get more insight from you so that I can make sure to improve the related supporting material for the Slam.

@agilgur5
Copy link
Contributor

agilgur5 commented Oct 20, 2023

The SI was adopted by CLOMonitor (not via Scorecard) at the beginning of this month in order to streamline the hygiene checks you listed there.

Is it going to be added to Scorecard? It's a bit confusing that most, but not all, of the security checks are via Scorecard

If there's a clarity gap

Not really a clarity gap, I understand what SECURITY-INSIGHTS.yml is for and have read the spec etc. I am bit surprised that CLOMonitor is pushing it despite current lack of adoption though, hence why I asked (especially since Token-Permissions went through some refinement too, so there is history for that).

More specifically, the lack of adoption for it currently (including in Scorecard) combined with how much information there is to fill out for it (plus yet another file at the root of the repo) makes the effort/value ratio feel not really worthwhile right now.
Especially as SECURITY-INSIGHTS.yml does not really improve any security on its own as it's just a metadata file; feels like there are (much?) more valuable uses of time for the same effort (e.g. adding SLSA Level 3+ provenance, adding CodeQL, fixing vulns per #12031, adding step-security/harden-runner, etc).

@eddie-knight
Copy link
Contributor Author

I can't speak on the Scorecard roadmap (but there are folks in the CNCF Slack #security-slam channel who can). I believe there is discussion of extending Scorecard to reference the SI in situations where less-predictable things such as SBOM artifacts are needing to be detected.

That's the idea behind integrating it into CLOMonitor— we tried to capture the results of those discussions in a CLOMonitor GitHub issue for posterity. You can see there was debate around whether the new checks should be in the Documentation or Code check set... but the intent at the end of the day is to use these new checks to find the harder-to-detect elements that TAG Security recommends.

@agilgur5
Copy link
Contributor

agilgur5 commented Oct 27, 2023

Yea I guess I am confused as to the difference between CLOMonitor's security checks and Scorecard's checks; one would think they'd be the same.

I see that Security Policy and SBOMs are also CLOMonitor-only.
It could make sense if Scorecard was all quantitative things and CLOMonitor was all metadata, but that's also not the case right now since Scorecard has "OpenSSF Best Practices Badge", "License", etc (also "Maintained", which is more of an opinionated check).
Some standardization and/or explicit delegation of checks between the two would be good to see.

More specifically, the lack of adoption for it currently (including in Scorecard) combined with how much information there is to fill out for it (plus yet another file at the root of the repo) makes the effort/value ratio feel not really worthwhile right now.

Well ok this got completed in CD argoproj/argo-cd#16135 so the effort is now significantly reduced as Workflows would be very similar

@eddie-knight
Copy link
Contributor Author

CLOMonitor currently runs a subset of scorecard checks and run a few additional checks that have been recommended by either TAG Security or maintainers of graduated CNCF projects.

To compare and contrast a bit... In scorecard, nobody is reasonably expected to have a full 10/10 score, so checks can't be optionally skipped. In contrast, projects can skip CLOMonitor checks (including the checks that harness Scorecard) by providing a written justification in the CLOMonitor config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues area/docs Incorrect, missing, or mistakes in docs type/feature Feature request type/security Security related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants