diff --git a/README.md b/README.md index e9227466..1122e80c 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ When you merge this PR: - Upon successful retrieval of the OTP, it will publish the package to Npm. - Create a Github release with change logs (You can customize release notes using [release.yml](https://docs.github.com/en/repositories/releasing-projects-on-github/automatically-generated-release-notes#example-configuration)) - Leave a comment on each issues that are linked to the pull reqeuests of this release. This feature can be turned off by the `notify-on-the-issue` flag. -- _(Optional)_ If `provenance: true` was set, NPM will add a [Provenance](#provenance) notice to the package's public NPM page. +- _(Optional)_ If `provenance: true`, NPM will add a [Provenance](#provenance) notice to the package's public NPM page. When you close the PR without merging it: nothing will happen. @@ -232,6 +232,7 @@ If `provenance: true` is added to your `release.yml`'s **inputs**, NPM will [gen NPM has some internal [requirements](https://docs.npmjs.com/generating-provenance-statements#prerequisites) for generating provenance. Unfortunately as of May 2023, not all are documented by NPM; some key requirements are: - `id-token: write` must be added to your `release.yml`'s **permissions** +- The package must have public access. - NPM must be on version 9.5.0 or greater (this will be met if our recommended `runs-on: ubuntu-latest` is used) - NPM has some undocumented internal requirements on `package.json` completeness. For example, the [repository field](https://docs.npmjs.com/cli/v9/configuring-npm/package-json#repository) is required, and some NPM versions may require its `"url"` property to match the format `"git+https://github.com/user/repo"`. @@ -284,7 +285,8 @@ jobs: | `version-prefix` | No | A prefix to apply to the version number, which reflects in the tag and GitHub release names.
(_Default: 'v'_) | | `prerelease-prefix` | No | A prefix to apply to the prerelease version number. | | `base-tag` | No | Choose a specific tag release for your release notes. This input allows you to specify a base release (for example, v1.0.0) and will include all changes made in releases between the base release and the latest release. This input is only used for generating release notes and has no functional implications on the rest of the workflow. | -| `provenance`| No | Set as true to have NPM [generate a provenance statement](https://docs.npmjs.com/generating-provenance-statements). See [Provenance section above](#provenance) for requirements.
(_Default: `false`_) | +| `provenance`| No | Set as true to have NPM [generate a provenance statement](https://docs.npmjs.com/generating-provenance-statements). See [Provenance section above](#provenance) for requirements.
(_Default: `false`_) +| `access`| No | Set as `public` or `restricted` to change an NPM package's access status when next published. ## Motivation diff --git a/action.yml b/action.yml index ef8a5515..6302a8a9 100644 --- a/action.yml +++ b/action.yml @@ -80,6 +80,9 @@ inputs: required: false type: boolean default: 'false' + access: + description: 'If defined, sets package to public or restricted via `--access` flag on `npm publish`. Supported values are "restricted" or "public".' + required: false runs: using: 'composite' diff --git a/dist/index.js b/dist/index.js index 14a5362f..91527bc7 100644 --- a/dist/index.js +++ b/dist/index.js @@ -73459,7 +73459,7 @@ class SemVer { version = version.version } } else if (typeof version !== 'string') { - throw new TypeError(`Invalid Version: ${(__nccwpck_require__(3837).inspect)(version)}`) + throw new TypeError(`Invalid version. Must be a string. Got type "${typeof version}".`) } if (version.length > MAX_LENGTH) { @@ -78934,6 +78934,7 @@ module.exports = { ZIP_EXTENSION: '.zip', APP_NAME: 'optic-release-automation[bot]', AUTO_INPUT: 'auto', + ACCESS_OPTIONS: ['public', 'restricted'], } @@ -79211,7 +79212,7 @@ module.exports = async function ({ context, inputs, packageVersion }) { const core = __nccwpck_require__(2186) const semver = __nccwpck_require__(1383) -const { PR_TITLE_PREFIX } = __nccwpck_require__(6818) +const { ACCESS_OPTIONS, PR_TITLE_PREFIX } = __nccwpck_require__(6818) const { callApi } = __nccwpck_require__(4235) const { tagVersionInGit } = __nccwpck_require__(9143) const { revertCommit } = __nccwpck_require__(5765) @@ -79219,10 +79220,7 @@ const { publishToNpm } = __nccwpck_require__(1433) const { notifyIssues } = __nccwpck_require__(8361) const { logError, logInfo, logWarning } = __nccwpck_require__(653) const { execWithOutput } = __nccwpck_require__(8632) -const { - checkProvenanceViability, - getNpmVersion, -} = __nccwpck_require__(3365) +const { getProvenanceOptions, getNpmVersion } = __nccwpck_require__(3365) module.exports = async function ({ github, context, inputs }) { logInfo('** Starting Release **') @@ -79308,22 +79306,39 @@ module.exports = async function ({ github, context, inputs }) { const opticToken = inputs['optic-token'] const npmToken = inputs['npm-token'] const provenance = /true/i.test(inputs['provenance']) + const access = inputs['access'] + + if (access && !ACCESS_OPTIONS.includes(access)) { + core.setFailed( + `Invalid "access" option provided ("${access}"), should be one of "${ACCESS_OPTIONS.join( + '", "' + )}"` + ) + return + } + + const publishOptions = { + npmToken, + opticToken, + opticUrl, + npmTag, + version, + provenance, + access, + } - // Fail fast with meaningful error if user wants provenance but their setup won't deliver if (provenance) { - const npmVersion = await getNpmVersion() - checkProvenanceViability(npmVersion) + const extraOptions = await getProvenanceOptions( + await getNpmVersion(), + publishOptions + ) + if (extraOptions) { + Object.assign(publishOptions, extraOptions) + } } if (npmToken) { - await publishToNpm({ - npmToken, - opticToken, - opticUrl, - npmTag, - version, - provenance, - }) + await publishToNpm(publishOptions) } else { logWarning('missing npm-token') } @@ -79660,11 +79675,11 @@ exports.execWithOutput = execWithOutput "use strict"; -const fs = __nccwpck_require__(7147) const pMap = __nccwpck_require__(1855) const { logWarning } = __nccwpck_require__(653) const { getPrNumbersFromReleaseNotes } = __nccwpck_require__(4098) +const { getLocalInfo } = __nccwpck_require__(4349) async function getLinkedIssueNumbers(github, prNumber, repoOwner, repoName) { const data = await github.graphql( @@ -79747,8 +79762,7 @@ async function notifyIssues( repo, release ) { - const packageJsonFile = fs.readFileSync('./package.json', 'utf8') - const packageJson = JSON.parse(packageJsonFile) + const packageJson = getLocalInfo() const { name: packageName, version: packageVersion } = packageJson const { body: releaseNotes, html_url: releaseUrl } = release @@ -79800,6 +79814,52 @@ async function notifyIssues( exports.notifyIssues = notifyIssues +/***/ }), + +/***/ 4349: +/***/ ((module, __unused_webpack_exports, __nccwpck_require__) => { + +"use strict"; + +const fs = __nccwpck_require__(7147) +const { execWithOutput } = __nccwpck_require__(8632) + +/** + * Get info from the registry about a package that is already published. + * + * Returns null if package is not published to NPM. + */ +async function getPublishedInfo() { + try { + const packageInfo = await execWithOutput('npm', ['view', '--json']) + return packageInfo ? JSON.parse(packageInfo) : null + } catch (error) { + if (!error?.message?.match(/code E404/)) { + throw error + } + return null + } +} + +/** + * Get info from the local package.json file. + * + * This might need to become a bit more sophisticated if support for monorepos is added, + * @see https://github.com/nearform-actions/optic-release-automation-action/issues/177 + */ +function getLocalInfo() { + const packageJsonFile = fs.readFileSync('./package.json', 'utf8') + const packageInfo = JSON.parse(packageJsonFile) + + return packageInfo +} + +module.exports = { + getLocalInfo, + getPublishedInfo, +} + + /***/ }), /***/ 3365: @@ -79809,6 +79869,7 @@ exports.notifyIssues = notifyIssues const semver = __nccwpck_require__(1383) const { execWithOutput } = __nccwpck_require__(8632) +const { getLocalInfo, getPublishedInfo } = __nccwpck_require__(4349) /** * Abort if the user specified they want NPM provenance, but their CI's NPM version doesn't support it. @@ -79848,18 +79909,51 @@ function checkPermissions(npmVersion) { } /** - * Fail fast and throw a meaningful error if NPM Provenance will fail silently or misleadingly. + * NPM does an internal check on access that fails unnecessarily for first-time publication + * of unscoped packages to NPM. Unscoped packages are always public, but NPM's provenance generation + * doesn't realise this unless it sees the status in a previous release or in explicit options. + */ +async function getAccessAdjustment({ access } = {}) { + // Don't overrule any user-set access preference. + if (access) return + + const { name: packageName, publishConfig } = getLocalInfo() + + // Don't do anything for scoped packages - those require being made public explicitly. + // Let NPM's own validation handle it if a user tries to get provenance on a private package. + // `.startsWith('@')` is what a lot of NPM internal code use to detect scoped packages, + // they don't export any more sophisticated scoped name detector any more. + if (packageName.startsWith('@')) return + + // Don't do anything if the user has set any access control in package.json publishConfig. + // https://docs.npmjs.com/cli/v9/configuring-npm/package-json#publishconfig + // Let NPM deal with that internally when `npm publish` reads the local package.json file. + if (publishConfig?.access) return + + // Don't do anything if package is already published. + const publishedInfo = await getPublishedInfo() + if (publishedInfo) return + + // Set explicit public access **only** if it's unscoped (inherently public), a first publish + // (so we know NPM will fail to realise that this is inherently public), and the user + // has not attempted to explicitly set access themselves anywhere. + return { access: 'public' } +} + +/** + * Fail fast and throw a meaningful error if NPM Provenance will fail silently or misleadingly, + * and where necessary, provide new publish options without overriding user preferences or expectations. * * @see https://docs.npmjs.com/generating-provenance-statements - * - * @param {string} npmVersion */ -function checkProvenanceViability(npmVersion) { +async function getProvenanceOptions(npmVersion, publishOptions) { if (!npmVersion) throw new Error('Current npm version not provided') checkIsSupported(npmVersion) checkPermissions(npmVersion) - // There are various other provenance requirements, such as specific package.json properties, but these - // may change in future NPM versions, and do fail with meaningful errors, so we let NPM handle those. + + const extraOptions = await getAccessAdjustment(publishOptions) + + return extraOptions } /** @@ -79871,8 +79965,9 @@ async function getNpmVersion() { } module.exports = { - checkProvenanceViability, + getProvenanceOptions, getNpmVersion, + getAccessAdjustment, checkIsSupported, checkPermissions, } @@ -79887,23 +79982,16 @@ module.exports = { const { execWithOutput } = __nccwpck_require__(8632) +const { getPublishedInfo } = __nccwpck_require__(4349) async function allowNpmPublish(version) { // We need to check if the package was already published. This can happen if // the action was already executed before, but it failed in its last step // (GH release). - let packageName = null - try { - const packageInfo = await execWithOutput('npm', ['view', '--json']) - packageName = packageInfo ? JSON.parse(packageInfo).name : null - } catch (error) { - if (!error?.message?.match(/code E404/)) { - throw error - } - } + const packageInfo = await getPublishedInfo() // Package has not been published before - if (!packageName) { + if (!packageInfo?.name) { return true } @@ -79917,7 +80005,7 @@ async function allowNpmPublish(version) { // We handle both and consider them as package version not existing packageVersionInfo = await execWithOutput('npm', [ 'view', - `${packageName}@${version}`, + `${packageInfo.name}@${version}`, ]) } catch (error) { if (!error?.message?.match(/code E404/)) { @@ -79935,6 +80023,7 @@ async function publishToNpm({ npmTag, version, provenance, + access, }) { await execWithOutput('npm', [ 'config', @@ -79943,6 +80032,11 @@ async function publishToNpm({ ]) const flags = ['--tag', npmTag] + + if (access) { + flags.push('--access', access) + } + if (provenance) { flags.push('--provenance') } diff --git a/src/const.js b/src/const.js index be244653..8aae5747 100644 --- a/src/const.js +++ b/src/const.js @@ -5,4 +5,5 @@ module.exports = { ZIP_EXTENSION: '.zip', APP_NAME: 'optic-release-automation[bot]', AUTO_INPUT: 'auto', + ACCESS_OPTIONS: ['public', 'restricted'], } diff --git a/src/release.js b/src/release.js index 4de4ca6a..4ae06baf 100644 --- a/src/release.js +++ b/src/release.js @@ -3,7 +3,7 @@ const core = require('@actions/core') const semver = require('semver') -const { PR_TITLE_PREFIX } = require('./const') +const { ACCESS_OPTIONS, PR_TITLE_PREFIX } = require('./const') const { callApi } = require('./utils/callApi') const { tagVersionInGit } = require('./utils/tagVersion') const { revertCommit } = require('./utils/revertCommit') @@ -11,10 +11,7 @@ const { publishToNpm } = require('./utils/publishToNpm') const { notifyIssues } = require('./utils/notifyIssues') const { logError, logInfo, logWarning } = require('./log') const { execWithOutput } = require('./utils/execWithOutput') -const { - checkProvenanceViability, - getNpmVersion, -} = require('./utils/provenance') +const { getProvenanceOptions, getNpmVersion } = require('./utils/provenance') module.exports = async function ({ github, context, inputs }) { logInfo('** Starting Release **') @@ -100,22 +97,39 @@ module.exports = async function ({ github, context, inputs }) { const opticToken = inputs['optic-token'] const npmToken = inputs['npm-token'] const provenance = /true/i.test(inputs['provenance']) + const access = inputs['access'] + + if (access && !ACCESS_OPTIONS.includes(access)) { + core.setFailed( + `Invalid "access" option provided ("${access}"), should be one of "${ACCESS_OPTIONS.join( + '", "' + )}"` + ) + return + } + + const publishOptions = { + npmToken, + opticToken, + opticUrl, + npmTag, + version, + provenance, + access, + } - // Fail fast with meaningful error if user wants provenance but their setup won't deliver if (provenance) { - const npmVersion = await getNpmVersion() - checkProvenanceViability(npmVersion) + const extraOptions = await getProvenanceOptions( + await getNpmVersion(), + publishOptions + ) + if (extraOptions) { + Object.assign(publishOptions, extraOptions) + } } if (npmToken) { - await publishToNpm({ - npmToken, - opticToken, - opticUrl, - npmTag, - version, - provenance, - }) + await publishToNpm(publishOptions) } else { logWarning('missing npm-token') } diff --git a/src/utils/notifyIssues.js b/src/utils/notifyIssues.js index 6bb1c13a..66c8ba50 100644 --- a/src/utils/notifyIssues.js +++ b/src/utils/notifyIssues.js @@ -1,10 +1,10 @@ 'use strict' -const fs = require('fs') const pMap = require('p-map') const { logWarning } = require('../log') const { getPrNumbersFromReleaseNotes } = require('./releaseNotes') +const { getLocalInfo } = require('../utils/packageInfo') async function getLinkedIssueNumbers(github, prNumber, repoOwner, repoName) { const data = await github.graphql( @@ -87,8 +87,7 @@ async function notifyIssues( repo, release ) { - const packageJsonFile = fs.readFileSync('./package.json', 'utf8') - const packageJson = JSON.parse(packageJsonFile) + const packageJson = getLocalInfo() const { name: packageName, version: packageVersion } = packageJson const { body: releaseNotes, html_url: releaseUrl } = release diff --git a/src/utils/packageInfo.js b/src/utils/packageInfo.js new file mode 100644 index 00000000..6c5183eb --- /dev/null +++ b/src/utils/packageInfo.js @@ -0,0 +1,38 @@ +'use strict' +const fs = require('fs') +const { execWithOutput } = require('../utils/execWithOutput') + +/** + * Get info from the registry about a package that is already published. + * + * Returns null if package is not published to NPM. + */ +async function getPublishedInfo() { + try { + const packageInfo = await execWithOutput('npm', ['view', '--json']) + return packageInfo ? JSON.parse(packageInfo) : null + } catch (error) { + if (!error?.message?.match(/code E404/)) { + throw error + } + return null + } +} + +/** + * Get info from the local package.json file. + * + * This might need to become a bit more sophisticated if support for monorepos is added, + * @see https://github.com/nearform-actions/optic-release-automation-action/issues/177 + */ +function getLocalInfo() { + const packageJsonFile = fs.readFileSync('./package.json', 'utf8') + const packageInfo = JSON.parse(packageJsonFile) + + return packageInfo +} + +module.exports = { + getLocalInfo, + getPublishedInfo, +} diff --git a/src/utils/provenance.js b/src/utils/provenance.js index 6204cf10..5d4af7fc 100644 --- a/src/utils/provenance.js +++ b/src/utils/provenance.js @@ -1,6 +1,7 @@ 'use strict' const semver = require('semver') const { execWithOutput } = require('./execWithOutput') +const { getLocalInfo, getPublishedInfo } = require('./packageInfo') /** * Abort if the user specified they want NPM provenance, but their CI's NPM version doesn't support it. @@ -40,18 +41,51 @@ function checkPermissions(npmVersion) { } /** - * Fail fast and throw a meaningful error if NPM Provenance will fail silently or misleadingly. + * NPM does an internal check on access that fails unnecessarily for first-time publication + * of unscoped packages to NPM. Unscoped packages are always public, but NPM's provenance generation + * doesn't realise this unless it sees the status in a previous release or in explicit options. + */ +async function getAccessAdjustment({ access } = {}) { + // Don't overrule any user-set access preference. + if (access) return + + const { name: packageName, publishConfig } = getLocalInfo() + + // Don't do anything for scoped packages - those require being made public explicitly. + // Let NPM's own validation handle it if a user tries to get provenance on a private package. + // `.startsWith('@')` is what a lot of NPM internal code use to detect scoped packages, + // they don't export any more sophisticated scoped name detector any more. + if (packageName.startsWith('@')) return + + // Don't do anything if the user has set any access control in package.json publishConfig. + // https://docs.npmjs.com/cli/v9/configuring-npm/package-json#publishconfig + // Let NPM deal with that internally when `npm publish` reads the local package.json file. + if (publishConfig?.access) return + + // Don't do anything if package is already published. + const publishedInfo = await getPublishedInfo() + if (publishedInfo) return + + // Set explicit public access **only** if it's unscoped (inherently public), a first publish + // (so we know NPM will fail to realise that this is inherently public), and the user + // has not attempted to explicitly set access themselves anywhere. + return { access: 'public' } +} + +/** + * Fail fast and throw a meaningful error if NPM Provenance will fail silently or misleadingly, + * and where necessary, provide new publish options without overriding user preferences or expectations. * * @see https://docs.npmjs.com/generating-provenance-statements - * - * @param {string} npmVersion */ -function checkProvenanceViability(npmVersion) { +async function getProvenanceOptions(npmVersion, publishOptions) { if (!npmVersion) throw new Error('Current npm version not provided') checkIsSupported(npmVersion) checkPermissions(npmVersion) - // There are various other provenance requirements, such as specific package.json properties, but these - // may change in future NPM versions, and do fail with meaningful errors, so we let NPM handle those. + + const extraOptions = await getAccessAdjustment(publishOptions) + + return extraOptions } /** @@ -63,8 +97,9 @@ async function getNpmVersion() { } module.exports = { - checkProvenanceViability, + getProvenanceOptions, getNpmVersion, + getAccessAdjustment, checkIsSupported, checkPermissions, } diff --git a/src/utils/publishToNpm.js b/src/utils/publishToNpm.js index 6a7bc7f0..6edd3d31 100644 --- a/src/utils/publishToNpm.js +++ b/src/utils/publishToNpm.js @@ -1,23 +1,16 @@ 'use strict' const { execWithOutput } = require('./execWithOutput') +const { getPublishedInfo } = require('./packageInfo') async function allowNpmPublish(version) { // We need to check if the package was already published. This can happen if // the action was already executed before, but it failed in its last step // (GH release). - let packageName = null - try { - const packageInfo = await execWithOutput('npm', ['view', '--json']) - packageName = packageInfo ? JSON.parse(packageInfo).name : null - } catch (error) { - if (!error?.message?.match(/code E404/)) { - throw error - } - } + const packageInfo = await getPublishedInfo() // Package has not been published before - if (!packageName) { + if (!packageInfo?.name) { return true } @@ -31,7 +24,7 @@ async function allowNpmPublish(version) { // We handle both and consider them as package version not existing packageVersionInfo = await execWithOutput('npm', [ 'view', - `${packageName}@${version}`, + `${packageInfo.name}@${version}`, ]) } catch (error) { if (!error?.message?.match(/code E404/)) { @@ -49,6 +42,7 @@ async function publishToNpm({ npmTag, version, provenance, + access, }) { await execWithOutput('npm', [ 'config', @@ -57,6 +51,11 @@ async function publishToNpm({ ]) const flags = ['--tag', npmTag] + + if (access) { + flags.push('--access', access) + } + if (provenance) { flags.push('--provenance') } diff --git a/test/notifyIssues.test.js b/test/notifyIssues.test.js index 23bf956e..5b2df8d2 100644 --- a/test/notifyIssues.test.js +++ b/test/notifyIssues.test.js @@ -29,7 +29,9 @@ function setup() { .returns('{ "name": "packageName", "version": "1.0.0"}') const { notifyIssues } = proxyquire('../src/utils/notifyIssues', { - fs: { readFileSync: readFileSyncStub }, + './packageInfo': proxyquire('../src/utils/packageInfo', { + fs: { readFileSync: readFileSyncStub }, + }), }) return { notifyIssues } diff --git a/test/packageInfo.test.js b/test/packageInfo.test.js new file mode 100644 index 00000000..646e1805 --- /dev/null +++ b/test/packageInfo.test.js @@ -0,0 +1,117 @@ +'use strict' +const tap = require('tap') +const sinon = require('sinon') +const proxyquire = require('proxyquire') + +const { getLocalInfo, getPublishedInfo } = require('../src/utils/packageInfo') + +const mockPackageInfo = { + name: 'some-package-name', + license: 'some-license', + publishConfig: { + access: 'restricted', + }, +} + +const setupPublished = ({ + value = JSON.stringify(mockPackageInfo), + error, +} = {}) => { + const execWithOutputStub = sinon.stub() + const args = ['npm', ['view', '--json']] + + if (value) { + execWithOutputStub.withArgs(...args).returns(value) + } + if (error) { + execWithOutputStub.withArgs(...args).throws(error) + } + + return proxyquire('../src/utils/packageInfo', { + './execWithOutput': { execWithOutput: execWithOutputStub }, + }) +} + +const setupLocal = ({ value = JSON.stringify(mockPackageInfo) } = {}) => { + const readFileSyncStub = sinon + .stub() + .withArgs('./package.json', 'utf8') + .returns(value) + + return proxyquire('../src/utils/packageInfo', { + fs: { readFileSync: readFileSyncStub }, + }) +} + +tap.test('getPublishedInfo does not get any info for this package', async t => { + // Check it works for real: this package is a Github Action, not published on NPM, so expect null + const packageInfo = await getPublishedInfo() + t.notOk(packageInfo) +}) + +tap.test('getPublishedInfo parses any valid JSON it finds', async t => { + const mocks = setupPublished() + + const packageInfo = await mocks.getPublishedInfo() + t.match(packageInfo, mockPackageInfo) +}) + +tap.test( + 'getPublishedInfo continues and returns null if the request 404s', + async t => { + const mocks = setupPublished({ + value: JSON.stringify(mockPackageInfo), + error: new Error('code E404 - package not found'), + }) + + const packageInfo = await mocks.getPublishedInfo() + t.match(packageInfo, null) + } +) + +tap.test( + 'getPublishedInfo throws if it encounters an internal error', + async t => { + const mocks = setupPublished({ + value: "[{ 'this:' is not ] valid}j.s.o.n()", + }) + + await t.rejects(mocks.getPublishedInfo, /JSON/) + } +) + +tap.test( + 'getPublishedInfo continues and returns null if the request returns null', + async t => { + const mocks = setupPublished({ + value: null, + }) + + const packageInfo = await mocks.getPublishedInfo() + t.match(packageInfo, null) + } +) + +tap.test('getPublishedInfo throws if it hits a non-404 error', async t => { + const mocks = setupPublished({ + error: new Error('code E418 - unexpected teapot'), + }) + + await t.rejects(mocks.getPublishedInfo, /teapot/) +}) + +tap.test( + 'getLocalInfo gets real name and stable properties of this package', + async t => { + const packageInfo = getLocalInfo() + // Check it works for real using real package.json properties that are stable + t.equal(packageInfo.name, 'optic-release-automation-action') + t.equal(packageInfo.license, 'MIT') + } +) + +tap.test('getLocalInfo gets data from stringified JSON from file', async t => { + const mocks = setupLocal() + const packageInfo = mocks.getLocalInfo() + t.match(packageInfo, mockPackageInfo) +}) diff --git a/test/provenance.test.js b/test/provenance.test.js index 12144c8b..2cd661a9 100644 --- a/test/provenance.test.js +++ b/test/provenance.test.js @@ -3,11 +3,13 @@ const tap = require('tap') const semver = require('semver') const sinon = require('sinon') +const proxyquire = require('proxyquire') const { + getProvenanceOptions, checkIsSupported, checkPermissions, - checkProvenanceViability, getNpmVersion, + // getAccessAdjustment needs proxyquire to mock internal package.json getter results } = require('../src/utils/provenance') const MINIMUM_VERSION = '9.5.0' @@ -73,12 +75,74 @@ tap.test('checkPermissions passes on minimum version with env', async t => { t.doesNotThrow(() => checkIsSupported(MINIMUM_VERSION)) }) +const setupAccessAdjustment = ({ local, published }) => { + const { getAccessAdjustment } = proxyquire('../src/utils/provenance', { + './packageInfo': { + getPublishedInfo: async () => published, + getLocalInfo: () => local, + }, + }) + return getAccessAdjustment +} +const unscopedPackageName = 'unscoped-fake-package' +const scopedPackageName = '@scoped/fake-package' + +tap.test( + 'getAccessAdjustment returns { access: public } if unscoped, unpublished and no access option', + async t => { + const getAccessAdjustment = setupAccessAdjustment({ + local: { name: unscopedPackageName }, + published: null, + }) + t.match(await getAccessAdjustment(), { access: 'public' }) + } +) + +tap.test( + 'getAccessAdjustment does nothing if passed defined access option', + async t => { + const getAccessAdjustment = setupAccessAdjustment({ + local: { name: unscopedPackageName }, + published: null, + }) + t.notOk(await getAccessAdjustment({ access: 'public' })) + } +) + tap.test( - 'checkProvenanceViability fails fast if NPM version unavailable', + 'getAccessAdjustment does nothing if package.json defines access', async t => { - t.throws( - () => checkProvenanceViability(), - 'Current npm version not provided' - ) + const getAccessAdjustment = setupAccessAdjustment({ + local: { + name: unscopedPackageName, + publishConfig: { access: 'public ' }, + }, + published: null, + }) + t.notOk(await getAccessAdjustment()) } ) + +tap.test( + 'getAccessAdjustment does nothing if package.json name is scoped', + async t => { + const getAccessAdjustment = setupAccessAdjustment({ + local: { name: scopedPackageName }, + published: null, + }) + t.notOk(await getAccessAdjustment()) + } +) + +tap.test('getAccessAdjustment does nothing if package is on npm', async t => { + const getAccessAdjustment = setupAccessAdjustment({ + local: { name: unscopedPackageName }, + published: { name: unscopedPackageName }, + }) + t.notOk(await getAccessAdjustment()) +}) + +tap.test( + 'getProvenanceOptions fails fast if NPM version unavailable', + async t => t.rejects(getProvenanceOptions, 'Current npm version not provided') +) diff --git a/test/publishToNpm.test.js b/test/publishToNpm.test.js index 9dab7e2d..9d0691e2 100644 --- a/test/publishToNpm.test.js +++ b/test/publishToNpm.test.js @@ -4,7 +4,11 @@ const tap = require('tap') const sinon = require('sinon') const proxyquire = require('proxyquire') -const setup = () => { +const setup = ({ + packageName = 'fakeTestPkg', + published = true, + mockPackageInfo, +} = {}) => { const execWithOutputStub = sinon.stub() execWithOutputStub .withArgs('curl', [ @@ -12,15 +16,24 @@ const setup = () => { 'https://optic-test.run.app/api/generate/optic-token', ]) .returns('otp123') - execWithOutputStub - .withArgs('npm', ['view', '--json']) - .returns('{"name":"fakeTestPkg"}') // npm behavior < v8.13.0 - execWithOutputStub.withArgs('npm', ['view', 'fakeTestPkg@v5.1.3']).returns('') + execWithOutputStub + .withArgs('npm', ['view', `${packageName}@v5.1.3`]) + .returns('') + + const getLocalInfo = () => ({ name: packageName }) + const getPublishedInfo = async () => + published ? { name: packageName } : null const publishToNpmProxy = proxyquire('../src/utils/publishToNpm', { './execWithOutput': { execWithOutput: execWithOutputStub }, + './packageInfo': mockPackageInfo + ? mockPackageInfo({ execWithOutputStub, getLocalInfo, getPublishedInfo }) + : { + getLocalInfo, + getPublishedInfo, + }, }) return { execWithOutputStub, publishToNpmProxy } @@ -47,22 +60,19 @@ tap.test('Should publish to npm with optic', async t => { ]) t.pass('npm config') - // We skip calls in these checks: - // - 1 used to get the package name - // - 2 used to check if the package version is already published - sinon.assert.calledWithExactly(execWithOutputStub.getCall(3), 'npm', [ + sinon.assert.calledWithExactly(execWithOutputStub, 'npm', [ 'pack', '--dry-run', ]) t.pass('npm pack called') - sinon.assert.calledWithExactly(execWithOutputStub.getCall(4), 'curl', [ + sinon.assert.calledWithExactly(execWithOutputStub, 'curl', [ '-s', 'https://optic-test.run.app/api/generate/optic-token', ]) t.pass('curl called') - sinon.assert.calledWithExactly(execWithOutputStub.getCall(5), 'npm', [ + sinon.assert.calledWithExactly(execWithOutputStub, 'npm', [ 'publish', '--otp', 'otp123', @@ -74,10 +84,10 @@ tap.test('Should publish to npm with optic', async t => { tap.test( "Should publish to npm when package hasn't been published before", - async () => { - const { publishToNpmProxy, execWithOutputStub } = setup() - - execWithOutputStub.withArgs('npm', ['view', '--json']).resolves('') + async t => { + const { publishToNpmProxy, execWithOutputStub } = setup({ + published: false, + }) await publishToNpmProxy.publishToNpm({ npmToken: 'a-token', @@ -90,15 +100,18 @@ tap.test( 'pack', '--dry-run', ]) + t.pass('npm pack called') + sinon.assert.calledWithExactly(execWithOutputStub, 'npm', [ 'publish', '--tag', 'latest', ]) + t.pass('npm publish called') } ) -tap.test('Should publish to npm without optic', async () => { +tap.test('Should publish to npm without optic', async t => { const { publishToNpmProxy, execWithOutputStub } = setup() await publishToNpmProxy.publishToNpm({ npmToken: 'a-token', @@ -111,16 +124,19 @@ tap.test('Should publish to npm without optic', async () => { 'pack', '--dry-run', ]) + t.pass('npm pack called') + sinon.assert.calledWithExactly(execWithOutputStub, 'npm', [ 'publish', '--tag', 'latest', ]) + t.pass('npm publish called') }) tap.test( 'Should skip npm package publication when it was already published', - async () => { + async t => { const { publishToNpmProxy, execWithOutputStub } = setup() execWithOutputStub @@ -141,18 +157,28 @@ tap.test( '--tag', 'latest', ]) + t.pass('publish never called with otp') + sinon.assert.neverCalledWith(execWithOutputStub, 'npm', [ 'publish', '--tag', 'latest', ]) + t.pass('publish never called') } ) tap.test('Should stop action if package info retrieval fails', async t => { t.plan(3) - const { publishToNpmProxy, execWithOutputStub } = setup() - + const { publishToNpmProxy, execWithOutputStub } = setup({ + // Use original getPublishedInfo logic with execWithOutputStub injected into it + mockPackageInfo: ({ getLocalInfo, execWithOutputStub }) => ({ + getLocalInfo, + getPublishedInfo: proxyquire('../src/utils/packageInfo', { + './execWithOutput': { execWithOutput: execWithOutputStub }, + }).getPublishedInfo, + }), + }) execWithOutputStub .withArgs('npm', ['view', '--json']) .throws(new Error('Network Error')) @@ -226,8 +252,16 @@ tap.test( tap.test( 'Should continue action if package info returns not found', - async () => { - const { publishToNpmProxy, execWithOutputStub } = setup() + async t => { + const { publishToNpmProxy, execWithOutputStub } = setup({ + // Use original getPublishedInfo logic with execWithOutputStub injected into it + mockPackageInfo: ({ getLocalInfo, execWithOutputStub }) => ({ + getLocalInfo, + getPublishedInfo: proxyquire('../src/utils/packageInfo', { + './execWithOutput': { execWithOutput: execWithOutputStub }, + }).getPublishedInfo, + }), + }) execWithOutputStub .withArgs('npm', ['view', '--json']) @@ -248,17 +282,20 @@ tap.test( 'pack', '--dry-run', ]) + t.pass('npm pack called') + sinon.assert.calledWithExactly(execWithOutputStub, 'npm', [ 'publish', '--tag', 'latest', ]) + t.pass('npm publish called') } ) tap.test( 'Should continue action if package version info returns not found', - async () => { + async t => { const { publishToNpmProxy, execWithOutputStub } = setup() execWithOutputStub @@ -276,11 +313,14 @@ tap.test( 'pack', '--dry-run', ]) + t.pass('npm pack called') + sinon.assert.calledWithExactly(execWithOutputStub, 'npm', [ 'publish', '--tag', 'latest', ]) + t.pass('npm publish called') } ) @@ -301,3 +341,22 @@ tap.test('Adds --provenance flag when provenance option provided', async () => { '--provenance', ]) }) + +tap.test('Adds --access flag if provided as an input', async () => { + const { publishToNpmProxy, execWithOutputStub } = setup() + await publishToNpmProxy.publishToNpm({ + npmToken: 'a-token', + opticUrl: 'https://optic-test.run.app/api/generate/', + npmTag: 'latest', + version: 'v5.1.3', + access: 'public', + }) + + sinon.assert.calledWithExactly(execWithOutputStub, 'npm', [ + 'publish', + '--tag', + 'latest', + '--access', + 'public', + ]) +}) diff --git a/test/release.test.js b/test/release.test.js index 6f893a08..25a6749a 100644 --- a/test/release.test.js +++ b/test/release.test.js @@ -66,7 +66,7 @@ const DEFAULT_ACTION_DATA = { /** * @param {{ npmVersion: string | undefined, env: Record | undefined }} [options] */ -function setup({ npmVersion, env } = {}) { +function setup({ npmVersion, env, isPublished = true, isScoped = true } = {}) { if (env) { // Add any test-specific environment variables. They get cleaned up by tap.afterEach(sinon.restore). Object.entries(env).forEach(([key, value]) => { @@ -92,6 +92,16 @@ function setup({ npmVersion, env } = {}) { const publishToNpmStub = sinon.stub(publishToNpmAction, 'publishToNpm') const notifyIssuesStub = sinon.stub(notifyIssuesAction, 'notifyIssues') + const packageName = isScoped ? '@some/package-name' : 'some-package-name' + const provenanceProxy = proxyquire('../src/utils/provenance', { + './packageInfo': { + getLocalInfo: () => ({ name: packageName }), + getPublishedInfo: async () => + isPublished ? { name: packageName } : null, + }, + }) + if (npmVersion) provenanceProxy.getNpmVersion = () => npmVersion + const callApiStub = sinon .stub(callApiAction, 'callApi') .resolves({ data: { body: 'test_body', html_url: 'test_url' } }) @@ -102,13 +112,10 @@ function setup({ npmVersion, env } = {}) { './utils/revertCommit': revertCommitStub, './utils/publishToNpm': publishToNpmStub, './utils/notifyIssues': notifyIssuesStub, + './utils/provenance': provenanceProxy, '@actions/core': coreStub, } - if (npmVersion) { - proxyStubs['./utils/provenance'] = { getNpmVersion: () => npmVersion } - } - const release = proxyquire('../src/release', proxyStubs) return { @@ -218,7 +225,7 @@ tap.test('Should publish to npm without optic', async () => { tap.test( 'Should publish with provenance if flag set and conditions met', - async () => { + async t => { const { release, stubs } = setup({ npmVersion: '9.5.0', // valid env: { ACTIONS_ID_TOKEN_REQUEST_URL: 'https://example.com' }, // valid @@ -233,12 +240,15 @@ tap.test( }) sinon.assert.notCalled(stubs.coreStub.setFailed) + t.pass('not failed') + sinon.assert.calledWithMatch(stubs.publishToNpmStub, { npmToken: 'a-token', opticUrl: 'https://optic-test.run.app/api/generate/', npmTag: 'latest', provenance: true, }) + t.pass('publish called') } ) @@ -277,12 +287,208 @@ tap.test('Aborts publish with provenance if missing permission', async () => { provenance: 'true', }, }) + sinon.assert.calledWithMatch( stubs.coreStub.setFailed, 'Provenance generation in GitHub Actions requires "write" access to the "id-token" permission' ) }) +tap.test('Should publish with --access public if flag set', async t => { + const { release, stubs } = setup() + await release({ + ...DEFAULT_ACTION_DATA, + inputs: { + 'app-name': APP_NAME, + 'npm-token': 'a-token', + access: 'public', + }, + }) + + sinon.assert.notCalled(stubs.coreStub.setFailed) + t.pass('did not set failed') + + sinon.assert.calledWithMatch(stubs.publishToNpmStub, { + npmToken: 'a-token', + opticUrl: 'https://optic-test.run.app/api/generate/', + npmTag: 'latest', + access: 'public', + }) + t.pass('called publishToNpm') +}) + +tap.test('Should publish with --access restricted if flag set', async t => { + const { release, stubs } = setup() + await release({ + ...DEFAULT_ACTION_DATA, + inputs: { + 'app-name': APP_NAME, + 'npm-token': 'a-token', + access: 'restricted', + }, + }) + + sinon.assert.notCalled(stubs.coreStub.setFailed) + t.pass('did not set failed') + + sinon.assert.calledWithMatch(stubs.publishToNpmStub, { + npmToken: 'a-token', + opticUrl: 'https://optic-test.run.app/api/generate/', + npmTag: 'latest', + access: 'restricted', + }) + t.pass('called publishToNpm') +}) + +tap.test('Should disallow unsupported --access flag', async () => { + const { release, stubs } = setup() + + const invalidString = + 'public; node -e "throw new Error(`arbitrary command executed`)"' + + await release({ + ...DEFAULT_ACTION_DATA, + inputs: { + 'app-name': APP_NAME, + 'npm-token': 'a-token', + access: invalidString, + }, + }) + + sinon.assert.calledWithMatch( + stubs.coreStub.setFailed, + `Invalid "access" option provided ("${invalidString}"), should be one of "public", "restricted"` + ) +}) + +tap.test( + 'Should publish with --access public and provenance if unscoped and unpublished', + async t => { + const { release, stubs } = setup({ + isScoped: false, + isPublished: false, + npmVersion: '9.5.0', // valid + env: { ACTIONS_ID_TOKEN_REQUEST_URL: 'https://example.com' }, // valid + }) + await release({ + ...DEFAULT_ACTION_DATA, + inputs: { + 'app-name': APP_NAME, + 'npm-token': 'a-token', + provenance: true, + }, + }) + + sinon.assert.notCalled(stubs.coreStub.setFailed) + t.pass('did not set failed') + + sinon.assert.calledWithMatch(stubs.publishToNpmStub, { + npmToken: 'a-token', + opticUrl: 'https://optic-test.run.app/api/generate/', + npmTag: 'latest', + access: 'public', + provenance: true, + }) + t.pass('called publishToNpm') + } +) + +tap.test( + 'Should not override access restricted with provenance while unscoped and unpublished', + async t => { + const { release, stubs } = setup({ + isScoped: false, + isPublished: false, + npmVersion: '9.5.0', // valid + env: { ACTIONS_ID_TOKEN_REQUEST_URL: 'https://example.com' }, // valid + }) + + await release({ + ...DEFAULT_ACTION_DATA, + inputs: { + 'app-name': APP_NAME, + 'npm-token': 'a-token', + provenance: true, + access: 'restricted', + }, + }) + + sinon.assert.notCalled(stubs.coreStub.setFailed) + t.pass('did not set failed') + + sinon.assert.calledWithMatch(stubs.publishToNpmStub, { + npmToken: 'a-token', + opticUrl: 'https://optic-test.run.app/api/generate/', + npmTag: 'latest', + access: 'restricted', + provenance: true, + }) + t.pass('called publishToNpm') + } +) + +tap.test( + 'Should publish with provenance and not add access when scoped and unpublished', + async t => { + const { release, stubs } = setup({ + isScoped: true, + isPublished: false, + npmVersion: '9.5.0', // valid + env: { ACTIONS_ID_TOKEN_REQUEST_URL: 'https://example.com' }, // valid + }) + await release({ + ...DEFAULT_ACTION_DATA, + inputs: { + 'app-name': APP_NAME, + 'npm-token': 'a-token', + provenance: true, + }, + }) + + sinon.assert.notCalled(stubs.coreStub.setFailed) + t.pass('did not set failed') + + sinon.assert.calledWithMatch(stubs.publishToNpmStub, { + npmToken: 'a-token', + opticUrl: 'https://optic-test.run.app/api/generate/', + npmTag: 'latest', + provenance: true, + }) + t.pass('called publishToNpm') + } +) + +tap.test( + 'Should publish with provenance and not add access when unscoped and published', + async t => { + const { release, stubs } = setup({ + isScoped: false, + isPublished: true, + npmVersion: '9.5.0', // valid + env: { ACTIONS_ID_TOKEN_REQUEST_URL: 'https://example.com' }, // valid + }) + await release({ + ...DEFAULT_ACTION_DATA, + inputs: { + 'app-name': APP_NAME, + 'npm-token': 'a-token', + provenance: true, + }, + }) + + sinon.assert.notCalled(stubs.coreStub.setFailed) + t.pass('did not set failed') + + sinon.assert.calledWithMatch(stubs.publishToNpmStub, { + npmToken: 'a-token', + opticUrl: 'https://optic-test.run.app/api/generate/', + npmTag: 'latest', + provenance: true, + }) + t.pass('called publishToNpm') + } +) + tap.test('Should not publish to npm if there is no npm token', async () => { const { release, stubs } = setup() stubs.callApiStub.throws()