From b2fad8e2104f7d0ba348decec61163f52a869801 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 02:08:33 -0700 Subject: [PATCH 1/4] security: fail closed for Sentry payloads --- src/services/monitoring/sentry.test.ts | 301 ++++++++++++++++++++++++- src/services/monitoring/sentry.ts | 210 ++++++++++++++++- 2 files changed, 504 insertions(+), 7 deletions(-) diff --git a/src/services/monitoring/sentry.test.ts b/src/services/monitoring/sentry.test.ts index df78030b..00b4dce0 100644 --- a/src/services/monitoring/sentry.test.ts +++ b/src/services/monitoring/sentry.test.ts @@ -4,6 +4,8 @@ const mockInit = jest.fn(); const mockCaptureException = jest.fn(); const mockSetUser = jest.fn(); +type SentryModule = typeof import('./sentry'); + jest.mock('@sentry/react', () => ({ init: mockInit, captureException: mockCaptureException, @@ -18,10 +20,11 @@ function setNodeEnv(value: string | undefined) { }); } -function initializeFor(nodeEnv: string, locationPath = '/') { +function initializeFor(nodeEnv: string, locationPath = '/'): SentryModule | undefined { const originalNodeEnv = process.env.NODE_ENV; const originalDsn = process.env.REACT_APP_SENTRY_DSN; const originalLocation = `${window.location.pathname}${window.location.search}${window.location.hash}`; + let sentryModule: SentryModule | undefined; setNodeEnv(nodeEnv); process.env.REACT_APP_SENTRY_DSN = 'https://public@example.ingest.sentry.io/1'; window.history.replaceState(null, '', locationPath); @@ -29,7 +32,8 @@ function initializeFor(nodeEnv: string, locationPath = '/') { try { jest.isolateModules(() => { // eslint-disable-next-line global-require - require('./sentry').initSentry(); + sentryModule = require('./sentry') as SentryModule; + sentryModule.initSentry(); }); } finally { window.history.replaceState(null, '', originalLocation); @@ -37,6 +41,8 @@ function initializeFor(nodeEnv: string, locationPath = '/') { if (originalDsn === undefined) delete process.env.REACT_APP_SENTRY_DSN; else process.env.REACT_APP_SENTRY_DSN = originalDsn; } + + return sentryModule; } describe('Sentry environment and callback isolation', () => { @@ -72,4 +78,295 @@ describe('Sentry environment and callback isolation', () => { initializeFor('production', locationPath); expect(mockInit).not.toHaveBeenCalled(); }); + + test('disables replay, tracing, default PII, breadcrumbs, and transactions', () => { + initializeFor('production', '/events'); + + const options = mockInit.mock.calls[0][0]; + expect(options).toEqual(expect.objectContaining({ + autoSessionTracking: false, + beforeBreadcrumb: expect.any(Function), + beforeSend: expect.any(Function), + beforeSendTransaction: expect.any(Function), + integrations: expect.any(Function), + replaysOnErrorSampleRate: 0, + replaysSessionSampleRate: 0, + sendClientReports: false, + sendDefaultPii: false, + tracesSampleRate: 0, + })); + expect(options.integrations([ + { name: 'Breadcrumbs' }, + { name: 'BrowserProfiling' }, + { name: 'BrowserTracing' }, + { name: 'ContextLines' }, + { name: 'ExtraErrorData' }, + { name: 'Feedback' }, + { name: 'HttpContext' }, + { name: 'Replay' }, + { name: 'UnknownFutureIntegration' }, + { name: 'GlobalHandlers' }, + { name: 'TryCatch' }, + ])).toEqual([ + { name: 'GlobalHandlers' }, + { name: 'TryCatch' }, + ]); + expect(options.beforeBreadcrumb({ message: 'private form value' })).toBeNull(); + expect(options.beforeSendTransaction({ transaction: '/account?token=secret' })).toBeNull(); + }); + + test('keeps only allowlisted error type and safe stack locations', () => { + initializeFor('production', '/events'); + + const options = mockInit.mock.calls[0][0]; + const canaries = [ + 'member-canary@example.test', + '+1-555-010-4242', + '123 Canary Street', + '1940-01-02', + 'Emergency Contact Canary', + 'oauth-token-canary', + 'reset-code-canary', + 'provider-response-canary', + 'private-form-value-canary', + 'request-body-canary', + 'capability-token-canary', + 'uid-canary', + ]; + const unsafeEvent = { + event_id: '0123456789abcdef0123456789abcdef', + timestamp: 1_700_000_000, + level: 'error', + platform: 'javascript', + environment: 'production', + release: 'mprc-web-e86a0f7', + dist: canaries[5], + logger: canaries[6], + user: { id: canaries[11], email: canaries[0], phone: canaries[1] }, + request: { + url: `https://runmprc.com/register/success?token=${canaries[10]}#fragment`, + data: canaries[9], + headers: { authorization: `Bearer ${canaries[5]}` }, + }, + message: `${canaries[0]} ${canaries[2]} ${canaries[3]}`, + breadcrumbs: [{ message: canaries[8], data: { phone: canaries[1] } }], + tags: { email: canaries[0] }, + extra: { emergencyContact: canaries[4], provider: canaries[7] }, + contexts: { auth: { resetCode: canaries[6] } }, + exception: { + values: [{ + type: 'TypeError', + value: `${canaries[0]} failed with ${canaries[5]}`, + module: canaries[7], + stacktrace: { + frames: [{ + filename: `https://${canaries[11]}.example.test/static/js/main.e86a0f7a.js?token=${canaries[10]}#fragment`, + function: canaries[5], + module: canaries[2], + platform: canaries[3], + abs_path: `https://runmprc.com/main.js?token=${canaries[10]}`, + lineno: 42, + colno: 7, + in_app: true, + context_line: canaries[8], + pre_context: [canaries[2]], + post_context: [canaries[4]], + vars: { response: canaries[7] }, + }, { + filename: `/static/js/${canaries[10]}.js`, + function: canaries[5], + lineno: 99, + }], + }, + }, { + type: canaries[5], + value: canaries[9], + }], + }, + }; + + const unsafeHint = { + attachments: [{ + data: canaries[9], + filename: canaries[0], + }], + }; + const sanitized = options.beforeSend(unsafeEvent, unsafeHint); + expect(unsafeHint.attachments).toEqual([]); + expect(sanitized).toEqual({ + environment: 'production', + event_id: '0123456789abcdef0123456789abcdef', + exception: { + values: [{ + stacktrace: { + frames: [{ + colno: 7, + filename: '/static/js/main.e86a0f7a.js', + in_app: true, + lineno: 42, + }], + }, + type: 'TypeError', + }, { + type: 'Error', + }], + }, + level: 'error', + platform: 'javascript', + release: 'mprc-web-e86a0f7', + timestamp: 1_700_000_000, + }); + + const serialized = JSON.stringify(sanitized); + canaries.forEach((canary) => expect(serialized).not.toContain(canary)); + expect(options.beforeSend({ message: canaries[8] }, {})).toBeNull(); + }); + + test('fails closed for malformed events and stack frames', () => { + initializeFor('production', '/events'); + + const options = mockInit.mock.calls[0][0]; + [ + null, + undefined, + {}, + { exception: null }, + { exception: { values: null } }, + { exception: { values: {} } }, + { exception: { values: [null, 'not-an-exception'] } }, + ].forEach((event) => expect(options.beforeSend(event, {})).toBeNull()); + + const throwingEvent = {}; + Object.defineProperty(throwingEvent, 'exception', { + get: () => { throw new Error('private-getter-canary'); }, + }); + expect(options.beforeSend(throwingEvent, {})).toBeNull(); + + const coercingIdentifier = { + privateValue: 'coercion-canary', + toString: () => '0123456789abcdef0123456789abcdef', + }; + const coerced = options.beforeSend({ + event_id: coercingIdentifier, + release: coercingIdentifier, + exception: { values: [{ type: 'TypeError' }] }, + }, {}); + expect(coerced).toEqual({ + exception: { values: [{ type: 'TypeError' }] }, + }); + expect(JSON.stringify(coerced)).not.toContain('coercion-canary'); + + expect(options.beforeSend({ + exception: { + values: [{ + type: 'TypeError', + stacktrace: { + frames: [{ + filename: '/static/js/123.e86a0f7a.chunk.js', + lineno: 10_000_001, + colno: 100_001, + }], + }, + }], + }, + }, {})).toEqual({ + exception: { + values: [{ + stacktrace: { + frames: [{ filename: '/static/js/123.e86a0f7a.chunk.js' }], + }, + type: 'TypeError', + }], + }, + }); + }); + + test('reads each retained event field once before validating it', () => { + initializeFor('production', '/events'); + + const options = mockInit.mock.calls[0][0]; + const readCounts = new Map(); + const mutationCanary = 'mutating-getter-canary'; + const defineMutatingGetter = ( + target: Record, + key: string, + label: string, + safeValue: unknown, + ) => Object.defineProperty(target, key, { + configurable: true, + get: () => { + const count = (readCounts.get(label) || 0) + 1; + readCounts.set(label, count); + return count === 1 ? safeValue : mutationCanary; + }, + }); + + const frame: Record = {}; + defineMutatingGetter(frame, 'filename', 'frame.filename', '/static/js/main.e86a0f7a.js'); + defineMutatingGetter(frame, 'lineno', 'frame.lineno', 42); + defineMutatingGetter(frame, 'colno', 'frame.colno', 7); + defineMutatingGetter(frame, 'in_app', 'frame.in_app', true); + + const stacktrace: Record = {}; + defineMutatingGetter(stacktrace, 'frames', 'stacktrace.frames', [frame]); + + const exception: Record = {}; + defineMutatingGetter(exception, 'type', 'exception.type', 'TypeError'); + defineMutatingGetter(exception, 'stacktrace', 'exception.stacktrace', stacktrace); + + const exceptionContainer: Record = {}; + defineMutatingGetter(exceptionContainer, 'values', 'exception.values', [exception]); + + const event: Record = {}; + defineMutatingGetter(event, 'exception', 'event.exception', exceptionContainer); + defineMutatingGetter( + event, + 'event_id', + 'event.event_id', + '0123456789abcdef0123456789abcdef', + ); + defineMutatingGetter(event, 'timestamp', 'event.timestamp', 1_700_000_000); + defineMutatingGetter(event, 'level', 'event.level', 'error'); + defineMutatingGetter(event, 'platform', 'event.platform', 'javascript'); + defineMutatingGetter(event, 'environment', 'event.environment', 'production'); + defineMutatingGetter(event, 'release', 'event.release', 'mprc-web-e86a0f7'); + + const sanitized = options.beforeSend(event, {}); + expect(sanitized).toEqual({ + environment: 'production', + event_id: '0123456789abcdef0123456789abcdef', + exception: { + values: [{ + stacktrace: { + frames: [{ + colno: 7, + filename: '/static/js/main.e86a0f7a.js', + in_app: true, + lineno: 42, + }], + }, + type: 'TypeError', + }], + }, + level: 'error', + platform: 'javascript', + release: 'mprc-web-e86a0f7', + timestamp: 1_700_000_000, + }); + expect(Array.from(readCounts.values()).every((count) => count === 1)).toBe(true); + expect(JSON.stringify(sanitized)).not.toContain(mutationCanary); + }); + + test('clears rather than attaching direct member identifiers', () => { + const sentryModule = initializeFor('production', '/account'); + + sentryModule?.setUserContext({ + uid: 'uid-canary', + email: 'member-canary@example.test', + }); + + expect(mockSetUser).toHaveBeenCalledWith(null); + expect(JSON.stringify(mockSetUser.mock.calls)).not.toContain('uid-canary'); + expect(JSON.stringify(mockSetUser.mock.calls)).not.toContain('member-canary@example.test'); + }); }); diff --git a/src/services/monitoring/sentry.ts b/src/services/monitoring/sentry.ts index a20e423c..1a1c68aa 100644 --- a/src/services/monitoring/sentry.ts +++ b/src/services/monitoring/sentry.ts @@ -1,4 +1,11 @@ import * as Sentry from '@sentry/react'; +import type { + Event, + EventHint, + Exception, + SeverityLevel, + StackFrame, +} from '@sentry/react'; import hasCapabilityCallbackState from './capabilityCallback'; @@ -13,11 +20,195 @@ import hasCapabilityCallbackState from './capabilityCallback'; let initialized = false; +const SAFE_EVENT_ID = /^[a-f0-9]{32}$/i; +const SAFE_BUNDLE_FILENAME = /^\/static\/js\/(?:main|[0-9]+)\.[a-f0-9]{8}(?:\.chunk)?\.js$/i; +const SAFE_RELEASE = /^(?:mprc-web[-@])?[a-f0-9]{7,40}$/i; +const SAFE_ENVIRONMENTS = new Set(['preview', 'production', 'staging']); +const SAFE_ERROR_TYPES = new Set([ + 'AggregateError', + 'DOMException', + 'Error', + 'EvalError', + 'FirebaseError', + 'RangeError', + 'ReferenceError', + 'SyntaxError', + 'TypeError', + 'URIError', +]); +const SAFE_LEVELS: ReadonlySet = new Set([ + 'debug', + 'info', + 'log', + 'warning', + 'error', + 'fatal', +]); +const SAFE_DEFAULT_INTEGRATIONS = new Set(['GlobalHandlers', 'TryCatch']); + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isSafeLevel(value: unknown): value is SeverityLevel { + return typeof value === 'string' && SAFE_LEVELS.has(value as SeverityLevel); +} + +function safeInteger(value: unknown, maximum: number): number | undefined { + return Number.isSafeInteger(value) + && (value as number) >= 0 + && (value as number) <= maximum + ? value as number + : undefined; +} + +function safeFilename(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + + let path = value; + try { + if (/^[A-Za-z][A-Za-z0-9+.-]*:\/\//.test(path)) { + path = new URL(path).pathname; + } + } catch { + return undefined; + } + + [path] = path.split(/[?#]/, 1); + if (!path.startsWith('/')) path = `/${path}`; + return SAFE_BUNDLE_FILENAME.test(path) ? path : undefined; +} + +function sanitizeStackFrame(frame: unknown): StackFrame | undefined { + if (!isRecord(frame)) return undefined; + + const filenameCandidate = frame.filename; + const linenoCandidate = frame.lineno; + const colnoCandidate = frame.colno; + const inAppCandidate = frame.in_app; + const filename = safeFilename(filenameCandidate); + if (!filename) return undefined; + + const safeFrame: StackFrame = { filename }; + const lineno = safeInteger(linenoCandidate, 10_000_000); + const colno = safeInteger(colnoCandidate, 100_000); + + if (lineno !== undefined) safeFrame.lineno = lineno; + if (colno !== undefined) safeFrame.colno = colno; + if (typeof inAppCandidate === 'boolean') safeFrame.in_app = inAppCandidate; + + return safeFrame; +} + +function sanitizeException(exception: unknown): Exception | undefined { + if (!isRecord(exception)) return undefined; + + const typeCandidate = exception.type; + const stacktraceCandidate = exception.stacktrace; + const stacktrace = isRecord(stacktraceCandidate) ? stacktraceCandidate : undefined; + const framesCandidate = stacktrace?.frames; + const rawFrames = Array.isArray(framesCandidate) ? framesCandidate : []; + const frames = rawFrames + .slice(-50) + .map(sanitizeStackFrame) + .filter((frame): frame is StackFrame => frame !== undefined); + const safeException: Exception = { + type: typeof typeCandidate === 'string' && SAFE_ERROR_TYPES.has(typeCandidate) + ? typeCandidate + : 'Error', + }; + + if (frames && frames.length > 0) { + safeException.stacktrace = { frames }; + } + + return safeException; +} + +/** + * Sentry receives only a small diagnostic projection. Unknown fields are + * intentionally dropped so new SDK integrations cannot silently add PII. + */ +export function sanitizeSentryEvent(event: Event): Event | null { + try { + if (!isRecord(event)) return null; + + const exceptionCandidate = event.exception; + const exceptionContainer = isRecord(exceptionCandidate) ? exceptionCandidate : undefined; + const exceptionValuesCandidate = exceptionContainer?.values; + const rawExceptions = Array.isArray(exceptionValuesCandidate) + ? exceptionValuesCandidate + : []; + const exceptions = rawExceptions + .slice(0, 5) + .map(sanitizeException) + .filter((exception): exception is Exception => exception !== undefined); + + if (exceptions.length === 0) return null; + + const safeEvent: Event = { + exception: { values: exceptions }, + }; + + const eventIdCandidate = event.event_id; + const timestampCandidate = event.timestamp; + const levelCandidate = event.level; + const platformCandidate = event.platform; + const environmentCandidate = event.environment; + const releaseCandidate = event.release; + + if (typeof eventIdCandidate === 'string' && SAFE_EVENT_ID.test(eventIdCandidate)) { + safeEvent.event_id = eventIdCandidate; + } + if ( + typeof timestampCandidate === 'number' + && Number.isFinite(timestampCandidate) + && timestampCandidate >= 0 + && timestampCandidate <= 4_102_444_800 + ) { + safeEvent.timestamp = timestampCandidate; + } + if (isSafeLevel(levelCandidate)) { + safeEvent.level = levelCandidate; + } + + if (platformCandidate === 'javascript') safeEvent.platform = platformCandidate; + if ( + typeof environmentCandidate === 'string' + && SAFE_ENVIRONMENTS.has(environmentCandidate) + ) { + safeEvent.environment = environmentCandidate; + } + if (typeof releaseCandidate === 'string' && SAFE_RELEASE.test(releaseCandidate)) { + safeEvent.release = releaseCandidate; + } + + return safeEvent; + } catch { + return null; + } +} + +function sanitizeSentryEventAndHint(event: Event, hint: EventHint): Event | null { + try { + // Sentry serializes hint attachments after beforeSend. Clearing the same + // hint object is therefore part of the fail-closed payload boundary. + const attachments = hint?.attachments; + if (attachments !== undefined) { + if (!Array.isArray(attachments)) return null; + attachments.splice(0, attachments.length); + } + return sanitizeSentryEvent(event); + } catch { + return null; + } +} + export function initSentry(): void { if (initialized) return; // Local/test sessions must not reach an outside monitoring service. In // production, do not initialize on callback URLs carrying OAuth or checkout - // capabilities. #111 owns the broader hosted redaction/replay policy. + // capabilities. Hosted events then pass through the allowlist below. if ( process.env.NODE_ENV !== 'production' || (typeof window !== 'undefined' && hasCapabilityCallbackState(window.location)) @@ -30,9 +221,18 @@ export function initSentry(): void { Sentry.init({ dsn, environment, - tracesSampleRate: 0.1, + autoSessionTracking: false, + sendClientReports: false, + sendDefaultPii: false, + tracesSampleRate: 0, replaysSessionSampleRate: 0, - replaysOnErrorSampleRate: 1.0, + replaysOnErrorSampleRate: 0, + integrations: (defaultIntegrations) => defaultIntegrations.filter( + (integration) => SAFE_DEFAULT_INTEGRATIONS.has(integration.name), + ), + beforeBreadcrumb: () => null, + beforeSend: sanitizeSentryEventAndHint, + beforeSendTransaction: () => null, }); initialized = true; } @@ -44,8 +244,8 @@ export function captureException(err: unknown): void { } export function setUserContext(user: { uid: string; email?: string | null }): void { - if (!initialized) return; - Sentry.setUser({ id: user.uid, email: user.email || undefined }); + if (!initialized || !user) return; + Sentry.setUser(null); } export function clearUserContext(): void { From 2399de6b5ef8f6d58b977356b85c18cfc6aad7fb Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 03:09:39 -0700 Subject: [PATCH 2/4] docs: record Sentry privacy boundary --- OPERATIONS_RUNBOOK.md | 2 ++ SECURITY.md | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index 4c6efd87..c9a8765c 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -520,4 +520,6 @@ Issue #124 adds that complete committed frontend suite to the hosted frontend jo On the isolated #99 branch rebased over #124, Node 20 local evidence is: 10/10 standalone SPA navigation/referrer-policy tests; 31/31 focused Firebase/monitoring tests; 4 frontend suites / 47 tests; 17/17 Functions tests; a clean diagnostic build; and a demo-only CLI smoke in which Auth, Firestore, and Functions listened on the three documented loopback ports. No browser account flow, production callback, Stripe/Strava/email call, Firebase deployment, or production data was used. #126 adds the standalone SPA suite as its own named blocking hosted step; only exact pull-request and post-merge step results prove that gate ran. +The #134 remediation-ledger entry is source/test evidence only. For OBS-001A1 [#134](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/134) and [PR #137](https://github.com/Run-MPRC/Run-MPRC.github.io/pull/137), keep six evidence states separate: **source/tests** means the exact reviewed Sentry source plus mocked synthetic-canary results; **merge** means the resulting `main` SHA; **website** means a hosting publication record plus a separate check that `runmprc.com` serves that exact revision; **Firebase** is unchanged by this issue, so no Firebase deployment may be inferred; **Sentry provider** remains unverified for transport/ingestion, IP handling, retention, access, consent, deletion, and vendor terms; **live behavior** remains unverified unless an approved no-PII synthetic check proves the expected bounded event at the exact website revision. Never use member data or intentionally trigger an error in a real member flow. A green check, merged PR, preview, or website publication proves only its named surface and does not prove a later provider or live state. + Provider configuration, production secrets, Netlify publication, Firebase deployment, and live behavior always require separate dated evidence. Local tests and a green GitHub summary do not prove them. diff --git a/SECURITY.md b/SECURITY.md index bdb5b8a7..a37a6f04 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -102,7 +102,7 @@ These entries are implementation evidence, not a production risk-acceptance deci | RISK-013 | PROMO-001 sets both current Checkout creators to `allow_promotion_codes: false`; the webhook quarantines nonzero discounts plus unconfigured tax/shipping totals. | Add exact creator-payload tests, inventory pre-change Sessions/provider promotion configuration, keep the guard through staging/deploy, and implement an authoritative discount/tax/shipping contract before ever enabling it. | | Part of RISK-014 | SUPPLY-001A [#127](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/127) removes the unused direct `gpxparser` dependency and 50 lockfile entries without changing any retained package version, source URL, or integrity hash. A fresh Node 20 install lowers the root production audit from 33 findings (3 critical, 9 high) to 22 (1 critical, 8 high), and the optimized production asset hashes remain identical. | Remediate or time-bound the remaining root and Functions findings through the separately tested router, Firebase, Stripe, build-system, and scanning slices; do not use a forced audit upgrade. | | Part of RISK-024 | SEC-001 protects token documents from every browser client. | OAUTH-001 must still add transactional refresh/versioning, scope/account binding, disconnect/revocation, IAM/encryption decision, and redacted lifecycle audit. | -| Part of RISK-027 | #99 keeps App Check, Firebase Analytics, and Sentry from initializing in local/test even when public provider configuration is present. Initial capability callback routes also fail closed before App Check, Firebase Analytics, or Sentry startup. | #111 still owns general hosted URL/event/log redaction, already-initialized SPA transitions, replay/consent/retention policy, payload inspection, and provider evidence. ABUSE-001A must defer the three callback callables; DATA-001A and OAUTH-001C within canonical tracker [#88](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/88) own their safe protected handoffs. | +| Part of RISK-027 | #99 blocks App Check, Firebase Analytics, and Sentry in local/test and on initial capability-callback startup. OBS-001A1 [#134](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/134) source disables Sentry replay, tracing, browser session tracking, client reports, member user context, breadcrumbs, attachments, transactions, and unapproved default integrations. Its fail-closed allowlist retains only bounded low-cardinality event fields, generic exception types, and compiled-bundle path/line metadata; synthetic tests use a mocked Sentry client and make no provider call. | #111 still owns Firebase Analytics allowlisting and any residual hosted telemetry boundary. Source/tests and merge evidence do not prove website publication, the exact `runmprc.com` revision, Sentry transport/ingestion or provider IP handling, retention, access, consent, deletion, or vendor terms; verify those separately under #110. Callback protected handoffs remain with ABUSE-001A, DATA-001A, and OAUTH-001C in #88. | | Part of RISK-035 | The Jest environment is repaired, the stale frontend smoke test is replaced, #124 adds the complete frontend suite as a named blocking hosted step, and #126 adds the standalone SPA callback suite as its own named blocking step. | Keep exact hosted evidence on #124/#126; make lint non-mutating/fail-closed, prove required branch checks, and add the broader TEST-001 suite. | | RISK-026, RISK-036 | #135 merged through PR #138 as `9eafab1217aff7058c42240aaba72d7b93f8ed24`, replacing automatic frontend-first/fail-open GitHub deployment with a tested manual exact-current-commit gate. Post-merge staging and synthetic production probes failed closed before authentication or mutation, and published neither Firebase nor Pages. | #133 must configure protected environments and least-privilege OIDC/WIF; #136 must prove staged/target deployment and clearing/readback of the existing Pages `runmprc.com` claim; a protected WEB-001 child must establish Netlify publication and rollback. No Firebase, Pages, live-host, or provider-setting change is proven by source/static tests alone. | | RISK-039 | The #118 source slice uses an empty authenticated callable request, bounded Firebase Auth identity fields, one transactional create-only helper shared with signup, constant responses, generic failures, and a UI that hides Edit until setup and the Rules-protected read succeed. Signup and recovery never change custom claims; browser profile creation remains denied. | Source review/merge is not deployment. Under #105, deploy the exact #100 Rules plus both `createMemberOnSignUp` and `ensureMemberProfile` before the website, prove App Check policy, use a synthetic staged account, verify rollback, then record website, Function, Rules, and live behavior separately. Never repair a real profile manually. | From e9e5f3136bbe00e31ab2392c3862248abc271668 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 03:17:22 -0700 Subject: [PATCH 3/4] security: reject poisoned Sentry arrays --- SECURITY.md | 2 +- src/services/monitoring/sentry.test.ts | 86 +++++++++++++++++++++++--- src/services/monitoring/sentry.ts | 49 +++++++++------ 3 files changed, 109 insertions(+), 28 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index a37a6f04..90537908 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -102,7 +102,7 @@ These entries are implementation evidence, not a production risk-acceptance deci | RISK-013 | PROMO-001 sets both current Checkout creators to `allow_promotion_codes: false`; the webhook quarantines nonzero discounts plus unconfigured tax/shipping totals. | Add exact creator-payload tests, inventory pre-change Sessions/provider promotion configuration, keep the guard through staging/deploy, and implement an authoritative discount/tax/shipping contract before ever enabling it. | | Part of RISK-014 | SUPPLY-001A [#127](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/127) removes the unused direct `gpxparser` dependency and 50 lockfile entries without changing any retained package version, source URL, or integrity hash. A fresh Node 20 install lowers the root production audit from 33 findings (3 critical, 9 high) to 22 (1 critical, 8 high), and the optimized production asset hashes remain identical. | Remediate or time-bound the remaining root and Functions findings through the separately tested router, Firebase, Stripe, build-system, and scanning slices; do not use a forced audit upgrade. | | Part of RISK-024 | SEC-001 protects token documents from every browser client. | OAUTH-001 must still add transactional refresh/versioning, scope/account binding, disconnect/revocation, IAM/encryption decision, and redacted lifecycle audit. | -| Part of RISK-027 | #99 blocks App Check, Firebase Analytics, and Sentry in local/test and on initial capability-callback startup. OBS-001A1 [#134](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/134) source disables Sentry replay, tracing, browser session tracking, client reports, member user context, breadcrumbs, attachments, transactions, and unapproved default integrations. Its fail-closed allowlist retains only bounded low-cardinality event fields, generic exception types, and compiled-bundle path/line metadata; synthetic tests use a mocked Sentry client and make no provider call. | #111 still owns Firebase Analytics allowlisting and any residual hosted telemetry boundary. Source/tests and merge evidence do not prove website publication, the exact `runmprc.com` revision, Sentry transport/ingestion or provider IP handling, retention, access, consent, deletion, or vendor terms; verify those separately under #110. Callback protected handoffs remain with ABUSE-001A, DATA-001A, and OAUTH-001C in #88. | +| Part of RISK-027 | #99 blocks App Check, Firebase Analytics, and Sentry in local/test and on initial capability-callback startup. OBS-001A1 [#134](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/134) source disables Sentry replay, tracing, browser session tracking, client reports, member user context, breadcrumbs, attachments, transactions, and unapproved default integrations. Its fail-closed allowlist retains only strictly validated non-content event metadata, generic exception types, and compiled-bundle path/line metadata; synthetic tests use a mocked Sentry client and make no provider call. | #111 still owns Firebase Analytics allowlisting and any residual hosted telemetry boundary. Source/tests and merge evidence do not prove website publication, the exact `runmprc.com` revision, Sentry transport/ingestion or provider IP handling, retention, access, consent, deletion, or vendor terms; verify those separately under #110. Callback protected handoffs remain with ABUSE-001A, DATA-001A, and OAUTH-001C in #88. | | Part of RISK-035 | The Jest environment is repaired, the stale frontend smoke test is replaced, #124 adds the complete frontend suite as a named blocking hosted step, and #126 adds the standalone SPA callback suite as its own named blocking step. | Keep exact hosted evidence on #124/#126; make lint non-mutating/fail-closed, prove required branch checks, and add the broader TEST-001 suite. | | RISK-026, RISK-036 | #135 merged through PR #138 as `9eafab1217aff7058c42240aaba72d7b93f8ed24`, replacing automatic frontend-first/fail-open GitHub deployment with a tested manual exact-current-commit gate. Post-merge staging and synthetic production probes failed closed before authentication or mutation, and published neither Firebase nor Pages. | #133 must configure protected environments and least-privilege OIDC/WIF; #136 must prove staged/target deployment and clearing/readback of the existing Pages `runmprc.com` claim; a protected WEB-001 child must establish Netlify publication and rollback. No Firebase, Pages, live-host, or provider-setting change is proven by source/static tests alone. | | RISK-039 | The #118 source slice uses an empty authenticated callable request, bounded Firebase Auth identity fields, one transactional create-only helper shared with signup, constant responses, generic failures, and a UI that hides Edit until setup and the Rules-protected read succeed. Signup and recovery never change custom claims; browser profile creation remains denied. | Source review/merge is not deployment. Under #105, deploy the exact #100 Rules plus both `createMemberOnSignUp` and `ensureMemberProfile` before the website, prove App Check policy, use a synthetic staged account, verify rollback, then record website, Function, Rules, and live behavior separately. Never repair a real profile manually. | diff --git a/src/services/monitoring/sentry.test.ts b/src/services/monitoring/sentry.test.ts index 00b4dce0..8d965ca9 100644 --- a/src/services/monitoring/sentry.test.ts +++ b/src/services/monitoring/sentry.test.ts @@ -185,14 +185,7 @@ describe('Sentry environment and callback isolation', () => { }, }; - const unsafeHint = { - attachments: [{ - data: canaries[9], - filename: canaries[0], - }], - }; - const sanitized = options.beforeSend(unsafeEvent, unsafeHint); - expect(unsafeHint.attachments).toEqual([]); + const sanitized = options.beforeSend(unsafeEvent, {}); expect(sanitized).toEqual({ environment: 'production', event_id: '0123456789abcdef0123456789abcdef', @@ -220,9 +213,18 @@ describe('Sentry environment and callback isolation', () => { const serialized = JSON.stringify(sanitized); canaries.forEach((canary) => expect(serialized).not.toContain(canary)); expect(options.beforeSend({ message: canaries[8] }, {})).toBeNull(); + + const unsafeHint = { + attachments: [{ + data: canaries[9], + filename: canaries[0], + }], + }; + expect(options.beforeSend(unsafeEvent, unsafeHint)).toBeNull(); + expect(unsafeHint.attachments).toHaveLength(1); }); - test('fails closed for malformed events and stack frames', () => { + test('fails closed for malformed events, hints, and stack frames', () => { initializeFor('production', '/events'); const options = mockInit.mock.calls[0][0]; @@ -242,6 +244,26 @@ describe('Sentry environment and callback isolation', () => { }); expect(options.beforeSend(throwingEvent, {})).toBeNull(); + const validEvent = { exception: { values: [{ type: 'TypeError' }] } }; + expect(options.beforeSend(validEvent, { attachments: {} })).toBeNull(); + expect(options.beforeSend(validEvent, { + attachments: Object.freeze([{ data: 'frozen-attachment-canary' }]), + })).toBeNull(); + + const poisonedAttachments = [{ data: 'poisoned-attachment-canary' }]; + const noOpSplice = jest.fn(() => []); + Object.defineProperty(poisonedAttachments, 'splice', { value: noOpSplice }); + expect(options.beforeSend(validEvent, { + attachments: poisonedAttachments, + })).toBeNull(); + expect(noOpSplice).not.toHaveBeenCalled(); + + const throwingHint = {}; + Object.defineProperty(throwingHint, 'attachments', { + get: () => { throw new Error('attachment-getter-canary'); }, + }); + expect(options.beforeSend(validEvent, throwingHint)).toBeNull(); + const coercingIdentifier = { privateValue: 'coercion-canary', toString: () => '0123456789abcdef0123456789abcdef', @@ -281,6 +303,52 @@ describe('Sentry environment and callback isolation', () => { }); }); + test('does not invoke methods overridden by untrusted array subclasses', () => { + initializeFor('production', '/events'); + + const options = mockInit.mock.calls[0][0]; + const methodCanary = 'private-array-method-canary'; + class PoisonedArray extends Array {} + const frames = new PoisonedArray>(); + frames[0] = { + filename: '/static/js/main.e86a0f7a.js', + function: methodCanary, + }; + const exceptions = new PoisonedArray>(); + exceptions[0] = { + type: 'TypeError', + value: methodCanary, + stacktrace: { frames }, + }; + const poisonedMethods: jest.Mock[] = []; + + [frames, exceptions].forEach((array) => { + ['slice', 'map', 'filter'].forEach((method) => { + const poisonedMethod = jest.fn(() => { + throw new Error(methodCanary); + }); + poisonedMethods[poisonedMethods.length] = poisonedMethod; + Object.defineProperty(array, method, { value: poisonedMethod }); + }); + }); + + const sanitized = options.beforeSend({ + exception: { values: exceptions }, + }, {}); + expect(sanitized).toEqual({ + exception: { + values: [{ + stacktrace: { + frames: [{ filename: '/static/js/main.e86a0f7a.js' }], + }, + type: 'TypeError', + }], + }, + }); + poisonedMethods.forEach((method) => expect(method).not.toHaveBeenCalled()); + expect(JSON.stringify(sanitized)).not.toContain(methodCanary); + }); + test('reads each retained event field once before validating it', () => { initializeFor('production', '/events'); diff --git a/src/services/monitoring/sentry.ts b/src/services/monitoring/sentry.ts index 1a1c68aa..9a856570 100644 --- a/src/services/monitoring/sentry.ts +++ b/src/services/monitoring/sentry.ts @@ -107,11 +107,19 @@ function sanitizeException(exception: unknown): Exception | undefined { const stacktraceCandidate = exception.stacktrace; const stacktrace = isRecord(stacktraceCandidate) ? stacktraceCandidate : undefined; const framesCandidate = stacktrace?.frames; - const rawFrames = Array.isArray(framesCandidate) ? framesCandidate : []; - const frames = rawFrames - .slice(-50) - .map(sanitizeStackFrame) - .filter((frame): frame is StackFrame => frame !== undefined); + const frames: StackFrame[] = []; + if (Array.isArray(framesCandidate)) { + const frameCountCandidate = framesCandidate.length; + const frameCount = Number.isSafeInteger(frameCountCandidate) + && frameCountCandidate >= 0 + ? frameCountCandidate + : 0; + const firstFrame = frameCount > 50 ? frameCount - 50 : 0; + for (let index = firstFrame; index < frameCount; index += 1) { + const frame = sanitizeStackFrame(framesCandidate[index]); + if (frame !== undefined) frames[frames.length] = frame; + } + } const safeException: Exception = { type: typeof typeCandidate === 'string' && SAFE_ERROR_TYPES.has(typeCandidate) ? typeCandidate @@ -136,13 +144,21 @@ export function sanitizeSentryEvent(event: Event): Event | null { const exceptionCandidate = event.exception; const exceptionContainer = isRecord(exceptionCandidate) ? exceptionCandidate : undefined; const exceptionValuesCandidate = exceptionContainer?.values; - const rawExceptions = Array.isArray(exceptionValuesCandidate) - ? exceptionValuesCandidate - : []; - const exceptions = rawExceptions - .slice(0, 5) - .map(sanitizeException) - .filter((exception): exception is Exception => exception !== undefined); + const exceptions: Exception[] = []; + if (Array.isArray(exceptionValuesCandidate)) { + const rawExceptionCount = exceptionValuesCandidate.length; + const safeExceptionCount = Number.isSafeInteger(rawExceptionCount) + && rawExceptionCount >= 0 + ? rawExceptionCount + : 0; + const exceptionCount = safeExceptionCount < 5 + ? safeExceptionCount + : 5; + for (let index = 0; index < exceptionCount; index += 1) { + const exception = sanitizeException(exceptionValuesCandidate[index]); + if (exception !== undefined) exceptions[exceptions.length] = exception; + } + } if (exceptions.length === 0) return null; @@ -191,13 +207,10 @@ export function sanitizeSentryEvent(event: Event): Event | null { function sanitizeSentryEventAndHint(event: Event, hint: EventHint): Event | null { try { - // Sentry serializes hint attachments after beforeSend. Clearing the same - // hint object is therefore part of the fail-closed payload boundary. + // Sentry serializes hint attachments after beforeSend. Do not trust input + // array methods or mutation here: any event carrying attachments is dropped. const attachments = hint?.attachments; - if (attachments !== undefined) { - if (!Array.isArray(attachments)) return null; - attachments.splice(0, attachments.length); - } + if (attachments !== undefined) return null; return sanitizeSentryEvent(event); } catch { return null; From a0c9065635046c948f1ba92fdf1fe8175b693c40 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 03:21:42 -0700 Subject: [PATCH 4/4] security: stabilize Sentry attachment hints --- src/services/monitoring/sentry.test.ts | 48 +++++++++++++++++++++++--- src/services/monitoring/sentry.ts | 36 ++++++++++++++++--- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/services/monitoring/sentry.test.ts b/src/services/monitoring/sentry.test.ts index 8d965ca9..392b5e6e 100644 --- a/src/services/monitoring/sentry.test.ts +++ b/src/services/monitoring/sentry.test.ts @@ -224,6 +224,31 @@ describe('Sentry environment and callback isolation', () => { expect(unsafeHint.attachments).toHaveLength(1); }); + test('stabilizes the attachment-free hint before the SDK reads it again', () => { + initializeFor('production', '/events'); + + const options = mockInit.mock.calls[0][0]; + const hint: Record = {}; + const event = { exception: { values: [{ type: 'TypeError' }] } }; + const sanitized = options.beforeSend(event, hint); + + expect(sanitized).toEqual(event); + // Mirrors BaseClient.sendEvent's later read after beforeSend returns. + expect(hint.attachments).toBeUndefined(); + expect(Object.getOwnPropertyDescriptor(hint, 'attachments')).toEqual({ + configurable: false, + enumerable: false, + value: undefined, + writable: false, + }); + expect(() => { + Object.defineProperty(hint, 'attachments', { + value: [{ data: 'late-attachment-canary' }], + }); + }).toThrow(); + expect(hint.attachments).toBeUndefined(); + }); + test('fails closed for malformed events, hints, and stack frames', () => { initializeFor('production', '/events'); @@ -258,11 +283,26 @@ describe('Sentry environment and callback isolation', () => { })).toBeNull(); expect(noOpSplice).not.toHaveBeenCalled(); - const throwingHint = {}; - Object.defineProperty(throwingHint, 'attachments', { - get: () => { throw new Error('attachment-getter-canary'); }, + const accessorHint = {}; + let attachmentReadCount = 0; + const attachmentGetter = jest.fn(() => { + attachmentReadCount += 1; + return attachmentReadCount === 1 + ? undefined + : [{ data: 'private-hint-toctou-canary' }]; + }); + Object.defineProperty(accessorHint, 'attachments', { + get: attachmentGetter, }); - expect(options.beforeSend(validEvent, throwingHint)).toBeNull(); + expect(options.beforeSend(validEvent, accessorHint)).toBeNull(); + expect(attachmentGetter).not.toHaveBeenCalled(); + + const inheritedAttachmentGetter = jest.fn(() => undefined); + const inheritedHint = Object.create(Object.defineProperty({}, 'attachments', { + get: inheritedAttachmentGetter, + })); + expect(options.beforeSend(validEvent, inheritedHint)).toBeNull(); + expect(inheritedAttachmentGetter).not.toHaveBeenCalled(); const coercingIdentifier = { privateValue: 'coercion-canary', diff --git a/src/services/monitoring/sentry.ts b/src/services/monitoring/sentry.ts index 9a856570..ee180598 100644 --- a/src/services/monitoring/sentry.ts +++ b/src/services/monitoring/sentry.ts @@ -205,12 +205,40 @@ export function sanitizeSentryEvent(event: Event): Event | null { } } +function stabilizeAttachmentFreeHint(hint: EventHint): boolean { + if (!isRecord(hint)) return false; + + const descriptor = Object.getOwnPropertyDescriptor(hint, 'attachments'); + if (descriptor === undefined) { + // Reject inherited values/accessors. A new own, immutable data property + // below prevents a later SDK read from observing a changed prototype. + if ('attachments' in hint) return false; + } else if (!('value' in descriptor) || descriptor.value !== undefined) { + // Inspect the descriptor instead of reading the property so accessors + // cannot return a different value when Sentry reads the hint again. + return false; + } + + Object.defineProperty(hint, 'attachments', { + configurable: false, + enumerable: descriptor?.enumerable ?? false, + value: undefined, + writable: false, + }); + + const stableDescriptor = Object.getOwnPropertyDescriptor(hint, 'attachments'); + return stableDescriptor !== undefined + && 'value' in stableDescriptor + && stableDescriptor.value === undefined + && stableDescriptor.configurable === false + && stableDescriptor.writable === false; +} + function sanitizeSentryEventAndHint(event: Event, hint: EventHint): Event | null { try { - // Sentry serializes hint attachments after beforeSend. Do not trust input - // array methods or mutation here: any event carrying attachments is dropped. - const attachments = hint?.attachments; - if (attachments !== undefined) return null; + // Sentry reads hint attachments again after beforeSend. Stabilize that + // future read without invoking an attachment getter or input array method. + if (!stabilizeAttachmentFreeHint(hint)) return null; return sanitizeSentryEvent(event); } catch { return null;