security: sanitize rendered research-report HTML by StressTestor · Pull Request #364 · odysseus-dev/odysseus

StressTestor · 2026-06-01T08:24:19Z

Summary

The visual research report (GET /api/research/report/{id}) is served under a deliberately relaxed CSP (script-src 'self' 'unsafe-inline', core/middleware.py) because the report template uses inline scripts. Its content is built from LLM output over crawled web pages (untrusted), and two values reached that HTML unsanitized:

src/visual_report.py::_md_to_html renders the report markdown with markdown.markdown(...), which passes raw HTML through verbatim, so <script>, <img onerror=…>, <svg onload=…>, and javascript: links carried in crawled content execute in the app origin when a user opens their report.
category arrives from the /api/research/start request body with no enum check and is interpolated raw into <body class="category-{category}">, an attribute-injection sink independent of the markdown body.

This allowlist-sanitizes the rendered markdown with nh3 (the maintained ammonia binding), keeping everything the report emits (tables, fenced code, <details>/<summary>, heading/toc anchors, codehilite classes, external-link target/rel) while dropping active content, and html.escapes the category. I re-confirmed both are still live on current main before rebasing.

Linked Issue

Part of #169

Type of Change

Bug fix (non-breaking — fixes a confirmed issue)
New feature (non-breaking — adds new behaviour)
Breaking change (changes or removes existing behaviour)
Refactor / cleanup (behaviour unchanged)
Documentation only
CI / tooling / configuration

Checklist

I searched open issues and open PRs — this is not a duplicate.
This PR targets main
My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. (Verified via the security and visual_report test suites below, not a full app run. Happy to do one if you want it.)

How to Test

The XSS is still live on main. Before the fix:

python -c "from src.visual_report import _md_to_html; print(_md_to_html('<script>alert(document.domain)</script>'))"
# -> <p><script>alert(document.domain)</script></p>   (executes under the report's 'unsafe-inline' CSP)

After:

python -m pytest tests/test_security_regressions.py -k "md_to_html or escapes_request_category" -q

The new cases assert <script>/onerror/onload/javascript: are stripped, that normal report formatting (heading + toc anchor, tables, fenced code, <details>, external links with rel) survives, and that a malicious or non-string category cannot break out of the class attribute. They fail on main and pass with this change; the three existing tests/test_visual_report*.py suites still pass. (The other ~29 failures in test_security_regressions.py are missing-dependency ModuleNotFoundErrors, identical on a clean main checkout in my environment.)

Visual / UI changes

None. This is security-only and preserves all legitimate report formatting (locked by test_md_to_html_preserves_normal_report_formatting). No rendering or style change to screenshot.

Rebased onto current main, which had drifted ~689 commits since the branch was opened.

lalalune

✅ Tested & LGTM

Re-verified on a clean checkout of pull/364/head with real dependencies (SQLAlchemy / FastAPI / Pydantic, Python 3.12): pytest tests/test_security_regressions.py → 34 passed, 1 warning in 0.66s.

What it does: Research-report markdown is built from LLM output over crawled pages (untrusted) and report pages run under script-src 'self' 'unsafe-inline' (confirmed core/middleware.py:69); python-markdown passes raw HTML through verbatim -> genuine stored-XSS path. Fix runs nh3.clean() with an allowlist built from nh3 safe defaults plus exactly the report's own tags/attrs (details/summary, heading ids, codehilite classes, table align, link rel/target, img). Also html.escape()s the request-supplied category that lands in (separate attribute-injection XSS). nh3 API usage verified correct. nh3 ships prebuilt wheels so it doesn't compromise install reliability.

On-mission (security & privacy).

lalalune

✅ Tested & LGTM

Re-verified on a clean checkout of pull/364/head with real dependencies (SQLAlchemy / FastAPI / Pydantic, Python 3.12): pytest tests/test_security_regressions.py → 34 passed, 1 warning in 0.66s.

What it does: Research-report markdown is built from LLM output over crawled pages (untrusted) and report pages run under script-src 'self' 'unsafe-inline' (confirmed core/middleware.py:69); python-markdown passes raw HTML through verbatim -> genuine stored-XSS path. Fix runs nh3.clean() with an allowlist built from nh3 safe defaults plus exactly the report's own tags/attrs (details/summary, heading ids, codehilite classes, table align, link rel/target, img). Also html.escape()s the request-supplied category that lands in (separate attribute-injection XSS). nh3 API usage verified correct. nh3 ships prebuilt wheels so it doesn't compromise install reliability.

On-mission (security & privacy).

alteixeira20 · 2026-06-03T21:43:32Z

Thanks for the PR. This currently needs a rebase / conflict resolution before I can review it properly.

Could you please rebase onto the current main, resolve the conflicts, and ping me here once it is updated?

Sorry for the extra delay — we are still adjusting to the large influx of contributions and trying to keep reviews careful and fair.

The visual research report is assembled from LLM output over crawled web pages (untrusted content) and served under a relaxed `script-src 'unsafe-inline'` CSP. Two values reached that HTML without sanitization: - `_md_to_html` rendered the report markdown via python-markdown, which passes raw HTML through verbatim, so `<script>` / `<img onerror>` / `<svg onload>` / `javascript:` links carried in crawled content ran in the app origin. - `category` (from the /api/research/start request body, no enum check) was interpolated raw into `<body class="category-{category}">`. Allowlist-sanitize the rendered markdown with nh3, keeping the formatting the report emits (tables, code, details/summary, toc anchors, codehilite classes, external-link target/rel) while dropping active content, and html.escape the category. Adds regression tests.

