diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c32201a64ed..bc1d68890ba 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1559,16 +1559,6 @@ "count": 1 } }, - "packages/phishing-controller/src/CacheManager.test.ts": { - "@typescript-eslint/explicit-function-return-type": { - "count": 2 - } - }, - "packages/phishing-controller/src/CacheManager.ts": { - "@typescript-eslint/naming-convention": { - "count": 3 - } - }, "packages/phishing-controller/src/PathTrie.ts": { "@typescript-eslint/explicit-function-return-type": { "count": 2 @@ -1576,10 +1566,7 @@ }, "packages/phishing-controller/src/PhishingController.ts": { "@typescript-eslint/explicit-function-return-type": { - "count": 14 - }, - "@typescript-eslint/naming-convention": { - "count": 1 + "count": 11 }, "@typescript-eslint/prefer-nullish-coalescing": { "count": 6 @@ -1610,7 +1597,7 @@ }, "packages/phishing-controller/src/utils.ts": { "@typescript-eslint/explicit-function-return-type": { - "count": 5 + "count": 4 }, "@typescript-eslint/prefer-nullish-coalescing": { "count": 1 @@ -2343,4 +2330,4 @@ "count": 10 } } -} +} \ No newline at end of file diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index 66ab0fe4ea1..32d4547c1a5 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -7,10 +7,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `PhishingDataService`, a `BaseDataService` subclass that now performs all network requests for `PhishingController` (stalelist, hotlist diffs, C2 domain blocklist, URL/token/address scans, and approvals) + - Exposes the messenger actions `PhishingDataService:getStalelist`, `PhishingDataService:getHotlistDiffs`, `PhishingDataService:getC2DomainBlocklist`, `PhishingDataService:scanUrl`, `PhishingDataService:bulkScanUrls`, `PhishingDataService:scanToken`, `PhishingDataService:bulkScanTokens`, `PhishingDataService:scanAddress`, and `PhishingDataService:getApprovals`, making query results available to the UI via `@metamask/react-data-query` + - Requests are wrapped in a shared retry policy, configurable via the `policyOptions` constructor option; circuit breaking is disabled by default because the service spans four independent API hosts and a broken circuit caused by one host would pause phishing-list updates from the others + - Scan results are cached per URL hostname, token, and address for `SCAN_RESULT_STALE_TIME` (1 minute, matching the previous cache TTLs); bulk scans only request items without a fresh cached result and coalesce them into batched API calls (up to 50 URLs / 100 tokens per request), and single and bulk URL scans share cache entries; approvals are never cached + - The query cache is persisted between sessions by default (`persistenceConfig`, max age 5 minutes), which requires the `StorageService:setItem`, `StorageService:getItem`, and `StorageService:removeItem` messenger actions and an `init` call during client initialization (automatic with `@metamask/wallet`); pass `persistenceConfig: null` to disable + +- Export the `resolveChainName` utility, which maps chain IDs to the chain names used in scan query keys, enabling UI consumers to construct `PhishingDataService` query keys + ### Changed +- **BREAKING:** `PhishingController` no longer performs network requests directly; a `PhishingDataService` must be registered and its method actions delegated to the controller's messenger + - `PhishingControllerMessenger` now requires the `PhishingDataService` method actions listed above as allowed actions +- **BREAKING:** Remove the `urlScanCache`, `tokenScanCache`, and `addressScanCache` properties from `PhishingControllerState`; scan results are now cached (and persisted) by `PhishingDataService`'s query cache + - Client state migrations should remove these properties from persisted `PhishingController` state +- **BREAKING:** Remove the `urlScanCacheTTL`, `urlScanCacheMaxSize`, `tokenScanCacheTTL`, `tokenScanCacheMaxSize`, `addressScanCacheTTL`, and `addressScanCacheMaxSize` options from `PhishingControllerOptions`; scan result freshness is now controlled by `SCAN_RESULT_STALE_TIME` in `PhishingDataService` +- Tokens for which the bulk scanning API returns no result are now negatively cached for `SCAN_RESULT_STALE_TIME` instead of being re-requested on every call +- `scanUrl` now reports the underlying error message in `fetchError` for network errors instead of `'timeout of 8000ms exceeded'` +- Malformed API responses (e.g. a stalelist without a numeric `lastUpdated`, or scan results without a `recommendedAction`/`result_type`) are now rejected and treated as request failures instead of being passed through - Bump `@metamask/transaction-controller` from `^69.4.0` to `^69.5.2` ([#9780](https://github.com/MetaMask/core/pull/9780), [#9798](https://github.com/MetaMask/core/pull/9798), [#9823](https://github.com/MetaMask/core/pull/9823)) +### Removed + +- **BREAKING:** Remove the `CacheEntry` type; the custom cache manager has been replaced by `PhishingDataService`'s query cache +- **BREAKING:** Remove the `DEFAULT_URL_SCAN_CACHE_TTL`, `DEFAULT_URL_SCAN_CACHE_MAX_SIZE`, `DEFAULT_TOKEN_SCAN_CACHE_TTL`, `DEFAULT_TOKEN_SCAN_CACHE_MAX_SIZE`, `DEFAULT_ADDRESS_SCAN_CACHE_TTL`, and `DEFAULT_ADDRESS_SCAN_CACHE_MAX_SIZE` constants + ## [17.3.1] ### Changed diff --git a/packages/phishing-controller/package.json b/packages/phishing-controller/package.json index 179d6998abb..ed266366c2d 100644 --- a/packages/phishing-controller/package.json +++ b/packages/phishing-controller/package.json @@ -57,10 +57,15 @@ "dependencies": { "@metamask/address-book-controller": "^7.1.2", "@metamask/base-controller": "^9.1.0", + "@metamask/base-data-service": "^0.1.3", "@metamask/controller-utils": "^12.3.0", "@metamask/messenger": "^2.0.0", + "@metamask/storage-service": "^1.0.2", + "@metamask/superstruct": "^3.4.1", "@metamask/transaction-controller": "^69.5.2", + "@metamask/utils": "^11.11.0", "@noble/hashes": "^1.8.0", + "@tanstack/query-core": "^4.43.0", "@types/punycode": "^2.1.0", "ethereum-cryptography": "^2.1.2", "fastest-levenshtein": "^1.0.16", diff --git a/packages/phishing-controller/src/BulkTokenScan.test.ts b/packages/phishing-controller/src/BulkTokenScan.test.ts index 7f2ab12fc82..b012582d680 100644 --- a/packages/phishing-controller/src/BulkTokenScan.test.ts +++ b/packages/phishing-controller/src/BulkTokenScan.test.ts @@ -1,4 +1,3 @@ -import { safelyExecuteWithTimeout } from '@metamask/controller-utils'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { MessengerActions, @@ -16,19 +15,11 @@ import type { PhishingControllerMessenger, PhishingControllerOptions, } from './PhishingController.js'; +import { PhishingDataService } from './PhishingDataService.js'; +import type { PhishingDataServiceMessenger } from './PhishingDataService.js'; import { TokenScanResultType } from './types.js'; import type { BulkTokenScanRequest, TokenScanApiResponse } from './types.js'; -jest.mock('@metamask/controller-utils', () => ({ - ...jest.requireActual('@metamask/controller-utils'), - safelyExecuteWithTimeout: jest.fn(), -})); - -const mockSafelyExecuteWithTimeout = - safelyExecuteWithTimeout as jest.MockedFunction< - typeof safelyExecuteWithTimeout - >; - const controllerName = 'PhishingController'; type AllPhishingControllerActions = @@ -38,11 +29,13 @@ type AllPhishingControllerEvents = MessengerEvents; type RootMessenger = Messenger< MockAnyNamespace, - AllPhishingControllerActions, - AllPhishingControllerEvents, + AllPhishingControllerActions | MessengerActions, + AllPhishingControllerEvents | MessengerEvents, RootMessenger >; +const createdDataServices: PhishingDataService[] = []; + /** * Creates and returns a root messenger for testing * @@ -55,7 +48,8 @@ function getRootMessenger(): RootMessenger { } /** - * Constructs a messenger with transaction events enabled. + * Constructs a messenger with transaction events enabled, plus a real + * PhishingDataService so that tests exercise the full request path via nock. * * @returns A restricted messenger that can listen to TransactionController events. */ @@ -72,8 +66,34 @@ function getMessengerWithTransactionEvents() { parent: rootMessenger, }); + const dataServiceMessenger = new Messenger< + 'PhishingDataService', + MessengerActions, + MessengerEvents, + RootMessenger + >({ + namespace: 'PhishingDataService', + parent: rootMessenger, + }); + createdDataServices.push( + new PhishingDataService({ + messenger: dataServiceMessenger, + policyOptions: { maxRetries: 0 }, + persistenceConfig: null, + }), + ); + rootMessenger.delegate({ - actions: [], + actions: [ + 'PhishingDataService:getStalelist', + 'PhishingDataService:getHotlistDiffs', + 'PhishingDataService:getC2DomainBlocklist', + 'PhishingDataService:scanUrl', + 'PhishingDataService:bulkScanUrls', + 'PhishingDataService:bulkScanTokens', + 'PhishingDataService:scanAddress', + 'PhishingDataService:getApprovals', + ], events: ['TransactionController:stateChange'], messenger, }); @@ -105,21 +125,15 @@ describe('PhishingController - Bulk Token Scanning', () => { controller = getPhishingController(); consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); - - // Reset the mock to its default behavior (pass through to real implementation) - mockSafelyExecuteWithTimeout.mockImplementation( - (fn, throwOnTimeout, timeout) => { - return jest - .requireActual('@metamask/controller-utils') - .safelyExecuteWithTimeout(fn, throwOnTimeout, timeout); - }, - ); }); afterEach(() => { cleanAll(); consoleErrorSpy.mockRestore(); consoleWarnSpy.mockRestore(); + while (createdDataServices.length > 0) { + createdDataServices.pop()?.destroy(); + } }); describe('bulkScanTokens', () => { @@ -420,22 +434,31 @@ describe('PhishingController - Bulk Token Scanning', () => { }); it('should handle API timeout and return empty results', async () => { + jest.useFakeTimers({ + doNotFake: ['nextTick', 'queueMicrotask'], + now: 1_000_000, + }); const tokens = ['0x1234567890123456789012345678901234567890']; - // Mock safelyExecuteWithTimeout to return null (simulating a timeout) - mockSafelyExecuteWithTimeout.mockResolvedValueOnce(null); + nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT) + .delayConnection(10000) + .reply(200, { results: {} }); const request: BulkTokenScanRequest = { chainId: '0x1', tokens, }; - const result = await controller.bulkScanTokens(request); + const promise = controller.bulkScanTokens(request); + jest.advanceTimersByTime(8000); + const result = await promise; expect(result).toStrictEqual({}); expect(consoleErrorSpy).toHaveBeenCalledWith( 'Error scanning tokens: timeout of 8000ms exceeded', ); + jest.useRealTimers(); }); }); diff --git a/packages/phishing-controller/src/CacheManager.test.ts b/packages/phishing-controller/src/CacheManager.test.ts deleted file mode 100644 index 5bbf8c92cb5..00000000000 --- a/packages/phishing-controller/src/CacheManager.test.ts +++ /dev/null @@ -1,200 +0,0 @@ -import { CacheManager } from './CacheManager.js'; -import * as utils from './utils.js'; - -describe('CacheManager', () => { - let updateStateSpy: jest.Mock; - let cache: CacheManager<{ value: string }>; - - beforeEach(() => { - jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); - jest - .spyOn(utils, 'fetchTimeNow') - .mockImplementation(() => Math.floor(Date.now() / 1000)); - updateStateSpy = jest.fn(); - cache = new CacheManager<{ value: string }>({ - cacheTTL: 300, // 5 minutes - maxCacheSize: 3, - updateState: updateStateSpy, - }); - }); - - afterEach(() => { - jest.useRealTimers(); - jest.restoreAllMocks(); - }); - - describe('constructor', () => { - it('should initialize with empty cache when no initialCache provided', () => { - const emptyCache = new CacheManager<{ value: string }>({ - // eslint-disable-next-line no-empty-function - updateState: () => {}, - }); - expect(emptyCache.get('test-key')).toBeUndefined(); - }); - - it('should initialize with provided initialCache data', () => { - const now = Math.floor(Date.now() / 1000); - const initialCache = { - 'test-key': { - data: { value: 'test-value' }, - timestamp: now, - }, - }; - - const cacheWithInitialData = new CacheManager<{ value: string }>({ - initialCache, - // eslint-disable-next-line no-empty-function - updateState: () => {}, - }); - - expect(cacheWithInitialData.get('test-key')).toStrictEqual({ - value: 'test-value', - }); - }); - }); - - describe('get', () => { - it('should return undefined for non-existent keys', () => { - expect(cache.get('non-existent')).toBeUndefined(); - }); - - it('should return data for existing keys', () => { - cache.set('key1', { value: 'value1' }); - expect(cache.get('key1')).toStrictEqual({ value: 'value1' }); - }); - - it('should return undefined for expired entries', () => { - cache.set('key1', { value: 'value1' }); - - // Fast forward time past TTL - jest.advanceTimersByTime(301 * 1000); - - expect(cache.get('key1')).toBeUndefined(); - }); - }); - - describe('set', () => { - it('should add new entries', () => { - cache.set('key1', { value: 'value1' }); - expect(cache.get('key1')).toStrictEqual({ value: 'value1' }); - }); - - it('should update existing entries', () => { - cache.set('key1', { value: 'value1' }); - cache.set('key1', { value: 'updated-value' }); - expect(cache.get('key1')).toStrictEqual({ value: 'updated-value' }); - }); - - it('should call updateState when adding entries', () => { - cache.set('key1', { value: 'value1' }); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - }); - - it('should evict oldest entries when cache exceeds max size', () => { - cache.set('key1', { value: 'value1' }); - cache.set('key2', { value: 'value2' }); - cache.set('key3', { value: 'value3' }); - cache.set('key4', { value: 'value4' }); // This should evict key1 - - expect(cache.get('key1')).toBeUndefined(); - expect(cache.get('key2')).toStrictEqual({ value: 'value2' }); - expect(cache.get('key3')).toStrictEqual({ value: 'value3' }); - expect(cache.get('key4')).toStrictEqual({ value: 'value4' }); - }); - }); - - describe('delete', () => { - it('should remove entries', () => { - cache.set('key1', { value: 'value1' }); - expect(cache.delete('key1')).toBe(true); - expect(cache.get('key1')).toBeUndefined(); - }); - - it('should return false when deleting non-existent keys', () => { - expect(cache.delete('non-existent')).toBe(false); - }); - - it('should call updateState when deleting entries', () => { - cache.set('key1', { value: 'value1' }); - updateStateSpy.mockClear(); - cache.delete('key1'); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - }); - }); - - describe('clear', () => { - it('should remove all entries', () => { - cache.set('key1', { value: 'value1' }); - cache.set('key2', { value: 'value2' }); - cache.clear(); - expect(cache.get('key1')).toBeUndefined(); - expect(cache.get('key2')).toBeUndefined(); - }); - - it('should call updateState', () => { - cache.set('key1', { value: 'value1' }); - updateStateSpy.mockClear(); - cache.clear(); - expect(updateStateSpy).toHaveBeenCalledTimes(1); - }); - }); - - describe('setTTL', () => { - it('should update the TTL', () => { - cache.setTTL(600); - expect(cache.getTTL()).toBe(600); - }); - }); - - describe('setMaxSize', () => { - it('should update the max size', () => { - cache.setMaxSize(5); - expect(cache.getMaxSize()).toBe(5); - }); - - it('should evict entries if new size is smaller than current cache size', () => { - cache.set('key1', { value: 'value1' }); - cache.set('key2', { value: 'value2' }); - cache.set('key3', { value: 'value3' }); - cache.setMaxSize(2); // This should evict key1 - - expect(cache.get('key1')).toBeUndefined(); - expect(cache.get('key2')).toStrictEqual({ value: 'value2' }); - expect(cache.get('key3')).toStrictEqual({ value: 'value3' }); - }); - }); - - describe('getSize', () => { - it('should return the current cache size', () => { - expect(cache.getSize()).toBe(0); - cache.set('key1', { value: 'value1' }); - expect(cache.getSize()).toBe(1); - cache.set('key2', { value: 'value2' }); - expect(cache.getSize()).toBe(2); - cache.delete('key1'); - expect(cache.getSize()).toBe(1); - }); - }); - - describe('keys', () => { - it('should return all cache keys', () => { - cache.set('key1', { value: 'value1' }); - cache.set('key2', { value: 'value2' }); - expect(cache.keys()).toStrictEqual(['key1', 'key2']); - }); - }); - - describe('getAllEntries', () => { - it('should return all cache entries', () => { - const now = Math.floor(Date.now() / 1000); - cache.set('key1', { value: 'value1' }); - cache.set('key2', { value: 'value2' }); - const entries = cache.getAllEntries(); - expect(Object.keys(entries)).toStrictEqual(['key1', 'key2']); - expect(entries.key1.data).toStrictEqual({ value: 'value1' }); - expect(entries.key2.data).toStrictEqual({ value: 'value2' }); - expect(entries.key1.timestamp).toBeGreaterThanOrEqual(now); - expect(entries.key2.timestamp).toBeGreaterThanOrEqual(now); - }); - }); -}); diff --git a/packages/phishing-controller/src/CacheManager.ts b/packages/phishing-controller/src/CacheManager.ts deleted file mode 100644 index 9dd4b256353..00000000000 --- a/packages/phishing-controller/src/CacheManager.ts +++ /dev/null @@ -1,210 +0,0 @@ -import { fetchTimeNow } from './utils.js'; - -/** - * Generic cache entry type that wraps the data with a timestamp - */ -export type CacheEntry = { - data: T; - timestamp: number; -}; - -/** - * Configuration options for CacheManager - */ -export type CacheManagerOptions = { - cacheTTL?: number; - maxCacheSize?: number; - initialCache?: Record>; - updateState: (cache: Record>) => void; -}; - -/** - * Generic cache manager with TTL and size limit support - * - * @template T - The type of data to cache - */ -export class CacheManager { - #cacheTTL: number; - - #maxCacheSize: number; - - readonly #cache: Map>; - - readonly #updateState: (cache: Record>) => void; - - /** - * Constructor for CacheManager - * - * @param options - Cache configuration options - * @param options.cacheTTL - Time to live in seconds for cached entries - * @param options.maxCacheSize - Maximum number of entries in the cache - * @param options.initialCache - Initial cache state - * @param options.updateState - Function to update the state when cache changes - */ - constructor({ - cacheTTL = 300, // 5 minutes default - maxCacheSize = 100, - initialCache = {}, - updateState, - }: CacheManagerOptions) { - this.#cacheTTL = cacheTTL; - this.#maxCacheSize = maxCacheSize; - this.#cache = new Map(Object.entries(initialCache)); - this.#updateState = updateState; - this.#evictEntries(); - } - - /** - * Set the time-to-live for cached entries - * - * @param ttl - The TTL in seconds - */ - setTTL(ttl: number): void { - this.#cacheTTL = ttl; - } - - /** - * Get the current TTL setting - * - * @returns The TTL in seconds - */ - getTTL(): number { - return this.#cacheTTL; - } - - /** - * Set the maximum cache size - * - * @param maxSize - The maximum cache size - */ - setMaxSize(maxSize: number): void { - this.#maxCacheSize = maxSize; - this.#evictEntries(); - } - - /** - * Get the current maximum cache size - * - * @returns The maximum cache size - */ - getMaxSize(): number { - return this.#maxCacheSize; - } - - /** - * Get the current cache size - * - * @returns The current number of entries in the cache - */ - getSize(): number { - return this.#cache.size; - } - - /** - * Clear the cache - */ - clear(): void { - this.#cache.clear(); - this.#persistCache(); - } - - /** - * Get a cached result if it exists and is not expired - * - * @param key - The cache key - * @returns The cached data or undefined if not found or expired - */ - get(key: string): T | undefined { - const cacheEntry = this.#cache.get(key); - if (!cacheEntry) { - return undefined; - } - - // Check if the entry is expired - const now = fetchTimeNow(); - if (now - cacheEntry.timestamp > this.#cacheTTL) { - // Entry expired, remove it from cache - this.#cache.delete(key); - this.#persistCache(); - return undefined; - } - - return cacheEntry.data; - } - - /** - * Add an entry to the cache, evicting oldest entries if necessary - * - * @param key - The cache key - * @param data - The data to cache - */ - set(key: string, data: T): void { - this.#cache.set(key, { - data, - timestamp: fetchTimeNow(), - }); - - this.#evictEntries(); - this.#persistCache(); - } - - /** - * Delete a specific entry from the cache - * - * @param key - The cache key - * @returns True if an entry was deleted - */ - delete(key: string): boolean { - const result = this.#cache.delete(key); - if (result) { - this.#persistCache(); - } - return result; - } - - /** - * Get all keys in the cache - * - * @returns Array of cache keys - */ - keys(): string[] { - return Array.from(this.#cache.keys()); - } - - /** - * Get all entries in the cache (including expired ones) - * Useful for debugging or persistence - * - * @returns Record of all cache entries - */ - getAllEntries(): Record> { - return Object.fromEntries(this.#cache); - } - - /** - * Persist the current cache state - */ - #persistCache(): void { - this.#updateState(Object.fromEntries(this.#cache)); - } - - /** - * Evict oldest entries if cache exceeds max size - */ - #evictEntries(): void { - if (this.#cache.size <= this.#maxCacheSize) { - return; - } - - const entriesToRemove = this.#cache.size - this.#maxCacheSize; - let count = 0; - // Delete the oldest entries (Map maintains insertion order) - for (const key of this.#cache.keys()) { - if (count >= entriesToRemove) { - break; - } - this.#cache.delete(key); - count += 1; - } - } -} diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index d7437aa8a7c..80734ed321d 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -18,6 +18,10 @@ import { ListNames, METAMASK_HOTLIST_DIFF_FILE, METAMASK_STALELIST_FILE, + METAMASK_HOTLIST_DIFF_URL, + METAMASK_STALELIST_URL, + C2_DOMAIN_BLOCKLIST_URL, + phishingListKeyNameMap, PhishingController, PHISHING_CONFIG_BASE_URL, CLIENT_SIDE_DETECION_BASE_URL, @@ -34,6 +38,11 @@ import type { BulkPhishingDetectionScanResponse, PhishingControllerMessenger, } from './PhishingController.js'; +import { + PhishingDataService, + SCAN_RESULT_STALE_TIME, +} from './PhishingDataService.js'; +import type { PhishingDataServiceMessenger } from './PhishingDataService.js'; import { createMockStateChangePayload, createMockTransaction, @@ -62,11 +71,61 @@ type AllPhishingControllerEvents = MessengerEvents; type RootMessenger = Messenger< MockAnyNamespace, - AllPhishingControllerActions, - AllPhishingControllerEvents, + AllPhishingControllerActions | MessengerActions, + AllPhishingControllerEvents | MessengerEvents, RootMessenger >; +const PHISHING_DATA_SERVICE_ACTIONS = [ + 'PhishingDataService:getStalelist', + 'PhishingDataService:getHotlistDiffs', + 'PhishingDataService:getC2DomainBlocklist', + 'PhishingDataService:scanUrl', + 'PhishingDataService:bulkScanUrls', + 'PhishingDataService:bulkScanTokens', + 'PhishingDataService:scanAddress', + 'PhishingDataService:getApprovals', +] as const; + +const createdDataServices: PhishingDataService[] = []; + +/** + * Destroys all data services created during the current test, releasing their + * query cache resources. + */ +function destroyDataServices(): void { + while (createdDataServices.length > 0) { + createdDataServices.pop()?.destroy(); + } +} + +/** + * Constructs a real PhishingDataService wired to the given root messenger, so + * that controller tests exercise the full request path via nock. + * + * @param rootMessenger - The root messenger. + * @returns The data service. + */ +function setupDataService(rootMessenger: RootMessenger): PhishingDataService { + const dataServiceMessenger = new Messenger< + 'PhishingDataService', + MessengerActions, + MessengerEvents, + RootMessenger + >({ + namespace: 'PhishingDataService', + parent: rootMessenger, + }); + + const dataService = new PhishingDataService({ + messenger: dataServiceMessenger, + policyOptions: { maxRetries: 0 }, + persistenceConfig: null, + }); + createdDataServices.push(dataService); + return dataService; +} + type SetupMessengerOptions = { transactionControllerState?: TransactionControllerState; addressBookControllerState?: AddressBookControllerState; @@ -133,10 +192,13 @@ function setupMessenger(options: SetupMessengerOptions = {}): { parent: rootMessenger, }); + setupDataService(rootMessenger); + rootMessenger.delegate({ actions: [ 'AddressBookController:getState', 'TransactionController:getState', + ...PHISHING_DATA_SERVICE_ACTIONS, ], events: [ // eslint-disable-next-line no-restricted-syntax @@ -194,6 +256,7 @@ describe('PhishingController', () => { afterEach(() => { jest.useRealTimers(); cleanAll(); + destroyDataServices(); }); it('should have no default phishing lists', () => { @@ -201,6 +264,21 @@ describe('PhishingController', () => { expect(controller.state.phishingLists).toStrictEqual([]); }); + it('re-exports API URLs and list mappings for backwards compatibility', () => { + expect(METAMASK_STALELIST_URL).toBe( + `${PHISHING_CONFIG_BASE_URL}${METAMASK_STALELIST_FILE}`, + ); + expect(METAMASK_HOTLIST_DIFF_URL).toBe( + `${PHISHING_CONFIG_BASE_URL}${METAMASK_HOTLIST_DIFF_FILE}`, + ); + expect(C2_DOMAIN_BLOCKLIST_URL).toBe( + `${CLIENT_SIDE_DETECION_BASE_URL}${C2_DOMAIN_BLOCKLIST_ENDPOINT}`, + ); + expect(phishingListKeyNameMap.eth_phishing_detect_config).toBe( + ListNames.MetaMask, + ); + }); + it('should default to an empty whitelist', () => { const { controller } = getPhishingController(); expect(controller.state.whitelist).toStrictEqual([]); @@ -2833,7 +2911,12 @@ describe('PhishingController', () => { rootMessenger = createdMessenger; - jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); + // A nonzero epoch is required for the data service's query cache: a + // cached entry with `dataUpdatedAt` of 0 is treated as never fetched. + jest.useFakeTimers({ + doNotFake: ['nextTick', 'queueMicrotask'], + now: 1_000_000, + }); }); it('should return the scan result', async () => { @@ -3088,7 +3171,12 @@ describe('PhishingController', () => { rootMessenger = createdMessenger; - jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); + // A nonzero epoch is required for the data service's query cache: a + // cached entry with `dataUpdatedAt` of 0 is treated as never fetched. + jest.useFakeTimers({ + doNotFake: ['nextTick', 'queueMicrotask'], + now: 1_000_000, + }); }); afterEach(() => { @@ -3438,6 +3526,7 @@ describe('PhishingController', () => { // eslint-disable-next-line import-x/no-named-as-default-member expect(nock.pendingMocks()).toHaveLength(0); }); + it('should handle invalid URLs properly when mixed with valid URLs and cache results correctly', async () => { const validUrl = 'https://valid-example.com'; const invalidUrl = 'not-a-url'; @@ -3558,7 +3647,12 @@ describe('PhishingController', () => { rootMessenger = createdMessenger; - jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); + // A nonzero epoch is required for the data service's query cache: a + // cached entry with `dataUpdatedAt` of 0 is treated as never fetched. + jest.useFakeTimers({ + doNotFake: ['nextTick', 'queueMicrotask'], + now: 1_000_000, + }); }); afterEach(() => { @@ -3981,11 +4075,17 @@ describe('PhishingController', () => { describe('URL Scan Cache', () => { beforeEach(() => { - jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'], now: 0 }); + // A nonzero epoch is required for the data service's query cache: a + // cached entry with `dataUpdatedAt` of 0 is treated as never fetched. + jest.useFakeTimers({ + doNotFake: ['nextTick', 'queueMicrotask'], + now: 1_000_000, + }); }); afterEach(() => { jest.useRealTimers(); cleanAll(); + destroyDataServices(); }); it('should cache scan results and return them on subsequent calls', async () => { @@ -4030,9 +4130,8 @@ describe('URL Scan Cache', () => { fetchSpy.mockRestore(); }); - it('should expire cache entries after TTL', async () => { + it('should expire cache entries after the scan result stale time', async () => { const testDomain = 'example.com'; - const cacheTTL = 300; // 5 minutes nock(PHISHING_DETECTION_BASE_URL) .get( @@ -4052,25 +4151,23 @@ describe('URL Scan Cache', () => { recommendedAction: RecommendedAction.None, }); - const { rootMessenger } = getPhishingController({ - urlScanCacheTTL: cacheTTL, - }); + const { rootMessenger } = getPhishingController(); await rootMessenger.call( 'PhishingController:scanUrl', `https://${testDomain}`, ); - // Before TTL expires, should use cache - jest.advanceTimersByTime((cacheTTL - 10) * 1000); + // Before the stale time elapses, should use cache + jest.advanceTimersByTime(SCAN_RESULT_STALE_TIME - 10_000); await rootMessenger.call( 'PhishingController:scanUrl', `https://${testDomain}`, ); expect(pendingMocks()).toHaveLength(1); // One mock remaining - // After TTL expires, should fetch again - jest.advanceTimersByTime(11 * 1000); + // After the stale time elapses, should fetch again + jest.advanceTimersByTime(11_000); await rootMessenger.call( 'PhishingController:scanUrl', `https://${testDomain}`, @@ -4078,66 +4175,6 @@ describe('URL Scan Cache', () => { expect(pendingMocks()).toHaveLength(0); // All mocks used }); - it('should evict oldest entries when cache exceeds max size', async () => { - const maxCacheSize = 2; - const domains = ['domain1.com', 'domain2.com', 'domain3.com']; - - // Setup nock to respond to all three domains - domains.forEach((domain) => { - nock(PHISHING_DETECTION_BASE_URL) - .get( - `/${PHISHING_DETECTION_SCAN_ENDPOINT}?url=${encodeURIComponent( - domain, - )}`, - ) - .reply(200, { - recommendedAction: RecommendedAction.None, - }); - }); - - // Setup a second request for the first domain - nock(PHISHING_DETECTION_BASE_URL) - .get( - `/${PHISHING_DETECTION_SCAN_ENDPOINT}?url=${encodeURIComponent( - domains[0], - )}`, - ) - .reply(200, { - recommendedAction: RecommendedAction.Warn, - }); - - const { rootMessenger } = getPhishingController({ - urlScanCacheMaxSize: maxCacheSize, - }); - - // Fill the cache - await rootMessenger.call( - 'PhishingController:scanUrl', - `https://${domains[0]}`, - ); - jest.advanceTimersByTime(1000); // Ensure different timestamps - await rootMessenger.call( - 'PhishingController:scanUrl', - `https://${domains[1]}`, - ); - - // This should evict the oldest entry (domain1) - jest.advanceTimersByTime(1000); - await rootMessenger.call( - 'PhishingController:scanUrl', - `https://${domains[2]}`, - ); - - // Now domain1 should not be in cache and require a new fetch - await rootMessenger.call( - 'PhishingController:scanUrl', - `https://${domains[0]}`, - ); - - // All mocks should be used - expect(isDone()).toBe(true); - }); - it('should handle fetch errors and not cache them', async () => { const testDomain = 'example.com'; @@ -4281,13 +4318,10 @@ describe('URL Scan Cache', () => { ), ).toMatchInlineSnapshot(` { - "addressScanCache": {}, "c2DomainBlocklistLastFetched": 0, "hotlistLastFetched": 0, "phishingLists": [], "stalelistLastFetched": 0, - "tokenScanCache": {}, - "urlScanCache": {}, "whitelist": [], "whitelistPaths": {}, } @@ -4303,13 +4337,7 @@ describe('URL Scan Cache', () => { controller.metadata, 'usedInUi', ), - ).toMatchInlineSnapshot(` - { - "addressScanCache": {}, - "tokenScanCache": {}, - "urlScanCache": {}, - } - `); + ).toMatchInlineSnapshot(`{}`); }); }); }); @@ -4335,6 +4363,7 @@ describe('Transaction Controller State Change Integration', () => { afterEach(() => { bulkScanTokensSpy.mockRestore(); + destroyDataServices(); }); it('triggers bulk token scanning when transaction with token balance changes is added', async () => { @@ -4420,6 +4449,47 @@ describe('Transaction Controller State Change Integration', () => { expect(bulkScanTokensSpy).not.toHaveBeenCalled(); }); + it('groups tokens from multiple transactions on the same chain into one scan', async () => { + const transaction1 = createMockTransaction('test-tx-1', [ + TEST_ADDRESSES.USDC, + ]); + const transaction2 = createMockTransaction('test-tx-2', [ + TEST_ADDRESSES.MOCK_TOKEN_1, + ]); + const stateChangePayload = createMockStateChangePayload([ + transaction1, + transaction2, + ]); + + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: transaction1, + }, + { + op: 'add' as const, + path: ['transactions', 1], + value: transaction2, + }, + ], + ); + + await new Promise((resolve) => process.nextTick(resolve)); + + expect(bulkScanTokensSpy).toHaveBeenCalledTimes(1); + expect(bulkScanTokensSpy).toHaveBeenCalledWith({ + chainId: transaction1.chainId.toLowerCase(), + tokens: [ + TEST_ADDRESSES.USDC.toLowerCase(), + TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(), + ], + }); + }); + it('does not trigger bulk token scanning when transaction has no token balance changes', async () => { const mockTransaction = createMockTransaction('test-tx-1', []); @@ -4571,6 +4641,8 @@ describe('Transaction Controller State Change Integration', () => { }); describe('Address poisoning detection', () => { + afterEach(destroyDataServices); + const ADDRESS_BOOK_RECIPIENT = '0x1234bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb5678' as `0x${string}`; const CONFIRMED_TX_RECIPIENT = diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index 3ec4df29c1c..e8827edc1ce 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -9,12 +9,9 @@ import type { ControllerGetStateAction, ControllerStateChangeEvent, } from '@metamask/base-controller'; -import { - isValidHexAddress, - safelyExecute, - safelyExecuteWithTimeout, -} from '@metamask/controller-utils'; +import { HttpError, isValidHexAddress } from '@metamask/controller-utils'; import type { Messenger } from '@metamask/messenger'; +import { getErrorMessage } from '@metamask/utils'; import type { TransactionControllerGetStateAction, TransactionControllerState, @@ -29,8 +26,6 @@ import type { Patch } from 'immer'; import { toASCII } from 'punycode/punycode.js'; import { findSimilarAddresses } from './address-poisoning.js'; -import { CacheManager } from './CacheManager.js'; -import type { CacheEntry } from './CacheManager.js'; import { convertListToTrie, insertToTrie, @@ -42,23 +37,31 @@ import type { PhishingControllerMethodActions, PhishingControllerTestOriginAction, } from './PhishingController-method-action-types.js'; +import type { PhishingDataServiceMethodActions } from './PhishingDataService-method-action-types.js'; import { PhishingDetector } from './PhishingDetector.js'; import { PhishingDetectorResultType, RecommendedAction, AddressScanResultType, + ListKeys, + phishingListNameKeyMap, + phishingListKeyNameMap, } from './types.js'; import type { PhishingDetectorResult, PhishingDetectionScanResult, - TokenScanCacheData, BulkTokenScanResponse, BulkTokenScanRequest, TokenScanApiResponse, - AddressScanCacheData, AddressScanResult, SimilarAddressMatch, ApprovalsResponse, + BulkPhishingDetectionScanResponse, + C2DomainBlocklistResponse, + DataResultWrapper, + Hotlist, + PhishingListState, + PhishingStalelist, } from './types.js'; import { applyDiffs, @@ -67,8 +70,6 @@ import { roundToNearestMinute, getHostnameFromWebUrl, getPhishingDetectionScanUrlParam, - buildCacheKey, - splitCacheHits, resolveChainName, getPathnameFromUrl, isAddressScanSupportedChain, @@ -76,200 +77,46 @@ import { isTokenScanSupportedChain, } from './utils.js'; -export const PHISHING_CONFIG_BASE_URL = - 'https://phishing-detection.api.cx.metamask.io'; -export const METAMASK_STALELIST_FILE = '/v1/stalelist'; -export const METAMASK_HOTLIST_DIFF_FILE = '/v2/diffsSince'; - -export const CLIENT_SIDE_DETECION_BASE_URL = - 'https://client-side-detection.api.cx.metamask.io'; -export const C2_DOMAIN_BLOCKLIST_ENDPOINT = '/v1/request-blocklist'; - -export const PHISHING_DETECTION_BASE_URL = - 'https://dapp-scanning.api.cx.metamask.io'; -export const PHISHING_DETECTION_SCAN_ENDPOINT = 'v2/scan'; -export const PHISHING_DETECTION_BULK_SCAN_ENDPOINT = 'bulk-scan'; - -export const SECURITY_ALERTS_BASE_URL = - 'https://security-alerts.api.cx.metamask.io'; -export const TOKEN_BULK_SCANNING_ENDPOINT = '/token/scan-bulk'; -export const ADDRESS_SCAN_ENDPOINT = '/address/evm/scan'; -export const APPROVALS_ENDPOINT = '/address/evm/approvals'; - -// Cache configuration defaults -export const DEFAULT_URL_SCAN_CACHE_TTL = 1 * 60; // 1 minute in seconds -export const DEFAULT_URL_SCAN_CACHE_MAX_SIZE = 250; -export const DEFAULT_TOKEN_SCAN_CACHE_TTL = 1 * 60; // 1 minute in seconds -export const DEFAULT_TOKEN_SCAN_CACHE_MAX_SIZE = 1000; -export const DEFAULT_ADDRESS_SCAN_CACHE_TTL = 1 * 60; // 1 minute in seconds -export const DEFAULT_ADDRESS_SCAN_CACHE_MAX_SIZE = 1000; +export { + PHISHING_CONFIG_BASE_URL, + METAMASK_STALELIST_FILE, + METAMASK_HOTLIST_DIFF_FILE, + CLIENT_SIDE_DETECION_BASE_URL, + C2_DOMAIN_BLOCKLIST_ENDPOINT, + PHISHING_DETECTION_BASE_URL, + PHISHING_DETECTION_SCAN_ENDPOINT, + PHISHING_DETECTION_BULK_SCAN_ENDPOINT, + SECURITY_ALERTS_BASE_URL, + TOKEN_BULK_SCANNING_ENDPOINT, + ADDRESS_SCAN_ENDPOINT, + APPROVALS_ENDPOINT, + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + C2_DOMAIN_BLOCKLIST_URL, +} from './PhishingDataService.js'; +export { ListKeys, ListNames, phishingListKeyNameMap } from './types.js'; +export type { + ListTypes, + EthPhishingResponse, + C2DomainBlocklistResponse, + PhishingStalelist, + PhishingListState, + HotlistDiff, + DataResultWrapper, + Hotlist, + BulkPhishingDetectionScanResponse, +} from './types.js'; export const C2_DOMAIN_BLOCKLIST_REFRESH_INTERVAL = 5 * 60; // 5 mins in seconds export const HOTLIST_REFRESH_INTERVAL = 5 * 60; // 5 mins in seconds export const STALELIST_REFRESH_INTERVAL = 30 * 24 * 60 * 60; // 30 days in seconds -export const METAMASK_STALELIST_URL = `${PHISHING_CONFIG_BASE_URL}${METAMASK_STALELIST_FILE}`; -export const METAMASK_HOTLIST_DIFF_URL = `${PHISHING_CONFIG_BASE_URL}${METAMASK_HOTLIST_DIFF_FILE}`; -export const C2_DOMAIN_BLOCKLIST_URL = `${CLIENT_SIDE_DETECION_BASE_URL}${C2_DOMAIN_BLOCKLIST_ENDPOINT}`; - -/** - * @type ListTypes - * - * Type outlining the types of lists provided by aggregating different source lists - */ -export type ListTypes = - | 'fuzzylist' - | 'blocklist' - | 'blocklistPaths' - | 'allowlist' - | 'c2DomainBlocklist'; - -/** - * @type EthPhishingResponse - * - * Configuration response from the eth-phishing-detect package - * consisting of approved and unapproved website origins - * - * @property blacklist - List of unapproved origins - * @property fuzzylist - List of fuzzy-matched unapproved origins - * @property tolerance - Fuzzy match tolerance level - * @property version - Version number of this configuration - * @property whitelist - List of approved origins - */ -export type EthPhishingResponse = { - blacklist: string[]; - fuzzylist: string[]; - tolerance: number; - version: number; - whitelist: string[]; -}; - -/** - * @type C2DomainBlocklistResponse - * - * Response for blocklist update requests - * - * @property recentlyAdded - List of c2 domains recently added to the blocklist - * @property recentlyRemoved - List of c2 domains recently removed from the blocklist - * @property lastFetchedAt - Timestamp of the last fetch request - */ -export type C2DomainBlocklistResponse = { - recentlyAdded: string[]; - recentlyRemoved: string[]; - lastFetchedAt: string; -}; - -/** - * PhishingStalelist defines the expected type of the stalelist from the API. - * - * allowlist - List of approved origins. - * blocklist - List of unapproved origins (hostname-only entries). - * blocklistPaths - Trie of unapproved origins with paths (hostname + path entries). - * fuzzylist - List of fuzzy-matched unapproved origins. - * tolerance - Fuzzy match tolerance level - * lastUpdated - Timestamp of last update. - * version - Stalelist data structure iteration. - */ -export type PhishingStalelist = { - allowlist: string[]; - blocklist: string[]; - blocklistPaths: string[]; - fuzzylist: string[]; - tolerance: number; - version: number; - lastUpdated: number; -}; - -/** - * @type PhishingListState - * - * type defining the persisted list state. This is the persisted state that is updated frequently with `this.maybeUpdateState()`. - * - * @property allowlist - List of approved origins (legacy naming "whitelist") - * @property blocklist - List of unapproved origins (legacy naming "blacklist") - * @property blocklistPaths - Trie of unapproved origins with paths (hostname + path, no query params). - * @property c2DomainBlocklist - List of hashed hostnames that C2 requests are blocked against. - * @property fuzzylist - List of fuzzy-matched unapproved origins - * @property tolerance - Fuzzy match tolerance level - * @property lastUpdated - Timestamp of last update. - * @property version - Version of the phishing list state. - * @property name - Name of the list. Used for attribution. - */ -export type PhishingListState = { - allowlist: string[]; - blocklist: string[]; - blocklistPaths: PathTrie; - c2DomainBlocklist: string[]; - fuzzylist: string[]; - tolerance: number; - version: number; - lastUpdated: number; - name: ListNames; -}; - -/** - * @type HotlistDiff - * - * type defining the expected type of the diffs in hotlist.json file. - * - * @property url - Url of the diff entry. - * @property timestamp - Timestamp at which the diff was identified. - * @property targetList - The list name where the diff was identified. - * @property isRemoval - Was the diff identified a removal type. - */ -export type HotlistDiff = { - url: string; - timestamp: number; - targetList: `${ListKeys}.${ListTypes}`; - isRemoval?: boolean; -}; - -export type DataResultWrapper = { - data: T; -}; - -/** - * @type Hotlist - * - * Type defining expected hotlist.json file. - * - * @property url - Url of the diff entry. - * @property timestamp - Timestamp at which the diff was identified. - * @property targetList - The list name where the diff was identified. - * @property isRemoval - Was the diff identified a removal type. - */ -export type Hotlist = HotlistDiff[]; - -/** - * Enum containing upstream data provider source list keys. - * These are the keys denoting lists consumed by the upstream data provider. - */ -export enum ListKeys { - EthPhishingDetectConfig = 'eth_phishing_detect_config', -} - -/** - * Enum containing downstream client attribution names. - */ -export enum ListNames { - MetaMask = 'MetaMask', -} - -/** - * Maps from downstream client attribution name - * to list key sourced from upstream data provider. - */ -const phishingListNameKeyMap = { - [ListNames.MetaMask]: ListKeys.EthPhishingDetectConfig, -}; - -/** - * Maps from list key sourced from upstream data - * provider to downstream client attribution name. - */ -export const phishingListKeyNameMap = { - [ListKeys.EthPhishingDetectConfig]: ListNames.MetaMask, -}; +// Request timeouts, in milliseconds. +const URL_SCAN_TIMEOUT = 8000; +const BULK_URL_SCAN_TIMEOUT = 15000; +const TOKEN_SCAN_TIMEOUT = 8000; +const ADDRESS_SCAN_TIMEOUT = 5000; +const APPROVALS_TIMEOUT = 5000; const controllerName = 'PhishingController'; @@ -310,24 +157,6 @@ const metadata: StateMetadata = { includeInDebugSnapshot: false, usedInUi: false, }, - urlScanCache: { - includeInStateLogs: false, - persist: true, - includeInDebugSnapshot: false, - usedInUi: true, - }, - tokenScanCache: { - includeInStateLogs: false, - persist: true, - includeInDebugSnapshot: false, - usedInUi: true, - }, - addressScanCache: { - includeInStateLogs: false, - persist: true, - includeInDebugSnapshot: false, - usedInUi: true, - }, }; /** @@ -343,9 +172,6 @@ const getDefaultState = (): PhishingControllerState => { hotlistLastFetched: 0, stalelistLastFetched: 0, c2DomainBlocklistLastFetched: 0, - urlScanCache: {}, - tokenScanCache: {}, - addressScanCache: {}, }; }; @@ -359,9 +185,6 @@ const getDefaultState = (): PhishingControllerState => { * hotlistLastFetched - timestamp of the last hotlist fetch * stalelistLastFetched - timestamp of the last stalelist fetch * c2DomainBlocklistLastFetched - timestamp of the last c2 domain blocklist fetch - * urlScanCache - cache of URL scan results - * tokenScanCache - cache of token scan results - * addressScanCache - cache of address scan results */ export type PhishingControllerState = { phishingLists: PhishingListState[]; @@ -370,9 +193,6 @@ export type PhishingControllerState = { hotlistLastFetched: number; stalelistLastFetched: number; c2DomainBlocklistLastFetched: number; - urlScanCache: Record>; - tokenScanCache: Record>; - addressScanCache: Record>; }; /** @@ -382,23 +202,11 @@ export type PhishingControllerState = { * stalelistRefreshInterval - Polling interval used to fetch stale list. * hotlistRefreshInterval - Polling interval used to fetch hotlist diff list. * c2DomainBlocklistRefreshInterval - Polling interval used to fetch c2 domain blocklist. - * urlScanCacheTTL - Time to live in seconds for cached scan results. - * urlScanCacheMaxSize - Maximum number of entries in the scan cache. - * tokenScanCacheTTL - Time to live in seconds for cached token scan results. - * tokenScanCacheMaxSize - Maximum number of entries in the token scan cache. - * addressScanCacheTTL - Time to live in seconds for cached address scan results. - * addressScanCacheMaxSize - Maximum number of entries in the address scan cache. */ export type PhishingControllerOptions = { stalelistRefreshInterval?: number; hotlistRefreshInterval?: number; c2DomainBlocklistRefreshInterval?: number; - urlScanCacheTTL?: number; - urlScanCacheMaxSize?: number; - tokenScanCacheTTL?: number; - tokenScanCacheMaxSize?: number; - addressScanCacheTTL?: number; - addressScanCacheMaxSize?: number; messenger: PhishingControllerMessenger; state?: Partial; }; @@ -447,7 +255,8 @@ export type PhishingControllerEvents = PhishingControllerStateChangeEvent; */ type AllowedActions = | AddressBookControllerGetStateAction - | TransactionControllerGetStateAction; + | TransactionControllerGetStateAction + | PhishingDataServiceMethodActions; /** * The external events available to the PhishingController. @@ -462,19 +271,6 @@ export type PhishingControllerMessenger = Messenger< PhishingControllerEvents | AllowedEvents >; -/** - * BulkPhishingDetectionScanResponse - * - * Response for bulk phishing detection scan requests - * results - Record of domain names and their corresponding phishing detection scan results - * - * errors - Record of domain names and their corresponding errors - */ -export type BulkPhishingDetectionScanResponse = { - results: Record; - errors: Record; -}; - /** * Controller that manages community-maintained lists of approved and unapproved website origins. */ @@ -493,12 +289,6 @@ export class PhishingController extends BaseController< readonly #c2DomainBlocklistRefreshInterval: number; - readonly #urlScanCache: CacheManager; - - readonly #tokenScanCache: CacheManager; - - readonly #addressScanCache: CacheManager; - readonly #knownRecipients: Set; readonly #transactionRecipients: Set; @@ -531,12 +321,6 @@ export class PhishingController extends BaseController< * @param config.stalelistRefreshInterval - Polling interval used to fetch stale list. * @param config.hotlistRefreshInterval - Polling interval used to fetch hotlist diff list. * @param config.c2DomainBlocklistRefreshInterval - Polling interval used to fetch c2 domain blocklist. - * @param config.urlScanCacheTTL - Time to live in seconds for cached scan results. - * @param config.urlScanCacheMaxSize - Maximum number of entries in the scan cache. - * @param config.tokenScanCacheTTL - Time to live in seconds for cached token scan results. - * @param config.tokenScanCacheMaxSize - Maximum number of entries in the token scan cache. - * @param config.addressScanCacheTTL - Time to live in seconds for cached address scan results. - * @param config.addressScanCacheMaxSize - Maximum number of entries in the address scan cache. * @param config.messenger - The controller restricted messenger. * @param config.state - Initial state to set on this controller. */ @@ -544,12 +328,6 @@ export class PhishingController extends BaseController< stalelistRefreshInterval = STALELIST_REFRESH_INTERVAL, hotlistRefreshInterval = HOTLIST_REFRESH_INTERVAL, c2DomainBlocklistRefreshInterval = C2_DOMAIN_BLOCKLIST_REFRESH_INTERVAL, - urlScanCacheTTL = DEFAULT_URL_SCAN_CACHE_TTL, - urlScanCacheMaxSize = DEFAULT_URL_SCAN_CACHE_MAX_SIZE, - tokenScanCacheTTL = DEFAULT_TOKEN_SCAN_CACHE_TTL, - tokenScanCacheMaxSize = DEFAULT_TOKEN_SCAN_CACHE_MAX_SIZE, - addressScanCacheTTL = DEFAULT_ADDRESS_SCAN_CACHE_TTL, - addressScanCacheMaxSize = DEFAULT_ADDRESS_SCAN_CACHE_MAX_SIZE, messenger, state = {}, }: PhishingControllerOptions) { @@ -575,36 +353,6 @@ export class PhishingController extends BaseController< this.#onTransactionControllerStateChange.bind(this); this.#addressBookControllerStateChangeHandler = this.#onAddressBookControllerStateChange.bind(this); - this.#urlScanCache = new CacheManager({ - cacheTTL: urlScanCacheTTL, - maxCacheSize: urlScanCacheMaxSize, - initialCache: this.state.urlScanCache, - updateState: (cache) => { - this.update((draftState) => { - draftState.urlScanCache = cache; - }); - }, - }); - this.#tokenScanCache = new CacheManager({ - cacheTTL: tokenScanCacheTTL, - maxCacheSize: tokenScanCacheMaxSize, - initialCache: this.state.tokenScanCache, - updateState: (cache) => { - this.update((draftState) => { - draftState.tokenScanCache = cache; - }); - }, - }); - this.#addressScanCache = new CacheManager({ - cacheTTL: addressScanCacheTTL, - maxCacheSize: addressScanCacheMaxSize, - initialCache: this.state.addressScanCache, - updateState: (cache) => { - this.update((draftState) => { - draftState.addressScanCache = cache; - }); - }, - }); this.messenger.registerMethodActionHandlers( this, @@ -1231,58 +979,24 @@ export class PhishingController extends BaseController< const [hostname] = getHostnameFromWebUrl(url); - const cachedResult = this.#urlScanCache.get(scanUrlParam); - if (cachedResult) { - return cachedResult; - } - - const apiResponse = await safelyExecuteWithTimeout( - async () => { - const res = await fetch( - `${PHISHING_DETECTION_BASE_URL}/${PHISHING_DETECTION_SCAN_ENDPOINT}?url=${encodeURIComponent(scanUrlParam)}`, - { - method: 'GET', - headers: { - Accept: 'application/json', - }, - }, - ); - if (!res.ok) { - return { - error: `${res.status} ${res.statusText}`, - }; - } - const data = await res.json(); - return data; - }, - true, - 8000, - ); - - // Need to do it this way because safelyExecuteWithTimeout returns undefined for both timeouts and errors. - if (!apiResponse) { - return { - hostname: '', - recommendedAction: RecommendedAction.None, - fetchError: 'timeout of 8000ms exceeded', - }; - } else if ((apiResponse as { error?: string }).error) { + let scanResult: PhishingDetectionScanResult; + try { + scanResult = await this.#callWithTimeout( + this.messenger.call('PhishingDataService:scanUrl', scanUrlParam), + URL_SCAN_TIMEOUT, + ); + } catch (error) { return { hostname: '', recommendedAction: RecommendedAction.None, - fetchError: (apiResponse as { error: string }).error, + fetchError: getErrorMessage(error), }; } - const scanResult = apiResponse as PhishingDetectionScanResult; - const result = { + return { hostname, recommendedAction: scanResult.recommendedAction, }; - - this.#urlScanCache.set(scanUrlParam, result); - - return result; } /** @@ -1321,8 +1035,7 @@ export class PhishingController extends BaseController< errors: {}, }; - // Extract hostnames from URLs and check for validity and length constraints - const urlsToHostnames: Record = {}; + // Check URLs for validity and length constraints const urlsToFetch: string[] = []; for (const url of urls) { @@ -1333,22 +1046,13 @@ export class PhishingController extends BaseController< continue; } - const [hostname, ok] = getHostnameFromWebUrl(url); + const [, ok] = getHostnameFromWebUrl(url); if (!ok) { combinedResponse.errors[url] = ['url is not a valid web URL']; continue; } - // Check if result is already in cache - const cachedResult = this.#urlScanCache.get(hostname); - if (cachedResult) { - // Use cached result - combinedResponse.results[url] = cachedResult; - } else { - // Add to list of URLs to fetch - urlsToHostnames[url] = hostname; - urlsToFetch.push(url); - } + urlsToFetch.push(url); } // If there are URLs to fetch, process them in batches @@ -1367,12 +1071,7 @@ export class PhishingController extends BaseController< // Merge results and errors from all batches batchResults.forEach((batchResponse) => { - // Add results to cache and combine with response Object.entries(batchResponse.results).forEach(([url, result]) => { - const hostname = urlsToHostnames[url]; - if (hostname) { - this.#urlScanCache.set(hostname, result); - } combinedResponse.results[url] = result; }); @@ -1400,55 +1099,23 @@ export class PhishingController extends BaseController< chain: string, tokens: string[], ): Promise => { - const timeout = 8000; // 8 seconds - const apiResponse = await safelyExecuteWithTimeout( - async () => { - const response = await fetch( - `${SECURITY_ALERTS_BASE_URL}${TOKEN_BULK_SCANNING_ENDPOINT}`, - { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - chain, - tokens, - }), - }, - ); - - if (!response.ok) { - return { - error: `${response.status} ${response.statusText}`, - status: response.status, - statusText: response.statusText, - }; - } - - const data = await response.json(); - return data; - }, - true, - timeout, - ); - - if (!apiResponse) { - console.error(`Error scanning tokens: timeout of ${timeout}ms exceeded`); - return null; - } - - if ((apiResponse as { error?: string }).error) { - const { status, statusText } = apiResponse as { - status: number; - statusText: string; - }; - - console.warn(`Token bulk screening API error: ${status} ${statusText}`); + try { + return await this.#callWithTimeout( + this.messenger.call( + 'PhishingDataService:bulkScanTokens', + chain, + tokens, + ), + TOKEN_SCAN_TIMEOUT, + ); + } catch (error) { + if (error instanceof HttpError) { + console.warn(`Token bulk screening API error: ${error.message}`); + } else { + console.error(`Error scanning tokens: ${getErrorMessage(error)}`); + } return null; } - - return apiResponse as TokenScanApiResponse; }; /** @@ -1480,67 +1147,25 @@ export class PhishingController extends BaseController< }; } - const cacheKey = buildCacheKey(normalizedChainId, normalizedAddress); - const cachedResult = this.#addressScanCache.get(cacheKey); - if (cachedResult) { - return { - result_type: cachedResult.result_type, - label: cachedResult.label, - }; - } - - const apiResponse = await safelyExecuteWithTimeout( - async () => { - const res = await fetch( - `${SECURITY_ALERTS_BASE_URL}${ADDRESS_SCAN_ENDPOINT}`, - { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - chain, - address: normalizedAddress, - }), - }, - ); - if (!res.ok) { - return { - error: `${res.status} ${res.statusText}`, - }; - } - const data: AddressScanResult = await res.json(); - return data; - }, - true, - 5000, - ); - - if (!apiResponse) { + try { + const scanResult = await this.#callWithTimeout( + this.messenger.call( + 'PhishingDataService:scanAddress', + chain, + normalizedAddress, + ), + ADDRESS_SCAN_TIMEOUT, + ); return { - result_type: AddressScanResultType.ErrorResult, - label: '', + result_type: scanResult.result_type, + label: scanResult.label, }; - } else if ((apiResponse as { error?: string }).error) { + } catch { return { result_type: AddressScanResultType.ErrorResult, label: '', }; } - - const scanResult = apiResponse as AddressScanResult; - const result: AddressScanCacheData = { - result_type: scanResult.result_type, - label: scanResult.label, - }; - - this.#addressScanCache.set(cacheKey, result); - - return { - result_type: scanResult.result_type, - label: scanResult.label, - }; } /** @@ -1566,44 +1191,18 @@ export class PhishingController extends BaseController< return { approvals: [] }; } - const apiResponse = await safelyExecuteWithTimeout( - async () => { - const res = await fetch( - `${SECURITY_ALERTS_BASE_URL}${APPROVALS_ENDPOINT}`, - { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - chain, - address: normalizedAddress, - }), - }, - ); - if (!res.ok) { - return { error: `${res.status} ${res.statusText}` }; - } - const data: ApprovalsResponse = await res.json(); - return data; - }, - true, - 5000, - ); - - if (!apiResponse) { - return { approvals: [] }; - } - - if ( - (apiResponse as { error?: string }).error || - !Array.isArray((apiResponse as Partial).approvals) - ) { + try { + return await this.#callWithTimeout( + this.messenger.call( + 'PhishingDataService:getApprovals', + chain, + normalizedAddress, + ), + APPROVALS_TIMEOUT, + ); + } catch { return { approvals: [] }; } - - return apiResponse as ApprovalsResponse; }; /** @@ -1646,50 +1245,26 @@ export class PhishingController extends BaseController< // EVM addresses are case-insensitive; non-EVM addresses (e.g. Solana // base58) are case-sensitive and must not be lowercased. const caseSensitive = !normalizedChainId.startsWith('0x'); + const normalizedTokens = caseSensitive + ? tokens + : tokens.map((tokenAddress) => tokenAddress.toLowerCase()); - // Split tokens into cached results and tokens that need to be fetched - const { cachedResults, tokensToFetch } = splitCacheHits( - this.#tokenScanCache, - normalizedChainId, - tokens, - caseSensitive, - ); + const results: BulkTokenScanResponse = {}; - const results: BulkTokenScanResponse = { ...cachedResults }; - - // If there are tokens to fetch, call the bulk token scan API - if (tokensToFetch.length > 0) { - const apiResponse = await this.#fetchTokenScanBulkResults( - chain, - tokensToFetch, - ); - if (apiResponse?.results) { - // Process API results and update cache - for (const tokenAddress of tokensToFetch) { - const normalizedAddress = caseSensitive - ? tokenAddress - : tokenAddress.toLowerCase(); - const tokenResult = apiResponse.results[normalizedAddress]; - - if (tokenResult?.result_type) { - const result = { - result_type: tokenResult.result_type, - chain: tokenResult.chain || normalizedChainId, - address: tokenResult.address || normalizedAddress, - }; - - // Update cache - const cacheKey = buildCacheKey( - normalizedChainId, - normalizedAddress, - caseSensitive, - ); - this.#tokenScanCache.set(cacheKey, { - result_type: tokenResult.result_type, - }); - - results[normalizedAddress] = result; - } + const apiResponse = await this.#fetchTokenScanBulkResults( + chain, + normalizedTokens, + ); + if (apiResponse?.results) { + for (const normalizedAddress of normalizedTokens) { + const tokenResult = apiResponse.results[normalizedAddress]; + + if (tokenResult?.result_type) { + results[normalizedAddress] = { + result_type: tokenResult.result_type, + chain: tokenResult.chain || normalizedChainId, + address: tokenResult.address || normalizedAddress, + }; } } } @@ -1706,61 +1281,27 @@ export class PhishingController extends BaseController< readonly #processBatch = async ( urls: string[], ): Promise => { - const apiResponse = await safelyExecuteWithTimeout( - async () => { - const res = await fetch( - `${PHISHING_DETECTION_BASE_URL}/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, - { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ urls }), + try { + return await this.#callWithTimeout( + this.messenger.call('PhishingDataService:bulkScanUrls', urls), + BULK_URL_SCAN_TIMEOUT, + ); + } catch (error) { + if (error instanceof HttpError) { + return { + results: {}, + errors: { + api_error: [error.message], }, - ); - - if (!res.ok) { - return { - error: `${res.status} ${res.statusText}`, - status: res.status, - statusText: res.statusText, - }; - } - - const data = await res.json(); - return data; - }, - true, - 15000, - ); - - // Handle timeout or network errors - if (!apiResponse) { - return { - results: {}, - errors: { - network_error: ['timeout of 15000ms exceeded'], - }, - }; - } - - // Handle HTTP error responses - if ((apiResponse as { error?: string }).error) { - const { status, statusText } = apiResponse as { - status: number; - statusText: string; - }; - + }; + } return { results: {}, errors: { - api_error: [`${status} ${statusText}`], + network_error: [getErrorMessage(error)], }, }; } - - return apiResponse as BulkPhishingDetectionScanResponse; }; /** @@ -1774,12 +1315,13 @@ export class PhishingController extends BaseController< let hotlistDiffsResponse: DataResultWrapper | null = null; let c2DomainBlocklistResponse: C2DomainBlocklistResponse | null = null; try { - const stalelistPromise = this.#queryConfig< - DataResultWrapper - >(METAMASK_STALELIST_URL); + const stalelistPromise = this.#safelyCallService(() => + this.messenger.call('PhishingDataService:getStalelist'), + ); - const c2DomainBlocklistPromise = - this.#queryConfig(C2_DOMAIN_BLOCKLIST_URL); + const c2DomainBlocklistPromise = this.#safelyCallService(() => + this.messenger.call('PhishingDataService:getC2DomainBlocklist'), + ); [stalelistResponse, c2DomainBlocklistResponse] = await Promise.all([ stalelistPromise, @@ -1787,10 +1329,14 @@ export class PhishingController extends BaseController< ]); // Fetching hotlist diffs relies on having a lastUpdated timestamp to do `GET /v1/diffsSince/:timestamp`, // so it doesn't make sense to call if there is not a timestamp to begin with. - if (stalelistResponse?.data && stalelistResponse.data.lastUpdated > 0) { - hotlistDiffsResponse = await this.#queryConfig< - DataResultWrapper - >(`${METAMASK_HOTLIST_DIFF_URL}/${stalelistResponse.data.lastUpdated}`); + const stalelistData = stalelistResponse?.data; + if (stalelistData && stalelistData.lastUpdated > 0) { + hotlistDiffsResponse = await this.#safelyCallService(() => + this.messenger.call( + 'PhishingDataService:getHotlistDiffs', + stalelistData.lastUpdated, + ), + ); } } finally { // Set `stalelistLastFetched` and `hotlistLastFetched` even for failed requests to prevent server @@ -1853,8 +1399,11 @@ export class PhishingController extends BaseController< ...this.state.phishingLists.map(({ lastUpdated }) => lastUpdated), ); - hotlistResponse = await this.#queryConfig>( - `${METAMASK_HOTLIST_DIFF_URL}/${lastDiffTimestamp}`, + hotlistResponse = await this.#safelyCallService(() => + this.messenger.call( + 'PhishingDataService:getHotlistDiffs', + lastDiffTimestamp, + ), ); } finally { // Set `hotlistLastFetched` even for failed requests to prevent server from being overwhelmed with @@ -1893,12 +1442,12 @@ export class PhishingController extends BaseController< * this function that prevents redundant configuration updates. */ async #updateC2DomainBlocklist() { - const c2DomainBlocklistResponse = - await this.#queryConfig( - `${C2_DOMAIN_BLOCKLIST_URL}?timestamp=${roundToNearestMinute( - this.state.c2DomainBlocklistLastFetched, - )}`, - ); + const c2DomainBlocklistResponse = await this.#safelyCallService(() => + this.messenger.call( + 'PhishingDataService:getC2DomainBlocklist', + roundToNearestMinute(this.state.c2DomainBlocklistLastFetched), + ), + ); if (!c2DomainBlocklistResponse) { return; @@ -1929,22 +1478,51 @@ export class PhishingController extends BaseController< this.updatePhishingDetector(); } - async #queryConfig( - input: RequestInfo, - ): Promise { - const response = await safelyExecute( - () => fetch(input, { cache: 'no-cache' }), - true, - ); - - switch (response?.status) { - case 200: { - return await response.json(); - } + /** + * Calls the data service, returning `null` instead of throwing if the call + * fails for any reason (network error, non-2xx response, or malformed + * response). + * + * @param call - The service call to execute. + * @returns The result of the call, or `null` if it failed. + */ + async #safelyCallService( + call: () => Promise, + ): Promise { + try { + return await call(); + } catch (error) { + console.error(error); + return null; + } + } - default: { - return null; - } + /** + * Awaits a promise, rejecting if it does not settle within the given + * timeout. On timeout, any eventual rejection of the original promise is + * suppressed to avoid unhandled rejections. + * + * @param promise - The promise to await. + * @param timeout - The timeout in milliseconds. + * @returns The result of the promise. + */ + async #callWithTimeout( + promise: Promise, + timeout: number, + ): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + promise, + new Promise((_resolve, reject) => { + timer = setTimeout(() => { + promise.catch(() => undefined); + reject(new Error(`timeout of ${timeout}ms exceeded`)); + }, timeout); + }), + ]); + } finally { + clearTimeout(timer); } } } diff --git a/packages/phishing-controller/src/PhishingDataService-method-action-types.ts b/packages/phishing-controller/src/PhishingDataService-method-action-types.ts new file mode 100644 index 00000000000..5390cda5ed5 --- /dev/null +++ b/packages/phishing-controller/src/PhishingDataService-method-action-types.ts @@ -0,0 +1,140 @@ +/** + * This file is auto generated. + * Do not edit manually. + */ + +import type { PhishingDataService } from './PhishingDataService.js'; + +/** + * Fetches the full phishing detection stalelist. + * + * @returns The stalelist response. + */ +export type PhishingDataServiceGetStalelistAction = { + type: `PhishingDataService:getStalelist`; + handler: PhishingDataService['getStalelist']; +}; + +/** + * Fetches the hotlist diffs recorded since the given timestamp. + * + * @param timestamp - The timestamp (in seconds) to fetch diffs since. + * @returns The hotlist diffs response. + */ +export type PhishingDataServiceGetHotlistDiffsAction = { + type: `PhishingDataService:getHotlistDiffs`; + handler: PhishingDataService['getHotlistDiffs']; +}; + +/** + * Fetches the C2 domain blocklist changes recorded since the given + * timestamp, or the current blocklist if no timestamp is given. + * + * @param timestamp - The timestamp (in seconds) to fetch changes since. + * @returns The C2 domain blocklist response. + */ +export type PhishingDataServiceGetC2DomainBlocklistAction = { + type: `PhishingDataService:getC2DomainBlocklist`; + handler: PhishingDataService['getC2DomainBlocklist']; +}; + +/** + * Scans a URL for phishing via the dapp-scanning API. + * + * @param url - The prepared URL parameter to scan (hostname, or hostname + * plus path for shared gateways). + * @returns The phishing detection scan result. + */ +export type PhishingDataServiceScanUrlAction = { + type: `PhishingDataService:scanUrl`; + handler: PhishingDataService['scanUrl']; +}; + +/** + * Scans a batch of URLs for phishing via the dapp-scanning API. + * + * Results are cached per hostname using the same query keys as + * {@link PhishingDataService.scanUrl}, so results are shared between single + * and bulk scans. Only hostnames without a fresh cached result are sent to + * the API, in requests of up to 50 URLs. + * + * @param urls - The URLs to scan. + * @returns The scan results, keyed by URL, and any batch-level errors. + */ +export type PhishingDataServiceBulkScanUrlsAction = { + type: `PhishingDataService:bulkScanUrls`; + handler: PhishingDataService['bulkScanUrls']; +}; + +/** + * Scans a token for malicious activity via the security-alerts API. + * + * Requests made while a bulk scan is being assembled are coalesced into a + * single request to the bulk scanning endpoint. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param token - The token address to scan. + * @returns The token scan result, or `null` if the API returned no result + * for the token. + */ +export type PhishingDataServiceScanTokenAction = { + type: `PhishingDataService:scanToken`; + handler: PhishingDataService['scanToken']; +}; + +/** + * Scans a batch of tokens for malicious activity via the security-alerts + * API. + * + * Results are cached per token; only tokens without a fresh cached result + * are sent to the API, in requests of up to 100 tokens. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param tokens - The token addresses to scan. + * @returns The token scan results, keyed by token address. Tokens for which + * the API returned no result are omitted. + */ +export type PhishingDataServiceBulkScanTokensAction = { + type: `PhishingDataService:bulkScanTokens`; + handler: PhishingDataService['bulkScanTokens']; +}; + +/** + * Scans an address for security alerts via the security-alerts API. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param address - The address to scan. + * @returns The address scan result. + */ +export type PhishingDataServiceScanAddressAction = { + type: `PhishingDataService:scanAddress`; + handler: PhishingDataService['scanAddress']; +}; + +/** + * Gets token approvals for an address with security enrichments via the + * security-alerts API. Approvals reflect live account state and are never + * cached. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param address - The address to get approvals for. + * @returns The approvals response. + */ +export type PhishingDataServiceGetApprovalsAction = { + type: `PhishingDataService:getApprovals`; + handler: PhishingDataService['getApprovals']; +}; + +/** + * Union of all PhishingDataService action types. + */ +export type PhishingDataServiceMethodActions = + | PhishingDataServiceGetStalelistAction + | PhishingDataServiceGetHotlistDiffsAction + | PhishingDataServiceGetC2DomainBlocklistAction + | PhishingDataServiceScanUrlAction + | PhishingDataServiceBulkScanUrlsAction + | PhishingDataServiceScanTokenAction + | PhishingDataServiceBulkScanTokensAction + | PhishingDataServiceScanAddressAction + | PhishingDataServiceGetApprovalsAction; diff --git a/packages/phishing-controller/src/PhishingDataService.test.ts b/packages/phishing-controller/src/PhishingDataService.test.ts new file mode 100644 index 00000000000..bc1f76993ee --- /dev/null +++ b/packages/phishing-controller/src/PhishingDataService.test.ts @@ -0,0 +1,712 @@ +import { ConstantBackoff } from '@metamask/base-data-service'; +import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; +import type { + MessengerActions, + MessengerEvents, + MockAnyNamespace, +} from '@metamask/messenger'; +import nock, { cleanAll } from 'nock'; + +import { flushPromises } from '../../../tests/helpers.js'; +import { + PhishingDataService, + C2_DOMAIN_BLOCKLIST_ENDPOINT, + CLIENT_SIDE_DETECION_BASE_URL, + METAMASK_HOTLIST_DIFF_FILE, + METAMASK_STALELIST_FILE, + PHISHING_CONFIG_BASE_URL, + PHISHING_DETECTION_BASE_URL, + PHISHING_DETECTION_BULK_SCAN_ENDPOINT, + PHISHING_DETECTION_SCAN_ENDPOINT, + SECURITY_ALERTS_BASE_URL, + TOKEN_BULK_SCANNING_ENDPOINT, + ADDRESS_SCAN_ENDPOINT, + APPROVALS_ENDPOINT, + SCAN_RESULT_STALE_TIME, +} from './PhishingDataService.js'; +import type { PhishingDataServiceMessenger } from './PhishingDataService.js'; +import { TokenScanResultType } from './types.js'; +import type { TokenScanApiResponse } from './types.js'; + +const createdServices: PhishingDataService[] = []; + +const STALELIST_RESPONSE = { + data: { + allowlist: [], + blocklist: ['phishing.example.com'], + blocklistPaths: [], + fuzzylist: [], + tolerance: 2, + version: 1, + lastUpdated: 1700000000, + }, +}; + +describe('PhishingDataService', () => { + afterEach(() => { + jest.useRealTimers(); + cleanAll(); + while (createdServices.length > 0) { + createdServices.pop()?.destroy(); + } + }); + + describe('constructor', () => { + it('applies default options when only a messenger is given', () => { + const rootMessenger = createRootMessenger(); + const messenger: PhishingDataServiceMessenger = new Messenger({ + namespace: 'PhishingDataService', + parent: rootMessenger, + }); + const service = new PhishingDataService({ messenger }); + createdServices.push(service); + + expect(service.name).toBe('PhishingDataService'); + }); + }); + + describe('getStalelist', () => { + it('returns the stalelist from the API', async () => { + nock(PHISHING_CONFIG_BASE_URL) + .get(METAMASK_STALELIST_FILE) + .reply(200, STALELIST_RESPONSE); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:getStalelist', + ); + + expect(response).toStrictEqual(STALELIST_RESPONSE); + }); + + it('throws if the API returns a non-200 status', async () => { + nock(PHISHING_CONFIG_BASE_URL).get(METAMASK_STALELIST_FILE).reply(500); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:getStalelist'), + ).rejects.toThrow('500 Internal Server Error'); + }); + + it('throws if the API returns a malformed response', async () => { + nock(PHISHING_CONFIG_BASE_URL) + .get(METAMASK_STALELIST_FILE) + .reply(200, { data: { lastUpdated: 'not a number' } }); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:getStalelist'), + ).rejects.toThrow('Malformed response received from stalelist endpoint'); + }); + }); + + describe('getHotlistDiffs', () => { + it('returns the hotlist diffs recorded since the given timestamp', async () => { + const diffs = { + data: [ + { + url: 'phishing.example.com', + timestamp: 1700000001, + targetList: 'eth_phishing_detect_config.blocklist', + }, + ], + }; + nock(PHISHING_CONFIG_BASE_URL) + .get(`${METAMASK_HOTLIST_DIFF_FILE}/1700000000`) + .reply(200, diffs); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:getHotlistDiffs', + 1700000000, + ); + + expect(response).toStrictEqual(diffs); + }); + + it('throws if the API returns a malformed response', async () => { + nock(PHISHING_CONFIG_BASE_URL) + .get(`${METAMASK_HOTLIST_DIFF_FILE}/1700000000`) + .reply(200, { data: 'not an array' }); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:getHotlistDiffs', 1700000000), + ).rejects.toThrow( + 'Malformed response received from hotlist diffs endpoint', + ); + }); + }); + + describe('getC2DomainBlocklist', () => { + it('returns the C2 domain blocklist when no timestamp is given', async () => { + const blocklist = { + recentlyAdded: ['0415f1f1'], + recentlyRemoved: [], + lastFetchedAt: '2024-01-01T00:00:00Z', + }; + nock(CLIENT_SIDE_DETECION_BASE_URL) + .get(C2_DOMAIN_BLOCKLIST_ENDPOINT) + .reply(200, blocklist); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:getC2DomainBlocklist', + ); + + expect(response).toStrictEqual(blocklist); + }); + + it('passes the given timestamp to the API', async () => { + const blocklist = { + recentlyAdded: [], + recentlyRemoved: ['0415f1f1'], + lastFetchedAt: '2024-01-01T00:00:00Z', + }; + nock(CLIENT_SIDE_DETECION_BASE_URL) + .get(C2_DOMAIN_BLOCKLIST_ENDPOINT) + .query({ timestamp: 1700000000 }) + .reply(200, blocklist); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:getC2DomainBlocklist', + 1700000000, + ); + + expect(response).toStrictEqual(blocklist); + }); + + it('throws if the API returns a malformed response', async () => { + nock(CLIENT_SIDE_DETECION_BASE_URL) + .get(C2_DOMAIN_BLOCKLIST_ENDPOINT) + .reply(200, { recentlyAdded: 'not an array' }); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:getC2DomainBlocklist'), + ).rejects.toThrow( + 'Malformed response received from C2 domain blocklist endpoint', + ); + }); + }); + + describe('scanUrl', () => { + it('returns the scan result from the API', async () => { + nock(PHISHING_DETECTION_BASE_URL) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: 'example.com' }) + .reply(200, { hostname: 'example.com', recommendedAction: 'NONE' }); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:scanUrl', + 'example.com', + ); + + expect(response).toStrictEqual({ + hostname: 'example.com', + recommendedAction: 'NONE', + }); + }); + + it('serves a repeated scan of the same URL from the cache within the stale time', async () => { + jest.useFakeTimers({ + doNotFake: ['nextTick', 'queueMicrotask'], + now: 1_000_000, + }); + nock(PHISHING_DETECTION_BASE_URL) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: 'example.com' }) + .reply(200, { recommendedAction: 'NONE' }) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: 'example.com' }) + .reply(200, { recommendedAction: 'BLOCK' }); + const { rootMessenger } = createService(); + + const response1 = await rootMessenger.call( + 'PhishingDataService:scanUrl', + 'example.com', + ); + const response2 = await rootMessenger.call( + 'PhishingDataService:scanUrl', + 'example.com', + ); + expect(response1).toStrictEqual(response2); + + // Once the result goes stale, the URL is scanned again. + jest.advanceTimersByTime(SCAN_RESULT_STALE_TIME + 1); + const response3 = await rootMessenger.call( + 'PhishingDataService:scanUrl', + 'example.com', + ); + expect(response3).toStrictEqual({ recommendedAction: 'BLOCK' }); + }); + + it('throws if the API returns a non-200 status', async () => { + nock(PHISHING_DETECTION_BASE_URL) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: 'example.com' }) + .reply(404); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:scanUrl', 'example.com'), + ).rejects.toThrow('404 Not Found'); + }); + + it('throws if the API returns a malformed response', async () => { + nock(PHISHING_DETECTION_BASE_URL) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: 'example.com' }) + .reply(200, {}); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:scanUrl', 'example.com'), + ).rejects.toThrow('Malformed response received from URL scan endpoint'); + }); + }); + + describe('bulkScanUrls', () => { + it('returns the scan results from the API', async () => { + const urls = ['https://example1.com', 'https://example2.com']; + const apiResponse = { + results: { + 'https://example1.com': { + hostname: 'example1.com', + recommendedAction: 'NONE', + }, + 'https://example2.com': { + hostname: 'example2.com', + recommendedAction: 'BLOCK', + }, + }, + errors: {}, + }; + nock(PHISHING_DETECTION_BASE_URL) + .post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, { urls }) + .reply(200, apiResponse); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:bulkScanUrls', + urls, + ); + + expect(response).toStrictEqual(apiResponse); + }); + + it('throws if the API returns a malformed response', async () => { + nock(PHISHING_DETECTION_BASE_URL) + .post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`) + .reply(200, { results: {} }); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:bulkScanUrls', [ + 'https://example1.com', + ]), + ).rejects.toThrow( + 'Malformed response received from bulk URL scan endpoint', + ); + }); + }); + + describe('bulkScanTokens', () => { + it('returns the scan results from the API', async () => { + const tokens = ['0x1234567890123456789012345678901234567890']; + const apiResponse = { + results: { + [tokens[0]]: { result_type: 'Benign' }, + }, + }; + nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT, { chain: 'ethereum', tokens }) + .reply(200, apiResponse); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:bulkScanTokens', + 'ethereum', + tokens, + ); + + expect(response).toStrictEqual(apiResponse); + }); + + it('accepts a response without a results field', async () => { + nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT) + .reply(200, {}); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:bulkScanTokens', + 'ethereum', + ['0x1234567890123456789012345678901234567890'], + ); + + expect(response).toStrictEqual({ results: {} }); + }); + + it('throws if the API returns a malformed response', async () => { + nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT) + .reply(200, { results: 'not a record' }); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call('PhishingDataService:bulkScanTokens', 'ethereum', [ + '0x1234567890123456789012345678901234567890', + ]), + ).rejects.toThrow( + 'Malformed response received from bulk token scan endpoint', + ); + }); + }); + + describe('scanToken', () => { + it('returns the scan result for a single token from the bulk API', async () => { + const token = '0x1234567890123456789012345678901234567890'; + nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT, { + chain: 'ethereum', + tokens: [token], + }) + .reply(200, { + results: { + [token]: { result_type: 'Benign' }, + }, + }); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:scanToken', + 'ethereum', + token, + ); + + expect(response).toStrictEqual({ result_type: 'Benign' }); + }); + + it('returns null if the API returned no result for the token', async () => { + const token = '0x1234567890123456789012345678901234567890'; + nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT) + .reply(200, { results: {} }); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:scanToken', + 'ethereum', + token, + ); + + expect(response).toBeNull(); + }); + + it('shares cached results with bulkScanTokens', async () => { + const token = '0x1234567890123456789012345678901234567890'; + nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT, { + chain: 'ethereum', + tokens: [token], + }) + .reply(200, { + results: { + [token]: { result_type: 'Malicious' }, + }, + }); + const { rootMessenger } = createService(); + + await rootMessenger.call( + 'PhishingDataService:bulkScanTokens', + 'ethereum', + [token], + ); + + // Served from the cache; there is no remaining nock interceptor, so a + // fetch would throw. + const response = await rootMessenger.call( + 'PhishingDataService:scanToken', + 'ethereum', + token, + ); + + expect(response).toStrictEqual({ result_type: 'Malicious' }); + }); + }); + + describe('batching', () => { + it('splits large token batches into requests of up to 100 tokens', async () => { + const tokens = Array.from( + { length: 120 }, + (_, index) => `0x${index.toString().padStart(40, '0')}`, + ); + const firstChunk = tokens.slice(0, 100); + const secondChunk = tokens.slice(100); + const buildResults = (chunk: string[]): TokenScanApiResponse['results'] => + Object.fromEntries( + chunk.map((token) => [ + token, + { result_type: TokenScanResultType.Benign }, + ]), + ); + + const scope = nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT, { + chain: 'ethereum', + tokens: firstChunk, + }) + .reply(200, { results: buildResults(firstChunk) }) + .post(TOKEN_BULK_SCANNING_ENDPOINT, { + chain: 'ethereum', + tokens: secondChunk, + }) + .reply(200, { results: buildResults(secondChunk) }); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:bulkScanTokens', + 'ethereum', + tokens, + ); + + expect(scope.isDone()).toBe(true); + expect(Object.keys(response.results ?? {})).toHaveLength(120); + }); + + it('coalesces retried queries into a new batched request', async () => { + const token = '0x1234567890123456789012345678901234567890'; + const scope = nock(SECURITY_ALERTS_BASE_URL) + .post(TOKEN_BULK_SCANNING_ENDPOINT) + .reply(500) + .post(TOKEN_BULK_SCANNING_ENDPOINT) + .reply(200, { + results: { + [token]: { result_type: 'Benign' }, + }, + }); + const { rootMessenger } = createService({ + options: { + policyOptions: { maxRetries: 1, backoff: new ConstantBackoff(0) }, + }, + }); + + const response = await rootMessenger.call( + 'PhishingDataService:scanToken', + 'ethereum', + token, + ); + + expect(scope.isDone()).toBe(true); + expect(response).toStrictEqual({ result_type: 'Benign' }); + }); + }); + + describe('scanAddress', () => { + it('returns the scan result from the API', async () => { + nock(SECURITY_ALERTS_BASE_URL) + .post(ADDRESS_SCAN_ENDPOINT, { + chain: 'ethereum', + address: '0x1234567890123456789012345678901234567890', + }) + .reply(200, { result_type: 'Benign', label: '' }); + const { rootMessenger } = createService(); + + const response = await rootMessenger.call( + 'PhishingDataService:scanAddress', + 'ethereum', + '0x1234567890123456789012345678901234567890', + ); + + expect(response).toStrictEqual({ result_type: 'Benign', label: '' }); + }); + + it('throws if the API returns a malformed response', async () => { + nock(SECURITY_ALERTS_BASE_URL).post(ADDRESS_SCAN_ENDPOINT).reply(200, {}); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call( + 'PhishingDataService:scanAddress', + 'ethereum', + '0x1234567890123456789012345678901234567890', + ), + ).rejects.toThrow( + 'Malformed response received from address scan endpoint', + ); + }); + }); + + describe('getApprovals', () => { + it('returns the approvals from the API without caching them', async () => { + const firstResponse = { approvals: [] }; + const secondResponse = { + approvals: [ + { + allowance: {}, + asset: {}, + exposure: {}, + spender: {}, + verdict: 'Benign', + }, + ], + }; + nock(SECURITY_ALERTS_BASE_URL) + .post(APPROVALS_ENDPOINT, { + chain: 'ethereum', + address: '0x1234567890123456789012345678901234567890', + }) + .reply(200, firstResponse) + .post(APPROVALS_ENDPOINT, { + chain: 'ethereum', + address: '0x1234567890123456789012345678901234567890', + }) + .reply(200, secondResponse); + const { rootMessenger } = createService(); + + const response1 = await rootMessenger.call( + 'PhishingDataService:getApprovals', + 'ethereum', + '0x1234567890123456789012345678901234567890', + ); + const response2 = await rootMessenger.call( + 'PhishingDataService:getApprovals', + 'ethereum', + '0x1234567890123456789012345678901234567890', + ); + + expect(response1).toStrictEqual(firstResponse); + expect(response2).toStrictEqual(secondResponse); + }); + + it('throws if the API returns a malformed response', async () => { + nock(SECURITY_ALERTS_BASE_URL) + .post(APPROVALS_ENDPOINT) + .reply(200, { approvals: 'not an array' }); + const { rootMessenger } = createService(); + + await expect( + rootMessenger.call( + 'PhishingDataService:getApprovals', + 'ethereum', + '0x1234567890123456789012345678901234567890', + ), + ).rejects.toThrow('Malformed response received from approvals endpoint'); + }); + }); + + describe('persistence', () => { + it('persists the query cache using the StorageService by default', async () => { + jest.useFakeTimers({ + doNotFake: ['nextTick', 'queueMicrotask'], + now: 1_000_000, + }); + nock(PHISHING_DETECTION_BASE_URL) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: 'example.com' }) + .reply(200, { recommendedAction: 'NONE' }); + + const setItem = jest.fn(); + const { rootMessenger } = createService({ + options: { persistenceConfig: undefined }, + setItemMock: setItem, + }); + + await rootMessenger.call('PhishingDataService:scanUrl', 'example.com'); + + // The persistence write is debounced; advance past the write delay. + jest.advanceTimersByTime(15_000); + await flushPromises(); + + expect(setItem).toHaveBeenCalledWith( + 'PhishingDataService', + 'cache', + expect.objectContaining({ + timestamp: expect.any(Number), + state: expect.any(Object), + }), + ); + }); + }); + + describe('direct method calls', () => { + it('does the same thing as the messenger action', async () => { + nock(PHISHING_CONFIG_BASE_URL) + .get(METAMASK_STALELIST_FILE) + .reply(200, STALELIST_RESPONSE); + const { service } = createService(); + + const response = await service.getStalelist(); + + expect(response).toStrictEqual(STALELIST_RESPONSE); + }); + }); +}); + +/** + * The type of the messenger populated with all external actions and events + * required by the service under test. + */ +type RootMessenger = Messenger< + MockAnyNamespace, + MessengerActions, + MessengerEvents +>; + +/** + * Constructs the messenger populated with all external actions and events + * required by the service under test. + * + * @returns The root messenger. + */ +function createRootMessenger(): RootMessenger { + return new Messenger({ namespace: MOCK_ANY_NAMESPACE }); +} + +/** + * Constructs the service under test. + * + * @param args - The arguments to this function. + * @param args.options - The options that the service constructor takes. All + * are optional and will be filled in with defaults as needed (including + * `messenger`). + * @param args.setItemMock - Optional mock `StorageService:setItem` handler to + * register and delegate to the service messenger, enabling persistence. + * @returns The new service, root messenger, and service messenger. + */ +function createService({ + options = {}, + setItemMock, +}: { + options?: Partial[0]>; + setItemMock?: jest.Mock; +} = {}): { + service: PhishingDataService; + rootMessenger: RootMessenger; + messenger: PhishingDataServiceMessenger; +} { + const rootMessenger = createRootMessenger(); + const messenger: PhishingDataServiceMessenger = new Messenger({ + namespace: 'PhishingDataService', + parent: rootMessenger, + }); + if (setItemMock) { + rootMessenger.registerActionHandler('StorageService:setItem', setItemMock); + rootMessenger.delegate({ + actions: ['StorageService:setItem'], + messenger, + }); + } + const service = new PhishingDataService({ + messenger, + policyOptions: { maxRetries: 0 }, + persistenceConfig: null, + ...options, + }); + createdServices.push(service); + + return { service, rootMessenger, messenger }; +} diff --git a/packages/phishing-controller/src/PhishingDataService.ts b/packages/phishing-controller/src/PhishingDataService.ts new file mode 100644 index 00000000000..51f098b372f --- /dev/null +++ b/packages/phishing-controller/src/PhishingDataService.ts @@ -0,0 +1,789 @@ +import { BaseDataService } from '@metamask/base-data-service'; +import type { + CreateServicePolicyOptions, + DataServiceCacheUpdatedEvent, + DataServiceGranularCacheUpdatedEvent, + DataServiceInvalidateQueriesAction, + PersistenceConfiguration, +} from '@metamask/base-data-service'; +import { HttpError } from '@metamask/controller-utils'; +import type { Messenger } from '@metamask/messenger'; +import type { Infer, Struct } from '@metamask/superstruct'; +import { + array, + is, + number, + optional, + record, + string, + type, + unknown, +} from '@metamask/superstruct'; +import type { + StorageServiceGetItemAction, + StorageServiceRemoveItemAction, + StorageServiceSetItemAction, +} from '@metamask/storage-service'; +import { Duration, inMilliseconds } from '@metamask/utils'; +import type { Json } from '@metamask/utils'; +import type { QueryClientConfig } from '@tanstack/query-core'; + +import type { PhishingDataServiceMethodActions } from './PhishingDataService-method-action-types.js'; +import type { + AddressScanResult, + ApprovalsResponse, + BulkPhishingDetectionScanResponse, + C2DomainBlocklistResponse, + DataResultWrapper, + Hotlist, + PhishingDetectionScanResult, + PhishingStalelist, + TokenScanApiResponse, +} from './types.js'; +import { getHostnameFromWebUrl } from './utils.js'; + +/** + * A single token's scan result as returned by the bulk token scanning + * endpoint. + */ +export type TokenScanResultResponse = TokenScanApiResponse['results'][string]; + +// === GENERAL === + +/** + * The name of the {@link PhishingDataService}, used to namespace the service's + * actions and events. + */ +export const serviceName = 'PhishingDataService'; + +export const PHISHING_CONFIG_BASE_URL = + 'https://phishing-detection.api.cx.metamask.io'; +export const METAMASK_STALELIST_FILE = '/v1/stalelist'; +export const METAMASK_HOTLIST_DIFF_FILE = '/v2/diffsSince'; + +export const CLIENT_SIDE_DETECION_BASE_URL = + 'https://client-side-detection.api.cx.metamask.io'; +export const C2_DOMAIN_BLOCKLIST_ENDPOINT = '/v1/request-blocklist'; + +export const PHISHING_DETECTION_BASE_URL = + 'https://dapp-scanning.api.cx.metamask.io'; +export const PHISHING_DETECTION_SCAN_ENDPOINT = 'v2/scan'; +export const PHISHING_DETECTION_BULK_SCAN_ENDPOINT = 'bulk-scan'; + +export const SECURITY_ALERTS_BASE_URL = + 'https://security-alerts.api.cx.metamask.io'; +export const TOKEN_BULK_SCANNING_ENDPOINT = '/token/scan-bulk'; +export const ADDRESS_SCAN_ENDPOINT = '/address/evm/scan'; +export const APPROVALS_ENDPOINT = '/address/evm/approvals'; + +export const METAMASK_STALELIST_URL = `${PHISHING_CONFIG_BASE_URL}${METAMASK_STALELIST_FILE}`; +export const METAMASK_HOTLIST_DIFF_URL = `${PHISHING_CONFIG_BASE_URL}${METAMASK_HOTLIST_DIFF_FILE}`; +export const C2_DOMAIN_BLOCKLIST_URL = `${CLIENT_SIDE_DETECION_BASE_URL}${C2_DOMAIN_BLOCKLIST_ENDPOINT}`; + +/** + * The maximum number of URLs sent to the bulk dapp-scanning endpoint in one + * request. + */ +const MAX_URLS_PER_SCAN_REQUEST = 50; + +/** + * The maximum number of tokens sent to the bulk token scanning endpoint in + * one request. + */ +const MAX_TOKENS_PER_SCAN_REQUEST = 100; + +/** + * How long scan results (URL, bulk URL, token, and address scans) are + * considered fresh by the query cache. Mirrors the 1-minute TTL previously + * enforced by the controller's scan caches; scan verdicts can change quickly, + * so this value is a security parameter and should not be raised casually. + */ +export const SCAN_RESULT_STALE_TIME = inMilliseconds(1, Duration.Minute); + +/** + * Default persistence configuration for the service's query cache. The max + * age matches the longest useful lifetime of any cached entry: scan results + * go stale after {@link SCAN_RESULT_STALE_TIME} and list queries are always + * refetched, so a persisted cache older than this holds nothing usable. + */ +export const DEFAULT_PHISHING_PERSISTENCE_CONFIG: PersistenceConfiguration = { + maxAge: inMilliseconds(5, Duration.Minute), +}; + +// === MESSENGER === + +/** + * All of the methods within {@link PhishingDataService} that are exposed via + * the messenger. + */ +const MESSENGER_EXPOSED_METHODS = [ + 'getStalelist', + 'getHotlistDiffs', + 'getC2DomainBlocklist', + 'scanUrl', + 'bulkScanUrls', + 'scanToken', + 'bulkScanTokens', + 'scanAddress', + 'getApprovals', +] as const; + +/** + * Invalidates cached queries for {@link PhishingDataService}. + */ +export type PhishingDataServiceInvalidateQueriesAction = + DataServiceInvalidateQueriesAction; + +/** + * Actions that {@link PhishingDataService} exposes to other consumers. + */ +export type PhishingDataServiceActions = + | PhishingDataServiceMethodActions + | PhishingDataServiceInvalidateQueriesAction; + +/** + * Actions from other messengers that {@link PhishingDataService} calls. + * The StorageService actions are required for query cache persistence. + */ +type AllowedActions = + | StorageServiceGetItemAction + | StorageServiceSetItemAction + | StorageServiceRemoveItemAction; + +/** + * Published when {@link PhishingDataService}'s cache is updated. + */ +export type PhishingDataServiceCacheUpdatedEvent = DataServiceCacheUpdatedEvent< + typeof serviceName +>; + +/** + * Published when a key within {@link PhishingDataService}'s cache is updated. + */ +export type PhishingDataServiceGranularCacheUpdatedEvent = + DataServiceGranularCacheUpdatedEvent; + +/** + * Events that {@link PhishingDataService} exposes to other consumers. + */ +export type PhishingDataServiceEvents = + | PhishingDataServiceCacheUpdatedEvent + | PhishingDataServiceGranularCacheUpdatedEvent; + +/** + * Events from other messengers that {@link PhishingDataService} subscribes to. + */ +type AllowedEvents = never; + +/** + * The messenger which is restricted to actions and events accessed by + * {@link PhishingDataService}. + */ +export type PhishingDataServiceMessenger = Messenger< + typeof serviceName, + PhishingDataServiceActions | AllowedActions, + PhishingDataServiceEvents | AllowedEvents +>; + +// === RESPONSE VALIDATION === + +// The structs below intentionally validate only the shape that the consuming +// code depends on for control flow, mirroring the tolerance of the previous +// in-controller fetching: a response that is missing auxiliary fields is +// passed through rather than rejected. +const StalelistResponseStruct = type({ + data: type({ + lastUpdated: number(), + }), +}); + +const HotlistDiffsResponseStruct = type({ + data: array(unknown()), +}); + +const C2DomainBlocklistResponseStruct = type({ + recentlyAdded: array(string()), + recentlyRemoved: array(string()), +}); + +const ScanUrlResponseStruct = type({ + recommendedAction: string(), +}); + +const BulkScanUrlsResponseStruct = type({ + results: record(string(), unknown()), + errors: record(string(), array(string())), +}); + +const BulkScanTokensResponseStruct = type({ + results: optional(record(string(), unknown())), +}); + +const ScanAddressResponseStruct = type({ + result_type: string(), +}); + +const ApprovalsResponseStruct = type({ + approvals: array(unknown()), +}); + +// === BATCH LOADING === + +type BatchLoader = { + /** + * Registers an item to be resolved by the next executed batch. + * + * @param key - The item key, as understood by the batch endpoint. + * @returns The item's result, or `null` if the batch response did not + * include it. + */ + load: (key: string) => Promise; + /** + * Executes all pending items, in requests of up to the configured batch + * size. Items registered after a flush (e.g. by a retry) are scheduled for + * a later flush automatically. + */ + flush: () => void; +}; + +/** + * Creates a loader that coalesces individual item lookups into batched + * requests. This preserves the per-item caching granularity of the query + * cache while keeping the batched network behavior of the bulk endpoints. + * + * @param options - The loader options. + * @param options.maxBatchSize - The maximum number of items per request. + * @param options.executeBatch - Executes one batched request, returning + * results keyed by item. + * @returns The batch loader. + */ +function createBatchLoader({ + maxBatchSize, + executeBatch, +}: { + maxBatchSize: number; + executeBatch: (keys: string[]) => Promise>; +}): BatchLoader { + type PendingItem = { + key: string; + resolve: (value: Json | null) => void; + reject: (error: unknown) => void; + }; + let pending: PendingItem[] = []; + let flushScheduled = false; + + const executeChunk = async (chunk: PendingItem[]): Promise => { + try { + const results = await executeBatch(chunk.map((item) => item.key)); + for (const item of chunk) { + item.resolve(results[item.key] ?? null); + } + } catch (error) { + for (const item of chunk) { + item.reject(error); + } + } + }; + + const flush = (): void => { + flushScheduled = false; + const batch = pending; + pending = []; + for (let index = 0; index < batch.length; index += maxBatchSize) { + // Errors are routed to the chunk's items, so this promise never + // rejects. + executeChunk(batch.slice(index, index + maxBatchSize)).catch( + /* istanbul ignore next */ + () => undefined, + ); + } + }; + + return { + async load(key: string): Promise { + return new Promise((resolve, reject) => { + pending.push({ key, resolve, reject }); + // Items registered outside an explicit flush (e.g. by the retry + // policy re-running a query) are coalesced via the microtask queue. + if (!flushScheduled) { + flushScheduled = true; + queueMicrotask(() => { + if (flushScheduled) { + flush(); + } + }); + } + }); + }, + flush, + }; +} + +// === SERVICE DEFINITION === + +/** + * This service is responsible for all network requests made on behalf of + * `PhishingController`: fetching the phishing configuration lists (stalelist, + * hotlist diffs, and C2 domain blocklist) and calling the dapp-scanning and + * security-alerts APIs (URL, token, and address scans). + * + * Scan results are cached by the underlying query cache for + * {@link SCAN_RESULT_STALE_TIME} and persisted between sessions when + * `persistenceConfig` is enabled (the default), which requires the + * `StorageService:getItem`, `StorageService:setItem`, and + * `StorageService:removeItem` messenger actions to be delegated to this + * service's messenger, plus a call to `init` during client initialization. + * + * List queries are always refetched when requested; the controller remains + * responsible for deciding when the lists are out of date. + * + * Note that a single retry/circuit-breaker policy is shared across all + * endpoints of this service. The policy only counts consecutive failures, so + * an outage of one API is unlikely to pause requests to the others unless + * failures arrive without any interleaved successes. + */ +export class PhishingDataService extends BaseDataService< + typeof serviceName, + PhishingDataServiceMessenger +> { + /** + * Constructs a new PhishingDataService object. + * + * @param args - The constructor arguments. + * @param args.messenger - The messenger suited for this service. + * @param args.queryClientConfig - Configuration for the underlying TanStack + * Query client. + * @param args.policyOptions - Options to pass to `createServicePolicy`, + * which is used to wrap each request. See + * {@link CreateServicePolicyOptions}. + * @param args.persistenceConfig - Configuration for persisting the query + * cache between sessions. Defaults to + * {@link DEFAULT_PHISHING_PERSISTENCE_CONFIG}; pass `null` to disable + * persistence. + */ + constructor({ + messenger, + queryClientConfig = {}, + policyOptions = {}, + persistenceConfig = DEFAULT_PHISHING_PERSISTENCE_CONFIG, + }: { + messenger: PhishingDataServiceMessenger; + queryClientConfig?: QueryClientConfig; + policyOptions?: CreateServicePolicyOptions; + persistenceConfig?: PersistenceConfiguration | null; + }) { + super({ + name: serviceName, + messenger, + queryClientConfig, + // Circuit breaking is disabled by default: this service talks to four + // independent API hosts through a single shared policy, so a broken + // circuit caused by one host's outage would also pause phishing-list + // updates from the others. Protection against hammering a failing host + // comes from the controller's refresh-interval bookkeeping and the scan + // result stale times, matching the previous in-controller behavior. + policyOptions: { + maxConsecutiveFailures: Number.MAX_SAFE_INTEGER, + ...policyOptions, + }, + persistenceConfig: persistenceConfig ?? undefined, + }); + + this.messenger.registerMethodActionHandlers( + this, + MESSENGER_EXPOSED_METHODS, + ); + } + + /** + * Fetches the full phishing detection stalelist. + * + * @returns The stalelist response. + */ + async getStalelist(): Promise> { + const jsonResponse = await this.fetchQuery({ + queryKey: [`${this.name}:getStalelist`], + queryFn: async () => this.#getJson(METAMASK_STALELIST_URL), + staleTime: 0, + }); + + return this.#validate( + jsonResponse, + StalelistResponseStruct, + 'stalelist', + ) as DataResultWrapper; + } + + /** + * Fetches the hotlist diffs recorded since the given timestamp. + * + * @param timestamp - The timestamp (in seconds) to fetch diffs since. + * @returns The hotlist diffs response. + */ + async getHotlistDiffs( + timestamp: number, + ): Promise> { + const jsonResponse = await this.fetchQuery({ + queryKey: [`${this.name}:getHotlistDiffs`, timestamp], + queryFn: async () => + this.#getJson(`${METAMASK_HOTLIST_DIFF_URL}/${timestamp}`), + staleTime: 0, + }); + + return this.#validate( + jsonResponse, + HotlistDiffsResponseStruct, + 'hotlist diffs', + ) as DataResultWrapper; + } + + /** + * Fetches the C2 domain blocklist changes recorded since the given + * timestamp, or the current blocklist if no timestamp is given. + * + * @param timestamp - The timestamp (in seconds) to fetch changes since. + * @returns The C2 domain blocklist response. + */ + async getC2DomainBlocklist( + timestamp?: number, + ): Promise { + const url = + timestamp === undefined + ? C2_DOMAIN_BLOCKLIST_URL + : `${C2_DOMAIN_BLOCKLIST_URL}?timestamp=${timestamp}`; + + const jsonResponse = await this.fetchQuery({ + queryKey: [`${this.name}:getC2DomainBlocklist`, timestamp ?? null], + queryFn: async () => this.#getJson(url), + staleTime: 0, + }); + + return this.#validate( + jsonResponse, + C2DomainBlocklistResponseStruct, + 'C2 domain blocklist', + ) as C2DomainBlocklistResponse; + } + + /** + * Scans a URL for phishing via the dapp-scanning API. + * + * @param url - The prepared URL parameter to scan (hostname, or hostname + * plus path for shared gateways). + * @returns The phishing detection scan result. + */ + async scanUrl(url: string): Promise { + const jsonResponse = await this.fetchQuery({ + queryKey: [`${this.name}:scanUrl`, url], + queryFn: async () => { + const response = await fetch( + `${PHISHING_DETECTION_BASE_URL}/${PHISHING_DETECTION_SCAN_ENDPOINT}?url=${encodeURIComponent(url)}`, + { + method: 'GET', + headers: { + Accept: 'application/json', + }, + }, + ); + return this.#toJson(response); + }, + staleTime: SCAN_RESULT_STALE_TIME, + }); + + return this.#validate( + jsonResponse, + ScanUrlResponseStruct, + 'URL scan', + ) as PhishingDetectionScanResult; + } + + /** + * Scans a batch of URLs for phishing via the dapp-scanning API. + * + * Results are cached per hostname using the same query keys as + * {@link PhishingDataService.scanUrl}, so results are shared between single + * and bulk scans. Only hostnames without a fresh cached result are sent to + * the API, in requests of up to 50 URLs. + * + * @param urls - The URLs to scan. + * @returns The scan results, keyed by URL, and any batch-level errors. + */ + async bulkScanUrls( + urls: string[], + ): Promise { + const errors: Record = {}; + const loader = createBatchLoader({ + maxBatchSize: MAX_URLS_PER_SCAN_REQUEST, + executeBatch: async (batchUrls) => { + const jsonResponse = await this.#postJson( + `${PHISHING_DETECTION_BASE_URL}/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, + { urls: batchUrls }, + ); + const response = this.#validate( + jsonResponse, + BulkScanUrlsResponseStruct, + 'bulk URL scan', + ) as BulkPhishingDetectionScanResponse; + for (const [key, messages] of Object.entries(response.errors)) { + errors[key] = [...(errors[key] ?? []), ...messages]; + } + return response.results as Record; + }, + }); + + const entries = urls.map((url) => { + const [hostname] = getHostnameFromWebUrl(url); + return this.fetchQuery({ + queryKey: [`${this.name}:scanUrl`, hostname], + queryFn: async () => loader.load(url), + staleTime: SCAN_RESULT_STALE_TIME, + }).then((result) => [url, hostname, result] as const); + }); + loader.flush(); + + const results: Record = {}; + for (const [url, hostname, result] of await Promise.all(entries)) { + if (result !== null) { + const scanResult = result as PhishingDetectionScanResult; + // Entries seeded by single-URL scans hold the raw scan response, + // which may not include the hostname; fill it in from the URL. + results[url] = { + ...scanResult, + hostname: scanResult.hostname ?? hostname, + }; + } + } + + return { results, errors }; + } + + /** + * Scans a token for malicious activity via the security-alerts API. + * + * Requests made while a bulk scan is being assembled are coalesced into a + * single request to the bulk scanning endpoint. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param token - The token address to scan. + * @returns The token scan result, or `null` if the API returned no result + * for the token. + */ + async scanToken( + chain: string, + token: string, + ): Promise { + const loader = this.#createTokenScanLoader(chain); + const result = this.#fetchTokenScanQuery(loader, chain, token); + loader.flush(); + return await result; + } + + /** + * Scans a batch of tokens for malicious activity via the security-alerts + * API. + * + * Results are cached per token; only tokens without a fresh cached result + * are sent to the API, in requests of up to 100 tokens. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param tokens - The token addresses to scan. + * @returns The token scan results, keyed by token address. Tokens for which + * the API returned no result are omitted. + */ + async bulkScanTokens( + chain: string, + tokens: string[], + ): Promise { + const loader = this.#createTokenScanLoader(chain); + const entries = tokens.map((token) => + this.#fetchTokenScanQuery(loader, chain, token).then( + (result) => [token, result] as const, + ), + ); + loader.flush(); + + const results: TokenScanApiResponse['results'] = {}; + for (const [token, result] of await Promise.all(entries)) { + if (result !== null) { + results[token] = result; + } + } + + return { results }; + } + + /** + * Creates a batch loader that resolves token scans through the bulk + * scanning endpoint. + * + * @param chain - The chain name (e.g. `ethereum`). + * @returns The batch loader. + */ + #createTokenScanLoader(chain: string): BatchLoader { + return createBatchLoader({ + maxBatchSize: MAX_TOKENS_PER_SCAN_REQUEST, + executeBatch: async (batchTokens) => { + const jsonResponse = await this.#postJson( + `${SECURITY_ALERTS_BASE_URL}${TOKEN_BULK_SCANNING_ENDPOINT}`, + { chain, tokens: batchTokens }, + ); + const response = this.#validate( + jsonResponse, + BulkScanTokensResponseStruct, + 'bulk token scan', + ) as TokenScanApiResponse; + return (response.results ?? {}) as Record; + }, + }); + } + + /** + * Fetches a single token scan query backed by the given batch loader. + * + * @param loader - The batch loader used to resolve cache misses. + * @param chain - The chain name (e.g. `ethereum`). + * @param token - The token address to scan. + * @returns The token scan result, or `null` if the API returned no result. + */ + async #fetchTokenScanQuery( + loader: BatchLoader, + chain: string, + token: string, + ): Promise { + const result = await this.fetchQuery({ + queryKey: [`${this.name}:scanToken`, chain, token], + queryFn: async () => loader.load(token), + staleTime: SCAN_RESULT_STALE_TIME, + }); + return result as TokenScanResultResponse | null; + } + + /** + * Scans an address for security alerts via the security-alerts API. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param address - The address to scan. + * @returns The address scan result. + */ + async scanAddress( + chain: string, + address: string, + ): Promise { + const jsonResponse = await this.fetchQuery({ + queryKey: [`${this.name}:scanAddress`, chain, address], + queryFn: async () => + this.#postJson(`${SECURITY_ALERTS_BASE_URL}${ADDRESS_SCAN_ENDPOINT}`, { + chain, + address, + }), + staleTime: SCAN_RESULT_STALE_TIME, + }); + + return this.#validate( + jsonResponse, + ScanAddressResponseStruct, + 'address scan', + ) as AddressScanResult; + } + + /** + * Gets token approvals for an address with security enrichments via the + * security-alerts API. Approvals reflect live account state and are never + * cached. + * + * @param chain - The chain name (e.g. `ethereum`). + * @param address - The address to get approvals for. + * @returns The approvals response. + */ + async getApprovals( + chain: string, + address: string, + ): Promise { + const jsonResponse = await this.fetchQuery({ + queryKey: [`${this.name}:getApprovals`, chain, address], + queryFn: async () => + this.#postJson(`${SECURITY_ALERTS_BASE_URL}${APPROVALS_ENDPOINT}`, { + chain, + address, + }), + staleTime: 0, + cacheTime: 0, + }); + + return this.#validate( + jsonResponse, + ApprovalsResponseStruct, + 'approvals', + ) as ApprovalsResponse; + } + + /** + * Performs a GET request against a phishing configuration endpoint. + * + * @param url - The URL to fetch. + * @returns The parsed JSON response. + */ + async #getJson(url: string): Promise { + const response = await fetch(url, { cache: 'no-cache' }); + return this.#toJson(response); + } + + /** + * Performs a POST request with a JSON body. + * + * @param url - The URL to fetch. + * @param body - The request body, serialized as JSON. + * @returns The parsed JSON response. + */ + async #postJson(url: string, body: Record): Promise { + const response = await fetch(url, { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + }, + body: JSON.stringify(body), + }); + return this.#toJson(response); + } + + /** + * Parses a response as JSON, throwing an {@link HttpError} for non-2xx + * responses. The error message intentionally matches the + * ` ` format historically produced by + * `PhishingController` so that consumers relying on it keep working. + * + * @param response - The response to parse. + * @returns The parsed JSON response. + */ + async #toJson(response: Response): Promise { + if (!response.ok) { + throw new HttpError( + response.status, + `${response.status} ${response.statusText}`, + ); + } + return response.json(); + } + + /** + * Validates a response against a struct, throwing if it is malformed. + * + * @param response - The response to validate. + * @param struct - The struct to validate against. + * @param endpointName - The name of the endpoint, used in error messages. + * @returns The validated response. + */ + #validate( + response: unknown, + struct: Struct, + endpointName: string, + ): Infer> { + if (!is(response, struct)) { + throw new Error( + `Malformed response received from ${endpointName} endpoint`, + ); + } + return response; + } +} diff --git a/packages/phishing-controller/src/index.ts b/packages/phishing-controller/src/index.ts index 0f963ea60c4..6dbdf8876e4 100644 --- a/packages/phishing-controller/src/index.ts +++ b/packages/phishing-controller/src/index.ts @@ -31,11 +31,11 @@ export { ApprovalResultType, ApprovalFeatureType, } from './types.js'; -export type { CacheEntry } from './CacheManager.js'; export { PHISHING_DETECTION_PATH_BASED_ROOT_DOMAINS, getPhishingDetectionScanUrlParam, isPhishingDetectionPathBasedHostname, + resolveChainName, } from './utils.js'; export type { @@ -50,3 +50,29 @@ export type { PhishingControllerGetApprovalsAction, PhishingControllerCheckAddressPoisoningAction, } from './PhishingController-method-action-types.js'; + +export { + PhishingDataService, + SCAN_RESULT_STALE_TIME, + DEFAULT_PHISHING_PERSISTENCE_CONFIG, +} from './PhishingDataService.js'; +export type { TokenScanResultResponse } from './PhishingDataService.js'; +export type { + PhishingDataServiceActions, + PhishingDataServiceEvents, + PhishingDataServiceMessenger, + PhishingDataServiceInvalidateQueriesAction, + PhishingDataServiceCacheUpdatedEvent, + PhishingDataServiceGranularCacheUpdatedEvent, +} from './PhishingDataService.js'; +export type { + PhishingDataServiceGetStalelistAction, + PhishingDataServiceGetHotlistDiffsAction, + PhishingDataServiceGetC2DomainBlocklistAction, + PhishingDataServiceScanUrlAction, + PhishingDataServiceBulkScanUrlsAction, + PhishingDataServiceScanTokenAction, + PhishingDataServiceBulkScanTokensAction, + PhishingDataServiceScanAddressAction, + PhishingDataServiceGetApprovalsAction, +} from './PhishingDataService-method-action-types.js'; diff --git a/packages/phishing-controller/src/types.ts b/packages/phishing-controller/src/types.ts index 44a9f677d02..b8ac85c7ea2 100644 --- a/packages/phishing-controller/src/types.ts +++ b/packages/phishing-controller/src/types.ts @@ -1,4 +1,178 @@ /* eslint-disable @typescript-eslint/naming-convention */ +import type { PathTrie } from './PathTrie.js'; + +/** + * @type ListTypes + * + * Type outlining the types of lists provided by aggregating different source lists + */ +export type ListTypes = + | 'fuzzylist' + | 'blocklist' + | 'blocklistPaths' + | 'allowlist' + | 'c2DomainBlocklist'; + +/** + * @type EthPhishingResponse + * + * Configuration response from the eth-phishing-detect package + * consisting of approved and unapproved website origins + * + * @property blacklist - List of unapproved origins + * @property fuzzylist - List of fuzzy-matched unapproved origins + * @property tolerance - Fuzzy match tolerance level + * @property version - Version number of this configuration + * @property whitelist - List of approved origins + */ +export type EthPhishingResponse = { + blacklist: string[]; + fuzzylist: string[]; + tolerance: number; + version: number; + whitelist: string[]; +}; + +/** + * @type C2DomainBlocklistResponse + * + * Response for blocklist update requests + * + * @property recentlyAdded - List of c2 domains recently added to the blocklist + * @property recentlyRemoved - List of c2 domains recently removed from the blocklist + * @property lastFetchedAt - Timestamp of the last fetch request + */ +export type C2DomainBlocklistResponse = { + recentlyAdded: string[]; + recentlyRemoved: string[]; + lastFetchedAt: string; +}; + +/** + * PhishingStalelist defines the expected type of the stalelist from the API. + * + * allowlist - List of approved origins. + * blocklist - List of unapproved origins (hostname-only entries). + * blocklistPaths - Trie of unapproved origins with paths (hostname + path entries). + * fuzzylist - List of fuzzy-matched unapproved origins. + * tolerance - Fuzzy match tolerance level + * lastUpdated - Timestamp of last update. + * version - Stalelist data structure iteration. + */ +export type PhishingStalelist = { + allowlist: string[]; + blocklist: string[]; + blocklistPaths: string[]; + fuzzylist: string[]; + tolerance: number; + version: number; + lastUpdated: number; +}; + +/** + * @type PhishingListState + * + * type defining the persisted list state. This is the persisted state that is updated frequently with `this.maybeUpdateState()`. + * + * @property allowlist - List of approved origins (legacy naming "whitelist") + * @property blocklist - List of unapproved origins (legacy naming "blacklist") + * @property blocklistPaths - Trie of unapproved origins with paths (hostname + path, no query params). + * @property c2DomainBlocklist - List of hashed hostnames that C2 requests are blocked against. + * @property fuzzylist - List of fuzzy-matched unapproved origins + * @property tolerance - Fuzzy match tolerance level + * @property lastUpdated - Timestamp of last update. + * @property version - Version of the phishing list state. + * @property name - Name of the list. Used for attribution. + */ +export type PhishingListState = { + allowlist: string[]; + blocklist: string[]; + blocklistPaths: PathTrie; + c2DomainBlocklist: string[]; + fuzzylist: string[]; + tolerance: number; + version: number; + lastUpdated: number; + name: ListNames; +}; + +/** + * @type HotlistDiff + * + * type defining the expected type of the diffs in hotlist.json file. + * + * @property url - Url of the diff entry. + * @property timestamp - Timestamp at which the diff was identified. + * @property targetList - The list name where the diff was identified. + * @property isRemoval - Was the diff identified a removal type. + */ +export type HotlistDiff = { + url: string; + timestamp: number; + targetList: `${ListKeys}.${ListTypes}`; + isRemoval?: boolean; +}; + +export type DataResultWrapper = { + data: T; +}; + +/** + * @type Hotlist + * + * Type defining expected hotlist.json file. + * + * @property url - Url of the diff entry. + * @property timestamp - Timestamp at which the diff was identified. + * @property targetList - The list name where the diff was identified. + * @property isRemoval - Was the diff identified a removal type. + */ +export type Hotlist = HotlistDiff[]; + +/** + * Enum containing upstream data provider source list keys. + * These are the keys denoting lists consumed by the upstream data provider. + */ +export enum ListKeys { + EthPhishingDetectConfig = 'eth_phishing_detect_config', +} + +/** + * Enum containing downstream client attribution names. + */ +export enum ListNames { + MetaMask = 'MetaMask', +} + +/** + * Maps from downstream client attribution name + * to list key sourced from upstream data provider. + */ +export const phishingListNameKeyMap = { + [ListNames.MetaMask]: ListKeys.EthPhishingDetectConfig, +}; + +/** + * Maps from list key sourced from upstream data + * provider to downstream client attribution name. + */ +export const phishingListKeyNameMap = { + [ListKeys.EthPhishingDetectConfig]: ListNames.MetaMask, +}; + +/** + * BulkPhishingDetectionScanResponse + * + * Response for bulk phishing detection scan requests + * results - Record of domain names and their corresponding phishing detection scan results + * + * errors - Record of domain names and their corresponding errors + */ +export type BulkPhishingDetectionScanResponse = { + results: Record; + errors: Record; +}; + /** * Represents the result of checking a domain. */ diff --git a/packages/phishing-controller/src/utils.test.ts b/packages/phishing-controller/src/utils.test.ts index 14330fbdf1e..23bfa50efea 100644 --- a/packages/phishing-controller/src/utils.test.ts +++ b/packages/phishing-controller/src/utils.test.ts @@ -1,9 +1,7 @@ import { ListKeys, ListNames } from './PhishingController.js'; import type { PhishingListState } from './PhishingController.js'; -import type { TokenScanResultType } from './types.js'; import { applyDiffs, - buildCacheKey, domainToParts, fetchTimeNow, generateParentDomains, @@ -20,7 +18,6 @@ import { resolveChainName, roundToNearestMinute, sha256Hash, - splitCacheHits, validateConfig, } from './utils.js'; @@ -1183,43 +1180,6 @@ describe('generateParentDomains', () => { }); }); -describe('buildCacheKey', () => { - it('should create cache key with lowercase chainId and address', () => { - const chainId = '0x1'; - const address = '0x1234ABCD'; - const result = buildCacheKey(chainId, address); - expect(result).toBe('0x1:0x1234abcd'); - }); - - it('should handle already lowercase inputs', () => { - const chainId = '0xa'; - const address = '0xdeadbeef'; - const result = buildCacheKey(chainId, address); - expect(result).toBe('0xa:0xdeadbeef'); - }); - - it('should handle mixed case inputs', () => { - const chainId = '0X89'; - const address = '0XaBcDeF123456'; - const result = buildCacheKey(chainId, address); - expect(result).toBe('0x89:0xabcdef123456'); - }); - - it('should preserve address casing when caseSensitive is true', () => { - const chainId = 'solana'; - const address = 'Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr'; - const result = buildCacheKey(chainId, address, true); - expect(result).toBe('solana:Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr'); - }); - - it('should lowercase address when caseSensitive is false (default)', () => { - const chainId = 'solana'; - const address = 'Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr'; - const result = buildCacheKey(chainId, address); - expect(result).toBe('solana:gh9zwemdlj8dsckntktqpbnwlnnbjuszag9vp2kgtkjr'); - }); -}); - describe('resolveChainName', () => { it('should resolve known chain IDs to chain names', () => { expect(resolveChainName('0x1')).toBe('ethereum'); @@ -1287,133 +1247,6 @@ describe('isAddressScanSupportedChain', () => { }); }); -describe('splitCacheHits', () => { - const mockCache = { - get: jest.fn(), - }; - - beforeEach(() => { - mockCache.get.mockClear(); - }); - - it('should split tokens correctly when some are cached', () => { - const chainId = '0x1'; - const tokens = ['0xTOKEN1', '0xTOKEN2', '0xTOKEN3']; - - // Mock cache to return data for token1 only - const mockResponses = new Map([ - ['0x1:0xtoken1', { result_type: 'Benign' as TokenScanResultType }], - ]); - mockCache.get.mockImplementation((key: string) => mockResponses.get(key)); - - const result = splitCacheHits(mockCache, chainId, tokens); - - expect(result.cachedResults).toStrictEqual({ - '0xtoken1': { - result_type: 'Benign', - chain: '0x1', - address: '0xtoken1', - }, - }); - expect(result.tokensToFetch).toStrictEqual(['0xtoken2', '0xtoken3']); - }); - - it('should handle all tokens being cached', () => { - const chainId = '0x89'; - const tokens = ['0xTOKEN1', '0xTOKEN2']; - - mockCache.get.mockReturnValue({ - result_type: 'Warning' as TokenScanResultType, - }); - - const result = splitCacheHits(mockCache, chainId, tokens); - - expect(result.cachedResults).toStrictEqual({ - '0xtoken1': { - result_type: 'Warning', - chain: '0x89', - address: '0xtoken1', - }, - '0xtoken2': { - result_type: 'Warning', - chain: '0x89', - address: '0xtoken2', - }, - }); - expect(result.tokensToFetch).toStrictEqual([]); - }); - - it('should handle no tokens being cached', () => { - const chainId = '0xa'; - const tokens = ['0xTOKEN1', '0xTOKEN2']; - - mockCache.get.mockReturnValue(undefined); - - const result = splitCacheHits(mockCache, chainId, tokens); - - expect(result.cachedResults).toStrictEqual({}); - expect(result.tokensToFetch).toStrictEqual(['0xtoken1', '0xtoken2']); - }); - - it('should handle empty token list', () => { - const chainId = '0x1'; - const tokens: string[] = []; - - const result = splitCacheHits(mockCache, chainId, tokens); - - expect(result.cachedResults).toStrictEqual({}); - expect(result.tokensToFetch).toStrictEqual([]); - expect(mockCache.get).not.toHaveBeenCalled(); - }); - - it('should normalize addresses to lowercase', () => { - const chainId = '0X1'; - const tokens = ['0XTOKEN1']; - - mockCache.get.mockReturnValue({ - result_type: 'Malicious' as TokenScanResultType, - }); - - const result = splitCacheHits(mockCache, chainId, tokens); - - expect(mockCache.get).toHaveBeenCalledWith('0x1:0xtoken1'); - expect(result.cachedResults).toHaveProperty('0xtoken1'); - expect(result.cachedResults['0xtoken1'].address).toBe('0xtoken1'); - }); - - it('should preserve address casing when caseSensitive is true', () => { - const chainId = 'solana'; - const tokens = ['Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr']; - - mockCache.get.mockReturnValue(undefined); - - const result = splitCacheHits(mockCache, chainId, tokens, true); - - // tokensToFetch should preserve original casing - expect(result.tokensToFetch).toStrictEqual([ - 'Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr', - ]); - }); - - it('should return cached result with preserved casing when caseSensitive is true', () => { - const chainId = 'solana'; - const token = 'Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr'; - - mockCache.get.mockReturnValue({ - result_type: 'Benign' as TokenScanResultType, - }); - - const result = splitCacheHits(mockCache, chainId, [token], true); - - expect(result.cachedResults[token]).toStrictEqual({ - result_type: 'Benign', - chain: 'solana', - address: token, - }); - expect(result.tokensToFetch).toStrictEqual([]); - }); -}); - describe('getHostnameAndPathComponents', () => { it.each([ [ diff --git a/packages/phishing-controller/src/utils.ts b/packages/phishing-controller/src/utils.ts index 57dd71ab5b1..0c59328d4d6 100644 --- a/packages/phishing-controller/src/utils.ts +++ b/packages/phishing-controller/src/utils.ts @@ -2,8 +2,6 @@ import { bytesToHex } from '@noble/hashes/utils'; import { sha256 } from 'ethereum-cryptography/sha256'; import { deleteFromTrie, insertToTrie, deepCopyPathTrie } from './PathTrie.js'; -import type { Hotlist, PhishingListState } from './PhishingController.js'; -import { ListKeys, phishingListKeyNameMap } from './PhishingController.js'; import type { PhishingDetectorList, PhishingDetectorConfiguration, @@ -12,13 +10,15 @@ import { ADDRESS_SCAN_SUPPORTED_CHAINS, APPROVAL_SUPPORTED_CHAINS, DEFAULT_CHAIN_ID_TO_NAME, + ListKeys, + phishingListKeyNameMap, TOKEN_SCAN_SUPPORTED_CHAINS, } from './types.js'; import type { AddressScanSupportedChain, ApprovalSupportedChain, - TokenScanCacheData, - TokenScanResult, + Hotlist, + PhishingListState, TokenScanSupportedChain, } from './types.js'; @@ -483,25 +483,6 @@ export const generateParentDomains = ( return domains; }; -/** - * Builds a cache key for a token scan result. - * - * @param chainId - The chain ID. - * @param address - The token address. - * @param caseSensitive - When `true`, the address is kept as-is (for chains - * like Solana where addresses are case-sensitive). When `false` (default), - * the address is lowercased (appropriate for EVM). - * @returns The cache key. - */ -export const buildCacheKey = ( - chainId: string, - address: string, - caseSensitive = false, -) => { - const normalizedAddress = caseSensitive ? address : address.toLowerCase(); - return `${chainId.toLowerCase()}:${normalizedAddress}`; -}; - /** * Determines whether a chain name is supported for token approval scanning. * @@ -548,45 +529,3 @@ export const resolveChainName = ( ): string | null => { return mapping[chainId.toLowerCase() as keyof typeof mapping] ?? null; }; - -/** - * Split tokens into cached results and tokens that need to be fetched. - * - * @param cache - Cache-like object with get method. - * @param cache.get - Method to retrieve cached data by key. - * @param chainId - The chain ID. - * @param tokens - Array of token addresses. - * @param caseSensitive - When `true`, token addresses are kept as-is (for - * chains like Solana where addresses are case-sensitive). When `false` - * (default), addresses are lowercased (appropriate for EVM). - * @returns Object containing cached results and tokens to fetch. - */ -export const splitCacheHits = ( - cache: { get: (key: string) => TokenScanCacheData | undefined }, - chainId: string, - tokens: string[], - caseSensitive = false, -): { - cachedResults: Record; - tokensToFetch: string[]; -} => { - const cachedResults: Record = {}; - const tokensToFetch: string[] = []; - - for (const address of tokens) { - const normalizedAddress = caseSensitive ? address : address.toLowerCase(); - const key = buildCacheKey(chainId, normalizedAddress, caseSensitive); - const hit = cache.get(key); - if (hit) { - cachedResults[normalizedAddress] = { - result_type: hit.result_type, - chain: chainId, - address: normalizedAddress, - }; - } else { - tokensToFetch.push(normalizedAddress); - } - } - - return { cachedResults, tokensToFetch }; -}; diff --git a/packages/phishing-controller/tsconfig.build.json b/packages/phishing-controller/tsconfig.build.json index 3f312d587cd..c4921cabbd1 100644 --- a/packages/phishing-controller/tsconfig.build.json +++ b/packages/phishing-controller/tsconfig.build.json @@ -6,20 +6,26 @@ "rootDir": "./src" }, "references": [ + { + "path": "../address-book-controller/tsconfig.build.json" + }, { "path": "../base-controller/tsconfig.build.json" }, { - "path": "../controller-utils/tsconfig.build.json" + "path": "../base-data-service/tsconfig.build.json" }, { - "path": "../transaction-controller/tsconfig.build.json" + "path": "../controller-utils/tsconfig.build.json" }, { "path": "../messenger/tsconfig.build.json" }, { - "path": "../address-book-controller/tsconfig.build.json" + "path": "../storage-service/tsconfig.build.json" + }, + { + "path": "../transaction-controller/tsconfig.build.json" } ], "include": ["../../types", "./src"] diff --git a/packages/phishing-controller/tsconfig.json b/packages/phishing-controller/tsconfig.json index 63ae98b5725..2713d5242ba 100644 --- a/packages/phishing-controller/tsconfig.json +++ b/packages/phishing-controller/tsconfig.json @@ -4,20 +4,26 @@ "baseUrl": "./" }, "references": [ + { + "path": "../address-book-controller" + }, { "path": "../base-controller" }, { - "path": "../controller-utils" + "path": "../base-data-service" }, { - "path": "../transaction-controller" + "path": "../controller-utils" }, { "path": "../messenger" }, { - "path": "../address-book-controller" + "path": "../storage-service" + }, + { + "path": "../transaction-controller" } ], "include": ["../../types", "./src", "./tests"] diff --git a/yarn.lock b/yarn.lock index fbead227c86..d6cb8860ed1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8453,10 +8453,15 @@ __metadata: "@metamask/address-book-controller": "npm:^7.1.2" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/base-controller": "npm:^9.1.0" + "@metamask/base-data-service": "npm:^0.1.3" "@metamask/controller-utils": "npm:^12.3.0" "@metamask/messenger": "npm:^2.0.0" + "@metamask/storage-service": "npm:^1.0.2" + "@metamask/superstruct": "npm:^3.4.1" "@metamask/transaction-controller": "npm:^69.5.2" + "@metamask/utils": "npm:^11.11.0" "@noble/hashes": "npm:^1.8.0" + "@tanstack/query-core": "npm:^4.43.0" "@ts-bridge/cli": "npm:^0.6.4" "@types/jest": "npm:^30.0.0" "@types/punycode": "npm:^2.1.0"