feat: attribute dapp-scanning URL requests

packages/assets-controllers/CHANGELOG.md

@@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Bump `@ethersproject/address` from `^5.7.0` to `^5.8.0` ([#10478](https://github.com/MetaMask/core/pull/10478))
 - Bump `@tanstack/query-core` from `^5.89.0` to `^5.103.2` ([#10511](https://github.com/MetaMask/core/pull/10511))
 
+- `NftController` now attributes its `PhishingController:bulkScanUrls` calls to the `nft-detection` request source, so NFT metadata URL scans are distinguishable from other callers in phishing-detection service metrics ([#10357](https://github.com/MetaMask/core/pull/10357))
+
 ## [112.0.4]
 
 ### Changed

packages/assets-controllers/src/NftController.test.ts

@@ -33,7 +33,10 @@ import type {
   NetworkClientId,
 } from '@metamask/network-controller';
 import type { BulkPhishingDetectionScanResponse } from '@metamask/phishing-controller';
-import { RecommendedAction } from '@metamask/phishing-controller';
+import {
+  RecommendedAction,
+  RequestSourceFlow,
+} from '@metamask/phishing-controller';
 import { getDefaultPreferencesState } from '@metamask/preferences-controller';
 import type { PreferencesState } from '@metamask/preferences-controller';
 import type { Hex } from '@metamask/utils';
@@ -5287,6 +5290,29 @@ describe('NftController', () => {
       expect(safeNft?.externalLink).toBe('http://legitimate-domain.com');
     });
 
+    it('should attribute URL scans to the NFT detection flow', async () => {
+      const mockBulkScanUrls = jest.fn().mockResolvedValue({ results: {} });
+
+      const { nftController } = setupController({
+        bulkScanUrlsMock: mockBulkScanUrls,
+      });
+
+      await nftController.addNft('0xsafe', '1', 'mainnet', {
+        nftMetadata: {
+          name: 'Safe NFT',
+          description: 'NFT with safe links',
+          image: 'http://safe-site.com/image.png',
+          standard: ERC721,
+        },
+        userAddress: OWNER_ADDRESS,
+      });
+
+      expect(mockBulkScanUrls).toHaveBeenCalledWith(
+        expect.any(Array),
+        RequestSourceFlow.NftDetection,
+      );
+    });
+
     it('should handle errors during phishing detection when adding NFTs', async () => {
       const mockBulkScanUrls = jest
         .fn()
@@ -5472,9 +5498,10 @@ describe('NftController', () => {
       });
 
       // Verify only HTTP(S) URLs were sent for scanning
-      expect(mockBulkScanUrls).toHaveBeenCalledWith([
-        'https://secure-site.com',
-      ]);
+      expect(mockBulkScanUrls).toHaveBeenCalledWith(
+        ['https://secure-site.com'],
+        RequestSourceFlow.NftDetection,
+      );
 
       const storedNft =
         nftController.state.allNfts[OWNER_ADDRESS][ChainId.mainnet][0];
@@ -5605,9 +5632,10 @@ describe('NftController', () => {
       });
 
       // Should not throw error
-      expect(mockBulkScanUrls).toHaveBeenCalledWith([
-        'http://image.com/image.png',
-      ]);
+      expect(mockBulkScanUrls).toHaveBeenCalledWith(
+        ['http://image.com/image.png'],
+        RequestSourceFlow.NftDetection,
+      );
     });
   });
 

packages/assets-controllers/src/NftController.ts

@@ -35,7 +35,10 @@ import type {
   NetworkControllerGetNetworkClientByIdAction,
 } from '@metamask/network-controller';
 import type { PhishingControllerBulkScanUrlsAction } from '@metamask/phishing-controller';
-import { RecommendedAction } from '@metamask/phishing-controller';
+import {
+  RecommendedAction,
+  RequestSourceFlow,
+} from '@metamask/phishing-controller';
 import type { PreferencesControllerStateChangeEvent } from '@metamask/preferences-controller';
 import { rpcErrors } from '@metamask/rpc-errors';
 import type { Hex } from '@metamask/utils';
@@ -2299,6 +2302,7 @@ export class NftController extends BaseController<
           const bulkScanResponse = await this.messenger.call(
             'PhishingController:bulkScanUrls',
             batch,
+            RequestSourceFlow.NftDetection,
           );
 
           // Collect blocked URLs from this batch

packages/phishing-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add optional request-source attribution parameters to `PhishingController.scanUrl` and `bulkScanUrls`, emitting an `x-request-source` header. ([#10357](https://github.com/MetaMask/core/pull/10357))
+
 ### Changed
 
 - Bump `@types/punycode` from `^2.1.0` to `^2.1.4` ([#10441](https://github.com/MetaMask/core/pull/10441))

packages/phishing-controller/src/PhishingController-method-action-types.ts

@@ -75,6 +75,9 @@ export type PhishingControllerBypassAction = {
  * Only supports web URLs (`http:` / `https:`).
  *
  * @param url - The URL to scan.
+ * @param flow - Product flow used to attribute a network scan in the
+ * `x-request-source` header. If omitted, the header uses an unknown flow.
+ * Cached results do not make a network scan or emit a header.
  * @returns The phishing detection scan result.
  */
 export type PhishingControllerScanUrlAction = {
@@ -87,6 +90,9 @@ export type PhishingControllerScanUrlAction = {
  * It also only supports web URLs.
  *
  * @param urls - The URLs to scan.
+ * @param flow - Product flow used to attribute a network scan in the
+ * `x-request-source` header. If omitted, the header uses an unknown flow.
+ * Cached results do not make a network scan or emit a header.
  * @returns A mapping of URLs to their phishing detection scan results and errors.
  */
 export type PhishingControllerBulkScanUrlsAction = {

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

@@ -34,6 +34,7 @@ import type {
   BulkPhishingDetectionScanResponse,
   PhishingControllerMessenger,
 } from './PhishingController.js';
+import { RequestSourceFlow, RequestSourcePlatform } from './request-source.js';
 import {
   createMockStateChangePayload,
   createMockTransaction,
@@ -3057,6 +3058,108 @@ describe('PhishingController', () => {
     });
   });
 
+  describe('request source attribution', () => {
+    // NOTE: nock does not model CORS preflight, so nothing here can catch a
+    // server-side `Access-Control-Allow-Headers` allowlist that omits
+    // `x-request-source`. That is verified against the service, not in tests.
+    const scanResponse: PhishingDetectionScanResult = {
+      hostname: 'example.com',
+      recommendedAction: RecommendedAction.None,
+    };
+
+    it('reports the composed source on a single URL scan', async () => {
+      const { rootMessenger } = getPhishingController({
+        platform: RequestSourcePlatform.Mobile,
+      });
+      const scope = nock(PHISHING_DETECTION_BASE_URL)
+        .matchHeader('x-request-source', 'mobile-browser')
+        .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`)
+        .query({ url: 'example.com' })
+        .reply(200, scanResponse);
+
+      await rootMessenger.call(
+        'PhishingController:scanUrl',
+        'https://example.com',
+        RequestSourceFlow.Browser,
+      );
+
+      expect(scope.isDone()).toBe(true);
+    });
+
+    it('reports the composed source on a bulk URL scan', async () => {
+      const { rootMessenger } = getPhishingController({
+        platform: RequestSourcePlatform.Extension,
+      });
+      const scope = nock(PHISHING_DETECTION_BASE_URL)
+        .matchHeader('x-request-source', 'extension-nft-detection')
+        .post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`)
+        .reply(200, { results: {}, errors: {} });
+
+      await rootMessenger.call(
+        'PhishingController:bulkScanUrls',
+        ['https://example.com'],
+        RequestSourceFlow.NftDetection,
+      );
+
+      expect(scope.isDone()).toBe(true);
+    });
+
+    it('reports the platform sentinel when a caller omits the flow', async () => {
+      const { rootMessenger } = getPhishingController({
+        platform: RequestSourcePlatform.Extension,
+      });
+      const scope = nock(PHISHING_DETECTION_BASE_URL)
+        .matchHeader('x-request-source', 'extension-unknown')
+        .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`)
+        .query({ url: 'example.com' })
+        .reply(200, scanResponse);
+
+      await rootMessenger.call(
+        'PhishingController:scanUrl',
+        'https://example.com',
+      );
+
+      expect(scope.isDone()).toBe(true);
+    });
+
+    it('reports the bare sentinel when no platform is configured', async () => {
+      const { rootMessenger } = getPhishingController();
+      const scope = nock(PHISHING_DETECTION_BASE_URL)
+        .matchHeader('x-request-source', 'unknown')
+        .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`)
+        .query({ url: 'example.com' })
+        .reply(200, scanResponse);
+
+      await rootMessenger.call(
+        'PhishingController:scanUrl',
+        'https://example.com',
+        RequestSourceFlow.Browser,
+      );
+
+      expect(scope.isDone()).toBe(true);
+    });
+
+    it('degrades an unrecognised flow to the platform sentinel', async () => {
+      // Guards the plain-JavaScript call sites, which get no type checking.
+      const { rootMessenger } = getPhishingController({
+        platform: RequestSourcePlatform.Mobile,
+      });
+      const scope = nock(PHISHING_DETECTION_BASE_URL)
+        .matchHeader('x-request-source', 'mobile-unknown')
+        .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`)
+        .query({ url: 'example.com' })
+        .reply(200, scanResponse);
+
+      await rootMessenger.call(
+        'PhishingController:scanUrl',
+        'https://example.com',
+        'browsr' as RequestSourceFlow,
+      );
+
+      expect(scope.isDone()).toBe(true);
+    });
+  });
+
   describe('bulkScanUrls', () => {
     let rootMessenger: RootMessenger;
 

packages/phishing-controller/src/PhishingController.ts

@@ -43,6 +43,11 @@ import type {
   PhishingControllerTestOriginAction,
 } from './PhishingController-method-action-types.js';
 import { PhishingDetector } from './PhishingDetector.js';
+import { REQUEST_SOURCE_HEADER, buildRequestSource } from './request-source.js';
+import type {
+  RequestSourceFlow,
+  RequestSourcePlatform,
+} from './request-source.js';
 import {
   PhishingDetectorResultType,
   RecommendedAction,
@@ -388,6 +393,7 @@ export type PhishingControllerState = {
  * 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.
+ * platform - Client used to attribute URL scans in the `x-request-source` header.
  */
 export type PhishingControllerOptions = {
   stalelistRefreshInterval?: number;
@@ -401,6 +407,7 @@ export type PhishingControllerOptions = {
   addressScanCacheMaxSize?: number;
   messenger: PhishingControllerMessenger;
   state?: Partial<PhishingControllerState>;
+  platform?: RequestSourcePlatform;
 };
 
 const MESSENGER_EXPOSED_METHODS = [
@@ -509,6 +516,8 @@ export class PhishingController extends BaseController<
 
   readonly #addressBookRecipients: Set<string>;
 
+  readonly #platform?: RequestSourcePlatform;
+
   #inProgressHotlistUpdate?: Promise<void>;
 
   #inProgressStalelistUpdate?: Promise<void>;
@@ -539,6 +548,8 @@ export class PhishingController extends BaseController<
    * @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.
+   * @param config.platform - Client used to attribute URL scans in the
+   * `x-request-source` header. When omitted, URL scans use the `unknown` value.
    */
   constructor({
     stalelistRefreshInterval = STALELIST_REFRESH_INTERVAL,
@@ -552,6 +563,7 @@ export class PhishingController extends BaseController<
     addressScanCacheMaxSize = DEFAULT_ADDRESS_SCAN_CACHE_MAX_SIZE,
     messenger,
     state = {},
+    platform,
   }: PhishingControllerOptions) {
     super({
       name: controllerName,
@@ -563,6 +575,7 @@ export class PhishingController extends BaseController<
       },
     });
 
+    this.#platform = platform;
     this.#stalelistRefreshInterval = stalelistRefreshInterval;
     this.#hotlistRefreshInterval = hotlistRefreshInterval;
     this.#c2DomainBlocklistRefreshInterval = c2DomainBlocklistRefreshInterval;
@@ -1208,9 +1221,15 @@ export class PhishingController extends BaseController<
    * Only supports web URLs (`http:` / `https:`).
    *
    * @param url - The URL to scan.
+   * @param flow - Product flow used to attribute a network scan in the
+   * `x-request-source` header. If omitted, the header uses an unknown flow.
+   * Cached results do not make a network scan or emit a header.
    * @returns The phishing detection scan result.
    */
-  async scanUrl(url: string): Promise<PhishingDetectionScanResult> {
+  async scanUrl(
+    url: string,
+    flow?: RequestSourceFlow,
+  ): Promise<PhishingDetectionScanResult> {
     const [scanUrlParam, scanParamOk] = getPhishingDetectionScanUrlParam(url);
     if (!scanParamOk) {
       return {
@@ -1235,6 +1254,7 @@ export class PhishingController extends BaseController<
             method: 'GET',
             headers: {
               Accept: 'application/json',
+              [REQUEST_SOURCE_HEADER]: buildRequestSource(this.#platform, flow),
             },
           },
         );
@@ -1281,10 +1301,14 @@ export class PhishingController extends BaseController<
    * It also only supports web URLs.
    *
    * @param urls - The URLs to scan.
+   * @param flow - Product flow used to attribute a network scan in the
+   * `x-request-source` header. If omitted, the header uses an unknown flow.
+   * Cached results do not make a network scan or emit a header.
    * @returns A mapping of URLs to their phishing detection scan results and errors.
    */
   async bulkScanUrls(
     urls: string[],
+    flow?: RequestSourceFlow,
   ): Promise<BulkPhishingDetectionScanResponse> {
     if (!urls || urls.length === 0) {
       return {
@@ -1353,7 +1377,7 @@ export class PhishingController extends BaseController<
 
       // Process each batch in parallel
       const batchResults = await Promise.all(
-        batches.map((batchUrls) => this.#processBatch(batchUrls)),
+        batches.map((batchUrls) => this.#processBatch(batchUrls, flow)),
       );
 
       // Merge results and errors from all batches
@@ -1692,10 +1716,13 @@ export class PhishingController extends BaseController<
    * Process a batch of URLs (up to 50) for phishing detection.
    *
    * @param urls - A batch of URLs to scan.
+   * @param flow - The flow that caused this scan, reported via the
+   * `x-request-source` header.
    * @returns The scan results and errors for this batch.
    */
   readonly #processBatch = async (
     urls: string[],
+    flow?: RequestSourceFlow,
   ): Promise<BulkPhishingDetectionScanResponse> => {
     const apiResponse = await safelyExecuteWithTimeout(
       async () => {
@@ -1706,6 +1733,7 @@ export class PhishingController extends BaseController<
             headers: {
               Accept: 'application/json',
               'Content-Type': 'application/json',
+              [REQUEST_SOURCE_HEADER]: buildRequestSource(this.#platform, flow),
             },
             body: JSON.stringify({ urls }),
           },

packages/phishing-controller/src/index.ts

@@ -47,6 +47,14 @@ export type {
   ExtractedSignatureAddresses,
   ExtractSignatureAddressesOptions,
 } from './signature-address-extraction.js';
+export {
+  REQUEST_SOURCE_HEADER,
+  UNKNOWN_REQUEST_SOURCE,
+  RequestSourceFlow,
+  RequestSourcePlatform,
+  buildRequestSource,
+} from './request-source.js';
+export type { RequestSource } from './request-source.js';
 
 export type {
   PhishingControllerMaybeUpdateStateAction,