From ce39c338de7cff4abb79069d9818ddb848c67208 Mon Sep 17 00:00:00 2001 From: Brian Cristante <33549821+brcrista@users.noreply.github.com> Date: Mon, 14 Jun 2021 15:13:54 -0400 Subject: [PATCH 1/7] npm audit fix --- package-lock.json | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 526e751c82..078409f7da 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13828,9 +13828,9 @@ } }, "normalize-url": { - "version": "3.3.0", - "resolved": "https://registry.npmjs.org/normalize-url/-/normalize-url-3.3.0.tgz", - "integrity": "sha512-U+JJi7duF1o+u2pynbp2zXDW2/PADgC30f0GsHZtRh+HOcXHnw137TrNlyxxRvWW5fjKd3bcLHPxofWuCjaeZg==", + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/normalize-url/-/normalize-url-6.0.1.tgz", + "integrity": "sha512-VU4pzAuh7Kip71XEmO9aNREYAdMHFGTVj/i+CaTImS8x0i1d3jUZkXhqluy/PRgjPLMgsLQulYY3PJ/aSbSjpQ==", "dev": true }, "npm-bundled": { @@ -14487,13 +14487,13 @@ } }, "parse-url": { - "version": "5.0.2", - "resolved": "https://registry.npmjs.org/parse-url/-/parse-url-5.0.2.tgz", - "integrity": "sha512-Czj+GIit4cdWtxo3ISZCvLiUjErSo0iI3wJ+q9Oi3QuMYTI6OZu+7cewMWZ+C1YAnKhYTk6/TLuhIgCypLthPA==", + "version": "5.0.3", + "resolved": "https://registry.npmjs.org/parse-url/-/parse-url-5.0.3.tgz", + "integrity": "sha512-nrLCVMJpqo12X8uUJT4GJPd5AFaTOrGx/QpJy3HNcVtq0AZSstVIsnxS5fqNPuoqMUs3MyfBoOP6Zvu2Arok5A==", "dev": true, "requires": { "is-ssh": "^1.3.0", - "normalize-url": "^3.3.0", + "normalize-url": "^6.0.1", "parse-path": "^4.0.0", "protocols": "^1.4.0" } @@ -16416,9 +16416,9 @@ "dev": true }, "trim-newlines": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/trim-newlines/-/trim-newlines-3.0.0.tgz", - "integrity": "sha512-C4+gOpvmxaSMKuEf9Qc134F1ZuOHVXKRbtEflf4NTtuuJDEIJ9p5PXsalL8SkeRw+qit1Mo+yuvMPAKwWg/1hA==", + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/trim-newlines/-/trim-newlines-3.0.1.tgz", + "integrity": "sha512-c1PTsA3tYrIsLGkJkzHF+w9F2EyxfXGo4UyJc4pFL++FMjnq0HJS69T3M7d//gKrFKwy429bouPescbjecU+Zw==", "dev": true }, "trim-off-newlines": { From f1ba29d101a22f4bcc19be1f4496f6fdff1d1fcc Mon Sep 17 00:00:00 2001 From: Brian Cristante <33549821+brcrista@users.noreply.github.com> Date: Mon, 14 Jun 2021 15:14:28 -0400 Subject: [PATCH 2/7] Just pass --production instead of using a custom allow list --- .github/workflows/audit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 295488e2ce..474194ac10 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -32,7 +32,7 @@ jobs: run: npm run bootstrap - name: audit tools #disabled while we wait for https://github.com/actions/toolkit/issues/539 - run: npm audit --audit-level=moderate + run: npm audit --production --audit-level=moderate - name: audit packages run: npm run audit-all From a0fac74aabe75240fdae786cb77adc47f1e63cb8 Mon Sep 17 00:00:00 2001 From: Brian Cristante <33549821+brcrista@users.noreply.github.com> Date: Mon, 14 Jun 2021 15:34:56 -0400 Subject: [PATCH 3/7] remove outdated comment --- .github/workflows/audit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 474194ac10..bf554eac1f 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -31,7 +31,7 @@ jobs: - name: Bootstrap run: npm run bootstrap - - name: audit tools #disabled while we wait for https://github.com/actions/toolkit/issues/539 + - name: audit tools run: npm audit --production --audit-level=moderate - name: audit packages From 935647112d96fa5cf82e61314f7135376d24f291 Mon Sep 17 00:00:00 2001 From: Brian Cristante <33549821+brcrista@users.noreply.github.com> Date: Mon, 14 Jun 2021 16:22:44 -0400 Subject: [PATCH 4/7] scripts/audit-allow-list for NPM v7 --- .github/workflows/audit.yml | 2 +- scripts/audit-allow-list | 72 +++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100755 scripts/audit-allow-list diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index bf554eac1f..d13d6c243b 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -32,7 +32,7 @@ jobs: run: npm run bootstrap - name: audit tools - run: npm audit --production --audit-level=moderate + run: npm audit --audit-level=moderate --json | scripts/audit-allow-list - name: audit packages run: npm run audit-all diff --git a/scripts/audit-allow-list b/scripts/audit-allow-list new file mode 100755 index 0000000000..61c1008e16 --- /dev/null +++ b/scripts/audit-allow-list @@ -0,0 +1,72 @@ +#!/usr/bin/env node + +/* +This script takes the output of npm audit --json from stdin +and writes a filtered version to stdout. +The filtered version will have the entries listed in `AUDIT_ALLOW_LIST` removed. +Specifically, each property of `vulnerabilities` in the input is matched by name in the allow list. + +Sample output of `npm audit --json`: +{ + // Other properties ... + "vulnerabilities": { + "meow": { + // Other properties ... + }, + "trim-newlines": { + // Other properties ... + } + } +} +*/ + +'use strict' +const fs = require('fs'); + +const USAGE = "Usage: npm audit --json | scripts/audit-allow-list" + +const AUDIT_ALLOW_LIST = [ + { + name: "meow", + advisory: "https://www.npmjs.com/advisories/1753", + justification: "dependency of lerna (dev only)" + }, + { + name: "trim-newlines", + advisory: "https://www.npmjs.com/advisories/1753", + justification: "dependency of lerna (dev only)" + } +] + +/** + * @param audits - JavaScript object matching the schema of `npm audit --json` + * @param allowedAudits - List of package names to exclude from the audit +*/ +function filterAudits(audits, allowedAudits) { + for (const vuln in audits.vulnerabilities) { + if (allowedAudits.includes(vuln)) { + // Skip error handling; we shouldn't have any non-configurable properties here. + delete audits.vulnerabilities[vuln] + } + } + return audits +} + +const input = fs.readFileSync("/dev/stdin", "utf-8") +if (input === "") { + console.error(USAGE) + process.exit(1) +} + +// This function assumes `audits` has the right structure. +// Just let the error terminate the process if the input doesn't match the schema. +const filteredAudits = filterAudits(JSON.parse(input), AUDIT_ALLOW_LIST.map(x => x.name)) + +// This is to mimic the behavior of `npm audit`: +const numVulnerabilities = Object.keys(filteredAudits.vulnerabilities).length +if (numVulnerabilities > 0) { + console.log(filteredAudits) + console.log("There are some unrecognized vulnerabilities from `npm audit`.") + console.log("Run `npm audit` to get pretty-printed output.") + process.exit(1) +} \ No newline at end of file From 2122bd64b83d780aed5d855acd1bd8147115eb58 Mon Sep 17 00:00:00 2001 From: Brian Cristante <33549821+brcrista@users.noreply.github.com> Date: Tue, 15 Jun 2021 12:34:45 -0400 Subject: [PATCH 5/7] Update the script for NPM 6 --- scripts/audit-allow-list | 77 ++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/scripts/audit-allow-list b/scripts/audit-allow-list index 61c1008e16..b67cb1adef 100755 --- a/scripts/audit-allow-list +++ b/scripts/audit-allow-list @@ -6,18 +6,33 @@ and writes a filtered version to stdout. The filtered version will have the entries listed in `AUDIT_ALLOW_LIST` removed. Specifically, each property of `vulnerabilities` in the input is matched by name in the allow list. -Sample output of `npm audit --json`: +Sample output of `npm audit --json` (NPM v6): + { - // Other properties ... - "vulnerabilities": { - "meow": { - // Other properties ... - }, - "trim-newlines": { - // Other properties ... + "actions": [ + { + "action": "review", + "module": "trim-newlines", + "resolves": [ + { + "id": 1753, + "path": "lerna>@lerna/publish>@lerna/version>@lerna/conventional-commits>conventional-changelog-core>get-pkg-repo>meow>trim-newlines", + "dev": true, + "optional": false, + "bundled": false + } + ] } - } + ], + // Other properties ... } + + +The reason we have this script is that there may be low-severity or unexploitable vulnerabilities +that have not yet been fixed in newer versions of the package. + +Note: if we update to NPM v7, we will have to change this script because the `npm audit` output will be different. +See commit 935647112d96fa5cf82e61314f7135376d24f291 in https://github.com/actions/toolkit/pull/846. */ 'use strict' @@ -25,31 +40,30 @@ const fs = require('fs'); const USAGE = "Usage: npm audit --json | scripts/audit-allow-list" +// To add entires to the allow list: +// - Run `npm audit --json` +// - Copy `path` from each `actions[k].resolves` you want to allow +// - Fill in the `advisoryUrl` and `justification` (these are just for documentation) const AUDIT_ALLOW_LIST = [ { - name: "meow", - advisory: "https://www.npmjs.com/advisories/1753", - justification: "dependency of lerna (dev only)" + path: "lerna>@lerna/publish>@lerna/version>@lerna/conventional-commits>conventional-changelog-core>get-pkg-repo>meow>trim-newlines", + advisoryUrl: "https://www.npmjs.com/advisories/1753", + justification: "dependency of lerna (dev only); low severity" }, { - name: "trim-newlines", - advisory: "https://www.npmjs.com/advisories/1753", - justification: "dependency of lerna (dev only)" + path: "lerna>@lerna/version>@lerna/conventional-commits>conventional-changelog-core>get-pkg-repo>meow>trim-newlines", + advisoryUrl: "https://www.npmjs.com/advisories/1753", + justification: "dependency of lerna (dev only); low severity" } ] /** * @param audits - JavaScript object matching the schema of `npm audit --json` - * @param allowedAudits - List of package names to exclude from the audit + * @param allowedPaths - List of dependency paths to exclude from the audit */ -function filterAudits(audits, allowedAudits) { - for (const vuln in audits.vulnerabilities) { - if (allowedAudits.includes(vuln)) { - // Skip error handling; we shouldn't have any non-configurable properties here. - delete audits.vulnerabilities[vuln] - } - } - return audits +function filterVulnerabilities(audits, allowedPaths) { + const vulnerabilities = audits.actions.flatMap(x => x.resolves) + return vulnerabilities.filter(x => !allowedPaths.includes(x.path)) } const input = fs.readFileSync("/dev/stdin", "utf-8") @@ -58,15 +72,16 @@ if (input === "") { process.exit(1) } +const audits = JSON.parse(input) +const allowedPaths = AUDIT_ALLOW_LIST.map(x => x.path) // This function assumes `audits` has the right structure. // Just let the error terminate the process if the input doesn't match the schema. -const filteredAudits = filterAudits(JSON.parse(input), AUDIT_ALLOW_LIST.map(x => x.name)) +const remainingVulnerabilities = filterVulnerabilities(audits, allowedPaths) -// This is to mimic the behavior of `npm audit`: -const numVulnerabilities = Object.keys(filteredAudits.vulnerabilities).length -if (numVulnerabilities > 0) { - console.log(filteredAudits) - console.log("There are some unrecognized vulnerabilities from `npm audit`.") - console.log("Run `npm audit` to get pretty-printed output.") +// `npm audit` will return exit code 1 if it finds vulnerabilities. +// This script should do the same. +if (remainingVulnerabilities.length > 0) { + console.log("There are some unrecognized vulnerabilities from `npm audit`:") + console.log(JSON.stringify(remainingVulnerabilities, null, 2)) process.exit(1) } \ No newline at end of file From ccad444cbe111713415c45dc57308addc4421382 Mon Sep 17 00:00:00 2001 From: Brian Cristante <33549821+brcrista@users.noreply.github.com> Date: Tue, 15 Jun 2021 12:36:15 -0400 Subject: [PATCH 6/7] Pretty-print npm audit output if vulns are found --- .github/workflows/audit.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index d13d6c243b..7da8da9b2e 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -32,7 +32,8 @@ jobs: run: npm run bootstrap - name: audit tools - run: npm audit --audit-level=moderate --json | scripts/audit-allow-list + # `|| npm audit` to pretty-print the output if vulnerabilies are found after filtering. + run: npm audit --audit-level=moderate --json | scripts/audit-allow-list || npm audit --audit-level=moderate - name: audit packages run: npm run audit-all From d9e767737628a8778103245fc2179c38842d6571 Mon Sep 17 00:00:00 2001 From: Brian Cristante <33549821+brcrista@users.noreply.github.com> Date: Tue, 15 Jun 2021 12:46:51 -0400 Subject: [PATCH 7/7] Improve error message --- scripts/audit-allow-list | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/audit-allow-list b/scripts/audit-allow-list index b67cb1adef..761b9a2c8b 100755 --- a/scripts/audit-allow-list +++ b/scripts/audit-allow-list @@ -36,7 +36,7 @@ See commit 935647112d96fa5cf82e61314f7135376d24f291 in https://github.com/action */ 'use strict' -const fs = require('fs'); +const fs = require('fs') const USAGE = "Usage: npm audit --json | scripts/audit-allow-list" @@ -80,8 +80,10 @@ const remainingVulnerabilities = filterVulnerabilities(audits, allowedPaths) // `npm audit` will return exit code 1 if it finds vulnerabilities. // This script should do the same. -if (remainingVulnerabilities.length > 0) { - console.log("There are some unrecognized vulnerabilities from `npm audit`:") +const numVulnerabilities = remainingVulnerabilities.length +if (numVulnerabilities > 0) { + const pluralized = numVulnerabilities === 1 ? "y" : "ies" + console.log(`Found ${numVulnerabilities} unrecognized vulnerabilit${pluralized} from \`npm audit\`:`) console.log(JSON.stringify(remainingVulnerabilities, null, 2)) process.exit(1) } \ No newline at end of file