Scan submodules too. #581
Merged
Scan submodules too. #581
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
This seems to only be partially working currently, and I'm suspecting a bug in go-git with respect to submodules and recursion. ``` $ go run ./cmd/osv-scanner -r /tmp/pd-server/ Scanning dir /tmp/pd-server/ Scanning /tmp/pd-server/ at commit cf3f15a841ca21b53c6de654c9981a30ae0b590c Scanning submodule src/cpp-httplib at commit 0000000000000000000000000000000000000000 Scanning submodule pd-lib-builder at commit 5c2e137f7a7a03f4007494954ccb3e23753e7807 Scanning submodule src/json at commit 0000000000000000000000000000000000000000 Scanning submodule src/websocketpp at commit 0000000000000000000000000000000000000000 Scanned /tmp/pd-server/src/json/docs/mkdocs/requirements.txt file and found 49 packages Scanned /tmp/pd-server/src/json/tools/serve_header/requirements.txt file and found 2 packages ```
Codecov Report
@@ Coverage Diff @@
## main #581 +/- ##
==========================================
- Coverage 80.65% 80.29% -0.37%
==========================================
Files 78 78
Lines 5346 5379 +33
==========================================
+ Hits 4312 4319 +7
- Misses 866 885 +19
- Partials 168 175 +7
|
Is the bug you are referring to the 0000 hashes? I think that's because you are using Something I'm thinking about is whether we should be checking out the submodules and scanning the contents as well, but that's probably something we should put behind a flag since it makes changes to the file system. |
Is that all that's required to fix up this PR? It would be nice to get this in together with the CVE announcement.
Hmm, it seems to me that most contexts where OSV-Scanner would have this all set up already (i.e. developer machines, CI/CD), so unless there's a big ask we can probably skip this completely? |
@another-rex identified this solution to go-git/go-git#373
…anner into scan_submodules_too
At a functional level, yes, that plus what I've described at go-git/go-git#373 (comment)
Wow, that's incredible, and very unexpected (no pun intended). The lack of documentation for this particular code made this harder to work through than I'd have liked. For what it's worth I had checked out the submodules... By the looks of it, whilst this addresses the functional issue, for reasons unknown, it's still causing unexpected side-effects to the state of the submodules in the repository. Here's an end-to-end example with the current state of this PR: |
Open
|
Hmm looks like it only changes the status if the submodules are checked out, I'll look a bit more into this as well. |
5 tasks
|
So armed with the knowledge that gLinux's Git submodule behaviour is not consistent with upstream Git, rerunning this PR in a Cloud Shell against https://github.com/charlesneimog/pd-server @ cf3f15a841ca21b53c6de654c9981a30ae0b590c: |
oliverchang
approved these changes
Oct 30, 2023
oliverchang
requested changes
Oct 30, 2023
another-rex
approved these changes
Oct 30, 2023
This will include more precise information about the location of the vulnerability in the output: e.g. in https://github.com/charlesneimog/pd-server with submodules not initialized and at cf3f15a: ``` $ go run ./cmd/osv-scanner -r ../pd-server/ Scanning dir ../pd-server/ Scanning /home/apollock/pd-server/ at commit cf3f15a841ca21b53c6de654c9981a30ae0b590c Scanning submodule /home/apollock/pd-server/src/cpp-httplib at commit 227d2c20509f85a394133e2be6d0b0fc1fda54b2 Scanning submodule /home/apollock/pd-server/pd-lib-builder at commit 5c2e137f7a7a03f4007494954ccb3e23753e7807 Scanning submodule /home/apollock/pd-server/src/json at commit 4c6cde72e533158e044252718c013a48bcff346c Scanning submodule /home/apollock/pd-server/src/websocketpp at commit 1b11fd301531e6df35a6107c1e8665b1e77a2d8e ╭────────────────────────────────┬──────┬───────────┬─────────────────────┬─────────────────────┬──────────────────────────────╮ │ OSV URL │ CVSS │ ECOSYSTEM │ PACKAGE │ VERSION │ SOURCE │ ├────────────────────────────────┼──────┼───────────┼─────────────────────┴─────────────────────┼──────────────────────────────┤ │ https://osv.dev/CVE-2023-26130 │ 8.8 │ GIT │ 227d2c20509f85a394133e2be6d0b0fc1fda54b2 │ ../pd-server/src/cpp-httplib │ ╰────────────────────────────────┴──────┴───────────┴───────────────────────────────────────────┴──────────────────────────────╯ exit status 1 ```
oliverchang
approved these changes
Oct 30, 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.
Using https://github.com/charlesneimog/pd-server (at cf3f15a) as the example:
With submodules not initialized:
With submodules initialized: