feat: add npm audit licenses #3452

bnb · 2021-06-22T20:10:43Z

Begins to implement npm audit licenses, which is from npm/rfcs#18.

lib/audit.js

ljharb · 2021-06-22T20:58:31Z

lib/audit.js

+    const auditType = this.npm.config.get('audit-type')
+    if (auditType === 'license') {
+      this.auditLicenses(args).then(() => cb()).catch(cb)
+    } else { 
+      this.auditAdvisories(args).then(() => cb()).catch(cb)
+    }


does this mean that npm audit can only do either licenses or advisories, but not both?

Should audit-type be validated somehow so it's only "license" or "advisory"?

@ljharb in pairing with @ruyadorno today, we intentionally kept this limited to "getting it working" and not fully implementing. This was the minimum way to get it working. There will be individual ways to do advisories, licenses, and both (presuming both will be the default, since license is going to be opt-in rather than opt-out).

Ruy's recommendation was npm audit --audit-type=license as a more npm-y API, so we went with that. I'm not opposed to other APIs though and presume we'll end up bikeshedding a bit.

When I just want one type, that totally makes sense to me - but when I want both, I'd want some config way to per-command override the global audit-type=license i plan to configure.

As for the default, I'd assume licenses aren't validated unless there's a config for it, which means it'd be off initially?

oop, sorry thought I replied to this last night:

the intent is to have npm audit do both licenses and advisories by default. It can optionally do just licenses or advisories.

the implementation here is an intentionally basic one that's the result of @ruyadorno and myself pairing trying to get a very basic implementation working that can be expanded on to include everything that's been scoped out, hence WIP status of the PR.

OK - so what audit-type value would I provide to override (a global config setting that restricts it to "just licenses")?

advisories: https://github.com/npm/cli/pull/3452/files#diff-2a8ed1f0d31e4c759861aa941a2dc7e4965c10d96a79bad6499301a9a0dadbc2R56

bnb · 2021-09-09T21:02:14Z

Updated this a bit. You can run it with `node . audit --audit-type=license --json" to get JSON output.

I've got user configuration working (based on audit in package.json - I'm not attached to this, it's just the path I took and I'm happy to revisit later). The API is slightly different than licensee:

It does not take BlueOak categorization. If we want that, I think it should come in a separate RFC. It would be trivial to add.
Defaults to correcting assertions (licensee does not), allowing opt-out if end-users want that.
All properties are on audit > licenses in package.json. This is slightly different than the config of licensee. All the available properties (outside of the aforementioned blueoak property) are available, just under the license property.
What licensee calls spdx I have called identifiers since that's a more... clear and concise name to those who don't know what SPDX License IDs are. The code in the PR maps identifiers to spdx when we run licensee.

Some additional notes:

Added --audit-type=advisories that simply duplicates how audit is presently run, with the intent of eventually adding the license checking to the default case. Ideally if the output is JSON, a single object should be returned rather than two different objects. I think this needs to be done outside of the npm/cli code, though... this specific concept leads me to believe there's a larger refactoring that needs to happen here, since it very quickly leads to other questions like "how is this data integrated naturally into non-JSON audit UI" 🙃
I've generally done my best to leave comments where I realized additional work was needed. I hope none of them are half-thought out but some might be :P

timhaines · 2021-09-09T21:12:40Z

FWIW, the npm docs refer to SPDX - it's something they seek to educate on. https://docs.npmjs.com/cli/v7/configuring-npm/package-json#license

Perhaps it's worth considering spdx_identifers or similar, to indicate it shouldn't be free-form.

bnb · 2021-09-10T16:54:29Z

Perhaps it's worth considering spdx_identifers or similar, to indicate it shouldn't be free-form.

no other property used by npm uses an underscore - I don't think this is a particularly npm-y approach.

IMO that exact doc you linked is the justification for using identifiers. The license property isn't spdx_license_id or some variant - it's just license, despite the fact that it should be an SPDX license ID (it realistically doesn't have to be).

ljharb · 2021-09-10T17:21:45Z

Does npm even allow publishing anymore with a non-spdx license field?

bnb · 2021-09-10T17:23:32Z

You can publish freeform text in that field, yes. If not, it'd prevent things like BlueOak's license from being used which is IMO a very bad thing for open source and progress.

ljharb · 2021-09-10T18:28:00Z

Free form text, but not non-strings?

timhaines · 2021-09-10T18:39:22Z

FWIW, Blue Oak's license has an SPDX code; BlueOak-1.0.0. But yeah, you do see all kinds of content in that field. "See license in license" is somewhat common.

bnb · 2021-09-13T19:47:31Z

Free form text, but not non-strings?

AFAIK you can still do an object (the npmjs.com site renders it correctly despite it being deprecated) but you really shouldn't.

bnb · 2021-10-06T16:58:58Z

Small update on this: After pairing with @izs it seems like the best approach is to update Arborist (specifically, updating AuditResolver to support licenses in addition to vulnerabilities), since without doing so it's relatively challenging to get nicer bits of npm audit that the ecosystem would expect.

I'll probably be a bit slow getting around to that. Happy to pair on it if anyone's interested.

Starting to implemenent [RFC: Improve signature verification](npm/rfcs#550) Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452)) This command will verify registry signatures stored in the packument against a public key on the registry. It currently supports: - Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object - Validates public keys are not expired, compared to the version created date - Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys` - Errors when encountering invalid signatures - json/human format output TODO - [ ] Fix tests and implement test cases - [ ] Expired public key - [ ] No public keys - [ ] Missing signatures with a public key on the registry - [ ] Missing signatures without a public key on the registry - [ ] Install with valid signatures - [ ] Install with invalid signatures - [ ] Third party registry with signatures and keys - [ ] Tests for the different formats (json, human) - [ ] Tests to omit type of dependency (e.g dev deps) - [ ] Fetch signatures and integrity from `pacote.manifest` - [ ] Caching story for public keys? Currently cached for one week, assumes we'll double sign for longer when rotating keys - [ ] Validate early return conditionals for arb nodes, a lot of cases silently return, e.g. no version, are these correct? - [ ] What other checks do we want? - [ ] Strict mode to error if any signatures are missing when a registry does not return public keys? - [ ] Do we want to explitly trust keys from third party registries and store in .npmrc?

