osv-scanner
osv-scanner copied to clipboard
deduplicate sarif outputs for GitHub
Fixes #2331
I did use Copilot coding agent to help me with this but I've tweaked and sense checked the output. I hope it's acceptable, let me know if there are any problems :pray:
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
Hmm not sure what to do about "Copilot" not having signed the CLA? I guess if this solution looks good I can go rewrite the history back to me
To make the snapshot tests pass, you need to run with UPDATE_SNAPS=true env variable when running the tests.
Thanks @another-rex!
- I did refactor those tests to use more of a matrix approach - I think it's easier to see what's going on and be sure that all combinations are tested. I expect it to be less brittle for you going forward too.
- I also changed the signature tests to depend on the invariance rather than the specific hash values - again, I hope that means it's easier to see what's being tested, that it's comprehensive, and fewer changes to make if edge cases emerge.
- I think the snapshots are good
I'm not sure what to do about Copilot's authorship and the CLA - if I need to rewrite the history and force push or something I can do that, let me know.
Yes, you would have to rewrite copilots commit and force push here, thanks!
@another-rex hopefully the ownership is sorted now, CLA check seems happy now
OK I think that's snapshots updated and a couple of linter issues fixed.
OS-specific tests are failing, I think that's because the fingerprints depend on file paths, which vary by OS (both location and OS-specific formatting). Not sure what to do for the best - is a fingerprint be the same if the path to the file is different? Does it matter that a partialFingerprint for the same package doesn't match when you compare Windows and Ubuntu, but does match when it's seen more than once in the same OS? I don't think so?
A relatively simple way forward: remove the file element, so you get a duplicate when the CVE and package match regardless of where the file is. I think that should be robust across OS and fairly confident it solves the problem I have (screenshot)
.
Alternative - remove the comparison from the snapshot tests - probably trickier, may eliminate useful tests
Complex - try and deal with the cross-OS case - not sure this is really doable robustly, how would I compare CVE-1234-5678 occurring in file://var/lib/dpkg/status with some equivalent on Windows?
I'll go with the first option unless advised otherwise
Sorry for the noise @another-rex that lints and tests OK for me. I've enabled the checks pipeline in my own repo but I get failures uploading coverage reports and I can't tell whether the tests are passing otherwise. Anyway, hopefully they are!
Hmm... I think a fingerprint should depend on the filepath (I believe you would want to have two entries show up if you had two files with the same package/version),
For the snapshot tests, we can just replace it with a placeholder variable so it doesn't match. I pushed up a change with this replacement (needed to move a few packages around), let's see if this works.
Codecov Report
:x: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 67.82%. Comparing base (b8a7531) to head (b37363e).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| internal/testutility/jsonreplace.go | 80.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #2333 +/- ##
==========================================
+ Coverage 67.80% 67.82% +0.02%
==========================================
Files 171 171
Lines 13005 13018 +13
==========================================
+ Hits 8818 8830 +12
- Misses 3502 3503 +1
Partials 685 685
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Seems to work first try! Let me know if you're happy with this.
@G-Rath Can you take a quick look at my additional changes in case I missed anything obvious.
Works for me @another-rex thanks!! Now I don't have to write a hacky dedupe script in my own project :D