[K9VULN-9195] Add Cargo.toml matcher to resolve metadata for rust dependencies#96
[K9VULN-9195] Add Cargo.toml matcher to resolve metadata for rust dependencies#96rjcoulter22 merged 16 commits intomainfrom
Cargo.toml matcher to resolve metadata for rust dependencies#96Conversation
Go test coverage reportTotal test coverage: 90.4% (4396/4862) Test coverage has changed in the current files, with 11 lines missing coverage. |
31414c3 to
a8bcfac
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
d827fdf to
3bc234e
Compare
Cargo.toml matcher to resolve metadata for rust dependencies
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
piloulacdog
left a comment
There was a problem hiding this comment.
Feedback related to tests:
-
to facilitate review examples, could we have,n the fixtures, always both the
Cargo.tomland associatedCargo.lock. It would facilitate later tests and understanding as we know what A (Cargo.toml) should lead to B (Cargo.lock). Even if for some we might not useCargo.lock, I think it's important for our understand to have both. An example here is that me who doesn't really know how A lead to B, I could quickly glance at the two files and quickly create the mental mapping. Having mostly only 1/2 makes it more complex. -
Could we also create a "complex" test in
cmd/datadog-sbom-generator/fixtureswhich would be covered bymain_test.go
piloulacdog
left a comment
There was a problem hiding this comment.
While we for sure make decision on what we support and what we don't when it comes to enrichment of Cargo.lock with Cargo.toml (ie, not supporting workspaces), we should make sure to clarify it here:
datadog-sbom-generator/README.md
Lines 129 to 133 in d6967fb
| @@ -0,0 +1,23 @@ | |||
| [package] | |||
There was a problem hiding this comment.
we should add to this example:
- same library, multiple version (based on the scope)
- renamed library: example:
serde09 = { package = "serde", version = "0.9" }
There was a problem hiding this comment.
Did the first but I think I will add the second in a separate task (or leave it to whoever picks up said task)
There was a problem hiding this comment.
i think it might still be worth it listing it in the test case, and with a test that clearly state that we don't support it. Hopefully in the future we only have to fix the test, and not have to work on the fixture!
6fdc821 to
7060b6b
Compare
7060b6b to
9279eff
Compare
|
Before updating the README I want to align on what we want to support in this PR - I think for now we can leave off library aliasing and workspaces and create two separate tickets for each to do later |
piloulacdog
left a comment
There was a problem hiding this comment.
approving to not block.
But we should before merging:
- increase complexity of our integration case in rust (having the library name override: we should correctly not match it currently)
- update the readme to make sure that we explain what we do not support (workspace, version alias)
🚀 Motivation
The SBOM generator was not capturing metadata such as file location and transtivity information for Rust dependencies, making it difficult to identify where direct dependencies are declared in Cargo.toml files.
📚 Documentation
📝 Summary
This PR implements a Cargo.toml matcher for the Rust ecosystem to enrich package data with location evidence. At a high level the matcher takes in the dependencies we parse from the
Cargo.lockfile, and matches it against the user definedCargo.tomlfile to try and resolve metadata for these dependencies (see this doc for details on these two files).The new matcher:
Cargo.tomlfile to give us the different dependencies defined (regular, build, dev, workspace)🧪 Testing
🚧 Staging validation
🆘 Recovery
Notes for on-call - select only one: