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

Security scanning in PR actions #178

Open
jon-whit opened this issue Feb 20, 2024 · 4 comments
Open

Security scanning in PR actions #178

jon-whit opened this issue Feb 20, 2024 · 4 comments

Comments

@jon-whit
Copy link

👋 We use grpc-health-probe in the OpenFGA project, and we actually embed the binary in our built images. We do this so that we can ship a single image but reference different target entrypoints. This is particularly handy for tooling such as Helm charts and Docker Compose, because it allows us to reduce our dependency to just our image which includes the tools needed for health checks and to run the server.

We've quite regularly noticed that dependencies of grpc-health-probe have unresolved vulnerabilities reported.
Screenshot 2024-02-20 at 11 24 35 AM

Security vulnerability scanning is much more common in cloud-native software delivery today, and since a broader gRPC community may depend on this tool, it would be nice if there was security scanning implemented on top of the artifacts produced by this project.

Would you be open to a PR that adds a vulnerability scan to the PR Github action workflow?

The most common scanners used include:

I would recommend at least adding scans using govulncheck and trivy as these are widely used across the CNCF ecosystem.

@ahmetb
Copy link
Collaborator

ahmetb commented Feb 20, 2024

No because the issues flagged by scanners don't come up during PRs. The project is largely in maintenance mode, yet so many useless scanners alerts are reported frequently due to vulns found in pkgs we import, albeit none of these are really applicable or exploitable in the conditions this tool is used in.

@jon-whit
Copy link
Author

jon-whit commented Feb 20, 2024

@ahmetb

The project is largely in maintenance mode

What is "maintenance mode" considered to be to your maintainer team? In my experience maintenance mode refers to patches and vulnerability fixes, but no new major features etc.. But I understand that may differ project to project.

No because the issues flagged by scanners don't come up during PRs.

How do you figure? If you upgrade a package in the scope of a PR, and if that package introduces a vulnerability that may be exploitable in your software artifacts, then is it not your responsibility as maintainers to make sure you do your dilligence to patch that by upgrading that dependency to the appropriate version including the fix? Likewise, is it not the projects responsibility to release patch fixes when new vulnerabilities are discovered that impact the software artifacts that have previously been released? At minimum it should be a maintainer team's responsibility to at least review vulnerability reports and ascertain whether that the vulnerability is or is not impacting your software. Otherwise it's pure negligence on the maintainers part.

@ahmetb
Copy link
Collaborator

ahmetb commented Feb 20, 2024

maintenance mode refers to patches and vulnerability fixes, but no new major features etc.

Correct. That's the stage the proejct is in.

How do you figure? If you upgrade a package in the scope of a PR, and if that package introduces a vulnerability that may be exploitable in your software artifacts

We have @dependabot that actively sends PRs for vulnerabilities. On top of that, GitHub has security checkers enabled in the repo and if we were merging something with vulns, it does alert.

But that doesn't happen because most all PRs we do nowadays is updating things to the very last version. So, as I said, vuln reports come in much later after the code is merged and the vuln is discovered; not "at the time of merging".

@jon-whit
Copy link
Author

@ahmetb gotcha. So it sounds like the majority of PRs are strictly for dependency updates and for patch fixes etc. As you said:

But that doesn't happen because most all PRs we do nowadays is updating things to the very last version. So, as I said, vuln reports come in much later after the code is merged and the vuln is discovered; not "at the time of merging".

Doesn't this highlight even more why it would be a good idea to accept a community contribution to setup a vulnerability scanning step in the Github actions workflow to prevent vulnerabilities from being introduced in these PRs? If these are the only kinds of PRs that are really getting merged today, then it seems better to block the PR before the code gets into the released artifact in the first place. Why would you not want to do that?

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

2 participants