feat: make MM Connect deeplink parsing more robust by jiexi · Pull Request #26648 · MetaMask/metamask-mobile

jiexi · 2026-02-26T19:26:50Z

Description

Adds more robust parsing to the MM Connect deeplink handler

Changelog

CHANGELOG entry: null

MM Connect not released to public yet

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1125?atlOrigin=eyJpIjoiNzg3ZGFiZWExYTE2NDdmZmFiMDM5YWNmN2MzNWRmY2MiLCJwIjoiaiJ9

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
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 format if applicable
I've applied the right labels on the PR (see labeling guidelines). 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.

Note

Medium Risk
Tightens validation for incoming MM Connect deeplink payloads and adds decompression size limits, which could reject previously accepted connection requests and affect connection onboarding flows.

Overview
Hardens MM Connect deeplink parsing and validation. isConnectionRequest() now enforces stricter constraints on connection request fields (UUID + handshake channel format, base64 public key decoding/size, allowed mode, future expiresAt, length limits, and http(s)-only dapp URLs with optional icon URL validation).

Adds additional safety checks in deeplink handling. ConnectionRegistry.parseConnectionRequest() now rejects decompressed payloads over 1MB, and handleConnectDeeplink() documents and tests the existing internal-origin block as defense-in-depth.

Expands automated coverage. Adds a comprehensive connection-request.test.ts suite and updates connection-registry.test.ts to use per-test fixtures plus new cases for oversized decompressed payloads and internal-origin blocking.

^{Written by Cursor Bugbot for commit c63abfe. This will update automatically on new commits. Configure here.}

jiexi · 2026-02-26T19:27:15Z