StressTestor · 2026-06-04T09:14:28Z

rebased onto current main and resolved the conflicts.

heads up, the report XSS is still live on main: _md_to_html still renders the report markdown with raw HTML passthrough, and category still lands unescaped in the body class. the rebased fix allowlist-sanitizes the render with nh3 and escapes the category, keeps all the normal formatting, and the regression tests fail on main / pass with the change.

ready when you are.

alteixeira20

LGFM. I checked the PR patch and focused security validation.

The PR adds nh3 to requirements.txt; my local venv did not have that new dependency yet, so the first test run failed with ModuleNotFoundError: No module named 'nh3'. After installing the dependency locally, the focused validation passed:

python3 -m pytest tests/test_security_regressions.py -q
# 81 passed, 1 warning

python3 -m py_compile src/visual_report.py tests/test_security_regressions.py
git diff --check upstream/main...HEAD

I also ran a direct malicious-input probe against the actual report rendering helpers. It confirmed that:

<script> is stripped
inline event handlers such as onerror / onload are stripped
javascript: links are stripped
normal report formatting is preserved, including headings, tables, fenced code blocks, details/summary, and external links
request category is escaped before reaching the body class

Scope checked:

requirements.txt
src/visual_report.py
tests/test_security_regressions.py

This is a focused fix for untrusted research-report HTML rendering and category attribute injection.

StressTestor · 2026-06-04T19:13:37Z

appreciate the merge.

you mentioned the influx of contributions, so i figured i'd offer. i do security and bug-bounty work for a living, so if it helps while you're buried, happy to spot-check incoming security PRs and flag the real ones (and the ones quietly making things worse). no strings, just an extra set of eyes that reads diffs all day. either way, glad this one landed.

alteixeira20 · 2026-06-04T19:20:24Z

@StressTestor Really appreciate that, thank you.

That kind of security review help would be highly valuable, especially right now with the current influx of PRs. I’m finishing up some CI-related fixes first so the test signal is cleaner and PR audits are easier to trust, but once that is stable, an extra experienced set of eyes on security-sensitive diffs would help a lot.

StressTestor · 2026-06-04T19:25:05Z

sounds good, no rush. ping me whenever the CI's settled and i'll start sanity-checking the security-sensitive ones. cleaner test signal first is the right call.

The visual research report is assembled from LLM output over crawled web pages (untrusted content) and served under a relaxed `script-src 'unsafe-inline'` CSP. Two values reached that HTML without sanitization: - `_md_to_html` rendered the report markdown via python-markdown, which passes raw HTML through verbatim, so `<script>` / `<img onerror>` / `<svg onload>` / `javascript:` links carried in crawled content ran in the app origin. - `category` (from the /api/research/start request body, no enum check) was interpolated raw into `<body class="category-{category}">`. Allowlist-sanitize the rendered markdown with nh3, keeping the formatting the report emits (tables, code, details/summary, toc anchors, codehilite classes, external-link target/rel) while dropping active content, and html.escape the category. Adds regression tests.

lalalune approved these changes Jun 2, 2026

View reviewed changes

alteixeira20 added bug Something isn't working stale pr This pr need a merge from main, it is out of sync labels Jun 3, 2026

StressTestor force-pushed the security/sanitize-research-report-html branch from 2a77c75 to 144715d Compare June 4, 2026 09:13

github-actions Bot added the ready for review Description complete — ready for maintainer review label Jun 4, 2026

alteixeira20 removed the stale pr This pr need a merge from main, it is out of sync label Jun 4, 2026

alteixeira20 approved these changes Jun 4, 2026

View reviewed changes

alteixeira20 merged commit fa1fe7f into odysseus-dev:main Jun 4, 2026
1 check passed

mahdi-salmanzade mentioned this pull request Jun 4, 2026

fix(security): sanitize deep-research report HTML (stored XSS via scraped content) #1356

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

security: sanitize rendered research-report HTML#364

security: sanitize rendered research-report HTML#364
alteixeira20 merged 1 commit into
odysseus-dev:mainfrom
StressTestor:security/sanitize-research-report-html

StressTestor commented Jun 1, 2026 •

edited

Loading

Uh oh!

lalalune left a comment

Uh oh!

lalalune left a comment

Uh oh!

alteixeira20 commented Jun 3, 2026

Uh oh!

StressTestor commented Jun 4, 2026

Uh oh!

alteixeira20 left a comment

Uh oh!

Uh oh!

StressTestor commented Jun 4, 2026

Uh oh!

alteixeira20 commented Jun 4, 2026

Uh oh!

StressTestor commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

StressTestor commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linked Issue

Type of Change

Checklist

How to Test

Visual / UI changes

Uh oh!

lalalune left a comment

Choose a reason for hiding this comment

Uh oh!

lalalune left a comment

Choose a reason for hiding this comment

Uh oh!

alteixeira20 commented Jun 3, 2026

Uh oh!

StressTestor commented Jun 4, 2026

Uh oh!

alteixeira20 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

StressTestor commented Jun 4, 2026

Uh oh!

alteixeira20 commented Jun 4, 2026

Uh oh!

StressTestor commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

StressTestor commented Jun 1, 2026 •

edited

Loading