feat(audit) Initial addition of yarn audit command.

[rally25rs]

Summary

Adds the yarn audit command.

Adds the --audit flag to yarn add / install / upgrade

Currently the functionality is fairly basic. It roughly copies the initial audit release of npm including the style of reporting.

The audit command will return 0 if no issues are found, otherwise returns non-0 exit code.

Unlike npm, yarn doesn't currently auto-run audit on add/install/upgrade. Partly this is due to the additional network call and a lack of an easy way to mock it during unit tests, so every unit test tries to hit npm's API to audit the packages being "installed" during unit tests.
To run an audit during add/install/upgrade, please specify the --audit flag.

If the --offline flag is specified along with --audit then audit will be skipped (and a message noting this will be printed)

If you would like audit to always be run, you can add to the .yarnrc file:

--add.audit true
--install.audit true
--upgrade.audit true

implements #5808

Test plan

Added some basic unit tests.

When reporting audit issues, please run with --verbose and include the "Audit API Request" and "Audit API Response" JSON.

[buildsize]

File name	Previous Size	New Size	Change
yarn-[version].noarch.rpm	1.07 MB	1.09 MB	17.64 KB (2%)
yarn-[version].js	4.2 MB	4.26 MB	65.11 KB (2%)
yarn-legacy-[version].js	4.37 MB	4.44 MB	68.66 KB (2%)
yarn-v[version].tar.gz	1.08 MB	1.1 MB	19.54 KB (2%)
yarn_[version]all.deb	791.86 KB	803.57 KB	11.72 KB (1%)

[arcanis]

Hey Jeff! Thanks for your work on this! A few comments:

• The --audit flag isn't working, it should be listed in src/cli/index.js
• Running an install with --audit is subject to the integrity check; we shouldn't bailout in this case
• The vulnerable package names are not shown, making it hard to know what should be done

I'm also not convinced by the workflow - right now you must add --audit when doing an install, meaning that if something is vulnerable you'll only know it after it's been installed (and its postinstall scripts have run). That doesn't look great from a security perspective. What would you think of this?:

• run --audit by default on yarn add and yarn upgrade, possibly prompting the user to confirm when they inadvertently add a vulnerable package (either explicitly or through transitive dependencies)
  • A problem in this case is that without specific treatment it would also report the errors caused by the packages that already are in the dependency tree - maybe we should filter them out in this case?
• don't run --audit by default on yarn install (like it currently is, no change there)
• add a yarn audit command that simply validate the current state (exit code 1 if vulnerable)
  • yarn audit wouldn't do a full install, and would use the versions currently in the lockfile
  • I guess doing the resolution if missing would also be ok; basically, skip fetch/link/build

What do you think?

[arcanis]

Wait, sorry, missed that the yarn audit command already exists with your PR; so basically the only change I'd suggest would be to run it by default on yarn add (and filter out the existing errors), but that's your call 😃

I'd also maybe suggest to make --audit on install prompt the user at install-time when they would install a vulnerable packages (before they're actually installed).

[arcanis]

Not sure this works - when running install --audit twice, the second time I get no reports

[arcanis]

I think it's because the bailout is done at the end of the resolution step, before the audit gets initialized

[rally25rs]

Yeah I'm pretty sure yarn just bails before it gets that far, like it would have during a normal install.

[arcanis]

I think it should be safe to disable the integrity check altogether when --audit is set.

[rally25rs]

@arcanis

I'd suggest would be to run it by default on yarn add

that was my original intent, like npm does, but it breaks all the existing unit tests because it has to make a network call. if you can think of a way to mock out that request by default for all tests, that would be great. Also I think in Discord @BYK said he was a bit concerned about adding another network call.

I'd also maybe suggest to make --audit on install prompt the user at install-time when they would install a vulnerable packages (before they're actually installed).

I think I left a comment in the code where I could bailout if there are errors, but npm doesn't by default. Prompting the user gets tricky because what if this is in a CI environment or --non-interactive is passed.

The vulnerable package names are not shown, making it hard to know what should be done

Is this just on a yarn install --audit? I believe npm does the same; just prints the summary and tells the user to "run yarn audit for more info".

---

In addition I noticed a couple other ... oddities.

This command uses npm's api and half the code is a copy/paste from them, but yarn audit reports more errors than npm audit does. In the case I saw, npm wasn't reporting errors that were from a transitive dep of a github repo dependency. When the npm audit API sends back a "resolution recommendation" it wants tot ell the user to run yarn upgrade foo@123 to resolve the vulnerability, but foo is a transitive dep, so that won't fix it. 😕

example:

yarn audit v1.11.0-0
# Run yarn upgrade url-parse@1.4.3 to resolve 2 vulnerabilities
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Open Redirect                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ url-parse                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=1.4.3                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ cxcore-ui-build-tasks                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ cxcore-ui-build-tasks > webpack-dev-server > sockjs-client > │
│               │ eventsource > original > url-parse                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/678                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

where cxcore-ui-build-tasks is a github url dependency. npm audit doesn't report that vulnerability at all, but the api returns the recommendation:

{
  "actions": [
    {
      "action": "update",
      "module": "url-parse",
      "depth": 6,
      "target": "1.4.3",
      "resolves": [
        {
          "id": 678,
          "path": "cxcore-ui-build-tasks>webpack-dev-server>sockjs-client>eventsource>original>url-parse",
          "dev": false,
          "optional": false,
          "bundled": false
        },
        {
          "id": 678,
          "path": "cxcore-ui-build-tasks>webpack-dev-server>sockjs-client>url-parse",
          "dev": false,
          "optional": false,
          "bundled": false
        }
      ]
    },

but update url-parse@1.4.3 isn't going to do anything.
So I think I might be constructing the JSON request / dep tree incorrectly. I might be reporting these packages by their hoisted location so npm thinks url-parse is at the top of the hierarchy.
Not sure yet...

[rally25rs]

OK I think this should be in a good place for an initial release. It's not feature-rich, but it does report audit problems correctly at this point.

[arcanis]

Looks good to me! I'll merge the changelog, update this PR, and merge it 👍

[Gudahtt]

I believe this PR is the reason the nightly tests on Ubuntu 16.0.4 have been failing.

The dependency cli-table3 doesn't support Node v4. The line the test is failing on is this one, due to the use of let.

The reason the other end-to-end tests aren't affected is that they use Node v6.

I don't yet know why the other tests aren't failing on Node v4. Perhaps this library is never invoked outside of the end-to-end tests.