diff --git a/packages/phishing-controller/src/PhishingDetector.test.ts b/packages/phishing-controller/src/PhishingDetector.test.ts index 2684c5d6f7..0423cc4a59 100644 --- a/packages/phishing-controller/src/PhishingDetector.test.ts +++ b/packages/phishing-controller/src/PhishingDetector.test.ts @@ -75,7 +75,7 @@ describe('PhishingDetector', () => { ); }); - [ + it.each([ undefined, null, true, @@ -88,46 +88,116 @@ describe('PhishingDetector', () => { return { name: 'test', version: 1 }; }, {}, - ].forEach((mockInvalidName) => { - it('throws an error when config name is invalid', async () => { - await expect( - withPhishingDetector( - [ - { - allowlist: [], - blocklist: ['blocked-by-first.com'], - fuzzylist: [], - // @ts-expect-error testing invalid input - name: mockInvalidName, - tolerance: 2, - version: 1, - }, - ], - async () => mockInvalidName, - ), - ).rejects.toThrow("Invalid config parameter: 'name'"); - }); + ])('logs an error when config name is %p', async (mockInvalidName) => { + // Mock console.error to track error logs without cluttering test output + const consoleErrorMock = jest.spyOn(console, 'error'); + + let detector; + + await withPhishingDetector( + [ + { + allowlist: [], + blocklist: ['blocked-by-first.com'], + fuzzylist: [], + // @ts-expect-error Testing invalid input + name: mockInvalidName, + tolerance: 2, + version: 1, + }, + ], + async ({ detector: d }) => { + detector = d; + }, + ); + + // Ensure the detector is still defined + expect(detector).toBeDefined(); + + // Check that console.error was called (you can further specify the error if needed) + expect(console.error).toHaveBeenCalled(); + + // Restore the original console.error implementation + consoleErrorMock.mockRestore(); }); - it('throws an error when tolerance is provided without fuzzylist', async () => { - await expect( - withPhishingDetector( - [ - // @ts-expect-error testing invalid input - { - allowlist: [], - blocklist: [], - name: 'first', - tolerance: 2, - version: 1, - }, - ], - async () => null, - ), - ).rejects.toThrow('Fuzzylist tolerance provided without fuzzylist'); + it('drops the invalid config and retains the valid config', async () => { + const consoleErrorMock = jest.spyOn(console, 'error'); + + let detector: PhishingDetector | undefined; + + await withPhishingDetector( + [ + // Invalid config + { + allowlist: [], + blocklist: ['blocked-by-first.com'], + fuzzylist: [], + name: undefined, + tolerance: 2, + version: 1, + }, + // Valid config + { + allowlist: [], + blocklist: ['blocked-by-second.com'], + fuzzylist: [], + name: 'MetaMask', + tolerance: 2, + version: 1, + }, + ], + async ({ detector: d }) => { + detector = d; + }, + ); + + expect(detector).toBeDefined(); + + const result = detector?.check('https://blocked-by-second.com'); + + expect(result).toBeDefined(); + expect(result?.type).toBe('blocklist'); + + const resultInvalid = detector?.check('https://blocked-by-first.com'); + + expect(resultInvalid).toBeDefined(); + expect(resultInvalid?.type).toBe('all'); + + expect(console.error).toHaveBeenCalled(); + + consoleErrorMock.mockRestore(); }); - [ + it('logs an error when tolerance is provided without fuzzylist', async () => { + const consoleErrorMock = jest.spyOn(console, 'error'); + + let detector; + + await withPhishingDetector( + [ + // @ts-expect-error testing invalid input + { + allowlist: [], + blocklist: [], + name: 'first', + tolerance: 2, + version: 1, + }, + ], + async ({ detector: d }) => { + detector = d; + }, + ); + + expect(detector).toBeDefined(); + + expect(console.error).toHaveBeenCalledWith(expect.any(Error)); + + consoleErrorMock.mockRestore(); + }); + + it.each([ undefined, null, true, @@ -137,26 +207,41 @@ describe('PhishingDetector', () => { return { name: 'test', version: 1 }; }, {}, - ].forEach((mockInvalidVersion) => { - it('throws an error when config version is invalid', async () => { - await expect( - withPhishingDetector( - [ - { - allowlist: [], - blocklist: ['blocked-by-first.com'], - fuzzylist: [], - name: 'first', - tolerance: 2, - // @ts-expect-error testing invalid input - version: mockInvalidVersion, - }, - ], - async () => null, - ), - ).rejects.toThrow("Invalid config parameter: 'version'"); - }); - }); + ])( + 'logs an error when config version is %p', + async (mockInvalidVersion) => { + // Mock console.error to track error logs without cluttering test output + const consoleErrorMock = jest.spyOn(console, 'error'); + + let detector; + + await withPhishingDetector( + [ + { + allowlist: [], + blocklist: ['blocked-by-first.com'], + fuzzylist: [], + name: 'first', + tolerance: 2, + // @ts-expect-error Testing invalid input + version: mockInvalidVersion, + }, + ], + async ({ detector: d }) => { + detector = d; + }, + ); + + // Ensure the detector is still defined + expect(detector).toBeDefined(); + + // Check that console.error was called with an error + expect(console.error).toHaveBeenCalledWith(expect.any(Error)); + + // Restore the original console.error implementation + consoleErrorMock.mockRestore(); + }, + ); }); describe('with legacy config', () => { diff --git a/packages/phishing-controller/src/utils.test.ts b/packages/phishing-controller/src/utils.test.ts index ac8f65d818..299db2f745 100644 --- a/packages/phishing-controller/src/utils.test.ts +++ b/packages/phishing-controller/src/utils.test.ts @@ -8,6 +8,7 @@ import { getHostnameFromUrl, matchPartsAgainstList, processConfigs, + // processConfigs, processDomainList, roundToNearestMinute, sha256Hash, @@ -287,30 +288,151 @@ describe('domainToParts', () => { }); describe('processConfigs', () => { - it('correctly converts a list of configs to a list of processed configs', () => { + let consoleErrorMock: jest.SpyInstance; + + beforeEach(() => { + consoleErrorMock = jest.spyOn(console, 'error'); + }); + + afterEach(() => { + consoleErrorMock.mockRestore(); + }); + + it('correctly processes a list of valid configs', () => { const configs = [ { allowlist: ['example.com'], blocklist: ['sub.example.com'], fuzzylist: ['fuzzy.example.com'], tolerance: 2, + version: 1, + name: 'MetaMask', }, ]; const result = processConfigs(configs); - expect(result).toStrictEqual([ + expect(result).toHaveLength(1); + expect(result[0].name).toBe('MetaMask'); + + expect(console.error).not.toHaveBeenCalled(); + }); + + it('filters out invalid configs and logs errors', () => { + const configs = [ { - allowlist: [['com', 'example']], - blocklist: [['com', 'example', 'sub']], - fuzzylist: [['com', 'example', 'fuzzy']], + allowlist: ['example.com'], + blocklist: ['sub.example.com'], + fuzzylist: [], tolerance: 2, + version: 1, + name: 'MetaMask', }, - ]); + { + allowlist: [], + version: 1, + name: undefined, + }, + ]; + + const result = processConfigs(configs); + + expect(result).toHaveLength(1); + expect(result[0].name).toBe('MetaMask'); + + expect(console.error).toHaveBeenCalledTimes(1); + }); + + it('returns an empty array when called with no arguments', () => { + const result = processConfigs(); + expect(result).toStrictEqual([]); + }); + + it('filters out invalid configs and logs errors with multiple configs', () => { + const configs = [ + { + allowlist: ['example.com'], + blocklist: ['sub.example.com'], + fuzzylist: [], + tolerance: 2, + version: 1, + name: 'MetaMask', + }, + { + allowlist: [], + version: 1, + name: undefined, + }, + { + allowlist: ['example.com'], + blocklist: ['sub.example.com'], + fuzzylist: [], + tolerance: 2, + version: 1, + name: 'name', + }, + { + allowlist: [], + version: 1, + name: '', + }, + ]; + + const result = processConfigs(configs); + + expect(result).toHaveLength(2); + expect(result[0].name).toBe('MetaMask'); + expect(result[1].name).toBe('name'); + + expect(console.error).toHaveBeenCalledTimes(2); }); - it('can be called with no arguments', () => { - expect(processConfigs()).toStrictEqual([]); + it('returns an empty array when all configs are invalid', () => { + const configs = [ + { + allowlist: [], + version: 1, + name: undefined, + }, + { + blocklist: [], + fuzzylist: [], + tolerance: 2, + version: null, + name: '', + }, + ]; + + // @ts-expect-error testing invalid input + const result = processConfigs(configs); + + expect(result).toStrictEqual([]); + + expect(console.error).toHaveBeenCalledTimes(2); + }); + + it('logs errors for invalid tolerance or version types', () => { + const configs = [ + { + allowlist: ['example.com'], + blocklist: ['sub.example.com'], + tolerance: 'invalid', + version: 1, + }, + { + allowlist: ['example.com'], + blocklist: ['sub.example.com'], + tolerance: 2, + version: {}, + }, + ]; + + // @ts-expect-error testing invalid input + const result = processConfigs(configs); + + expect(result).toStrictEqual([]); + + expect(console.error).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/phishing-controller/src/utils.ts b/packages/phishing-controller/src/utils.ts index bffd8f60bd..157505f76c 100644 --- a/packages/phishing-controller/src/utils.ts +++ b/packages/phishing-controller/src/utils.ts @@ -200,16 +200,28 @@ export const getDefaultPhishingDetectorConfig = ({ }); /** - * Processes the configurations for the phishing detector. + * Processes the configurations for the phishing detector, filtering out any invalid configs. * - * @param configs - the configurations to process. - * @returns the processed configurations. + * @param configs - The configurations to process. + * @returns An array of processed and valid configurations. */ -export const processConfigs = (configs: PhishingDetectorList[] = []) => { - return configs.map((config: PhishingDetectorList) => { - validateConfig(config); - return { ...config, ...getDefaultPhishingDetectorConfig(config) }; - }); +export const processConfigs = ( + configs: PhishingDetectorList[] = [], +): PhishingDetectorConfiguration[] => { + return configs + .filter((config) => { + try { + validateConfig(config); + return true; + } catch (error) { + console.error(error); + return false; + } + }) + .map((config) => ({ + ...config, + ...getDefaultPhishingDetectorConfig(config), + })); }; /**