Skip to content

refactor(purl): guard against empty package name in New - #4

Closed
DmitriyLewen wants to merge 3 commits into
mainfrom
refactor/purl-move-newpurl-helper
Closed

refactor(purl): guard against empty package name in New#4
DmitriyLewen wants to merge 3 commits into
mainfrom
refactor/purl-move-newpurl-helper

Conversation

@DmitriyLewen

Copy link
Copy Markdown
Owner

Description

Follow-up to review feedback in aquasecurity#10298: move the pkg.Name == "" guard from pkg/fanal/applier/docker.go::newPURL into purl.New so the safety check is centralized and applied for all callers. The TODO move to "purl" package in applier/docker.go is removed.

OCI is unaffected — it returns earlier inside the switch (OCI PURLs are derived from metadata.RepoDigests and deliberately ignore pkg.Name).

Latent bug fix in pkg/report/github/github.go::buildPurl

Previously a package with empty Name produced a malformed PURL like pkg:npm/@1.0.0 and slipped past the packageUrl == nil check. Now buildPurl correctly returns "". A regression test in pkg/report/github/github_test.go covers this.

Changes

  • pkg/purl/purl.go: add the if name == "" guard after the type-switch; document the (nil, nil) contract on New; expand the OCI-case comment; drop the redundant if name == "" from the Golang case (now covered by the shared guard).
  • pkg/fanal/applier/docker.go: remove the duplicated guard and stale TODO from newPURL; the function stays as the local error+log wrapper.
  • pkg/purl/purl_test.go: add two cases — pkg.Name == "" and a Maven name that parses to empty.
  • pkg/report/github/github_test.go: regression case for an unnamed npm package.

Related issues

N/A — follow-up to a review comment.

Related PRs

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

Move the `pkg.Name == ""` guard from applier's newPURL into purl.New so
the safety check is applied for all callers and call sites. OCI is
unaffected — it returns earlier inside the switch (OCI PURLs are derived
from metadata, not pkg.Name).

This also fixes a latent bug in pkg/report/github/github.go::buildPurl:
previously, a package with empty Name produced a malformed PURL like
`pkg:<type>/@<version>` and slipped past the `packageUrl == nil` check;
now buildPurl correctly returns "".
Regression test for the fix in purl.New: an npm package without a Name
must now produce an empty PackageUrl instead of a malformed
`pkg:npm/@<version>`.
- spell out concrete (nil, nil) scenarios in New's godoc: empty pkg.Name,
  parseGolang for local paths, parseOCI without RepoDigests
- expand the OCI case comment to explain the metadata.RepoDigests
  derivation and why the empty-name guard does not apply
- add a Maven test case where parsing collapses Name to ""
@DmitriyLewen

Copy link
Copy Markdown
Owner Author

Closing — mislabeled target repo. Recreating against aquasecurity/trivy.

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.

1 participant