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

Pull request 1003 #1008

Merged
merged 3 commits into from
Oct 6, 2021
Merged

Conversation

HadwaAbdelhalem
Copy link
Contributor

Fixes #1003
Closes #1003

This PR updates the Auzre/autorest version to rid of the github.com/dgrijalva/jwt-go.

Link to CI note the failures in the current CI is the same on the Main branch CI .

The failures are are AKS due to this open issue on Azure/AKS. While the remaining are Azure VM related which I am currently investigating and will create issues to track them

Copy link
Contributor

@yorinasub17 yorinasub17 left a comment

Choose a reason for hiding this comment

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

Updates LGTM! Can you run go mod tidy at the root of this project? I ran it and it detected some additional packages to clear.

@@ -125,6 +127,8 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumC
github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this doesn't fully resolve the issue since the vulnerable package is still being pulled in, but go mod why indicates that we are not using this code path since the packages we import are not pulling it in. So I think that is ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yorinasub17 , Yes I am not sure why it is still pulled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Digging into this further, this may actually be pulled in as a transient dependency of the azure-sdk-go version we are currently at: https://github.com/Azure/azure-sdk-for-go/blob/v46.0.0/Gopkg.lock#L31-L37

It looks like this specific dependency was updated in v50.0.0.

I'm not sure how disruptive it is to update the azure-sdk-for-go package, but we should consider updating to that version and above. Let me take a crack at that.

I'll go ahead and kick off a regression build with our test pipeline, and if that passes, I'll merge this in and start a new PR attempting to update the Azure SDK.

Copy link
Contributor

@yorinasub17 yorinasub17 left a comment

Choose a reason for hiding this comment

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

Precommit hook passed, so going to merge this in. Afterwards, I'll start hacking away at upgrading the azure-sdk-for-go dependency to see if it will bump the jwt-go dep.

@yorinasub17 yorinasub17 merged commit 770349f into gruntwork-io:master Oct 6, 2021
@yorinasub17
Copy link
Contributor

Thanks for the effort on this by the way!

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.

cve-2020-26160 vulnerability in dgrijalva/jwt-go
2 participants