HTML injection: attachment filenames are substituted into templates unescaped #463

An attachment filename containing " breaks out of the HTML attribute it is written into, letting arbitrary markup render as live DOM. A filename containing ' breaks the embedded JavaScript string in the onclick handler.

Found while building the browser test layer in #461, and independently confirmed by two reviewers.

Mechanism

Sources/XCTestHTMLReportCore/Classes/HTMLTemplates.swift:1582 and four sibling lines:

<span class="icon preview-icon" data="[[FILENAME]]" onclick="showScreenshot('[[FILENAME]]')"></span>

Substitution is a raw String.replacingOccurrences in Classes/Protocols/HTML.swift, fed by Classes/Models/Attachment.swift:307-309:

"SOURCE": source ?? "",
"FILENAME": filename,
"NAME": displayName,

None of the three is escaped.

This is a gap, not a missing capability

The codebase already has String.stringByEscapingXMLChars (Classes/Extensions/String+Xml.swift:10, escapes & < > " ') and already applies it — Activity.swift:93, Summary.swift, and throughout JUnitReport.swift. It is simply not applied to Attachment's placeholders, nor to those in Test.swift, Iteration.swift, Run.swift, RunDestination.swift, or TestScreenshotFlow.swift.

Reproduction

The synthetic fixture already contains a hostile filename:

FileName with DoubleQuote"SingleQuote'LessThan<GreaterThan>Ampersand&.txt

Rendered, the data="..." attribute terminates early and a spurious <greaterthan> element enters the DOM. Confirmed in Chromium.

Severity

Bounded by requiring control over an attachment's filename — normally test-author-controlled. But this repository publishes a rendered report to a public GitHub Pages URL on every merge to main (added in #458), which makes it a real stored-injection surface rather than a local-file curiosity.

Guard already in place

SummarySeamTests.testHostileAttachmentFilenameDoesNotBreakOutOfAttribute (added in #461) asserts the render escapes the filename. It is wrapped in XCTExpectFailure because it fails today. When this issue is fixed, that test flips to an unexpected pass, which XCTExpectFailure turns into a hard failure — so remove the wrapper as part of the fix, and the assertion becomes a permanent regression test.

🤖 Generated with Claude Code