Skip to content

Commit be2b91c

Browse files
authored
fix: redact SDK related URLs (#26690)
<!-- 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** <!-- 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? --> - Redact sensitive data from SDKConnectV2 / MWP debug and error logs - Add `redactUrl` utility to strip query/fragment params from deeplink URLs before logging - Reduce message payload logging to method + id metadata only ## **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: ## **Related issues** Fixes: [WAPI-1117](https://consensyssoftware.atlassian.net/browse/WAPI-1117) ## **Manual testing steps** ```gherkin 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** <!-- 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 Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-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. [WAPI-1117]: https://consensyssoftware.atlassian.net/browse/WAPI-1117?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: changes are limited to logging/error messages in SDKConnectV2, with minimal behavioral impact aside from altered log output and one test expectation. > > **Overview** > Prevents sensitive SDKConnectV2/MWP connection parameters from being written to logs by introducing `redactUrl()` and using it when throwing/logging deeplink handling errors. > > Reduces verbosity of connection message logging by logging only JSON-RPC `method` and `id` (instead of full payloads), and adjusts connect-deeplink success logging to avoid dumping full `ConnectionInfo`. Updates the `handleMwpDeeplink` non-string URL test to expect the new redacted/invalid URL message. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit dd3a3d0. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 2cd2fd7 commit be2b91c

4 files changed

Lines changed: 36 additions & 8 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ describe('ConnectionRegistry', () => {
249249

250250
// @ts-expect-error test non-string input
251251
await expect(registry.handleMwpDeeplink(null)).rejects.toThrow(
252-
'Invalid MWP deeplink: null',
252+
'Invalid MWP deeplink: [invalid URL]',
253253
);
254254
});
255255
});

app/core/SDKConnectV2/services/connection-registry.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { IConnectionStore } from '../types/connection-store';
1111
import { IHostApplicationAdapter } from '../types/host-application-adapter';
1212
import { Connection } from './connection';
1313
import { ConnectionInfo } from '../types/connection-info';
14-
import logger from './logger';
14+
import logger, { redactUrl } from './logger';
1515
import { ACTIONS, PREFIXES } from '../../../constants/deeplinks';
1616
import { decompressPayloadB64 } from '../utils/compression-utils';
1717
import { whenStoreReady } from '../utils/when-store-ready';
@@ -89,7 +89,7 @@ export class ConnectionRegistry {
8989

9090
public async handleMwpDeeplink(url: string): Promise<void> {
9191
if (!this.isMwpDeeplink(url)) {
92-
throw new Error(`Invalid MWP deeplink: ${url}`);
92+
throw new Error(`Invalid MWP deeplink: ${redactUrl(url)}`);
9393
}
9494

9595
try {
@@ -164,7 +164,7 @@ export class ConnectionRegistry {
164164
if (this.deeplinks.has(url)) return;
165165
this.deeplinks.add(url);
166166

167-
logger.debug('Handling connect deeplink:', url);
167+
logger.debug('Handling connect deeplink:', redactUrl(url));
168168

169169
let conn: Connection | undefined;
170170
let connInfo: ConnectionInfo | undefined;
@@ -194,9 +194,9 @@ export class ConnectionRegistry {
194194
this.connections.set(conn.id, conn);
195195
await this.store.save(connInfo);
196196
this.hostapp.syncConnectionList(Array.from(this.connections.values()));
197-
logger.debug('Handled connect deeplink.', connInfo);
197+
logger.debug('Handled connect deeplink.', connInfo?.id);
198198
} catch (error) {
199-
logger.error('Failed to handle connect deeplink:', error, url);
199+
logger.error('Failed to handle connect deeplink:', error, redactUrl(url));
200200
this.hostapp.showConnectionError();
201201
if (conn) await this.disconnect(conn.id);
202202
} finally {

app/core/SDKConnectV2/services/connection.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,14 @@ export class Connection {
3939
this.bridge = new RPCBridgeAdapter(this.info);
4040

4141
this.client.on('message', async (payload) => {
42-
logger.debug('Received message:', this.id, payload);
42+
const data =
43+
payload && typeof payload === 'object' && 'data' in payload
44+
? (payload.data as Record<string, unknown>)
45+
: undefined;
46+
logger.debug('Received message:', this.id, {
47+
method: data?.method,
48+
id: data?.id,
49+
});
4350

4451
const isWalletCreateSessionRequest =
4552
payload &&
@@ -76,7 +83,14 @@ export class Connection {
7683
});
7784

7885
this.bridge.on('response', (payload) => {
79-
logger.debug('Sending message:', this.id, payload);
86+
const responseData =
87+
'data' in payload
88+
? (payload.data as Record<string, unknown>)
89+
: (payload as Record<string, unknown>);
90+
logger.debug('Sending message:', this.id, {
91+
method: responseData?.method,
92+
id: responseData?.id,
93+
});
8094

8195
// If the payload includes an id, its a JSON-RPC response, otherwise its a notification
8296
if ('data' in payload && 'id' in payload.data) {

app/core/SDKConnectV2/services/logger.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,20 @@ const prettify = (...args: unknown[]) =>
55
.map((arg) => (typeof arg === 'object' ? JSON.stringify(arg) : String(arg)))
66
.join(' ');
77

8+
/**
9+
* Redacts query and fragment parameters from a URL to prevent
10+
* leaking sensitive connection parameters (channel ID, public key)
11+
* in logs.
12+
*/
13+
export const redactUrl = (url: string): string => {
14+
try {
15+
const parsed = new URL(url);
16+
return `${parsed.protocol}//${parsed.host}${parsed.pathname}?[REDACTED]`;
17+
} catch {
18+
return '[invalid URL]';
19+
}
20+
};
21+
822
export default {
923
debug: (...args: unknown[]) => {
1024
if (process.env.SDK_CONNECT_V2_DEBUG === 'true') {

0 commit comments

Comments
 (0)