From 3c263ef8af9c70f840ca20e4521caaa125fd8ba7 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 13:24:13 -0400 Subject: [PATCH 01/12] fix: change to the oidc flow for more granualr control over log levels --- lib/utils/oidc.js | 54 ++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index fd88cc844040f..b7cb5f60f5bc6 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -30,6 +30,7 @@ async function oidc ({ packageName, registry, opts, config }) { /** @see https://github.com/watson/ci-info/blob/v4.2.0/vendors.json#L161C13-L161C22 */ ciInfo.GITLAB )) { + log.silly('oidc', 'Not running OIDC, not in a supported CI environment') return undefined } @@ -67,14 +68,11 @@ async function oidc ({ packageName, registry, opts, config }) { process.env.ACTIONS_ID_TOKEN_REQUEST_URL && process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN ) { - log.silly('oidc', '"GITHUB_ACTIONS" detected with "ACTIONS_ID_" envs, fetching id_token') - /** * The specification for an audience is `npm:registry.npmjs.org`, * where "registry.npmjs.org" can be any supported registry. */ const audience = `npm:${new URL(registry).hostname}` - log.silly('oidc', `Using audience: ${audience}`) const url = new URL(process.env.ACTIONS_ID_TOKEN_REQUEST_URL) url.searchParams.append('audience', audience) const startTime = Date.now() @@ -96,17 +94,19 @@ async function oidc ({ packageName, registry, opts, config }) { const json = await response.json() if (!response.ok) { - throw new Error(`Failed to fetch id_token from GitHub: received an invalid response`) + log.silly('oidc', `Failed to fetch id_token from GitHub: received an invalid response`) + return undefined } if (!json.value) { - throw new Error(`Failed to fetch id_token from GitHub: missing value`) + log.silly('oidc', `Failed to fetch id_token from GitHub: missing value`) + return undefined } - log.silly('oidc', 'GITHUB_ACTIONS valid fetch response for id_token') idToken = json.value } else { - throw new Error('GITHUB_ACTIONS detected. If you intend to publish using OIDC, please set workflow permissions for `id-token: write`') + log.silly('oidc', 'GITHUB_ACTIONS detected. If you intend to publish using OIDC, please set workflow permissions for `id-token: write`') + return undefined } } } @@ -130,22 +130,31 @@ async function oidc ({ packageName, registry, opts, config }) { } const escapedPackageName = npa(packageName).escapedName - const response = await npmFetch.json(new URL(`/-/npm/v1/oidc/token/exchange/package/${escapedPackageName}`, registry), { - ...{ - ...opts, - [authTokenKey]: idToken, // Use the idToken as the auth token for the request - }, - method: 'POST', - headers: { - ...opts.headers, - 'Content-Type': 'application/json', - // this will not work because the existing auth token will replace it. - // authorization: `Bearer ${idToken}`, - }, - }) + let response + try { + response = await npmFetch.json(new URL(`/-/npm/v1/oidc/token/exchange/package/${escapedPackageName}`, registry), { + ...{ + ...opts, + [authTokenKey]: idToken, // Use the idToken as the auth token for the request + }, + method: 'POST', + headers: { + ...opts.headers, + 'Content-Type': 'application/json', + // this will not work because the existing auth token will replace it. + // authorization: `Bearer ${idToken}`, + }, + }) + } catch (error) { + if (error?.body?.message) { + log.verbose('oidc', `Registry body response error message "${error.body.message}"`) + } + return undefined + } if (!response?.token) { - throw new Error('OIDC token exchange failure: missing token in response body') + log.silly('oidc', 'OIDC token exchange failure: missing token in response body') + return undefined } /* * The "opts" object is a clone of npm.flatOptions and is passed through the `publish` command, @@ -158,9 +167,6 @@ async function oidc ({ packageName, registry, opts, config }) { log.silly('oidc', `OIDC token successfully retrieved`) } catch (error) { log.verbose('oidc', error.message) - if (error?.body?.message) { - log.verbose('oidc', `Registry body response error message "${error.body.message}"`) - } } return undefined } From 621c61290927d3d5d408c0307615d3c0a96f0a2b Mon Sep 17 00:00:00 2001 From: Reggi Date: Fri, 27 Jun 2025 13:25:08 -0400 Subject: [PATCH 02/12] change to verbose 1 Co-authored-by: Chris Sidi --- lib/utils/oidc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index b7cb5f60f5bc6..c1bcd5d3eb7f3 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -94,7 +94,7 @@ async function oidc ({ packageName, registry, opts, config }) { const json = await response.json() if (!response.ok) { - log.silly('oidc', `Failed to fetch id_token from GitHub: received an invalid response`) + log.verbose('oidc', `Failed to fetch id_token from GitHub: received an invalid response`) return undefined } From f79b0a79f75cdb1a8a6f53a0cdd5d2e95ebf0e2a Mon Sep 17 00:00:00 2001 From: Reggi Date: Fri, 27 Jun 2025 13:25:18 -0400 Subject: [PATCH 03/12] change to verbose 2 Co-authored-by: Chris Sidi --- lib/utils/oidc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index c1bcd5d3eb7f3..4e34c763d2b9a 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -99,7 +99,7 @@ async function oidc ({ packageName, registry, opts, config }) { } if (!json.value) { - log.silly('oidc', `Failed to fetch id_token from GitHub: missing value`) + log.verbose('oidc', `Failed to fetch id_token from GitHub: missing value`) return undefined } From beaf3b75aad5f7d042dff8bc7773a0ce522030e8 Mon Sep 17 00:00:00 2001 From: Reggi Date: Fri, 27 Jun 2025 13:25:36 -0400 Subject: [PATCH 04/12] change to verbose 3 Co-authored-by: Chris Sidi --- lib/utils/oidc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index 4e34c763d2b9a..49585e8c1c5c2 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -153,7 +153,7 @@ async function oidc ({ packageName, registry, opts, config }) { } if (!response?.token) { - log.silly('oidc', 'OIDC token exchange failure: missing token in response body') + log.verbose('oidc', 'OIDC token exchange failure: missing token in response body') return undefined } /* From 526db8e6e064d2a89c8f86837a9f739448e2bed0 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 13:46:28 -0400 Subject: [PATCH 05/12] add test --- test/lib/commands/publish.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 7b08a644adae5..0a93c54bb22a7 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1223,5 +1223,23 @@ t.test('oidc token exchange', t => { }, })) + t.test('trigger any generic error within the try catch block', oidcPublishTest({ + config: { + '//registry.npmjs.org/:_authToken': 'existing-fallback-token', + }, + publishOptions: { + token: 'existing-fallback-token', + }, + load: { + 'ci-info': Object.defineProperty({}, 'GITHUB_ACTIONS', { + get () { + throw new Error('getter error') + }, + enumerable: true, + configurable: true, + }), + }, + })) + t.end() }) From 5fe6bd66c280f8f25eeb7db88fdf5ba842d952ae Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 14:19:56 -0400 Subject: [PATCH 06/12] forgot mocks --- test/lib/commands/publish.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 0a93c54bb22a7..7bb830f6b875c 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1231,13 +1231,15 @@ t.test('oidc token exchange', t => { token: 'existing-fallback-token', }, load: { - 'ci-info': Object.defineProperty({}, 'GITHUB_ACTIONS', { - get () { - throw new Error('getter error') - }, - enumerable: true, - configurable: true, - }), + mocks: { + 'ci-info': Object.defineProperty({}, 'GITHUB_ACTIONS', { + get () { + throw new Error('getter error') + }, + enumerable: true, + configurable: true, + }), + }, }, })) From 51c43fe6f5b0ee4ca1d52c1fbad8b536ea2a5019 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 15:12:28 -0400 Subject: [PATCH 07/12] ignore catch block --- lib/utils/oidc.js | 1 + test/lib/commands/publish.js | 21 +-------------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index 49585e8c1c5c2..2e6b37c04ffb9 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -165,6 +165,7 @@ async function oidc ({ packageName, registry, opts, config }) { opts[authTokenKey] = response.token config.set(authTokenKey, response.token, 'user') log.silly('oidc', `OIDC token successfully retrieved`) + /* istanbul ignore next */ } catch (error) { log.verbose('oidc', error.message) } diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 7bb830f6b875c..d5ab728181cdf 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -6,6 +6,7 @@ const Arborist = require('@npmcli/arborist') const path = require('node:path') const fs = require('node:fs') const { MockOidc } = require('../../fixtures/mock-oidc') +const tmock = require('../../fixtures/tmock') const pkg = '@npmcli/test-package' const token = 'test-auth-token' @@ -1223,25 +1224,5 @@ t.test('oidc token exchange', t => { }, })) - t.test('trigger any generic error within the try catch block', oidcPublishTest({ - config: { - '//registry.npmjs.org/:_authToken': 'existing-fallback-token', - }, - publishOptions: { - token: 'existing-fallback-token', - }, - load: { - mocks: { - 'ci-info': Object.defineProperty({}, 'GITHUB_ACTIONS', { - get () { - throw new Error('getter error') - }, - enumerable: true, - configurable: true, - }), - }, - }, - })) - t.end() }) From 8dcaff1422bf031bac17b000e81ab57cb17860e9 Mon Sep 17 00:00:00 2001 From: Reggi Date: Fri, 27 Jun 2025 15:14:16 -0400 Subject: [PATCH 08/12] log error Co-authored-by: Chris Sidi --- lib/utils/oidc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index 2e6b37c04ffb9..4ca84f1b06ac3 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -167,7 +167,7 @@ async function oidc ({ packageName, registry, opts, config }) { log.silly('oidc', `OIDC token successfully retrieved`) /* istanbul ignore next */ } catch (error) { - log.verbose('oidc', error.message) + log.verbose('oidc', 'Failure checking OIDC config', error) } return undefined } From 28d4329ebaf87068661ae159d3f292b3abb1ff5d Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 15:15:09 -0400 Subject: [PATCH 09/12] rm tmock --- test/lib/commands/publish.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index d5ab728181cdf..7b08a644adae5 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -6,7 +6,6 @@ const Arborist = require('@npmcli/arborist') const path = require('node:path') const fs = require('node:fs') const { MockOidc } = require('../../fixtures/mock-oidc') -const tmock = require('../../fixtures/tmock') const pkg = '@npmcli/test-package' const token = 'test-auth-token' From 177d16b8458efb8231e50a91bb84ccc9a2e3cb47 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 15:17:50 -0400 Subject: [PATCH 10/12] other line --- lib/utils/oidc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/oidc.js b/lib/utils/oidc.js index 4ca84f1b06ac3..279b6335da352 100644 --- a/lib/utils/oidc.js +++ b/lib/utils/oidc.js @@ -165,8 +165,8 @@ async function oidc ({ packageName, registry, opts, config }) { opts[authTokenKey] = response.token config.set(authTokenKey, response.token, 'user') log.silly('oidc', `OIDC token successfully retrieved`) - /* istanbul ignore next */ } catch (error) { + /* istanbul ignore next */ log.verbose('oidc', 'Failure checking OIDC config', error) } return undefined From 733e6c3c831873b561a85ee9c6bedc4c7e878961 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 15:34:45 -0400 Subject: [PATCH 11/12] fix tests --- mock-registry/lib/index.js | 4 ++-- test/lib/commands/publish.js | 23 ++++++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index f442499627042..03ea6d4c14c3a 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -631,11 +631,11 @@ class MockRegistry { } } - mockOidcTokenExchange ({ packageName, idToken, token, statusCode = 200 } = {}) { + mockOidcTokenExchange ({ packageName, idToken, statusCode = 200, body } = {}) { const encodedPackageName = npa(packageName).escapedName this.nock.post(this.fullPath(`/-/npm/v1/oidc/token/exchange/package/${encodedPackageName}`)) .matchHeader('authorization', `Bearer ${idToken}`) - .reply(statusCode, statusCode !== 500 ? { token } : { message: 'Internal Server Error' }) + .reply(statusCode, body || { message: 'Internal Server Error' }) } } diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 7b08a644adae5..bc962be2df259 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1071,6 +1071,9 @@ t.test('oidc token exchange', t => { mockOidcTokenExchangeOptions: { statusCode: 500, idToken: 'github-jwt-id-token', + body: { + message: 'oidc token exchange failed', + }, }, publishOptions: { token: 'existing-fallback-token', @@ -1138,7 +1141,9 @@ t.test('oidc token exchange', t => { }, mockOidcTokenExchangeOptions: { idToken: 'github-jwt-id-token', - token: 'exchange-token', + body: { + token: 'exchange-token', + }, }, publishOptions: { token: 'exchange-token', @@ -1152,7 +1157,9 @@ t.test('oidc token exchange', t => { }, mockOidcTokenExchangeOptions: { idToken: 'gitlab-jwt-id-token', - token: 'exchange-token', + body: { + token: 'exchange-token', + }, }, publishOptions: { token: 'exchange-token', @@ -1172,7 +1179,9 @@ t.test('oidc token exchange', t => { }, mockOidcTokenExchangeOptions: { idToken: 'github-jwt-id-token', - token: 'exchange-token', + body: { + token: 'exchange-token', + }, }, publishOptions: { token: 'exchange-token', @@ -1190,7 +1199,9 @@ t.test('oidc token exchange', t => { }, mockOidcTokenExchangeOptions: { idToken: 'github-jwt-id-token', - token: 'exchange-token', + body: { + token: 'exchange-token', + }, }, publishOptions: { token: 'exchange-token', @@ -1213,7 +1224,9 @@ t.test('oidc token exchange', t => { }, mockOidcTokenExchangeOptions: { idToken: 'github-jwt-id-token', - token: 'exchange-token', + body: { + token: 'exchange-token', + }, }, publishOptions: { token: 'exchange-token', From 8902c9240f22b79b27157c1d4deedada39bd1b08 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 27 Jun 2025 15:54:09 -0400 Subject: [PATCH 12/12] fix tests again --- mock-registry/lib/index.js | 2 +- test/lib/commands/publish.js | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 03ea6d4c14c3a..65cf4b8983aa3 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -635,7 +635,7 @@ class MockRegistry { const encodedPackageName = npa(packageName).escapedName this.nock.post(this.fullPath(`/-/npm/v1/oidc/token/exchange/package/${encodedPackageName}`)) .matchHeader('authorization', `Bearer ${idToken}`) - .reply(statusCode, body || { message: 'Internal Server Error' }) + .reply(statusCode, body || {}) } } diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index bc962be2df259..13a284e0c1f5b 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1080,6 +1080,24 @@ t.test('oidc token exchange', t => { }, })) + t.test('token exchange 500 (with no body message) with fallback', oidcPublishTest({ + oidcOptions: { github: true }, + config: { + '//registry.npmjs.org/:_authToken': 'existing-fallback-token', + }, + mockGithubOidcOptions: { + audience: 'npm:registry.npmjs.org', + idToken: 'github-jwt-id-token', + }, + mockOidcTokenExchangeOptions: { + statusCode: 500, + idToken: 'github-jwt-id-token', + }, + publishOptions: { + token: 'existing-fallback-token', + }, + })) + t.test('token exchange invalid body with fallback', oidcPublishTest({ oidcOptions: { github: true }, config: {