Skip to content

fix: handle non-url origins - #623

Open
0xEdouardEth wants to merge 4 commits into
mainfrom
fix/validate-origin-url
Open

fix: handle non-url origins#623
0xEdouardEth wants to merge 4 commits into
mainfrom
fix/validate-origin-url

Conversation

@0xEdouardEth

@0xEdouardEth 0xEdouardEth commented Jun 25, 2026

Copy link
Copy Markdown

Explanation

This PR updates the Solana Snap to handle request origins that are not valid URLs, such as the stable wallet-connect origin used by MetaMask Mobile WalletConnect multichain requests.

The Snap still uses valid URL origins for dapp display and Blockaid wallet metadata. Non-URL origins are now treated as in-app metadata at the Security Alerts API boundary, so values such as wallet-connect or internal MetaMask origins are not sent as dapp domains. Unknown invalid origins are hidden from confirmation UI instead of showing meaningless identifiers.

Solana sign-in requests still require a real URL origin, because the sign-in flow relies on the origin as a domain. If the origin is not a valid URL, the sign-in confirmation is rejected instead of building a misleading sign-in message.

Tests were added/updated for origin parsing, Security Alerts API metadata, transaction scan forwarding, confirmation rendering, and background refresh behavior.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
    • N/A - no package changelog update needed for this internal behavior fix.
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them
    • N/A - this does not introduce a breaking API change.

Validation

yarn eslint <changed files>

Result: passed.

ENVIRONMENT=test ... yarn workspace @metamask/solana-wallet-snap jest \
  packages/snap/src/core/clients/security-alerts-api/SecurityAlertsApiClient.test.ts \
  packages/snap/src/core/services/transaction-scan/TransactionScan.test.ts \
  packages/snap/src/core/utils/parseOrigin.test.ts \
  --runInBand --coverage=false

Result: 3 test suites passed, 32 tests passed.

Note: tests that install the local Snap require a matching generated bundle/manifest shasum. I did not commit generated bundle/manifest artifacts in this PR.


Note

Medium Risk
Touches confirmation and security-scan origin handling on signing paths; sign-in now hard-fails for WalletConnect/MetaMask origins, which is intentional but behavior-changing for those flows.

Overview
Adds first-class support for non-URL request origins such as wallet-connect and internal metamask, aligned with MetaMask Mobile WalletConnect multichain flows.

Origin parsing is centralized via KNOWN_ORIGIN_LABELS (case-insensitive labels for MetaMask and WalletConnect). parseOrigin now only accepts known origins or http(s) URLs; other schemes and malformed values throw clearer errors. New isKnownOrigin helper supports UI and flow guards.

Security scan still maps internal metamask to METAMASK_ORIGIN_URL for Blockaid; wallet-connect is forwarded unchanged to the Security Alerts API (new tests).

Sign-in confirmations reject known non-URL origins because sign-in needs a real dapp domain—WalletConnect/MetaMask metadata origins fail fast instead of building a misleading message.

Skipped Jest cases get jest/no-disabled-tests eslint comments only.

Reviewed by Cursor Bugbot for commit 2e14da0. Bugbot is set up for automated code reviews on this repo. Configure here.

@0xEdouardEth
0xEdouardEth force-pushed the fix/validate-origin-url branch 2 times, most recently from f742c84 to 42b2163 Compare June 26, 2026 12:59
@0xEdouardEth
0xEdouardEth force-pushed the fix/validate-origin-url branch from 42b2163 to dfb3939 Compare June 26, 2026 13:45
@0xEdouardEth
0xEdouardEth marked this pull request as ready for review June 26, 2026 13:45
@0xEdouardEth
0xEdouardEth requested a review from a team as a code owner June 26, 2026 13:45
Comment thread packages/snap/src/core/services/transaction-scan/TransactionScan.ts
@0xEdouardEth
0xEdouardEth force-pushed the fix/validate-origin-url branch 2 times, most recently from 7d7ad1d to 1cb7163 Compare June 26, 2026 13:54
@0xEdouardEth
0xEdouardEth force-pushed the fix/validate-origin-url branch from 1cb7163 to 03982a5 Compare June 26, 2026 13:56

it('returns false for other origins', () => {
expect(isKnownOrigin('https://example.com')).toBe(false);
expect(isKnownOrigin('metamask')).toBe(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metamask is the known origin according to isKnownOrigin, isn't it ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes correct!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in this commit: d63f072

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Solana Snap’s origin handling so that certain stable non-URL origins (e.g., wallet-connect) are treated as known in-app origins rather than being parsed as URLs, and ensures sign-in flows reject known non-URL origins.

Changes:

  • Centralizes known-origin display handling via KNOWN_ORIGIN_LABELS and updates parseOrigin to only accept http(s) URLs or known-origin tokens.
  • Adds isKnownOrigin and uses it to hard-reject sign-in requests that come from known non-URL origins.
  • Updates/extends unit tests around origin parsing and transaction scan origin forwarding.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/snap/src/features/confirmation/views/ConfirmSignIn/render.tsx Rejects known non-URL origins for sign-in confirmations.
packages/snap/src/core/utils/parseOrigin.ts Adds known-origin mapping, restricts URL parsing to http(s), and exports isKnownOrigin.
packages/snap/src/core/utils/parseOrigin.test.ts Expands tests for known origins, invalid origins, and isKnownOrigin.
packages/snap/src/core/services/transaction-scan/TransactionScan.ts Clarifies (via comment) what origin shapes may be forwarded to the scan client.
packages/snap/src/core/services/transaction-scan/TransactionScan.test.ts Adds coverage for MetaMask-origin mapping and WalletConnect origin forwarding.
packages/snap/src/core/constants/solana.ts Introduces WALLET_CONNECT_ORIGIN and KNOWN_ORIGIN_LABELS.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +4
import {
KNOWN_ORIGIN_LABELS,
WALLET_CONNECT_ORIGIN,
} from '../constants/solana';
Comment on lines +132 to +142
describe('isKnownOrigin', () => {
it('returns true for the WalletConnect origin', () => {
expect(isKnownOrigin('wallet-connect')).toBe(true);
});

it('returns false for other origins', () => {
expect(isKnownOrigin('https://example.com')).toBe(false);
expect(isKnownOrigin('metamask')).toBe(false);
expect(isKnownOrigin(undefined)).toBe(false);
});
});
options?: string[];
account?: SolanaKeyringAccount;
}): Promise<TransactionScanResult | null> {
// The origin could be METAMASK_ORIGIN_URL, WALLET_CONNECT_ORIGIN or any valid URL.
@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants