From dc06fb9195fb34611ef08273b693dc5720d25e20 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Sep 2025 15:52:27 -0230 Subject: [PATCH 1/9] fix: Strip `accessToken` from `AuthenticationController` state logs The metadata property was recently added to the `AuthenticationController` in #6470, but we forgot to strip out the `accessToken`. This is currently being stripped from state logs in both clients. The metadata was updated to strip out this token. Additionally, some improvements were added to the metadata snapshot tests to ensure that all properties are represented in the fixture state. The `@metamask/utils` package was added as a dependency because the new metadata state deriver uses `Json` in its signature, so that type is now used in the exported types for this package. --- .../base-controller/src/BaseController.ts | 1 + packages/profile-sync-controller/package.json | 1 + .../AuthenticationController.test.ts | 134 +++++++++++++++--- .../AuthenticationController.ts | 23 ++- yarn.lock | 1 + 5 files changed, 143 insertions(+), 17 deletions(-) diff --git a/packages/base-controller/src/BaseController.ts b/packages/base-controller/src/BaseController.ts index 2346db8fc44..d1ae9cbe72d 100644 --- a/packages/base-controller/src/BaseController.ts +++ b/packages/base-controller/src/BaseController.ts @@ -394,6 +394,7 @@ export function deriveStateFromMetadata< metadata: StateMetadata, metadataProperty: keyof StatePropertyMetadata, ): Record { + // Cast to keyof ControllerState because `Object.keys` erases index type, returning string return (Object.keys(state) as (keyof ControllerState)[]).reduce< Record >((derivedState, key) => { diff --git a/packages/profile-sync-controller/package.json b/packages/profile-sync-controller/package.json index 69db896c5b8..062cec516b5 100644 --- a/packages/profile-sync-controller/package.json +++ b/packages/profile-sync-controller/package.json @@ -103,6 +103,7 @@ "@metamask/base-controller": "^8.3.0", "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", + "@metamask/utils": "^11.4.2", "@noble/ciphers": "^1.3.0", "@noble/hashes": "^1.8.0", "immer": "^9.0.6", diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts index 3b82c86808d..aa4273f81ac 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -14,6 +14,7 @@ import type { LoginResponse } from '../../sdk'; import { Platform } from '../../sdk'; import { arrangeAuthAPIs } from '../../sdk/__fixtures__/auth'; import { MOCK_USER_PROFILE_LINEAGE_RESPONSE } from '../../sdk/mocks/auth'; +import { hasProperty } from '@metamask/utils'; const MOCK_ENTROPY_SOURCE_IDS = [ 'MOCK_ENTROPY_SOURCE_ID', @@ -543,6 +544,7 @@ describe('metadata', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, metametrics: createMockAuthMetaMetrics(), + state: mockSignedInState(), }); expect( @@ -553,41 +555,114 @@ describe('metadata', () => { ), ).toMatchInlineSnapshot(` Object { - "isSignedIn": false, + "isSignedIn": true, } `); }); - it('includes expected state in state logs', () => { - const controller = new AuthenticationController({ - messenger: createMockAuthenticationMessenger().messenger, - metametrics: createMockAuthMetaMetrics(), - }); + describe('includeInStateLogs', () => { + it('includes expected state in state logs, with access token stripped out', () => { + const controller = new AuthenticationController({ + messenger: createMockAuthenticationMessenger().messenger, + metametrics: createMockAuthMetaMetrics(), + state: mockSignedInState(), + }); - expect( - deriveStateFromMetadata( + const derivedState = deriveStateFromMetadata( controller.state, controller.metadata, 'includeInStateLogs', - ), - ).toMatchInlineSnapshot(` - Object { - "isSignedIn": false, - } - `); + ); + + expect(derivedState).toMatchInlineSnapshot(` + Object { + "isSignedIn": true, + "srpSessionData": Object { + "MOCK_ENTROPY_SOURCE_ID": Object { + "profile": Object { + "identifierId": "da9a9fc7b09edde9cc23cec9b7e11a71fb0ab4d2ddd8af8af905306f3e1456fb", + "metaMetricsId": "561ec651-a844-4b36-a451-04d6eac35740", + "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", + }, + "token": Object { + "expiresIn": 1757528159922, + "obtainedAt": 0, + }, + }, + "MOCK_ENTROPY_SOURCE_ID2": Object { + "profile": Object { + "identifierId": "da9a9fc7b09edde9cc23cec9b7e11a71fb0ab4d2ddd8af8af905306f3e1456fb", + "metaMetricsId": "561ec651-a844-4b36-a451-04d6eac35740", + "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", + }, + "token": Object { + "expiresIn": 1757528159922, + "obtainedAt": 0, + }, + }, + }, + } + `); + }); + + it('returns expected state in state logs when srpSessionData is unset', () => { + const controller = new AuthenticationController({ + messenger: createMockAuthenticationMessenger().messenger, + metametrics: createMockAuthMetaMetrics(), + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(` + Object { + "isSignedIn": false, + } + `); + }); }); it('persists expected state', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, metametrics: createMockAuthMetaMetrics(), + state: mockSignedInState(), }); expect( deriveStateFromMetadata(controller.state, controller.metadata, 'persist'), ).toMatchInlineSnapshot(` Object { - "isSignedIn": false, + "isSignedIn": true, + "srpSessionData": Object { + "MOCK_ENTROPY_SOURCE_ID": Object { + "profile": Object { + "identifierId": "da9a9fc7b09edde9cc23cec9b7e11a71fb0ab4d2ddd8af8af905306f3e1456fb", + "metaMetricsId": "561ec651-a844-4b36-a451-04d6eac35740", + "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", + }, + "token": Object { + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "expiresIn": 1757528159923, + "obtainedAt": 0, + }, + }, + "MOCK_ENTROPY_SOURCE_ID2": Object { + "profile": Object { + "identifierId": "da9a9fc7b09edde9cc23cec9b7e11a71fb0ab4d2ddd8af8af905306f3e1456fb", + "metaMetricsId": "561ec651-a844-4b36-a451-04d6eac35740", + "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", + }, + "token": Object { + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "expiresIn": 1757528159923, + "obtainedAt": 0, + }, + }, + }, } `); }); @@ -596,6 +671,7 @@ describe('metadata', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, metametrics: createMockAuthMetaMetrics(), + state: mockSignedInState(), }); expect( @@ -606,7 +682,33 @@ describe('metadata', () => { ), ).toMatchInlineSnapshot(` Object { - "isSignedIn": false, + "isSignedIn": true, + "srpSessionData": Object { + "MOCK_ENTROPY_SOURCE_ID": Object { + "profile": Object { + "identifierId": "da9a9fc7b09edde9cc23cec9b7e11a71fb0ab4d2ddd8af8af905306f3e1456fb", + "metaMetricsId": "561ec651-a844-4b36-a451-04d6eac35740", + "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", + }, + "token": Object { + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "expiresIn": 1757528159924, + "obtainedAt": 0, + }, + }, + "MOCK_ENTROPY_SOURCE_ID2": Object { + "profile": Object { + "identifierId": "da9a9fc7b09edde9cc23cec9b7e11a71fb0ab4d2ddd8af8af905306f3e1456fb", + "metaMetricsId": "561ec651-a844-4b36-a451-04d6eac35740", + "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", + }, + "token": Object { + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "expiresIn": 1757528159924, + "obtainedAt": 0, + }, + }, + }, } `); }); diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 0478ff1897d..615c478b029 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -11,6 +11,7 @@ import type { KeyringControllerUnlockEvent, } from '@metamask/keyring-controller'; import type { HandleSnapRequest } from '@metamask/snaps-controllers'; +import type { Json } from '@metamask/utils'; import { createSnapPublicKeyRequest, @@ -49,7 +50,27 @@ const metadata: StateMetadata = { usedInUi: true, }, srpSessionData: { - includeInStateLogs: true, + // Remove access token from state logs + includeInStateLogs: (srpSessionData) => { + // Using non-null assertion here to assert that it's not undefined, because if it was, this + // wouldn't get called. + // The type includes `| undefined` only because we don't yet use `strictOptionalTypes` + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return Object.entries(srpSessionData!).reduce>( + (sanitizedSrpSessionData, [key, value]) => { + const token: Partial<(typeof value)['token']> = { + ...value.token, + }; + delete token.accessToken; + sanitizedSrpSessionData[key] = { + ...value, + token, + }; + return sanitizedSrpSessionData; + }, + {}, + ); + }, persist: true, anonymous: false, usedInUi: true, diff --git a/yarn.lock b/yarn.lock index 3b747dc56b3..775598bebb0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4264,6 +4264,7 @@ __metadata: "@metamask/snaps-controllers": "npm:^14.0.1" "@metamask/snaps-sdk": "npm:^9.0.0" "@metamask/snaps-utils": "npm:^11.0.0" + "@metamask/utils": "npm:^11.4.2" "@noble/ciphers": "npm:^1.3.0" "@noble/hashes": "npm:^1.8.0" "@types/jest": "npm:^27.4.1" From 1d6855d4205628c4a0686fc26b5002776c5f6d84 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Sep 2025 15:56:30 -0230 Subject: [PATCH 2/9] Update changelog --- packages/profile-sync-controller/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 46f49f8686a..a0c7c5d3d71 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Implement deferred login pattern in `SRPJwtBearerAuth` to prevent race conditions during concurrent authentication attempts ([#6353](https://github.com/MetaMask/core/pull/6353)) - Add `#deferredLogin` method that ensures only one login operation executes at a time using Promise map caching - Bump `@metamask/base-controller` from `^8.1.0` to `^8.3.0` ([#6355](https://github.com/MetaMask/core/pull/6355), [#6465](https://github.com/MetaMask/core/pull/6465)) +- Add dependency on `@metamask/utils` ([#6553](https://github.com/MetaMask/core/pull/6553)) ### Removed From 50627c6844969d4e7d5f40cad45352c54aa3ad46 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Sep 2025 15:57:37 -0230 Subject: [PATCH 3/9] Remove accidentally committed comment --- packages/base-controller/src/BaseController.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/base-controller/src/BaseController.ts b/packages/base-controller/src/BaseController.ts index d1ae9cbe72d..2346db8fc44 100644 --- a/packages/base-controller/src/BaseController.ts +++ b/packages/base-controller/src/BaseController.ts @@ -394,7 +394,6 @@ export function deriveStateFromMetadata< metadata: StateMetadata, metadataProperty: keyof StatePropertyMetadata, ): Record { - // Cast to keyof ControllerState because `Object.keys` erases index type, returning string return (Object.keys(state) as (keyof ControllerState)[]).reduce< Record >((derivedState, key) => { From 8e9fac08185938db15c4ece1264c5f97b7cad8b2 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Sep 2025 16:08:41 -0230 Subject: [PATCH 4/9] Fix test failures and lint error --- .../AuthenticationController.test.ts | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts index aa4273f81ac..5275337d507 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -14,21 +14,29 @@ import type { LoginResponse } from '../../sdk'; import { Platform } from '../../sdk'; import { arrangeAuthAPIs } from '../../sdk/__fixtures__/auth'; import { MOCK_USER_PROFILE_LINEAGE_RESPONSE } from '../../sdk/mocks/auth'; -import { hasProperty } from '@metamask/utils'; const MOCK_ENTROPY_SOURCE_IDS = [ 'MOCK_ENTROPY_SOURCE_ID', 'MOCK_ENTROPY_SOURCE_ID2', ]; -const mockSignedInState = (): AuthenticationControllerState => { +/** + * Return mock state for the scenario where a user is signed in. + * + * @param options - Options. + * @param options.expiresIn - The timestamp to use for the `expiresIn` token property. + * @returns Mock AuthenticationController state reflecting a signed in user. + */ +const mockSignedInState = ({ + expiresIn = Date.now() + 3600, +}: { expiresIn?: number } = {}): AuthenticationControllerState => { const srpSessionData = {} as Record; MOCK_ENTROPY_SOURCE_IDS.forEach((id) => { srpSessionData[id] = { token: { accessToken: MOCK_OATH_TOKEN_RESPONSE.access_token, - expiresIn: Date.now() + 3600, + expiresIn, obtainedAt: 0, }, profile: { @@ -378,8 +386,9 @@ describe('AuthenticationController', () => { ); for (const id of MOCK_ENTROPY_SOURCE_IDS) { - const resultWithEntropySourceId = - await controller.getSessionProfile(id); + const resultWithEntropySourceId = await controller.getSessionProfile( + id, + ); expect(resultWithEntropySourceId).toBeDefined(); expect(resultWithEntropySourceId).toStrictEqual( originalState.srpSessionData?.[id]?.profile, @@ -544,7 +553,8 @@ describe('metadata', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, metametrics: createMockAuthMetaMetrics(), - state: mockSignedInState(), + // Set `expiresIn` to an arbitrary number so that it stays consistent between test runs + state: mockSignedInState({ expiresIn: 1_000 }), }); expect( @@ -565,7 +575,8 @@ describe('metadata', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, metametrics: createMockAuthMetaMetrics(), - state: mockSignedInState(), + // Set `expiresIn` to an arbitrary number so that it stays consistent between test runs + state: mockSignedInState({ expiresIn: 1_000 }), }); const derivedState = deriveStateFromMetadata( @@ -585,7 +596,7 @@ describe('metadata', () => { "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", }, "token": Object { - "expiresIn": 1757528159922, + "expiresIn": 1000, "obtainedAt": 0, }, }, @@ -596,7 +607,7 @@ describe('metadata', () => { "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", }, "token": Object { - "expiresIn": 1757528159922, + "expiresIn": 1000, "obtainedAt": 0, }, }, @@ -629,7 +640,8 @@ describe('metadata', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, metametrics: createMockAuthMetaMetrics(), - state: mockSignedInState(), + // Set `expiresIn` to an arbitrary number so that it stays consistent between test runs + state: mockSignedInState({ expiresIn: 1_000 }), }); expect( @@ -646,7 +658,7 @@ describe('metadata', () => { }, "token": Object { "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", - "expiresIn": 1757528159923, + "expiresIn": 1000, "obtainedAt": 0, }, }, @@ -658,7 +670,7 @@ describe('metadata', () => { }, "token": Object { "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", - "expiresIn": 1757528159923, + "expiresIn": 1000, "obtainedAt": 0, }, }, @@ -671,7 +683,8 @@ describe('metadata', () => { const controller = new AuthenticationController({ messenger: createMockAuthenticationMessenger().messenger, metametrics: createMockAuthMetaMetrics(), - state: mockSignedInState(), + // Set `expiresIn` to an arbitrary number so that it stays consistent between test runs + state: mockSignedInState({ expiresIn: 1_000 }), }); expect( @@ -692,7 +705,7 @@ describe('metadata', () => { }, "token": Object { "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", - "expiresIn": 1757528159924, + "expiresIn": 1000, "obtainedAt": 0, }, }, @@ -704,7 +717,7 @@ describe('metadata', () => { }, "token": Object { "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", - "expiresIn": 1757528159924, + "expiresIn": 1000, "obtainedAt": 0, }, }, From c7d38a402ffc3d160ae85fdec0f08000fc5d1f25 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Sep 2025 16:31:47 -0230 Subject: [PATCH 5/9] Fix another lint error --- .../authentication/AuthenticationController.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts index 5275337d507..50933b12378 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -386,9 +386,8 @@ describe('AuthenticationController', () => { ); for (const id of MOCK_ENTROPY_SOURCE_IDS) { - const resultWithEntropySourceId = await controller.getSessionProfile( - id, - ); + const resultWithEntropySourceId = + await controller.getSessionProfile(id); expect(resultWithEntropySourceId).toBeDefined(); expect(resultWithEntropySourceId).toStrictEqual( originalState.srpSessionData?.[id]?.profile, From 8cc1bc24a7981b401c123e2646dec1880bd4bf91 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 11 Sep 2025 10:26:19 -0230 Subject: [PATCH 6/9] Make state deriver a bit more concise --- .../authentication/AuthenticationController.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 615c478b029..c44bde69841 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -58,13 +58,11 @@ const metadata: StateMetadata = { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion return Object.entries(srpSessionData!).reduce>( (sanitizedSrpSessionData, [key, value]) => { - const token: Partial<(typeof value)['token']> = { - ...value.token, - }; - delete token.accessToken; + const { accessToken: _unused, ...tokenWithoutAccessToken } = + value.token; sanitizedSrpSessionData[key] = { ...value, - token, + token: tokenWithoutAccessToken, }; return sanitizedSrpSessionData; }, From fb418b9ea2df7559db270e671cf4e585124d73ad Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 11 Sep 2025 10:35:28 -0230 Subject: [PATCH 7/9] Safer handling of null property value --- .../authentication/AuthenticationController.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index c44bde69841..7e9a62443a8 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -52,11 +52,15 @@ const metadata: StateMetadata = { srpSessionData: { // Remove access token from state logs includeInStateLogs: (srpSessionData) => { - // Using non-null assertion here to assert that it's not undefined, because if it was, this - // wouldn't get called. - // The type includes `| undefined` only because we don't yet use `strictOptionalTypes` - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return Object.entries(srpSessionData!).reduce>( + // Unreachable branch, included just to fix a type error for the case where this property is + // unset. The type gets collapsed to include `| undefined` even though `undefined` is never + // set here, because we don't yet use `exactOptionalPropertyTypes`. + // TODO: Remove branch after enabling `exactOptionalPropertyTypes` + // ref: https://github.com/MetaMask/core/issues/6565 + if (srpSessionData === null || srpSessionData === undefined) { + return null; + } + return Object.entries(srpSessionData).reduce>( (sanitizedSrpSessionData, [key, value]) => { const { accessToken: _unused, ...tokenWithoutAccessToken } = value.token; From 32bfec75fddd6070c5fe1ae2955382ce169767c4 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 11 Sep 2025 12:26:59 -0230 Subject: [PATCH 8/9] Update changelog --- packages/profile-sync-controller/CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index a0c7c5d3d71..90be0fae741 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/keyring-api` from `^20.1.0` to `^21.0.0` ([#6560](https://github.com/MetaMask/core/pull/6560)) - Bump `@metamask/keyring-internal-api` from `^8.1.0` to `^9.0.0` ([#6560](https://github.com/MetaMask/core/pull/6560)) +- Strip `srpSessionData.token.accessToken` from state logs ([#6553](https://github.com/MetaMask/core/pull/6553)) + - We haven't started using the `includeInStateLogs` metadata yet in clients, so this will have no functional impact. This change brings this metadata into alignment with the hard-coded state log generation performed by clients.today. +- Add dependency on `@metamask/utils` ([#6553](https://github.com/MetaMask/core/pull/6553)) ## [25.0.0] @@ -24,7 +27,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Implement deferred login pattern in `SRPJwtBearerAuth` to prevent race conditions during concurrent authentication attempts ([#6353](https://github.com/MetaMask/core/pull/6353)) - Add `#deferredLogin` method that ensures only one login operation executes at a time using Promise map caching - Bump `@metamask/base-controller` from `^8.1.0` to `^8.3.0` ([#6355](https://github.com/MetaMask/core/pull/6355), [#6465](https://github.com/MetaMask/core/pull/6465)) -- Add dependency on `@metamask/utils` ([#6553](https://github.com/MetaMask/core/pull/6553)) ### Removed From 04098a8fb28325978a918dfb2f7ff2c5793ae7f1 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 12 Sep 2025 16:30:48 -0230 Subject: [PATCH 9/9] Update utils to match version used elsewhere (it was updated in a recent PR) --- packages/profile-sync-controller/package.json | 2 +- yarn.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/profile-sync-controller/package.json b/packages/profile-sync-controller/package.json index 062cec516b5..1135aac29ed 100644 --- a/packages/profile-sync-controller/package.json +++ b/packages/profile-sync-controller/package.json @@ -103,7 +103,7 @@ "@metamask/base-controller": "^8.3.0", "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", - "@metamask/utils": "^11.4.2", + "@metamask/utils": "^11.8.0", "@noble/ciphers": "^1.3.0", "@noble/hashes": "^1.8.0", "immer": "^9.0.6", diff --git a/yarn.lock b/yarn.lock index 775598bebb0..3e8f7dd140c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4264,7 +4264,7 @@ __metadata: "@metamask/snaps-controllers": "npm:^14.0.1" "@metamask/snaps-sdk": "npm:^9.0.0" "@metamask/snaps-utils": "npm:^11.0.0" - "@metamask/utils": "npm:^11.4.2" + "@metamask/utils": "npm:^11.8.0" "@noble/ciphers": "npm:^1.3.0" "@noble/hashes": "npm:^1.8.0" "@types/jest": "npm:^27.4.1"