Implemenents [RFC: Improve signature verification](npm/rfcs#550) Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452)) This command will verify registry signatures stored in the packument against a public key on the registry. Supporting: - Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object - Validates public keys are not expired - Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys` - Errors when encountering invalid signatures - Output: json/human formats

Implemenents [RFC: Improve signature verification](npm/rfcs#550) Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452)) This command will verify registry signatures stored in the packument against a public key on the registry. Supporting: - Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object - Validates public keys are not expired - Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys` - Errors when encountering invalid signatures - Output: json/human formats Co-authored-by: Michael Garvin <wraithgar@github.com>

* feat: add npm audit signatures Implements [RFC: Improve signature verification](npm/rfcs#550) Adds a new sub-command to `audit`: `npm audit signatures` (following [`npm audit licenses`](#3452)) This command will verify registry signatures stored in the packument against a public key on the registry. Supporting: - Any registry that implements `host/-/npm/v1/keys` endpoint and provides `signatures` in the packument `dist` object - Validates public keys are not expired - Errors when encountering packages with missing signatures when the registry returns keys at `host/-/npm/v1/keys` - Errors when encountering invalid signatures - Output: json/human formats

wraithgar · 2022-11-01T20:56:13Z

Closing this while folks work on it. Please do keep your branch in the forked repo. Trying to clear out our pull requests so that they are more manageable.

bnb added 2 commits June 22, 2021 20:12

feat: add license checking with json output

1432cca

test: test for license checking with json output

3075d72

bnb force-pushed the npm-audit-licenses branch from 9bdfbc7 to 3075d72 Compare June 22, 2021 20:18

deps: move licensee to dep rather than devDep

46a6471

ljharb reviewed Jun 22, 2021

View reviewed changes

darcyclarke added Release 7.x work is associated with a specific npm 7 release semver:minor new backwards-compatible feature labels Jun 23, 2021

bnb added 3 commits September 7, 2021 14:08

chore: minor style changes

d88b262

feat: some more work on Audit

f3e183d

chore: add licenses to allow in license check

73aed2b

Merge branch 'latest' into npm-audit-licenses

9b2cd1e

wraithgar added Release 8.x work is associated with a specific npm 8 release and removed Release 7.x work is associated with a specific npm 7 release labels Oct 7, 2021

wraithgar changed the base branch from latest to release-next January 26, 2022 20:45

wraithgar changed the base branch from release-next to latest March 9, 2022 18:25

feelepxyz mentioned this pull request Apr 29, 2022

feat: add npm audit signatures #4827

Merged

fritzy force-pushed the latest branch 2 times, most recently from 3037d35 to f3b0c43 Compare September 14, 2022 23:09

lukekarrys force-pushed the latest branch from 591d1d1 to 9e74d3e Compare October 19, 2022 19:50

wraithgar closed this Nov 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add npm audit licenses #3452

feat: add npm audit licenses #3452

bnb commented Jun 22, 2021 •

edited

Loading

ljharb Jun 22, 2021

bnb Jun 22, 2021

ljharb Jun 22, 2021

bnb Jun 23, 2021

ljharb Jun 23, 2021 •

edited

Loading

bnb Sep 15, 2021

bnb commented Sep 9, 2021 •

edited

Loading

timhaines commented Sep 9, 2021

bnb commented Sep 10, 2021

ljharb commented Sep 10, 2021

bnb commented Sep 10, 2021

ljharb commented Sep 10, 2021

timhaines commented Sep 10, 2021

bnb commented Sep 13, 2021

bnb commented Oct 6, 2021

wraithgar commented Nov 1, 2022

feat: add npm audit licenses #3452

feat: add npm audit licenses #3452

Conversation

bnb commented Jun 22, 2021 • edited Loading

ljharb Jun 22, 2021

Choose a reason for hiding this comment

bnb Jun 22, 2021

Choose a reason for hiding this comment

ljharb Jun 22, 2021

Choose a reason for hiding this comment

bnb Jun 23, 2021

Choose a reason for hiding this comment

ljharb Jun 23, 2021 • edited Loading

Choose a reason for hiding this comment

bnb Sep 15, 2021

Choose a reason for hiding this comment

bnb commented Sep 9, 2021 • edited Loading

timhaines commented Sep 9, 2021

bnb commented Sep 10, 2021

ljharb commented Sep 10, 2021

bnb commented Sep 10, 2021

ljharb commented Sep 10, 2021

timhaines commented Sep 10, 2021

bnb commented Sep 13, 2021

bnb commented Oct 6, 2021

wraithgar commented Nov 1, 2022

bnb commented Jun 22, 2021 •

edited

Loading

ljharb Jun 23, 2021 •

edited

Loading

bnb commented Sep 9, 2021 •

edited

Loading