Skip to content

Commit 3cd468e

Browse files
AugmentedModeAugmentedMode
committed
fix: changelog
2 parents cbd9b76 + f92caba commit 3cd468e

File tree

3 files changed

+438
-1
lines changed

3 files changed

+438
-1
lines changed

‎packages/phishing-controller/CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
- Added methods to `PhishingController`: `setUrlScanCacheTTL`, `setUrlScanCacheMaxSize`, `clearUrlScanCache`
1515
- Added URL scan cache state to `PhishingControllerState`
1616
- Added configuration options: `urlScanCacheTTL` and `urlScanCacheMaxSize`
17+
- Add `bulkScanUrls` method to `PhishingController` for scanning multiple URLs for phishing in bulk ([#5682](https://github.com/MetaMask/core/pull/5682))
18+
- Add `BulkPhishingDetectionScanResponse` type for bulk URL scan results ([#5682](https://github.com/MetaMask/core/pull/5682))
19+
- Add `PHISHING_DETECTION_BULK_SCAN_ENDPOINT` constant ([#5682](https://github.com/MetaMask/core/pull/5682))
1720

1821
### Changed
1922

‎packages/phishing-controller/src/PhishingController.test.ts‎

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
C2_DOMAIN_BLOCKLIST_ENDPOINT,
1616
PHISHING_DETECTION_BASE_URL,
1717
PHISHING_DETECTION_SCAN_ENDPOINT,
18+
PHISHING_DETECTION_BULK_SCAN_ENDPOINT,
19+
type BulkPhishingDetectionScanResponse,
1820
} from './PhishingController';
1921
import { formatHostnameToUrl } from './tests/utils';
2022
import type { PhishingDetectionScanResult } from './types';
@@ -2588,6 +2590,287 @@ describe('PhishingController', () => {
25882590
expect(scope.isDone()).toBe(true);
25892591
});
25902592
});
2593+
2594+
describe('bulkScanUrls', () => {
2595+
let controller: PhishingController;
2596+
let clock: sinon.SinonFakeTimers;
2597+
const testUrls: string[] = [
2598+
'https://example1.com',
2599+
'https://example2.com',
2600+
'https://example3.com',
2601+
];
2602+
const mockResponse: BulkPhishingDetectionScanResponse = {
2603+
results: {
2604+
'https://example1.com': {
2605+
domainName: 'example1.com',
2606+
recommendedAction: RecommendedAction.None,
2607+
},
2608+
'https://example2.com': {
2609+
domainName: 'example2.com',
2610+
recommendedAction: RecommendedAction.Block,
2611+
},
2612+
'https://example3.com': {
2613+
domainName: 'example3.com',
2614+
recommendedAction: RecommendedAction.None,
2615+
},
2616+
},
2617+
errors: {},
2618+
};
2619+
2620+
beforeEach(() => {
2621+
controller = getPhishingController();
2622+
clock = sinon.useFakeTimers();
2623+
});
2624+
2625+
afterEach(() => {
2626+
clock.restore();
2627+
});
2628+
2629+
it('should return the scan results for multiple URLs', async () => {
2630+
const scope = nock(PHISHING_DETECTION_BASE_URL)
2631+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2632+
urls: testUrls,
2633+
})
2634+
.reply(200, mockResponse);
2635+
2636+
const response = await controller.bulkScanUrls(testUrls);
2637+
expect(response).toStrictEqual(mockResponse);
2638+
expect(scope.isDone()).toBe(true);
2639+
});
2640+
2641+
it('should handle empty URL arrays', async () => {
2642+
const response = await controller.bulkScanUrls([]);
2643+
expect(response).toStrictEqual({
2644+
results: {},
2645+
errors: {},
2646+
});
2647+
});
2648+
2649+
it('should enforce maximum URL limit', async () => {
2650+
const tooManyUrls = Array(251).fill('https://example.com');
2651+
const response = await controller.bulkScanUrls(tooManyUrls);
2652+
expect(response).toStrictEqual({
2653+
results: {},
2654+
errors: {
2655+
too_many_urls: ['Maximum of 250 URLs allowed per request'],
2656+
},
2657+
});
2658+
});
2659+
2660+
it('should validate URL length', async () => {
2661+
const longUrl = `https://example.com/${'a'.repeat(2048)}`;
2662+
const response = await controller.bulkScanUrls([longUrl]);
2663+
expect(response).toStrictEqual({
2664+
results: {},
2665+
errors: {
2666+
[longUrl]: ['URL length must not exceed 2048 characters'],
2667+
},
2668+
});
2669+
});
2670+
2671+
it.each([
2672+
[400, 'Bad Request'],
2673+
[401, 'Unauthorized'],
2674+
[403, 'Forbidden'],
2675+
[404, 'Not Found'],
2676+
[500, 'Internal Server Error'],
2677+
[502, 'Bad Gateway'],
2678+
[503, 'Service Unavailable'],
2679+
[504, 'Gateway Timeout'],
2680+
])(
2681+
'should return an error response on %i status code',
2682+
async (statusCode, statusText) => {
2683+
const scope = nock(PHISHING_DETECTION_BASE_URL)
2684+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2685+
urls: testUrls,
2686+
})
2687+
.reply(statusCode);
2688+
2689+
const response = await controller.bulkScanUrls(testUrls);
2690+
expect(response).toStrictEqual({
2691+
results: {},
2692+
errors: {
2693+
api_error: [`${statusCode} ${statusText}`],
2694+
},
2695+
});
2696+
expect(scope.isDone()).toBe(true);
2697+
},
2698+
);
2699+
2700+
it('should handle timeouts correctly', async () => {
2701+
const scope = nock(PHISHING_DETECTION_BASE_URL)
2702+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2703+
urls: testUrls,
2704+
})
2705+
.delayConnection(20000)
2706+
.reply(200, {});
2707+
2708+
const promise = controller.bulkScanUrls(testUrls);
2709+
clock.tick(15000);
2710+
const response = await promise;
2711+
expect(response).toStrictEqual({
2712+
results: {},
2713+
errors: {
2714+
network_error: ['timeout of 15000ms exceeded'],
2715+
},
2716+
});
2717+
expect(scope.isDone()).toBe(false);
2718+
});
2719+
2720+
it('should process URLs in batches when more than 50 URLs are provided', async () => {
2721+
const batchSize = 50;
2722+
const totalUrls = 120;
2723+
const manyUrls = Array(totalUrls)
2724+
.fill(0)
2725+
.map((_, i) => `https://example${i}.com`);
2726+
2727+
// Expected batches
2728+
const batch1 = manyUrls.slice(0, batchSize);
2729+
const batch2 = manyUrls.slice(batchSize, 2 * batchSize);
2730+
const batch3 = manyUrls.slice(2 * batchSize);
2731+
2732+
// Mock responses for each batch
2733+
const mockBatch1Response: BulkPhishingDetectionScanResponse = {
2734+
results: batch1.reduce<Record<string, PhishingDetectionScanResult>>(
2735+
(acc, url) => {
2736+
acc[url] = {
2737+
domainName: url.replace('https://', ''),
2738+
recommendedAction: RecommendedAction.None,
2739+
};
2740+
return acc;
2741+
},
2742+
{},
2743+
),
2744+
errors: {},
2745+
};
2746+
2747+
const mockBatch2Response: BulkPhishingDetectionScanResponse = {
2748+
results: batch2.reduce<Record<string, PhishingDetectionScanResult>>(
2749+
(acc, url) => {
2750+
acc[url] = {
2751+
domainName: url.replace('https://', ''),
2752+
recommendedAction: RecommendedAction.None,
2753+
};
2754+
return acc;
2755+
},
2756+
{},
2757+
),
2758+
errors: {},
2759+
};
2760+
2761+
const mockBatch3Response: BulkPhishingDetectionScanResponse = {
2762+
results: batch3.reduce<Record<string, PhishingDetectionScanResult>>(
2763+
(acc, url) => {
2764+
acc[url] = {
2765+
domainName: url.replace('https://', ''),
2766+
recommendedAction: RecommendedAction.None,
2767+
};
2768+
return acc;
2769+
},
2770+
{},
2771+
),
2772+
errors: {},
2773+
};
2774+
2775+
// Setup nock to handle all three batch requests
2776+
const scope1 = nock(PHISHING_DETECTION_BASE_URL)
2777+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2778+
urls: batch1,
2779+
})
2780+
.reply(200, mockBatch1Response);
2781+
2782+
const scope2 = nock(PHISHING_DETECTION_BASE_URL)
2783+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2784+
urls: batch2,
2785+
})
2786+
.reply(200, mockBatch2Response);
2787+
2788+
const scope3 = nock(PHISHING_DETECTION_BASE_URL)
2789+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2790+
urls: batch3,
2791+
})
2792+
.reply(200, mockBatch3Response);
2793+
2794+
const response = await controller.bulkScanUrls(manyUrls);
2795+
2796+
// Verify all scopes were called
2797+
expect(scope1.isDone()).toBe(true);
2798+
expect(scope2.isDone()).toBe(true);
2799+
expect(scope3.isDone()).toBe(true);
2800+
2801+
// Check all results were merged correctly
2802+
const combinedResults = {
2803+
...mockBatch1Response.results,
2804+
...mockBatch2Response.results,
2805+
...mockBatch3Response.results,
2806+
};
2807+
2808+
expect(Object.keys(response.results)).toHaveLength(totalUrls);
2809+
expect(response.results).toStrictEqual(combinedResults);
2810+
});
2811+
2812+
it('should handle mixed results with both successful scans and errors', async () => {
2813+
const mixedResponse: BulkPhishingDetectionScanResponse = {
2814+
results: {
2815+
'https://example1.com': {
2816+
domainName: 'example1.com',
2817+
recommendedAction: RecommendedAction.None,
2818+
},
2819+
},
2820+
errors: {
2821+
'https://example2.com': ['Failed to process URL'],
2822+
'https://example3.com': ['Domain not found'],
2823+
},
2824+
};
2825+
2826+
const scope = nock(PHISHING_DETECTION_BASE_URL)
2827+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2828+
urls: testUrls,
2829+
})
2830+
.reply(200, mixedResponse);
2831+
2832+
const response = await controller.bulkScanUrls(testUrls);
2833+
expect(response).toStrictEqual(mixedResponse);
2834+
expect(scope.isDone()).toBe(true);
2835+
});
2836+
2837+
it('should have error merging issues when multiple batches return errors with the same key', async () => {
2838+
// Create enough URLs to need two batches (over 50)
2839+
const batchSize = 50;
2840+
const totalUrls = 100;
2841+
const manyUrls = Array(totalUrls)
2842+
.fill(0)
2843+
.map((_, i) => `https://example${i}.com`);
2844+
2845+
// The URLs will be split into two batches
2846+
const batch1 = manyUrls.slice(0, batchSize);
2847+
const batch2 = manyUrls.slice(batchSize);
2848+
2849+
// Setup nock to handle both batch requests with different error responses
2850+
const scope1 = nock(PHISHING_DETECTION_BASE_URL)
2851+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2852+
urls: batch1,
2853+
})
2854+
.reply(404, { error: 'Not Found' });
2855+
2856+
const scope2 = nock(PHISHING_DETECTION_BASE_URL)
2857+
.post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`, {
2858+
urls: batch2,
2859+
})
2860+
.reply(500, { error: 'Internal Server Error' });
2861+
2862+
const response = await controller.bulkScanUrls(manyUrls);
2863+
2864+
expect(scope1.isDone()).toBe(true);
2865+
expect(scope2.isDone()).toBe(true);
2866+
2867+
// With the fixed implementation, we should now preserve all errors
2868+
expect(response.errors).toHaveProperty('api_error');
2869+
expect(response.errors.api_error).toHaveLength(2);
2870+
expect(response.errors.api_error).toContain('404 Not Found');
2871+
expect(response.errors.api_error).toContain('500 Internal Server Error');
2872+
});
2873+
});
25912874
});
25922875

25932876
describe('URL Scan Cache', () => {

0 commit comments

Comments
 (0)