-
Notifications
You must be signed in to change notification settings - Fork 0
chore: add empty dep-graph when there are no dependencies #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
2931a01 to
baf1368
Compare
c0d5ab3 to
5b5372d
Compare
1009858 to
171bf6c
Compare
6af74c1 to
3178baf
Compare
3efc404 to
03a1e8e
Compare
3178baf to
88d4414
Compare
ed74368 to
d0516fc
Compare
88d4414 to
c46066c
Compare
b8b3e7c to
0797713
Compare
135c2d5 to
e0d894c
Compare
e0d894c to
a6187de
Compare
a6187de to
47444b9
Compare
| Sbom: []byte(`{"mock":"sbom"}`), | ||
| Sbom: []byte(`{"mock":"sbom"}`), | ||
| Metadata: scaplugin.Metadata{ | ||
| PackageManager: "pip", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pip package manager confuses me. Why are we impersonating a different package manager which we already support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to be changed. We have an RCR pending second Jedi's review to add a new package manager to Registry. Once Registry understands uv, we can use it. Before then, not sure if we can use it and why risk it if we can easily adjust once all bits are in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I was missing this context.
internal/uv/uvclient.go
Outdated
|
|
||
| // Verifies that the SBOM is valid JSON and has a root component. | ||
| func validateSBOM(sbomData []byte) error { | ||
| func validateSBOM(sbomData []byte) (*scaplugin.Metadata, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe i'm getting hung up on the name too much, but this function does not seem to only validate, but also parse the input and now also return some metadata? should we name it differently to reflect this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, updated the name
pkg/depgraph/sbom_resolution.go
Outdated
| return depGraphList, nil | ||
| } | ||
|
|
||
| func workflowDataFromDepGraph(depGraph any, normalisedTargetFile, targetFileFromPlugin string) (workflow.Data, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the any type intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was as the dep-graph from the conversion endpoint was just being unmarshalled into an any, but after you mentioned it I thought it might be worth exploring unmarshalling into a proper DepGraph type. I had to implement a custom UnmarshalJSON method to achieve this as we only want to unmarshall the data to a DepGraph if it has type "depGraph", but it seems to work. Have a look at the latest commit and let me know what you think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! ok i didn't see that this was coming from this polymorphic "facts" array.
a13142e to
d2ed715
Compare
d2ed715 to
6f764e2
Compare
6f764e2 to
4d6f028
Compare
What this does
If a uv project has no dependencies then the SBOM will have no components with a pip PURL, e.g.:
{ "bomFormat": "CycloneDX", "specVersion": "1.5", "version": 1, "serialNumber": "urn:uuid:ac3b8db9-2725-4a70-b080-86bec40fe7db", "metadata": { "timestamp": "2025-11-24T15:20:24.447458000Z", "tools": [ { "vendor": "Astral Software Inc.", "name": "uv", "version": "0.9.11" } ], "component": { "type": "library", "bom-ref": "[email protected]", "name": "no-deps", "version": "0.1.0" } }, "components": [], "dependencies": [ { "ref": "[email protected]", "dependsOn": [] } ] }This means that, when the SBOM is converted to dep-graphs, no dep-graphs are returned as the conversion endpoint cannot determine the ecosystem.
To work around this, we decided to manually add an empty dep-graph in this case, which this PR implements. To achieve this I've added a new
Metadatafield to theFindingsstruct which contains enough information to construct an empty dep-graph. I'm not sure if this is the best approach so happy to hear alternative ideas here.