From 2c7b5dfd020ac1fb2acdb7667998b2f373ce99d9 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Sun, 2 Aug 2026 08:39:40 +0200 Subject: [PATCH] fix(security): redact gallery share tokens from analytics tracking (GHSA-7m6c) (stable) (#953) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): redact gallery share tokens from analytics page-view tracking (GHSA-7m6c) * fix(security): codex round-1 — actually disable raw auto-tracking (GHSA-7m6c) The previous patch was inert: App.tsx passed autoTrack:true (so Umami's data-auto-track=false was never set) and the sanitized trackPageView had no caller (useAnalytics sits outside ), so the raw token URL still hit the collector. - Umami: drop autoTrack:true → data-auto-track=false; page views now come from a sanitized manual tracker. - Rybbit: its initial-load auto pageview can't be intercepted client-side, so use native data-mask-patterns=['/gallery/**'] to strip the token on every auto-tracked view; skip manual tracking for it to avoid double counting. - Mount INSIDE so manual tracking runs. --------- Co-authored-by: Paul Nothaft --- frontend/src/App.tsx | 13 +++-- frontend/src/services/analytics.service.ts | 64 +++++++++++++++++++--- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 854369a9..99ab31de 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -3,7 +3,7 @@ import { BrowserRouter as Router, Routes, Route, Navigate, useParams } from 'rea import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { ToastContainer } from 'react-toastify'; import 'react-toastify/dist/ReactToastify.css'; -import { analyticsService } from './services/analytics.service'; +import { analyticsService, AnalyticsRouteTracker } from './services/analytics.service'; import { GalleryAuthProvider, MaintenanceProvider } from './contexts'; import { ThemeProvider } from './contexts/ThemeContext'; @@ -115,8 +115,10 @@ function AnalyticsBootstrap() { provider: 'rybbit', hostUrl: settings.rybbit_url, websiteId: settings.rybbit_website_id, - autoTrack: true, doNotTrack: true, + // Mask every /gallery/* path (they embed the share token) so Rybbit's + // auto-tracked page views never carry the secret (GHSA-7m6c). + maskPatterns: ['/gallery/**'], }); return; } @@ -138,7 +140,9 @@ function AnalyticsBootstrap() { provider: 'umami', hostUrl: settings.umami_url, websiteId: settings.umami_website_id, - autoTrack: true, + // autoTrack omitted → data-auto-track="false": Umami must NOT read the + // raw window.location (token leak). Page views come from the manual, + // sanitized AnalyticsRouteTracker instead (GHSA-7m6c). doNotTrack: true, }); return; @@ -151,7 +155,7 @@ function AnalyticsBootstrap() { provider: 'umami', hostUrl: envUmamiUrl, websiteId: envUmamiWebsiteId, - autoTrack: true, + // autoTrack omitted → data-auto-track="false" (see above, GHSA-7m6c). doNotTrack: true, }); } @@ -194,6 +198,7 @@ function App() { + diff --git a/frontend/src/services/analytics.service.ts b/frontend/src/services/analytics.service.ts index 21317678..2d47d726 100644 --- a/frontend/src/services/analytics.service.ts +++ b/frontend/src/services/analytics.service.ts @@ -29,6 +29,9 @@ interface RybbitInitConfig extends BaseInitConfig { provider: 'rybbit'; websiteId: string; hostUrl: string; + // URL path patterns whose value must never reach the collector (they embed + // the gallery share token). Rendered into Rybbit's data-mask-patterns. + maskPatterns?: string[]; } interface CustomInitConfig extends BaseInitConfig { @@ -56,7 +59,7 @@ declare global { }; rybbit?: { event: (eventName: string, eventData?: any) => void; - pageview?: () => void; + pageview?: (path?: string) => void; }; } } @@ -85,7 +88,12 @@ class AnalyticsService { script.defer = true; script.src = `${config.hostUrl.replace(/\/+$/, '')}/script.js`; script.setAttribute('data-website-id', config.websiteId); - if (config.autoTrack === false) script.setAttribute('data-auto-track', 'false'); + // Auto-track OFF by default (GHSA-7m6c): Umami's auto page-view capture + // reads window.location verbatim, so a gallery URL /gallery/:slug/:token + // would ship the secret share token to the analytics collector. Page + // views are fired manually through trackPageView(), which redacts the + // token. Only an explicit autoTrack:true opts back into raw capture. + if (config.autoTrack !== true) script.setAttribute('data-auto-track', 'false'); if (config.doNotTrack !== false) script.setAttribute('data-do-not-track', 'true'); if (config.domains?.length) script.setAttribute('data-domains', config.domains.join(',')); document.head.appendChild(script); @@ -100,6 +108,16 @@ class AnalyticsService { script.defer = true; script.src = `${config.hostUrl.replace(/\/+$/, '')}/api/script.js`; script.setAttribute('data-site-id', config.websiteId); + // GHSA-7m6c: Rybbit auto-tracks page views (initial load + SPA route + // changes) reading window.location, so a gallery URL would ship the raw + // share token. Unlike Umami we CAN'T fix this with a manual tracker — + // the initial-load pageview fires before any of our code runs. Instead + // use Rybbit's native data-mask-patterns, which replaces matching paths + // with the pattern string in analytics, stripping the token on every + // auto-tracked pageview including the first. + if (config.maskPatterns?.length) { + script.setAttribute('data-mask-patterns', JSON.stringify(config.maskPatterns)); + } document.head.appendChild(script); } else if (config.provider === 'custom') { // The admin-pasted HTML is sanitised server-side (see @@ -148,13 +166,35 @@ class AnalyticsService { // 'none' / 'custom' / unloaded → silently ignore. } + // Redact secrets from a URL before it reaches the analytics collector + // (GHSA-7m6c): drop the query string entirely and replace token-looking + // path segments (long hex / opaque IDs — e.g. the gallery share token in + // /gallery/:slug/:token) with a placeholder. Failing safe: on any parse + // issue return just the pathname without the query. + private sanitizeTrackedUrl(url: string): string { + try { + const pathOnly = url.split('?')[0].split('#')[0]; + return pathOnly + .split('/') + .map((seg) => + /^[0-9a-fA-F]{16,}$/.test(seg) || /^[A-Za-z0-9_-]{20,}$/.test(seg) ? '[redacted]' : seg) + .join('/'); + } catch { + return url.split('?')[0]; + } + } + trackPageView(url?: string, referrer?: string) { if (!this.initialized) return; - if (this.provider === 'umami' && typeof window !== 'undefined' && window.umami) { - window.umami.trackView(url, referrer, this.websiteId || undefined); - } else if (this.provider === 'rybbit' && typeof window !== 'undefined' && window.rybbit?.pageview) { - window.rybbit.pageview(); - } + // Only Umami is manually tracked here: its auto-track is disabled (so the + // raw token URL never hits the collector) and this sanitized call is the + // ONLY page-view source. Rybbit keeps its own auto-tracking with + // data-mask-patterns doing the redaction, so a manual call would + // double-count — skip it. 'none'/'custom' have no page-view API. + if (this.provider !== 'umami' || typeof window === 'undefined' || !window.umami) return; + const raw = url ?? window.location.pathname; + const safe = this.sanitizeTrackedUrl(raw); + window.umami.trackView(safe, referrer, this.websiteId || undefined); } // Gallery-specific tracking events @@ -209,3 +249,13 @@ export const useAnalytics = () => { return analyticsService; }; + +// Renderless component that drives manual page-view tracking. MUST be mounted +// INSIDE (useLocation needs router context) — that's why the +// AnalyticsBootstrap init, which lives outside the Router, can't do this +// itself. Without a mounted caller trackPageView never fires and Umami — whose +// auto-track we deliberately disable — records nothing. +export const AnalyticsRouteTracker = (): null => { + useAnalytics(); + return null; +};