+  if (metadata.dapp.icon) {
+    if (
+      typeof metadata.dapp.icon !== 'string' ||
+      metadata.dapp.icon.length > 2048 // this seems long still


Maybe we drop it to 1000 and add validation that its only ever https:// ?

Blocking data uris

github-actions · 2026-02-26T19:27:17Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

jiexi · 2026-02-26T19:27:18Z

    !metadata.dapp.url ||
-    typeof metadata.dapp.url !== 'string'
+    typeof metadata.dapp.url !== 'string' ||
+    metadata.dapp.url.length > 2048


this seems long. thoughts?

seems like a reasonable limit to me!

jiexi · 2026-02-26T19:27:43Z

+    metadata.sdk.version.length > 256 || // this seems long still
    !metadata.sdk.platform ||
-    typeof metadata.sdk.platform !== 'string'
+    typeof metadata.sdk.platform !== 'string' ||
+    metadata.sdk.platform.length > 256 // this seems long still


thoughts on length?

I think we can lower this. Our expected values are something like JavaScript, React Native, iOS, Android right?

yeah i think so

i'll make Version and Platform both 64

jiexi · 2026-02-26T19:29:54Z

+    if (jsonString.length > 1024 * 1024) {
+      throw new Error('Decompressed payload too large (max 1MB).');
+    }
+


I believe @chakra-guy has a fix to make this use streaming decompression instead

adonesky1 · 2026-02-27T03:26:09Z

The two blocks connection requests with 'metamask' as origin/dapp name tests (lines 665 and 695 of connection-registry.test.ts) now pass for the wrong reason. With the new stricter isConnectionRequest validation, url: 'metamask' fails the new URL() check before the INTERNAL_ORIGINS guard is ever reached. The assertions still pass — but they're no longer exercising the origin-blocking logic. Worth updating these tests to use a valid-URL internal origin so they actually test what they claim.

EDIT => addressed here a65d13e

…king logic The URL test used `metamask` which now fails isConnectionRequests new URL() validation before reaching the INTERNAL_ORIGINS guard. The name test had no assertions and passed vacuously. Mock INTERNAL_ORIGINS with a valid URL so the URL test reaches the origin check, and add assertions to the name test.

…l to http(s) (#26833) ## **Description** The `new URL(metadata.dapp.url)` validation added in the parent branch ensures `dapp.url` is always a parseable URL. However, the production `INTERNAL_ORIGINS` array contains plain strings (`'metamask'`, `'MetaMask Mobile'`, `process.env.MM_FOX_CODE`), so the existing `INTERNAL_ORIGINS.includes(dapp.url)` check became dead code — no validated URL can equal those strings. The test masked this by mocking `INTERNAL_ORIGINS` with `'https://internal.metamask.io'`, a value absent from the production array. This PR: 1. **Restricts `dapp.url` to `http:`/`https:` schemes** in `isConnectionRequest()` — this is the primary security boundary. It prevents a malicious dApp from sending `metamask://...` or other non-web schemes as its URL, which could match internal origin strings. 2. **Retains the original `INTERNAL_ORIGINS.includes()` check as documented defense-in-depth** — currently redundant but will activate if the upstream URL validation is ever relaxed. 3. **Adds cross-referencing security comments** in both locations so future developers understand the invariant linking them. 4. **Removes the `INTERNAL_ORIGINS` mock** from the test file so tests exercise the real production values instead of a synthetic URL that hid the dead code. ## **Changelog** CHANGELOG entry: null ## **Related issues** Refs: parent branch `jl/make-mmc-parsing-more-robust` ## **Manual testing steps** ```gherkin Feature: SDKConnectV2 internal origin blocking Scenario: dApp with non-http scheme is rejected at validation Given a dApp connection request with url "metamask://evil.com" When isConnectionRequest validates the request Then it returns false Scenario: dApp with internal origin name is blocked Given a dApp connection request with dapp.name "metamask" When ConnectionRegistry handles the deeplink Then it throws "External transactions cannot use internal origins" ``` ## **Screenshots/Recordings** N/A — no UI changes. ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/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.

… into jl/make-mmc-parsing-more-robust

… organization This commit refactors the test file by moving the mock connection request, connection info, and valid deeplink definitions into the `beforeEach` block. This change enhances the organization of the test setup, ensuring that the mock data is reset before each test case, improving test reliability and readability.

… into jl/make-mmc-parsing-more-robust

This reverts commit 5c202e2.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

github-actions · 2026-03-02T22:46:43Z

🔍 Smart E2E Test Selection

Selected E2E tags: SmokeNetworkExpansion, SmokeMultiChainAPI, SmokeNetworkAbstractions
Selected Performance tags: None (no tests recommended)
Risk Level: high
AI Confidence: 85%

click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are security-focused hardening of the SDKConnectV2 connection request validation. Key changes include:

connection-request.ts: Added stricter input validation including:
- Public key validation (must decode to exactly 33 bytes for compressed secp256k1)
- Channel format validation (must match handshake: pattern)
- Mode validation (must be 'trusted' or 'untrusted')
- Expiration validation (must be in the future)
- Length limits on various fields
- Critical security fix: URL protocol validation ensuring dapp.url must be http:// or https:// to prevent internal origin spoofing
connection-registry.ts: Added 1MB payload size limit after decompression to prevent DoS attacks

These changes affect the SDKConnectV2 module which is the entry point for:

QR code scanning for dApp connections (QRScanner/utils.ts)
Deep link handling for SDK connections (DeeplinkManager/handlers/legacy/handleDeeplink.ts)

The selected tags cover dApp connection flows:

SmokeNetworkExpansion: Tests dApp connect/disconnect flows, multi-chain provider, Solana Wallet Standard compliance
SmokeMultiChainAPI: Tests CAIP-25 multi-chain session API for dApp permission management
SmokeNetworkAbstractions: Tests network management and dApp chain permissions

Risk is HIGH because these are security-critical changes to the connection validation boundary that could potentially break legitimate dApp connections if validation is too strict, or leave security holes if validation is insufficient.

Performance Test Selection:
These changes are purely validation logic changes in the SDKConnectV2 connection request handling. They add stricter input validation (field length limits, format validation, URL protocol checks) and a payload size limit. These are synchronous validation checks that execute quickly and don't affect UI rendering, data loading, account lists, or any performance-sensitive user flows. No performance tests are needed.

View GitHub Actions results

github-actions · 2026-03-02T23:01:37Z

⚠️ E2E Fixture Validation — Structural changes detected

Category	Count
New keys	60
Missing keys	0
Type mismatches	0
Value mismatches	7 (informational)

The committed fixture schema is out of date. To update, comment:

@metamaskbot update-mobile-fixture

View full details | Download diff report

WIP test

4facd51

jiexi requested a review from a team as a code owner February 26, 2026 19:26

metamaskbot added the team-wallet-integrations Wallet Integrations team label Feb 26, 2026

jiexi commented Feb 26, 2026

View reviewed changes

github-actions Bot added the size-M label Feb 26, 2026

Merge branch 'main' into jl/make-mmc-parsing-more-robust

e93e89f

jiexi commented Feb 26, 2026

View reviewed changes

cursor Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread app/core/SDKConnectV2/types/connection-request.ts

Comment thread app/core/SDKConnectV2/services/connection-registry.test.ts

Comment thread app/core/SDKConnectV2/services/connection-registry.test.ts

jiexi added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label Feb 26, 2026

fix fixtures

2232e3b

jiexi and others added 5 commits February 27, 2026 10:49

lint

f3a35b2

lint

3bf8535

fix lint

f526e21

fix test

2c35589

cursor Bot reviewed Feb 27, 2026

View reviewed changes

Comment thread app/core/SDKConnectV2/types/connection-request.ts

Comment thread app/core/SDKConnectV2/services/connection-registry.test.ts Outdated

adonesky1 and others added 5 commits March 2, 2026 15:14

Merge branch 'main' into jl/make-mmc-parsing-more-robust

f6818f8

Merge remote-tracking branch 'origin/jl/make-mmc-parsing-more-robust'…

432c81e

… into jl/make-mmc-parsing-more-robust

limit streaming decompression to 1MB

5c202e2

github-actions Bot added size-L and removed size-M labels Mar 2, 2026

jiexi added 2 commits March 2, 2026 13:29

Merge branch 'main' into jl/make-mmc-parsing-more-robust

e8396a5

Merge remote-tracking branch 'origin/jl/make-mmc-parsing-more-robust'…

362062c

… into jl/make-mmc-parsing-more-robust

cursor Bot reviewed Mar 2, 2026

View reviewed changes

Comment thread app/core/SDKConnectV2/types/connection-request.ts

jiexi added 2 commits March 2, 2026 13:48

Revert "limit streaming decompression to 1MB"

9201dae

This reverts commit 5c202e2.

make sdk.version and sdk.platform max 64 chars

bbc7bfa

cursor Bot reviewed Mar 2, 2026

View reviewed changes

Comment thread app/core/SDKConnectV2/types/connection-request.ts Outdated

make dapp.url and dapp.icon max length 1024

c2fbfb7

adonesky1 approved these changes Mar 2, 2026

View reviewed changes

jiexi enabled auto-merge March 2, 2026 22:24

Merge branch 'main' into jl/make-mmc-parsing-more-robust

c63abfe

jiexi added this pull request to the merge queue Mar 2, 2026

Merged via the queue into main with commit ca66895 Mar 2, 2026
90 checks passed

jiexi deleted the jl/make-mmc-parsing-more-robust branch March 2, 2026 23:40

github-actions Bot locked and limited conversation to collaborators Mar 2, 2026

metamaskbot added the release-7.69.0 Issue or pull request that will be included in release 7.69.0 label Mar 2, 2026

Uh oh!

Conversation

jiexi commented Feb 26, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 26, 2026

Uh oh!

jiexi Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adonesky1 commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Mar 2, 2026

🔍 Smart E2E Test Selection

Uh oh!

github-actions Bot commented Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jiexi commented Feb 26, 2026 •

edited by cursor Bot

Loading

jiexi Feb 26, 2026 •

edited

Loading

adonesky1 commented Feb 27, 2026 •

edited

Loading