The XSS My Agent Caught in Its Own Output
I built a slide compiler that renders user-supplied image URLs into HTML. The AI reviewer that runs on every PR caught that I'd forgotten to validate the URL scheme. Here's what the finding looked like and why escaping wasn't enough.
I built a slide compiler that renders user-supplied image URLs into HTML. The AI reviewer that runs on every PR caught that I'd forgotten to validate the URL scheme. Here's what the finding looked like and why escaping wasn't enough.
I built a tool last week that turns structured JSON into a self-contained HTML slide deck. One of the slide types is image, which takes a src URL and renders it as an <img> tag.
My AI reviewer scored the PR 3/5 and flagged one P1 finding: the image URL validation was insufficient.
Specifically: javascript:alert(1) would pass through.
What I got wrong
The renderer had a helper function that escapes values for use in HTML attributes:
def _attr(value: Any) -> str:
return html.escape(str(value), quote=True)
And the image slide used it like this:
def _render_image(image: dict[str, Any]) -> str:
caption = _escape(image.get("caption", ""))
return f"""<figure class="image-panel" data-role="image">
<img src="{_attr(image["src"])}" alt="{_attr(image.get("alt", ""))}">
...
"""
html.escape("javascript:alert(1)", quote=True) returns javascript:alert(1). The string contains no <, >, &, or " characters, so there’s nothing to escape. The output is:
<img src="javascript:alert(1)">
Which browsers will execute. Same with data:text/html,<script>alert(1)</script>.
The schema validated that src was a non-empty string. The renderer escaped it for HTML. Both of those things happened correctly. The URL scheme was never checked.
The fix
Three lines:
import urllib.parse
def _validate_image_src(src: str) -> None:
scheme = urllib.parse.urlparse(src).scheme
if scheme not in ("", "http", "https"):
raise ValueError(f"image src scheme {scheme!r} is not allowed; use http, https, or a relative path")
Called in _render_image before the string ever touches the HTML template. Five regression tests: four dangerous schemes rejected (javascript:, data:, vbscript:, file:), one safe pair passing (https:// and a relative path ./photo.jpg).
Why escaping wasn’t enough
HTML escaping prevents markup injection: characters that would break out of the current HTML context. <script> becomes <script>, which the browser shows as text rather than executing. That is the right defense for content that goes into HTML text nodes or attribute values where the value is data.
URL schemes are different. javascript: is not a text value being injected into markup — it’s a URI scheme that the browser interprets as an instruction when it loads the resource. HTML escaping knows nothing about URI semantics. It can’t, and isn’t supposed to. They’re solving different problems.
The common mistake is thinking “I’m escaping user input before it goes into HTML, so I’m safe.” You’re safe against markup injection. You’re not necessarily safe against URI interpretation, which runs at the resource-loading layer, not the parsing layer.
The right defense at the URL layer is: explicitly enumerate what schemes are allowed and reject everything else. An empty string (relative path) and http/https are the safe set for image sources in a web context. Everything else fails closed.
Caught by the reviewer, not the tests
The initial seven tests covered rendering, schema validation, duplicate slide IDs, unknown types, and output size limits. None of them tested the image URL scheme.
The AI reviewer read the diff and asked: what happens if someone passes javascript:alert(1) as the image src? It was a correctness check, not a fuzzing run. The finding came from reading the code, not from running it.
This is the class of bugs that static analysis and schema validation don’t catch: semantic security properties that aren’t encoded in the type system or the schema. The schema says src is a string. The type system agrees. Both are correct. Neither knows that some strings are safer than others.
A security review pass, run by something that has seen this class of bug before, catches it. That’s what happened here.
The fix is in the PR. It’s not in production yet. But the P1 got found before merge, which is the right order.