From ea1fdadf707a9b3ebba21818f3b82bbf8e7723c9 Mon Sep 17 00:00:00 2001 From: rene Date: Wed, 19 Aug 2026 21:00:55 +0100 Subject: [PATCH 1/2] fix(sentry): stop console output from becoming breadcrumbs Why: - The SDK forwards console output as breadcrumbs by default, with the raw argument list attached. The app logs an instalment amount and a wallet balance when a recurring rule is skipped, per-wallet drift on import, a category name at seed time, and validation messages that interpolate the rejected value. All of it was reaching Sentry. What: - A named guard drops every console-category breadcrumb before it enters the scope, which also covers native crashes since native sync patches the same scope method. It is a blanket drop, not a content filter: deciding which log line carries money is a losing game, and the guard has to protect the logs nobody has written yet. Other categories pass through by reference. Values still reach the device system log, which no Sentry-side guard can address. --- app/__tests__/sentryBreadcrumbGuard.test.ts | 190 ++++++++++++++++++ app/_layout.tsx | 4 + .../observability/sentryBreadcrumbGuard.ts | 55 +++++ 3 files changed, 249 insertions(+) create mode 100644 app/__tests__/sentryBreadcrumbGuard.test.ts create mode 100644 src/core/observability/sentryBreadcrumbGuard.ts diff --git a/app/__tests__/sentryBreadcrumbGuard.test.ts b/app/__tests__/sentryBreadcrumbGuard.test.ts new file mode 100644 index 0000000..8226f4e --- /dev/null +++ b/app/__tests__/sentryBreadcrumbGuard.test.ts @@ -0,0 +1,190 @@ +/** + * Sentry Breadcrumb Guard Tests + * + * DoD: + * - A breadcrumb of category 'console' is dropped (the hook returns null), for + * every console level, including when the payload carries financial values. + * - Breadcrumbs of every other category pass through by reference, unchanged. + * - The guard is actually INSTALLED in the app's Sentry.init - asserted on the + * options object handed to Sentry.init, not by calling the guard in + * isolation. A filter that is never installed is the same as no filter. + */ + +import type { Breadcrumb } from '@sentry/react-native'; + +// jest hoists jest.mock above imports; only vars prefixed `mock` may be +// referenced inside the factories. +const mockRunMigrations = jest.fn(); +const mockInitializeSeedData = jest.fn(); +const mockAssertStoreReadable = jest.fn(); +const mockProcessRecurringRules = jest.fn(); +const mockResetCorruptedStore = jest.fn(); +const mockLoadSettings = jest.fn(); + +// Sentry itself is mocked so that `init` records the options it was called +// with. Everything else below only exists so that importing ../_layout - which +// is what makes Sentry.init run - does not drag in the real boot graph. +jest.mock('@sentry/react-native', () => ({ + init: jest.fn(), + wrap: (c: unknown) => c, + captureMessage: jest.fn(), + captureException: jest.fn(), +})); + +jest.mock('expo-router', () => { + const Stack: any = () => null; + // eslint-disable-next-line react/display-name -- inert stand-in, never rendered + Stack.Screen = () => null; + return { Stack, router: { back: jest.fn() }, unstable_settings: {} }; +}); + +jest.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 0, bottom: 0, left: 0, right: 0 }), +})); + +jest.mock('@/src/components/security/SecurityGate', () => ({ SecurityGate: () => null })); +jest.mock('@/src/core/security/SecurityContext', () => ({ SecurityProvider: () => null })); +jest.mock('@/src/features/onboarding/screens/OnboardingScreen', () => ({ + OnboardingScreen: () => null, +})); + +jest.mock('@/src/data/migrations', () => ({ runMigrations: () => mockRunMigrations() })); +jest.mock('@/src/data/seed', () => ({ initializeSeedData: () => mockInitializeSeedData() })); +jest.mock('@/src/data/storage/sql/database', () => ({ + assertStoreReadable: () => mockAssertStoreReadable(), +})); +jest.mock('@/src/data/services/RecurringTransactionEngine', () => ({ + processRecurringRules: () => mockProcessRecurringRules(), +})); +jest.mock('@/src/data/services/storeRecoveryService', () => ({ + resetCorruptedStore: () => mockResetCorruptedStore(), +})); +jest.mock('@/src/data/services/settingsService', () => ({ + loadSettings: () => mockLoadSettings(), +})); + +jest.mock('@/src/domain/useCases', () => ({ + verifyFinancialIntegrity: jest.fn().mockResolvedValue(true), +})); + +jest.mock('@/src/core/di/container', () => ({ + container: { + recurringTransactionRepository: {}, + transactionRepository: {}, + walletRepository: {}, + }, + getUseCaseDeps: () => ({ runInTransaction: (fn: () => unknown) => fn() }), +})); + +jest.mock('@/src/core/events/dataEvents', () => ({ + dataEvents: { emit: jest.fn(), emitMultiple: jest.fn() }, +})); + +// NOT mocked: the guard itself. Test 3 compares the installed hook against this +// exact reference, so it must be the real implementation. +import { dropConsoleBreadcrumbs } from '@/src/core/observability/sentryBreadcrumbGuard'; +import * as Sentry from '@sentry/react-native'; + +// Importing the root layout is what runs Sentry.init, at module scope. +import '../_layout'; + +// Captured at module scope on purpose: Sentry.init fires exactly once, during +// the import above, so a jest.clearAllMocks() in a beforeEach would erase the +// evidence before any test could read it. +const initOptions = (Sentry.init as jest.Mock).mock.calls[0][0]; + +describe('dropConsoleBreadcrumbs', () => { + it("drops a 'console' breadcrumb carrying financial values", () => { + const breadcrumb: Breadcrumb = { + category: 'console', + level: 'error', + message: '[WalletRepository] Validation failed: balance 1234567 out of range', + data: { + logger: 'console', + arguments: ['[WalletRepository] Validation failed: balance 1234567 out of range'], + }, + }; + + expect(dropConsoleBreadcrumbs(breadcrumb)).toBeNull(); + }); + + it("drops 'console' breadcrumbs at every level - the category is what matters", () => { + // Sentry tags console breadcrumbs by category, not by level, so log, + // warn and error all arrive as category 'console'. + for (const level of ['log', 'debug', 'info', 'warning', 'error'] as const) { + expect(dropConsoleBreadcrumbs({ category: 'console', level })).toBeNull(); + } + }); + + it('drops the balance-drift log from the v5 import verbatim', () => { + const message = + '[import] Balance drift after v5 import - opening_balance derivation is suspect: ' + + 'w-1 (stored 500000, ledger 499750, drift 250)'; + + expect( + dropConsoleBreadcrumbs({ + category: 'console', + level: 'warning', + message, + data: { logger: 'console', arguments: [message] }, + }), + ).toBeNull(); + }); + + it('passes navigation breadcrumbs through unchanged, by reference', () => { + const breadcrumb: Breadcrumb = { + category: 'navigation', + data: { from: '/(tabs)', to: '/transaction/42' }, + }; + + expect(dropConsoleBreadcrumbs(breadcrumb)).toBe(breadcrumb); + }); + + it('passes the other useful categories through unchanged, by reference', () => { + // These are what made the earlier crash-report probe readable; the guard + // must not touch them. + const categories = ['ui.click', 'http', 'device.event', 'sentry.event', 'sentry.transaction']; + + for (const category of categories) { + const breadcrumb: Breadcrumb = { category, message: 'kept' }; + expect(dropConsoleBreadcrumbs(breadcrumb)).toBe(breadcrumb); + } + }); + + it('passes an unknown category through - the default is allow, not deny', () => { + // Proves the guard is a single deny rule rather than an allowlist, so a + // category introduced by a future SDK version is not silently lost. + const breadcrumb: Breadcrumb = { category: 'some.future.category' }; + expect(dropConsoleBreadcrumbs(breadcrumb)).toBe(breadcrumb); + + const uncategorised: Breadcrumb = { message: 'no category at all' }; + expect(dropConsoleBreadcrumbs(uncategorised)).toBe(uncategorised); + }); +}); + +describe('the guard is installed in the app Sentry.init', () => { + it('calls Sentry.init exactly once, at module scope', () => { + expect(Sentry.init as jest.Mock).toHaveBeenCalledTimes(1); + }); + + it('passes dropConsoleBreadcrumbs as beforeBreadcrumb', () => { + // Identity, not shape: proves the hook that actually runs is the one + // the tests above exercise, and not a lookalike defined elsewhere. + expect(initOptions.beforeBreadcrumb).toBe(dropConsoleBreadcrumbs); + }); + + it('the installed hook drops console and keeps navigation', () => { + // Behaviour of the wired hook, independent of the identity assertion: + // if the guard is ever re-wrapped, this still holds it to its contract. + expect(initOptions.beforeBreadcrumb({ category: 'console', message: 'balance 999' })).toBeNull(); + + const nav: Breadcrumb = { category: 'navigation' }; + expect(initOptions.beforeBreadcrumb(nav)).toBe(nav); + }); + + it('leaves the rest of the init options intact', () => { + expect(typeof initOptions.dsn).toBe('string'); + expect(initOptions.dsn.length).toBeGreaterThan(0); + expect(initOptions.debug).toBe(false); + }); +}); diff --git a/app/_layout.tsx b/app/_layout.tsx index 28d56ac..04f9f11 100644 --- a/app/_layout.tsx +++ b/app/_layout.tsx @@ -2,6 +2,7 @@ import { SecurityGate } from "@/src/components/security/SecurityGate"; import { container, getUseCaseDeps } from "@/src/core/di/container"; import { ErrorBoundary } from "@/src/core/error/ErrorBoundary"; import { dataEvents } from "@/src/core/events/dataEvents"; +import { dropConsoleBreadcrumbs } from "@/src/core/observability/sentryBreadcrumbGuard"; import { SecurityProvider } from "@/src/core/security/SecurityContext"; import { runMigrations } from "@/src/data/migrations"; import { initializeSeedData } from "@/src/data/seed"; @@ -41,6 +42,9 @@ Sentry.init({ process.env.EXPO_PUBLIC_SENTRY_DSN || "https://placeholder@sentry.io/placeholder", debug: false, + // Console output must never become a breadcrumb: this app logs wallet + // balances and validation messages. See the guard for the full rationale. + beforeBreadcrumb: dropConsoleBreadcrumbs, }); // Module scope so it runs exactly once, at import, before React mounts and before diff --git a/src/core/observability/sentryBreadcrumbGuard.ts b/src/core/observability/sentryBreadcrumbGuard.ts new file mode 100644 index 0000000..bcab240 --- /dev/null +++ b/src/core/observability/sentryBreadcrumbGuard.ts @@ -0,0 +1,55 @@ +/** + * Sentry Breadcrumb Guard + * + * Drops every breadcrumb whose category is 'console' before it can reach a + * Sentry event. Wired into the single Sentry.init in app/_layout.tsx. + * + * WHY THIS EXISTS - do not delete it as redundant, and do not narrow it. + * + * This app prints financial and user data to the console in several places: + * - src/data/migrations/v5_import_from_asyncstorage.ts:110 - per-wallet + * stored balance, ledger balance and drift, with the wallet id + * - src/data/services/RecurringTransactionEngine.ts:111 - instalment and + * rule detail; the surrounding error logs carry rule ids + * - src/data/seed/seedService.ts:78 - a category name + * - src/data/repositories/WalletRepository.ts:58 and + * src/data/repositories/RecurringTransactionRepository.ts:60 - the + * ValidationError message, which embeds the value that was rejected + * Roughly 60 further console calls in src/ and app/ are unguarded, and nothing + * stops the next one from being added. + * + * @sentry/react-native 7.2.0 turns every one of those calls into a breadcrumb + * BY DEFAULT. getDefaultIntegrations() always installs breadcrumbsIntegration(), + * whose React Native wrapper hard-defaults 'console: true' - it is not gated on + * platform, on __DEV__, or on anything else. The resulting breadcrumb carries + * the joined text in .message AND the raw argument list in .data.arguments, and + * scopeSync mirrors it into the native SDK's scope, so a native crash reports it + * too. So this is not defence against a future SDK change: without this guard, + * balances ship with the next captured event today. + * + * The drop is deliberately blanket. It is NOT a content filter: deciding which + * log lines happen to carry money is a losing game, and it would have to be + * re-decided every time somebody adds a console call. Categories other than + * 'console' - navigation, ui.click, http, device events, Sentry's own + * breadcrumbs - pass through untouched, because those are the ones that make a + * crash report worth reading. + * + * Note this guard has no effect on the device system log. Sentry's console + * instrumentation always calls the original console method; this filter runs + * strictly downstream of that, inside Sentry's own pipeline. Everything logged + * here is still readable via adb logcat / Console.app. That is a separate + * problem and is not addressed by anything in this file. + */ + +import type { Breadcrumb } from '@sentry/react-native'; + +/** + * Sentry beforeBreadcrumb hook. Returns null for console breadcrumbs, which + * discards them; returns every other breadcrumb unchanged. + */ +export function dropConsoleBreadcrumbs(breadcrumb: Breadcrumb): Breadcrumb | null { + if (breadcrumb.category === 'console') { + return null; + } + return breadcrumb; +} From 572a1a1b393120e6d65d8d1ec5e57fd4fc17a335 Mon Sep 17 00:00:00 2001 From: rene Date: Wed, 19 Aug 2026 23:42:11 +0100 Subject: [PATCH 2/2] fix(sentry): stop sending persistent identifiers and network breadcrumbs Why: - The SDK attached two persistent identifiers that no option can disable: the installation id on every event, and an iOS device hash derived from identifierForVendor that survives reinstall as long as any other app from the same vendor remains. Neither was declared, and neither helps debug a crash. Network breadcrumbs and release-health sessions carried the same identifier for metrics this project does not use. What: - A beforeSend guard strips the user object, the device hash and the root flag, written defensively because an exception there would be swallowed and the event would ship unstripped. XHR breadcrumbs are disabled through the resolver form so the rest of the default integration set is preserved by reference rather than re-enumerated. Session tracking is off. Culture context and touch breadcrumbs are deliberately kept: five locales and timezone-sensitive recurring dates make them diagnostic, and touch carries component names only. Crash grouping is unaffected; "users affected" and crash-free-users are not. --- app/__tests__/sentryBreadcrumbGuard.test.ts | 249 +++++++++++++++++- app/__tests__/storeRecoveryBoot.test.tsx | 3 + app/_layout.tsx | 16 +- .../observability/sentryBreadcrumbGuard.ts | 165 +++++++++++- 4 files changed, 420 insertions(+), 13 deletions(-) diff --git a/app/__tests__/sentryBreadcrumbGuard.test.ts b/app/__tests__/sentryBreadcrumbGuard.test.ts index 8226f4e..8edd55b 100644 --- a/app/__tests__/sentryBreadcrumbGuard.test.ts +++ b/app/__tests__/sentryBreadcrumbGuard.test.ts @@ -1,16 +1,28 @@ /** - * Sentry Breadcrumb Guard Tests + * Sentry Privacy Guard Tests * - * DoD: + * DoD - breadcrumb guard (V-34): * - A breadcrumb of category 'console' is dropped (the hook returns null), for * every console level, including when the payload carries financial values. * - Breadcrumbs of every other category pass through by reference, unchanged. - * - The guard is actually INSTALLED in the app's Sentry.init - asserted on the - * options object handed to Sentry.init, not by calling the guard in - * isolation. A filter that is never installed is the same as no filter. + * + * DoD - identifier guard (V-33): + * - An event carrying user, contexts.app.device_app_hash and contexts.os.rooted + * comes back with all three gone. + * - An event missing every one of them passes through without throwing. + * - Culture context and touch breadcrumbs survive, asserted BY REFERENCE, so a + * later "strip more" edit fails here instead of silently shipping. + * - XHR breadcrumbs are off while the rest of the default integration set is + * still there, asserted on the RESOLVED list, not on the option shape. + * - Auto session tracking is explicitly false. + * + * DoD - both: + * - The guards are actually INSTALLED in the app's Sentry.init - asserted on + * the options object handed to Sentry.init, not by calling them in + * isolation. A guard that is never installed is the same as no guard. */ -import type { Breadcrumb } from '@sentry/react-native'; +import type { Breadcrumb, Event } from '@sentry/react-native'; // jest hoists jest.mock above imports; only vars prefixed `mock` may be // referenced inside the factories. @@ -29,6 +41,13 @@ jest.mock('@sentry/react-native', () => ({ wrap: (c: unknown) => c, captureMessage: jest.fn(), captureException: jest.fn(), + // Stand-in for the real factory. It only has to report the integration's + // name and echo back the options it was constructed with - that is all the + // resolver reads, and all the integrations test asserts on. + breadcrumbsIntegration: jest.fn((options?: unknown) => ({ + name: 'Breadcrumbs', + options, + })), })); jest.mock('expo-router', () => { @@ -80,9 +99,14 @@ jest.mock('@/src/core/events/dataEvents', () => ({ dataEvents: { emit: jest.fn(), emitMultiple: jest.fn() }, })); -// NOT mocked: the guard itself. Test 3 compares the installed hook against this -// exact reference, so it must be the real implementation. -import { dropConsoleBreadcrumbs } from '@/src/core/observability/sentryBreadcrumbGuard'; +// NOT mocked: the guards themselves. The wiring tests compare the installed +// hooks against these exact references, so they must be the real +// implementations. +import { + dropConsoleBreadcrumbs, + stripPersistentIdentifiers, + withoutXhrBreadcrumbs, +} from '@/src/core/observability/sentryBreadcrumbGuard'; import * as Sentry from '@sentry/react-native'; // Importing the root layout is what runs Sentry.init, at module scope. @@ -188,3 +212,210 @@ describe('the guard is installed in the app Sentry.init', () => { expect(initOptions.debug).toBe(false); }); }); + +describe('stripPersistentIdentifiers', () => { + /** An event shaped like one the native layers actually produce on iOS. */ + function eventWithIdentifiers(): Event { + return { + event_id: 'abc123', + user: { id: '982C1F97-0000-4000-8000-000000000000' }, + contexts: { + app: { + app_version: '1.0.0', + app_identifier: 'com.renkakpo.valto.app', + device_app_hash: '5f2b0c1d9e8a7c6b5a4938271605f4e3d2c1b0a9', + }, + os: { name: 'iOS', version: '18.2', rooted: false }, + device: { model: 'iPhone15,2', memory_size: 6000000000 }, + culture: { locale: 'fr-FR', timezone: 'Africa/Lome' }, + }, + }; + } + + it('removes the user, device_app_hash and rooted in one pass', () => { + const stripped = stripPersistentIdentifiers(eventWithIdentifiers()); + + expect(stripped.user).toBeUndefined(); + expect('user' in stripped).toBe(false); + expect(stripped.contexts?.app).not.toHaveProperty('device_app_hash'); + expect(stripped.contexts?.os).not.toHaveProperty('rooted'); + }); + + it('returns the same event object - it never drops the report', () => { + // beforeSend returning null would discard the crash entirely. Stripping + // identifiers must never cost us the report they were attached to. + const event = eventWithIdentifiers(); + expect(stripPersistentIdentifiers(event)).toBe(event); + }); + + it('keeps the diagnostic remainder of the contexts it edits', () => { + // Proves the strip is surgical: it removes two named fields, not the + // app and os contexts that carry them. + const stripped = stripPersistentIdentifiers(eventWithIdentifiers()); + + expect(stripped.contexts?.app?.app_version).toBe('1.0.0'); + expect(stripped.contexts?.app?.app_identifier).toBe('com.renkakpo.valto.app'); + expect(stripped.contexts?.os?.name).toBe('iOS'); + expect(stripped.contexts?.os?.version).toBe('18.2'); + expect(stripped.contexts?.device?.model).toBe('iPhone15,2'); + }); + + it('KEEPS the culture context, asserted by reference', () => { + // Deliberate: five locales, plus currency- and recurring-date defects + // where the reporter's timezone was the deciding fact. If somebody + // "finishes the job" by stripping culture too, this fails. + const culture = { locale: 'fr-FR', timezone: 'Africa/Lome' }; + const event: Event = { contexts: { culture } }; + + expect(stripPersistentIdentifiers(event).contexts?.culture).toBe(culture); + }); + + it('KEEPS touch breadcrumbs, asserted by reference', () => { + // Touch breadcrumbs carry React component display names only - no + // sentry-label prop and no component-annotate babel plugin in this repo. + // They ride along with a crash and are diagnostic context, not analytics. + const breadcrumbs: Breadcrumb[] = [ + { category: 'touch', type: 'user', message: 'Touch event within element: AddButton' }, + ]; + const event: Event = { breadcrumbs }; + + const stripped = stripPersistentIdentifiers(event); + expect(stripped.breadcrumbs).toBe(breadcrumbs); + expect(stripped.breadcrumbs?.[0]).toBe(breadcrumbs[0]); + + // And the breadcrumb hook lets them through in the first place. + expect(dropConsoleBreadcrumbs(breadcrumbs[0])).toBe(breadcrumbs[0]); + }); + + it('is a no-op on an event with no contexts at all', () => { + // The defensive path. device_app_hash is iOS only, and a JS event + // captured before the native scope is read has no app context. A throw + // here happens inside Sentry's pipeline, which swallows it and sends + // the event UNSTRIPPED - the exact failure this guard exists to avoid. + const event: Event = { event_id: 'no-contexts' }; + + expect(() => stripPersistentIdentifiers(event)).not.toThrow(); + expect(stripPersistentIdentifiers(event)).toBe(event); + }); + + it('is a no-op on an Android-shaped event with contexts but no app or os', () => { + const event: Event = { contexts: { device: { model: 'Pixel 7' } } }; + + expect(() => stripPersistentIdentifiers(event)).not.toThrow(); + expect(stripPersistentIdentifiers(event).contexts?.device?.model).toBe('Pixel 7'); + }); + + it('is a no-op when app and os exist but carry none of the stripped fields', () => { + const event: Event = { + contexts: { app: { app_version: '1.0.0' }, os: { name: 'Android' } }, + }; + + expect(() => stripPersistentIdentifiers(event)).not.toThrow(); + expect(stripPersistentIdentifiers(event).contexts?.app?.app_version).toBe('1.0.0'); + expect(stripPersistentIdentifiers(event).contexts?.os?.name).toBe('Android'); + }); +}); + +describe('withoutXhrBreadcrumbs', () => { + // A stand-in for the SDK default list. Only the names matter: the resolver + // matches on name and passes everything else through untouched. + function defaultIntegrations() { + return [ + { name: 'ReactNativeErrorHandlers' }, + { name: 'InboundFilters' }, + { name: 'Breadcrumbs' }, + { name: 'Dedupe' }, + { name: 'DeviceContext' }, + { name: 'ExpoContext' }, + ] as unknown as Parameters[0]; + } + + it('replaces the breadcrumbs integration with one that has xhr off', () => { + const defaults = defaultIntegrations(); + const resolved = withoutXhrBreadcrumbs(defaults); + + const breadcrumbs = resolved.filter((i) => i.name === 'Breadcrumbs'); + expect(breadcrumbs).toHaveLength(1); + // The original default instance is gone, not merely shadowed. + expect(resolved).not.toContain(defaults[2]); + expect((breadcrumbs[0] as unknown as { options: unknown }).options).toEqual({ xhr: false }); + }); + + it('keeps every other default integration, by reference', () => { + // The point of deriving from the defaults instead of hand-writing the + // list: nothing can be lost here without this failing. + const defaults = defaultIntegrations(); + const resolved = withoutXhrBreadcrumbs(defaults); + + for (const integration of defaults) { + if (integration.name === 'Breadcrumbs') { + continue; + } + expect(resolved).toContain(integration); + } + + expect(resolved).toHaveLength(defaults.length); + expect(resolved.map((i) => i.name).sort()).toEqual(defaults.map((i) => i.name).sort()); + }); + + it('still swaps when the list has no breadcrumbs integration to replace', () => { + const resolved = withoutXhrBreadcrumbs([ + { name: 'Dedupe' }, + ] as unknown as Parameters[0]); + + expect(resolved.map((i) => i.name)).toEqual(['Dedupe', 'Breadcrumbs']); + }); +}); + +describe('the identifier guards are installed in the app Sentry.init', () => { + it('passes stripPersistentIdentifiers as beforeSend', () => { + // Identity, not shape. This is the assertion that matters: a guard + // that is defined but never installed is the same as no guard. + expect(initOptions.beforeSend).toBe(stripPersistentIdentifiers); + }); + + it('passes withoutXhrBreadcrumbs as the integrations resolver', () => { + expect(initOptions.integrations).toBe(withoutXhrBreadcrumbs); + }); + + it('the installed integrations resolver disables xhr and keeps the rest', () => { + // Asserted on the RESOLVED list produced by the wired resolver, not on + // the option shape, so a resolver that is wired but inert still fails. + const defaults = [ + { name: 'ReactNativeErrorHandlers' }, + { name: 'Breadcrumbs' }, + { name: 'DeviceContext' }, + ]; + const resolved = initOptions.integrations(defaults); + + expect(resolved).toContain(defaults[0]); + expect(resolved).toContain(defaults[2]); + expect(resolved).not.toContain(defaults[1]); + + const breadcrumbs = resolved.filter((i: { name: string }) => i.name === 'Breadcrumbs'); + expect(breadcrumbs).toHaveLength(1); + expect(breadcrumbs[0].options).toEqual({ xhr: false }); + }); + + it('the installed beforeSend strips all three fields', () => { + const stripped = initOptions.beforeSend({ + user: { id: 'installation-uuid' }, + contexts: { + app: { app_version: '1.0.0', device_app_hash: 'deadbeef' }, + os: { name: 'iOS', rooted: false }, + }, + }); + + expect(stripped.user).toBeUndefined(); + expect(stripped.contexts.app).not.toHaveProperty('device_app_hash'); + expect(stripped.contexts.os).not.toHaveProperty('rooted'); + expect(stripped.contexts.app.app_version).toBe('1.0.0'); + }); + + it('turns auto session tracking off explicitly', () => { + // Explicit false, not absent: both native layers default this to ON, so + // an unset option silently re-enables sessions keyed on the identifier + // beforeSend now strips. + expect(initOptions.enableAutoSessionTracking).toBe(false); + }); +}); diff --git a/app/__tests__/storeRecoveryBoot.test.tsx b/app/__tests__/storeRecoveryBoot.test.tsx index bf5e7ea..257cd11 100644 --- a/app/__tests__/storeRecoveryBoot.test.tsx +++ b/app/__tests__/storeRecoveryBoot.test.tsx @@ -29,6 +29,9 @@ jest.mock('@sentry/react-native', () => ({ wrap: (c: unknown) => c, captureMessage: jest.fn(), captureException: jest.fn(), + // Imported by the privacy guards module; never called here because init is + // a mock and only the real init resolves the integrations list. + breadcrumbsIntegration: jest.fn(() => ({ name: 'Breadcrumbs' })), })); jest.mock('expo-router', () => { diff --git a/app/_layout.tsx b/app/_layout.tsx index 04f9f11..490d215 100644 --- a/app/_layout.tsx +++ b/app/_layout.tsx @@ -2,7 +2,11 @@ import { SecurityGate } from "@/src/components/security/SecurityGate"; import { container, getUseCaseDeps } from "@/src/core/di/container"; import { ErrorBoundary } from "@/src/core/error/ErrorBoundary"; import { dataEvents } from "@/src/core/events/dataEvents"; -import { dropConsoleBreadcrumbs } from "@/src/core/observability/sentryBreadcrumbGuard"; +import { + dropConsoleBreadcrumbs, + stripPersistentIdentifiers, + withoutXhrBreadcrumbs, +} from "@/src/core/observability/sentryBreadcrumbGuard"; import { SecurityProvider } from "@/src/core/security/SecurityContext"; import { runMigrations } from "@/src/data/migrations"; import { initializeSeedData } from "@/src/data/seed"; @@ -45,6 +49,16 @@ Sentry.init({ // Console output must never become a breadcrumb: this app logs wallet // balances and validation messages. See the guard for the full rationale. beforeBreadcrumb: dropConsoleBreadcrumbs, + // The SDK attaches two persistent identifiers no option can disable, plus a + // jailbreak flag. beforeSend is the only place to remove them. + beforeSend: stripPersistentIdentifiers, + // Derives from the SDK default list - it does not replace it - to turn off + // XHR breadcrumbs only. + integrations: withoutXhrBreadcrumbs, + // Off explicitly, not by default: release health sessions are keyed on the + // install identifier that beforeSend now strips, so they measure nothing. + // Both native layers default this to ON, so leaving it unset re-enables it. + enableAutoSessionTracking: false, }); // Module scope so it runs exactly once, at import, before React mounts and before diff --git a/src/core/observability/sentryBreadcrumbGuard.ts b/src/core/observability/sentryBreadcrumbGuard.ts index bcab240..257f1a8 100644 --- a/src/core/observability/sentryBreadcrumbGuard.ts +++ b/src/core/observability/sentryBreadcrumbGuard.ts @@ -1,8 +1,23 @@ /** - * Sentry Breadcrumb Guard + * Sentry Privacy Guards + * + * Everything the app does to stop the Sentry SDK collecting data the project + * never declared. Three hooks, all wired into the single Sentry.init in + * app/_layout.tsx: + * + * dropConsoleBreadcrumbs - beforeBreadcrumb (registry V-34) + * stripPersistentIdentifiers - beforeSend (registry V-33) + * withoutXhrBreadcrumbs - integrations (registry V-33) + * + * The file is still named sentryBreadcrumbGuard.ts because the breadcrumb + * guard landed first; the name is historical, the scope is not. + * + * =========================================================================== + * 1. dropConsoleBreadcrumbs - beforeBreadcrumb + * =========================================================================== * * Drops every breadcrumb whose category is 'console' before it can reach a - * Sentry event. Wired into the single Sentry.init in app/_layout.tsx. + * Sentry event. * * WHY THIS EXISTS - do not delete it as redundant, and do not narrow it. * @@ -39,9 +54,94 @@ * strictly downstream of that, inside Sentry's own pipeline. Everything logged * here is still readable via adb logcat / Console.app. That is a separate * problem and is not addressed by anything in this file. + * + * =========================================================================== + * 2. stripPersistentIdentifiers - beforeSend + * =========================================================================== + * + * The SDK attaches TWO persistent identifiers that nothing in this project + * asked for, and NO SentryOptions flag turns either of them off. beforeSend is + * the only lever. + * + * event.user + * The per-install identifier. Both native layers inject it with the same + * rule - "if there is no user id, make one": + * - iOS: RNSentry.mm fetchNativeDeviceContexts sets + * user = { id: PrivateSentrySDKOnly.installationID }, and + * SentryClient.m setUserIdIfNoUserSet does the same for + * native events. The value is a UUID persisted in a file + * named INSTALLATION under Library/Caches. + * - Android: InternalSentrySdk.serializeScope sets + * User.setId(Installation.id(context)), a UUID persisted in + * a file named INSTALLATION under getFilesDir(). + * It survives launches and app updates, which is exactly what makes it a + * persistent identifier in the Play Data Safety sense. The app never calls + * Sentry.setUser, so there is no real user behind it to preserve. + * + * contexts.app.device_app_hash (iOS only) + * SHA1 of identifierForVendor + hw.machine + hw.model + bundle id, built + * by SentryCrashMonitor_System.m getDeviceAndAppHash and put on the SCOPE + * at hub init, so it rides on every event rather than only on crashes. + * identifierForVendor outlives a reinstall for as long as any other app + * from the same vendor is installed, so this one is MORE persistent than + * event.user, not less. + * + * contexts.os.rooted + * A jailbreak / root flag. Not an identifier, but it is a security + * attribute of the user's device that this project has no use for: nothing + * here branches on it and no bug has ever been diagnosed with it. + * + * WHAT IS DELIBERATELY NOT STRIPPED - do not "finish the job" by removing it: + * + * contexts.culture (locale, timezone, calendar, is_24_hour_format) + * KEEP. The app ships five locales and has produced real defects in + * currency formatting and in recurring-transaction date arithmetic where + * the reporter's timezone was the deciding fact. Removing culture blinds + * us to precisely the class of bug this project keeps finding. + * + * Touch breadcrumbs (category 'touch', from Sentry.wrap) + * KEEP. They are attached to a crash, never streamed on their own, and + * they carry no user-visible text: the SDK reads only a `sentry-label` + * prop or a configured labelName, neither of which this app sets, and + * there is no babel.config.js so the Sentry component-annotate plugin is + * not active either. What ships is React component display names. That is + * diagnostic context, not analytics. + * + * contexts.device / contexts.os / contexts.app, minus the two fields above + * KEEP - model, OS version, app version, memory, screen. This is the + * "crash logs and diagnostics" the store listing already declares. + * + * Losses accepted with this hook, so nobody rediscovers them as bugs: Sentry's + * "users affected" count and the crash-free-USERS rate stop being meaningful. + * Crash GROUPING is unaffected - it is computed from fingerprint, stack trace + * and exception type, never from event.user. + * + * =========================================================================== + * 3. withoutXhrBreadcrumbs - integrations + * =========================================================================== + * + * breadcrumbsIntegration hard-defaults `xhr: true` in its React Native + * wrapper, so every XMLHttpRequest becomes a breadcrumb carrying URL, method + * and status. This app makes almost no network calls, and the two URLs that do + * exist - the Metro dev server and the Sentry DSN host - are already filtered + * by the SDK's own beforeBreadcrumb. So the option buys nothing and collects + * request metadata nobody declared. + * + * It is turned off by DERIVING from the default integration list rather than + * by handing Sentry a hand-written one. The audit confirmed the remaining + * defaults are all wanted (device/OS/app context, native release, RN info, + * Expo OTA context, modules loader, dedupe, inbound filters, rewrite frames), + * and a hand-rolled list would silently lose them at the next SDK bump. */ -import type { Breadcrumb } from '@sentry/react-native'; +import { breadcrumbsIntegration } from '@sentry/react-native'; +import type { Breadcrumb, Event } from '@sentry/react-native'; + +/** + * The SDK's Integration type, taken from a factory it already exports, so this + * module does not have to reach past @sentry/react-native into @sentry/core. + */ +type SentryIntegration = ReturnType; /** * Sentry beforeBreadcrumb hook. Returns null for console breadcrumbs, which @@ -53,3 +153,62 @@ export function dropConsoleBreadcrumbs(breadcrumb: Breadcrumb): Breadcrumb | nul } return breadcrumb; } + +/** + * Sentry beforeSend hook. Removes the two persistent identifiers and the + * jailbreak flag from an outgoing event, then returns the event. + * + * Mutate-and-return is the SDK contract for beforeSend: return the event to + * send it, or null to drop it. This guard NEVER drops - a crash report with + * the identifiers removed is still worth having. + * + * Every lookup is defensive on purpose. `contexts` and each individual context + * are absent on some platforms and some event types - device_app_hash is iOS + * only, and a JS-only event captured before the native scope is read has no + * app context at all - so a missing key must be a no-op, never a throw. An + * exception here would be raised inside Sentry's own pipeline, where the SDK + * would swallow it and send the event UNSTRIPPED. + */ +export function stripPersistentIdentifiers(event: T): T { + // The whole user object, not just .id: the SDK only ever populates it with + // the installation identifier, so there is nothing else in there to keep. + delete event.user; + + const contexts = event.contexts; + if (contexts) { + const app = contexts.app; + if (app) { + delete app.device_app_hash; + } + + const os = contexts.os; + if (os) { + delete os.rooted; + } + } + + return event; +} + +/** + * Sentry `integrations` resolver. Receives the SDK's default integration list + * and returns it with the breadcrumbs integration swapped for one that has XHR + * breadcrumbs disabled. + * + * Matching is done against the replacement's OWN name rather than a hard-coded + * 'Breadcrumbs' string, so the swap cannot silently degrade into an append if + * the SDK ever renames the integration. + */ +export function withoutXhrBreadcrumbs( + defaultIntegrations: SentryIntegration[], +): SentryIntegration[] { + // console stays true: console breadcrumbs are killed downstream by + // dropConsoleBreadcrumbs, and leaving the flag alone keeps this a + // single-purpose change. + const replacement = breadcrumbsIntegration({ xhr: false }); + + return [ + ...defaultIntegrations.filter((integration) => integration.name !== replacement.name), + replacement, + ]; +}