fix(security): redact gallery share tokens from analytics tracking (GHSA-7m6c) (stable) (#953)
* 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 <Router>), 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 <AnalyticsRouteTracker/> INSIDE <Router> so manual tracking runs. --------- Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
co-authored by
Paul Nothaft
Paul Nothaft
parent
5d5db4e766
commit
2c7b5dfd02
@@ -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() {
|
||||
<DynamicFavicon />
|
||||
<RobotsMetaTags />
|
||||
<Router>
|
||||
<AnalyticsRouteTracker />
|
||||
<MaintenanceWrapper>
|
||||
<SkipLink />
|
||||
<Routes>
|
||||
|
||||
@@ -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 <Router> (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;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user