Skip to content

Commit 906bfdd

Browse files
authored
feat(metrics): add eth-chainlist domain check to isPublicEndpointUrl cp-13.17.0 (#39623)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Add eth-chainlist domain checking to isPublicEndpointUrl to improve coverage for identifying public RPC endpoints in metrics. If a domain is defined in eth-chainlist (cached), it's considered public and safe to report. Initialization only happens in background context to avoid ~300KB memory footprint in UI. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/39623?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/WPC-354 ## **Manual testing steps** ```gherkin Feature: RPC domain analytics Scenario: Verify rpc_domain shows actual host when switching to public RPC via banner # Setup - Add Ink network with local RPC Given user navigates to Settings → Networks → Add Network And user adds Ink network (Chain ID: 57073) with local RPC endpoint: http://127.0.0.1:8545 And user also adds public RPC endpoint: https://rpc-qnd.inkonchain.com And user sets the local RPC as the default endpoint And user switches to Ink network # Trigger degraded state When user disconnects local RPC (or it becomes unavailable) And user waits for banner showing "Still connecting to Ink..." # Trigger RPC update from banner Then the "Update RPC" button appears on the banner When user clicks "Update RPC" on the banner And user is navigated to Edit Network screen And user switches default RPC to https://rpc-qnd.inkonchain.com # Verify analytics in Segment When user checks Segment dashboard for "Network Connection Banner RPC Updated" event Then the event property from_rpc_domain should be "custom" (local RPC is private) And the event property to_rpc_domain should be "rpc-qnd.inkonchain.com" (known public domain) Scenario: Verify rpc_domain for Infura networks using Switch to MetaMask default # Setup - Configure Arbitrum with local RPC Given user starts a local Ganache server: npx ganache --chain.chainId 42161 And user navigates to Settings → Networks → Arbitrum One And user adds a new RPC endpoint: http://127.0.0.1:8545 And user sets the local RPC as the default endpoint # Trigger degraded state When user stops the Ganache server (Ctrl+C) And user waits for banner showing "Still connecting to Arbitrum One..." # Switch to Infura via banner button Then the "Switch to MetaMask default RPC" button appears on the banner When user clicks "Switch to MetaMask default RPC" Then the toast "Updated to MetaMask default" appears # Verify analytics When user checks Segment for "Network Connection Banner Switch To MetaMask Default RPC Clicked" Then rpc_domain should be "custom" (the local RPC being switched from) ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes privacy-sensitive URL sanitization used for metrics and introduces a new background↔UI call path; misclassification could leak or over-redact RPC domains in analytics. > > **Overview** > **Improves RPC-domain sanitization for metrics** by moving `isPublicEndpointUrl` into `app/scripts/lib/util.ts` and extending it to treat endpoints as public when their hostname appears in the cached safe chainlist, in addition to existing Infura/Quicknode/known-custom URL checks. > > To reduce privacy risk, chainlist-derived hostnames are filtered to exclude `localhost`, any IPv4/IPv6 literal, and RFC 6761 *special-use* domains (new `isSpecialUseDomain` + `ip-regex` dependency). UI metrics paths (`useNetworkConnectionBanner`, networks form banner tracking) now call `isPublicEndpointUrl` via `submitRequestToBackground('isPublicEndpointUrl')` (with error isolation), and tests were updated/added accordingly while removing the old `shared/lib/network-utils` `isPublicEndpointUrl` coverage. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 14ec257. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 1b0fd13 commit 906bfdd

File tree

13 files changed

+433
-185
lines changed

13 files changed

+433
-185
lines changed

app/scripts/lib/network-controller/messenger-action-handlers.test.ts

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { HttpError } from '@metamask/controller-utils';
2-
import * as networkUtilsModule from '../../../../shared/lib/network-utils';
2+
import * as utilModule from '../util';
33
import {
44
onRpcEndpointDegraded,
55
onRpcEndpointUnavailable,
@@ -14,8 +14,8 @@ describe('onRpcEndpointUnavailable', () => {
1414
Parameters<typeof networkControllerUtilsModule.shouldCreateRpcServiceEvents>
1515
>;
1616
let isPublicEndpointUrlMock: jest.SpyInstance<
17-
ReturnType<typeof networkUtilsModule.isPublicEndpointUrl>,
18-
Parameters<typeof networkUtilsModule.isPublicEndpointUrl>
17+
ReturnType<typeof utilModule.isPublicEndpointUrl>,
18+
Parameters<typeof utilModule.isPublicEndpointUrl>
1919
>;
2020

2121
beforeEach(() => {
@@ -24,10 +24,7 @@ describe('onRpcEndpointUnavailable', () => {
2424
'shouldCreateRpcServiceEvents',
2525
);
2626

27-
isPublicEndpointUrlMock = jest.spyOn(
28-
networkUtilsModule,
29-
'isPublicEndpointUrl',
30-
);
27+
isPublicEndpointUrlMock = jest.spyOn(utilModule, 'isPublicEndpointUrl');
3128
});
3229

3330
it('calls shouldCreateRpcServiceEvents with the correct parameters', () => {
@@ -170,8 +167,8 @@ describe('onRpcEndpointDegraded', () => {
170167
Parameters<typeof networkControllerUtilsModule.shouldCreateRpcServiceEvents>
171168
>;
172169
let isPublicEndpointUrlMock: jest.SpyInstance<
173-
ReturnType<typeof networkUtilsModule.isPublicEndpointUrl>,
174-
Parameters<typeof networkUtilsModule.isPublicEndpointUrl>
170+
ReturnType<typeof utilModule.isPublicEndpointUrl>,
171+
Parameters<typeof utilModule.isPublicEndpointUrl>
175172
>;
176173

177174
beforeEach(() => {
@@ -180,10 +177,7 @@ describe('onRpcEndpointDegraded', () => {
180177
'shouldCreateRpcServiceEvents',
181178
);
182179

183-
isPublicEndpointUrlMock = jest.spyOn(
184-
networkUtilsModule,
185-
'isPublicEndpointUrl',
186-
);
180+
isPublicEndpointUrlMock = jest.spyOn(utilModule, 'isPublicEndpointUrl');
187181
});
188182

189183
it('calls shouldCreateRpcServiceEvents with the correct parameters', () => {

app/scripts/lib/network-controller/messenger-action-handlers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
MetaMetricsEventName,
66
} from '../../../../shared/constants/metametrics';
77
import { onlyKeepHost } from '../../../../shared/lib/only-keep-host';
8-
import { isPublicEndpointUrl } from '../../../../shared/lib/network-utils';
8+
import { isPublicEndpointUrl } from '../util';
99
import MetaMetricsController from '../../controllers/metametrics-controller';
1010
import { shouldCreateRpcServiceEvents } from './utils';
1111

app/scripts/lib/util.test.js

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import {
3030
extractRpcDomain,
3131
isKnownDomain,
3232
initializeRpcProviderDomains,
33+
isPublicEndpointUrl,
34+
isSpecialUseDomain,
3335
} from './util';
3436

3537
// Mock the module
@@ -608,4 +610,145 @@ describe('app utils', () => {
608610
expect(extractRpcDomain('https://')).toBe('invalid');
609611
});
610612
});
613+
614+
describe('isPublicEndpointUrl', () => {
615+
const MOCK_INFURA_PROJECT_ID = 'test-project-id';
616+
617+
it('should return true for Infura URLs', () => {
618+
expect(
619+
isPublicEndpointUrl(
620+
`https://mainnet.infura.io/v3/${MOCK_INFURA_PROJECT_ID}`,
621+
MOCK_INFURA_PROJECT_ID,
622+
),
623+
).toBe(true);
624+
});
625+
626+
it('should return false for unknown URLs', () => {
627+
expect(
628+
isPublicEndpointUrl(
629+
'https://unknown.example.com',
630+
MOCK_INFURA_PROJECT_ID,
631+
),
632+
).toBe(false);
633+
});
634+
635+
describe('localhost and IP addresses', () => {
636+
it('should return false for localhost', () => {
637+
expect(
638+
isPublicEndpointUrl('http://localhost:8545', MOCK_INFURA_PROJECT_ID),
639+
).toBe(false);
640+
});
641+
642+
it('should return false for any IPv4 address', () => {
643+
// Loopback
644+
expect(
645+
isPublicEndpointUrl('http://127.0.0.1:8545', MOCK_INFURA_PROJECT_ID),
646+
).toBe(false);
647+
// Private ranges
648+
expect(
649+
isPublicEndpointUrl('http://10.0.0.1:8545', MOCK_INFURA_PROJECT_ID),
650+
).toBe(false);
651+
expect(
652+
isPublicEndpointUrl(
653+
'http://192.168.1.1:8545',
654+
MOCK_INFURA_PROJECT_ID,
655+
),
656+
).toBe(false);
657+
// Public IPs should also return false (public providers use domain names)
658+
expect(
659+
isPublicEndpointUrl('http://8.8.8.8:8545', MOCK_INFURA_PROJECT_ID),
660+
).toBe(false);
661+
});
662+
663+
it('should return false for any IPv6 address', () => {
664+
expect(
665+
isPublicEndpointUrl('http://[::1]:8545', MOCK_INFURA_PROJECT_ID),
666+
).toBe(false);
667+
expect(
668+
isPublicEndpointUrl(
669+
'http://[2001:db8::1]:8545',
670+
MOCK_INFURA_PROJECT_ID,
671+
),
672+
).toBe(false);
673+
});
674+
});
675+
});
676+
677+
describe('isSpecialUseDomain', () => {
678+
describe('RFC 6761 special-use TLDs', () => {
679+
it('should return true for .test TLD', () => {
680+
expect(isSpecialUseDomain('myapp.test')).toBe(true);
681+
expect(isSpecialUseDomain('rpc.myapp.test')).toBe(true);
682+
});
683+
684+
it('should return true for .localhost TLD', () => {
685+
expect(isSpecialUseDomain('myapp.localhost')).toBe(true);
686+
expect(isSpecialUseDomain('rpc.myapp.localhost')).toBe(true);
687+
});
688+
689+
it('should return true for .invalid TLD', () => {
690+
expect(isSpecialUseDomain('myapp.invalid')).toBe(true);
691+
expect(isSpecialUseDomain('rpc.myapp.invalid')).toBe(true);
692+
});
693+
694+
it('should return true for .example TLD', () => {
695+
expect(isSpecialUseDomain('myapp.example')).toBe(true);
696+
expect(isSpecialUseDomain('rpc.myapp.example')).toBe(true);
697+
});
698+
699+
it('should return true for .local TLD', () => {
700+
expect(isSpecialUseDomain('myapp.local')).toBe(true);
701+
expect(isSpecialUseDomain('rpc.myapp.local')).toBe(true);
702+
});
703+
});
704+
705+
describe('RFC 6761 reserved example domains', () => {
706+
it('should return true for example.com', () => {
707+
expect(isSpecialUseDomain('example.com')).toBe(true);
708+
expect(isSpecialUseDomain('rpc.example.com')).toBe(true);
709+
expect(isSpecialUseDomain('api.rpc.example.com')).toBe(true);
710+
});
711+
712+
it('should return true for example.net', () => {
713+
expect(isSpecialUseDomain('example.net')).toBe(true);
714+
expect(isSpecialUseDomain('rpc.example.net')).toBe(true);
715+
expect(isSpecialUseDomain('api.rpc.example.net')).toBe(true);
716+
});
717+
718+
it('should return true for example.org', () => {
719+
expect(isSpecialUseDomain('example.org')).toBe(true);
720+
expect(isSpecialUseDomain('rpc.example.org')).toBe(true);
721+
expect(isSpecialUseDomain('api.rpc.example.org')).toBe(true);
722+
});
723+
});
724+
725+
describe('case insensitivity', () => {
726+
it('should be case insensitive', () => {
727+
expect(isSpecialUseDomain('EXAMPLE.COM')).toBe(true);
728+
expect(isSpecialUseDomain('MyApp.TEST')).toBe(true);
729+
expect(isSpecialUseDomain('RPC.Example.Org')).toBe(true);
730+
});
731+
});
732+
733+
describe('valid public domains', () => {
734+
it('should return false for regular domains', () => {
735+
expect(isSpecialUseDomain('infura.io')).toBe(false);
736+
expect(isSpecialUseDomain('mainnet.infura.io')).toBe(false);
737+
expect(isSpecialUseDomain('alchemy.com')).toBe(false);
738+
expect(isSpecialUseDomain('rpc.ankr.com')).toBe(false);
739+
});
740+
741+
it('should return false for domains containing but not ending with special TLDs', () => {
742+
expect(isSpecialUseDomain('test-rpc.com')).toBe(false);
743+
expect(isSpecialUseDomain('example-provider.io')).toBe(false);
744+
expect(isSpecialUseDomain('localhost-rpc.net')).toBe(false);
745+
});
746+
});
747+
748+
describe('edge cases', () => {
749+
it('should return false for empty string', () => {
750+
expect(isSpecialUseDomain('')).toBe(false);
751+
});
752+
});
753+
});
611754
});

0 commit comments

Comments
 (0)