From ebc8ba6a61b45f8e746ea3892c14ae0a3d0ea6be Mon Sep 17 00:00:00 2001 From: Daniel Stockman Date: Wed, 1 Aug 2018 18:18:04 -0700 Subject: [PATCH] feat(publish): Validate npm registry and package access prerequisites Fixes #55 Fixes #1045 Fixes #1347 --- .../publish/__tests__/publish-canary.test.js | 2 + .../publish/__tests__/publish-command.test.js | 2 + .../__tests__/publish-licenses.test.js | 2 + .../publish-lifecycle-scripts.test.js | 2 + .../publish-relative-file-specifiers.test.js | 2 + .../publish/__tests__/publish-tagging.test.js | 2 + commands/publish/index.js | 12 +++- .../__mocks__/verify-npm-package-access.js | 4 ++ .../lib/__mocks__/verify-npm-registry.js | 4 ++ .../publish/lib/verify-npm-package-access.js | 71 +++++++++++++++++++ commands/publish/lib/verify-npm-registry.js | 34 +++++++++ commands/publish/package.json | 2 + package-lock.json | 2 + 13 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 commands/publish/lib/__mocks__/verify-npm-package-access.js create mode 100644 commands/publish/lib/__mocks__/verify-npm-registry.js create mode 100644 commands/publish/lib/verify-npm-package-access.js create mode 100644 commands/publish/lib/verify-npm-registry.js diff --git a/commands/publish/__tests__/publish-canary.test.js b/commands/publish/__tests__/publish-canary.test.js index 0619922095..214a6dbc2f 100644 --- a/commands/publish/__tests__/publish-canary.test.js +++ b/commands/publish/__tests__/publish-canary.test.js @@ -5,6 +5,8 @@ jest.unmock("@lerna/collect-updates"); // local modules _must_ be explicitly mocked jest.mock("../lib/get-packages-without-license"); +jest.mock("../lib/verify-npm-package-access"); +jest.mock("../lib/verify-npm-registry"); const fs = require("fs-extra"); const path = require("path"); diff --git a/commands/publish/__tests__/publish-command.test.js b/commands/publish/__tests__/publish-command.test.js index 85807e2b5f..fddbe2bdcf 100644 --- a/commands/publish/__tests__/publish-command.test.js +++ b/commands/publish/__tests__/publish-command.test.js @@ -2,6 +2,8 @@ // local modules _must_ be explicitly mocked jest.mock("../lib/get-packages-without-license"); +jest.mock("../lib/verify-npm-package-access"); +jest.mock("../lib/verify-npm-registry"); // FIXME: better mock for version command jest.mock("../../version/lib/git-push"); jest.mock("../../version/lib/is-anything-committed"); diff --git a/commands/publish/__tests__/publish-licenses.test.js b/commands/publish/__tests__/publish-licenses.test.js index a553ae3a11..21b7ffb310 100644 --- a/commands/publish/__tests__/publish-licenses.test.js +++ b/commands/publish/__tests__/publish-licenses.test.js @@ -1,6 +1,8 @@ "use strict"; // local modules _must_ be explicitly mocked +jest.mock("../lib/verify-npm-package-access"); +jest.mock("../lib/verify-npm-registry"); jest.mock("../lib/create-temp-licenses", () => jest.fn(() => Promise.resolve())); jest.mock("../lib/remove-temp-licenses", () => jest.fn(() => Promise.resolve())); // FIXME: better mock for version command diff --git a/commands/publish/__tests__/publish-lifecycle-scripts.test.js b/commands/publish/__tests__/publish-lifecycle-scripts.test.js index 7cf1e3786c..02381e323d 100644 --- a/commands/publish/__tests__/publish-lifecycle-scripts.test.js +++ b/commands/publish/__tests__/publish-lifecycle-scripts.test.js @@ -2,6 +2,8 @@ // local modules _must_ be explicitly mocked jest.mock("../lib/get-packages-without-license"); +jest.mock("../lib/verify-npm-package-access"); +jest.mock("../lib/verify-npm-registry"); // FIXME: better mock for version command jest.mock("../../version/lib/git-push"); jest.mock("../../version/lib/is-anything-committed"); diff --git a/commands/publish/__tests__/publish-relative-file-specifiers.test.js b/commands/publish/__tests__/publish-relative-file-specifiers.test.js index daea957ac9..aaace1b72d 100644 --- a/commands/publish/__tests__/publish-relative-file-specifiers.test.js +++ b/commands/publish/__tests__/publish-relative-file-specifiers.test.js @@ -5,6 +5,8 @@ jest.unmock("@lerna/collect-updates"); // local modules _must_ be explicitly mocked jest.mock("../lib/get-packages-without-license"); +jest.mock("../lib/verify-npm-package-access"); +jest.mock("../lib/verify-npm-registry"); // FIXME: better mock for version command jest.mock("../../version/lib/git-push"); jest.mock("../../version/lib/is-anything-committed"); diff --git a/commands/publish/__tests__/publish-tagging.test.js b/commands/publish/__tests__/publish-tagging.test.js index 55c0ca8321..08b5179298 100644 --- a/commands/publish/__tests__/publish-tagging.test.js +++ b/commands/publish/__tests__/publish-tagging.test.js @@ -2,6 +2,8 @@ // local modules _must_ be explicitly mocked jest.mock("../lib/get-packages-without-license"); +jest.mock("../lib/verify-npm-package-access"); +jest.mock("../lib/verify-npm-registry"); // FIXME: better mock for version command jest.mock("../../version/lib/git-push"); jest.mock("../../version/lib/is-anything-committed"); diff --git a/commands/publish/index.js b/commands/publish/index.js index 9d80cd090f..e156a55f44 100644 --- a/commands/publish/index.js +++ b/commands/publish/index.js @@ -25,6 +25,8 @@ const getTaggedPackages = require("./lib/get-tagged-packages"); const getPackagesWithoutLicense = require("./lib/get-packages-without-license"); const gitCheckout = require("./lib/git-checkout"); const removeTempLicenses = require("./lib/remove-temp-licenses"); +const verifyNpmRegistry = require("./lib/verify-npm-registry"); +const verifyNpmPackageAccess = require("./lib/verify-npm-package-access"); module.exports = factory; @@ -89,7 +91,9 @@ class PublishCommand extends Command { ) : [this.packagesToPublish]; - return Promise.resolve().then(() => this.prepareLicenseActions()); + return Promise.resolve() + .then(() => this.prepareRegistryActions()) + .then(() => this.prepareLicenseActions()); }); } @@ -232,6 +236,12 @@ class PublishCommand extends Command { }); } + prepareRegistryActions() { + return Promise.resolve() + .then(() => verifyNpmRegistry(this.project.rootPath, this.npmConfig)) + .then(() => verifyNpmPackageAccess(this.packagesToPublish, this.project.rootPath, this.npmConfig)); + } + updateCanaryVersions() { const publishableUpdates = this.updates.filter(node => !node.pkg.private); diff --git a/commands/publish/lib/__mocks__/verify-npm-package-access.js b/commands/publish/lib/__mocks__/verify-npm-package-access.js new file mode 100644 index 0000000000..bda732969a --- /dev/null +++ b/commands/publish/lib/__mocks__/verify-npm-package-access.js @@ -0,0 +1,4 @@ +"use strict"; + +// to mock user modules, you _must_ call `jest.mock('./path/to/module')` +module.exports = jest.fn(() => Promise.resolve()); diff --git a/commands/publish/lib/__mocks__/verify-npm-registry.js b/commands/publish/lib/__mocks__/verify-npm-registry.js new file mode 100644 index 0000000000..bda732969a --- /dev/null +++ b/commands/publish/lib/__mocks__/verify-npm-registry.js @@ -0,0 +1,4 @@ +"use strict"; + +// to mock user modules, you _must_ call `jest.mock('./path/to/module')` +module.exports = jest.fn(() => Promise.resolve()); diff --git a/commands/publish/lib/verify-npm-package-access.js b/commands/publish/lib/verify-npm-package-access.js new file mode 100644 index 0000000000..ee7ba09b37 --- /dev/null +++ b/commands/publish/lib/verify-npm-package-access.js @@ -0,0 +1,71 @@ +"use strict"; + +const log = require("npmlog"); +const childProcess = require("@lerna/child-process"); +const getExecOpts = require("@lerna/get-npm-exec-opts"); +const ValidationError = require("@lerna/validation-error"); + +module.exports = verifyNpmPackageAccess; + +function verifyNpmPackageAccess(packages, location, { registry }) { + log.silly("verifyNpmPackageAccess"); + + const args = [ + "access", + "ls-packages", + // immediate feedback from request errors, not excruciatingly slow retries + // @see https://docs.npmjs.com/misc/config#fetch-retries + "--fetch-retries=0", + // including http requests makes raw logging easier to debug for end users + "--loglevel=http", + ]; + const opts = getExecOpts({ location }, registry); + + // we do not need special log handling + delete opts.pkg; + + return childProcess.exec("npm", args, opts).then( + result => { + const permission = JSON.parse(result.stdout); + + for (const pkg of packages) { + if (permission[pkg.name] !== "read-write") { + throw new ValidationError( + "EACCESS", + "You do not have write permission required to publish %j", + pkg.name + ); + } + } + }, + // only catch npm error, not validation error above + ({ stderr }) => { + // pass if registry does not support ls-packages endpoint + if (/E500/.test(stderr) && /ECONNREFUSED/.test(stderr)) { + // most likely a private registry (npm Enterprise, verdaccio, etc) + log.warn( + "EREGISTRY", + "Registry %j does not support `npm access ls-packages`, skipping permission checks...", + registry + ); + + // don't log redundant errors + return; + } + + if (/ENEEDAUTH/.test(stderr)) { + throw new ValidationError( + "ENEEDAUTH", + "You must be logged in to publish packages. Use `npm login` and try again." + ); + } + + // Log the error cleanly to stderr, it already has npmlog decorations + log.pause(); + console.error(stderr); // eslint-disable-line no-console + log.resume(); + + throw new ValidationError("EWHOAMI", "Authentication error. Use `npm whoami` to troubleshoot."); + } + ); +} diff --git a/commands/publish/lib/verify-npm-registry.js b/commands/publish/lib/verify-npm-registry.js new file mode 100644 index 0000000000..b6a8066f3a --- /dev/null +++ b/commands/publish/lib/verify-npm-registry.js @@ -0,0 +1,34 @@ +"use strict"; + +const log = require("npmlog"); +const childProcess = require("@lerna/child-process"); +const getExecOpts = require("@lerna/get-npm-exec-opts"); +const ValidationError = require("@lerna/validation-error"); + +module.exports = verifyNpmRegistry; + +function verifyNpmRegistry(location, { registry }) { + log.silly("verifyNpmRegistry"); + + const args = [ + "ping", + // immediate feedback from request errors, not excruciatingly slow retries + // @see https://docs.npmjs.com/misc/config#fetch-retries + "--fetch-retries=0", + // including http requests makes raw logging easier to debug for end users + "--loglevel=http", + ]; + const opts = getExecOpts({ location }, registry); + + // we do not need special log handling + delete opts.pkg; + + return childProcess.exec("npm", args, opts).catch(({ stderr }) => { + // Log the error cleanly to stderr, it already has npmlog decorations + log.pause(); + console.error(stderr); // eslint-disable-line no-console + log.resume(); + + throw new ValidationError("EREGISTRY", "Connection to npm registry failed"); + }); +} diff --git a/commands/publish/package.json b/commands/publish/package.json index 286fce4543..37edeb923f 100644 --- a/commands/publish/package.json +++ b/commands/publish/package.json @@ -36,11 +36,13 @@ "@lerna/child-process": "file:../../core/child-process", "@lerna/collect-updates": "file:../../utils/collect-updates", "@lerna/command": "file:../../core/command", + "@lerna/get-npm-exec-opts": "file:../../utils/get-npm-exec-opts", "@lerna/npm-dist-tag": "file:../../utils/npm-dist-tag", "@lerna/npm-publish": "file:../../utils/npm-publish", "@lerna/output": "file:../../utils/output", "@lerna/run-lifecycle": "file:../../utils/run-lifecycle", "@lerna/run-parallel-batches": "file:../../utils/run-parallel-batches", + "@lerna/validation-error": "file:../../core/validation-error", "@lerna/version": "file:../version", "fs-extra": "^6.0.1", "npm-package-arg": "^6.0.0", diff --git a/package-lock.json b/package-lock.json index e8e706eeaf..cbc127b132 100644 --- a/package-lock.json +++ b/package-lock.json @@ -585,11 +585,13 @@ "@lerna/child-process": "file:core/child-process", "@lerna/collect-updates": "file:utils/collect-updates", "@lerna/command": "file:core/command", + "@lerna/get-npm-exec-opts": "file:utils/get-npm-exec-opts", "@lerna/npm-dist-tag": "file:utils/npm-dist-tag", "@lerna/npm-publish": "file:utils/npm-publish", "@lerna/output": "file:utils/output", "@lerna/run-lifecycle": "file:utils/run-lifecycle", "@lerna/run-parallel-batches": "file:utils/run-parallel-batches", + "@lerna/validation-error": "file:core/validation-error", "@lerna/version": "file:commands/version", "fs-extra": "^6.0.1", "npm-package-arg": "^6.0.0",