Skip to content

Commit

Permalink
fix: sub dependencies (#439)
Browse files Browse the repository at this point in the history
This change aims to fix the following case when the same packages have different locations.

```
node_modules/svgo/node_modules/css-select:css-select@3.1.2
node_modules/css-select:css-select@4.1.3
```
  • Loading branch information
ybiquitous authored Jun 29, 2021
1 parent e68a5a2 commit 5d970e8
Show file tree
Hide file tree
Showing 14 changed files with 4,842 additions and 4,735 deletions.
9,323 changes: 4,680 additions & 4,643 deletions dist/index.cjs

Large diffs are not rendered by default.

15 changes: 14 additions & 1 deletion lib/__tests__/__snapshots__/aggregateReport.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,23 @@ exports[`aggregateReport() 1`] = `
Object {
"added": Array [
Object {
"location": null,
"name": "arrify",
"version": "2.0.1",
},
],
"packageCount": 3,
"packageCount": 4,
"packageUrls": Object {
"arrify": Object {
"name": "arrify",
"type": "github",
"url": "https://github.com/sindresorhus/arrify",
},
"css-select": Object {
"name": "css-select",
"type": "github",
"url": "https://github.com/fb55/css-select",
},
"xmldom": Object {
"name": "xmldom",
"type": "github",
Expand All @@ -28,12 +34,19 @@ Object {
},
"removed": Array [
Object {
"location": "svgo/node_modules/css-select",
"name": "css-select",
"version": "3.1.2",
},
Object {
"location": null,
"name": "xmldom",
"version": "0.5.0",
},
],
"updated": Array [
Object {
"location": null,
"name": "y18n",
"previousVersion": "4.0.0",
"severity": "High",
Expand Down
13 changes: 7 additions & 6 deletions lib/__tests__/aggregateReport.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ test("aggregateReport()", async () => {
});

const before = new Map();
before.set("y18n", "4.0.0");
before.set("xmldom", "0.5.0");
before.set("rimraf", "3.0.2");
before.set("y18n:y18n", "4.0.0");
before.set("xmldom:xmldom", "0.5.0");
before.set("rimraf:rimraf", "3.0.2");
before.set("svgo/node_modules/css-select:css-select", "3.1.2");

const after = new Map();
after.set("y18n", "4.0.1");
after.set("arrify", "2.0.1");
after.set("rimraf", "3.0.2");
after.set("y18n:y18n", "4.0.1");
after.set("arrify:arrify", "2.0.1");
after.set("rimraf:rimraf", "3.0.2");

const result = await aggregateReport(audit, before, after);
expect(result).toMatchSnapshot();
Expand Down
2 changes: 1 addition & 1 deletion lib/__tests__/buildPullRequestBody.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ This pull request fixes the vulnerable packages via npm [7.7.0](https://github.c
| Package | Version | Source | Detail |
|:--------|:-------:|:------:|:-------|
| [minimist](https://www.npmjs.com/package/minimist/v/1.2.4) | \`1.2.1\` → \`1.2.4\` | [github](https://github.com/substack/minimist) | - |
| [minimist](https://www.npmjs.com/package/minimist/v/1.2.4) (\`foo/node_modules/minimist\`) | \`1.2.1\` → \`1.2.4\` | [github](https://github.com/substack/minimist) | - |
| [mocha](https://www.npmjs.com/package/mocha/v/1.4.3) | \`1.3.0\` → \`1.4.3\` | [github](https://github.com/mochajs/mocha) | **[Low]** Prototype Pollution ([ref](https://npmjs.com/advisories/1179)) |
</details>
Expand Down
6 changes: 6 additions & 0 deletions lib/__tests__/fixtures/empty_package.json/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions lib/__tests__/fixtures/noversion_package.json/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions lib/__tests__/fixtures/report.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
{
"name": "minimist",
"version": "1.2.4",
"location": "foo/node_modules/minimist",
"previousVersion": "1.2.1",
"severity": null,
"title": null,
Expand All @@ -13,6 +14,7 @@
{
"name": "mocha",
"version": "1.4.3",
"location": null,
"previousVersion": "1.3.0",
"severity": "Low",
"title": "Prototype Pollution",
Expand Down
5 changes: 3 additions & 2 deletions lib/__tests__/listPackages.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ const fixtures = new URL("./fixtures", import.meta.url).pathname;
test("listPackages()", async () => {
const packages = await listPackages({ silent: true });
expect(packages.size).not.toEqual(0);
expect(packages.get("jest")).toMatch(/^\d+\.\d+\.\d+$/u);
expect(packages.get("@actions/core")).toMatch(/^\d+\.\d+\.\d+$/u);
expect(packages.get("jest:jest")).toMatch(/^\d+\.\d+\.\d+$/u);
expect(packages.get("@actions/core:@actions/core")).toMatch(/^\d+\.\d+\.\d+$/u);
expect(packages.get("eslint/node_modules/globals:globals")).toMatch(/^\d+\.\d+\.\d+$/u);
});

test("listPackages() with an empty package.json", async () => {
Expand Down
20 changes: 16 additions & 4 deletions lib/aggregateReport.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import packageRepoUrls from "./packageRepoUrls.js";
import capitalize from "./utils/capitalize.js";
import semverToNumber from "./utils/semverToNumber.js";
import splitPackageName from "./utils/splitPackageName.js";

/**
* @param {{ name: string, version: string }} a
Expand Down Expand Up @@ -29,7 +30,8 @@ export default async function aggregateReport(audit, beforePackages, afterPackag
const added = [];
afterPackages.forEach((version, name) => {
if (!beforePackages.has(name)) {
added.push({ name, version });
const pkg = splitPackageName(name);
added.push({ name: pkg.name, location: pkg.location, version });
}
});
added.sort(byNameAndVersion);
Expand All @@ -38,7 +40,8 @@ export default async function aggregateReport(audit, beforePackages, afterPackag
const removed = [];
beforePackages.forEach((version, name) => {
if (!afterPackages.has(name)) {
removed.push({ name, version });
const pkg = splitPackageName(name);
removed.push({ name: pkg.name, location: pkg.location, version });
}
});
removed.sort(byNameAndVersion);
Expand All @@ -48,11 +51,20 @@ export default async function aggregateReport(audit, beforePackages, afterPackag
afterPackages.forEach((version, name) => {
const previousVersion = beforePackages.get(name);
if (version !== previousVersion && previousVersion != null) {
const info = audit.get(name);
const pkg = splitPackageName(name);
const info = audit.get(pkg.name);
const severity = info == null ? null : capitalize(info.severity);
const title = info == null ? null : info.title;
const url = info == null ? null : info.url;
updated.push({ name, version, previousVersion, severity, title, url });
updated.push({
name: pkg.name,
location: pkg.location,
version,
previousVersion,
severity,
title,
url,
});
}
});
updated.sort(byNameAndVersion);
Expand Down
83 changes: 49 additions & 34 deletions lib/buildPullRequestBody.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,43 @@ import { PACKAGE_NAME, PACKAGE_URL } from "./constants.js";
const EMPTY = "-";

/**
* @param {Report} report
* @param {string} npmVersion
* @returns {string}
* @param {string} name
* @param {string} version
* @param {string | null} location
*/
export default function buildPullRequestBody(report, npmVersion) {
/**
* @param {string} name
* @param {string} version
*/
const npmPackage = (name, version) =>
`[${name}](https://www.npmjs.com/package/${name}/v/${version})`;
const npmPackage = (name, version, location) => {
let result = `[${name}](https://www.npmjs.com/package/${name}/v/${version})`;
if (location != null) {
result += ` (\`${location}\`)`;
}
return result;
};

/**
* @param {...string} items
*/
const buildTableRow = (...items) => `| ${items.join(" | ")} |`;
/**
* @param {...string} items
*/
const buildTableRow = (...items) => `| ${items.join(" | ")} |`;

/**
* @param {string} name
* @returns {string}
*/
const repoLink = (name) => {
const url = report.packageUrls[name];
return url ? `[${url.type}](${url.url})` : EMPTY;
};
/**
* @param {Report} report
* @param {string} name
*/
const repoLink = (report, name) => {
const url = report.packageUrls[name];
return url ? `[${url.type}](${url.url})` : EMPTY;
};

/**
* @param {string} version
* @returns {string}
*/
const versionLabel = (version) => `\`${version}\``;
/**
* @param {string} version
*/
const versionLabel = (version) => `\`${version}\``;

/**
* @param {Report} report
* @param {string} npmVersion
* @returns {string}
*/
export default function buildPullRequestBody(report, npmVersion) {
const header = [];
header.push("| Package | Version | Source | Detail |");
header.push("|:--------|:-------:|:------:|:-------|");
Expand All @@ -51,16 +56,16 @@ export default function buildPullRequestBody(report, npmVersion) {
lines.push("");
lines.push(...header);

report.updated.forEach(({ name, version, previousVersion, severity, title, url }) => {
report.updated.forEach(({ name, version, location, previousVersion, severity, title, url }) => {
let extra = EMPTY;
if (severity != null && title != null && url != null) {
extra = `**[${severity}]** ${title} ([ref](${url}))`;
}
lines.push(
buildTableRow(
npmPackage(name, version),
npmPackage(name, version, location),
`${versionLabel(previousVersion)}${versionLabel(version)}`,
repoLink(name),
repoLink(report, name),
extra
)
);
Expand All @@ -76,9 +81,14 @@ export default function buildPullRequestBody(report, npmVersion) {
lines.push(`<summary><strong>Added (${report.added.length})</strong></summary>`);
lines.push("");
lines.push(...header);
report.added.forEach(({ name, version }) => {
report.added.forEach(({ name, version, location }) => {
lines.push(
buildTableRow(npmPackage(name, version), versionLabel(version), repoLink(name), EMPTY)
buildTableRow(
npmPackage(name, version, location),
versionLabel(version),
repoLink(report, name),
EMPTY
)
);
});
lines.push("");
Expand All @@ -91,9 +101,14 @@ export default function buildPullRequestBody(report, npmVersion) {
lines.push(`<summary><strong>Removed (${report.removed.length})</strong></summary>`);
lines.push("");
lines.push(...header);
report.removed.forEach(({ name, version }) => {
report.removed.forEach(({ name, version, location }) => {
lines.push(
buildTableRow(npmPackage(name, version), versionLabel(version), repoLink(name), EMPTY)
buildTableRow(
npmPackage(name, version, location),
versionLabel(version),
repoLink(report, name),
EMPTY
)
);
});
lines.push("");
Expand Down
63 changes: 30 additions & 33 deletions lib/listPackages.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,55 +2,52 @@ import { exec } from "@actions/exec";
import npmArgs from "./npmArgs.js";

/**
* @param {import("@actions/exec").ExecOptions?} options
* @param {import("@actions/exec").ExecOptions} [options]
* @returns {Promise<Map<string, string>>}
*/
export default async function listPackages(options = {}) {
const cwd = options.cwd || process.cwd();
let lines = "";
let stderr = "";
const returnCode = await exec("npm", npmArgs("ls", "--parseable", "--long", "--all"), {
listeners: {
stdout: (data) => {
lines += data.toString();
const returnCode = await exec(
"npm",
npmArgs("ls", "--parseable", "--long", "--all", "--package-lock-only"),
{
listeners: {
stdout: (data) => {
lines += data.toString();
},
stderr: (data) => {
stderr += data.toString();
},
},
stderr: (data) => {
stderr += data.toString();
},
},
ignoreReturnCode: true,
...options,
});
ignoreReturnCode: true,
...options,
cwd,
}
);

// NOTE: Ignore missing peer deps error.
if (returnCode !== 0 && !stderr.includes("npm ERR! missing:")) {
throw new Error(`"npm ls" failed`);
}

const packages = /** @type {Map<string, string>} */ new Map();
/** @type {Map<string, string>} */
const packages = new Map();

lines
.split("\n")
.filter((line) => line.trim())
.filter((line) => line.trim().length !== 0)
.map((line) => line.replace(`${cwd}/node_modules/`, ""))
.forEach((line) => {
const [, pkg] = line.split(":", 2);
if (pkg == null) {
throw new Error(`Invalid line: "${line}"`);
const versionSeparatorPosition = line.lastIndexOf("@");
if (versionSeparatorPosition === line.length - 1) {
return; // exclude when no version
}

const match = /^(?<name>@?\S+)@(?<version>\S+)$/u.exec(pkg);
if (match == null || match.groups == null) {
return; // skip
}

/* eslint-disable dot-notation, prefer-destructuring -- Prevent TS4111 */
const name = match.groups["name"];
const version = match.groups["version"];
/* eslint-enable */

if (name == null || version == null) {
throw new Error(`Invalid name and version: "${line}"`);
}

packages.set(name.trim(), version.trim());
const name = line.slice(0, versionSeparatorPosition);
const version = line.slice(versionSeparatorPosition + 1);
packages.set(name, version);
});

return packages;
}
Loading

0 comments on commit 5d970e8

Please sign in to comment.