From cee4bef17903737da75be503f22b86b17863be45 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Sun, 19 Apr 2026 23:16:34 +0200 Subject: [PATCH] fix: harden shipped telemetry config --- .github/workflows/release-stable.yml | 52 +++++++++++++++++++++ .github/workflows/release.yml | 52 +++++++++++++++++++++ src-tauri/src/telemetry.rs | 62 +++++++++++++++++++++++-- src/lib/telemetry.ts | 45 +++++++++++-------- src/lib/telemetryConfig.test.ts | 59 ++++++++++++++++++++++++ src/lib/telemetryConfig.ts | 67 ++++++++++++++++++++++++++++ 6 files changed, 314 insertions(+), 23 deletions(-) create mode 100644 src/lib/telemetryConfig.test.ts create mode 100644 src/lib/telemetryConfig.ts diff --git a/.github/workflows/release-stable.yml b/.github/workflows/release-stable.yml index c882e284..5a61687e 100644 --- a/.github/workflows/release-stable.yml +++ b/.github/workflows/release-stable.yml @@ -127,6 +127,58 @@ jobs: security set-key-partition-list -S apple-tool:,apple: -s -k "$KEYCHAIN_PASSWORD" "$KEYCHAIN_PATH" echo "KEYCHAIN_PATH=$KEYCHAIN_PATH" >> "$GITHUB_ENV" + - name: Validate telemetry env + env: + VITE_SENTRY_DSN: ${{ secrets.VITE_SENTRY_DSN }} + SENTRY_DSN: ${{ secrets.SENTRY_DSN }} + VITE_POSTHOG_KEY: ${{ secrets.VITE_POSTHOG_KEY }} + VITE_POSTHOG_HOST: ${{ secrets.VITE_POSTHOG_HOST }} + run: | + python3 <<'PY' + import os + import sys + from urllib.parse import urlparse + + def normalize(name: str) -> str: + value = os.getenv(name, "").strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in ("'", '"'): + value = value[1:-1].strip() + return value + + def is_http_url(value: str) -> bool: + parsed = urlparse(value) + return parsed.scheme in {"http", "https"} and bool(parsed.netloc) + + values = { + name: normalize(name) + for name in ( + "VITE_SENTRY_DSN", + "SENTRY_DSN", + "VITE_POSTHOG_KEY", + "VITE_POSTHOG_HOST", + ) + } + errors = [] + + for name in ("VITE_SENTRY_DSN", "SENTRY_DSN", "VITE_POSTHOG_HOST"): + value = values[name] + if not value: + errors.append(f"{name} must be set for release builds") + elif not is_http_url(value): + errors.append(f"{name} must be a valid http(s) URL") + + if not values["VITE_POSTHOG_KEY"]: + errors.append("VITE_POSTHOG_KEY must be set for release builds") + + if errors: + print("Telemetry env validation failed:", file=sys.stderr) + for error in errors: + print(f"- {error}", file=sys.stderr) + raise SystemExit(1) + + print("Telemetry env validation passed.") + PY + - name: Build Tauri app (with signing + notarization) env: TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a0a8ed58..ee424091 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -170,6 +170,58 @@ jobs: security set-key-partition-list -S apple-tool:,apple: -s -k "$KEYCHAIN_PASSWORD" "$KEYCHAIN_PATH" echo "KEYCHAIN_PATH=$KEYCHAIN_PATH" >> "$GITHUB_ENV" + - name: Validate telemetry env + env: + VITE_SENTRY_DSN: ${{ secrets.VITE_SENTRY_DSN }} + SENTRY_DSN: ${{ secrets.SENTRY_DSN }} + VITE_POSTHOG_KEY: ${{ secrets.VITE_POSTHOG_KEY }} + VITE_POSTHOG_HOST: ${{ secrets.VITE_POSTHOG_HOST }} + run: | + python3 <<'PY' + import os + import sys + from urllib.parse import urlparse + + def normalize(name: str) -> str: + value = os.getenv(name, "").strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in ("'", '"'): + value = value[1:-1].strip() + return value + + def is_http_url(value: str) -> bool: + parsed = urlparse(value) + return parsed.scheme in {"http", "https"} and bool(parsed.netloc) + + values = { + name: normalize(name) + for name in ( + "VITE_SENTRY_DSN", + "SENTRY_DSN", + "VITE_POSTHOG_KEY", + "VITE_POSTHOG_HOST", + ) + } + errors = [] + + for name in ("VITE_SENTRY_DSN", "SENTRY_DSN", "VITE_POSTHOG_HOST"): + value = values[name] + if not value: + errors.append(f"{name} must be set for release builds") + elif not is_http_url(value): + errors.append(f"{name} must be a valid http(s) URL") + + if not values["VITE_POSTHOG_KEY"]: + errors.append("VITE_POSTHOG_KEY must be set for release builds") + + if errors: + print("Telemetry env validation failed:", file=sys.stderr) + for error in errors: + print(f"- {error}", file=sys.stderr) + raise SystemExit(1) + + print("Telemetry env validation passed.") + PY + - name: Build Tauri app (with signing + notarization) env: TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} diff --git a/src-tauri/src/telemetry.rs b/src-tauri/src/telemetry.rs index 7cf07f51..2528aece 100644 --- a/src-tauri/src/telemetry.rs +++ b/src-tauri/src/telemetry.rs @@ -11,6 +11,30 @@ fn scrub_paths(input: &str) -> String { re.replace_all(input, "").to_string() } +fn normalize_embedded_env(raw: Option<&str>) -> Option { + let value = raw?.trim(); + if value.is_empty() { + return None; + } + + let unwrapped = match (value.chars().next(), value.chars().last()) { + (Some('"'), Some('"')) | (Some('\''), Some('\'')) if value.len() >= 2 => { + value[1..value.len() - 1].trim() + } + _ => value, + }; + + if unwrapped.is_empty() { + None + } else { + Some(unwrapped.to_string()) + } +} + +fn parse_embedded_sentry_dsn(raw: Option<&str>) -> Option { + normalize_embedded_env(raw)?.parse().ok() +} + /// Initialize Sentry if the user has opted in to crash reporting. /// Returns `true` if Sentry was initialized, `false` if skipped. pub fn init_sentry_from_settings() -> bool { @@ -23,14 +47,13 @@ pub fn init_sentry_from_settings() -> bool { return false; } - let dsn = option_env!("SENTRY_DSN").unwrap_or_default(); - if dsn.is_empty() { + let Some(dsn) = parse_embedded_sentry_dsn(option_env!("SENTRY_DSN")) else { return false; - } + }; let anonymous_id = settings.anonymous_id.unwrap_or_default(); let guard = sentry::init(sentry::ClientOptions { - dsn: dsn.parse().ok(), + dsn: Some(dsn), release: Some(env!("CARGO_PKG_VERSION").into()), send_default_pii: false, before_send: Some(std::sync::Arc::new(|mut event| { @@ -86,6 +109,37 @@ mod tests { assert_eq!(scrub_paths("Normal error message"), "Normal error message"); } + #[test] + fn test_normalize_embedded_env_trims_wrapping_quotes() { + assert_eq!( + normalize_embedded_env(Some(" \"value\" ")).as_deref(), + Some("value") + ); + assert_eq!( + normalize_embedded_env(Some(" 'value' ")).as_deref(), + Some("value") + ); + } + + #[test] + fn test_normalize_embedded_env_drops_blank_values() { + assert_eq!(normalize_embedded_env(Some(" ")), None); + assert_eq!(normalize_embedded_env(None), None); + } + + #[test] + fn test_parse_embedded_sentry_dsn_accepts_valid_trimmed_value() { + let parsed = + parse_embedded_sentry_dsn(Some(" \"https://public@example.ingest.sentry.io/1\" ")); + assert!(parsed.is_some()); + } + + #[test] + fn test_parse_embedded_sentry_dsn_rejects_invalid_value() { + let parsed = parse_embedded_sentry_dsn(Some("not a dsn")); + assert!(parsed.is_none()); + } + #[test] fn test_init_sentry_returns_false_without_dsn() { // Without SENTRY_DSN env var set at compile time, init should return false diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 34f4b2cb..965d40cd 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -1,8 +1,5 @@ import * as Sentry from '@sentry/react' - -const SENTRY_DSN = import.meta.env.VITE_SENTRY_DSN ?? '' -const POSTHOG_KEY = import.meta.env.VITE_POSTHOG_KEY ?? '' -const POSTHOG_HOST = import.meta.env.VITE_POSTHOG_HOST ?? 'https://us.i.posthog.com' +import { resolveFrontendTelemetryConfig } from './telemetryConfig' /** Pattern that matches absolute file paths (macOS / Linux / Windows). */ const PATH_PATTERN = /(?:\/[\w.-]+){2,}|[A-Z]:\\[\w\\.-]+/g @@ -11,24 +8,30 @@ function scrubPaths(input: string): string { return input.replace(PATH_PATTERN, '') } +function scrubSentryEvent(event: Sentry.ErrorEvent): Sentry.ErrorEvent { + if (event.message) event.message = scrubPaths(event.message) + for (const ex of event.exception?.values ?? []) { + if (ex.value) ex.value = scrubPaths(ex.value) + } + for (const breadcrumb of event.breadcrumbs ?? []) { + if (breadcrumb.message) breadcrumb.message = scrubPaths(breadcrumb.message) + } + return event +} + let sentryInitialized = false let posthogInstance: typeof import('posthog-js').default | null = null export function initSentry(anonymousId: string): void { - if (sentryInitialized || !SENTRY_DSN) return + if (sentryInitialized) return + + const { sentryDsn } = resolveFrontendTelemetryConfig() + if (!sentryDsn) return + Sentry.init({ - dsn: SENTRY_DSN, + dsn: sentryDsn, sendDefaultPii: false, - beforeSend(event) { - if (event.message) event.message = scrubPaths(event.message) - for (const ex of event.exception?.values ?? []) { - if (ex.value) ex.value = scrubPaths(ex.value) - } - for (const bc of event.breadcrumbs ?? []) { - if (bc.message) bc.message = scrubPaths(bc.message) - } - return event - }, + beforeSend: scrubSentryEvent, }) Sentry.setUser({ id: anonymousId }) sentryInitialized = true @@ -41,10 +44,14 @@ export function teardownSentry(): void { } export async function initPostHog(anonymousId: string, releaseChannel?: string): Promise { - if (posthogInstance || !POSTHOG_KEY) return + if (posthogInstance) return + + const { posthogKey, posthogHost } = resolveFrontendTelemetryConfig() + if (!posthogKey || !posthogHost) return + const posthog = (await import('posthog-js')).default - posthog.init(POSTHOG_KEY, { - api_host: POSTHOG_HOST, + posthog.init(posthogKey, { + api_host: posthogHost, autocapture: false, capture_pageview: false, persistence: 'memory', diff --git a/src/lib/telemetryConfig.test.ts b/src/lib/telemetryConfig.test.ts new file mode 100644 index 00000000..c74b839f --- /dev/null +++ b/src/lib/telemetryConfig.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest' +import { + _defaultPostHogHostForTest as defaultPostHogHost, + resolveFrontendTelemetryConfig, + sanitizeTelemetryEnvValue, +} from './telemetryConfig' + +describe('sanitizeTelemetryEnvValue', () => { + it('trims surrounding whitespace', () => { + expect(sanitizeTelemetryEnvValue(' value ')).toBe('value') + }) + + it('unwraps matching quotes after trimming', () => { + expect(sanitizeTelemetryEnvValue(' "value" ')).toBe('value') + expect(sanitizeTelemetryEnvValue(" 'value' ")).toBe('value') + }) + + it('returns an empty string for blank input', () => { + expect(sanitizeTelemetryEnvValue(' ')).toBe('') + expect(sanitizeTelemetryEnvValue(undefined)).toBe('') + }) +}) + +describe('resolveFrontendTelemetryConfig', () => { + it('keeps valid telemetry values after sanitizing them', () => { + expect(resolveFrontendTelemetryConfig({ + VITE_SENTRY_DSN: ' "https://public@example.ingest.sentry.io/123456" ', + VITE_POSTHOG_KEY: " 'phc_test_key' ", + VITE_POSTHOG_HOST: ' https://eu.i.posthog.com ', + })).toEqual({ + sentryDsn: 'https://public@example.ingest.sentry.io/123456', + posthogKey: 'phc_test_key', + posthogHost: 'https://eu.i.posthog.com', + }) + }) + + it('uses the default PostHog host when one is not configured', () => { + expect(resolveFrontendTelemetryConfig({ + VITE_SENTRY_DSN: 'https://public@example.ingest.sentry.io/123456', + VITE_POSTHOG_KEY: 'phc_test_key', + }).posthogHost).toBe(defaultPostHogHost) + }) + + it('drops invalid Sentry DSNs instead of passing them to the SDK', () => { + expect(resolveFrontendTelemetryConfig({ + VITE_SENTRY_DSN: 'not a dsn', + VITE_POSTHOG_KEY: 'phc_test_key', + VITE_POSTHOG_HOST: 'https://eu.i.posthog.com', + }).sentryDsn).toBe('') + }) + + it('drops invalid PostHog hosts instead of loading scripts from them', () => { + expect(resolveFrontendTelemetryConfig({ + VITE_SENTRY_DSN: 'https://public@example.ingest.sentry.io/123456', + VITE_POSTHOG_KEY: 'phc_test_key', + VITE_POSTHOG_HOST: 'not a url', + }).posthogHost).toBeNull() + }) +}) diff --git a/src/lib/telemetryConfig.ts b/src/lib/telemetryConfig.ts new file mode 100644 index 00000000..e742a375 --- /dev/null +++ b/src/lib/telemetryConfig.ts @@ -0,0 +1,67 @@ +const DEFAULT_POSTHOG_HOST = 'https://us.i.posthog.com' + +type TelemetryEnv = { + VITE_SENTRY_DSN?: string + VITE_POSTHOG_KEY?: string + VITE_POSTHOG_HOST?: string +} + +export type FrontendTelemetryConfig = { + sentryDsn: string + posthogKey: string + posthogHost: string | null +} + +function unwrapMatchingQuotes(value: string): string { + if (value.length < 2) return value + + const first = value[0] + const last = value[value.length - 1] + if (first !== last) return value + if (first !== '"' && first !== "'") return value + + return value.slice(1, -1).trim() +} + +export function sanitizeTelemetryEnvValue(value: string | undefined): string { + if (!value) return '' + + const trimmed = value.trim() + if (!trimmed) return '' + + return unwrapMatchingQuotes(trimmed) +} + +function isHttpUrl(value: string): boolean { + try { + const url = new URL(value) + return url.protocol === 'http:' || url.protocol === 'https:' + } catch { + return false + } +} + +function normalizeSentryDsn(value: string): string { + return isHttpUrl(value) ? value : '' +} + +function normalizePostHogHost(value: string): string | null { + if (!value) return DEFAULT_POSTHOG_HOST + return isHttpUrl(value) ? value : null +} + +export function resolveFrontendTelemetryConfig( + env: TelemetryEnv = import.meta.env as TelemetryEnv, +): FrontendTelemetryConfig { + const sentryDsn = normalizeSentryDsn( + sanitizeTelemetryEnvValue(env.VITE_SENTRY_DSN), + ) + const posthogKey = sanitizeTelemetryEnvValue(env.VITE_POSTHOG_KEY) + const posthogHost = normalizePostHogHost( + sanitizeTelemetryEnvValue(env.VITE_POSTHOG_HOST), + ) + + return { sentryDsn, posthogKey, posthogHost } +} + +export { DEFAULT_POSTHOG_HOST as _defaultPostHogHostForTest }