Skip to content

Commit c9876d7

Browse files
yunseo-kimyunseo-kim
authored
fix: provenance-file takes precedence over OIDC auto-generated provenance (#9882)
## Why Needed When publishing with an externally generated provenance bundle under OIDC trusted publishing (`npm publish <tarball> --provenance-file=<bundle>`), 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 <git@yunseo.kim>
1 parent b016aa2 commit c9876d7

8 files changed

Lines changed: 326 additions & 8 deletions

File tree

‎lib/utils/oidc.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ async function oidc ({ packageName, registry, opts, config }) {
143143

144144
try {
145145
const isDefaultProvenance = config.isDefault('provenance')
146-
// CircleCI doesn't support provenance yet, so skip the auto-enable logic
147-
if (isDefaultProvenance && !ciInfo.CIRCLE) {
146+
// CircleCI doesn't support provenance yet, so skip the auto-enable logic.
147+
// An explicitly provided provenance file always takes precedence over auto-generated provenance
148+
if (isDefaultProvenance && !ciInfo.CIRCLE && !opts.provenanceFile) {
148149
const [headerB64, payloadB64] = idToken.split('.')
149150
if (headerB64 && payloadB64) {
150151
const payloadJson = Buffer.from(payloadB64, 'base64').toString('utf8')
@@ -158,7 +159,6 @@ async function oidc ({ packageName, registry, opts, config }) {
158159
if (visibility?.public) {
159160
log.verbose('oidc', `Enabling provenance`)
160161
opts.provenance = true
161-
config.set('provenance', true, 'user')
162162
}
163163
}
164164
}

‎tap-snapshots/test/lib/docs.js.test.cjs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1750,6 +1750,9 @@ Set to \`false\` to suppress the progress bar.
17501750
When publishing from a supported cloud CI/CD system, the package will be
17511751
publicly linked to where it was built and published from.
17521752
1753+
When the \`provenance-file\` config is set, it takes precedence and automatic
1754+
provenance generation (including via trusted publishing/OIDC) is skipped.
1755+
17531756
This config cannot be used with: \`provenance-file\`
17541757
17551758
#### \`provenance-file\`
@@ -1759,6 +1762,9 @@ This config cannot be used with: \`provenance-file\`
17591762
17601763
When publishing, the provenance bundle at the given path will be used.
17611764
1765+
This takes precedence over automatic provenance generation in trusted
1766+
publishing flows.
1767+
17621768
This config cannot be used with: \`provenance\`
17631769
17641770
#### \`proxy\`

‎test/fixtures/mock-oidc.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ const mockOidc = async (t, {
101101
ciInfo.CIRCLE = CIRCLE
102102
})
103103

104-
const { npm, registry, joinedOutput, logs } = await loadNpmWithRegistry(t, {
104+
const { npm, registry, joinedOutput, logs, prefix } = await loadNpmWithRegistry(t, {
105105
config: {
106106
loglevel: 'silly',
107107
...config,
@@ -117,11 +117,12 @@ const mockOidc = async (t, {
117117
})
118118

119119
if (mockGithubOidcOptions) {
120-
const { idToken, audience, statusCode = 200 } = mockGithubOidcOptions
120+
const { idToken, audience, statusCode = 200, times = 1 } = mockGithubOidcOptions
121121
const url = new URL(ACTIONS_ID_TOKEN_REQUEST_URL)
122122
nock(url.origin)
123123
.get(url.pathname)
124124
.query({ audience })
125+
.times(times)
125126
.matchHeader('authorization', `Bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}`)
126127
.matchHeader('accept', 'application/json')
127128
.reply(statusCode, statusCode !== 500 ? { value: idToken } : { message: 'Internal Server Error' })
@@ -160,7 +161,7 @@ const mockOidc = async (t, {
160161
})
161162
}
162163

163-
return { npm, joinedOutput, logs, ACTIONS_ID_TOKEN_REQUEST_URL }
164+
return { npm, registry, prefix, joinedOutput, logs, ACTIONS_ID_TOKEN_REQUEST_URL }
164165
}
165166

166167
const oidcPublishTest = (opts) => {

‎test/lib/commands/publish.js‎

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ const { loadNpmWithRegistry } = require('../../fixtures/mock-npm')
33
const { cleanZlib } = require('../../fixtures/clean-snapshot')
44
const pacote = require('pacote')
55
const Arborist = require('@npmcli/arborist')
6+
const npa = require('npm-package-arg')
7+
const ssri = require('ssri')
68
const path = require('node:path')
79
const fs = require('node:fs')
810
const { circleciIdToken, githubIdToken, gitlabIdToken, oidcPublishTest, mockOidc } = require('../../fixtures/mock-oidc')
@@ -1552,6 +1554,256 @@ t.test('oidc token exchange - provenance', (t) => {
15521554
},
15531555
}))
15541556

1557+
const provenanceFileSources = [
1558+
{
1559+
name: 'CLI config',
1560+
options: provenanceBundlePath => ({
1561+
config: {
1562+
'provenance-file': provenanceBundlePath,
1563+
},
1564+
}),
1565+
},
1566+
{
1567+
// exercises Publish.#getManifest() and its flatten(filteredPublishConfig, opts)
1568+
// path: publishConfig must reach opts.provenanceFile before oidc() decides
1569+
// whether to enable automatic provenance
1570+
name: 'publishConfig',
1571+
options: provenanceBundlePath => ({
1572+
packageJson: {
1573+
publishConfig: {
1574+
'provenance-file': provenanceBundlePath,
1575+
},
1576+
},
1577+
}),
1578+
},
1579+
]
1580+
1581+
for (const { name, options } of provenanceFileSources) {
1582+
t.test(`${name} provenance-file takes precedence over OIDC auto-provenance`, async t => {
1583+
const bundleDir = t.testdir()
1584+
const provenanceBundlePath = path.join(
1585+
bundleDir,
1586+
'provenance-bundle.json'
1587+
)
1588+
// holder so the libnpmpack mock can return the tarball computed below
1589+
const packMock = { tarballData: null }
1590+
1591+
const sourceOptions = options(provenanceBundlePath)
1592+
1593+
const { npm, registry, prefix, joinedOutput } = await mockOidc(t, {
1594+
oidcOptions: { github: true },
1595+
config: {
1596+
'//registry.npmjs.org/:_authToken': 'existing-fallback-token',
1597+
...sourceOptions.config,
1598+
},
1599+
packageJson: sourceOptions.packageJson,
1600+
mockGithubOidcOptions: {
1601+
audience: 'npm:registry.npmjs.org',
1602+
idToken: githubPublicIdToken,
1603+
},
1604+
mockOidcTokenExchangeOptions: {
1605+
idToken: githubPublicIdToken,
1606+
body: {
1607+
token: 'exchange-token',
1608+
},
1609+
},
1610+
publishOptions: {
1611+
token: 'exchange-token',
1612+
noPut: true,
1613+
},
1614+
load: {
1615+
mocks: {
1616+
libnpmaccess: {
1617+
getVisibility: async () => ({ public: true }),
1618+
},
1619+
// publish a deterministic tarball so the bundle subject digest can match it
1620+
libnpmpack: async () => packMock.tarballData,
1621+
// libnpmpublish must be mocked as a module so its internal require of
1622+
// sigstore is intercepted: a user-supplied bundle is only verified,
1623+
// generation (attest) must never run
1624+
libnpmpublish: t.mock('libnpmpublish', {
1625+
'libnpmpublish/lib/provenance': t.mock('libnpmpublish/lib/provenance', {
1626+
sigstore: {
1627+
verify: async () => {},
1628+
attest: async () => {
1629+
throw new Error('sigstore.attest must not be called when provenance-file is configured')
1630+
},
1631+
},
1632+
}),
1633+
}),
1634+
},
1635+
},
1636+
})
1637+
1638+
// compute the tarball integrity the same way libnpmpublish does so the
1639+
// provenance bundle subject matches the packed tarball
1640+
packMock.tarballData = await pacote.tarball(prefix, { Arborist })
1641+
const integrity = ssri.fromData(packMock.tarballData, { algorithms: ['sha512'] })
1642+
const spec = npa.resolve(pkg, '1.0.0')
1643+
const provenanceBundle = {
1644+
mediaType: 'application/vnd.dev.sigstore.bundle+json;version=0.2',
1645+
verificationMaterial: {
1646+
x509CertificateChain: {
1647+
certificates: [{ rawBytes: 'dGVzdA==' }],
1648+
},
1649+
tlogEntries: [],
1650+
},
1651+
dsseEnvelope: {
1652+
payload: Buffer.from(JSON.stringify({
1653+
_type: 'https://in-toto.io/Statement/v0.1',
1654+
subject: [
1655+
{
1656+
name: npa.toPurl(spec),
1657+
digest: { sha512: integrity.sha512[0].hexDigest() },
1658+
},
1659+
],
1660+
predicateType: 'https://slsa.dev/provenance/v0.2',
1661+
predicate: {},
1662+
})).toString('base64'),
1663+
payloadType: 'application/vnd.in-toto+json',
1664+
signatures: [{
1665+
/* eslint-disable-next-line max-len */
1666+
sig: 'MEUCIQDqHtpkk1d0rMGLmf3qet9jLale3KVn8Pnywpwt7ln+9AIgG9CJvvUmyemhNYHz0DfJ4vMfKk1TMg+m3hR0mISXJos=',
1667+
keyid: '',
1668+
}],
1669+
},
1670+
}
1671+
fs.writeFileSync(provenanceBundlePath, JSON.stringify(provenanceBundle, null, 2))
1672+
1673+
let publishedBody
1674+
registry.nock
1675+
.put(`/${spec.escapedName}`, (body) => {
1676+
publishedBody = body
1677+
return true
1678+
})
1679+
.matchHeader('authorization', 'Bearer exchange-token')
1680+
// optional so a failed publish does not leave a pending mock behind
1681+
.optionally()
1682+
.reply(200, {})
1683+
1684+
// libnpmpublish checks package visibility itself before generating
1685+
// provenance; optional so it is only consumed if generation is attempted
1686+
registry.nock
1687+
.get(`/-/package/${spec.escapedName}/visibility`)
1688+
.optionally()
1689+
.reply(200, { public: true })
1690+
1691+
await npm.exec('publish', [])
1692+
1693+
t.match(joinedOutput(), '+ @npmcli/test-package@1.0.0')
1694+
1695+
const attachment =
1696+
publishedBody?._attachments[`${pkg}-1.0.0.sigstore`]
1697+
1698+
t.ok(attachment, 'published packument includes supplied provenance')
1699+
t.strictSame(
1700+
JSON.parse(attachment.data),
1701+
provenanceBundle,
1702+
'published sigstore bundle is the user-supplied provenance file'
1703+
)
1704+
})
1705+
}
1706+
1707+
t.test('automatic provenance does not leak between workspace publishes', async t => {
1708+
const provenanceBundlePath = path.join(t.testdir(), 'provenance-bundle.json')
1709+
const autoPackage = 'workspace-auto-provenance'
1710+
const filePackage = 'workspace-file-provenance'
1711+
const publishCalls = []
1712+
const prefixDir = {
1713+
'package.json': JSON.stringify({
1714+
name: 'workspace-root',
1715+
version: '1.0.0',
1716+
workspaces: [autoPackage, filePackage],
1717+
}),
1718+
[autoPackage]: {
1719+
'package.json': JSON.stringify({
1720+
name: autoPackage,
1721+
version: '1.0.0',
1722+
}),
1723+
},
1724+
[filePackage]: {
1725+
'package.json': JSON.stringify({
1726+
name: filePackage,
1727+
version: '1.0.0',
1728+
publishConfig: {
1729+
'provenance-file': provenanceBundlePath,
1730+
},
1731+
}),
1732+
},
1733+
}
1734+
1735+
const { npm, registry } = await mockOidc(t, {
1736+
oidcOptions: { github: true },
1737+
packageName: autoPackage,
1738+
config: {
1739+
'//registry.npmjs.org/:_authToken': 'existing-fallback-token',
1740+
workspaces: true,
1741+
},
1742+
mockGithubOidcOptions: {
1743+
audience: 'npm:registry.npmjs.org',
1744+
idToken: githubPublicIdToken,
1745+
times: 2,
1746+
},
1747+
mockOidcTokenExchangeOptions: {
1748+
idToken: githubPublicIdToken,
1749+
body: {
1750+
token: 'exchange-token',
1751+
},
1752+
},
1753+
publishOptions: {
1754+
noPut: true,
1755+
},
1756+
load: {
1757+
prefixDir,
1758+
mocks: {
1759+
libnpmaccess: {
1760+
getVisibility: async () => ({ public: true }),
1761+
},
1762+
// mocked as a plain module so the publish options each workspace
1763+
// receives can be recorded verbatim
1764+
libnpmpublish: {
1765+
publish: async (manifest, _tarballData, opts) => {
1766+
publishCalls.push({
1767+
name: manifest.name,
1768+
provenance: opts.provenance,
1769+
provenanceFile: opts.provenanceFile,
1770+
})
1771+
},
1772+
},
1773+
},
1774+
},
1775+
})
1776+
1777+
registry.mockOidcTokenExchange({
1778+
packageName: filePackage,
1779+
idToken: githubPublicIdToken,
1780+
body: {
1781+
token: 'exchange-token',
1782+
},
1783+
})
1784+
registry.publish(filePackage, { noPut: true })
1785+
1786+
await npm.exec('publish', [])
1787+
1788+
t.strictSame(publishCalls, [
1789+
{
1790+
name: autoPackage,
1791+
provenance: true,
1792+
provenanceFile: null,
1793+
},
1794+
{
1795+
name: filePackage,
1796+
provenance: false,
1797+
provenanceFile: provenanceBundlePath,
1798+
},
1799+
])
1800+
t.equal(
1801+
npm.config.isDefault('provenance'),
1802+
true,
1803+
'automatic provenance does not mutate shared config'
1804+
)
1805+
})
1806+
15551807
const brokenJwts = [
15561808
'x.invalid-jwt.x',
15571809
'x.invalid-jwt.',

‎workspaces/config/lib/definitions/definitions.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,6 +2007,10 @@ const definitions = {
20072007
description: `
20082008
When publishing from a supported cloud CI/CD system, the package will be
20092009
publicly linked to where it was built and published from.
2010+
2011+
When the \`provenance-file\` config is set, it takes precedence and
2012+
automatic provenance generation (including via trusted publishing/OIDC)
2013+
is skipped.
20102014
`,
20112015
flatten,
20122016
}),
@@ -2017,6 +2021,9 @@ const definitions = {
20172021
exclusive: ['provenance'],
20182022
description: `
20192023
When publishing, the provenance bundle at the given path will be used.
2024+
2025+
This takes precedence over automatic provenance generation in trusted
2026+
publishing flows.
20202027
`,
20212028
flatten,
20222029
}),

‎workspaces/libnpmpublish/README.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,15 @@ A couple of options of note:
5858

5959
* `opts.provenance` - when running in a supported CI environment, will trigger
6060
the generation of a signed provenance statement to be published alongside
61-
the package. Mutually exclusive with the `provenanceFile` option.
61+
the package. Mutually exclusive with the `provenanceFile` option; providing
62+
both will throw an `EUSAGE` error. In the npm CLI's trusted
63+
publishing flows, automatic provenance generation is skipped when
64+
`provenanceFile` is supplied.
6265

6366
* `opts.provenanceFile` - specifies the path to an externally-generated
6467
provenance statement to be published alongside the package. Mutually
65-
exclusive with the `provenance` option. The specified file should be a
68+
exclusive with the `provenance` option; providing both will throw an
69+
`EUSAGE` error. The specified file should be a
6670
[Sigstore Bundle](https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_bundle.proto)
6771
containing a [DSSE](https://github.com/secure-systems-lab/dsse)-packaged
6872
provenance statement.

‎workspaces/libnpmpublish/lib/publish.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,12 @@ const buildMetadata = async (registry, manifest, tarballData, spec, opts) => {
144144

145145
// Handle case where --provenance flag was set to true
146146
let transparencyLogUrl
147+
if (provenance === true && provenanceFile) {
148+
throw Object.assign(
149+
new Error('provenance and provenanceFile cannot be used together'),
150+
{ code: 'EUSAGE' }
151+
)
152+
}
147153
if (provenance === true || provenanceFile) {
148154
let provenanceBundle
149155
const subject = {

0 commit comments

Comments
 (0)