Support Yarn auditing #26
Conversation
| | -m | --moderate | Prevents integration with moderate or higher vulnerabilities (default `false`) | | ||
| | -h | --high | Prevents integration with high or critical vulnerabilities (default `false`) | | ||
| | -c | --critical | Prevents integration only with critical vulnerabilities (default `false`) | | ||
| | -p | --package-manager | Choose a package manager [_choices_: `auto`, `npm`, `yarn`] (default `auto`) | |
| case 'auto': { | ||
| const packageLockExists = fs.existsSync('package-lock.json'); | ||
| if (packageLockExists) return 'npm'; | ||
| const shrinkwrapExists = fs.existsSync('npm-shrinkwrap.json'); |
There was a problem hiding this comment.
New support for npm-shrinkwrap.json. This is not related to the Yarn feature but is a quick and easy addition for completeness.
| console.error('\x1b[31m%s\x1b[0m', 'Exiting...'); | ||
| process.exitCode = 1; | ||
| } | ||
| const message = err.message || err; |
There was a problem hiding this comment.
I don't love this approach. Since we mostly reject(Error(...)), the default is to use err.message. However, uncaught exceptions are often returned as a string (one example of this currently is not having a package-lock.json or npm-shrinkwrap.json when running with npm.
| */ | ||
|
|
||
| const childProcess = require('child_process'); | ||
| const spawn = require('cross-spawn'); |
There was a problem hiding this comment.
We use cross-spawn instead of purely exec since Yarn uses JSON-lines format instead of a single JSON object. This allows us to process one line at a time using spawn. Since spawn has generally better cross-platform compatibility using the cross-spawn library, I introduced it just in case.
There was a problem hiding this comment.
All in all it works on my windows machine and I couldn't find any errors in the code.
There are 3 things that could be improved (but don't necessarily need to be for this version):
- Have a cleaner cut between the auditer and the package managers. Currently npm-auditer and yarn-auditer also both implement their own reporters instead of using a shared one.
- Separate functionality into separate commits. There's a lot going on in this PR, and not everything is related to the topic.
- Write better commit messages. Ideally each commit message should be a documentation of what happens in the commit so a reviewer would know what to look out for just by reading the commit message
- Modify description to suggest Yarn support - Add homepage and bugs URLs - Add yarn keyword
- Semver: check if yarn audit is supported - cross-spawn: yarn audit parsing is done with spawn
The intention is to add a yarn-auditer, so have to adjust naming. Also changed a yellow console.log to console.warn.
- Checks whether yarn audit is supported - Uses cross-spawn to run yarn audit --json. Since yarn audit using json-lines format, the audit can be processed one line at a time.
- Added supporting documentation for the new flag - audit-ci will figure out the correct auditer by default - Modified error handling to display uncaught exceptions
|
Hi @dbartholomae, I took your suggestions and revised this PR a bit. I redid the commit history to clearly separate the logic for each commit. I removed changes regarding the npm auditer from this PR in favour of another PR. I did not do much for separating the auditors in this PR. The reason is that I decided to stick with |
Supports Yarn auditing: #16
npm-shrinkwrap.json(drop-in replacement forpackage-lock.json)