Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(nodejs): add v9 pnpm lock file support #6617

Merged
merged 15 commits into from
May 21, 2024

Conversation

DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented May 3, 2024

Description

add v9 pnpm lock file support.

  • Dev field used for v9

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 May 3, 2024
!!! note
Trivy currently only supports Lockfile [v6][pnpm-lockfile-v6] or earlier.
#### lock file v9 version
Trivy supports `Dev` field for `pnpm-lock.yaml` v9 or later. Use the `--include-dev-deps` flag to include the developer's dependencies in the result.
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 have not seen any requests to use Dev field for version 6.0 or earlier lock files.
But if users want, we can add this in another PR.

@DmitriyLewen DmitriyLewen marked this pull request as ready for review May 8, 2024 09:07
@DmitriyLewen DmitriyLewen requested a review from knqyf263 as a code owner May 8, 2024 09:07
Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

Looks almost good

"strconv"
"strings"

"github.com/samber/lo"
"golang.org/x/exp/maps"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"golang.org/x/exp/maps"
"maps"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is weird, but maps package doesn't have Values and Keys functions:
https://pkg.go.dev/maps

// - "registry.npmjs.org/@babel/generator/7.21.9" => "@babel/generator/7.21.9"
// - "/lodash/4.17.10" => "lodash/4.17.10"
_, depPath, _ = strings.Cut(depPath, "/")
// trimRegistry trims registry (or `/` prefix) for depPath.
Copy link
Collaborator

Choose a reason for hiding this comment

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

PURL has vcs_url. I think we should keep this information if it is not registry.npmjs.org.
https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#npm

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 am not sure if using registry URL in package name is correct.
I suggest keeping depPath in ExternalReferences
and check ExternalReferences in purl for npm:

trivy/pkg/purl/purl.go

Lines 425 to 429 in bbaf595

func parseNpm(pkgName string) (string, string) {
// the name must be lowercased
name := strings.ToLower(pkgName)
return parsePkgName(name)
}

e.g.
private.npmjs.org/@babel/runtime@7.18.3 =>

ID:           "@babel/runtime@7.18.3",
Name:         "@babel/runtime",
Version:      "7.18.3",
Relationship: ftypes.RelationshipIndirect,
ExternalReferences: []ftypes.ExternalRef{
	{
		Type: ftypes.RefVCS,
		URL:  "private.npmjs.org/@babel/runtime@7.18.3",
	},
},

@knqyf263 wdyt?

Copy link
Collaborator

@knqyf263 knqyf263 May 21, 2024

Choose a reason for hiding this comment

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

Yes, I meant to suggest using ExternalReferences. I didn't explain clearly.

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 update this logic - 2d50522
take a look, when you have time, please

@knqyf263 knqyf263 added this pull request to the merge queue May 21, 2024
Merged via the queue into aquasecurity:main with commit 1e08648 May 21, 2024
17 checks passed
@DmitriyLewen DmitriyLewen deleted the fix-pnpm/add-v9-support branch May 30, 2024 05:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(pnpm): add support of Lockfile v9
2 participants