Closed skahn007gl closed 1 week ago
Whilst I'm not a maintainer here, I am aware of the scope of this work and did work on the RHEL changes for fix status, so thought I'd add some review comments to try and help to save some time for the reviewers.
@jhebden-gl Thanks or the feedback. I have pushed a new change set with your suggested changes
Hi @knqyf263 👋🏻 - I was wondering if you might be able to assist with a review on this PR? We are hoping to use this data downstream. Thank you!
fix linter error, please https://github.com/aquasecurity/trivy-db/actions/runs/9608945100/job/26502649422?pr=407#step:4:26
Done :)
@skahn007gl Thanks for updating. At last, could you add a test with a non-fixed status? Adding a new advisory or updating the existing one.
@skahn007gl Also, could you open a PR with this change in Trivy? It's better to test this change with Trivy before merging this PR.
Status
here and update unit tests
https://github.com/aquasecurity/trivy/blob/4369a19af771f81df141530bacdc8680e7120ac7/pkg/detector/ospkg/ubuntu/ubuntu.go#L100-L110Tips: Use replace until this PR gets merged.
@skahn007gl Also, could you open a PR with this change in Trivy? It's better to test this change with Trivy before merging this PR.
- Add
Status
here and update unit tests https://github.com/aquasecurity/trivy/blob/4369a19af771f81df141530bacdc8680e7120ac7/pkg/detector/ospkg/ubuntu/ubuntu.go#L100-L110- Update integration tests https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233
- Update documentation in two places https://github.com/aquasecurity/trivy/blob/648ead9553eb2cbeac90e3ef7330a70c352255ac/docs/docs/configuration/filtering.md#by-status https://github.com/aquasecurity/trivy/blob/648ead9553eb2cbeac90e3ef7330a70c352255ac/docs/docs/coverage/os/ubuntu.md#status
Tips: Use replace until this PR gets merged.
Hey @knqyf263 I am unsure what needs to change in the existing integration tests. Is my understanding of the tests correct that it tests 2 scenarios?
This includes the status field in the .golden test data.
I have made the other changes and raised a PR to Trivy :) Trivy PR7020
There is a DB fixture here. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/ubuntu.yaml
We need to add a status here and ensure the status appears in the result.
This bucket must also be updated if you want to add a new vulnerability. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/vulnerability.yaml
When adding a new bucket for Ubuntu, that needs to be added here. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/data-source.yaml#L137-L141
There is a DB fixture here. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/ubuntu.yaml
We need to add a status here and ensure the status appears in the result.
This bucket must also be updated if you want to add a new vulnerability. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/vulnerability.yaml
When adding a new bucket for Ubuntu, that needs to be added here. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/data-source.yaml#L137-L141
@knqyf263 How are the .golden files generated/created?
I have updated the DB fixture with an existing CVE for ubuntu 18.04 that would have a non fixed status, but i'm unable to trigger a failure on the existing ubuntu18.04 integration tests.
- bucket: ubuntu 18.04
pairs:
- bucket: libspring-java
pairs:
- key: CVE-2022-22965
value:
Status: deferred
deferred
is not our status.
Corrected to fix_deferred
, but still no failure in the test run.
I am assuming I need to update ubuntu-1804.json.golden
to update the expected results for the tests linked https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233
If this is the case, How are the .golden files generated/created?
Thanks for the help and guidance on this as I work through it.
I am assuming I need to update ubuntu-1804.json.golden to update the expected results for the tests linked https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233
No, the test should fail due to a mismatch with the golden file. You need to update the golden file once you confirm the test fails. Did you update all the buckets I listed above? It may be easier for us to debug if you push the change to your PR in Trivy.
I am assuming I need to update ubuntu-1804.json.golden to update the expected results for the tests linked https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233
No, the test should fail due to a mismatch with the golden file. You need to update the golden file once you confirm the test fails. Did you update all the buckets I listed above? It may be easier for us to debug if you push the change to your PR in Trivy.
Thanks for confirming, I agree it will be easier to debug if i share my changes. I will update the PR shortly.
@knqyf263 I have pushed the updated DB fixture and OS bucket. I used an existing vulnerability.
Note the changes do not include the use of replace
in go.mod
, but i do use it locally.
e.g: replace github.com/aquasecurity/trivy-db => /xxx/yyy/zzz/trivy-db
You added Ubuntu 21.10, but the scanned image is not Ubuntu 21.10. The data is not used.
You added Ubuntu 21.10, but the scanned image is not Ubuntu 21.10. The data is not used.
Adjusted to ubuntu 16.04 as there is an image under integration/testdata/fixtures/images
Added test for ubuntu 16.04
into integration/standalone_tar_test.go
note: Test does not use the correct json.golden
as there is no existing one for ubuntu 16.04
Successfully generated a failed test result.
I am running the tests via mage test:integration
(mage is new to me) or go test -timeout 15m -v -tags=integration ./integration/...
But i do not see any test artefacts generated, only pass/fail stat's.
Is there a way to generate the output from the comparison?.
I have pushed my updates.
Hey @knqyf263 Sorry for the delays on my side, I've had some personal life stuff crop up.
I have simplified the integration test to use as much as existing as possible to understand how these tests hang together, I believe I am still missing something here.
When i run the integration test's, there is no test failure for ubuntu-1804
I added some dummy data to to ubuntu-1804.json.golden
This induces an error, but the error indicates it did not expect the new vulnerability.
- Vulnerabilities: ([]types.DetectedVulnerability) (len=6) {
+ Vulnerabilities: ([]types.DetectedVulnerability) (len=5) {
Summary
resolves #408
Trivy Ubuntu advisories provide a
FixedVersion
when there is a released fix for a package,Affected
can be inferred when the advisory is present without aFixedVersion
, it does not expose any other status that Canonical use to indicate the status of a fix. This is insufficient to infer a status ofignored
,pending
orneeded
as these status show the package is affected and in the process of getting to a fixed version or not if the status isignored
.This change exposes the Status provided in launchpad advisories without changed existing behaviour to populate the notes field in an advisory with
Status:$status
Changes
gitignore:21
:: name change ofcache
,assets
to_cache
,_assets
to reflect directory name change to support gopls lint ignore declaration of_
prefix on directories.MakeFile
:: Adjusting naming convention ofcache
&_assets
to avoid gopls linter causing max open file errors on MacOS.pkg/types/status.go
:: New status added to support all potential status on launchpad advisories.pkg/vulnsrc/ubuntu/ubuntu.go
:: Expanded target status to capture all provided status's. Adjusted logic to use these status. Added ubuntu status to trivy status normalisation function.pkg/vulnsrc/ubuntu/ubuntu_test.go
:: Added test for status normalisation.