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

Feature: maintainer annotation #1907

Open
laurentsimon opened this issue May 12, 2022 · 6 comments
Open

Feature: maintainer annotation #1907

laurentsimon opened this issue May 12, 2022 · 6 comments
Assignees
Labels
kind/enhancement New feature or request

Comments

@laurentsimon
Copy link
Contributor

Certain maintainers disagree or are unable to satisfy certain Scorecard checks. It would be useful to provide a way for them to explain their reasoning, if they want. We can surface this in the results next to each non-compliant alert.

Ideally this would be an inlined comment, similar to linter comments - in Go we use // nolinter, for example.

In some cases, it may require a config file (like the OSSF SECURITY-INSIGHT.md). But I'd like to see if we can start with inlined "annotations", because this makes it easier to keep the annotation in sync with the code.

Immediate use case: OSS-Fuzz cannot be pinned by hash because of the way it's setup.

Note: the SECURITY-INSIGHT.md is more appropriate for repo-wide insight, like "my unit tests are stored in this folder". (I'm unable to find the repo for it)

@evverx
Copy link
Contributor

evverx commented Nov 23, 2022

It would be useful to provide a way for them to explain their reasoning, if they want. ... OSS-Fuzz cannot be pinned by hash because of the way it's setup

I wonder why maintainers should waste their time explaining their reasoning? I kind of get that to avoid wasting even more time on receiving bogus promotional PRs triggered semi-automatically (and fixing imaginary "security" issues) it would probably be easier to mark CIFuzz/CFLite as false positives once and for all but I'm not sure why all the options boil down to wasting maintainers' time instead of choosing sensible defaults.

@eddie-knight
Copy link
Contributor

I really like how CLOMonitor displays the justification for exempt checks:

image

@evverx
Copy link
Contributor

evverx commented Nov 23, 2022

Looking at https://clomonitor.io/docs/topics/checks/#exemptions it seems CLOMonitor can't disable checks partially in the sense that if it supported the "Pinned-Dependencies" check it would only be possible to turn it off entirely instead of excluding CIFuzz and CFLite. Even if it was possible to mark only those two actions someone would have to spend their time on adding those exemptions to every repository where those actions are used and I think that would be a waste of time as well.

Then again, I don't care about those false positives much but when those false positives turn into semi-automated PRs wasting maintainers time I don't think it's useful. In the case of CFLite those PRs actually can be considered malicious because they pin the action only without the docker images, which can lead to potentially half-broken fuzzing infrastructure and that isn't ideal because it's used to make sure that patches fixing buffer overflows, UAFs and stuff like that are backported correctly.

@github-actions
Copy link

This issue is stale because it has been open for 60 days with no activity.

Copy link

This issue is stale because it has been open for 60 days with no activity.

Copy link

This issue has been marked stale because it has been open for 60 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
Status: In Progress
Development

No branches or pull requests

7 participants