Skip to content

ci: add Go vulnerability checker per PR and to Makefile#9873

Closed
odeke-em wants to merge 1 commit intotendermint:mainfrom
orijtech:Makefile-add-govulncheck
Closed

ci: add Go vulnerability checker per PR and to Makefile#9873
odeke-em wants to merge 1 commit intotendermint:mainfrom
orijtech:Makefile-add-govulncheck

Conversation

@odeke-em
Copy link
Contributor

Adds the ability to run the Go vulnerability checker to be run on all tests in continuous integration. This helps us flag vulnerable code proactively and massively increase supply chain security.

Fixes #9872

Adds the ability to run the Go vulnerability checker to
be run on all tests in continuous integration. This helps us
flag vulnerable code proactively and massively increase supply
chain security.

Fixes #9872
@odeke-em odeke-em requested a review from ebuchman as a code owner December 13, 2022 03:57
@odeke-em odeke-em requested a review from a team December 13, 2022 03:57
@sergio-mena
Copy link
Contributor

sergio-mena commented Dec 13, 2022

Hi @odeke-em, could you give us a bit more information on what kind of problems this tool will help us spot?

EDIT: OK I see you wrote about it in #9873

Copy link
Contributor

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM

Comment on lines +83 to +84
- name: "Go vulnerability check"
run: make vulncheck
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular reason why the vulnerability check is part of the build workflow?

I'd prefer it if it were its own standalone workflow, so we can clearly distinguish between build failures and vulnerability-related failures in CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You def don't want builds to succeed with vulnerabilities. However if you want the override, just caution that many passes can be made optional and if builds succeed, people can ignore them. Help me with a confirmation that you'd prefer it as a different pass.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd just add the vulnerability check workflow as a required workflow, like our other required workflows.

Not sure why the Build workflow isn't required at the moment 😄 I'll fix that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the Build workflow isn't required at the moment

Ah, because the v0.34 branch doesn't have a separate "Build" workflow (it's part of the "Tests" workflow there).

Another reason to add this vulnerability check as a distinct workflow, because then it can be automatically backported with minimal effort to the release branches.

@thanethomson thanethomson mentioned this pull request Dec 16, 2022
3 tasks
@thanethomson
Copy link
Contributor

Superseded by #9903. Thanks @odeke-em for the suggestion!

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

Successfully merging this pull request may close these issues.

ci: add Go vulnerability checker to proactively flag vulnerable code

3 participants