Skip to content

Conversation

@DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented Apr 22, 2024

Description

We currently skip main module when parsing Go binary files because these versions have always been (devel) and we couldn't detect vulnerabilities for these packages.

But modules installed using go install (see #1837 (comment)) use semver versions.
It will also be correct to add core modules to SBOM formats even with the (devel) version.

Related issues

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 DmitriyLewen self-assigned this Apr 22, 2024
@DmitriyLewen DmitriyLewen marked this pull request as ready for review April 22, 2024 06:57
@DmitriyLewen DmitriyLewen requested a review from knqyf263 as a code owner April 22, 2024 06:57
// Only binaries installed with `go install` contain semver version of the main module.
// Other binaries use the `(devel)` version.
// See https://github.com/aquasecurity/trivy/issues/1837#issuecomment-1832523477.
Version: info.Main.Version,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to use an empty string rather than (devel). What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it.
I am concerned about user questions:
(devel) indicates that we have determined version. It's not correct version, but it's version that the Go binary contains.
If we use empty version - some users may say: this is bug, because some dependencies don't have versions (for example #6456 - the problem here is different, but even in such situations questions arise).

But we can write about this in the documentation.


So I checked a bit:
looks like it work good. e.g. purl simply doesn't contain version.
CycloneDX and SPDX work well:

{
      "bom-ref": "pkg:golang/github.com/aquasecurity/trivy",
      "type": "library",
      "name": "github.com/aquasecurity/trivy",
      "purl": "pkg:golang/github.com/aquasecurity/trivy",
      "properties": [
        {
          "name": "aquasecurity:trivy:PkgType",
          "value": "gobinary"
        }
      ]
    },
"name": "github.com/aquasecurity/trivy",
      "SPDXID": "SPDXRef-Package-5290f6d6347a1886",
      "supplier": "NOASSERTION",
      "downloadLocation": "NONE",
      "filesAnalyzed": false,
      "sourceInfo": "package found in: trivy",
      "licenseConcluded": "NONE",
      "licenseDeclared": "NONE",
      "externalRefs": [
        {
          "referenceCategory": "PACKAGE-MANAGER",
          "referenceType": "purl",
          "referenceLocator": "pkg:golang/github.com/aquasecurity/trivy"
        }
      ],
      "attributionTexts": [
        "PkgType: gobinary"
      ],
      "primaryPackagePurpose": "LIBRARY"

So I like your solution. We will get more correct purl. The (devel) version will not force users to look at the docs to understand what it is.

But then we also need to overwrite (devel) for dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used empty string instead of (devel) - 35f6401

// Only binaries installed with `go install` contain semver version of the main module.
// Other binaries use the `(devel)` version.
// See https://github.com/aquasecurity/trivy/issues/1837#issuecomment-1832523477.
Version: lo.Ternary(info.Main.Version != "(devel)", info.Main.Version, ""), // Use empty string instead of `(devel)`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid user confusion, we might want to add debug log where we hide (devel) version.
@knqyf263 wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in 8edf7c5

@knqyf263 knqyf263 added this pull request to the merge queue Apr 22, 2024
Merged via the queue into aquasecurity:main with commit e32215c Apr 22, 2024
fl0pp5 pushed a commit to altlinux/trivy that referenced this pull request May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Detect the version of the Go binary itself

2 participants