Skip to content

Commit 485d97f

Browse files
fix(security): address CodeQL XSS alerts #84 and #85
- seo.py: escape spec_id in title for DB-unavailable fallback - proxy.py: add security headers (X-Content-Type-Options, Referrer-Policy) - proxy.py: add security comment documenting trusted GCS content model - test_proxy.py: add test for security headers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent e9f567a commit 485d97f

3 files changed

Lines changed: 31 additions & 2 deletions

File tree

api/routers/proxy.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ async def proxy_html(url: str, origin: str | None = None):
158158
except httpx.RequestError as e:
159159
raise HTTPException(status_code=502, detail="Failed to connect to storage") from e
160160

161+
# Security: Content is fetched from our controlled GCS bucket (pyplots-images),
162+
# which only contains HTML generated by our own workflows. The URL validation
163+
# above ensures only our bucket is accessible. This is NOT arbitrary user HTML -
164+
# it's our own trusted interactive plot output (plotly, bokeh, altair, etc.).
161165
html_content = response.text
162166

163167
# Generate script with correct target origin
@@ -172,4 +176,8 @@ async def proxy_html(url: str, origin: str | None = None):
172176
# Fallback: append to end
173177
html_content += size_script
174178

175-
return HTMLResponse(content=html_content)
179+
# Security headers for defense-in-depth (content is from trusted GCS bucket)
180+
return HTMLResponse(
181+
content=html_content,
182+
headers={"X-Content-Type-Options": "nosniff", "Referrer-Policy": "strict-origin-when-cross-origin"},
183+
)

api/routers/seo.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ async def seo_spec_overview(spec_id: str, db: AsyncSession | None = Depends(opti
127127
# Fallback when DB unavailable
128128
return HTMLResponse(
129129
BOT_HTML_TEMPLATE.format(
130-
title=f"{spec_id} | pyplots.ai",
130+
title=f"{html.escape(spec_id)} | pyplots.ai",
131131
description=DEFAULT_DESCRIPTION,
132132
image=DEFAULT_HOME_IMAGE,
133133
url=f"https://pyplots.ai/{html.escape(spec_id)}",

tests/unit/api/test_proxy.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,27 @@ def test_valid_url_injects_script_before_body(self, mock_client_class, client):
163163
assert SIZE_REPORTER_SCRIPT.strip() in response.text
164164
assert response.text.index("<script>") < response.text.index("</body>")
165165

166+
@patch("api.routers.proxy.httpx.AsyncClient")
167+
def test_security_headers_present(self, mock_client_class, client):
168+
"""Response should include security headers."""
169+
mock_response = AsyncMock()
170+
mock_response.text = "<html><body><h1>Test</h1></body></html>"
171+
mock_response.raise_for_status = lambda: None
172+
173+
mock_client = AsyncMock()
174+
mock_client.get = AsyncMock(return_value=mock_response)
175+
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
176+
mock_client.__aexit__ = AsyncMock(return_value=None)
177+
mock_client_class.return_value = mock_client
178+
179+
response = client.get(
180+
"/proxy/html", params={"url": "https://storage.googleapis.com/pyplots-images/plots/test/plot.html"}
181+
)
182+
183+
assert response.status_code == 200
184+
assert response.headers.get("x-content-type-options") == "nosniff"
185+
assert response.headers.get("referrer-policy") == "strict-origin-when-cross-origin"
186+
166187
@patch("api.routers.proxy.httpx.AsyncClient")
167188
def test_valid_url_injects_script_before_html_if_no_body(self, mock_client_class, client):
168189
"""If no </body>, inject before </html>."""

0 commit comments

Comments
 (0)