Add support for detecting vulnerable Go packages in PRs #1024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Builds atop PR #1012 by supporting lookup of Go dependencies in OVS as well as their checksums in the Go sum database. Because we need to do another lookup in the go sum database, we also need to keep state during the lookups, so I collapsed the two methods of the RepoQuerier interface into one. Fixes: #912
When I renamed the PR scan actions, I forgot to rename the associated constant in the code. This was not caught by unit tests because the unit tests use the constant directly, but using a policy that set the "review" action would have triggered an error.
…onstant to string on every iteration OSV itself is case-sensitive, but that doesn't make a whole lot of sense for us, let's just treat the package databases in a case-insensitive manner.
This was a remnant of adopting Luke's old code - the way we looked up which part of the PR to patch was specific to JS. Let's add a method to the patch object that is able to find where to apply the patch.
Just some code hardening
JAORMX
reviewed
Sep 26, 2023
| @@ -98,7 +108,7 @@ func newNpmRepository(endpoint string) *npmRepository { | |||
| // check that npmRepository implements RepoQuerier | |||
| var _ RepoQuerier = (*npmRepository)(nil) | |||
|
|
|||
| func (n *npmRepository) NewRequest(ctx context.Context, dep *pb.Dependency) (*http.Request, error) { | |||
| func (n *npmRepository) newRequest(ctx context.Context, dep *pb.Dependency) (*http.Request, error) { | |||
| pkgUrl := fmt.Sprintf("%s/%s/latest", n.endpoint, dep.Name) | |||
There was a problem hiding this comment.
I know it's not from this PR, but can we change from Sprintf to using url.Url instead in the near future? Having manual string operations like this could backfire since we're relying on rule authors fore these parameters. Lets treat it as untrusted input and validate instead.
JAORMX
reviewed
Sep 26, 2023
| var _ RepoQuerier = (*goProxyRepository)(nil) | ||
|
|
||
| func (r *goProxyRepository) goProxyRequest(ctx context.Context, dep *pb.Dependency) (*http.Request, error) { | ||
| pkgUrl := fmt.Sprintf("%s/%s/@latest", r.proxyEndpoint, dep.Name) |
There was a problem hiding this comment.
should we use url.Url here instead? https://pkg.go.dev/net/url#URL
| } | ||
|
|
||
| func (r *goProxyRepository) goSumRequest(ctx context.Context, depName, depVersion string) (*http.Request, error) { | ||
| sumUrl := fmt.Sprintf("%s/lookup/%s@%s", r.sumEndpoint, depName, depVersion) |
There was a problem hiding this comment.
same here https://pkg.go.dev/net/url#URL
lukehinds
approved these changes
Sep 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
vulncheck: Support Go depenndecies in PR vulnerability scanner
Builds atop PR #1012 by supporting lookup of Go dependencies in OVS as well as their checksums in the Go sum database.
Because we need to do another lookup in the go sum database, we also need to keep state during the lookups, so I collapsed the two methods of the RepoQuerier interface into one.
Fixes: #912
The other patches in the PR fix issues that I found during testing.
To view the new code in action, see
jakubtestorg/bad-go#3 - you might want to click the
show resolved to see the diff that Mediator was suggesting.