From 73e3f8311e837f3715599c9ac5b4eaa3867e61d0 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 16 Sep 2026 13:33:08 -0500 Subject: [PATCH 1/6] feat: emit x-request-source header on dapp-scanning URL scans Add request-source attribution to the two dapp-scanning URL scan endpoints so phishing-detection service metrics can attribute scan volume to the client flow that caused it. Co-Authored-By: Claude Opus 5 (1M context) --- packages/assets-controllers/CHANGELOG.md | 2 + .../src/NftController.test.ts | 42 +++++- .../assets-controllers/src/NftController.ts | 6 +- packages/phishing-controller/CHANGELOG.md | 10 ++ .../PhishingController-method-action-types.ts | 6 + .../src/PhishingController.test.ts | 103 +++++++++++++++ .../src/PhishingController.ts | 33 ++++- packages/phishing-controller/src/index.ts | 8 ++ .../src/request-source.test.ts | 115 +++++++++++++++++ .../phishing-controller/src/request-source.ts | 121 ++++++++++++++++++ 10 files changed, 436 insertions(+), 10 deletions(-) create mode 100644 packages/phishing-controller/src/request-source.test.ts create mode 100644 packages/phishing-controller/src/request-source.ts diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 66984a2d82e..686604a1fc7 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/transaction-controller` from `^72.0.0` to `^72.0.1` ([#10462](https://github.com/MetaMask/core/pull/10462)) - Bump `@ethersproject/bignumber` from `^5.7.0` to `^5.8.0` ([#10479](https://github.com/MetaMask/core/pull/10479)) +- `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 diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 9ebbc5e6404..4481d0e2603 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/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, + ); }); }); diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index e2797b83e9f..4614f3bd3c9 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/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 diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index 63c14148c85..7aa45a1f456 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add request-source attribution to dapp-scanning URL scans, emitted as an `x-request-source` header ([#10260](https://github.com/MetaMask/core/pull/10260)) + - Add an optional `platform` constructor option and an optional `flow` parameter to `scanUrl` and `bulkScanUrls`. The header value is composed as `-`. + - Export the `RequestSourcePlatform` and `RequestSourceFlow` enums, the `RequestSource` type, the `REQUEST_SOURCE_HEADER` and `UNKNOWN_REQUEST_SOURCE` constants, and the `buildRequestSource` helper. + - `RequestSourcePlatform` is `Extension` or `Mobile`. `RequestSourceFlow` is one of `dapp-connection`, `browser`, `rpc-trust-signals`, `confirmations`, `reveal-srp`, or `nft-detection`. + - Both values are validated at runtime, since some consumers call these methods from untyped JavaScript. An unrecognised or absent value degrades to a sentinel (`-unknown`, or `unknown` when no platform is configured) and never throws, so attribution cannot fail a scan. + - The header is untrusted observability metadata and must not be used for authentication, authorization, or rate-limit decisions. + - Only the dapp-scanning endpoints (`v2/scan`, `bulk-scan`) are covered. The Security Alerts endpoints used by `scanAddress`, `bulkScanTokens`, and `getApprovals` are unchanged. + ### Changed - Bump `@types/punycode` from `^2.1.0` to `^2.1.4` ([#10441](https://github.com/MetaMask/core/pull/10441)) diff --git a/packages/phishing-controller/src/PhishingController-method-action-types.ts b/packages/phishing-controller/src/PhishingController-method-action-types.ts index cfd36b561ed..1b83d315b8b 100644 --- a/packages/phishing-controller/src/PhishingController-method-action-types.ts +++ b/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 - The flow that caused this scan, reported via the + * `x-request-source` header. Callers should always supply this; omitting it + * reports the scan as unattributed. * @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 - The flow that caused this scan, reported via the + * `x-request-source` header. Callers should always supply this; omitting it + * reports the scan as unattributed. * @returns A mapping of URLs to their phishing detection scan results and errors. */ export type PhishingControllerBulkScanUrlsAction = { diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index cbbc6d343b0..f13409f2895 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/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; diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index 62f9e132227..42a54eb8ae1 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/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 emitting URL scans, reported via the `x-request-source` header. */ export type PhishingControllerOptions = { stalelistRefreshInterval?: number; @@ -401,6 +407,7 @@ export type PhishingControllerOptions = { addressScanCacheMaxSize?: number; messenger: PhishingControllerMessenger; state?: Partial; + platform?: RequestSourcePlatform; }; const MESSENGER_EXPOSED_METHODS = [ @@ -509,6 +516,8 @@ export class PhishingController extends BaseController< readonly #addressBookRecipients: Set; + readonly #platform?: RequestSourcePlatform; + #inProgressHotlistUpdate?: Promise; #inProgressStalelistUpdate?: Promise; @@ -539,6 +548,9 @@ 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 emitting URL scans, reported via the + * `x-request-source` header. When omitted, URL scans report an unattributed + * sentinel instead. */ constructor({ stalelistRefreshInterval = STALELIST_REFRESH_INTERVAL, @@ -552,6 +564,7 @@ export class PhishingController extends BaseController< addressScanCacheMaxSize = DEFAULT_ADDRESS_SCAN_CACHE_MAX_SIZE, messenger, state = {}, + platform, }: PhishingControllerOptions) { super({ name: controllerName, @@ -563,6 +576,7 @@ export class PhishingController extends BaseController< }, }); + this.#platform = platform; this.#stalelistRefreshInterval = stalelistRefreshInterval; this.#hotlistRefreshInterval = hotlistRefreshInterval; this.#c2DomainBlocklistRefreshInterval = c2DomainBlocklistRefreshInterval; @@ -1208,9 +1222,15 @@ export class PhishingController extends BaseController< * Only supports web URLs (`http:` / `https:`). * * @param url - The URL to scan. + * @param flow - The flow that caused this scan, reported via the + * `x-request-source` header. Callers should always supply this; omitting it + * reports the scan as unattributed. * @returns The phishing detection scan result. */ - async scanUrl(url: string): Promise { + async scanUrl( + url: string, + flow?: RequestSourceFlow, + ): Promise { const [scanUrlParam, scanParamOk] = getPhishingDetectionScanUrlParam(url); if (!scanParamOk) { return { @@ -1235,6 +1255,7 @@ export class PhishingController extends BaseController< method: 'GET', headers: { Accept: 'application/json', + [REQUEST_SOURCE_HEADER]: buildRequestSource(this.#platform, flow), }, }, ); @@ -1281,10 +1302,14 @@ export class PhishingController extends BaseController< * It also only supports web URLs. * * @param urls - The URLs to scan. + * @param flow - The flow that caused this scan, reported via the + * `x-request-source` header. Callers should always supply this; omitting it + * reports the scan as unattributed. * @returns A mapping of URLs to their phishing detection scan results and errors. */ async bulkScanUrls( urls: string[], + flow?: RequestSourceFlow, ): Promise { if (!urls || urls.length === 0) { return { @@ -1353,7 +1378,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 +1717,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 => { const apiResponse = await safelyExecuteWithTimeout( async () => { @@ -1706,6 +1734,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 }), }, diff --git a/packages/phishing-controller/src/index.ts b/packages/phishing-controller/src/index.ts index 5a656acf38c..d019a08a6bb 100644 --- a/packages/phishing-controller/src/index.ts +++ b/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, diff --git a/packages/phishing-controller/src/request-source.test.ts b/packages/phishing-controller/src/request-source.test.ts new file mode 100644 index 00000000000..2377cb2de69 --- /dev/null +++ b/packages/phishing-controller/src/request-source.test.ts @@ -0,0 +1,115 @@ +import { + REQUEST_SOURCE_HEADER, + RequestSourceFlow, + RequestSourcePlatform, + UNKNOWN_REQUEST_SOURCE, + buildRequestSource, +} from './request-source.js'; + +describe('REQUEST_SOURCE_HEADER', () => { + it('is the header name agreed with the phishing detection service', () => { + expect(REQUEST_SOURCE_HEADER).toBe('x-request-source'); + }); +}); + +describe('buildRequestSource', () => { + const validCombinations: [ + RequestSourcePlatform, + RequestSourceFlow, + string, + ][] = [ + [ + RequestSourcePlatform.Extension, + RequestSourceFlow.DappConnection, + 'Extension-dapp-connection', + ], + [ + RequestSourcePlatform.Extension, + RequestSourceFlow.RpcTrustSignals, + 'Extension-rpc-trust-signals', + ], + [ + RequestSourcePlatform.Extension, + RequestSourceFlow.Confirmations, + 'Extension-confirmations', + ], + [ + RequestSourcePlatform.Extension, + RequestSourceFlow.RevealSrp, + 'Extension-reveal-srp', + ], + [ + RequestSourcePlatform.Extension, + RequestSourceFlow.NftDetection, + 'Extension-nft-detection', + ], + [ + RequestSourcePlatform.Mobile, + RequestSourceFlow.DappConnection, + 'Mobile-dapp-connection', + ], + [RequestSourcePlatform.Mobile, RequestSourceFlow.Browser, 'Mobile-browser'], + [ + RequestSourcePlatform.Mobile, + RequestSourceFlow.RpcTrustSignals, + 'Mobile-rpc-trust-signals', + ], + [ + RequestSourcePlatform.Mobile, + RequestSourceFlow.NftDetection, + 'Mobile-nft-detection', + ], + ]; + + it.each(validCombinations)( + 'composes %s + %s into %s', + (platform, flow, expected) => { + expect(buildRequestSource(platform, flow)).toBe(expected); + }, + ); + + it('falls back to the platform sentinel when no flow is given', () => { + expect(buildRequestSource(RequestSourcePlatform.Extension)).toBe( + 'Extension-unknown', + ); + expect(buildRequestSource(RequestSourcePlatform.Mobile)).toBe( + 'Mobile-unknown', + ); + }); + + it('falls back to the platform sentinel when the flow is not recognised', () => { + // Guards against a typo from an untyped (JavaScript) call site, which would + // otherwise be reported as a real attribution value. + expect( + buildRequestSource( + RequestSourcePlatform.Extension, + 'dapp-conection' as RequestSourceFlow, + ), + ).toBe('Extension-unknown'); + }); + + it('returns the bare sentinel when no platform is configured', () => { + expect(buildRequestSource()).toBe(UNKNOWN_REQUEST_SOURCE); + expect( + buildRequestSource(undefined, RequestSourceFlow.DappConnection), + ).toBe(UNKNOWN_REQUEST_SOURCE); + }); + + it('returns the bare sentinel when the platform is not recognised', () => { + expect( + buildRequestSource( + 'Desktop' as RequestSourcePlatform, + RequestSourceFlow.DappConnection, + ), + ).toBe(UNKNOWN_REQUEST_SOURCE); + }); + + it('never throws, so attribution cannot fail a scan', () => { + expect(() => + buildRequestSource( + null as unknown as RequestSourcePlatform, + null as unknown as RequestSourceFlow, + ), + ).not.toThrow(); + }); +}); diff --git a/packages/phishing-controller/src/request-source.ts b/packages/phishing-controller/src/request-source.ts new file mode 100644 index 00000000000..7cc3349f2ab --- /dev/null +++ b/packages/phishing-controller/src/request-source.ts @@ -0,0 +1,121 @@ +/** + * Request header carrying the first-party flow that caused a scan. + * + * This is untrusted observability metadata. It must never be used for + * authentication, authorization, or to bypass rate limits. + */ +export const REQUEST_SOURCE_HEADER = 'x-request-source'; + +/** + * Emitted when the platform is not configured, so unattributed traffic is + * measurable rather than indistinguishable from a client that predates the + * header. + */ +export const UNKNOWN_REQUEST_SOURCE = 'unknown'; + +/** + * The client emitting the scan. Supplied once, when the controller is + * constructed, since a single instance only ever runs on one platform. + */ +export enum RequestSourcePlatform { + Extension = 'Extension', + Mobile = 'Mobile', +} + +/** + * The flow that caused a scan. Supplied per call, since one controller serves + * many flows. + * + * Values are bounded because the phishing detection service records them as a + * Prometheus label and normalizes anything it does not recognise to + * {@link UNKNOWN_REQUEST_SOURCE}. + */ +export enum RequestSourceFlow { + /** + * A connect prompt was shown, or an advanced permission was granted. + */ + DappConnection = 'dapp-connection', + /** + * Main-frame navigation in the mobile in-app browser. + */ + Browser = 'browser', + /** + * Origin scan triggered by dapp RPC traffic rather than by a user action. + * High request count, low distinct-URL count, so the cache absorbs most of + * it. + * + * The trigger differs by client: Mobile scans on every EIP-1193 request + * carrying an origin, whereas Extension scans only when a connected origin + * reads its own connection state (`eth_accounts`, or `wallet_getSession` on + * the Multichain transport). Mobile volume is therefore expected to be much + * higher, and the two are not directly comparable. + */ + RpcTrustSignals = 'rpc-trust-signals', + /** + * A dapp-initiated transaction or signature raised a confirmation. + */ + Confirmations = 'confirmations', + /** + * The user opened the Reveal Secret Recovery Phrase screen, which scans the + * active tab's origin. + */ + RevealSrp = 'reveal-srp', + /** + * NFT metadata, image, and external URLs scanned when NFTs are added or + * auto-detected. + */ + NftDetection = 'nft-detection', +} + +/** + * A composed `x-request-source` value. + */ +export type RequestSource = + | `${RequestSourcePlatform}-${RequestSourceFlow}` + | `${RequestSourcePlatform}-${typeof UNKNOWN_REQUEST_SOURCE}` + | typeof UNKNOWN_REQUEST_SOURCE; + +/** + * Checks whether a value is a recognised platform. + * + * @param value - The value to check. + * @returns Whether the value is a {@link RequestSourcePlatform}. + */ +const isKnownPlatform = (value?: string): value is RequestSourcePlatform => + Object.values(RequestSourcePlatform).includes(value as RequestSourcePlatform); + +/** + * Checks whether a value is a recognised flow. + * + * @param value - The value to check. + * @returns Whether the value is a {@link RequestSourceFlow}. + */ +const isKnownFlow = (value?: string): value is RequestSourceFlow => + Object.values(RequestSourceFlow).includes(value as RequestSourceFlow); + +/** + * Builds the `x-request-source` header value. + * + * Both arguments are validated at runtime rather than trusted, because some + * call sites are plain JavaScript and get no compile-time checking. An + * unrecognised value degrades to a sentinel; it never throws, so attribution + * cannot fail a scan. + * + * @param platform - The client emitting the scan. + * @param flow - The flow that caused the scan. + * @returns The composed source, or a sentinel when either part is unknown. + */ +export function buildRequestSource( + platform?: RequestSourcePlatform, + flow?: RequestSourceFlow, +): RequestSource { + if (!isKnownPlatform(platform)) { + return UNKNOWN_REQUEST_SOURCE; + } + + if (!isKnownFlow(flow)) { + return `${platform}-${UNKNOWN_REQUEST_SOURCE}`; + } + + return `${platform}-${flow}`; +} From 3ebf53cb29db3d520c9afeb03e67f1bca0cd2c4d Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Tue, 22 Sep 2026 17:50:14 -0500 Subject: [PATCH 2/6] chore: update changelog --- packages/phishing-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index 7aa45a1f456..dace5d84cfe 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add request-source attribution to dapp-scanning URL scans, emitted as an `x-request-source` header ([#10260](https://github.com/MetaMask/core/pull/10260)) +- Add request-source attribution to dapp-scanning URL scans, emitted as an `x-request-source` header ([#10357](https://github.com/MetaMask/core/pull/10357)) - Add an optional `platform` constructor option and an optional `flow` parameter to `scanUrl` and `bulkScanUrls`. The header value is composed as `-`. - Export the `RequestSourcePlatform` and `RequestSourceFlow` enums, the `RequestSource` type, the `REQUEST_SOURCE_HEADER` and `UNKNOWN_REQUEST_SOURCE` constants, and the `buildRequestSource` helper. - `RequestSourcePlatform` is `Extension` or `Mobile`. `RequestSourceFlow` is one of `dapp-connection`, `browser`, `rpc-trust-signals`, `confirmations`, `reveal-srp`, or `nft-detection`. From 6ae787a6bae4b7e51e2f7236ad018266d23646be Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Fri, 25 Sep 2026 14:08:29 -0500 Subject: [PATCH 3/6] chore: lowercase the request source platform --- packages/phishing-controller/src/request-source.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/phishing-controller/src/request-source.ts b/packages/phishing-controller/src/request-source.ts index 7cc3349f2ab..4099feda13a 100644 --- a/packages/phishing-controller/src/request-source.ts +++ b/packages/phishing-controller/src/request-source.ts @@ -18,8 +18,8 @@ export const UNKNOWN_REQUEST_SOURCE = 'unknown'; * constructed, since a single instance only ever runs on one platform. */ export enum RequestSourcePlatform { - Extension = 'Extension', - Mobile = 'Mobile', + Extension = 'extension', + Mobile = 'mobile', } /** From c8e925e3f593f1f449db245c8b5ce5eb595a3ce0 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Fri, 25 Sep 2026 14:46:23 -0500 Subject: [PATCH 4/6] docs: improve --- .../PhishingController-method-action-types.ts | 12 ++-- .../src/PhishingController.ts | 19 +++---- .../phishing-controller/src/request-source.ts | 55 +++++++------------ 3 files changed, 34 insertions(+), 52 deletions(-) diff --git a/packages/phishing-controller/src/PhishingController-method-action-types.ts b/packages/phishing-controller/src/PhishingController-method-action-types.ts index 1b83d315b8b..7a666345031 100644 --- a/packages/phishing-controller/src/PhishingController-method-action-types.ts +++ b/packages/phishing-controller/src/PhishingController-method-action-types.ts @@ -75,9 +75,9 @@ export type PhishingControllerBypassAction = { * Only supports web URLs (`http:` / `https:`). * * @param url - The URL to scan. - * @param flow - The flow that caused this scan, reported via the - * `x-request-source` header. Callers should always supply this; omitting it - * reports the scan as unattributed. + * @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 = { @@ -90,9 +90,9 @@ export type PhishingControllerScanUrlAction = { * It also only supports web URLs. * * @param urls - The URLs to scan. - * @param flow - The flow that caused this scan, reported via the - * `x-request-source` header. Callers should always supply this; omitting it - * reports the scan as unattributed. + * @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 = { diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index 42a54eb8ae1..c38981ab032 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -393,7 +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 emitting URL scans, reported via the `x-request-source` header. + * platform - Client used to attribute URL scans in the `x-request-source` header. */ export type PhishingControllerOptions = { stalelistRefreshInterval?: number; @@ -548,9 +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 emitting URL scans, reported via the - * `x-request-source` header. When omitted, URL scans report an unattributed - * sentinel instead. + * @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, @@ -1222,9 +1221,9 @@ export class PhishingController extends BaseController< * Only supports web URLs (`http:` / `https:`). * * @param url - The URL to scan. - * @param flow - The flow that caused this scan, reported via the - * `x-request-source` header. Callers should always supply this; omitting it - * reports the scan as unattributed. + * @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( @@ -1302,9 +1301,9 @@ export class PhishingController extends BaseController< * It also only supports web URLs. * * @param urls - The URLs to scan. - * @param flow - The flow that caused this scan, reported via the - * `x-request-source` header. Callers should always supply this; omitting it - * reports the scan as unattributed. + * @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( diff --git a/packages/phishing-controller/src/request-source.ts b/packages/phishing-controller/src/request-source.ts index 4099feda13a..370785f425e 100644 --- a/packages/phishing-controller/src/request-source.ts +++ b/packages/phishing-controller/src/request-source.ts @@ -1,21 +1,16 @@ /** - * Request header carrying the first-party flow that caused a scan. - * - * This is untrusted observability metadata. It must never be used for - * authentication, authorization, or to bypass rate limits. + * HTTP header used to attribute a phishing-detection URL scan to a MetaMask + * client and product flow. */ export const REQUEST_SOURCE_HEADER = 'x-request-source'; /** - * Emitted when the platform is not configured, so unattributed traffic is - * measurable rather than indistinguishable from a client that predates the - * header. + * Request-source value used when the client platform is unknown. */ export const UNKNOWN_REQUEST_SOURCE = 'unknown'; /** - * The client emitting the scan. Supplied once, when the controller is - * constructed, since a single instance only ever runs on one platform. + * MetaMask client that initiated a phishing-detection URL scan. */ export enum RequestSourcePlatform { Extension = 'extension', @@ -23,52 +18,41 @@ export enum RequestSourcePlatform { } /** - * The flow that caused a scan. Supplied per call, since one controller serves - * many flows. - * - * Values are bounded because the phishing detection service records them as a - * Prometheus label and normalizes anything it does not recognise to - * {@link UNKNOWN_REQUEST_SOURCE}. + * Product flow that initiated a phishing-detection URL scan. */ export enum RequestSourceFlow { /** - * A connect prompt was shown, or an advanced permission was granted. + * A dapp requested account-access permission, such as via + * `eth_requestAccounts` or `wallet_requestPermissions`. */ DappConnection = 'dapp-connection', /** - * Main-frame navigation in the mobile in-app browser. + * A main-frame navigation in the mobile in-app browser. */ Browser = 'browser', /** - * Origin scan triggered by dapp RPC traffic rather than by a user action. - * High request count, low distinct-URL count, so the cache absorbs most of - * it. - * - * The trigger differs by client: Mobile scans on every EIP-1193 request - * carrying an origin, whereas Extension scans only when a connected origin - * reads its own connection state (`eth_accounts`, or `wallet_getSession` on - * the Multichain transport). Mobile volume is therefore expected to be much - * higher, and the two are not directly comparable. + * Dapp RPC traffic used as a trust signal. */ RpcTrustSignals = 'rpc-trust-signals', /** - * A dapp-initiated transaction or signature raised a confirmation. + * A dapp request that opens a transaction, signature, or permission approval. */ Confirmations = 'confirmations', /** - * The user opened the Reveal Secret Recovery Phrase screen, which scans the - * active tab's origin. + * A scan of the active dapp origin when the user opens the Secret Recovery + * Phrase reveal screen. */ RevealSrp = 'reveal-srp', /** - * NFT metadata, image, and external URLs scanned when NFTs are added or + * NFT metadata, image, and external URLs scanned while NFTs are added or * auto-detected. */ NftDetection = 'nft-detection', } /** - * A composed `x-request-source` value. + * Valid {@link REQUEST_SOURCE_HEADER} value: a platform and flow, a platform + * with an unknown flow, or an unknown platform. */ export type RequestSource = | `${RequestSourcePlatform}-${RequestSourceFlow}` @@ -94,12 +78,11 @@ const isKnownFlow = (value?: string): value is RequestSourceFlow => Object.values(RequestSourceFlow).includes(value as RequestSourceFlow); /** - * Builds the `x-request-source` header value. + * Builds a value for {@link REQUEST_SOURCE_HEADER}. * - * Both arguments are validated at runtime rather than trusted, because some - * call sites are plain JavaScript and get no compile-time checking. An - * unrecognised value degrades to a sentinel; it never throws, so attribution - * cannot fail a scan. + * Returns {@link UNKNOWN_REQUEST_SOURCE} when `platform` is unknown, or a + * platform-specific unknown value when `flow` is unknown. This function never + * throws for an unrecognised input. * * @param platform - The client emitting the scan. * @param flow - The flow that caused the scan. From 5a24d2ffcc3b482f23d1ababf14a2df68d9309f6 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Mon, 28 Sep 2026 10:02:47 -0500 Subject: [PATCH 5/6] fix: lint --- .../src/PhishingController.test.ts | 8 +++---- .../src/request-source.test.ts | 24 +++++++++---------- .../phishing-controller/src/request-source.ts | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index f13409f2895..21d049dc353 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -3072,7 +3072,7 @@ describe('PhishingController', () => { platform: RequestSourcePlatform.Mobile, }); const scope = nock(PHISHING_DETECTION_BASE_URL) - .matchHeader('x-request-source', 'Mobile-browser') + .matchHeader('x-request-source', 'mobile-browser') .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) .query({ url: 'example.com' }) .reply(200, scanResponse); @@ -3091,7 +3091,7 @@ describe('PhishingController', () => { platform: RequestSourcePlatform.Extension, }); const scope = nock(PHISHING_DETECTION_BASE_URL) - .matchHeader('x-request-source', 'Extension-nft-detection') + .matchHeader('x-request-source', 'extension-nft-detection') .post(`/${PHISHING_DETECTION_BULK_SCAN_ENDPOINT}`) .reply(200, { results: {}, errors: {} }); @@ -3109,7 +3109,7 @@ describe('PhishingController', () => { platform: RequestSourcePlatform.Extension, }); const scope = nock(PHISHING_DETECTION_BASE_URL) - .matchHeader('x-request-source', 'Extension-unknown') + .matchHeader('x-request-source', 'extension-unknown') .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) .query({ url: 'example.com' }) .reply(200, scanResponse); @@ -3145,7 +3145,7 @@ describe('PhishingController', () => { platform: RequestSourcePlatform.Mobile, }); const scope = nock(PHISHING_DETECTION_BASE_URL) - .matchHeader('x-request-source', 'Mobile-unknown') + .matchHeader('x-request-source', 'mobile-unknown') .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) .query({ url: 'example.com' }) .reply(200, scanResponse); diff --git a/packages/phishing-controller/src/request-source.test.ts b/packages/phishing-controller/src/request-source.test.ts index 2377cb2de69..58645661b2a 100644 --- a/packages/phishing-controller/src/request-source.test.ts +++ b/packages/phishing-controller/src/request-source.test.ts @@ -21,43 +21,43 @@ describe('buildRequestSource', () => { [ RequestSourcePlatform.Extension, RequestSourceFlow.DappConnection, - 'Extension-dapp-connection', + 'extension-dapp-connection', ], [ RequestSourcePlatform.Extension, RequestSourceFlow.RpcTrustSignals, - 'Extension-rpc-trust-signals', + 'extension-rpc-trust-signals', ], [ RequestSourcePlatform.Extension, RequestSourceFlow.Confirmations, - 'Extension-confirmations', + 'extension-confirmations', ], [ RequestSourcePlatform.Extension, RequestSourceFlow.RevealSrp, - 'Extension-reveal-srp', + 'extension-reveal-srp', ], [ RequestSourcePlatform.Extension, RequestSourceFlow.NftDetection, - 'Extension-nft-detection', + 'extension-nft-detection', ], [ RequestSourcePlatform.Mobile, RequestSourceFlow.DappConnection, - 'Mobile-dapp-connection', + 'mobile-dapp-connection', ], - [RequestSourcePlatform.Mobile, RequestSourceFlow.Browser, 'Mobile-browser'], + [RequestSourcePlatform.Mobile, RequestSourceFlow.Browser, 'mobile-browser'], [ RequestSourcePlatform.Mobile, RequestSourceFlow.RpcTrustSignals, - 'Mobile-rpc-trust-signals', + 'mobile-rpc-trust-signals', ], [ RequestSourcePlatform.Mobile, RequestSourceFlow.NftDetection, - 'Mobile-nft-detection', + 'mobile-nft-detection', ], ]; @@ -70,10 +70,10 @@ describe('buildRequestSource', () => { it('falls back to the platform sentinel when no flow is given', () => { expect(buildRequestSource(RequestSourcePlatform.Extension)).toBe( - 'Extension-unknown', + 'extension-unknown', ); expect(buildRequestSource(RequestSourcePlatform.Mobile)).toBe( - 'Mobile-unknown', + 'mobile-unknown', ); }); @@ -85,7 +85,7 @@ describe('buildRequestSource', () => { RequestSourcePlatform.Extension, 'dapp-conection' as RequestSourceFlow, ), - ).toBe('Extension-unknown'); + ).toBe('extension-unknown'); }); it('returns the bare sentinel when no platform is configured', () => { diff --git a/packages/phishing-controller/src/request-source.ts b/packages/phishing-controller/src/request-source.ts index 370785f425e..669201f317f 100644 --- a/packages/phishing-controller/src/request-source.ts +++ b/packages/phishing-controller/src/request-source.ts @@ -31,7 +31,7 @@ export enum RequestSourceFlow { */ Browser = 'browser', /** - * Dapp RPC traffic used as a trust signal. + * Dapp RPC traffic used as a trust signal. */ RpcTrustSignals = 'rpc-trust-signals', /** From 5c123d25319d784361f2d2c47da68140f6101d04 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Mon, 28 Sep 2026 14:25:48 -0500 Subject: [PATCH 6/6] chore: refine changelog --- packages/phishing-controller/CHANGELOG.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index dace5d84cfe..3b99ebc60fd 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -9,13 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add request-source attribution to dapp-scanning URL scans, emitted as an `x-request-source` header ([#10357](https://github.com/MetaMask/core/pull/10357)) - - Add an optional `platform` constructor option and an optional `flow` parameter to `scanUrl` and `bulkScanUrls`. The header value is composed as `-`. - - Export the `RequestSourcePlatform` and `RequestSourceFlow` enums, the `RequestSource` type, the `REQUEST_SOURCE_HEADER` and `UNKNOWN_REQUEST_SOURCE` constants, and the `buildRequestSource` helper. - - `RequestSourcePlatform` is `Extension` or `Mobile`. `RequestSourceFlow` is one of `dapp-connection`, `browser`, `rpc-trust-signals`, `confirmations`, `reveal-srp`, or `nft-detection`. - - Both values are validated at runtime, since some consumers call these methods from untyped JavaScript. An unrecognised or absent value degrades to a sentinel (`-unknown`, or `unknown` when no platform is configured) and never throws, so attribution cannot fail a scan. - - The header is untrusted observability metadata and must not be used for authentication, authorization, or rate-limit decisions. - - Only the dapp-scanning endpoints (`v2/scan`, `bulk-scan`) are covered. The Security Alerts endpoints used by `scanAddress`, `bulkScanTokens`, and `getApprovals` are unchanged. +- 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