ossf / scorecard-action

Official GitHub Action for OpenSSF Scorecard.
Apache License 2.0
253 stars 70 forks source link

Reintroduce a way to silence individual warnings #143

Open jonasbb opened 2 years ago

jonasbb commented 2 years ago

Some warnings created by this action can be quite noisy, and there does not seem to be a way to silence certain error classes. It seems that in #16 this feature was removed.

The Pinned-Dependencies check creates many warnings for GitHub Actions if they are not pinned by hash. Every update to the pinned tag will re-surface the same line in the Security Dashboard, even if it was previously selected as "won't fix". This creates unnecessary many alarms and due to the sheer number of "uses" in actions is by far the largest contributor of warnings.

All of GitHub's documentation also uses the form actions/checkout@v3, so this check conflicts with that. Instead, this is recommended actions/checkout@5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f # v2.3.4. The problem here is that the comment will not get updated by tools such as dependabot and thus does not contribute meaningfully, even is harmful as it suggests a different version than actually used.

I can see why somebody would want this check, but for my projects I do not see this as a benefit and would like to disable the check.

azeemshaikh38 commented 2 years ago

Thanks for reporting this @jonasbb. This is on our roadmap, and we are still considering design options to achieve this in a more generic and granular way (for example, ignore only specific libraries in the Pinned-Dependencies check).

I'm surprised that the won't fix option re-surfaces the same issues. @laurentsimon is that expected?

laurentsimon commented 2 years ago

Thanks for the report, @jonasbb

re: GitHub doc conflict with hash pinning. We're aware that dependabot has this issue :/ Note, though, that GitHub doc does recommend pinning by hash, see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions but I don't think their workflow examples use it consistently :(

I'm also surprised the Won't Fix does not silence the alerts, but I suppose it's because they see a code change and do not take into account the fact that the only change was via a dependabot PR. I've created https://github.com/github/codeql-action/issues/986 an issue on their repo to request for this feature. Please let us know if you agree this would be a good long-term fix

jonasbb commented 2 years ago

I'm also surprised the Won't Fix does not silence the alerts,

Maybe I was a bit unclear on that point. Let me expand on that. I can select "Won't Fix" and the alert will be silenced until the offending line changes. Next dependabot will create a PR updating all actions/checkout@v2 to actions/checkout@v3. This closes all code scanning alerts for the old lines and creates a new code scanning alert for each uses: actions/checkout@v3 line.

I don't really know if a change like actions/checkout@v2 to actions/checkout@v3 should create a new code scanning alert or simply update the old one. However, since it creates a new alert, this feels quite invasive to me and forces me to apply "Won't Fix" to all actions/checkout@v3 alerts. An action like checkout can be used many times, thus drown out other alerts.

Alerts for other action lines are not impacted.

Note, though, that GitHub doc does recommend pinning by hash, see docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions but I don't think their workflow examples use it consistently :(

I understand the recommendation from a security perspective. For the small project where I am testing the action using hashes does not feel ergonomic enough. GitHub's docs heavily favor tags though, like on docs.github.com, and the action descriptions. I don't mind the alerts appearing by default, but being able to silence them would be great.

laurentsimon commented 2 years ago

I'm also surprised the Won't Fix does not silence the alerts,

Maybe I was a bit unclear on that point. Let me expand on that. I can select "Won't Fix" and the alert will be silenced until the offending line changes. Next dependabot will create a PR updating all actions/checkout@v2 to actions/checkout@v3. This closes all code scanning alerts for the old lines and creates a new code scanning alert for each uses: actions/checkout@v3 line.

I don't really know if a change like actions/checkout@v2 to actions/checkout@v3 should create a new code scanning alert or simply update the old one. However, since it creates a new alert, this feels quite invasive to me and forces me to apply "Won't Fix" to all actions/checkout@v3 alerts. An action like checkout can be used many times, thus drown out other alerts.

I agree no new alert should be created. I created https://github.com/github/codeql-action/issues/986 to see if code-scanning platform can improve the way they show alerts.

rneatherway commented 2 years ago

GitHub's docs heavily favor tags though, like on docs.github.com, and the action descriptions.

Tags are favoured for first-party actions (i.e. those under the github or actions orgs) because you can rely on GitHub not to introduce malicious code into them. If you don't trust GitHub, then using the Actions platform as a whole becomes difficult.

For third-party actions pinning to the sha is recommended.

laurentsimon commented 2 years ago

See https://github.com/github/codeql-action/issues/986#issuecomment-1171269954 for possible fix

jonasbb commented 2 years ago

@laurentsimon Fixing the “wrong” fingerprints, as mentioned in the linked issue, would definitely lessen the impact of the Pinned-Dependencies check. I do not see this as a fix to this issue, but rather a related problem. The issue here is rather that scorecard has decided to enforce certain policies, which, I feel, are too much of a burden to uphold. As such, constantly working around the security reports has the same downside, and the solution is to not use the action.

Even with a fixed fingerprint, the action will still warn on every newly introduced unpinned dependency or potentially more often. It really depends on the fingerprint.

laurentsimon commented 2 years ago

I don't disagree. We created https://github.com/ossf/scorecard-action/issues/729 to discuss customization of the Action. Can you share your thought in the issue? We'd love to hear if the proposed format would fit your needs, for example.

/cc @raghavkaul