From 1df32e6d112792a15aa3f61ecc192963354ece21 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Mon, 23 Mar 2026 11:40:05 +0100 Subject: [PATCH 1/5] use `generate-method-action-types` and `MESSENGER_EXPOSED_METHODS` in `phishing-controller` --- packages/phishing-controller/package.json | 2 + .../PhishingController-method-action-types.ts | 125 +++ .../src/PhishingController.test.ts | 779 ++++++++++++------ .../src/PhishingController.ts | 109 +-- packages/phishing-controller/src/index.ts | 11 + yarn.lock | 1 + 6 files changed, 719 insertions(+), 308 deletions(-) create mode 100644 packages/phishing-controller/src/PhishingController-method-action-types.ts diff --git a/packages/phishing-controller/package.json b/packages/phishing-controller/package.json index 9a2d869b8d3..daedfd70973 100644 --- a/packages/phishing-controller/package.json +++ b/packages/phishing-controller/package.json @@ -40,6 +40,7 @@ "build:docs": "typedoc", "changelog:update": "../../scripts/update-changelog.sh @metamask/phishing-controller", "changelog:validate": "../../scripts/validate-changelog.sh @metamask/phishing-controller", + "generate-method-action-types": "tsx ../../scripts/generate-method-action-types.ts", "since-latest-release": "../../scripts/since-latest-release.sh", "test": "NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter", "test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache", @@ -65,6 +66,7 @@ "jest": "^29.7.0", "nock": "^13.3.1", "ts-jest": "^29.2.5", + "tsx": "^4.20.5", "typedoc": "^0.25.13", "typedoc-plugin-missing-exports": "^2.0.0", "typescript": "~5.3.3" diff --git a/packages/phishing-controller/src/PhishingController-method-action-types.ts b/packages/phishing-controller/src/PhishingController-method-action-types.ts new file mode 100644 index 00000000000..7daef1c465b --- /dev/null +++ b/packages/phishing-controller/src/PhishingController-method-action-types.ts @@ -0,0 +1,125 @@ +/** + * This file is auto generated by `scripts/generate-method-action-types.ts`. + * Do not edit manually. + */ + +import type { PhishingController } from './PhishingController'; + +/** + * Conditionally update the phishing configuration. + * + * If the stalelist configuration is out of date, this function will call `updateStalelist` + * to update the configuration. This will automatically grab the hotlist, + * so it isn't necessary to continue on to download the hotlist and the c2 domain blocklist. + * + */ +export type PhishingControllerMaybeUpdateStateAction = { + type: `PhishingController:maybeUpdateState`; + handler: PhishingController['maybeUpdateState']; +}; + +/** + * Determines if a given origin is unapproved. + * + * It is strongly recommended that you call {@link maybeUpdateState} before calling this, + * to check whether the phishing configuration is up-to-date. It will be updated if necessary + * by calling {@link updateStalelist} or {@link updateHotlist}. + * + * @param origin - Domain origin of a website. + * @returns Whether the origin is an unapproved origin. + */ +export type PhishingControllerTestAction = { + type: `PhishingController:test`; + handler: PhishingController['test']; +}; + +/** + * Checks if a request URL's domain is blocked against the request blocklist. + * + * This method is used to determine if a specific request URL is associated with a malicious + * command and control (C2) domain. The URL's hostname is hashed and checked against a configured + * blocklist of known malicious domains. + * + * @param origin - The full request URL to be checked. + * @returns An object indicating whether the URL's domain is blocked and relevant metadata. + */ +export type PhishingControllerIsBlockedRequestAction = { + type: `PhishingController:isBlockedRequest`; + handler: PhishingController['isBlockedRequest']; +}; + +/** + * Temporarily marks a given origin as approved. + * + * @param origin - The origin to mark as approved. + */ +export type PhishingControllerBypassAction = { + type: `PhishingController:bypass`; + handler: PhishingController['bypass']; +}; + +/** + * Scan a URL for phishing. It will only scan the hostname of the URL. It also only supports + * web URLs. + * + * @param url - The URL to scan. + * @returns The phishing detection scan result. + */ +export type PhishingControllerScanUrlAction = { + type: `PhishingController:scanUrl`; + handler: PhishingController['scanUrl']; +}; + +/** + * Scan multiple URLs for phishing in bulk. It will only scan the hostnames of the URLs. + * It also only supports web URLs. + * + * @param urls - The URLs to scan. + * @returns A mapping of URLs to their phishing detection scan results and errors. + */ +export type PhishingControllerBulkScanUrlsAction = { + type: `PhishingController:bulkScanUrls`; + handler: PhishingController['bulkScanUrls']; +}; + +/** + * Scan an address for security alerts. + * + * @param chainId - The chain ID in hex format (e.g., '0x1' for Ethereum). + * @param address - The address to scan. + * @returns The address scan result. + */ +export type PhishingControllerScanAddressAction = { + type: `PhishingController:scanAddress`; + handler: PhishingController['scanAddress']; +}; + +/** + * Scan multiple tokens for malicious activity in bulk. + * + * @param request - The bulk scan request containing chainId and tokens. + * @param request.chainId - The chain identifier. Accepts a hex chain ID for + * EVM chains (e.g. `'0x1'` for Ethereum) or a chain name for non-EVM chains + * (e.g. `'solana'`). + * @param request.tokens - Array of token addresses to scan. + * @returns A mapping of token addresses to their scan results. For EVM chains, + * addresses are lowercased; for non-EVM chains, original casing is preserved. + * Tokens that fail to scan are omitted. + */ +export type PhishingControllerBulkScanTokensAction = { + type: `PhishingController:bulkScanTokens`; + handler: PhishingController['bulkScanTokens']; +}; + +/** + * Union of all PhishingController action types. + */ +export type PhishingControllerMethodActions = + | PhishingControllerMaybeUpdateStateAction + | PhishingControllerTestAction + | PhishingControllerIsBlockedRequestAction + | PhishingControllerBypassAction + | PhishingControllerScanUrlAction + | PhishingControllerBulkScanUrlsAction + | PhishingControllerScanAddressAction + | PhishingControllerBulkScanTokensAction; diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index db01e841848..0aa16fee1c1 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -105,12 +105,16 @@ function setupMessenger(): { * @param options - The Phishing Controller options. * @returns The constructed Phishing Controller. */ -function getPhishingController(options?: Partial) { - const { messenger } = setupMessenger(); - return new PhishingController({ +function getPhishingController(options?: Partial): { + controller: PhishingController; + rootMessenger: RootMessenger; +} { + const { messenger, rootMessenger } = setupMessenger(); + const controller = new PhishingController({ messenger, ...options, }); + return { controller, rootMessenger }; } describe('PhishingController', () => { @@ -120,20 +124,26 @@ describe('PhishingController', () => { }); it('should have no default phishing lists', () => { - const controller = getPhishingController(); + const { controller } = getPhishingController(); expect(controller.state.phishingLists).toStrictEqual([]); }); it('should default to an empty whitelist', () => { - const controller = getPhishingController(); + const { controller } = getPhishingController(); expect(controller.state.whitelist).toStrictEqual([]); }); it('should return false if the hostname is in the whitelist', async () => { const whitelistedHostname = 'example.com'; - const controller = getPhishingController(); - controller.bypass(formatHostnameToUrl(whitelistedHostname)); - const result = controller.test(whitelistedHostname); + const { rootMessenger } = getPhishingController(); + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(whitelistedHostname), + ); + const result = rootMessenger.call( + 'PhishingController:test', + whitelistedHostname, + ); expect(result).toMatchObject({ result: false, @@ -143,9 +153,15 @@ describe('PhishingController', () => { it('should return false if the URL is in the whitelist', async () => { const whitelistedHostname = 'example.com'; - const controller = getPhishingController(); - controller.bypass(formatHostnameToUrl(whitelistedHostname)); - const result = controller.test(`https://${whitelistedHostname}/path`); + const { rootMessenger } = getPhishingController(); + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(whitelistedHostname), + ); + const result = rootMessenger.call( + 'PhishingController:test', + `https://${whitelistedHostname}/path`, + ); expect(result).toMatchObject({ result: false, @@ -156,9 +172,12 @@ describe('PhishingController', () => { it('returns false if the URL is in the whitelistPaths', async () => { const whitelistedURL = 'https://example.com/path'; - const controller = getPhishingController(); - controller.bypass(whitelistedURL); - const result = controller.test(whitelistedURL); + const { rootMessenger } = getPhishingController(); + rootMessenger.call('PhishingController:bypass', whitelistedURL); + const result = rootMessenger.call( + 'PhishingController:test', + whitelistedURL, + ); expect(result).toMatchObject({ result: false, type: PhishingDetectorResultType.All, @@ -192,9 +211,12 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - const result = controller.test(`https://${allowlistedHostname}/path`); + const result = rootMessenger.call( + 'PhishingController:test', + `https://${allowlistedHostname}/path`, + ); expect(result).toMatchObject({ result: false, @@ -244,7 +266,7 @@ describe('PhishingController', () => { ], }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, state: { phishingLists: [ @@ -318,24 +340,24 @@ describe('PhishingController', () => { it('should not have stalelist be out of date immediately after maybeUpdateState is called', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller, rootMessenger } = getPhishingController({ stalelistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); expect(controller.isStalelistOutOfDate()).toBe(true); - await controller.maybeUpdateState(); + await rootMessenger.call('PhishingController:maybeUpdateState'); expect(controller.isStalelistOutOfDate()).toBe(false); expect(nockScope.isDone()).toBe(true); }); it('should not be out of date after maybeUpdateStalelist is called but before refresh interval has passed', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller, rootMessenger } = getPhishingController({ stalelistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); expect(controller.isStalelistOutOfDate()).toBe(true); - await controller.maybeUpdateState(); + await rootMessenger.call('PhishingController:maybeUpdateState'); jest.advanceTimersByTime(1000 * 5); expect(controller.isStalelistOutOfDate()).toBe(false); expect(nockScope.isDone()).toBe(true); @@ -343,12 +365,14 @@ describe('PhishingController', () => { it('should still be out of date while update is in progress', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller, rootMessenger } = getPhishingController({ stalelistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); // do not wait - const maybeUpdatePhisingListPromise = controller.maybeUpdateState(); + const maybeUpdatePhisingListPromise = rootMessenger.call( + 'PhishingController:maybeUpdateState', + ); expect(controller.isStalelistOutOfDate()).toBe(true); await maybeUpdatePhisingListPromise; expect(controller.isStalelistOutOfDate()).toBe(false); @@ -359,13 +383,14 @@ describe('PhishingController', () => { it('should call update only if it is out of date, otherwise it should not call update', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller, rootMessenger } = getPhishingController({ stalelistRefreshInterval: 10, }); expect(controller.isStalelistOutOfDate()).toBe(false); - await controller.maybeUpdateState(); + await rootMessenger.call('PhishingController:maybeUpdateState'); expect( - controller.test( + rootMessenger.call( + 'PhishingController:test', formatHostnameToUrl('this-should-not-be-in-default-blocklist.com'), ), ).toMatchObject({ @@ -374,7 +399,8 @@ describe('PhishingController', () => { }); expect( - controller.test( + rootMessenger.call( + 'PhishingController:test', formatHostnameToUrl('this-should-not-be-in-default-allowlist.com'), ), ).toMatchObject({ @@ -383,10 +409,11 @@ describe('PhishingController', () => { }); jest.advanceTimersByTime(1000 * 10); - await controller.maybeUpdateState(); + await rootMessenger.call('PhishingController:maybeUpdateState'); expect( - controller.test( + rootMessenger.call( + 'PhishingController:test', formatHostnameToUrl('this-should-not-be-in-default-blocklist.com'), ), ).toMatchObject({ @@ -395,7 +422,8 @@ describe('PhishingController', () => { }); expect( - controller.test( + rootMessenger.call( + 'PhishingController:test', formatHostnameToUrl('this-should-not-be-in-default-allowlist.com'), ), ).toMatchObject({ @@ -428,13 +456,13 @@ describe('PhishingController', () => { doNotFake: ['nextTick', 'queueMicrotask'], now: 50, }); - const controller = getPhishingController({ + const { controller, rootMessenger } = getPhishingController({ hotlistRefreshInterval: 10, stalelistRefreshInterval: 50, }); jest.advanceTimersByTime(1000 * 10); expect(controller.isHotlistOutOfDate()).toBe(true); - await controller.maybeUpdateState(); + await rootMessenger.call('PhishingController:maybeUpdateState'); expect(controller.isHotlistOutOfDate()).toBe(false); }); @@ -447,17 +475,17 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller, rootMessenger } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(true); - await controller.maybeUpdateState(); + await rootMessenger.call('PhishingController:maybeUpdateState'); expect(controller.isC2DomainBlocklistOutOfDate()).toBe(false); }); it('replaces existing phishing lists with completely new list from phishing detection API', async () => { - const { messenger } = setupMessenger(); + const { messenger, rootMessenger } = setupMessenger(); const controller = new PhishingController({ messenger, stalelistRefreshInterval: 10, @@ -514,7 +542,7 @@ describe('PhishingController', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); jest.advanceTimersByTime(1000 * 10); - await controller.maybeUpdateState(); + await rootMessenger.call('PhishingController:maybeUpdateState'); expect(controller.state.phishingLists).toStrictEqual([ { @@ -541,7 +569,7 @@ describe('PhishingController', () => { describe('isStalelistOutOfDate', () => { it('should not be out of date upon construction', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ stalelistRefreshInterval: 10, }); @@ -550,7 +578,7 @@ describe('PhishingController', () => { it('should not be out of date after some of the refresh interval has passed', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ stalelistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 5); @@ -560,7 +588,7 @@ describe('PhishingController', () => { it('should be out of date after the refresh interval has passed', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ stalelistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); @@ -570,7 +598,7 @@ describe('PhishingController', () => { it('should be out of date if the refresh interval has passed and an update is in progress', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ stalelistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); @@ -584,7 +612,7 @@ describe('PhishingController', () => { it('should not be out of date if the phishing lists were just updated', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ stalelistRefreshInterval: 10, }); await controller.updateStalelist(); @@ -594,7 +622,7 @@ describe('PhishingController', () => { it('should not be out of date if the phishing lists were recently updated', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ stalelistRefreshInterval: 10, }); await controller.updateStalelist(); @@ -605,7 +633,7 @@ describe('PhishingController', () => { it('should be out of date if the time elapsed since the last update equals the refresh interval', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ stalelistRefreshInterval: 10, }); await controller.updateStalelist(); @@ -618,7 +646,7 @@ describe('PhishingController', () => { describe('isHotlistOutOfDate', () => { it('should not be out of date upon construction', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, }); @@ -627,7 +655,7 @@ describe('PhishingController', () => { it('should not be out of date after some of the refresh interval has passed', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 5); @@ -637,7 +665,7 @@ describe('PhishingController', () => { it('should be out of date after the refresh interval has passed', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); @@ -647,7 +675,7 @@ describe('PhishingController', () => { it('should be out of date if the refresh interval has passed and an update is in progress', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, state: { phishingLists: [ @@ -676,7 +704,7 @@ describe('PhishingController', () => { it('should not be out of date if the phishing lists were just updated', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, }); await controller.updateHotlist(); @@ -686,7 +714,7 @@ describe('PhishingController', () => { it('should not be out of date if the phishing lists were recently updated', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, }); await controller.updateHotlist(); @@ -697,7 +725,7 @@ describe('PhishingController', () => { it('should be out of date if the time elapsed since the last update equals the refresh interval', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ hotlistRefreshInterval: 10, }); await controller.updateHotlist(); @@ -710,7 +738,7 @@ describe('PhishingController', () => { describe('isC2DomainBlocklistOutOfDate', () => { it('should not be out of date upon construction', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); @@ -719,7 +747,7 @@ describe('PhishingController', () => { it('should not be out of date after some of the refresh interval has passed', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 5); @@ -729,7 +757,7 @@ describe('PhishingController', () => { it('should be out of date after the refresh interval has passed', () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); @@ -739,7 +767,7 @@ describe('PhishingController', () => { it('should be out of date if the refresh interval has passed and an update is in progress', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); jest.advanceTimersByTime(1000 * 10); @@ -753,7 +781,7 @@ describe('PhishingController', () => { it('should not be out of date if the C2 domain blocklist was just updated', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); await controller.updateC2DomainBlocklist(); @@ -763,7 +791,7 @@ describe('PhishingController', () => { it('should not be out of date if the C2 domain blocklist was recently updated', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); await controller.updateC2DomainBlocklist(); @@ -774,7 +802,7 @@ describe('PhishingController', () => { it('should be out of date if the time elapsed since the last update equals the refresh interval', async () => { jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ c2DomainBlocklistRefreshInterval: 10, }); await controller.updateC2DomainBlocklist(); @@ -809,9 +837,14 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - expect(controller.test(formatHostnameToUrl('metamask.io'))).toMatchObject({ + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('metamask.io'), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.Allowlist, name: ListNames.MetaMask, @@ -835,9 +868,14 @@ describe('PhishingController', () => { .get(`${METAMASK_HOTLIST_DIFF_FILE}/${1}`) .reply(200, { data: [] }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - expect(controller.test(formatHostnameToUrl('i❤.ws'))).toMatchObject({ + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('i❤.ws'), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, }); @@ -860,9 +898,14 @@ describe('PhishingController', () => { .get(`${METAMASK_HOTLIST_DIFF_FILE}/${1}`) .reply(200, { data: [] }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - expect(controller.test(formatHostnameToUrl('xn--i-7iq.ws'))).toMatchObject({ + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('xn--i-7iq.ws'), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, }); @@ -893,9 +936,14 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - expect(controller.test(formatHostnameToUrl('etnerscan.io'))).toMatchObject({ + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('etnerscan.io'), + ), + ).toMatchObject({ result: true, type: PhishingDetectorResultType.Blocklist, name: ListNames.MetaMask, @@ -927,10 +975,13 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); expect( - controller.test(formatHostnameToUrl('myetherẉalletṭ.com')), + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('myetherẉalletṭ.com'), + ), ).toMatchObject({ result: true, type: PhishingDetectorResultType.Blocklist, @@ -963,10 +1014,13 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); expect( - controller.test(formatHostnameToUrl('xn--myetherallet-4k5fwn.com')), + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('xn--myetherallet-4k5fwn.com'), + ), ).toMatchObject({ result: true, type: PhishingDetectorResultType.Blocklist, @@ -1007,10 +1061,11 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); expect( - controller.test( + rootMessenger.call( + 'PhishingController:test', formatHostnameToUrl('e4d600ab9141b7a9859511c77e63b9b3.com'), ), ).toMatchObject({ @@ -1037,10 +1092,11 @@ describe('PhishingController', () => { .get(`${METAMASK_HOTLIST_DIFF_FILE}/${1}`) .reply(500); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); expect( - controller.test( + rootMessenger.call( + 'PhishingController:test', formatHostnameToUrl('e4d600ab9141b7a9859511c77e63b9b3.com'), ), ).toMatchObject({ @@ -1074,9 +1130,14 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - expect(controller.test(formatHostnameToUrl('opensea.io'))).toMatchObject({ + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('opensea.io'), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.Allowlist, name: ListNames.MetaMask, @@ -1108,9 +1169,14 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - expect(controller.test(formatHostnameToUrl('ohpensea.io'))).toMatchObject({ + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl('ohpensea.io'), + ), + ).toMatchObject({ result: true, type: PhishingDetectorResultType.Fuzzy, name: ListNames.MetaMask, @@ -1133,10 +1199,11 @@ describe('PhishingController', () => { }) .get(`${METAMASK_HOTLIST_DIFF_FILE}/${1}`) .reply(200, { data: [] }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); expect( - controller.test( + rootMessenger.call( + 'PhishingController:test', formatHostnameToUrl('this-is-the-official-website-of-opensea.io'), ), ).toMatchObject({ @@ -1170,16 +1237,27 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); const unsafeDomain = 'electrum.mx'; assert.equal( - controller.test(formatHostnameToUrl(unsafeDomain)).result, + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ).result, true, 'Example unsafe domain seems to be safe', ); - controller.bypass(formatHostnameToUrl(unsafeDomain)); - expect(controller.test(formatHostnameToUrl(unsafeDomain))).toMatchObject({ + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(unsafeDomain), + ); + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, }); @@ -1210,17 +1288,31 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); const unsafeDomain = 'electrum.mx'; assert.equal( - controller.test(formatHostnameToUrl(unsafeDomain)).result, + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ).result, true, 'Example unsafe domain seems to be safe', ); - controller.bypass(formatHostnameToUrl(unsafeDomain)); - controller.bypass(formatHostnameToUrl(unsafeDomain)); - expect(controller.test(formatHostnameToUrl(unsafeDomain))).toMatchObject({ + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(unsafeDomain), + ); + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(unsafeDomain), + ); + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, }); @@ -1251,16 +1343,27 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); const unsafeDomain = 'myetherẉalletṭ.com'; assert.equal( - controller.test(formatHostnameToUrl(unsafeDomain)).result, + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ).result, true, 'Example unsafe domain seems to be safe', ); - controller.bypass(formatHostnameToUrl(unsafeDomain)); - expect(controller.test(formatHostnameToUrl(unsafeDomain))).toMatchObject({ + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(unsafeDomain), + ); + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, }); @@ -1291,16 +1394,27 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); const unsafeDomain = 'xn--myetherallet-4k5fwn.com'; assert.equal( - controller.test(formatHostnameToUrl(unsafeDomain)).result, + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ).result, true, 'Example unsafe domain seems to be safe', ); - controller.bypass(formatHostnameToUrl(unsafeDomain)); - expect(controller.test(formatHostnameToUrl(unsafeDomain))).toMatchObject({ + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(unsafeDomain), + ); + expect( + rootMessenger.call( + 'PhishingController:test', + formatHostnameToUrl(unsafeDomain), + ), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, }); @@ -1331,16 +1445,18 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - expect(controller.test('https://example.com/path')).toMatchObject({ + expect( + rootMessenger.call('PhishingController:test', 'https://example.com/path'), + ).toMatchObject({ result: true, type: PhishingDetectorResultType.Blocklist, }); }); it('returns negative result if the hostname+pathname is in the whitelistPaths', async () => { - const controller = getPhishingController({ + const { rootMessenger } = getPhishingController({ state: { phishingLists: [ { @@ -1361,15 +1477,17 @@ describe('PhishingController', () => { ], }, }); - controller.bypass('https://example.com/path'); - expect(controller.test('https://example.com/path')).toMatchObject({ + rootMessenger.call('PhishingController:bypass', 'https://example.com/path'); + expect( + rootMessenger.call('PhishingController:test', 'https://example.com/path'), + ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, }); }); it('returns positive result even if the hostname+pathname contains percent encoding', async () => { - const controller = getPhishingController({ + const { rootMessenger } = getPhishingController({ state: { phishingLists: [ { @@ -1391,7 +1509,12 @@ describe('PhishingController', () => { }, }); - expect(controller.test('https://example.com/%70%61%74%68')).toMatchObject({ + expect( + rootMessenger.call( + 'PhishingController:test', + 'https://example.com/%70%61%74%68', + ), + ).toMatchObject({ result: true, type: PhishingDetectorResultType.Blocklist, }); @@ -1437,7 +1560,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller } = getPhishingController(); await controller.updateStalelist(); expect(controller.state.phishingLists).toStrictEqual([ @@ -1499,7 +1622,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller } = getPhishingController(); await controller.updateStalelist(); expect(controller.state.phishingLists).toStrictEqual([ @@ -1542,7 +1665,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller } = getPhishingController(); await controller.updateStalelist(); expect(controller.state.phishingLists).toStrictEqual([ { @@ -1570,7 +1693,7 @@ describe('PhishingController', () => { .get(`${METAMASK_HOTLIST_DIFF_FILE}/${1}`) .reply(304); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -1615,7 +1738,7 @@ describe('PhishingController', () => { .get(C2_DOMAIN_BLOCKLIST_ENDPOINT) .reply(500); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -1661,7 +1784,7 @@ describe('PhishingController', () => { .get(C2_DOMAIN_BLOCKLIST_ENDPOINT) .replyWithError('network error'); - const controller = getPhishingController(); + const { controller } = getPhishingController(); expect(await controller.updateStalelist()).toBeUndefined(); expect(controller.state.c2DomainBlocklistLastFetched).toBe(0); @@ -1690,7 +1813,7 @@ describe('PhishingController', () => { .delay(100) .reply(200, { data: [] }); - const controller = getPhishingController(); + const { controller } = getPhishingController(); const firstPromise = controller.updateStalelist(); const secondPromise = controller.updateStalelist(); @@ -1726,7 +1849,7 @@ describe('PhishingController', () => { .delay(100) .reply(200, { data: [] }); - const controller = getPhishingController(); + const { controller } = getPhishingController(); const firstPromise = controller.updateStalelist(); const secondPromise = controller.updateStalelist(); jest.advanceTimersByTime(1000 * 99); @@ -1754,7 +1877,7 @@ describe('PhishingController', () => { ], }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -1793,7 +1916,7 @@ describe('PhishingController', () => { .get(`${METAMASK_HOTLIST_DIFF_FILE}/${0}`) .reply(404); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -1834,7 +1957,7 @@ describe('PhishingController', () => { ], }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [], }, @@ -1857,7 +1980,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -1908,7 +2031,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -1959,7 +2082,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -2010,7 +2133,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -2052,7 +2175,7 @@ describe('PhishingController', () => { .get(`${C2_DOMAIN_BLOCKLIST_ENDPOINT}?timestamp=0`) .reply(404); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -2104,7 +2227,7 @@ describe('PhishingController', () => { }); // Initialize the controller with an existing state - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -2152,7 +2275,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -2202,7 +2325,7 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -2244,7 +2367,7 @@ describe('PhishingController', () => { .get(`${C2_DOMAIN_BLOCKLIST_ENDPOINT}?timestamp=0`) .replyWithError('network error'); - const controller = getPhishingController({ + const { controller } = getPhishingController({ state: { phishingLists: [ { @@ -2311,9 +2434,12 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - const result = controller.isBlockedRequest('https://example.com'); + const result = rootMessenger.call( + 'PhishingController:isBlockedRequest', + 'https://example.com', + ); expect(result).toMatchObject({ result: false, type: PhishingDetectorResultType.C2DomainBlocklist, @@ -2346,9 +2472,10 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - const result = controller.isBlockedRequest( + const result = rootMessenger.call( + 'PhishingController:isBlockedRequest', 'https://develop.d3bkcslj57l47p.amplifyapp.com', ); expect(result).toMatchObject({ @@ -2382,9 +2509,12 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - const result = controller.isBlockedRequest('https://example.com'); + const result = rootMessenger.call( + 'PhishingController:isBlockedRequest', + 'https://example.com', + ); expect(result).toMatchObject({ result: false, type: PhishingDetectorResultType.C2DomainBlocklist, @@ -2415,9 +2545,12 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - const result = controller.isBlockedRequest('#$@(%&@#$(%'); + const result = rootMessenger.call( + 'PhishingController:isBlockedRequest', + '#$@(%&@#$(%', + ); expect(result).toMatchObject({ result: false, type: PhishingDetectorResultType.C2DomainBlocklist, @@ -2427,9 +2560,13 @@ describe('PhishingController', () => { it('isBlockedRequest - should return false if the URL is in the whitelist', async () => { const whitelistedHostname = 'example.com'; - const controller = getPhishingController(); - controller.bypass(formatHostnameToUrl(whitelistedHostname)); - const result = controller.isBlockedRequest( + const { rootMessenger } = getPhishingController(); + rootMessenger.call( + 'PhishingController:bypass', + formatHostnameToUrl(whitelistedHostname), + ); + const result = rootMessenger.call( + 'PhishingController:isBlockedRequest', `https://${whitelistedHostname}/path`, ); @@ -2465,9 +2602,10 @@ describe('PhishingController', () => { lastFetchedAt: 1, }); - const controller = getPhishingController(); + const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); - const result = controller.isBlockedRequest( + const result = rootMessenger.call( + 'PhishingController:isBlockedRequest', `https://${allowlistedDomain}/path`, ); @@ -2478,35 +2616,40 @@ describe('PhishingController', () => { }); describe('bypass', () => { let controller: PhishingController; + let rootMessenger: RootMessenger; beforeEach(() => { - controller = getPhishingController({ - state: { - phishingLists: [ - { - allowlist: [], - blocklist: [], - c2DomainBlocklist: [], - blocklistPaths: { - 'example.com': { - path: {}, - }, - 'sub.example.com': { - path1: { - path2: {}, + const { controller: createdController, rootMessenger: createdMessenger } = + getPhishingController({ + state: { + phishingLists: [ + { + allowlist: [], + blocklist: [], + c2DomainBlocklist: [], + blocklistPaths: { + 'example.com': { + path: {}, + }, + 'sub.example.com': { + path1: { + path2: {}, + }, }, }, + fuzzylist: [], + tolerance: 0, + version: 0, + lastUpdated: 0, + name: ListNames.MetaMask, }, - fuzzylist: [], - tolerance: 0, - version: 0, - lastUpdated: 0, - name: ListNames.MetaMask, - }, - ], - whitelistPaths: {}, - }, - }); + ], + whitelistPaths: {}, + }, + }); + + controller = createdController; + rootMessenger = createdMessenger; }); describe('whitelist', () => { @@ -2515,8 +2658,8 @@ describe('PhishingController', () => { const hostname = getHostnameFromUrl(origin); // Call the bypass function - controller.bypass(origin); - controller.bypass(origin); + rootMessenger.call('PhishingController:bypass', origin); + rootMessenger.call('PhishingController:bypass', origin); // Verify that the whitelist has not changed expect(controller.state.whitelist).toContain(hostname); @@ -2529,7 +2672,7 @@ describe('PhishingController', () => { const hostname = getHostnameFromUrl(origin); // Call the bypass function - controller.bypass(origin); + rootMessenger.call('PhishingController:bypass', origin); // Verify that the whitelist now includes the new origin expect(controller.state.whitelist).toContain(hostname); @@ -2541,7 +2684,7 @@ describe('PhishingController', () => { const punycodeOrigin = 'xn--fsq.com'; // Example punycode domain // Call the bypass function - controller.bypass(punycodeOrigin); + rootMessenger.call('PhishingController:bypass', punycodeOrigin); // Verify that the whitelist now includes the punycode origin expect(controller.state.whitelist).toContain(punycodeOrigin); @@ -2553,7 +2696,7 @@ describe('PhishingController', () => { describe('whitelistPaths', () => { it('adds the matched path prefix within blocklistPaths to the whitelistPaths', () => { const origin = 'https://sub.example.com/path1/path2/path3'; - controller.bypass(origin); + rootMessenger.call('PhishingController:bypass', origin); expect(controller.state.whitelistPaths).toStrictEqual({ 'sub.example.com': { @@ -2567,7 +2710,7 @@ describe('PhishingController', () => { it('does not add if a matched path prefix is not present', () => { const origin = 'https://sub.example.com/path1/path3'; - controller.bypass(origin); + rootMessenger.call('PhishingController:bypass', origin); expect(controller.state.whitelistPaths).toStrictEqual({}); expect(controller.state.whitelist).toStrictEqual(['sub.example.com']); @@ -2575,8 +2718,8 @@ describe('PhishingController', () => { it('idempotent', () => { const origin = 'https://example.com/path'; - controller.bypass(origin); - controller.bypass(origin); + rootMessenger.call('PhishingController:bypass', origin); + rootMessenger.call('PhishingController:bypass', origin); expect(controller.state.whitelistPaths).toStrictEqual({ 'example.com': { @@ -2588,7 +2731,7 @@ describe('PhishingController', () => { it('if the pathname contains percent encoding, it is added decoded', () => { const origin = 'https://example.com/%70%61%74%68'; - controller.bypass(origin); + rootMessenger.call('PhishingController:bypass', origin); expect(controller.state.whitelistPaths).toStrictEqual({ 'example.com': { @@ -2600,7 +2743,7 @@ describe('PhishingController', () => { }); describe('scanUrl', () => { - let controller: PhishingController; + let rootMessenger: RootMessenger; const testUrl: string = 'https://example.com'; const mockResponse: PhishingDetectionScanResult = { @@ -2609,7 +2752,10 @@ describe('PhishingController', () => { }; beforeEach(() => { - controller = getPhishingController(); + const { rootMessenger: createdMessenger } = getPhishingController(); + + rootMessenger = createdMessenger; + jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); }); @@ -2619,7 +2765,10 @@ describe('PhishingController', () => { .query({ url: 'example.com' }) .reply(200, mockResponse); - const response = await controller.scanUrl(testUrl); + const response = await rootMessenger.call( + 'PhishingController:scanUrl', + testUrl, + ); expect(response).toMatchObject(mockResponse); expect(scope.isDone()).toBe(true); }); @@ -2641,7 +2790,10 @@ describe('PhishingController', () => { .query({ url: 'example.com' }) .reply(statusCode); - const response = await controller.scanUrl(testUrl); + const response = await rootMessenger.call( + 'PhishingController:scanUrl', + testUrl, + ); expect(response).toMatchObject({ hostname: '', recommendedAction: RecommendedAction.None, @@ -2658,7 +2810,7 @@ describe('PhishingController', () => { .delayConnection(10000) .reply(200, {}); - const promise = controller.scanUrl(testUrl); + const promise = rootMessenger.call('PhishingController:scanUrl', testUrl); jest.advanceTimersByTime(8000); const response = await promise; expect(response).toMatchObject({ @@ -2679,7 +2831,10 @@ describe('PhishingController', () => { .query({ url: expectedHostname }) .reply(200, mockResponse); - const response = await controller.scanUrl(urlWithQuery); + const response = await rootMessenger.call( + 'PhishingController:scanUrl', + urlWithQuery, + ); expect(response).toMatchObject(mockResponse); expect(scope.isDone()).toBe(true); }); @@ -2693,7 +2848,10 @@ describe('PhishingController', () => { .query({ url: expectedHostname }) .reply(200, mockResponse); - const response = await controller.scanUrl(urlWithHash); + const response = await rootMessenger.call( + 'PhishingController:scanUrl', + urlWithHash, + ); expect(response).toMatchObject(mockResponse); expect(scope.isDone()).toBe(true); }); @@ -2713,7 +2871,10 @@ describe('PhishingController', () => { .query({ url: expectedHostname }) .reply(200, subdomainResponse); - const response = await controller.scanUrl(complexUrl); + const response = await rootMessenger.call( + 'PhishingController:scanUrl', + complexUrl, + ); expect(response).toMatchObject(subdomainResponse); expect(scope.isDone()).toBe(true); }); @@ -2739,7 +2900,10 @@ describe('PhishingController', () => { ]; for (const invalidUrl of invalidUrls) { - const response = await controller.scanUrl(invalidUrl); + const response = await rootMessenger.call( + 'PhishingController:scanUrl', + invalidUrl, + ); expect(response).toMatchObject({ hostname: '', recommendedAction: RecommendedAction.None, @@ -2757,14 +2921,17 @@ describe('PhishingController', () => { .query({ url: expectedHostname }) .reply(200, mockResponse); - const response = await controller.scanUrl(urlWithAuth); + const response = await rootMessenger.call( + 'PhishingController:scanUrl', + urlWithAuth, + ); expect(response).toMatchObject(mockResponse); expect(scope.isDone()).toBe(true); }); }); describe('bulkScanUrls', () => { - let controller: PhishingController; + let rootMessenger: RootMessenger; const testUrls: string[] = [ 'https://example1.com', @@ -2790,7 +2957,10 @@ describe('PhishingController', () => { }; beforeEach(() => { - controller = getPhishingController(); + const { rootMessenger: createdMessenger } = getPhishingController(); + + rootMessenger = createdMessenger; + jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); }); @@ -2805,13 +2975,19 @@ describe('PhishingController', () => { }) .reply(200, mockResponse); - const response = await controller.bulkScanUrls(testUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + testUrls, + ); expect(response).toStrictEqual(mockResponse); expect(scope.isDone()).toBe(true); }); it('should handle empty URL arrays', async () => { - const response = await controller.bulkScanUrls([]); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + [], + ); expect(response).toStrictEqual({ results: {}, errors: {}, @@ -2820,7 +2996,10 @@ describe('PhishingController', () => { it('should enforce maximum URL limit', async () => { const tooManyUrls = Array(251).fill('https://example.com'); - const response = await controller.bulkScanUrls(tooManyUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + tooManyUrls, + ); expect(response).toStrictEqual({ results: {}, errors: { @@ -2831,7 +3010,10 @@ describe('PhishingController', () => { it('should validate URL length', async () => { const longUrl = `https://example.com/${'a'.repeat(2048)}`; - const response = await controller.bulkScanUrls([longUrl]); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + [longUrl], + ); expect(response).toStrictEqual({ results: {}, errors: { @@ -2858,7 +3040,10 @@ describe('PhishingController', () => { }) .reply(statusCode); - const response = await controller.bulkScanUrls(testUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + testUrls, + ); expect(response).toStrictEqual({ results: {}, errors: { @@ -2877,7 +3062,10 @@ describe('PhishingController', () => { .delayConnection(20000) .reply(200, {}); - const promise = controller.bulkScanUrls(testUrls); + const promise = rootMessenger.call( + 'PhishingController:bulkScanUrls', + testUrls, + ); jest.advanceTimersByTime(15000); const response = await promise; expect(response).toStrictEqual({ @@ -2963,7 +3151,10 @@ describe('PhishingController', () => { }) .reply(200, mockBatch3Response); - const response = await controller.bulkScanUrls(manyUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + manyUrls, + ); // Verify all scopes were called expect(scope1.isDone()).toBe(true); @@ -3001,7 +3192,10 @@ describe('PhishingController', () => { }) .reply(200, mixedResponse); - const response = await controller.bulkScanUrls(testUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + testUrls, + ); expect(response).toStrictEqual(mixedResponse); expect(scope.isDone()).toBe(true); }); @@ -3031,7 +3225,10 @@ describe('PhishingController', () => { }) .reply(500, { error: 'Internal Server Error' }); - const response = await controller.bulkScanUrls(manyUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + manyUrls, + ); expect(scope1.isDone()).toBe(true); expect(scope2.isDone()).toBe(true); @@ -3065,7 +3262,7 @@ describe('PhishingController', () => { recommendedAction: RecommendedAction.None, }); - await controller.scanUrl(cachedUrl); + await rootMessenger.call('PhishingController:scanUrl', cachedUrl); // Now set up the mock for the bulk API call with only the uncached URL const expectedPostBody = { @@ -3087,7 +3284,10 @@ describe('PhishingController', () => { .reply(200, bulkApiResponse); // Call bulkScanUrls with both URLs - const response = await controller.bulkScanUrls(mixedUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + mixedUrls, + ); // Verify that only the uncached URL was requested from the API expect(scope.isDone()).toBe(true); @@ -3099,7 +3299,10 @@ describe('PhishingController', () => { }); // Verify the newly fetched result is now in the cache - const newlyCachedResult = await controller.scanUrl(uncachedUrl); + const newlyCachedResult = await rootMessenger.call( + 'PhishingController:scanUrl', + uncachedUrl, + ); expect(newlyCachedResult).toStrictEqual( bulkApiResponse.results[uncachedUrl], ); @@ -3130,7 +3333,10 @@ describe('PhishingController', () => { .reply(200, bulkApiResponse); // Call bulkScanUrls with both URLs - const response = await controller.bulkScanUrls(mixedUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + mixedUrls, + ); // Verify that only the valid URL was requested from the API expect(scope.isDone()).toBe(true); @@ -3144,7 +3350,10 @@ describe('PhishingController', () => { ); // Verify the valid result is now in the cache - const cachedResult = await controller.scanUrl(validUrl); + const cachedResult = await rootMessenger.call( + 'PhishingController:scanUrl', + validUrl, + ); expect(cachedResult).toStrictEqual(bulkApiResponse.results[validUrl]); // Should not make a new API call for the cached URL @@ -3188,11 +3397,14 @@ describe('PhishingController', () => { }); // Cache the results - await controller.scanUrl(cachedUrls[0]); - await controller.scanUrl(cachedUrls[1]); + await rootMessenger.call('PhishingController:scanUrl', cachedUrls[0]); + await rootMessenger.call('PhishingController:scanUrl', cachedUrls[1]); // No API call should be made for bulkScanUrls - const response = await controller.bulkScanUrls(cachedUrls); + const response = await rootMessenger.call( + 'PhishingController:bulkScanUrls', + cachedUrls, + ); // Verify we got the results from cache expect(response.results[cachedUrls[0]]).toStrictEqual(cachedResults[0]); @@ -3205,7 +3417,7 @@ describe('PhishingController', () => { }); describe('scanAddress', () => { - let controller: PhishingController; + let rootMessenger: RootMessenger; const testChainId = '0x1'; const testAddress = '0x1234567890123456789012345678901234567890'; @@ -3215,7 +3427,10 @@ describe('PhishingController', () => { }; beforeEach(() => { - controller = getPhishingController(); + const { rootMessenger: createdMessenger } = getPhishingController(); + + rootMessenger = createdMessenger; + jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); }); @@ -3231,7 +3446,11 @@ describe('PhishingController', () => { }) .reply(200, mockResponse); - const response = await controller.scanAddress(testChainId, testAddress); + const response = await rootMessenger.call( + 'PhishingController:scanAddress', + testChainId, + testAddress, + ); expect(response).toMatchObject(mockResponse); expect(scope.isDone()).toBe(true); }); @@ -3255,7 +3474,11 @@ describe('PhishingController', () => { }) .reply(statusCode); - const response = await controller.scanAddress(testChainId, testAddress); + const response = await rootMessenger.call( + 'PhishingController:scanAddress', + testChainId, + testAddress, + ); expect(response).toMatchObject({ result_type: AddressScanResultType.ErrorResult, label: '', @@ -3273,7 +3496,11 @@ describe('PhishingController', () => { .delayConnection(10000) .reply(200, {}); - const promise = controller.scanAddress(testChainId, testAddress); + const promise = rootMessenger.call( + 'PhishingController:scanAddress', + testChainId, + testAddress, + ); jest.advanceTimersByTime(5000); const response = await promise; expect(response).toMatchObject({ @@ -3284,7 +3511,11 @@ describe('PhishingController', () => { }); it('will return an AddressScanResult with an ErrorResult when address is missing', async () => { - const response = await controller.scanAddress(testChainId, ''); + const response = await rootMessenger.call( + 'PhishingController:scanAddress', + testChainId, + '', + ); expect(response).toMatchObject({ result_type: AddressScanResultType.ErrorResult, label: '', @@ -3293,7 +3524,8 @@ describe('PhishingController', () => { it('will return an AddressScanResult with an ErrorResult when chain ID is unknown', async () => { const unknownChainId = '0x999999'; - const response = await controller.scanAddress( + const response = await rootMessenger.call( + 'PhishingController:scanAddress', unknownChainId, testAddress, ); @@ -3312,7 +3544,8 @@ describe('PhishingController', () => { }) .reply(200, mockResponse); - const response = await controller.scanAddress( + const response = await rootMessenger.call( + 'PhishingController:scanAddress', testChainId, mixedCaseAddress, ); @@ -3329,7 +3562,8 @@ describe('PhishingController', () => { }) .reply(200, mockResponse); - const response = await controller.scanAddress( + const response = await rootMessenger.call( + 'PhishingController:scanAddress', mixedCaseChainId, testAddress, ); @@ -3347,10 +3581,18 @@ describe('PhishingController', () => { }) .reply(200, mockResponse); - const result1 = await controller.scanAddress(testChainId, testAddress); + const result1 = await rootMessenger.call( + 'PhishingController:scanAddress', + testChainId, + testAddress, + ); expect(result1).toMatchObject(mockResponse); - const result2 = await controller.scanAddress(testChainId, testAddress); + const result2 = await rootMessenger.call( + 'PhishingController:scanAddress', + testChainId, + testAddress, + ); expect(result2).toMatchObject(mockResponse); expect(fetchSpy).toHaveBeenCalledTimes(1); @@ -3387,16 +3629,32 @@ describe('PhishingController', () => { }) .reply(200, mockResponse2); - const result1 = await controller.scanAddress(chainId1, testAddress); - const result2 = await controller.scanAddress(chainId2, testAddress); + const result1 = await rootMessenger.call( + 'PhishingController:scanAddress', + chainId1, + testAddress, + ); + const result2 = await rootMessenger.call( + 'PhishingController:scanAddress', + chainId2, + testAddress, + ); expect(result1).toMatchObject(mockResponse1); expect(result2).toMatchObject(mockResponse2); expect(scope1.isDone()).toBe(true); expect(scope2.isDone()).toBe(true); - const cachedResult1 = await controller.scanAddress(chainId1, testAddress); - const cachedResult2 = await controller.scanAddress(chainId2, testAddress); + const cachedResult1 = await rootMessenger.call( + 'PhishingController:scanAddress', + chainId1, + testAddress, + ); + const cachedResult2 = await rootMessenger.call( + 'PhishingController:scanAddress', + chainId2, + testAddress, + ); expect(cachedResult1).toMatchObject(mockResponse1); expect(cachedResult2).toMatchObject(mockResponse2); @@ -3429,15 +3687,21 @@ describe('URL Scan Cache', () => { recommendedAction: RecommendedAction.None, }); - const controller = getPhishingController(); + const { rootMessenger } = getPhishingController(); - const result1 = await controller.scanUrl(`https://${testDomain}`); + const result1 = await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(result1).toStrictEqual({ hostname: testDomain, recommendedAction: RecommendedAction.None, }); - const result2 = await controller.scanUrl(`https://${testDomain}`); + const result2 = await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(result2).toStrictEqual({ hostname: testDomain, recommendedAction: RecommendedAction.None, @@ -3471,20 +3735,29 @@ describe('URL Scan Cache', () => { recommendedAction: RecommendedAction.None, }); - const controller = getPhishingController({ + const { rootMessenger } = getPhishingController({ urlScanCacheTTL: cacheTTL, }); - await controller.scanUrl(`https://${testDomain}`); + await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); // Before TTL expires, should use cache jest.advanceTimersByTime((cacheTTL - 10) * 1000); - await controller.scanUrl(`https://${testDomain}`); + await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(pendingMocks()).toHaveLength(1); // One mock remaining // After TTL expires, should fetch again jest.advanceTimersByTime(11 * 1000); - await controller.scanUrl(`https://${testDomain}`); + await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(pendingMocks()).toHaveLength(0); // All mocks used }); @@ -3516,21 +3789,33 @@ describe('URL Scan Cache', () => { recommendedAction: RecommendedAction.Warn, }); - const controller = getPhishingController({ + const { rootMessenger } = getPhishingController({ urlScanCacheMaxSize: maxCacheSize, }); // Fill the cache - await controller.scanUrl(`https://${domains[0]}`); + await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${domains[0]}`, + ); jest.advanceTimersByTime(1000); // Ensure different timestamps - await controller.scanUrl(`https://${domains[1]}`); + await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${domains[1]}`, + ); // This should evict the oldest entry (domain1) jest.advanceTimersByTime(1000); - await controller.scanUrl(`https://${domains[2]}`); + await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${domains[2]}`, + ); // Now domain1 should not be in cache and require a new fetch - await controller.scanUrl(`https://${domains[0]}`); + await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${domains[0]}`, + ); // All mocks should be used expect(isDone()).toBe(true); @@ -3555,14 +3840,20 @@ describe('URL Scan Cache', () => { recommendedAction: RecommendedAction.None, }); - const controller = getPhishingController(); + const { rootMessenger } = getPhishingController(); // First call should result in an error response - const result1 = await controller.scanUrl(`https://${testDomain}`); + const result1 = await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(result1.fetchError).toBeDefined(); // Second call should try again (not use cache since errors aren't cached) - const result2 = await controller.scanUrl(`https://${testDomain}`); + const result2 = await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(result2.fetchError).toBeUndefined(); expect(result2.recommendedAction).toBe(RecommendedAction.None); @@ -3590,14 +3881,20 @@ describe('URL Scan Cache', () => { recommendedAction: RecommendedAction.None, }); - const controller = getPhishingController(); + const { rootMessenger } = getPhishingController(); // First call should result in an error - const result1 = await controller.scanUrl(`https://${testDomain}`); + const result1 = await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(result1.fetchError).toBeDefined(); // Second call should succeed (not use cache since errors aren't cached) - const result2 = await controller.scanUrl(`https://${testDomain}`); + const result2 = await rootMessenger.call( + 'PhishingController:scanUrl', + `https://${testDomain}`, + ); expect(result2.fetchError).toBeUndefined(); expect(result2.recommendedAction).toBe(RecommendedAction.None); @@ -3608,20 +3905,26 @@ describe('URL Scan Cache', () => { it('should handle invalid URLs and not cache them', async () => { const invalidUrl = 'not-a-valid-url'; - const controller = getPhishingController(); + const { rootMessenger } = getPhishingController(); // First call should return an error for invalid URL - const result1 = await controller.scanUrl(invalidUrl); + const result1 = await rootMessenger.call( + 'PhishingController:scanUrl', + invalidUrl, + ); expect(result1.fetchError).toBeDefined(); // Second call should also return an error (not from cache) - const result2 = await controller.scanUrl(invalidUrl); + const result2 = await rootMessenger.call( + 'PhishingController:scanUrl', + invalidUrl, + ); expect(result2.fetchError).toBeDefined(); }); describe('metadata', () => { it('includes expected state in debug snapshots', () => { - const controller = getPhishingController(); + const { controller } = getPhishingController(); expect( deriveStateFromMetadata( @@ -3633,7 +3936,7 @@ describe('URL Scan Cache', () => { }); it('includes expected state in state logs', () => { - const controller = getPhishingController(); + const { controller } = getPhishingController(); expect( deriveStateFromMetadata( @@ -3651,7 +3954,7 @@ describe('URL Scan Cache', () => { }); it('persists expected state', () => { - const controller = getPhishingController(); + const { controller } = getPhishingController(); expect( deriveStateFromMetadata( @@ -3675,7 +3978,7 @@ describe('URL Scan Cache', () => { }); it('includes expected state in UI', () => { - const controller = getPhishingController(); + const { controller } = getPhishingController(); expect( deriveStateFromMetadata( diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index e7f9354412e..5c2be4ba826 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -20,6 +20,11 @@ import { CacheManager } from './CacheManager'; import type { CacheEntry } from './CacheManager'; import { convertListToTrie, insertToTrie, matchedPathPrefix } from './PathTrie'; import type { PathTrie } from './PathTrie'; +import type { + PhishingControllerMaybeUpdateStateAction, + PhishingControllerMethodActions, + PhishingControllerTestAction, +} from './PhishingController-method-action-types'; import { PhishingDetector } from './PhishingDetector'; import { PhishingDetectorResultType, @@ -374,30 +379,26 @@ export type PhishingControllerOptions = { state?: Partial; }; -export type MaybeUpdateState = { - type: `${typeof controllerName}:maybeUpdateState`; - handler: PhishingController['maybeUpdateState']; -}; +const MESSENGER_EXPOSED_METHODS = [ + 'maybeUpdateState', + 'test', + 'isBlockedRequest', + 'bypass', + 'scanUrl', + 'bulkScanUrls', + 'bulkScanTokens', + 'scanAddress', +] as const; -export type TestOrigin = { - type: `${typeof controllerName}:testOrigin`; - handler: PhishingController['test']; -}; - -export type PhishingControllerBulkScanUrlsAction = { - type: `${typeof controllerName}:bulkScanUrls`; - handler: PhishingController['bulkScanUrls']; -}; - -export type PhishingControllerBulkScanTokensAction = { - type: `${typeof controllerName}:bulkScanTokens`; - handler: PhishingController['bulkScanTokens']; -}; +/** + * @deprecated Use `PhishingControllerTestAction` instead. + */ +export type TestOrigin = PhishingControllerTestAction; -export type PhishingControllerScanAddressAction = { - type: `${typeof controllerName}:scanAddress`; - handler: PhishingController['scanAddress']; -}; +/** + * @deprecated Use `PhishingControllerMaybeUpdateStateAction` instead. + */ +export type MaybeUpdateState = PhishingControllerMaybeUpdateStateAction; export type PhishingControllerGetStateAction = ControllerGetStateAction< typeof controllerName, @@ -406,11 +407,7 @@ export type PhishingControllerGetStateAction = ControllerGetStateAction< export type PhishingControllerActions = | PhishingControllerGetStateAction - | MaybeUpdateState - | TestOrigin - | PhishingControllerBulkScanUrlsAction - | PhishingControllerBulkScanTokensAction - | PhishingControllerScanAddressAction; + | PhishingControllerMethodActions; export type PhishingControllerStateChangeEvent = ControllerStateChangeEvent< typeof controllerName, @@ -558,7 +555,10 @@ export class PhishingController extends BaseController< }, }); - this.#registerMessageHandlers(); + this.messenger.registerMethodActionHandlers( + this, + MESSENGER_EXPOSED_METHODS, + ); this.updatePhishingDetector(); this.#subscribeToTransactionControllerStateChange(); @@ -571,37 +571,6 @@ export class PhishingController extends BaseController< ); } - /** - * Constructor helper for registering this controller's messaging system - * actions. - */ - #registerMessageHandlers(): void { - this.messenger.registerActionHandler( - `${controllerName}:maybeUpdateState` as const, - this.maybeUpdateState.bind(this), - ); - - this.messenger.registerActionHandler( - `${controllerName}:testOrigin` as const, - this.test.bind(this), - ); - - this.messenger.registerActionHandler( - `${controllerName}:bulkScanUrls` as const, - this.bulkScanUrls.bind(this), - ); - - this.messenger.registerActionHandler( - `${controllerName}:bulkScanTokens` as const, - this.bulkScanTokens.bind(this), - ); - - this.messenger.registerActionHandler( - `${controllerName}:scanAddress` as const, - this.scanAddress.bind(this), - ); - } - /** * Checks if a patch represents a transaction-level change or nested transaction property change * @@ -930,7 +899,7 @@ export class PhishingController extends BaseController< * @param url - The URL to scan. * @returns The phishing detection scan result. */ - scanUrl = async (url: string): Promise => { + async scanUrl(url: string): Promise { const [hostname, ok] = getHostnameFromWebUrl(url); if (!ok) { return { @@ -991,7 +960,7 @@ export class PhishingController extends BaseController< this.#urlScanCache.set(hostname, result); return result; - }; + } /** * Scan multiple URLs for phishing in bulk. It will only scan the hostnames of the URLs. @@ -1000,9 +969,9 @@ export class PhishingController extends BaseController< * @param urls - The URLs to scan. * @returns A mapping of URLs to their phishing detection scan results and errors. */ - bulkScanUrls = async ( + async bulkScanUrls( urls: string[], - ): Promise => { + ): Promise { if (!urls || urls.length === 0) { return { results: {}, @@ -1095,7 +1064,7 @@ export class PhishingController extends BaseController< } return combinedResponse; - }; + } /** * Fetch bulk token scan results from the security alerts API. @@ -1167,10 +1136,10 @@ export class PhishingController extends BaseController< * @param address - The address to scan. * @returns The address scan result. */ - scanAddress = async ( + async scanAddress( chainId: string, address: string, - ): Promise => { + ): Promise { if (!address || !chainId) { return { result_type: AddressScanResultType.ErrorResult, @@ -1249,7 +1218,7 @@ export class PhishingController extends BaseController< result_type: apiResponse.result_type, label: apiResponse.label, }; - }; + } /** * Scan multiple tokens for malicious activity in bulk. @@ -1263,9 +1232,9 @@ export class PhishingController extends BaseController< * addresses are lowercased; for non-EVM chains, original casing is preserved. * Tokens that fail to scan are omitted. */ - bulkScanTokens = async ( + async bulkScanTokens( request: BulkTokenScanRequest, - ): Promise => { + ): Promise { const { chainId, tokens } = request; if (!tokens || tokens.length === 0) { @@ -1340,7 +1309,7 @@ export class PhishingController extends BaseController< } return results; - }; + } /** * Process a batch of URLs (up to 50) for phishing detection. diff --git a/packages/phishing-controller/src/index.ts b/packages/phishing-controller/src/index.ts index 33b89b6e06c..96b33977591 100644 --- a/packages/phishing-controller/src/index.ts +++ b/packages/phishing-controller/src/index.ts @@ -20,3 +20,14 @@ export { AddressScanResultType, } from './types'; export type { CacheEntry } from './CacheManager'; + +export type { + PhishingControllerMaybeUpdateStateAction, + PhishingControllerTestAction, + PhishingControllerIsBlockedRequestAction, + PhishingControllerBypassAction, + PhishingControllerScanUrlAction, + PhishingControllerBulkScanUrlsAction, + PhishingControllerBulkScanTokensAction, + PhishingControllerScanAddressAction, +} from './PhishingController-method-action-types'; diff --git a/yarn.lock b/yarn.lock index ef25e8876a6..12fee5c87da 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4779,6 +4779,7 @@ __metadata: nock: "npm:^13.3.1" punycode: "npm:^2.1.1" ts-jest: "npm:^29.2.5" + tsx: "npm:^4.20.5" typedoc: "npm:^0.25.13" typedoc-plugin-missing-exports: "npm:^2.0.0" typescript: "npm:~5.3.3" From b76af8968ebd62c4598f1ae0470b6098526bf533 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Mon, 23 Mar 2026 11:51:01 +0100 Subject: [PATCH 2/5] update CHANGELOG --- packages/phishing-controller/CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index b65dccefbf1..17d5b212412 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -7,8 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Expose missing public `PhishingController` methods through its messenger ([#8269](https://github.com/MetaMask/core/pull/8269)) + - The following actions are now available: + - `PhishingController:bypass` + - `PhishingController:isBlockedRequest` + - `PhishingController:scanUrl` + - Corresponding action types (e.g. `PhishingControllerBypassAction`) are available as well. + ### Changed +- Deprecate action types in favor of `PhishingController...Action` types ([#8269](https://github.com/MetaMask/core/pull/8269)) + - The following action types have been renamed: + - `TestOrigin` is now `PhishingControllerTestAction`. + - `MaybeUpdateState` is now `PhishingControllerMaybeUpdateStateAction`. + - The old types are still exported but are now marked as deprecated and will + be removed in a future release. - `PhishingController` no longer advances `c2DomainBlocklistLastFetched` when the C2 domain blocklist fetch fails, allowing the blocklist to be retried on the next update cycle ([#8250](https://github.com/MetaMask/core/pull/8250)) - Reduce default cache TTL for `DEFAULT_URL_SCAN_CACHE_TTL`, `DEFAULT_TOKEN_SCAN_CACHE_TTL`, and `DEFAULT_ADDRESS_SCAN_CACHE_TTL` from 15 minutes to 1 minute ([#8254](https://github.com/MetaMask/core/pull/8254)) - Bump `@metamask/transaction-controller` from `^63.0.0` to `^63.1.0` ([#8272](https://github.com/MetaMask/core/pull/8272)) From 6c01f4bfb86aa6d2c3588bcae579a51c73a34251 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Mon, 23 Mar 2026 12:10:03 +0100 Subject: [PATCH 3/5] avoid breaking `testOrigin` action --- packages/phishing-controller/CHANGELOG.md | 5 +- .../PhishingController-method-action-types.ts | 8 +-- .../src/PhishingController.test.ts | 68 ++++++++++--------- .../src/PhishingController.ts | 23 +++++-- packages/phishing-controller/src/index.ts | 2 +- 5 files changed, 63 insertions(+), 43 deletions(-) diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index 17d5b212412..5d53327788d 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -18,9 +18,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Deprecate `test` method in favor of `testOrigin` ([#8269](https://github.com/MetaMask/core/pull/8269)) + - The `test` method is now renamed to `testOrigin` to better reflect its purpose of testing a domain origin for phishing. + - The old `test` method is still present but is now marked as deprecated and will be removed in a future release. - Deprecate action types in favor of `PhishingController...Action` types ([#8269](https://github.com/MetaMask/core/pull/8269)) - The following action types have been renamed: - - `TestOrigin` is now `PhishingControllerTestAction`. + - `TestOrigin` is now `PhishingControllerTestOriginAction`. - `MaybeUpdateState` is now `PhishingControllerMaybeUpdateStateAction`. - The old types are still exported but are now marked as deprecated and will be removed in a future release. diff --git a/packages/phishing-controller/src/PhishingController-method-action-types.ts b/packages/phishing-controller/src/PhishingController-method-action-types.ts index 7daef1c465b..fd525534c2f 100644 --- a/packages/phishing-controller/src/PhishingController-method-action-types.ts +++ b/packages/phishing-controller/src/PhishingController-method-action-types.ts @@ -28,9 +28,9 @@ export type PhishingControllerMaybeUpdateStateAction = { * @param origin - Domain origin of a website. * @returns Whether the origin is an unapproved origin. */ -export type PhishingControllerTestAction = { - type: `PhishingController:test`; - handler: PhishingController['test']; +export type PhishingControllerTestOriginAction = { + type: `PhishingController:testOrigin`; + handler: PhishingController['testOrigin']; }; /** @@ -116,7 +116,7 @@ export type PhishingControllerBulkScanTokensAction = { */ export type PhishingControllerMethodActions = | PhishingControllerMaybeUpdateStateAction - | PhishingControllerTestAction + | PhishingControllerTestOriginAction | PhishingControllerIsBlockedRequestAction | PhishingControllerBypassAction | PhishingControllerScanUrlAction diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index 0aa16fee1c1..b53a9f7ad70 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -141,7 +141,7 @@ describe('PhishingController', () => { formatHostnameToUrl(whitelistedHostname), ); const result = rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', whitelistedHostname, ); @@ -159,7 +159,7 @@ describe('PhishingController', () => { formatHostnameToUrl(whitelistedHostname), ); const result = rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', `https://${whitelistedHostname}/path`, ); @@ -175,7 +175,7 @@ describe('PhishingController', () => { const { rootMessenger } = getPhishingController(); rootMessenger.call('PhishingController:bypass', whitelistedURL); const result = rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', whitelistedURL, ); expect(result).toMatchObject({ @@ -214,7 +214,7 @@ describe('PhishingController', () => { const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); const result = rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', `https://${allowlistedHostname}/path`, ); @@ -390,7 +390,7 @@ describe('PhishingController', () => { await rootMessenger.call('PhishingController:maybeUpdateState'); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('this-should-not-be-in-default-blocklist.com'), ), ).toMatchObject({ @@ -400,7 +400,7 @@ describe('PhishingController', () => { expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('this-should-not-be-in-default-allowlist.com'), ), ).toMatchObject({ @@ -413,7 +413,7 @@ describe('PhishingController', () => { expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('this-should-not-be-in-default-blocklist.com'), ), ).toMatchObject({ @@ -423,7 +423,7 @@ describe('PhishingController', () => { expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('this-should-not-be-in-default-allowlist.com'), ), ).toMatchObject({ @@ -841,7 +841,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('metamask.io'), ), ).toMatchObject({ @@ -872,7 +872,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('i❤.ws'), ), ).toMatchObject({ @@ -902,7 +902,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('xn--i-7iq.ws'), ), ).toMatchObject({ @@ -940,7 +940,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('etnerscan.io'), ), ).toMatchObject({ @@ -979,7 +979,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('myetherẉalletṭ.com'), ), ).toMatchObject({ @@ -1018,7 +1018,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('xn--myetherallet-4k5fwn.com'), ), ).toMatchObject({ @@ -1065,7 +1065,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('e4d600ab9141b7a9859511c77e63b9b3.com'), ), ).toMatchObject({ @@ -1096,7 +1096,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('e4d600ab9141b7a9859511c77e63b9b3.com'), ), ).toMatchObject({ @@ -1134,7 +1134,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('opensea.io'), ), ).toMatchObject({ @@ -1173,7 +1173,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('ohpensea.io'), ), ).toMatchObject({ @@ -1203,7 +1203,7 @@ describe('PhishingController', () => { await controller.updateStalelist(); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl('this-is-the-official-website-of-opensea.io'), ), ).toMatchObject({ @@ -1242,7 +1242,7 @@ describe('PhishingController', () => { const unsafeDomain = 'electrum.mx'; assert.equal( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ).result, true, @@ -1254,7 +1254,7 @@ describe('PhishingController', () => { ); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ), ).toMatchObject({ @@ -1293,7 +1293,7 @@ describe('PhishingController', () => { const unsafeDomain = 'electrum.mx'; assert.equal( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ).result, true, @@ -1309,7 +1309,7 @@ describe('PhishingController', () => { ); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ), ).toMatchObject({ @@ -1348,7 +1348,7 @@ describe('PhishingController', () => { const unsafeDomain = 'myetherẉalletṭ.com'; assert.equal( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ).result, true, @@ -1360,7 +1360,7 @@ describe('PhishingController', () => { ); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ), ).toMatchObject({ @@ -1399,7 +1399,7 @@ describe('PhishingController', () => { const unsafeDomain = 'xn--myetherallet-4k5fwn.com'; assert.equal( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ).result, true, @@ -1411,7 +1411,7 @@ describe('PhishingController', () => { ); expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', formatHostnameToUrl(unsafeDomain), ), ).toMatchObject({ @@ -1448,7 +1448,10 @@ describe('PhishingController', () => { const { controller, rootMessenger } = getPhishingController(); await controller.updateStalelist(); expect( - rootMessenger.call('PhishingController:test', 'https://example.com/path'), + rootMessenger.call( + 'PhishingController:testOrigin', + 'https://example.com/path', + ), ).toMatchObject({ result: true, type: PhishingDetectorResultType.Blocklist, @@ -1479,7 +1482,10 @@ describe('PhishingController', () => { }); rootMessenger.call('PhishingController:bypass', 'https://example.com/path'); expect( - rootMessenger.call('PhishingController:test', 'https://example.com/path'), + rootMessenger.call( + 'PhishingController:testOrigin', + 'https://example.com/path', + ), ).toMatchObject({ result: false, type: PhishingDetectorResultType.All, @@ -1511,7 +1517,7 @@ describe('PhishingController', () => { expect( rootMessenger.call( - 'PhishingController:test', + 'PhishingController:testOrigin', 'https://example.com/%70%61%74%68', ), ).toMatchObject({ @@ -1770,7 +1776,6 @@ describe('PhishingController', () => { lastUpdated: 0, }, ]); - expect(controller.state.c2DomainBlocklistLastFetched).toBe(0); }); it('should not throw when there is a network error', async () => { @@ -1787,7 +1792,6 @@ describe('PhishingController', () => { const { controller } = getPhishingController(); expect(await controller.updateStalelist()).toBeUndefined(); - expect(controller.state.c2DomainBlocklistLastFetched).toBe(0); }); describe('an update is in progress', () => { diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index 5c2be4ba826..4c03ef5b87c 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -23,7 +23,7 @@ import type { PathTrie } from './PathTrie'; import type { PhishingControllerMaybeUpdateStateAction, PhishingControllerMethodActions, - PhishingControllerTestAction, + PhishingControllerTestOriginAction, } from './PhishingController-method-action-types'; import { PhishingDetector } from './PhishingDetector'; import { @@ -381,7 +381,7 @@ export type PhishingControllerOptions = { const MESSENGER_EXPOSED_METHODS = [ 'maybeUpdateState', - 'test', + 'testOrigin', 'isBlockedRequest', 'bypass', 'scanUrl', @@ -391,9 +391,9 @@ const MESSENGER_EXPOSED_METHODS = [ ] as const; /** - * @deprecated Use `PhishingControllerTestAction` instead. + * @deprecated Use `PhishingControllerTestOriginAction` instead. */ -export type TestOrigin = PhishingControllerTestAction; +export type TestOrigin = PhishingControllerTestOriginAction; /** * @deprecated Use `PhishingControllerMaybeUpdateStateAction` instead. @@ -767,7 +767,7 @@ export class PhishingController extends BaseController< * @param origin - Domain origin of a website. * @returns Whether the origin is an unapproved origin. */ - test(origin: string): PhishingDetectorResult { + testOrigin(origin: string): PhishingDetectorResult { const punycodeOrigin = toASCII(origin); const hostname = getHostnameFromUrl(punycodeOrigin); const hostnameWithPaths = hostname + getPathnameFromUrl(origin); @@ -782,6 +782,19 @@ export class PhishingController extends BaseController< return this.#detector.check(punycodeOrigin); } + /** + * Determines if a given origin is unapproved. + * + * It is strongly recommended that you call {@link maybeUpdateState} before calling this, + * to check whether the phishing configuration is up-to-date. It will be updated if necessary + * by calling {@link updateStalelist} or {@link updateHotlist}. + * + * @param origin - Domain origin of a website. + * @returns Whether the origin is an unapproved origin. + * @deprecated Use {@link testOrigin} instead. This method is exposed for backward compatibility and will be removed in a future release. + */ + test = this.testOrigin.bind(this); + /** * Checks if a request URL's domain is blocked against the request blocklist. * diff --git a/packages/phishing-controller/src/index.ts b/packages/phishing-controller/src/index.ts index 96b33977591..cbde12f379a 100644 --- a/packages/phishing-controller/src/index.ts +++ b/packages/phishing-controller/src/index.ts @@ -23,7 +23,7 @@ export type { CacheEntry } from './CacheManager'; export type { PhishingControllerMaybeUpdateStateAction, - PhishingControllerTestAction, + PhishingControllerTestOriginAction, PhishingControllerIsBlockedRequestAction, PhishingControllerBypassAction, PhishingControllerScanUrlAction, From 90e3e1ee36ee9ecbafb5547de347f433c9c84a10 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Mon, 23 Mar 2026 12:19:57 +0100 Subject: [PATCH 4/5] prune eslint suppressions --- eslint-suppressions.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index e4558240f7d..e19a86b5ed4 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1261,9 +1261,6 @@ } }, "packages/phishing-controller/src/PhishingController.test.ts": { - "@typescript-eslint/explicit-function-return-type": { - "count": 1 - }, "jest/unbound-method": { "count": 7 } From 053e94ec092f4e2a5468b696097e860f87445b92 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Tue, 24 Mar 2026 16:17:14 +0100 Subject: [PATCH 5/5] update action types file after rebase --- .../src/PhishingController-method-action-types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/phishing-controller/src/PhishingController-method-action-types.ts b/packages/phishing-controller/src/PhishingController-method-action-types.ts index fd525534c2f..27d032367e6 100644 --- a/packages/phishing-controller/src/PhishingController-method-action-types.ts +++ b/packages/phishing-controller/src/PhishingController-method-action-types.ts @@ -1,5 +1,5 @@ /** - * This file is auto generated by `scripts/generate-method-action-types.ts`. + * This file is auto generated. * Do not edit manually. */