Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix: sanitize sensitive items from UI state (#35003)
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

This PR ensures that several sensitive items are removed from the flat
state that is made available to the frontend.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`

If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`

(This helps the Release Engineer do their job more quickly and
accurately)
-->

CHANGELOG entry: tbd

Fixes: no issue logged so far

1. Open extension settings.
2. Go to "Advanced" and click "Download state logs".
3. Check whether sensitive fields have been sanitized.

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

<!-- [screenshots/recordings] -->

<!-- [screenshots/recordings] -->

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/35003?quickstart=1)
  • Loading branch information
matthiasgeihs authored and Gudahtt committed Aug 13, 2025
commit ad8df649d5097df7a83ad4150f8b1d2d4720ff3b
100 changes: 100 additions & 0 deletions app/scripts/lib/state-utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import {
SecretType,
SeedlessOnboardingControllerState,
} from '@metamask/seedless-onboarding-controller';
import { AuthenticationControllerState } from '@metamask/profile-sync-controller/auth';
import { KeyringControllerState } from '@metamask/keyring-controller';
import { sanitizeUIState } from './state-utils';

describe('State Utils', () => {
Expand Down Expand Up @@ -57,4 +63,98 @@ describe('State Utils', () => {
});
});
});

it('sanitizes keyring controller state', () => {
const state: Partial<KeyringControllerState> = {
vault: 'vault',
encryptionKey: 'encryptionKey',
encryptionSalt: 'encryptionSalt',
};

const sanitizedState = sanitizeUIState(state);

expect(sanitizedState).toStrictEqual({});
});

it('sanitizes authentication controller state', () => {
const state: Partial<AuthenticationControllerState> = {
srpSessionData: {
test1: {
token: {
accessToken: 'accessToken', // to be sanitized
expiresIn: 0,
obtainedAt: 0,
},
profile: {
identifierId: '',
profileId: '',
metaMetricsId: '',
},
},
},
};

const sanitizedState = sanitizeUIState(state);

expect(sanitizedState).toStrictEqual({
srpSessionData: {
test1: {
token: {
expiresIn: 0,
obtainedAt: 0,
},
profile: {
identifierId: '',
profileId: '',
metaMetricsId: '',
},
},
},
});
});

it('sanitizes seedless onboarding controller state', () => {
const state: Partial<SeedlessOnboardingControllerState> = {
vault: 'vault',
vaultEncryptionKey: 'vaultEncryptionKey',
vaultEncryptionSalt: 'vaultEncryptionSalt',
encryptedSeedlessEncryptionKey: 'encryptedSeedlessEncryptionKey',
encryptedKeyringEncryptionKey: 'encryptedKeyringEncryptionKey',
accessToken: 'accessToken',
metadataAccessToken: 'metadataAccessToken',
refreshToken: 'refreshToken',
revokeToken: 'revokeToken',
nodeAuthTokens: [
{
authToken: 'authToken', // to be sanitized
nodeIndex: 1,
nodePubKey: 'nodeUrl',
},
],
socialBackupsMetadata: [
{
hash: 'hash', // to be sanitized
type: SecretType.Mnemonic,
keyringId: 'keyringId',
},
],
};

const sanitizedState = sanitizeUIState(state);

expect(sanitizedState).toStrictEqual({
nodeAuthTokens: [
{
nodeIndex: 1,
nodePubKey: 'nodeUrl',
},
],
socialBackupsMetadata: [
{
type: SecretType.Mnemonic,
keyringId: 'keyringId',
},
],
});
});
});
83 changes: 82 additions & 1 deletion app/scripts/lib/state-utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { AuthenticationControllerState } from '@metamask/profile-sync-controller/auth';
import { SeedlessOnboardingControllerState } from '@metamask/seedless-onboarding-controller';
import { SnapControllerState } from '@metamask/snaps-controllers';
import { Snap } from '@metamask/snaps-utils';

