cli icon indicating copy to clipboard operation
cli copied to clipboard

feat: add npm audit licenses

Open bnb opened this issue 3 years ago • 9 comments

Begins to implement npm audit licenses, which is from https://github.com/npm/rfcs/pull/18.

bnb avatar Jun 22 '21 20:06 bnb

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

bnb avatar Sep 09 '21 21:09 bnb

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.

timhaines avatar Sep 09 '21 21:09 timhaines

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

bnb avatar Sep 10 '21 16:09 bnb

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

ljharb avatar Sep 10 '21 17:09 ljharb

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.

bnb avatar Sep 10 '21 17:09 bnb

Free form text, but not non-strings?

ljharb avatar Sep 10 '21 18:09 ljharb

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.

timhaines avatar Sep 10 '21 18:09 timhaines

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 avatar Sep 13 '21 19:09 bnb

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.

bnb avatar Oct 06 '21 16:10 bnb

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.

wraithgar avatar Nov 01 '22 20:11 wraithgar