Skip to content

Commit

Permalink
fix: remove old phishfort list from clients (#27743)
Browse files Browse the repository at this point in the history
<!--
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.
-->

## **Description**

<!--
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?
-->

This PR fixes an issue that prevented users from receiving the updated
hotlist from ETH Phishing Detect. While the client still fetched the
hotlist, the `PhishingDetector` was unable to update with the new URLs
included in the hotlist.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27743?quickstart=1)

## **Related issues**

Fixes: #27737

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] 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.

---------

Co-authored-by: Mark Stacey <[email protected]>
  • Loading branch information
AugmentedMode and Gudahtt authored Oct 9, 2024
1 parent b9a24a7 commit 50dceb5
Show file tree
Hide file tree
Showing 3 changed files with 197 additions and 0 deletions.
142 changes: 142 additions & 0 deletions app/scripts/migrations/126.1.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { migrate, version } from './126.1';

const oldVersion = 126.1;

const mockPhishingListMetaMask = {
allowlist: [],
blocklist: ['malicious1.com'],
c2DomainBlocklist: ['malicious2.com'],
fuzzylist: [],
tolerance: 0,
version: 1,
lastUpdated: Date.now(),
name: 'MetaMask',
};

const mockPhishingListPhishfort = {
allowlist: [],
blocklist: ['phishfort1.com'],
c2DomainBlocklist: ['phishfort2.com'],
fuzzylist: [],
tolerance: 0,
version: 1,
lastUpdated: Date.now(),
name: 'Phishfort',
};

describe(`migration #${version}`, () => {
it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {},
};

const newStorage = await migrate(oldStorage);

expect(newStorage.meta).toStrictEqual({ version });
});

it('keeps only the MetaMask phishing list in PhishingControllerState', async () => {
const oldState = {
PhishingController: {
phishingLists: [mockPhishingListMetaMask, mockPhishingListPhishfort],
whitelist: [],
hotlistLastFetched: 0,
stalelistLastFetched: 0,
c2DomainBlocklistLastFetched: 0,
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

const updatedPhishingController = transformedState.data
.PhishingController as Record<string, unknown>;

expect(updatedPhishingController.phishingLists).toStrictEqual([
mockPhishingListMetaMask,
]);
});

it('removes all phishing lists if MetaMask is not present', async () => {
const oldState = {
PhishingController: {
phishingLists: [mockPhishingListPhishfort],
whitelist: [],
hotlistLastFetched: 0,
stalelistLastFetched: 0,
c2DomainBlocklistLastFetched: 0,
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

const updatedPhishingController = transformedState.data
.PhishingController as Record<string, unknown>;

expect(updatedPhishingController.phishingLists).toStrictEqual([]);
});

it('does nothing if PhishingControllerState is empty', async () => {
const oldState = {
PhishingController: {
phishingLists: [],
whitelist: [],
hotlistLastFetched: 0,
stalelistLastFetched: 0,
c2DomainBlocklistLastFetched: 0,
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

const updatedPhishingController = transformedState.data
.PhishingController as Record<string, unknown>;

expect(updatedPhishingController.phishingLists).toStrictEqual([]);
});

it('does nothing if PhishingController is not in the state', async () => {
const oldState = {
NetworkController: {
providerConfig: {
chainId: '0x1',
},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

expect(transformedState.data).toStrictEqual(oldState);
});

it('does nothing if phishingLists is not an array (null)', async () => {
const oldState: Record<string, unknown> = {
PhishingController: {
phishingLists: null,
whitelist: [],
hotlistLastFetched: 0,
stalelistLastFetched: 0,
c2DomainBlocklistLastFetched: 0,
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

expect(transformedState.data).toStrictEqual(oldState);
});
});
54 changes: 54 additions & 0 deletions app/scripts/migrations/126.1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { hasProperty, isObject } from '@metamask/utils';
import { cloneDeep } from 'lodash';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

export const version = 126.1;

/**
* This migration removes `providerConfig` from the network controller state.
*
* @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist.
* @param originalVersionedData.meta - State metadata.
* @param originalVersionedData.meta.version - The current state version.
* @param originalVersionedData.data - The persisted MetaMask state, keyed by controller.
* @returns Updated versioned MetaMask extension state.
*/
export async function migrate(
originalVersionedData: VersionedData,
): Promise<VersionedData> {
const versionedData = cloneDeep(originalVersionedData);
versionedData.meta.version = version;
transformState(versionedData.data);
return versionedData;
}

function transformState(
state: Record<string, unknown>,
): Record<string, unknown> {
if (
hasProperty(state, 'PhishingController') &&
isObject(state.PhishingController) &&
hasProperty(state.PhishingController, 'phishingLists')
) {
const phishingController = state.PhishingController;

if (!Array.isArray(phishingController.phishingLists)) {
console.error(
`Migration ${version}: Invalid PhishingController.phishingLists state`,
);
return state;
}

phishingController.phishingLists = phishingController.phishingLists.filter(
(list) => list.name === 'MetaMask',
);

state.PhishingController = phishingController;
}

return state;
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ const migrations = [
require('./125'),
require('./125.1'),
require('./126'),
require('./126.1'),
require('./127'),
require('./128'),
require('./129'),
Expand Down

0 comments on commit 50dceb5

Please sign in to comment.