Expand All @@ -8,12 +10,16 @@ type FlattenedUIState = Record<string, any>;
const REMOVE_KEYS = [
'snapStates',
'unencryptedSnapStates',
'vault',
'phishingLists',
'whitelist',
'hotlistLastFetched',
'stalelistLastFetched',
'c2DomainBlocklistLastFetched',

// Keyring controller
'vault',
'encryptionKey',
'encryptionSalt',
];

export function sanitizeUIState(state: FlattenedUIState): FlattenedUIState {
Expand All @@ -24,6 +30,8 @@ export function sanitizeUIState(state: FlattenedUIState): FlattenedUIState {
}

sanitizeSnapData(newState);
sanitizeAuthenticationControllerState(newState);
sanitizeSeedlessOnboardingControllerState(newState);

return newState;
}
Expand Down Expand Up @@ -54,3 +62,76 @@ function stripLargeSnapData(snapData: Snap): Partial<Snap> {

return newData;
}

function sanitizeAuthenticationControllerState(state: FlattenedUIState) {
const srpSessionData = state.srpSessionData as
| AuthenticationControllerState['srpSessionData']
| undefined;

if (!srpSessionData) {
return;
}

state.srpSessionData = Object.entries(srpSessionData).reduce(
(acc, [key, value]) => {
const token = {
...value.token,
};
// @ts-expect-error - Intentionally sanitizing a required field.
delete token.accessToken;
acc[key] = {
...value,
token,
};
return acc;
},
{} as NonNullable<AuthenticationControllerState['srpSessionData']>,
);
}

function sanitizeSeedlessOnboardingControllerState(state: FlattenedUIState) {
const toDelete = [
'vault',
'vaultEncryptionKey',
'vaultEncryptionSalt',
'encryptedSeedlessEncryptionKey',
'encryptedKeyringEncryptionKey',
'accessToken',
'metadataAccessToken',
'refreshToken',
'revokeToken',
];
for (const key of toDelete) {
delete state[key];
}

// Manually sanitize the nodeAuthTokens.
const nodeAuthTokens =
state.nodeAuthTokens as SeedlessOnboardingControllerState['nodeAuthTokens'];

if (nodeAuthTokens) {
state.nodeAuthTokens = nodeAuthTokens.map((token) => {
const sanitizedToken = {
...token,
};
// @ts-expect-error - Intentionally sanitizing a required field.
delete sanitizedToken.authToken;
return sanitizedToken;
});
}

// Manually sanitize the socialBackupsMetadata.
const socialBackupsMetadata =
state.socialBackupsMetadata as SeedlessOnboardingControllerState['socialBackupsMetadata'];

if (socialBackupsMetadata) {
state.socialBackupsMetadata = socialBackupsMetadata.map((backup) => {
const sanitizedBackup = {
...backup,
};
// @ts-expect-error - Intentionally sanitizing a required field.
delete sanitizedBackup.hash;
return sanitizedBackup;
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,6 @@
"submitHistory": "object",
"delegations": "object",
"accountTree": "object",
"encryptionKey": "string",
"encryptionSalt": "string",
"socialBackupsMetadata": "object",
"srpSessionData": "object",
"enableEnforcedSimulations": true,
Expand Down
2 changes: 0 additions & 2 deletions test/integration/data/onboarding-completion-route.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@
"0xaa36a7": true
}
},
"encryptionKey": "{\"key\":{\"alg\":\"A256GCM\",\"ext\":true,\"k\":\"IWP6tqC2WeYfTxhHhTFU2m1hhO9GD5YhgYe2iRzu5GU\",\"key_ops\":[\"encrypt\",\"decrypt\"],\"kty\":\"oct\"},\"derivationOptions\":{\"algorithm\":\"PBKDF2\",\"params\":{\"iterations\":600000}}}",
"encryptionSalt": "6AA1VP9N1aU5bglaSiAhmE9PdLH3YCqiKJagv+wQCR8=",
"enforcedSimulationsSlippage": "number",
"enforcedSimulationsSlippageForTransactions": "object",
"ensEntries": {},
Expand Down
Loading