Skip to content

fix(security): prevent XSS in proxy endpoint via json.dumps() #3170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MarkusNeusinger wants to merge 4 commits into from
Closed

fix(security): prevent XSS in proxy endpoint via json.dumps() #3170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07b7b4b
docs: add comprehensive Plausible Analytics tracking documentation
claude Jan 5, 2026
135c88a
feat(analytics): enhance Plausible tracking with journey context and …
MarkusNeusinger Jan 5, 2026
359b8e1
docs: fix debounce time in plausible docs (500ms → 200ms)
MarkusNeusinger Jan 5, 2026
7deb69c
fix(security): prevent XSS in proxy endpoint via json.dumps()
MarkusNeusinger Jan 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions api/routers/proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""HTML proxy endpoint for interactive plots with size reporting."""

import json
from urllib.parse import urlparse

import httpx
Expand All @@ -14,7 +15,13 @@


def get_size_reporter_script(target_origin: str) -> str:
"""Generate size reporter script with specified target origin."""
"""Generate size reporter script with specified target origin.

Uses json.dumps() to safely encode the origin string for JavaScript context,
preventing XSS even if the origin contained special characters.
"""
# Safely encode for JavaScript string context (includes quotes)
safe_origin = json.dumps(target_origin)
return f"""
<script>
(function() {{
Expand All @@ -41,7 +48,7 @@ def get_size_reporter_script(target_origin: str) -> str:
type: 'pyplots-size',
width: Math.ceil(width),
height: Math.ceil(height)
}}, '{target_origin}');
}}, {safe_origin});
}}
}} catch (e) {{
// Silently fail if postMessage is blocked
Expand Down
14 changes: 11 additions & 3 deletions app/src/components/FilterBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export function FilterBar({
const timer = setTimeout(() => {
onTrackEvent('search_no_results', { query });
lastTrackedQueryRef.current = query;
}, 500);
}, 200);
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change reduces the debounce time from 500ms to 200ms for tracking search queries with no results. While this may improve analytics responsiveness, it's unrelated to the XSS security fix in proxy.py. Analytics tuning should be in a separate PR, not bundled with a security fix.

Suggested change
}, 200);
}, 500);

