fix(security): prevent XSS in proxy endpoint via json.dumps()#3170
Closed
MarkusNeusinger wants to merge 4 commits intomainfrom
Closed
fix(security): prevent XSS in proxy endpoint via json.dumps()#3170MarkusNeusinger wants to merge 4 commits intomainfrom
MarkusNeusinger wants to merge 4 commits intomainfrom
Conversation
…new events - Add page property to copy_code and download_image for user journey tracking (home, spec_overview, spec_detail) - Add toggle_grid_size event for view preference tracking - Add filter_remove event for filter removal tracking - Add spec property to SpecTabs copy_code (was missing) - Reduce debounce times: pageview 300ms→150ms, search_no_results 500ms→200ms - Move and update plausible.md documentation with Plausible dashboard setup guide 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CodeQL alert #84: target_origin was inserted into JavaScript without proper escaping. Fix uses json.dumps() to safely encode the origin string for JavaScript context, preventing XSS even if the origin contained special characters. - Add json import - Use json.dumps(target_origin) for safe JS string encoding - Update test to match double-quote format from json.dumps() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR claims to fix a CodeQL XSS vulnerability (alert #84) in the proxy endpoint by using json.dumps() to safely encode origin strings for JavaScript contexts. However, the PR contains significant scope creep with extensive unrelated analytics improvements bundled alongside the security fix.
Key Changes:
- Security fix: Uses
json.dumps()inapi/routers/proxy.pyto escape the origin string - Test update: Updated regex pattern to match double-quote format from
json.dumps() - Unrelated: Comprehensive analytics documentation and ~10 frontend event tracking enhancements
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
api/routers/proxy.py |
Security fix: Added json.dumps() to safely encode origin string for JavaScript context |
tests/unit/api/test_proxy.py |
Security fix: Updated test pattern to match double-quote format and added check for "*" wildcard |
docs/architecture/plausible.md |
UNRELATED: New 355-line analytics documentation file |
app/src/pages/SpecPage.tsx |
UNRELATED: Added page property to analytics events |
app/src/hooks/useFilterState.ts |
UNRELATED: Added filter_remove event tracking |
app/src/hooks/useAnalytics.ts |
UNRELATED: Reduced pageview debounce from 300ms to 150ms |
app/src/components/SpecTabs.tsx |
UNRELATED: Added spec and page properties to copy event |
app/src/components/ImageCard.tsx |
UNRELATED: Added page property to copy event |
app/src/components/FilterBar.tsx |
UNRELATED: Added toggle_grid_size tracking and reduced debounce to 200ms |
Comment on lines
+213
to
+229
| 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]); |
There was a problem hiding this comment.
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.
| ); | ||
|
|
||
| const trackPageview = useMemo(() => debounce(sendPageview, 300), [sendPageview]); | ||
| const trackPageview = useMemo(() => debounce(sendPageview, 150), [sendPageview]); |
There was a problem hiding this comment.
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]); |
| onClick={() => { | ||
| const newSize = imageSize === 'normal' ? 'compact' : 'normal'; | ||
| onImageSizeChange(newSize); | ||
| onTrackEvent('toggle_grid_size', { size: newSize }); |
There was a problem hiding this comment.
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 }); |
Comment on lines
+1
to
+355
| # Plausible Analytics Tracking | ||
|
|
||
| This document provides a comprehensive overview of Plausible Analytics implementation for pyplots.ai, including all tracked events, user interactions, and the required Plausible dashboard configuration. | ||
|
|
||
| ## Setup | ||
|
|
||
| **Location**: `app/index.html` | ||
|
|
||
| - Self-hosted Plausible script loaded via `/js/script.js` | ||
| - Custom endpoint: `/api/event` (proxied through nginx to avoid adblockers) | ||
| - Manual pageview tracking: `autoCapturePageviews: false` | ||
| - Production-only: Only tracks on `pyplots.ai` domain | ||
| - Privacy-focused: No cookies, GDPR-compliant | ||
|
|
||
| ## Page Views | ||
|
|
||
| **Implementation**: Query parameters converted to URL path segments for better analytics segmentation. | ||
|
|
||
| ### Filter-Based Pageviews | ||
|
|
||
| Filters create dynamic URLs with the following format: | ||
| ``` | ||
| https://pyplots.ai/{category}/{value}/{category}/{value}/... | ||
| ``` | ||
|
|
||
| **Ordered categories**: `lib`, `spec`, `plot`, `data`, `dom`, `feat` | ||
|
|
||
| **Examples**: | ||
| - `/?lib=matplotlib` → `https://pyplots.ai/lib/matplotlib` | ||
| - `/?lib=matplotlib&plot=scatter` → `https://pyplots.ai/lib/matplotlib/plot/scatter` | ||
| - `/?lib=matplotlib,seaborn` → `https://pyplots.ai/lib/matplotlib,seaborn` (OR logic) | ||
| - `/?lib=matplotlib&lib=seaborn` → `https://pyplots.ai/lib/matplotlib/lib/seaborn` (AND logic) | ||
|
|
||
| **Benefits**: | ||
| - Plausible shows popular filter combinations | ||
| - No manual event tracking needed for filter changes | ||
| - URL structure clearly shows user's browsing context | ||
|
|
||
| ### Static Pages | ||
|
|
||
| | URL | Description | | ||
| |-----|-------------| | ||
| | `/` | Home page (no filters) | | ||
| | `/catalog` | Catalog page (alphabetical spec list) | | ||
| | `/{spec_id}` | Spec overview page (grid of all implementations) | | ||
| | `/{spec_id}/{library}` | Spec detail page (single library implementation) | | ||
| | `/interactive/{spec_id}/{library}` | Interactive fullscreen view (HTML plots) | | ||
|
|
||
| **Total pageview tracking**: Automated via `trackPageview()` in all pages | ||
|
|
||
| ## Custom Events | ||
|
|
||
| ### Navigation Events | ||
|
|
||
| | Event Name | Properties | Where | Description | | ||
| |------------|-----------|-------|-------------| | ||
| | `navigate_to_spec` | `spec`, `library` | HomePage | User clicks image card to view spec detail | | ||
| | `switch_library` | `spec`, `library` | SpecPage | User switches library via pills in detail view | | ||
| | `select_implementation` | `spec`, `library` | SpecPage | User clicks implementation card in overview mode | | ||
| | `back_to_overview` | `spec`, `library` | SpecPage | User clicks main image to return to overview | | ||
| | `catalog_rotate` | `spec` | CatalogPage | User clicks image in catalog to rotate library | | ||
|
|
||
| ### Code Interaction Events | ||
|
|
||
| | Event Name | Properties | Where | Description | | ||
| |------------|-----------|-------|-------------| | ||
| | `copy_code` | `spec`, `library`, `method`, `page` | Multiple | User copies code to clipboard | | ||
| | `download_image` | `spec`, `library`, `page` | SpecPage | User downloads PNG image | | ||
|
|
||
| **Copy methods**: | ||
| - `card`: Quick copy button on image card (home grid) | ||
| - `image`: Copy button on main image (spec page) | ||
| - `tab`: Copy button in Code tab | ||
|
|
||
| **Page values** (for user journey tracking): | ||
| - `home`: HomePage grid view | ||
| - `spec_overview`: SpecPage showing all library implementations | ||
| - `spec_detail`: SpecPage showing single library implementation | ||
|
|
||
| ### Filter & Search Events | ||
|
|
||
| | Event Name | Properties | Where | Description | | ||
| |------------|-----------|-------|-------------| | ||
| | `search` | `query`, `category` | FilterBar | User searches and selects value | | ||
| | `search_no_results` | `query` | FilterBar | Search query returns no results (debounced 200ms) | | ||
| | `random` | `category`, `value`, `method` | HomePage | User triggers random filter | | ||
| | `filter_remove` | `category`, `value` | HomePage | User removes a filter | | ||
| | `toggle_grid_size` | `size` | FilterBar | User toggles between normal/compact view | | ||
|
|
||
| **Random methods**: | ||
| - `click`: Shuffle icon clicked | ||
| - `space`: Spacebar pressed | ||
| - `doubletap`: Mobile double-tap gesture | ||
|
|
||
| **Grid sizes**: | ||
| - `normal`: Larger cards (1-3 columns) | ||
| - `compact`: Smaller cards (2-6 columns) | ||
|
|
||
| ### Tab Interaction Events | ||
|
|
||
| | Event Name | Properties | Where | Description | | ||
| |------------|-----------|-------|-------------| | ||
| | `tab_click` | `tab`, `library` | SpecTabs | User opens a tab | | ||
| | `tab_collapse` | `library` | SpecTabs | User closes currently open tab | | ||
|
|
||
| **Tab names**: `code`, `specification`, `implementation`, `quality` | ||
|
|
||
| ### External Link Events | ||
|
|
||
| | Event Name | Properties | Where | Description | | ||
| |------------|-----------|-------|-------------| | ||
| | `external_link` | `destination`, `spec`, `library` | Footer | User clicks external link in footer | | ||
| | `open_interactive` | `spec`, `library` | SpecPage | User opens interactive HTML view | | ||
|
|
||
| **Destinations**: `linkedin`, `github`, `stats` | ||
|
|
||
| --- | ||
|
|
||
| ## Plausible Dashboard Configuration | ||
|
|
||
| ### Required Custom Properties | ||
|
|
||
| To see event properties in Plausible dashboard, you **MUST** register them as custom properties. | ||
|
|
||
| **Go to**: Plausible Dashboard → Site Settings → Custom Properties → Add Property | ||
|
|
||
| #### Properties to Register | ||
|
|
||
| | Property | Description | Used By Events | | ||
| |----------|-------------|----------------| | ||
| | `spec` | Plot specification ID | `copy_code`, `download_image`, `navigate_to_spec`, `switch_library`, `select_implementation`, `back_to_overview`, `catalog_rotate`, `external_link`, `open_interactive` | | ||
| | `library` | Library name (matplotlib, seaborn, etc.) | `copy_code`, `download_image`, `navigate_to_spec`, `switch_library`, `select_implementation`, `back_to_overview`, `external_link`, `open_interactive`, `tab_click`, `tab_collapse` | | ||
| | `method` | Action method (card, image, tab, click, space, doubletap) | `copy_code`, `random` | | ||
| | `page` | Page context (home, spec_overview, spec_detail) | `copy_code`, `download_image` | | ||
| | `category` | Filter category (lib, plot, dom, feat, data, spec) | `search`, `random`, `filter_remove` | | ||
| | `value` | Filter value | `random`, `filter_remove` | | ||
| | `query` | Search query text | `search`, `search_no_results` | | ||
| | `destination` | External link target (linkedin, github, stats) | `external_link` | | ||
| | `tab` | Tab name (code, specification, implementation, quality) | `tab_click` | | ||
| | `size` | Grid size (normal, compact) | `toggle_grid_size` | | ||
|
|
||
| ### Goals Configuration | ||
|
|
||
| **Go to**: Plausible Dashboard → Site Settings → Goals → Add Goal | ||
|
|
||
| #### Recommended Goals | ||
|
|
||
| | Goal Name | Type | Description | | ||
| |-----------|------|-------------| | ||
| | `copy_code` | Custom Event | Track code copies (primary conversion) | | ||
| | `download_image` | Custom Event | Track image downloads | | ||
| | `navigate_to_spec` | Custom Event | Track spec navigation | | ||
| | `search` | Custom Event | Track successful searches | | ||
| | `search_no_results` | Custom Event | Track failed searches (content gaps) | | ||
| | `random` | Custom Event | Track random filter usage | | ||
| | `filter_remove` | Custom Event | Track filter removal | | ||
| | `toggle_grid_size` | Custom Event | Track view preference | | ||
| | `external_link` | Custom Event | Track outbound clicks | | ||
| | `open_interactive` | Custom Event | Track interactive mode usage | | ||
| | `tab_click` | Custom Event | Track tab interactions | | ||
|
|
||
| ### Funnels (Optional) | ||
|
|
||
| **Example funnel**: Home → Spec → Copy | ||
|
|
||
| 1. Pageview `/` (home) | ||
| 2. `navigate_to_spec` event | ||
| 3. `copy_code` event | ||
|
|
||
| ### Dashboard Widgets | ||
|
|
||
| Recommended custom widgets: | ||
|
|
||
| 1. **Top Specs Copied**: `copy_code` breakdown by `spec` | ||
| 2. **Popular Libraries**: `copy_code` breakdown by `library` | ||
| 3. **Copy Journey**: `copy_code` breakdown by `page` | ||
| 4. **Search Terms**: `search` breakdown by `query` | ||
| 5. **Missing Content**: `search_no_results` breakdown by `query` | ||
| 6. **View Preference**: `toggle_grid_size` breakdown by `size` | ||
|
|
||
| --- | ||
|
|
||
| ## User Journey Tracking | ||
|
|
||
| ### Understanding `page` Property | ||
|
|
||
| The `page` property tracks **where** users perform actions to understand their journey: | ||
|
|
||
| ``` | ||
| User lands on pyplots.ai | ||
| │ | ||
| ├─→ Home (grid view) | ||
| │ └─→ copy_code { page: 'home' } | ||
| │ | ||
| ├─→ Spec Overview (/{spec_id}) | ||
| │ └─→ copy_code { page: 'spec_overview' } | ||
| │ └─→ download_image { page: 'spec_overview' } | ||
| │ | ||
| └─→ Spec Detail (/{spec_id}/{library}) | ||
| └─→ copy_code { page: 'spec_detail' } | ||
| └─→ download_image { page: 'spec_detail' } | ||
| ``` | ||
|
|
||
| ### Journey Examples in Plausible | ||
|
|
||
| **Q: Do users copy from search results or spec pages?** | ||
| - Filter `copy_code` by `page` property | ||
| - `home` = direct from search/browse | ||
| - `spec_overview` = after viewing all implementations | ||
| - `spec_detail` = after deep-diving into one library | ||
|
|
||
| **Q: Which library implementations are most downloaded?** | ||
| - Filter `download_image` by `library` property | ||
| - Filter by `page` to see if users download from overview or detail | ||
|
|
||
| --- | ||
|
|
||
| ## Complete Event Reference | ||
|
|
||
| ### Events Summary Table | ||
|
|
||
| | Event | Properties | Code Location | | ||
| |-------|------------|---------------| | ||
| | `copy_code` | `spec`, `library`, `method`, `page` | ImageCard.tsx, SpecPage.tsx, SpecTabs.tsx | | ||
| | `download_image` | `spec`, `library`, `page` | SpecPage.tsx | | ||
| | `navigate_to_spec` | `spec`, `library` | HomePage.tsx | | ||
| | `switch_library` | `spec`, `library` | SpecPage.tsx | | ||
| | `select_implementation` | `spec`, `library` | SpecPage.tsx | | ||
| | `back_to_overview` | `spec`, `library` | SpecPage.tsx | | ||
| | `catalog_rotate` | `spec` | CatalogPage.tsx | | ||
| | `search` | `query`, `category` | FilterBar.tsx | | ||
| | `search_no_results` | `query` | FilterBar.tsx | | ||
| | `random` | `category`, `value`, `method` | useFilterState.ts | | ||
| | `filter_remove` | `category`, `value` | useFilterState.ts | | ||
| | `toggle_grid_size` | `size` | FilterBar.tsx | | ||
| | `tab_click` | `tab`, `library` | SpecTabs.tsx | | ||
| | `tab_collapse` | `library` | SpecTabs.tsx | | ||
| | `external_link` | `destination`, `spec`, `library` | Footer.tsx | | ||
| | `open_interactive` | `spec`, `library` | SpecPage.tsx | | ||
| | `view_spec_overview` | `spec` | SpecPage.tsx | | ||
| | `view_spec` | `spec`, `library` | SpecPage.tsx | | ||
|
|
||
| --- | ||
|
|
||
| ## Property Values Reference | ||
|
|
||
| ### `spec` Values | ||
| Any valid specification ID (e.g., `scatter-basic`, `heatmap-correlation`, `bar-grouped`) | ||
|
|
||
| ### `library` Values | ||
| ``` | ||
| matplotlib | seaborn | plotly | bokeh | altair | plotnine | pygal | highcharts | letsplot | ||
| ``` | ||
|
|
||
| ### `method` Values | ||
| ``` | ||
| card # ImageCard copy button (home grid) | ||
| image # SpecPage image copy button | ||
| tab # SpecTabs code tab copy button | ||
| click # Random icon clicked | ||
| space # Spacebar pressed | ||
| doubletap # Mobile double-tap | ||
| ``` | ||
|
|
||
| ### `page` Values | ||
| ``` | ||
| home # HomePage grid view | ||
| spec_overview # SpecPage showing all libraries | ||
| spec_detail # SpecPage showing single library | ||
| ``` | ||
|
|
||
| ### `category` Values | ||
| ``` | ||
| lib # library filter | ||
| plot # plot_type filter | ||
| dom # domain filter | ||
| feat # features filter | ||
| data # data_type filter | ||
| spec # specification filter | ||
| ``` | ||
|
|
||
| ### `tab` Values | ||
| ``` | ||
| code # Python code tab | ||
| specification # Spec details tab | ||
| implementation # AI implementation review tab | ||
| quality # Quality score breakdown tab | ||
| ``` | ||
|
|
||
| ### `destination` Values | ||
| ``` | ||
| linkedin # LinkedIn profile link | ||
| github # GitHub repository link | ||
| stats # Plausible stats dashboard link | ||
| ``` | ||
|
|
||
| ### `size` Values | ||
| ``` | ||
| normal # Larger cards (1-3 columns) | ||
| compact # Smaller cards (2-6 columns) | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Code Locations | ||
|
|
||
| - **Plausible setup**: `app/index.html` (lines 59-68) | ||
| - **Analytics hook**: `app/src/hooks/useAnalytics.ts` | ||
| - **Pageview building**: `buildPlausibleUrl()` in useAnalytics.ts | ||
| - **Event tracking**: Passed via `onTrackEvent` prop throughout component tree | ||
|
|
||
| ## Testing | ||
|
|
||
| **Development**: Tracking disabled (not on pyplots.ai domain) | ||
|
|
||
| **Production testing**: | ||
| 1. Open https://pyplots.ai | ||
| 2. Open browser console | ||
| 3. Check for Plausible script load | ||
| 4. Trigger events and verify in Plausible dashboard (may take 1-2 min) | ||
|
|
||
| **Debug mode**: | ||
| ```javascript | ||
| // In browser console | ||
| window.plausible = function(...args) { console.log('Plausible:', args); }; | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Implementation Checklist | ||
|
|
||
| - [x] Plausible script loaded | ||
| - [x] Manual pageview tracking | ||
| - [x] Filter-based URL generation | ||
| - [x] Navigation events (`navigate_to_spec`, `switch_library`, etc.) | ||
| - [x] Code copy events with journey tracking (`copy_code` + `page`) | ||
| - [x] Download tracking (`download_image` + `page`) | ||
| - [x] Search events (`search`, `search_no_results`) | ||
| - [x] Random filter events (`random`) | ||
| - [x] Filter removal tracking (`filter_remove`) | ||
| - [x] Grid size toggle tracking (`toggle_grid_size`) | ||
| - [x] Tab interaction events (`tab_click`, `tab_collapse`) | ||
| - [x] External link events (`external_link`, `open_interactive`) | ||
|
|
||
| ### Plausible Dashboard Checklist | ||
|
|
||
| - [ ] Register all 10 custom properties (see table above) | ||
| - [ ] Create goals for key events | ||
| - [ ] Set up funnels (optional) | ||
| - [ ] Create custom dashboard widgets (optional) | ||
|
|
||
| --- | ||
|
|
||
| **Last Updated**: 2025-01-05 | ||
| **Status**: Production-ready with full journey tracking |
There was a problem hiding this comment.
This entire documentation file is unrelated to the XSS security fix described in the PR title and description. The PR is supposed to fix CodeQL alert #84 for XSS in proxy.py, but this adds comprehensive analytics documentation. This change should be in a separate PR focused on analytics/documentation improvements, not bundled with a security fix.
Comment on lines
+171
to
+177
| 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] |
There was a problem hiding this comment.
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.
| 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' }); |
There was a problem hiding this comment.
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' }); |
| 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' }); |
There was a problem hiding this comment.
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' }); |
| onTrackEvent('search_no_results', { query }); | ||
| lastTrackedQueryRef.current = query; | ||
| }, 500); | ||
| }, 200); |
There was a problem hiding this comment.
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); |
| onClick={() => { | ||
| const newSize = imageSize === 'normal' ? 'compact' : 'normal'; | ||
| onImageSizeChange(newSize); | ||
| onTrackEvent('toggle_grid_size', { size: newSize }); |
There was a problem hiding this comment.
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 }); |
Comment on lines
+159
to
+161
| trackEvent('download_image', { spec: specId, library: impl.library_id, page: isOverviewMode ? 'spec_overview' : 'spec_detail' }); | ||
| }, | ||
| [specId, trackEvent] | ||
| [specId, trackEvent, isOverviewMode] |
There was a problem hiding this comment.
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.
Owner
Author
|
Security fix pushed directly to main via cherry-pick. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
api/routers/proxy.pytarget_originwas being inserted into JavaScript without proper escapingjson.dumps()to safely encode the origin string for JavaScript contextChanges
import jsonto proxy.pyjson.dumps(target_origin)for safe JavaScript string encodingjson.dumps()Test plan
🤖 Generated with Claude Code