Move PDF viewer wrapper to html/template — fix stored XSS (C-2, L-6) #97

Merged

qwc merged 1 commit from fix/pdf-viewer-xss into main

2026-05-18 16:00:17 +02:00

qwc commented

2026-05-18 15:55:52 +02:00

Owner

Summary

Workstream item #2. Audit findings C-2 (Critical) stored XSS in the PDF viewer wrapper and L-6 hand-formatted HTML — both fixed by the same change.

servePDFViewer built the wrapper page with fmt.Fprintf, interpolating the project name and version tag directly into HTML. Project names are free-form text set by editors/admins; an editor could set a name like

</title><script>fetch('/admin/users')...</script>

and any viewer opening the PDF would execute the script in their session, same-origin to /admin.

Changes

New internal/templates/overlay/pdf_viewer.html, parsed via the existing html/template engine — all string fields auto-escape.
New templates.PDFViewerData and Engine.RenderPDFViewer; the pre-rendered overlay HTML is passed in as template.HTML so it still emits verbatim.
servePDFViewer replaced from ~40 lines of hand-formatted HTML with a templates.RenderPDFViewer call.
Regression test TestPDFViewerEscapesProjectName creates a project named </title><script>...</script>, fetches the viewer wrapper, asserts the raw payload is absent and an escaped form is present.

Version tags are already constrained by the C-1 validation regex (PR #96), so the project-name path was the remaining XSS sink.

Test plan

go test ./... passes
Regression test confirms the XSS payload is escaped, not rendered
Manual: open a PDF version in the browser, confirm the viewer renders the same as before (overlay toolbar, page-fragment routing, search hint banner)

🤖 Generated with Claude Code

## Summary Workstream item #2. Audit findings **C-2 (Critical)** stored XSS in the PDF viewer wrapper and **L-6** hand-formatted HTML — both fixed by the same change. `servePDFViewer` built the wrapper page with `fmt.Fprintf`, interpolating the project name and version tag directly into HTML. Project names are free-form text set by editors/admins; an editor could set a name like </title><script>fetch('/admin/users')...</script> and any viewer opening the PDF would execute the script in their session, same-origin to `/admin`. ## Changes - New `internal/templates/overlay/pdf_viewer.html`, parsed via the existing `html/template` engine — all string fields auto-escape. - New `templates.PDFViewerData` and `Engine.RenderPDFViewer`; the pre-rendered overlay HTML is passed in as `template.HTML` so it still emits verbatim. - `servePDFViewer` replaced from ~40 lines of hand-formatted HTML with a `templates.RenderPDFViewer` call. - Regression test `TestPDFViewerEscapesProjectName` creates a project named `</title><script>...</script>`, fetches the viewer wrapper, asserts the raw payload is absent and an escaped form is present. Version tags are already constrained by the C-1 validation regex (PR #96), so the project-name path was the remaining XSS sink. ## Test plan - [x] `go test ./...` passes - [x] Regression test confirms the XSS payload is escaped, not rendered - [ ] Manual: open a PDF version in the browser, confirm the viewer renders the same as before (overlay toolbar, page-fragment routing, search hint banner) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

qwc added 1 commit

2026-05-18 15:55:52 +02:00

Move PDF viewer wrapper from fmt.Fprintf to html/template (fix XSS)

CI / test (pull_request) Successful in 1m19s

Details

CI / build (pull_request) Successful in 56s

Details

CI / docker (pull_request) Has been skipped

Details

18c6c690d0

Audit findings C-2 (Critical, stored XSS) and L-6 (hand-formatted HTML
fragile to maintain).

servePDFViewer built the wrapper page with fmt.Fprintf, interpolating the
project name and version tag directly into HTML. Project names are
free-form text set by editors/admins; version tags are now constrained by
the C-1 validation regex but the project name path remained: an editor
could set a project name like

  </title><script>fetch('/admin/...')</script>

and any viewer (including admins) opening the PDF would execute the
script in their session, same-origin to /admin.

Moves the wrapper to internal/templates/overlay/pdf_viewer.html, parsed
via the existing html/template engine; all string fields are now
auto-escaped. The pre-rendered overlay HTML is passed in as template.HTML
so it still emits verbatim. Adds RenderPDFViewer on Engine that writes
directly to the response.

Regression test creates a project whose name contains the </title><script>
payload, fetches the viewer wrapper, and asserts the raw payload is absent
and an escaped form is present.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

qwc merged commit 0c3c9ead77 into main

2026-05-18 16:00:17 +02:00

qwc deleted branch fix/pdf-viewer-xss

2026-05-18 16:00:17 +02:00

qwc referenced this pull request from a commit

2026-05-18 16:00:18 +02:00

Merge pull request 'Move PDF viewer wrapper to html/template — fix stored XSS (C-2, L-6)' (#97) from fix/pdf-viewer-xss into main

qwc referenced this pull request from a commit

2026-07-13 13:19:10 +02:00

Merge pull request 'Move PDF viewer wrapper to html/template — fix stored XSS (C-2, L-6)' (#97) from fix/pdf-viewer-xss into main