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

Reintroduce a way to silence individual warnings #143

Open
jonasbb opened this issue Mar 14, 2022 · 8 comments
Open

Reintroduce a way to silence individual warnings #143

jonasbb opened this issue Mar 14, 2022 · 8 comments
Assignees

Comments

@jonasbb
Copy link
Contributor

jonasbb commented Mar 14, 2022

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
Copy link
Contributor

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
Copy link
Contributor

laurentsimon commented Mar 16, 2022

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 github/codeql-action#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
Copy link
Contributor Author

jonasbb commented Mar 16, 2022

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
Copy link
Contributor

laurentsimon commented Mar 16, 2022

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 github/codeql-action#986 to see if code-scanning platform can improve the way they show alerts.

@rneatherway
Copy link

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.

bors bot pushed a commit to jonasbb/serde_with that referenced this issue May 3, 2022
Without a way to supress analysis results more permanently the security
tab becomes too noisy. Either the action needs to allow an allowlist or
the GitHub UI needs to be better in permanently supressing lines.
The biggest annoyance is that each change to a action tag will trigger
a new warning, even if the same line was ignored before.

ossf/scorecard-action#143
bors bot added a commit to jonasbb/serde_with that referenced this issue May 3, 2022
446: Disable the OSSF Scorecard Action r=jonasbb a=jonasbb

Without a way to supress analysis results more permanently the security
tab becomes too noisy. Either the action needs to allow an allowlist or
the GitHub UI needs to be better in permanently supressing lines.
The biggest annoyance is that each change to a action tag will trigger
a new warning, even if the same line was ignored before.

ossf/scorecard-action#143

Co-authored-by: Jonas Bushart <[email protected]>
bors bot pushed a commit to jonasbb/serde_with that referenced this issue May 3, 2022
Without a way to supress analysis results more permanently the security
tab becomes too noisy. Either the action needs to allow an allowlist or
the GitHub UI needs to be better in permanently supressing lines.
The biggest annoyance is that each change to a action tag will trigger
a new warning, even if the same line was ignored before.

ossf/scorecard-action#143
bors bot added a commit to jonasbb/serde_with that referenced this issue May 3, 2022
446: Disable the OSSF Scorecard Action r=jonasbb a=jonasbb

Without a way to supress analysis results more permanently the security
tab becomes too noisy. Either the action needs to allow an allowlist or
the GitHub UI needs to be better in permanently supressing lines.
The biggest annoyance is that each change to a action tag will trigger
a new warning, even if the same line was ignored before.

ossf/scorecard-action#143

Co-authored-by: Jonas Bushart <[email protected]>
@laurentsimon
Copy link
Contributor

See github/codeql-action#986 (comment) for possible fix

@jonasbb
Copy link
Contributor Author

jonasbb commented Jul 19, 2022

@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
Copy link
Contributor

laurentsimon commented Jul 19, 2022

I don't disagree. We created #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

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

No branches or pull requests

4 participants