-
Notifications
You must be signed in to change notification settings - Fork 74
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
Replace directive is based on the to be replaced module path not the one which replacing it #256
Comments
Very interesting. To make sure I'm following correctly, this example implies an odd mismatch of We would expect I think I see a solution -> when we encounter a |
Yes, you followed it correctly. The example was there so we could have a common understanding about the issue, and to help reproduce/check if the future fix is sufficient. |
@SzekeresB Thanks for verifying. Could you give a quick look over PR #257 ? |
@bhamail I looked it over. The PR is looks good for me, (but I'm not familiar with nancy's code). I even checked your branch out, built it and run some dependency check with it. The reports now looks good for me (I don't see anything odd with them). Thanks for the fast fix! |
Thanks for creating an issue! Please fill out this form so we can be
sure to have all the information we need, and to minimize back and forth.
I'm trying to scan for dependency issues, add mitigation to them and rescan for result nancy repot seems incorrect.
Here's an example, so you could also recreate the issue.
Clone out a repo that contains indirect dependency, try to replace it with an equivalent fork, and look for the results.
Just to make it more visible I prepared an example.
The repo is: https://github.com/oauth2-proxy/oauth2-proxy
A good candidate for the demonstration is the
v7.1.2
tag.So the commands so far:
So If everything is right you are in this commit:
Running nancy against it reveals an issue:
I'm not gonna copy past the full report the relevant part is it found the issue (that's good)
The issue we are looking for is :
Let's make some mitigation, I would like to replace the
github.com/dgrijalva/jwt-go
dependency withgithub.com/golang-jwt/jwt/v4 v4.1.0
.For that, I add the replace directive but It would fail unless I also add the
github.com/golang-jwt/jwt/v4
with a different version to the required part of the code. This issue is probably on the golang side nothing to do with Nancy.So after applying the workaround to replace
dgrijalva/jwt-go
withgithub.com/golang-jwt/jwt/v4
in indirect dependienciesThe below golang error should disappear:
This means the diff should look like this for
go.mod
:Hit
go mod tidy
sogo.sum
would catch up.Now we have mitigation in place so nancy should be fine, let's run it again.
The report reveals the following, it indeed does not have any vulnerability:
"vulnerable": []
But these coordinates seems odd:It is so odd that the
github.com/dgrijalva/jwt-go
only haspreview-4.0.0
release as of today. So finding the vulnerability in the4.1.0
would be an interesting achievement.Normal functioning.
Probably fine tune the recently introduced replace directive a bit.
Respect go mod replace directive #255
I think the example that I provided covers everything.
cc @bhamail / @DarthHater
The text was updated successfully, but these errors were encountered: