diff --git a/.github/workflows/perf.yml b/.github/workflows/perf.yml index f6f6ddf7ff..4e59114003 100644 --- a/.github/workflows/perf.yml +++ b/.github/workflows/perf.yml @@ -22,6 +22,8 @@ jobs: with: cache: yarn - uses: salesforcecli/github-workflows/.github/actions/yarnInstallWithRetries@main + with: + ignore-scripts: true - run: yarn build - run: npm run test:perf | tee test/perf/output.txt diff --git a/src/logger/filters.ts b/src/logger/filters.ts index 1bace000b9..0b4d8013f7 100644 --- a/src/logger/filters.ts +++ b/src/logger/filters.ts @@ -35,6 +35,7 @@ const FILTERED_KEYS: FilteredKeyDefinition[] = [ // Any json attribute that contains the words "refresh" and "token" will have the attribute/value hidden { name: 'refresh_token', regex: 'refresh[^\'"]*token' }, { name: 'clientsecret' }, + { name: 'authcode' }, ]; const FILTERED_KEYS_FOR_PROCESSING: FilteredKeyForProcessing[] = FILTERED_KEYS.map((key) => ({ @@ -60,6 +61,11 @@ const replacementFunctions = FILTERED_KEYS_FOR_PROCESSING.flatMap( input .replace(new RegExp(accessTokenRegex, 'g'), '') .replace(new RegExp(sfdxAuthUrlRegex, 'g'), ''), + // conditional replacement for clientId: leave the value if it's the Platform CLI, otherwise redact it + (input: string): string => + input.replace(/(['"]client.*Id['"])\s*:\s*(['"][^'"]*['"])/gi, (all, key: string, value: string) => + value.includes('Platform CLI') ? `${key}:${value}` : `${key}:""` + ), ]); const fullReplacementChain = compose(...replacementFunctions); diff --git a/src/org/authInfo.ts b/src/org/authInfo.ts index 15f29537e0..a70f670691 100644 --- a/src/org/authInfo.ts +++ b/src/org/authInfo.ts @@ -35,6 +35,7 @@ import { Logger } from '../logger/logger'; import { SfError } from '../sfError'; import { matchesAccessToken, trimTo15 } from '../util/sfdc'; import { StateAggregator } from '../stateAggregator/stateAggregator'; +import { filterSecrets } from '../logger/filters'; import { Messages } from '../messages'; import { getLoginAudienceCombos, SfdcUrl } from '../util/sfdcUrl'; import { Connection, SFDX_HTTP_HEADERS } from './connection'; @@ -1106,7 +1107,14 @@ export class AuthInfo extends AsyncOptionalCreatable { this.logger.info(`Exchanging auth code for access token using loginUrl: ${options.loginUrl}`); authFields = await oauth2.requestToken(ensure(options.authCode)); } catch (err) { - throw messages.createError('authCodeExchangeError', [(err as Error).message]); + const msg = err instanceof Error ? `${err.name}::${err.message}` : typeof err === 'string' ? err : 'UNKNOWN'; + const redacted = filterSecrets(options); + throw SfError.create({ + message: messages.getMessage('authCodeExchangeError', [msg]), + name: 'AuthCodeExchangeError', + ...(err instanceof Error ? { cause: err } : {}), + data: (isArray(redacted) ? redacted[0] : redacted) as JwtOAuth2Config, + }); } const { orgId } = parseIdUrl(authFields.id); diff --git a/test/unit/logger/filterTest.ts b/test/unit/logger/filterTest.ts index 7914dba225..b39ffd21bf 100644 --- a/test/unit/logger/filterTest.ts +++ b/test/unit/logger/filterTest.ts @@ -17,18 +17,6 @@ describe('filters', () => { // eslint-disable-next-line camelcase access_token: sid, })}`; - const obj1 = { accessToken: `${sid}`, refreshToken: `${sid}` }; - const obj2 = { key: 'Access Token', value: `${sid}` }; - const arr1 = [ - { key: 'ACCESS token ', value: `${sid}` }, - { key: 'refresh TOKEN', value: `${sid}` }, - { key: 'Sfdx Auth Url', value: `${sid}` }, - ]; - const arr2 = [ - { key: ' AcCESS 78token', value: ` ${sid} ` }, - { key: ' refresh _TOKEn ', value: ` ${sid} ` }, - { key: ' SfdX__AuthUrl ', value: ` ${sid} ` }, - ]; it(`filters ${simpleString} correctly`, () => { const result = getUnwrapped(simpleString); @@ -42,8 +30,8 @@ describe('filters', () => { expect(result).to.contain('REDACTED ACCESS TOKEN'); }); - it('filters obj1 correctly', () => { - const result = getUnwrapped(obj1); + it('filters regular object correctly', () => { + const result = getUnwrapped({ accessToken: `${sid}`, refreshToken: `${sid}` }); assert(result); const bigString = JSON.stringify(result); expect(bigString).to.not.contain(sid); @@ -51,16 +39,55 @@ describe('filters', () => { expect(bigString).to.contain('refresh_token - HIDDEN'); }); - it('filters obj2 correctly', () => { - const result = getUnwrapped(obj2); + it('filters key/value object correctly', () => { + const result = getUnwrapped({ key: 'Access Token', value: `${sid}` }); assert(result); const bigString = JSON.stringify(result); expect(bigString).to.not.contain(sid); expect(bigString).to.contain('REDACTED ACCESS TOKEN'); }); - it('filters arr1 correctly', () => { - const result = getUnwrapped(arr1); + it('filters auth code correctly', () => { + const result = getUnwrapped({ authCode: 'authcode value' }); + assert(result); + const bigString = JSON.stringify(result); + expect(bigString).to.not.contain('authCode value'); + expect(bigString).to.contain('authcode - HIDDEN'); + }); + + describe('client id', () => { + it('filters clientId correctly', () => { + const result = getUnwrapped({ clientId: 'clientIdValue' }); + assert(result); + const bigString = JSON.stringify(result); + expect(bigString).to.not.contain('clientIdValue'); + expect(bigString).to.contain('REDACTED CLIENT ID'); + }); + + it('filters clientId correctly (case insensitive)', () => { + const result = getUnwrapped({ ClientId: 'clientIdValue' }); + assert(result); + const bigString = JSON.stringify(result); + expect(bigString).to.not.contain('clientIdValue'); + expect(bigString).to.contain('REDACTED CLIENT ID'); + }); + + it('filters clientId correctly (separator)', () => { + // eslint-disable-next-line camelcase + const result = getUnwrapped({ Client_Id: 'clientIdValue' }); + assert(result); + const bigString = JSON.stringify(result); + expect(bigString).to.not.contain('clientIdValue'); + expect(bigString).to.contain('REDACTED CLIENT ID'); + }); + }); + + it('filters array correctly', () => { + const result = getUnwrapped([ + { key: 'ACCESS token ', value: `${sid}` }, + { key: 'refresh TOKEN', value: `${sid}` }, + { key: 'Sfdx Auth Url', value: `${sid}` }, + ]); assert(result); assert(Array.isArray(result)); const bigString = JSON.stringify(result); @@ -70,8 +97,12 @@ describe('filters', () => { expect(bigString).to.contain('refresh_token - HIDDEN'); }); - it('filters arr2 correctly', () => { - const result = getUnwrapped(arr2); + it('filters another array correctly', () => { + const result = getUnwrapped([ + { key: ' AcCESS 78token', value: ` ${sid} ` }, + { key: ' refresh _TOKEn ', value: ` ${sid} ` }, + { key: ' SfdX__AuthUrl ', value: ` ${sid} ` }, + ]); assert(result); assert(Array.isArray(result)); const bigString = JSON.stringify(result); @@ -101,6 +132,24 @@ describe('filters', () => { expect(result).to.have.property('foo', 'bar'); expect(result).to.have.property('accessToken').contains('REDACTED ACCESS TOKEN'); }); + describe('clientId', () => { + it('default connected app', () => { + const input = { clientId: 'Platform CLI' }; + const result = getUnwrapped(input); + expect(result).to.deep.equal(input); + }); + it('default connected app (case insensitive)', () => { + const input = { ClientID: 'Platform CLI' }; + const result = getUnwrapped(input); + expect(result).to.deep.equal(input); + }); + it('default connected app (case insensitive)', () => { + // eslint-disable-next-line camelcase + const input = { client_id: 'Platform CLI' }; + const result = getUnwrapped(input); + expect(result).to.deep.equal(input); + }); + }); }); });