From bcf1c7981882f3f17f18ecfc550965b8da51d61c Mon Sep 17 00:00:00 2001 From: Yunseo Kim Date: Thu, 3 Sep 2026 17:33:50 +0000 Subject: [PATCH] fix: provenance-file takes precedence over OIDC auto-generated provenance (#9882) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Needed When publishing with an externally generated provenance bundle under OIDC trusted publishing (`npm publish --provenance-file=`), npm silently discarded the supplied bundle and published its own auto-generated provenance instead. Three layers interacted: 1. `lib/utils/oidc.js` auto-enabled provenance (`opts.provenance = true`) whenever the `provenance` config was at its default, without checking whether a `provenance-file` was supplied. 2. In `libnpmpublish`'s `buildMetadata()`, the inner `provenance === true` branch then ran `generateProvenance()`, so the `verifyProvenance(subject, provenanceFile)` branch, the only code path that reads the supplied file, never executed. 3. Every documented way to disable automatic provenance was unusable in combination with `--provenance-file` (config-layer mutual exclusivity error, env carve-out, or publishConfig flatten timing). ## What Changes - **`lib/utils/oidc.js`**: skip auto-enabling provenance when a provenance file is configured (`opts.provenanceFile`). `opts` already carries `provenanceFile` from every config source (CLI/env/npmrc/publishConfig) by the time the OIDC flow runs, so this covers all entry paths. The supplied bundle is now verified via `verifyProvenance()` and published, as documented. Automatic provenance is also no longer written to the shared config; it is set only on the current publish's `opts` (which reaches libnpmpublish via `otplease`). Previously the `user`-scoped config write leaked `provenance: true` into later workspace publishes during `npm publish --workspaces`, where a workspace with `publishConfig["provenance-file"]` would then hit the conflict check below. - **`libnpmpublish`**: `buildMetadata()` now throws ~~`EPROVENANCECONFLICT`~~ `EUSAGE` (updated per review) when both `provenance: true` and `provenanceFile` are provided. - **Docs**: config descriptions for `provenance` / `provenance-file` and the libnpmpublish README now state the precedence rule. ### ⚠️ ~~New error code (feedback requested)~~ Resolved: reuses `EUSAGE` ~~This PR introduces `EPROVENANCECONFLICT` in libnpmpublish, thrown when both `provenance: true` and `provenanceFile` are provided programmatically.~~ ~~**Rationale:** the README already documents the two as mutually exclusive, and silently preferring either direction discards a cryptographically meaningful artifact. Note the CLI's config layer reports the same conflict as a `TypeError` without an error code (pre-existing). Happy to align on `EUSAGE` or another convention per review.~~ **Update:** per review, the conflict reuses the existing `EUSAGE` code instead of introducing a new one (`Object.assign(new Error('provenance and provenanceFile cannot be used together'), { code: 'EUSAGE' })`), and the README documents `EUSAGE` accordingly. ## Testing - New CLI regression test: OIDC trusted publishing + `provenance-file` ~~config~~ asserts the published packument's sigstore attachment deep-equals the supplied bundle (and that sigstore generation is never invoked). **Update:** parameterized per review to cover both `provenance-file` sources, CLI config and `publishConfig`; the latter proves `publishConfig["provenance-file"]` is flattened into `opts.provenanceFile` via `Publish.#getManifest()` before `oidc()` decides whether to enable automatic provenance. - New libnpmpublish test: both options set → rejects with ~~`EPROVENANCECONFLICT`~~ `EUSAGE`, no registry PUT, generation not invoked. - New workspace regression test (per review): publishes two public workspaces in order (one where OIDC auto-enables provenance, then one with `publishConfig["provenance-file"]`), and asserts each publish receives exactly its own options (`{ provenance: true, provenanceFile: null }` then `{ provenance: false, provenanceFile }`) and that the shared config stays at its default (`npm.config.isDefault('provenance') === true`). Verified to fail on the pre-fix code for exactly the leak reason, and to pass after the fix. `mock-oidc` gained a `times` option on the GitHub id-token mock to serve one token request per workspace. - Full root suite green with 100% coverage; libnpmpublish workspace suite green; lint clean. ## References Fixes #9879 ## Out of scope (noted for follow-up) - `publishConfig.provenance: false` does not block the OIDC auto-enable (publishConfig flattens into `opts` only, so `config.isDefault('provenance')` stays true). A separate behavioral question about `isDefault` semantics. - Hardening `config.set` itself against bypassing load-time exclusivity is an `@npmcli/config` semver-major conversation; this flow no longer writes `provenance` to the config at all. --------- Signed-off-by: Yunseo Kim (cherry picked from commit c9876d7ea7150b0702e4151210b9fa1a8dbc7fbf) --- lib/utils/oidc.js | 6 +- tap-snapshots/test/lib/docs.js.test.cjs | 6 + test/fixtures/mock-oidc.js | 7 +- test/lib/commands/publish.js | 252 ++++++++++++++++++ .../config/lib/definitions/definitions.js | 7 + workspaces/libnpmpublish/README.md | 8 +- workspaces/libnpmpublish/lib/publish.js | 6 + workspaces/libnpmpublish/test/publish.js | 42 +++ 8 files changed, 326 insertions(+), 8 deletions(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index 00f32c642621c..203aaf3143a77 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -143,8 +143,9 @@ async function oidc ({ packageName, registry, opts, config }) { try { const isDefaultProvenance = config.isDefault('provenance') - // CircleCI doesn't support provenance yet, so skip the auto-enable logic - if (isDefaultProvenance && !ciInfo.CIRCLE) { + // CircleCI doesn't support provenance yet, so skip the auto-enable logic. + // An explicitly provided provenance file always takes precedence over auto-generated provenance + if (isDefaultProvenance && !ciInfo.CIRCLE && !opts.provenanceFile) { const [headerB64, payloadB64] = idToken.split('.') if (headerB64 && payloadB64) { const payloadJson = Buffer.from(payloadB64, 'base64').toString('utf8') @@ -158,7 +159,6 @@ async function oidc ({ packageName, registry, opts, config }) { if (visibility?.public) { log.verbose('oidc', `Enabling provenance`) opts.provenance = true - config.set('provenance', true, 'user') } } } diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 29c522dddec60..103d007acb8da 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -1631,6 +1631,9 @@ Set to \`false\` to suppress the progress bar. When publishing from a supported cloud CI/CD system, the package will be publicly linked to where it was built and published from. +When the \`provenance-file\` config is set, it takes precedence and automatic +provenance generation (including via trusted publishing/OIDC) is skipped. + This config cannot be used with: \`provenance-file\` #### \`provenance-file\` @@ -1640,6 +1643,9 @@ This config cannot be used with: \`provenance-file\` When publishing, the provenance bundle at the given path will be used. +This takes precedence over automatic provenance generation in trusted +publishing flows. + This config cannot be used with: \`provenance\` #### \`proxy\` diff --git a/test/fixtures/mock-oidc.js b/test/fixtures/mock-oidc.js index d15d52c1b819f..e2e11b27224b6 100644 --- a/test/fixtures/mock-oidc.js +++ b/test/fixtures/mock-oidc.js @@ -101,7 +101,7 @@ const mockOidc = async (t, { ciInfo.CIRCLE = CIRCLE }) - const { npm, registry, joinedOutput, logs } = await loadNpmWithRegistry(t, { + const { npm, registry, joinedOutput, logs, prefix } = await loadNpmWithRegistry(t, { config: { loglevel: 'silly', ...config, @@ -117,11 +117,12 @@ const mockOidc = async (t, { }) if (mockGithubOidcOptions) { - const { idToken, audience, statusCode = 200 } = mockGithubOidcOptions + const { idToken, audience, statusCode = 200, times = 1 } = mockGithubOidcOptions const url = new URL(ACTIONS_ID_TOKEN_REQUEST_URL) nock(url.origin) .get(url.pathname) .query({ audience }) + .times(times) .matchHeader('authorization', `Bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}`) .matchHeader('accept', 'application/json') .reply(statusCode, statusCode !== 500 ? { value: idToken } : { message: 'Internal Server Error' }) @@ -160,7 +161,7 @@ const mockOidc = async (t, { }) } - return { npm, joinedOutput, logs, ACTIONS_ID_TOKEN_REQUEST_URL } + return { npm, registry, prefix, joinedOutput, logs, ACTIONS_ID_TOKEN_REQUEST_URL } } const oidcPublishTest = (opts) => { diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 98576b08ea300..8b20c5671aad1 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -3,6 +3,8 @@ const { loadNpmWithRegistry } = require('../../fixtures/mock-npm') const { cleanZlib } = require('../../fixtures/clean-snapshot') const pacote = require('pacote') const Arborist = require('@npmcli/arborist') +const npa = require('npm-package-arg') +const ssri = require('ssri') const path = require('node:path') const fs = require('node:fs') const { circleciIdToken, githubIdToken, gitlabIdToken, oidcPublishTest, mockOidc } = require('../../fixtures/mock-oidc') @@ -1495,6 +1497,256 @@ t.test('oidc token exchange - provenance', (t) => { }, })) + const provenanceFileSources = [ + { + name: 'CLI config', + options: provenanceBundlePath => ({ + config: { + 'provenance-file': provenanceBundlePath, + }, + }), + }, + { + // exercises Publish.#getManifest() and its flatten(filteredPublishConfig, opts) + // path: publishConfig must reach opts.provenanceFile before oidc() decides + // whether to enable automatic provenance + name: 'publishConfig', + options: provenanceBundlePath => ({ + packageJson: { + publishConfig: { + 'provenance-file': provenanceBundlePath, + }, + }, + }), + }, + ] + + for (const { name, options } of provenanceFileSources) { + t.test(`${name} provenance-file takes precedence over OIDC auto-provenance`, async t => { + const bundleDir = t.testdir() + const provenanceBundlePath = path.join( + bundleDir, + 'provenance-bundle.json' + ) + // holder so the libnpmpack mock can return the tarball computed below + const packMock = { tarballData: null } + + const sourceOptions = options(provenanceBundlePath) + + const { npm, registry, prefix, joinedOutput } = await mockOidc(t, { + oidcOptions: { github: true }, + config: { + '//registry.npmjs.org/:_authToken': 'existing-fallback-token', + ...sourceOptions.config, + }, + packageJson: sourceOptions.packageJson, + mockGithubOidcOptions: { + audience: 'npm:registry.npmjs.org', + idToken: githubPublicIdToken, + }, + mockOidcTokenExchangeOptions: { + idToken: githubPublicIdToken, + body: { + token: 'exchange-token', + }, + }, + publishOptions: { + token: 'exchange-token', + noPut: true, + }, + load: { + mocks: { + libnpmaccess: { + getVisibility: async () => ({ public: true }), + }, + // publish a deterministic tarball so the bundle subject digest can match it + libnpmpack: async () => packMock.tarballData, + // libnpmpublish must be mocked as a module so its internal require of + // sigstore is intercepted: a user-supplied bundle is only verified, + // generation (attest) must never run + libnpmpublish: t.mock('libnpmpublish', { + 'libnpmpublish/lib/provenance': t.mock('libnpmpublish/lib/provenance', { + sigstore: { + verify: async () => {}, + attest: async () => { + throw new Error('sigstore.attest must not be called when provenance-file is configured') + }, + }, + }), + }), + }, + }, + }) + + // compute the tarball integrity the same way libnpmpublish does so the + // provenance bundle subject matches the packed tarball + packMock.tarballData = await pacote.tarball(prefix, { Arborist }) + const integrity = ssri.fromData(packMock.tarballData, { algorithms: ['sha512'] }) + const spec = npa.resolve(pkg, '1.0.0') + const provenanceBundle = { + mediaType: 'application/vnd.dev.sigstore.bundle+json;version=0.2', + verificationMaterial: { + x509CertificateChain: { + certificates: [{ rawBytes: 'dGVzdA==' }], + }, + tlogEntries: [], + }, + dsseEnvelope: { + payload: Buffer.from(JSON.stringify({ + _type: 'https://in-toto.io/Statement/v0.1', + subject: [ + { + name: npa.toPurl(spec), + digest: { sha512: integrity.sha512[0].hexDigest() }, + }, + ], + predicateType: 'https://slsa.dev/provenance/v0.2', + predicate: {}, + })).toString('base64'), + payloadType: 'application/vnd.in-toto+json', + signatures: [{ + /* eslint-disable-next-line max-len */ + sig: 'MEUCIQDqHtpkk1d0rMGLmf3qet9jLale3KVn8Pnywpwt7ln+9AIgG9CJvvUmyemhNYHz0DfJ4vMfKk1TMg+m3hR0mISXJos=', + keyid: '', + }], + }, + } + fs.writeFileSync(provenanceBundlePath, JSON.stringify(provenanceBundle, null, 2)) + + let publishedBody + registry.nock + .put(`/${spec.escapedName}`, (body) => { + publishedBody = body + return true + }) + .matchHeader('authorization', 'Bearer exchange-token') + // optional so a failed publish does not leave a pending mock behind + .optionally() + .reply(200, {}) + + // libnpmpublish checks package visibility itself before generating + // provenance; optional so it is only consumed if generation is attempted + registry.nock + .get(`/-/package/${spec.escapedName}/visibility`) + .optionally() + .reply(200, { public: true }) + + await npm.exec('publish', []) + + t.match(joinedOutput(), '+ @npmcli/test-package@1.0.0') + + const attachment = + publishedBody?._attachments[`${pkg}-1.0.0.sigstore`] + + t.ok(attachment, 'published packument includes supplied provenance') + t.strictSame( + JSON.parse(attachment.data), + provenanceBundle, + 'published sigstore bundle is the user-supplied provenance file' + ) + }) + } + + t.test('automatic provenance does not leak between workspace publishes', async t => { + const provenanceBundlePath = path.join(t.testdir(), 'provenance-bundle.json') + const autoPackage = 'workspace-auto-provenance' + const filePackage = 'workspace-file-provenance' + const publishCalls = [] + const prefixDir = { + 'package.json': JSON.stringify({ + name: 'workspace-root', + version: '1.0.0', + workspaces: [autoPackage, filePackage], + }), + [autoPackage]: { + 'package.json': JSON.stringify({ + name: autoPackage, + version: '1.0.0', + }), + }, + [filePackage]: { + 'package.json': JSON.stringify({ + name: filePackage, + version: '1.0.0', + publishConfig: { + 'provenance-file': provenanceBundlePath, + }, + }), + }, + } + + const { npm, registry } = await mockOidc(t, { + oidcOptions: { github: true }, + packageName: autoPackage, + config: { + '//registry.npmjs.org/:_authToken': 'existing-fallback-token', + workspaces: true, + }, + mockGithubOidcOptions: { + audience: 'npm:registry.npmjs.org', + idToken: githubPublicIdToken, + times: 2, + }, + mockOidcTokenExchangeOptions: { + idToken: githubPublicIdToken, + body: { + token: 'exchange-token', + }, + }, + publishOptions: { + noPut: true, + }, + load: { + prefixDir, + mocks: { + libnpmaccess: { + getVisibility: async () => ({ public: true }), + }, + // mocked as a plain module so the publish options each workspace + // receives can be recorded verbatim + libnpmpublish: { + publish: async (manifest, _tarballData, opts) => { + publishCalls.push({ + name: manifest.name, + provenance: opts.provenance, + provenanceFile: opts.provenanceFile, + }) + }, + }, + }, + }, + }) + + registry.mockOidcTokenExchange({ + packageName: filePackage, + idToken: githubPublicIdToken, + body: { + token: 'exchange-token', + }, + }) + registry.publish(filePackage, { noPut: true }) + + await npm.exec('publish', []) + + t.strictSame(publishCalls, [ + { + name: autoPackage, + provenance: true, + provenanceFile: null, + }, + { + name: filePackage, + provenance: false, + provenanceFile: provenanceBundlePath, + }, + ]) + t.equal( + npm.config.isDefault('provenance'), + true, + 'automatic provenance does not mutate shared config' + ) + }) + const brokenJwts = [ 'x.invalid-jwt.x', 'x.invalid-jwt.', diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index 4d07ff67ab040..1d78844e6ab5e 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -1881,6 +1881,10 @@ const definitions = { description: ` When publishing from a supported cloud CI/CD system, the package will be publicly linked to where it was built and published from. + + When the \`provenance-file\` config is set, it takes precedence and + automatic provenance generation (including via trusted publishing/OIDC) + is skipped. `, flatten, }), @@ -1891,6 +1895,9 @@ const definitions = { exclusive: ['provenance'], description: ` When publishing, the provenance bundle at the given path will be used. + + This takes precedence over automatic provenance generation in trusted + publishing flows. `, flatten, }), diff --git a/workspaces/libnpmpublish/README.md b/workspaces/libnpmpublish/README.md index 4daac34feaad1..3abc096f6d88d 100644 --- a/workspaces/libnpmpublish/README.md +++ b/workspaces/libnpmpublish/README.md @@ -53,11 +53,15 @@ A couple of options of note: * `opts.provenance` - when running in a supported CI environment, will trigger the generation of a signed provenance statement to be published alongside - the package. Mutually exclusive with the `provenanceFile` option. + the package. Mutually exclusive with the `provenanceFile` option; providing + both will throw an `EUSAGE` error. In the npm CLI's trusted + publishing flows, automatic provenance generation is skipped when + `provenanceFile` is supplied. * `opts.provenanceFile` - specifies the path to an externally-generated provenance statement to be published alongside the package. Mutually - exclusive with the `provenance` option. The specified file should be a + exclusive with the `provenance` option; providing both will throw an + `EUSAGE` error. The specified file should be a [Sigstore Bundle](https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_bundle.proto) containing a [DSSE](https://github.com/secure-systems-lab/dsse)-packaged provenance statement. diff --git a/workspaces/libnpmpublish/lib/publish.js b/workspaces/libnpmpublish/lib/publish.js index cfe85d2d29f57..2cfd543572958 100644 --- a/workspaces/libnpmpublish/lib/publish.js +++ b/workspaces/libnpmpublish/lib/publish.js @@ -134,6 +134,12 @@ const buildMetadata = async (registry, manifest, tarballData, spec, opts) => { // Handle case where --provenance flag was set to true let transparencyLogUrl + if (provenance === true && provenanceFile) { + throw Object.assign( + new Error('provenance and provenanceFile cannot be used together'), + { code: 'EUSAGE' } + ) + } if (provenance === true || provenanceFile) { let provenanceBundle const subject = { diff --git a/workspaces/libnpmpublish/test/publish.js b/workspaces/libnpmpublish/test/publish.js index 0a38f73f59f12..42f608c491b14 100644 --- a/workspaces/libnpmpublish/test/publish.js +++ b/workspaces/libnpmpublish/test/publish.js @@ -790,6 +790,48 @@ t.test('user-supplied provenance - success', async t => { t.ok(ret, 'publish succeeded') }) +t.test('provenance and provenanceFile together throws', async t => { + mockGlobals(t, { + 'process.env': { + CI: true, + GITHUB_ACTIONS: true, + ACTIONS_ID_TOKEN_REQUEST_URL: 'https://mock.oidc', + ACTIONS_ID_TOKEN_REQUEST_TOKEN: 'decafbad', + }, + }) + + const { publish } = t.mock('..', { + 'ci-info': { GITHUB_ACTIONS: true, name: 'GitHub Actions' }, + '../lib/provenance': { + generateProvenance: () => { + throw new Error('generateProvenance should not be called') + }, + verifyProvenance: () => { + throw new Error('verifyProvenance should not be called') + }, + }, + }) + + const manifest = { + name: '@npmcli/libnpmpublish-test', + version: '1.0.0', + description: 'test libnpmpublish package', + } + + await t.rejects( + publish(manifest, tarData, { + ...opts, + access: 'public', + provenance: true, + provenanceFile: './test/fixtures/valid-bundle.json', + }), + { + code: 'EUSAGE', + message: /cannot be used together/, + } + ) +}) + t.test('user-supplied provenance - failure', async t => { const { publish } = t.mock('..') const manifest = {