Copilot uses AI. Check for mistakes.
return () => clearTimeout(timer);
}
}, [searchQuery, searchResults.length, onTrackEvent]);
Expand Down Expand Up @@ -378,7 +378,11 @@ export function FilterBar({
{/* Grid size toggle */}
<Tooltip title={imageSize === 'normal' ? 'compact view' : 'normal view'}>
<Box
onClick={() => onImageSizeChange(imageSize === 'normal' ? 'compact' : 'normal')}
onClick={() => {
const newSize = imageSize === 'normal' ? 'compact' : 'normal';
onImageSizeChange(newSize);
onTrackEvent('toggle_grid_size', { size: newSize });
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds toggle_grid_size event tracking. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Suggested change
onTrackEvent('toggle_grid_size', { size: newSize });

Copilot uses AI. Check for mistakes.
}}
sx={{
display: 'flex',
alignItems: 'center',
Expand Down Expand Up @@ -588,7 +592,11 @@ export function FilterBar({
{/* Grid size toggle */}
<Tooltip title={imageSize === 'normal' ? 'compact view' : 'normal view'}>
<Box
onClick={() => onImageSizeChange(imageSize === 'normal' ? 'compact' : 'normal')}
onClick={() => {
const newSize = imageSize === 'normal' ? 'compact' : 'normal';
onImageSizeChange(newSize);
onTrackEvent('toggle_grid_size', { size: newSize });
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds toggle_grid_size event tracking. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Suggested change
onTrackEvent('toggle_grid_size', { size: newSize });

Copilot uses AI. Check for mistakes.
}}
sx={{
display: 'flex',
alignItems: 'center',
Expand Down
2 changes: 1 addition & 1 deletion app/src/components/ImageCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const ImageCard = memo(function ImageCard({
if (code) {
await navigator.clipboard.writeText(code);
setCopyState('copied');
onTrackEvent?.('copy_code', { spec: image.spec_id, library: image.library, method: 'card' });
onTrackEvent?.('copy_code', { spec: image.spec_id, library: image.library, method: 'card', page: 'home' });
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds the page property to the copy_code event tracking. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Suggested change
onTrackEvent?.('copy_code', { spec: image.spec_id, library: image.library, method: 'card', page: 'home' });
onTrackEvent?.('copy_code', { spec: image.spec_id, library: image.library, method: 'card' });

Copilot uses AI. Check for mistakes.
setTimeout(() => setCopyState('idle'), 2000);
} else {
setCopyState('idle');
Expand Down
2 changes: 1 addition & 1 deletion app/src/components/SpecTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export function SpecTabs({
try {
await navigator.clipboard.writeText(code);
setCopied(true);
onTrackEvent?.('copy_code', { library: libraryId, method: 'tab' });
onTrackEvent?.('copy_code', { spec: specId, library: libraryId, method: 'tab', page: 'spec_detail' });
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds the spec and page properties to the copy_code event tracking. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Suggested change
onTrackEvent?.('copy_code', { spec: specId, library: libraryId, method: 'tab', page: 'spec_detail' });
onTrackEvent?.('copy_code', { library: libraryId, method: 'tab' });

Copilot uses AI. Check for mistakes.
setTimeout(() => setCopied(false), 2000);
} catch (err) {
console.error('Copy failed:', err);
Expand Down
2 changes: 1 addition & 1 deletion app/src/hooks/useAnalytics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function useAnalytics() {
[isProduction]
);

const trackPageview = useMemo(() => debounce(sendPageview, 300), [sendPageview]);
const trackPageview = useMemo(() => debounce(sendPageview, 150), [sendPageview]);
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change reduces the debounce time from 300ms to 150ms for pageview tracking. While this may improve analytics responsiveness, it's unrelated to the XSS security fix in proxy.py. Analytics tuning should be in a separate PR, not bundled with a security fix.

Suggested change
const trackPageview = useMemo(() => debounce(sendPageview, 150), [sendPageview]);
const trackPageview = useMemo(() => debounce(sendPageview, 300), [sendPageview]);

Copilot uses AI. Check for mistakes.

const trackEvent = useCallback(
(name: string, props?: EventProps) => {
Expand Down
20 changes: 14 additions & 6 deletions app/src/hooks/useFilterState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,24 +210,32 @@ export function useFilterState({

// Remove a filter value from a specific group
const handleRemoveFilter = useCallback((groupIndex: number, value: string) => {
const group = activeFiltersRef.current[groupIndex];
if (group) {
onTrackEvent('filter_remove', { category: group.category, value });
}
setActiveFilters((prev) => {
const newFilters = [...prev];
const group = newFilters[groupIndex];
if (!group) return prev;
const grp = newFilters[groupIndex];
if (!grp) return prev;

const updatedValues = group.values.filter((v) => v !== value);
const updatedValues = grp.values.filter((v) => v !== value);
if (updatedValues.length === 0) {
return newFilters.filter((_, i) => i !== groupIndex);
}
newFilters[groupIndex] = { ...group, values: updatedValues };
newFilters[groupIndex] = { ...grp, values: updatedValues };
return newFilters;
});
}, []);
}, [onTrackEvent]);
Comment on lines +213 to +229
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds filter_remove event tracking. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Copilot uses AI. Check for mistakes.

// Remove entire group by index
const handleRemoveGroup = useCallback((groupIndex: number) => {
const group = activeFiltersRef.current[groupIndex];
if (group) {
onTrackEvent('filter_remove', { category: group.category, value: group.values.join(',') });
}
setActiveFilters((prev) => prev.filter((_, i) => i !== groupIndex));
}, []);
}, [onTrackEvent]);
Comment on lines +233 to +238
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds filter_remove event tracking for group removal. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Copilot uses AI. Check for mistakes.

// Random filter - replaces last filter slot (or adds first one)
const handleRandom = useCallback(
Expand Down
8 changes: 4 additions & 4 deletions app/src/pages/SpecPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ export function SpecPage() {
link.href = impl.preview_url;
link.download = `${specId}-${impl.library_id}.png`;
link.click();
trackEvent('download_image', { spec: specId, library: impl.library_id });
trackEvent('download_image', { spec: specId, library: impl.library_id, page: isOverviewMode ? 'spec_overview' : 'spec_detail' });
},
[specId, trackEvent]
[specId, trackEvent, isOverviewMode]
Comment on lines +159 to +161
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds the page property to the download_image event tracking. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Copilot uses AI. Check for mistakes.
);

// Handle copy code (works for both overview and detail mode)
Expand All @@ -168,13 +168,13 @@ export function SpecPage() {
try {
await navigator.clipboard.writeText(impl.code);
setCodeCopied(impl.library_id);
trackEvent('copy_code', { spec: specId, library: impl.library_id, method: 'image' });
trackEvent('copy_code', { spec: specId, library: impl.library_id, method: 'image', page: isOverviewMode ? 'spec_overview' : 'spec_detail' });
setTimeout(() => setCodeCopied(null), 2000);
} catch (err) {
console.error('Copy failed:', err);
}
},
[specId, trackEvent]
[specId, trackEvent, isOverviewMode]
Comment on lines +171 to +177
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds the page property to the copy_code event tracking. While this appears to be a useful analytics improvement, it's unrelated to the XSS security fix in proxy.py. Analytics enhancements should be in a separate PR, not bundled with a security fix.

Copilot uses AI. Check for mistakes.
);

// Track page view
Expand Down
Loading