diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 41d4162cc..008c80a58 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -422,6 +422,20 @@ jobs: # on macos-arm64 runners, causing OOM at node's ~2GB auto default. NODE_OPTIONS="--max-old-space-size=8192" cargo tauri build $PROFILE_FLAG -c "$TAURI_CONFIG_OVERRIDE" $MATRIX_ARGS + # Regression guard for #1403: if @sentry/vite-plugin silently no-op'd + # (e.g. SENTRY_AUTH_TOKEN missing, or the `sourcemaps.assets` glob + # didn't match dist/assets) production events arrive in Sentry as + # unsymbolicated minified frames. The plugin logs a warning then + # exits 0, so the only safe check is to inspect the shipped bundle + # for injected debug-IDs. Skipped when SENTRY_AUTH_TOKEN is empty + # (e.g. fork/PR builds where the upload was deliberately disabled). + - name: Verify Sentry source-map upload (frontend) + if: env.SENTRY_AUTH_TOKEN != '' + shell: bash + env: + SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} + run: node scripts/release/verify-sentry-sourcemaps.mjs + # ---- Sentry DIF uploads ------------------------------------------------ # Since #1061 the core lives in-process as a library linked into the # Tauri shell binary — there is exactly one Rust process and one @@ -444,12 +458,17 @@ jobs: run: | set -euo pipefail dif_dir="app/src-tauri/target/${MATRIX_TARGET}/${PROFILE}" - if [ -d "$dif_dir" ]; then - echo "==> Uploading symbols from $dif_dir to ${SENTRY_PROJECT}" - bash scripts/upload_sentry_symbols.sh "$VERSION" "$dif_dir" - else - echo "==> Skipping $dif_dir (not present)" + # Hard-fail when the target dir is missing instead of silently + # skipping (#1403). If we got past `cargo tauri build` with + # SENTRY_AUTH_TOKEN set and this directory doesn't exist, the + # build is broken and shipping it would leak un-symbolicated + # crashes to production. + if [ ! -d "$dif_dir" ]; then + echo "::error::Tauri DIF dir not present: $dif_dir — cargo tauri build did not produce a target tree for ${MATRIX_TARGET}." >&2 + exit 1 fi + echo "==> Uploading symbols from $dif_dir to ${SENTRY_PROJECT}" + bash scripts/upload_sentry_symbols.sh "$VERSION" "$dif_dir" # The standalone openhuman-core CLI has its own `sentry::init` in # `src/main.rs` and reports to `openhuman-core`. Only built when the @@ -468,12 +487,16 @@ jobs: run: | set -euo pipefail dif_dir="target/${MATRIX_TARGET}/${PROFILE}" - if [ -d "$dif_dir" ]; then - echo "==> Uploading symbols from $dif_dir to ${SENTRY_PROJECT}" - bash scripts/upload_sentry_symbols.sh "$VERSION" "$dif_dir" - else - echo "==> Skipping $dif_dir (not present)" + # build_sidecar only runs `cargo build --bin openhuman` for the + # standalone CLI, so this dir must exist when we reach this step. + # Hard-fail on miss (#1403) so we never ship a sidecar with + # un-symbolicated production crashes. + if [ ! -d "$dif_dir" ]; then + echo "::error::Core CLI DIF dir not present: $dif_dir — sidecar cargo build did not produce a target tree for ${MATRIX_TARGET}." >&2 + exit 1 fi + echo "==> Uploading symbols from $dif_dir to ${SENTRY_PROJECT}" + bash scripts/upload_sentry_symbols.sh "$VERSION" "$dif_dir" # ---- Linux + Windows installer upload --------------------------------- # When uploading to a GH Release: push .deb / .AppImage / .msi / .exe diff --git a/app/src/services/__tests__/analytics.test.ts b/app/src/services/__tests__/analytics.test.ts index 3dac3269b..5bd5e374a 100644 --- a/app/src/services/__tests__/analytics.test.ts +++ b/app/src/services/__tests__/analytics.test.ts @@ -12,8 +12,9 @@ const hoisted = vi.hoisted(() => ({ functionToStringIntegration: vi.fn(() => ({})), linkedErrorsIntegration: vi.fn(() => ({})), dedupeIntegration: vi.fn(() => ({})), - browserApiErrorsIntegration: vi.fn(() => ({})), - globalHandlersIntegration: vi.fn(() => ({})), + browserApiErrorsIntegration: vi.fn(() => ({ name: 'BrowserApiErrors' })), + globalHandlersIntegration: vi.fn(() => ({ name: 'GlobalHandlers' })), + httpContextIntegration: vi.fn(() => ({ name: 'HttpContext' })), analyticsEnabled: false, appEnvironment: 'staging' as 'staging' | 'production' | 'development', })); @@ -29,6 +30,7 @@ vi.mock('@sentry/react', () => ({ dedupeIntegration: hoisted.dedupeIntegration, browserApiErrorsIntegration: hoisted.browserApiErrorsIntegration, globalHandlersIntegration: hoisted.globalHandlersIntegration, + httpContextIntegration: hoisted.httpContextIntegration, })); // `initSentry()` reads `getCoreStateSnapshot().snapshot.analyticsEnabled` to @@ -157,7 +159,12 @@ describe('initSentry beforeSend manual-staging bypass', () => { message: 'something blew up', tags: { test: 'manual-staging' }, breadcrumbs: [{ message: 'should-be-stripped' }], - request: { url: 'https://api.example.com/secret' }, + request: { + url: 'https://api.example.com/secret', + cookies: 'session=abc', + data: { body: 'redacted' }, + headers: { 'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 14_0)' }, + }, extra: { token: 'redacted-please' }, contexts: { os: { name: 'macOS' }, app: { build: '123' } }, }) as Record | null; @@ -165,7 +172,15 @@ describe('initSentry beforeSend manual-staging bypass', () => { expect(result).not.toBeNull(); // PII / breadcrumbs / request body / extras must all be stripped. expect((result as { breadcrumbs: unknown[] }).breadcrumbs).toEqual([]); - expect(result).not.toHaveProperty('request'); + // Request envelope is narrowed to the User-Agent header only — keeping + // it lets Sentry's relay populate os/browser/device (#1403); URL, + // cookies, and body are dropped. + const req = (result as { request?: { headers?: Record; url?: string } }) + .request; + expect(req?.headers).toEqual({ 'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 14_0)' }); + expect(req).not.toHaveProperty('url'); + expect(req).not.toHaveProperty('cookies'); + expect(req).not.toHaveProperty('data'); expect(result).not.toHaveProperty('extra'); // `app` context is stripped — only os/browser/device kept. expect((result as { contexts: Record }).contexts).not.toHaveProperty('app'); @@ -183,6 +198,65 @@ describe('initSentry beforeSend manual-staging bypass', () => { expect(result).not.toBeNull(); }); + test('forwards release tag and registers httpContextIntegration (#1403)', async () => { + // Regression for #1403: production events arrived in Sentry with no + // `release` tag and no `os` context. The release must reach Sentry.init + // verbatim from `SENTRY_RELEASE`, and `httpContextIntegration` must be + // present so the User-Agent header is attached and the relay can derive + // `os` / `browser` / `device` server-side. + hoisted.init.mockReset(); + const { initSentry } = await import('../analytics'); + initSentry(); + + const opts = hoisted.init.mock.calls[0][0] as { + release: string; + integrations: Array<{ name?: string }>; + }; + expect(opts.release).toBe('openhuman@test+abc'); + const names = opts.integrations.map(i => i.name).filter(Boolean); + expect(names).toContain('HttpContext'); + }); + + test('keeps os/browser/device contexts and forwards them through beforeSend (#1403)', async () => { + hoisted.analyticsEnabled = true; // consent on so beforeSend doesn't drop. + const beforeSend = await captureBeforeSend(); + const result = beforeSend({ + message: 'real prod error', + tags: {}, + contexts: { + os: { name: 'macOS', version: '14.0' }, + browser: { name: 'Chrome', version: '119' }, + device: { family: 'Mac' }, + // Anything other than os/browser/device must be dropped by the + // privacy filter — if a future edit accidentally widens the + // allowlist, this assertion fails. + state: { redux: 'should-not-leak' }, + }, + }) as { contexts: Record } | null; + + expect(result).not.toBeNull(); + expect(result!.contexts).toMatchObject({ + os: { name: 'macOS', version: '14.0' }, + browser: { name: 'Chrome', version: '119' }, + device: { family: 'Mac' }, + }); + expect(result!.contexts).not.toHaveProperty('state'); + }); + + test('drops the entire request envelope when no User-Agent header is present', async () => { + hoisted.analyticsEnabled = true; + const beforeSend = await captureBeforeSend(); + const result = beforeSend({ + message: 'no-ua event', + tags: {}, + contexts: {}, + request: { url: 'https://leak/secret', headers: { 'X-Other': 'meh' } }, + }) as Record | null; + + expect(result).not.toBeNull(); + expect(result!.request).toBeUndefined(); + }); + test('drops manual-staging tagged events in production even with the tag', async () => { // Defense in depth: a stray `tags.test = 'manual-staging'` in production // must NOT bypass the consent gate. Capture beforeSend in staging, then diff --git a/app/src/services/analytics.ts b/app/src/services/analytics.ts index c378e4d08..62bea60c1 100644 --- a/app/src/services/analytics.ts +++ b/app/src/services/analytics.ts @@ -53,6 +53,14 @@ export function initSentry(): void { Sentry.dedupeIntegration(), Sentry.browserApiErrorsIntegration(), Sentry.globalHandlersIntegration(), + // #1403: production events were missing `os.name` / `browser.name` / + // `device.family` because Sentry derives those by parsing the + // User-Agent header server-side, and `defaultIntegrations: false` + // (above) drops the integration that attaches `event.request.headers`. + // Re-include it explicitly so platform context comes back. `beforeSend` + // narrows what survives from the request envelope (headers only, UA + // only) to keep this aligned with the privacy contract. + Sentry.httpContextIntegration(), ], sendDefaultPii: false, @@ -74,7 +82,12 @@ export function initSentry(): void { // Strip anything that could carry Redux / localStorage / request bodies. event.breadcrumbs = []; - delete event.request; + // Keep only the User-Agent header so Sentry's server-side relay can + // populate `os` / `browser` / `device` contexts (#1403). Drop URL, + // query string, cookies, and request body — anything that could leak + // user content or session state. + const ua = (event.request?.headers as Record | undefined)?.['User-Agent']; + event.request = ua ? { headers: { 'User-Agent': ua } } : undefined; delete event.extra; event.contexts = { os: event.contexts?.os, diff --git a/scripts/release/verify-sentry-sourcemaps.mjs b/scripts/release/verify-sentry-sourcemaps.mjs new file mode 100755 index 000000000..c1860096d --- /dev/null +++ b/scripts/release/verify-sentry-sourcemaps.mjs @@ -0,0 +1,78 @@ +#!/usr/bin/env node +// Post-build guard for #1403: verify that @sentry/vite-plugin actually +// uploaded source maps and injected debug-IDs into the production bundle. +// +// Failure modes this catches: +// - SENTRY_AUTH_TOKEN missing at build time -> plugin returned null and +// nothing was uploaded (bundle has no debug-ID comments). +// - sourcemap.assets glob mismatched cwd -> plugin logged "Didn't find +// any matching sources for debug ID upload" and exited 0; bundle has +// no debug-IDs and Sentry can't symbolicate. +// +// Run after `cargo tauri build` (which invokes Vite). Exits non-zero if +// fewer than `MIN_DEBUG_IDS` JS chunks under app/dist/assets/ contain a +// `// debugId=...` comment, or none of them reference `_sentryDebugIds`. +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, resolve } from 'node:path'; + +const ROOT = resolve(new URL('..', import.meta.url).pathname, '..'); +const ASSETS = join(ROOT, 'app', 'dist', 'assets'); +const MIN_DEBUG_IDS = 1; +// `// debugId=` is the marker @sentry/vite-plugin appends to every +// chunk it processes. The plugin also writes `globalThis._sentryDebugIds` +// at startup so the runtime can match captured stacks to uploaded maps. +const DEBUG_ID_RE = /\/\/[ #]?\s*debugId=[0-9a-f-]{36}/i; +const RUNTIME_MAP_RE = /_sentryDebugIds/; + +function listJsFiles(dir) { + let out = []; + for (const entry of readdirSync(dir)) { + const p = join(dir, entry); + const st = statSync(p); + if (st.isDirectory()) out = out.concat(listJsFiles(p)); + else if (entry.endsWith('.js')) out.push(p); + } + return out; +} + +function main() { + let files; + try { + files = listJsFiles(ASSETS); + } catch (err) { + console.error(`[verify-sentry-sourcemaps] ${ASSETS} not found — did Vite build succeed?`); + console.error(err.message); + process.exit(2); + } + + if (files.length === 0) { + console.error(`[verify-sentry-sourcemaps] no .js files under ${ASSETS}`); + process.exit(2); + } + + let withDebugId = 0; + let withRuntimeMap = 0; + for (const f of files) { + const src = readFileSync(f, 'utf8'); + if (DEBUG_ID_RE.test(src)) withDebugId += 1; + if (RUNTIME_MAP_RE.test(src)) withRuntimeMap += 1; + } + + console.log( + `[verify-sentry-sourcemaps] scanned ${files.length} files; ${withDebugId} carry debug-IDs; ${withRuntimeMap} reference _sentryDebugIds.` + ); + + if (withDebugId < MIN_DEBUG_IDS || withRuntimeMap < 1) { + console.error( + '[verify-sentry-sourcemaps] FAIL — Sentry source-map upload did not run or did not inject debug-IDs.\n' + + ' Likely causes:\n' + + ' - SENTRY_AUTH_TOKEN missing/empty at vite build time\n' + + ' - sourcemap.assets glob did not match dist/assets\n' + + ' - SENTRY_RELEASE / VITE_BUILD_SHA mismatch between upload and runtime\n' + + ' Without debug-IDs, production stack traces cannot be symbolicated. (#1403)' + ); + process.exit(1); + } +} + +main(); diff --git a/scripts/upload_sentry_symbols.sh b/scripts/upload_sentry_symbols.sh index 2c10855d2..971fc3152 100644 --- a/scripts/upload_sentry_symbols.sh +++ b/scripts/upload_sentry_symbols.sh @@ -234,17 +234,51 @@ upload_symbols() { "--log-level=warn" ) - # Find and upload all debug symbol files - if [[ -d "${symbols_path}" ]]; then - # Upload .dwp (dwarf packages), .debug files, and .pdb files - # Debug symbols are indexed by debug-ID, not release-scoped - log_info "Scanning for debug symbols in ${symbols_path}..." - sentry-cli "${upload_args[@]}" "${symbols_path}" || { - log_warn "Some debug symbols may have failed to upload" - } - else - log_warn "Symbols path does not exist: ${symbols_path}" - log_info "Looking for any release artifacts..." + # Find and upload all debug symbol files. The output is captured so the + # script can verify *something* was actually uploaded — sentry-cli exits + # 0 even when it found zero DIFs, which silently breaks symbolication + # for the Tauri shell and standalone core CLI exactly the way #1403 + # caught for the frontend. Fail loudly here so CI catches it on the + # build that produced the empty target dir, not weeks later when an + # event arrives unsymbolicated. + if [[ ! -d "${symbols_path}" ]]; then + log_error "Symbols path does not exist: ${symbols_path}" + log_error "Expected Cargo target dir with build artifacts. Did the build step complete?" + exit 1 + fi + + log_info "Scanning for debug symbols in ${symbols_path}..." + local upload_log + upload_log="$(mktemp)" + if ! sentry-cli "${upload_args[@]}" "${symbols_path}" 2>&1 | tee "${upload_log}"; then + log_error "sentry-cli upload-dif exited non-zero" + rm -f "${upload_log}" + exit 1 + fi + + # sentry-cli prints "Found N debug information files" when scanning, and + # "Uploaded N missing debug information files" / "No new debug + # information files to upload" after the upload phase. We accept either + # "Found > 0" or "Uploaded > 0" — the second covers re-runs where the + # DIFs are already on Sentry's side. Empty input dirs print neither and + # fall through to the failure branch. + local found=0 uploaded=0 already=0 + if grep -qE 'Found [1-9][0-9]* debug information' "${upload_log}"; then + found=1 + fi + if grep -qE 'Uploaded [1-9][0-9]* missing debug information' "${upload_log}"; then + uploaded=1 + fi + if grep -qE 'No new debug information files to upload|already exist' "${upload_log}"; then + already=1 + fi + rm -f "${upload_log}" + + if [[ "${found}" -eq 0 && "${uploaded}" -eq 0 && "${already}" -eq 0 ]]; then + log_error "sentry-cli upload-dif found zero debug information files in ${symbols_path}" + log_error "Production Sentry events from this build will NOT be symbolicated. (#1403)" + log_error "Likely causes: build profile produced no DWARF/PDB/dSYM, wrong target dir, or ${symbols_path} was cleaned before this step." + exit 1 fi # Finalize the release