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

QUA-678: Update golang crypto dependency version #497

Merged
merged 3 commits into from
Aug 9, 2022

Conversation

camillof
Copy link
Contributor

@camillof camillof commented Aug 2, 2022

Context

As reported on #496 a golang crypto dependency has a vulnerability on the version used by the test-reporter:
https://security.snyk.io/vuln/SNYK-GOLANG-GOLANGORGXCRYPTO-2825234

Details

This PR updates the golang.org/x/crypto dependency from revision 04eae0b62feaaf659a0ce2c4e8dc70b6ae2fff67 to revision 630584e8d5aaa1472863b49679b2d5548d80dcba. Gopkg.lock diff can be large because of format change on newer versions of dep but most important change is here: master...QUA-678/Fix-CC-reporter-go-crypto-vulnerability#diff-bbebd336cd92f353b3401e61be3cb9eb0267ea89704f556ce40c944cc5257e08R105

Snyk inspect before:
BEFORE

Snyk inspect after:
AFTER

Command run to update the dependency:
dep ensure -update golang.org/x/crypto

Closes #496

@CLAassistant
Copy link

CLAassistant commented Aug 2, 2022

CLA assistant check
All committers have signed the CLA.

@camillof camillof requested review from a team, giordanoluzardo and larkinscott and removed request for a team August 2, 2022 14:52
Copy link

@giordanoluzardo giordanoluzardo left a comment

Choose a reason for hiding this comment

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

LGTM @camillof 🚀

Copy link
Contributor

@fede-moya fede-moya left a comment

Choose a reason for hiding this comment

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

dep was deprecated at the beginnings of 2020 - https://github.com/golang/dep#dep

NOTE: Dep was an official experiment to implement a package manager for Go. As of 2020, Dep is deprecated and archived in favor of Go modules, which have had official support since Go 1.11. For more details, see https://golang.org/ref/mod.

There is an open PR that introduces the use of go modules #492. It would be great if we could put this update in the queue. However, the snyk captures are really useful proving these changes resolve the vulnerability. But I'm not sure the tests for the mac-os build have actually run, can we confirm that ?

Congrats on your first PR 🎉

@camillof camillof force-pushed the QUA-678/Fix-CC-reporter-go-crypto-vulnerability branch from 739d10e to 3506b46 Compare August 4, 2022 17:46
@camillof camillof changed the base branch from master to Fix-xcode-install-go-CircleCi August 4, 2022 17:46
@camillof camillof changed the base branch from Fix-xcode-install-go-CircleCi to master August 4, 2022 18:00
@camillof
Copy link
Contributor Author

camillof commented Aug 4, 2022

dep was deprecated at the beginnings of 2020 - https://github.com/golang/dep#dep

NOTE: Dep was an official experiment to implement a package manager for Go. As of 2020, Dep is deprecated and archived in favor of Go modules, which have had official support since Go 1.11. For more details, see https://golang.org/ref/mod.

There is an open PR that introduces the use of go modules #492. It would be great if we could put this update in the queue. However, the snyk captures are really useful proving these changes resolve the vulnerability. But I'm not sure the tests for the mac-os build have actually run, can we confirm that ?

Congrats on your first PR 🎉

Nice catch, indeed, the tests for mac-os build weren't running for a long time. A PR to fix that was merged here #498 so I rebased this branch, and now they are running just fine

I agree on stop using Dep, are you ok with me creating a new task for this?? As this PR could unblock the user who requested this change, and we can continue working on that migration?

@camillof camillof requested a review from fede-moya August 4, 2022 19:35
@fede-moya
Copy link
Contributor

@camillof

As this PR could unblock the user who requested this change, and we can continue working on that migration?

sounds good 👍🏼

@fede-moya
Copy link
Contributor

@camillof can you open one of these https://github.com/codeclimate/test-reporter/pull/489/files once you merge this one ?

@camillof camillof merged commit 89bfd8d into master Aug 9, 2022
@camillof
Copy link
Contributor Author

camillof commented Aug 9, 2022

@camillof can you open one of these https://github.com/codeclimate/test-reporter/pull/489/files once you merge this one ?

Thanks for the heads up! Here it is: #500

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.

Go Cryptography vulnerabilities detected by AWS Inspector
5 participants