feat(image): updated json report added package locations fields

[parvez0]

Description

This pull request introduces a new feature to include package location indexes in the JSON report.

Changes

1. Modified pkg/scanner/local/server.go to use the location data from the detected vulnerability.
2. Updated pkg/report/sarif.go to reuse the calculated location information if available.
3. Added a new key Location under DetectedVulnerability in pkg/types/vulnerability.go to update the extracted location data.
4. Updated integration/testdata/*.json.golden to verify Location field in json reports

Usage

$ git clone https://github.com/knqyf263/trivy-ci-test.git
$ cd trivy-ci-test
$ trivy fs --format template --format json -o trivy_report.json --scanners vuln .

Command output before this feature change

{
  "SchemaVersion": 2,
  "CreatedAt": "2024-03-16T21:54:44.700427+05:30",
  "ArtifactName": ".",
  "ArtifactType": "filesystem",
  "Metadata": {
    ...
  },
  "Results": [
    {
      "Target": "Cargo.lock",
      "Class": "lang-pkgs",
      "Type": "cargo",
      "Packages": [...],
      "Vulnerabilities": [
        {
          "VulnerabilityID": "CVE-2019-15542",
          "PkgID": "[email protected]",
          "PkgName": "ammonia",
          "PkgIdentifier": {
            "PURL": "pkg:cargo/[email protected]"
          },
          "InstalledVersion": "1.9.0",
          "FixedVersion": "2.1.0",
          "Status": "fixed",
          "Layer": {},
          "SeveritySource": "ghsa",
          "PrimaryURL": "https://avd.aquasec.com/nvd/cve-2019-15542",
          "DataSource": {...},
          "Title": "Uncontrolled recursion in ammonia",
          "Description": "An issue was discovered in the ammonia crate before 2.1.0 for Rust. There is uncontrolled recursion during HTML DOM tree serialization.",
          "Severity": "HIGH",
          "CweIDs": ["CWE-674],
          "VendorSeverity": {...},
          "CVSS": {...},
          "References": [...],
          "PublishedDate": "2019-08-26T18:15:12.467Z",
          "LastModifiedDate": "2020-08-24T17:37:01.14Z"
        }
      ]
    }
  ]
}

Command output after introducing this feature

{
  "SchemaVersion": 2,
  "CreatedAt": "2024-03-16T21:54:44.700427+05:30",
  "ArtifactName": ".",
  "ArtifactType": "filesystem",
  "Metadata": {
    ...
  },
  "Results": [
    {
      "Target": "Cargo.lock",
      "Class": "lang-pkgs",
      "Type": "cargo",
      "Packages": [...],
      "Vulnerabilities": [
        {
          "VulnerabilityID": "CVE-2019-15542",
          "PkgID": "[email protected]",
          "PkgName": "ammonia",
          "PkgIdentifier": {
            "PURL": "pkg:cargo/[email protected]"
          },
          "InstalledVersion": "1.9.0",
          "FixedVersion": "2.1.0",
          "Status": "fixed",
          "Layer": {},
          "SeveritySource": "ghsa",
          "PrimaryURL": "https://avd.aquasec.com/nvd/cve-2019-15542",
          "Locations": [
            {
              "StartLine": 2,
              "EndLine": 13
            }
          ],
          "DataSource": {...},
          "Title": "Uncontrolled recursion in ammonia",
          "Description": "An issue was discovered in the ammonia crate before 2.1.0 for Rust. There is uncontrolled recursion during HTML DOM tree serialization.",
          "Severity": "HIGH",
          "CweIDs": [ "CWE-674"],
          "VendorSeverity": {...},
          "CVSS": {...},
          "References": [...],
          "PublishedDate": "2019-08-26T18:15:12.467Z",
          "LastModifiedDate": "2020-08-24T17:37:01.14Z"
        }
      ]
    }
  ]
}

Related issues

• Close Include line numbers in json output #4418

Checklist

• I've read the guidelines for contributing to this repository.
• I've followed the conventions in the PR title.
• I've added tests that prove my fix is effective or that my feature works.
• I've updated the documentation with the relevant information (if needed).
• I've added usage information (if the PR introduces new options)
• I've included a "before" and "after" example to the description (if the PR is a user interface change).

[DmitriyLewen]

Hello @parvez0
Thanks for your work!

Left comments.

Regards, Dmitriy

[parvez0]

Hey @DmitriyLewen, needed your opinion on this. After integrating the Location Changes, I noticed that integration_test/client_server.go started failing for the busybox_with_cargo.lock and scan pox.xml test cases. The issue from these test cases is expecting DetectedVulnerability.Location to be nil. However, the output golden files busybox-with-lockfile.json.golden and pom.json.golden are also used in other integration tests where non-nil Location data is expected. To address this, I created two duplicate files that includes Location data. Please let me know if you have a better solution for this.

[DmitriyLewen]

@parvez0 You need to update rpc package.
Let me know when you're done making changes (to avoid conflicts) and i will update client/server part.

[parvez0]

@parvez0 You need to update rpc package. Let me know when you're done making changes (to avoid conflicts) and i will update client/server part.

Hey @DmitriyLewen what do I have to do in rpc package I haven't seen any changes that need to be made there, can you please let me know I'll try to do it as soon as possible

[DmitriyLewen]

Hm... we don't insert Locations in client/server mode...
I created #6366 for this bug.
After merge #6366 you need to merge main branch into this PR and follow these steps:

• add locations to Vulnerabilities (use 26 number). Use Location struct.
• run mage fmt.
• run mage:protoc.
• add Locations to Vulnerabilties as for Packages ( use ConvertToRPCLocations and ConvertFromRPCLocation functions)
• add Locations in testcase in convert_test.go.
• recheck that you don't need new test golden files in integration tests.

[DmitriyLewen]

@parvez0 thanks for your work!

@knqyf263 I approved PR. Take a looks, when you have time, please.

[knqyf263]

Originally, our policy was to include only minimal data in reports. It may be better to embed Package in DetectedVulnerability than to gradually add more and more attributes.

[DmitriyLewen]

sounds logical.
@parvez0 Do you have the time and opportunity to do this?

[parvez0]

Hey @DmitriyLewen, I'll take a look at this and update

[knqyf263]

We're so sorry. We discussed this offline and found out that embedding packages will get lots of duplicate data if multiple vulnerabilities are found in one package. We need to carefully think about it.

[parvez0]

Hey @knqyf263, I had the same question and was just about to reach out to you. Please let me know if you've settled on an approach for this and need my assistance to finalize it. In the meantime, I'll start looking into other issues that I can address.

[knqyf263]

@parvez0 Thanks for your understanding. I implemented my idea and would collect feedback from other maintainers.

[github-actions]

This PR is stale because it has been labeled with inactivity.