-
Notifications
You must be signed in to change notification settings - Fork 142
Description
Recently I was trying to debug a problem I was having with the Dependency Review Action, specifically with the allow-dependencies-licenses
option to provide a list of specific packages excluded from license checks (provided in the deny-licenses
option), where it would not work sometimes.
In my testing I noticed that the DRA gave me an Incompatible License
issue, even though it also recognised them as packages that are meant to be excluded from the check:
I tested it for several different packages and noticed that the common denominator was that this only happened for scoped packages that belong to a specific namespace/organisation e.g. @netdata/netdata-ui
.
To dig a little deeper, I looked at the response from the dependency graph API endpoint. In the response for the packages that didn't get handled by the DRA as we'd expect we see:
{
"change_type": "added",
"manifest": "package.json",
"ecosystem": "npm",
"name": "@netdata/charts",
"version": "^5.0.0",
"package_url": "pkg:npm/%40netdata/charts",
"license": null,
"source_repository_url": "https://github.com/netdata/charts",
"scope": "runtime",
"vulnerabilities": []
},
And another example:
{
"change_type": "added",
"manifest": "package.json",
"ecosystem": "npm",
"name": "@fontsource/noto-sans-phags-pa",
"version": "^5.0.0",
"package_url": "pkg:npm/%40fontsource/noto-sans-phags-pa",
"license": null,
"source_repository_url": "https://github.com/fontsource/font-files",
"scope": "runtime",
"vulnerabilities": []
},
So it appears that the %40
in the URL encoding was messing things up. (i.e it's comparing pkg:npm/%40netdata/charts
against pkg:npm/@netdata/charts
. So in other words the namespace will never match because it is a string literal comparison and @netdata
is not the same string as %40netdata
.
Looking at the DRA codebase, it looks like this comes up due to the URI encoding on line 177 in the src/licenses.ts.
And line 186 of licenses.ts#L186 then includes a namespace comparison that always returns false
as URL encoding compares the string @
with %40
, so package ends up never being excluded from license check:
exclusion.namespace === changeAsPackageURL.namespace &&
To Reproduce
Steps to reproduce the behaviour:
- Set up the DRA action for a repo and include both a list of
deny-licenses
andallow-dependencies-licenses
to provide a list of packages that are to be excluded from license checks. Ensure that a couple of scoped packages are added that have licenses that fall under thedeny-licenses
list too. - Open a PR that includes a modification to the
package.json
file to add one of the same scoped packages that are listed underallow-dependencies-licenses
- Wait for the DRA workflow to run on the PR...
- See error - should be able to see it under the PR checks and from the workflow run summary.
Expected behavior
A clear and concise description of what you expected to happen.
Screenshots
If applicable, add screenshots to help explain your problem.
Additional context
For comparison, non-scoped packages work just fine, with the API responses showing:
{
"change_type": "added",
"manifest": "package.json",
"ecosystem": "npm",
"name": "comfy",
"version": "^0.1.0",
"package_url": "pkg:npm/comfy",
"license": null,
"source_repository_url": "https://github.com/filp/comfy",
"scope": "runtime",
"vulnerabilities": []
}
And here are some of my PRs where I managed to replicate the problem:
And associated workflow runs:
- https://github.com/danielhardej/DRA-test-sandbox/actions/runs/18394639353?pr=8
- https://github.com/danielhardej/DRA-test-sandbox/actions/runs/18398006504?pr=10
cc:
@dangoor