diff --git a/packages/snap/src/core/constants/solana.ts b/packages/snap/src/core/constants/solana.ts index 6d21356e..72282f8f 100644 --- a/packages/snap/src/core/constants/solana.ts +++ b/packages/snap/src/core/constants/solana.ts @@ -8,6 +8,7 @@ export const MICRO_LAMPORTS_PER_LAMPORTS = 1_000_000n; export const LAMPORTS_PER_SOL = 1_000_000_000; export const DEFAULT_NETWORK_BLOCK_EXPLORER_URL = 'https://solscan.io'; export const METAMASK_ORIGIN = 'metamask'; +export const WALLET_CONNECT_ORIGIN = 'wallet-connect'; export const METAMASK_ORIGIN_URL = 'https://metamask.io'; /** @@ -181,3 +182,12 @@ export const Networks = { } as const; export type Caip10Address = `${Network}:${string}`; + +/** + * Map of known non-URL origins to their human-readable display labels. + * Keys are compared case-insensitively against the raw origin string. + */ +export const KNOWN_ORIGIN_LABELS: Record = { + [METAMASK_ORIGIN]: 'MetaMask', + [WALLET_CONNECT_ORIGIN]: 'WalletConnect', +}; diff --git a/packages/snap/src/core/services/assets/AssetsService.test.ts b/packages/snap/src/core/services/assets/AssetsService.test.ts index f313fbbb..681eb8b9 100644 --- a/packages/snap/src/core/services/assets/AssetsService.test.ts +++ b/packages/snap/src/core/services/assets/AssetsService.test.ts @@ -306,6 +306,7 @@ describe('AssetsService', () => { }); // With isIncremental = false, we do emit events, even when no assets changed + // eslint-disable-next-line jest/no-disabled-tests it.skip('does not emit events when no assets changed', async () => { jest .spyOn(mockAssetsRepository, 'getAll') @@ -399,6 +400,7 @@ describe('AssetsService', () => { }); // With isIncremental = false, we do emit events, even when no assets changed + // eslint-disable-next-line jest/no-disabled-tests it.skip('does not incorrectly mark assets as new when they are already in the saved state', async () => { // Mock that the asset already exists in saved state jest diff --git a/packages/snap/src/core/services/transaction-scan/TransactionScan.test.ts b/packages/snap/src/core/services/transaction-scan/TransactionScan.test.ts index 86ea6ac2..b8e6e54d 100644 --- a/packages/snap/src/core/services/transaction-scan/TransactionScan.test.ts +++ b/packages/snap/src/core/services/transaction-scan/TransactionScan.test.ts @@ -1,7 +1,11 @@ /* eslint-disable @typescript-eslint/naming-convention */ import type { SecurityAlertsApiClient } from '../../clients/security-alerts-api/SecurityAlertsApiClient'; import type { SecurityAlertSimulationValidationResponse } from '../../clients/security-alerts-api/types'; -import { Network } from '../../constants/solana'; +import { + METAMASK_ORIGIN_URL, + Network, + WALLET_CONNECT_ORIGIN, +} from '../../constants/solana'; import { MOCK_SOLANA_KEYRING_ACCOUNT_0 } from '../../test/mocks/solana-keyring-accounts'; import { trackError } from '../../utils/errors'; import type { ILogger } from '../../utils/logger'; @@ -61,6 +65,49 @@ describe('TransactionScan', () => { }); }); + it('passes the MetaMask URL to the client for the internal MetaMask origin', async () => { + const scanTransactionsSpy = jest + .spyOn(mockSecurityAlertsApiClient, 'scanTransactions') + .mockResolvedValue({ + status: 'SUCCESS', + } as SecurityAlertSimulationValidationResponse); + + await transactionScanService.scanTransaction({ + method: 'method', + accountAddress: 'accountAddress', + transaction: 'transaction', + scope: Network.Mainnet, + origin: 'metamask', + }); + + expect(scanTransactionsSpy).toHaveBeenCalledWith( + expect.objectContaining({ origin: METAMASK_ORIGIN_URL }), + ); + }); + + it('passes the WalletConnect origin to the client', async () => { + const scanTransactionsSpy = jest + .spyOn(mockSecurityAlertsApiClient, 'scanTransactions') + .mockResolvedValue({ + status: 'SUCCESS', + } as SecurityAlertSimulationValidationResponse); + + const result = await transactionScanService.scanTransaction({ + method: 'method', + accountAddress: 'accountAddress', + transaction: 'transaction', + scope: Network.Mainnet, + origin: WALLET_CONNECT_ORIGIN, + }); + + expect(result).toMatchObject({ + status: 'SUCCESS', + }); + expect(scanTransactionsSpy).toHaveBeenCalledWith( + expect.objectContaining({ origin: WALLET_CONNECT_ORIGIN }), + ); + }); + it('returns null if the scan fails', async () => { const error = new Error('Scan failed'); jest diff --git a/packages/snap/src/core/services/transaction-scan/TransactionScan.ts b/packages/snap/src/core/services/transaction-scan/TransactionScan.ts index ed7857f5..0600c5b9 100644 --- a/packages/snap/src/core/services/transaction-scan/TransactionScan.ts +++ b/packages/snap/src/core/services/transaction-scan/TransactionScan.ts @@ -59,6 +59,7 @@ export class TransactionScanService { options?: string[]; account?: SolanaKeyringAccount; }): Promise { + // The origin could be METAMASK_ORIGIN_URL, WALLET_CONNECT_ORIGIN or any valid URL. try { const result = await this.#securityAlertsApiClient.scanTransactions({ method, diff --git a/packages/snap/src/core/services/transactions/TransactionMapper.test.ts b/packages/snap/src/core/services/transactions/TransactionMapper.test.ts index 9828425b..3568fec3 100644 --- a/packages/snap/src/core/services/transactions/TransactionMapper.test.ts +++ b/packages/snap/src/core/services/transactions/TransactionMapper.test.ts @@ -1081,6 +1081,7 @@ describe('TransactionMapper', () => { // We end this transaction with more of all assets than we started with meaning // we think it's a receive transaction but it's not, it's actually a Swap. // https://solscan.io/tx/2m8z8uPZyoZwQpissDbhSfW5XDTFmpc7cSFithc5e1w8iCwFcvVkxHeaVhgFSdgUPb5cebbKGjuu48JMLPjfEATr + // eslint-disable-next-line jest/no-disabled-tests it.skip('maps swaps - #6 SOL -> USDC', async () => { jest .spyOn(mockTokenHelper, 'amountToUiAmountForMint') @@ -1150,6 +1151,7 @@ describe('TransactionMapper', () => { }); }); + // eslint-disable-next-line jest/no-disabled-tests it.skip('maps swaps - #7 SOL -> OBRIC', async () => { jest .spyOn(mockTokenHelper, 'amountToUiAmountForMint') diff --git a/packages/snap/src/core/utils/parseOrigin.test.ts b/packages/snap/src/core/utils/parseOrigin.test.ts index b209ed65..86c35036 100644 --- a/packages/snap/src/core/utils/parseOrigin.test.ts +++ b/packages/snap/src/core/utils/parseOrigin.test.ts @@ -1,14 +1,18 @@ -import { METAMASK_ORIGIN } from '../constants/solana'; -import { parseOrigin } from './parseOrigin'; +import { isKnownOrigin, parseOrigin } from './parseOrigin'; describe('parseOrigin', () => { - describe('when origin is MetaMask', () => { - it('returns "MetaMask" for metamask origin', () => { - expect(parseOrigin(METAMASK_ORIGIN)).toBe('MetaMask'); + describe('when origin is a known origin', () => { + it('returns the MetaMask label for the metamask origin', () => { + expect(parseOrigin('metamask')).toBe('MetaMask'); }); - it('returns "MetaMask" for exact string match', () => { - expect(parseOrigin('metamask')).toBe('MetaMask'); + it('returns the WalletConnect label for the wallet-connect origin', () => { + expect(parseOrigin('wallet-connect')).toBe('WalletConnect'); + }); + + it('matches known origins case-insensitively', () => { + expect(parseOrigin('MetaMask')).toBe('MetaMask'); + expect(parseOrigin('Wallet-Connect')).toBe('WalletConnect'); }); }); @@ -70,15 +74,25 @@ describe('parseOrigin', () => { }); describe('edge cases', () => { - it('throws an error for URLs without protocol', () => { - expect(() => parseOrigin('//example.com')).toThrow('Invalid URL'); - expect(() => parseOrigin('//www.example.com')).toThrow('Invalid URL'); + it('throws for URLs without protocol', () => { + expect(() => parseOrigin('//example.com')).toThrow( + 'Invalid origin: //example.com. Must be a valid URL or a known origin.', + ); + expect(() => parseOrigin('//www.example.com')).toThrow( + 'Invalid origin: //www.example.com. Must be a valid URL or a known origin.', + ); }); - it('handles URLs with custom protocols', () => { - expect(parseOrigin('ftp://example.com')).toBe('example.com'); - expect(parseOrigin('ws://example.com')).toBe('example.com'); - expect(parseOrigin('wss://example.com')).toBe('example.com'); + it('throws for non-HTTP URLs', () => { + expect(() => parseOrigin('ftp://example.com')).toThrow( + 'Invalid origin: ftp://example.com. Must be a valid URL or a known origin.', + ); + expect(() => parseOrigin('ws://example.com')).toThrow( + 'Invalid origin: ws://example.com. Must be a valid URL or a known origin.', + ); + expect(() => parseOrigin('wss://example.com')).toThrow( + 'Invalid origin: wss://example.com. Must be a valid URL or a known origin.', + ); }); it('handles complex subdomains', () => { @@ -92,15 +106,37 @@ describe('parseOrigin', () => { }); describe('error handling', () => { - it('throws error for invalid URLs', () => { - expect(() => parseOrigin('not-a-url')).toThrow('Invalid URL'); - expect(() => parseOrigin('http://')).toThrow('Invalid URL'); - expect(() => parseOrigin('https://')).toThrow('Invalid URL'); - expect(() => parseOrigin('')).toThrow('Invalid URL'); + it('throws for invalid URLs', () => { + expect(() => parseOrigin('not-a-url')).toThrow( + 'Invalid origin: not-a-url. Must be a valid URL or a known origin.', + ); + expect(() => parseOrigin('http://')).toThrow( + 'Invalid origin: http://. Must be a valid URL or a known origin.', + ); + expect(() => parseOrigin('https://')).toThrow( + 'Invalid origin: https://. Must be a valid URL or a known origin.', + ); + expect(() => parseOrigin('')).toThrow( + 'Invalid origin: . Must be a valid URL or a known origin.', + ); }); - it('throws error for malformed URLs', () => { - expect(() => parseOrigin('http://:8080')).toThrow('Invalid URL'); + it('throws for malformed URLs', () => { + expect(() => parseOrigin('http://:8080')).toThrow( + 'Invalid origin: http://:8080. Must be a valid URL or a known origin.', + ); }); }); }); + +describe('isKnownOrigin', () => { + it('returns true for the WalletConnect origin', () => { + expect(isKnownOrigin('wallet-connect')).toBe(true); + expect(isKnownOrigin('metamask')).toBe(true); + }); + + it('returns false for other origins', () => { + expect(isKnownOrigin('https://example.com')).toBe(false); + expect(isKnownOrigin(undefined)).toBe(false); + }); +}); diff --git a/packages/snap/src/core/utils/parseOrigin.ts b/packages/snap/src/core/utils/parseOrigin.ts index ad99e043..cc43facb 100644 --- a/packages/snap/src/core/utils/parseOrigin.ts +++ b/packages/snap/src/core/utils/parseOrigin.ts @@ -1,19 +1,51 @@ -import { METAMASK_ORIGIN } from '../constants/solana'; +import { + KNOWN_ORIGIN_LABELS, + WALLET_CONNECT_ORIGIN, +} from '../constants/solana'; /** - * Parses the origin from a string. + * Parses the origin into a human-readable display label. * * @param origin - The origin to parse. - * @returns The parsed origin. + * @returns The display label: a known-origin label, the hostname for http(s) URLs. */ export function parseOrigin(origin: string) { - if (origin === METAMASK_ORIGIN) { - return 'MetaMask'; + const knownLabel = KNOWN_ORIGIN_LABELS[origin?.toLowerCase()]; + if (knownLabel) { + return knownLabel; } try { - return new URL(origin).hostname; + const url = new URL(origin); + if (!isHttpOrHttpsUrl(url)) { + throw new Error('Invalid url'); + } + return url.hostname; } catch (error) { - throw new Error('Invalid URL'); + throw new Error( + `Invalid origin: ${origin}. Must be a valid URL or a known origin.`, + ); } } + +/** + * Checks whether an origin is a known origin. + * + * @param origin - The origin to check. + * @returns Whether the origin is a known origin. + */ +export function isKnownOrigin(origin: string | undefined): boolean { + return [...Object.keys(KNOWN_ORIGIN_LABELS)].includes( + origin?.toLowerCase() ?? '', + ); +} + +/** + * Checks whether a parsed URL uses an HTTP(S) protocol. + * + * @param url - The parsed URL to check. + * @returns Whether the URL uses HTTP or HTTPS. + */ +function isHttpOrHttpsUrl(url: URL): boolean { + return url.protocol === 'http:' || url.protocol === 'https:'; +} diff --git a/packages/snap/src/features/confirmation/views/ConfirmSignIn/render.tsx b/packages/snap/src/features/confirmation/views/ConfirmSignIn/render.tsx index 5dd19f6f..a4d3fbf8 100644 --- a/packages/snap/src/features/confirmation/views/ConfirmSignIn/render.tsx +++ b/packages/snap/src/features/confirmation/views/ConfirmSignIn/render.tsx @@ -8,6 +8,7 @@ import { getPreferences, showDialog, } from '../../../../core/utils/interface'; +import { isKnownOrigin } from '../../../../core/utils/parseOrigin'; import type { SolanaKeyringAccount } from '../../../../entities'; import { nameResolutionService } from '../../../../snapContext'; import type { ConfirmSignInProps } from './ConfirmSignIn'; @@ -32,6 +33,10 @@ export async function render( origin, } = request; + if (isKnownOrigin(origin)) { + throw new Error('Sign-in requests require a dapp URL origin.'); + } + const [preferences, accountDomain] = await Promise.all([ getPreferences(), nameResolutionService.resolveAddress(scope, account.address),