feat: support parsing gradle/verification-metadata.xml
#943
Conversation
G-Rath
force-pushed
the
support-gradle-dv
branch
from
ade252f to
5c7d87b
Compare
G-Rath
force-pushed
the
support-gradle-dv
branch
2 times, most recently
from
5bc2c13 to
5109113
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #943 +/- ##
==========================================
+ Coverage 64.07% 64.14% +0.07%
==========================================
Files 146 147 +1
Lines 11983 12008 +25
==========================================
+ Hits 7678 7703 +25
Misses 3853 3853
Partials 452 452 ☔ View full report in Codecov by Sentry. |
| ) | ||
|
|
||
| type GradleVerificationMetadataFile struct { | ||
| // XMLName xml.Name `xml:"project"` |
There was a problem hiding this comment.
thanks for the reminder - I was meant to double check what this is actually doing and if we need it
| | Dart | `pubspec.lock` | | ||
| | Elixir | `mix.lock` | | ||
| | Go | `go.mod` | | ||
| | Java | `buildscript-gradle.lockfile`<br>`gradle.lockfile`<br>`pom.xml`[\*](https://github.com/google/osv-scanner/issues/35),<br>`gradle/verification-metadata.xml` | |
There was a problem hiding this comment.
should this be alphabetical order?
There was a problem hiding this comment.
oh yeah good point - I originally thought the * was applying to all of these but its actually just pom.xml so we could sort it alphabetically.
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string |
There was a problem hiding this comment.
delete name if it is not used in t.Run
There was a problem hiding this comment.
it is being used though? t.Run(tt.name, func(t *testing.T) {
I support not putting a name on this these tests as I think it's obvious what they do, but we've implicitly got the convention of always including the name field even if its blank for consistency and so that you can choose to name a new test in future.
|
I think I reviewed it and pending changes from @G-Rath |
|
Also when we last discussed this, I believe some research was going to be done into the Maven ecosystem to determine how appropriate supporting this file was, which was to be done by @cuixq |
There was a problem hiding this comment.
I forgot to comment last time with my findings: I think it makes sense to support vertification-metadata.xml. I can see this in open source projects - some of these projects may not have gradle.build so verification-metadata.xml definitely helps with understanding these projects.
This adds support for parsing `gradle/verification-metadata.xml` files - since this seems to be like an actual lockfile it's very straightforward: we just parse the file as XML and extract out the name + version of "component". The interesting part of this is that unlike other project-relative lockfiles this file currently must exist in the `gradle` directory which raises questions about how `--recursive` comes into play previously we'd not enabled APK and DPKG checking by default but I feel that was more because they were absolute paths and so didn't make sense to do when people were scanning in "project mode". For now I've just taken the simple route of making the file `gradle/verification-metadata.xml` since that does just work (except for the "find parser" flow which checks against `path.Base` so that has the `gradle` omitted). Resolves google#915
This adds support for parsing
gradle/verification-metadata.xmlfiles - since this seems to be like an actual lockfile it's very straightforward: we just parse the file as XML and extract out the name + version of "component".The interesting part of this is that unlike other project-relative lockfiles this file currently must exist in the
gradledirectory which raises questions about how--recursivecomes into play previously we'd not enabled APK and DPKG checking by default but I feel that was more because they were absolute paths and so didn't make sense to do when people were scanning in "project mode".For now I've just taken the simple route of making the file
gradle/verification-metadata.xmlsince that does just work (except for the "find parser" flow which checks againstpath.Baseso that has thegradleomitted).Resolves #915