From 1051f52923f2052d3325694af690339c7bb6ba02 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Fri, 16 Aug 2024 09:54:03 -0600 Subject: [PATCH 1/4] adds scrubbing of G addresses from API call logs in Sentry --- .../popup/components/ErrorTracking/index.tsx | 22 +++++++++++++++++++ extension/src/popup/helpers/formatters.ts | 6 +++++ 2 files changed, 28 insertions(+) diff --git a/extension/src/popup/components/ErrorTracking/index.tsx b/extension/src/popup/components/ErrorTracking/index.tsx index dcd8ccea58..a9cc99fda4 100644 --- a/extension/src/popup/components/ErrorTracking/index.tsx +++ b/extension/src/popup/components/ErrorTracking/index.tsx @@ -4,7 +4,9 @@ import { Integrations } from "@sentry/tracing"; import { SENTRY_KEY } from "constants/env"; import { settingsDataSharingSelector } from "popup/ducks/settings"; +import { scrubPathGkey } from "popup/helpers/formatters"; import packageJson from "../../../../package.json"; +import { INDEXER_URL } from "@shared/constants/mercury"; export const ErrorTracking = () => { const isDataSharingAllowed = useSelector(settingsDataSharingSelector); @@ -19,6 +21,26 @@ export const ErrorTracking = () => { // Amplitude 4xx's on too many Posts, which is expected behavior /api\.amplitude\.com\/2\/httpapi/i, ], + beforeSend(event) { + if (!event.request) { + return event; + } + + const url = event.request?.url; + if (url?.includes(`${INDEXER_URL}/account-history`)) { + const route = "account-history/"; + const scrubbedUrl = scrubPathGkey(route, url); + event.request.url = scrubbedUrl; + } + + if (url?.includes(`${INDEXER_URL}/account-balances`)) { + const route = "account-balances/"; + const scrubbedUrl = scrubPathGkey(route, url); + event.request.url = scrubbedUrl; + } + + return event; + }, }); } diff --git a/extension/src/popup/helpers/formatters.ts b/extension/src/popup/helpers/formatters.ts index 150f4e9d39..93c2365a83 100644 --- a/extension/src/popup/helpers/formatters.ts +++ b/extension/src/popup/helpers/formatters.ts @@ -98,3 +98,9 @@ export const formatAmount = (val: string) => { export const formattedBuffer = (data: Buffer) => truncatedPublicKey(Buffer.from(data).toString("hex").toUpperCase()); + +export const scrubPathGkey = (route: string, url: string) => { + const start = url.indexOf(route); + const end = url.indexOf("?"); + return url.substring(start + route.length) + "REDACTED" + url.substring(end); +}; From 104bbe4d7f6cbcad3cc9e0ca9478a77493eaf53e Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Mon, 19 Aug 2024 09:29:17 -0600 Subject: [PATCH 2/4] tweak sentry logs to only use contract identifiers --- @shared/api/internal.ts | 8 ++++++-- extension/src/background/helpers/account.ts | 12 ++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index acda590084..54d7926fbd 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -446,13 +446,17 @@ export const getAccountIndexerBalances = async ( const data = (await response.json()) as AccountBalancesInterface; if (!response.ok) { const _err = JSON.stringify(data); - captureException(`Failed to fetch account balances - ${_err}`); + captureException( + `Failed to fetch account balances - ${response.status}: ${response.statusText}`, + ); throw new Error(_err); } if ("error" in data && (data?.error?.horizon || data?.error?.soroban)) { const _err = JSON.stringify(data.error); - captureException(`Failed to fetch account balances - ${_err}`); + captureException( + `Failed to fetch account balances - ${response.status}: ${response.statusText}`, + ); } const formattedBalances = {} as NonNullable< diff --git a/extension/src/background/helpers/account.ts b/extension/src/background/helpers/account.ts index 383d33e8ed..158925f9fa 100644 --- a/extension/src/background/helpers/account.ts +++ b/extension/src/background/helpers/account.ts @@ -256,10 +256,6 @@ export const subscribeAccount = async (publicKey: string) => { } } catch (e) { console.error(e); - // Turn on when Mercury is enabled - // captureException( - // `Failed to subscribe account with Mercury - ${JSON.stringify(e)}`, - // ); } return { publicKey }; @@ -296,9 +292,7 @@ export const subscribeTokenBalance = async ( } } catch (e) { console.error(e); - captureException( - `Failed to subscribe token balance - ${JSON.stringify(e)}`, - ); + captureException(`Failed to subscribe token balance - ${contractId}`); } }; @@ -324,9 +318,7 @@ export const subscribeTokenHistory = async ( } } catch (e) { console.error(e); - captureException( - `Failed to subscribe token history - ${JSON.stringify(e)}`, - ); + captureException(`Failed to subscribe token history - ${contractId}`); } }; From 5924f689f85ddfa6d1dff3863f583bb465bcd246 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Mon, 19 Aug 2024 09:56:13 -0600 Subject: [PATCH 3/4] adds scrubPathGKey tests and tweaks scrubber --- .../helpers/__tests__/formatters.test.js | 26 ++++++++++++++++++- extension/src/popup/helpers/formatters.ts | 10 ++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/extension/src/popup/helpers/__tests__/formatters.test.js b/extension/src/popup/helpers/__tests__/formatters.test.js index 26ea3c6926..0df4ddfb8b 100644 --- a/extension/src/popup/helpers/__tests__/formatters.test.js +++ b/extension/src/popup/helpers/__tests__/formatters.test.js @@ -1,5 +1,5 @@ import BigNumber from "bignumber.js"; -import { formatAmount } from "../formatters"; +import { formatAmount, scrubPathGkey } from "../formatters"; import { formatTokenAmount } from "../soroban"; describe("formatAmount", () => { @@ -15,3 +15,27 @@ describe("formatAmount", () => { expect(formatTokenAmount(value, 7)).toBe(formatted); }); }); + +describe("scrubPathGkey", () => { + const ADDRESS = "GCBDC5AVPZEOSO3IAASQZSVRJMHX3UCCZH5O7S53FPZ636LQ5RHEW65H"; + it("should redact a G address from a URL", () => { + const route = "account-history/"; + const url = + "http://0.0.0.0:3002/api/v1/account-history/GCBDC5AVPZEOSO3IAASQZSVRJMHX3UCCZH5O7S53FPZ636LQ5RHEW65H?network=PUBLIC"; + const expected = + "http://0.0.0.0:3002/api/v1/account-history/REDACTED?network=PUBLIC"; + expect(scrubPathGkey(route, url)).toBe(expected); + }); + it("should redact a G address from a URL with no query params", () => { + const route = "account-history/"; + const url = + "http://0.0.0.0:3002/api/v1/account-history/GCBDC5AVPZEOSO3IAASQZSVRJMHX3UCCZH5O7S53FPZ636LQ5RHEW65H"; + const expected = "http://0.0.0.0:3002/api/v1/account-history/REDACTED"; + expect(scrubPathGkey(route, url)).toBe(expected); + }); + it("should return the URL in cases where it cannot redact", () => { + const route = "account-history/"; + const url = "not-even-a-url"; + expect(scrubPathGkey(route, url)).toBe(url); + }); +}); diff --git a/extension/src/popup/helpers/formatters.ts b/extension/src/popup/helpers/formatters.ts index 93c2365a83..f8dc03c481 100644 --- a/extension/src/popup/helpers/formatters.ts +++ b/extension/src/popup/helpers/formatters.ts @@ -100,7 +100,11 @@ export const formattedBuffer = (data: Buffer) => truncatedPublicKey(Buffer.from(data).toString("hex").toUpperCase()); export const scrubPathGkey = (route: string, url: string) => { - const start = url.indexOf(route); - const end = url.indexOf("?"); - return url.substring(start + route.length) + "REDACTED" + url.substring(end); + try { + const [base, slug] = url.split(route); + const end = slug.indexOf("?") === -1 ? slug.length : slug.indexOf("?"); + return `${base}${route}` + "REDACTED" + slug.substring(end); + } catch (error) { + return url; + } }; From c129cb998f0e00ebd439ea6d29b423183bae8865 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Mon, 19 Aug 2024 10:08:08 -0600 Subject: [PATCH 4/4] fixes lint warnings --- @shared/api/internal.ts | 1 - extension/src/popup/components/ErrorTracking/index.tsx | 4 +++- extension/src/popup/helpers/formatters.ts | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index 54d7926fbd..4bea9b8e39 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -453,7 +453,6 @@ export const getAccountIndexerBalances = async ( } if ("error" in data && (data?.error?.horizon || data?.error?.soroban)) { - const _err = JSON.stringify(data.error); captureException( `Failed to fetch account balances - ${response.status}: ${response.statusText}`, ); diff --git a/extension/src/popup/components/ErrorTracking/index.tsx b/extension/src/popup/components/ErrorTracking/index.tsx index a9cc99fda4..0a3ef31339 100644 --- a/extension/src/popup/components/ErrorTracking/index.tsx +++ b/extension/src/popup/components/ErrorTracking/index.tsx @@ -5,8 +5,8 @@ import { Integrations } from "@sentry/tracing"; import { SENTRY_KEY } from "constants/env"; import { settingsDataSharingSelector } from "popup/ducks/settings"; import { scrubPathGkey } from "popup/helpers/formatters"; -import packageJson from "../../../../package.json"; import { INDEXER_URL } from "@shared/constants/mercury"; +import packageJson from "../../../../package.json"; export const ErrorTracking = () => { const isDataSharingAllowed = useSelector(settingsDataSharingSelector); @@ -30,12 +30,14 @@ export const ErrorTracking = () => { if (url?.includes(`${INDEXER_URL}/account-history`)) { const route = "account-history/"; const scrubbedUrl = scrubPathGkey(route, url); + // eslint-disable-next-line no-param-reassign event.request.url = scrubbedUrl; } if (url?.includes(`${INDEXER_URL}/account-balances`)) { const route = "account-balances/"; const scrubbedUrl = scrubPathGkey(route, url); + // eslint-disable-next-line no-param-reassign event.request.url = scrubbedUrl; } diff --git a/extension/src/popup/helpers/formatters.ts b/extension/src/popup/helpers/formatters.ts index f8dc03c481..a8ad8521e5 100644 --- a/extension/src/popup/helpers/formatters.ts +++ b/extension/src/popup/helpers/formatters.ts @@ -103,7 +103,7 @@ export const scrubPathGkey = (route: string, url: string) => { try { const [base, slug] = url.split(route); const end = slug.indexOf("?") === -1 ? slug.length : slug.indexOf("?"); - return `${base}${route}` + "REDACTED" + slug.substring(end); + return `${base}${route}${"REDACTED"}${slug.substring(end)}`; } catch (error) { return url; }