feat: Scan Homebrew inventory using git repository metadata#2510
feat: Scan Homebrew inventory using git repository metadata#2510another-rex merged 4 commits intogoogle:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2510 +/- ##
==========================================
- Coverage 71.67% 71.60% -0.07%
==========================================
Files 164 164
Lines 12081 12090 +9
==========================================
- Hits 8659 8657 -2
- Misses 2794 2805 +11
Partials 628 628 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Sorry for the delay in review, this is still on my radar, I will try to get around to it this week! |
another-rex
left a comment
There was a problem hiding this comment.
Nice! Looks great! One thing that could make this better is to add a docker image that uses homebrew itself to add the package (we can pin it to a specific version), and then we scan that image. (As a test under scan image
internal/imodels/imodels.go
Outdated
| newEco, err := osvecosystem.Parse("GIT") | ||
| if err != nil { | ||
| cmdlogger.Warnf("Warning: error parsing osvscanner.json ecosystem: %s", err.Error()) | ||
| return eco | ||
| } |
There was a problem hiding this comment.
I think this error message is incorrect, we should use MustParse here (if available, or just panic), no need to error since GIT should always successfully parse.
There was a problem hiding this comment.
Using MustParse solves this
internal/imodels/imodels.go
Outdated
| // Special case for Homebrew where we check if the source code repo is set, and if so, we set the ecosystem to GIT | ||
| // since Homebrew doesn't have a defined ecosystem in OSV, but if we have a source code repo, | ||
| // it's likely that the vulnerabilities will be associated with the source code repo and not the Homebrew package itself | ||
| if _, ok := pkg.Metadata.(*homebrewmetadata.Metadata); ok { |
There was a problem hiding this comment.
I think this whole thing can be more generic, if eco is empty, but source code repo name is filled out, it should just return the GIT ecosystem.
Could be done after scalibr PR |
19ede27 to
65e4dc2
Compare
|
I've added image test and updated some snapshots. It works locally, but CI is red in my fork. Not all tests work properly on my Ubuntu system, so how can I update the snapshots belonging only to my new test? |
|
You can use # runs just the one est
UPDATE_SNAPS=true go test ./cmd/osv-scanner/scan/... -run 'TestCommand_OCIImage/scanning_ubuntu_image_with_homebrew_extractor'
# runs a collection subtests
UPDATE_SNAPS=true go test ./cmd/osv-scanner/scan/... -run 'TestCommand_HomebrewWithAnnotators'
let us know if this is not related to your specific setup, as the majority of the test suite should be runnable on standard Ubuntu and friends without any extra setup (there are a few "exotic" tests like for Rust reachability which requires Rust, but they shouldn't be in the |
|
Thanks, @G-Rath. I've fixed an issue. It seems that the tests related to this PR are now working. |
another-rex
left a comment
There was a problem hiding this comment.
This looks great!
Apart from that comment on the dockerfile, LGTM.
| USER linuxbrew | ||
| WORKDIR /home/linuxbrew | ||
|
|
||
| RUN /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" |
There was a problem hiding this comment.
I'm a bit concerned about running an unpinned bash script just from the internet, we want to be as deterministic as possible in our tests to avoid it from changing the scan results without us changing the test files.
Pinning the official docker image for homebrew might be better, something like this
FROM ghcr.io/homebrew/ubuntu22.04:5.1.4@sha256:6b3c4bc0a7128cf5a78d2e641da6e88ac4195714e1315c4d2b522532d7fb1e7a
USER linuxbrew
WORKDIR /home/linuxbrew
ENV HOMEBREW_NO_AUTO_UPDATE=1 \
NONINTERACTIVE=1
# Install vulnerable package
RUN brew install cjson
# Make it vulnerable :)
RUN mv .linuxbrew/Cellar/cjson/* .linuxbrew/Cellar/cjson/1.7.17
There was a problem hiding this comment.
I used the raw homebrew installation as stated in the docs because the official homebrew image is heavy (up to 2 GB) and it takes extra time to run CI (prepare_test_image_testdata check: 4 minutes vs 7 minutes with ghcr.io/homebrew). I replaced the Dockerfile with the suggested one, and it seems to be working too.
f945d21 to
2e746b6
Compare
| // If ecosystem is empty and the source code repo is set we set the ecosystem to GIT | ||
| // since it's likely that the vulnerabilities will be associated with the source code repo | ||
| if eco.Ecosystem == "" && pkg.SourceCode != nil { | ||
| eco = osvecosystem.MustParse("GIT") |
There was a problem hiding this comment.
Was wondering why the tests are failing. Turns out our offline scanning has an assumption that Git repos have no ecosystem rather than GIT.
Pushed a fix for this and updated the snapshots
Hi!
In this PR, I've tried to provide a way of scanning homebrew inventories.
It refers to issues in scalibr and scanner.