Skip to content
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

feat(go): parse main module of go binary files #6530

Merged
merged 11 commits into from
Apr 22, 2024

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).

Sorry, something went wrong.

@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
pkg/dependency/parser/golang/binary/parse.go Outdated Show resolved Hide resolved
// 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

DmitriyLewen and others added 3 commits April 22, 2024 13:39

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Teppei Fukuda <knqyf263@gmail.com>
// 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
17 checks passed
fl0pp5 pushed a commit to altlinux/trivy that referenced this pull request May 6, 2024
Co-authored-by: Teppei Fukuda <knqyf263@gmail.com>
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