security(export): strip remote images on the soffice path to match the native path

[JohnMcLear]

Defense-in-depth / consistency fix — not a core vulnerability on its own. Core Etherpad never emits <img> tags in export HTML; they only appear when a plugin/hook injects them, so sanitising such content is primarily the injecting plugin's responsibility.

However, the native in-process export path (#7538) already calls stripRemoteImages() defensively, while the LibreOffice (soffice) path wrote the export HTML to the temp file verbatim. soffice is the only export path that performs outbound fetches for remote <img> URLs during conversion, so a plugin-injected remote image there becomes a blind SSRF sink. This change makes the soffice path consistent with the native path core already ships.

Change

One line: apply the existing stripRemoteImages(html) before writing the soffice temp file.

Test

New sofficeExportRemoteImage.ts regression test injects a remote image via exportHTMLAdditionalContent, intercepts the temp file via the exportConvert hook, and asserts no remote image URL survives. Existing 54 export tests still pass; tsc --noEmit clean.

Remote-image behaviour reported privately by meifukun (https://github.com/meifukun). No security advisory is filed for this — it is hardening, not a core vuln.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Harden soffice export by stripping remote sources (SSRF defense-in-depth)

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Strip remote `` URLs before writing HTML for LibreOffice conversion.
• Align soffice export sanitization behavior with the existing native export path.
• Add regression test ensuring plugin-injected remote images never reach the soffice temp file.

Diagram

graph TD
  A["Export request"] --> B["ExportHandler.doExport()"] --> C["ExportSanitizeHtml.stripRemoteImages()"] --> D[("Temp HTML file")] --> E["Hook: exportConvert"] --> F["LibreOffice (soffice)"] --> G["Converted export response"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Network-isolate the soffice conversion process

• ➕ Mitigates SSRF and other egress risks beyond just fetching
• ➕ Covers future conversion behaviors without relying on HTML sanitization correctness
• ➖ Higher operational complexity (namespaces/containers/AppArmor/SELinux)
• ➖ Harder to support across platforms, especially Windows

2. Disallow/strip images at plugin injection points

• ➕ Stops problematic content closer to the source (the injecting hook/plugin)
• ➕ Avoids needing multiple defensive checks in different export paths
• ➖ Not enforceable for all plugins; core cannot fully trust hook implementations
• ➖ Still benefits from core defense-in-depth because conversion is the risky boundary

3. Configure LibreOffice to never fetch remote resources (if available)

• ➕ Keeps HTML intact while removing the fetch behavior at the converter layer
• ➖ May not be reliably supported across LibreOffice versions
• ➖ Hard to validate and might not cover all remote resource types

Recommendation: The PR’s approach (reusing the existing stripRemoteImages() and applying it to the soffice temp-file path) is the best low-risk consistency fix: it matches the native export hardening already present and directly removes the specific SSRF sink (remote `` URLs) before soffice can fetch them. For stronger hardening, consider optional deployment guidance (or future work) to run soffice with network egress blocked, but that is appropriately out of scope for this minimal defense-in-depth patch.

Files changed (2) +115 / -1

Bug fix (1) +7 / -1

ExportHandler.tsSanitize remote images before writing soffice conversion input HTML +7/-1

Sanitize remote images before writing soffice conversion input HTML

• Applies 'stripRemoteImages(html)' on the LibreOffice (soffice) export path before writing the temporary HTML file. This aligns the soffice path with the existing native export sanitization to prevent converter-triggered remote fetches (blind SSRF) from plugin-injected '<img>' tags.

src/node/handler/ExportHandler.ts

Tests (1) +108 / -0

sofficeExportRemoteImage.tsAdd regression test ensuring soffice temp HTML never contains remote <img> URLs +108/-0

Add regression test ensuring soffice temp HTML never contains remote <img> URLs

• Introduces a backend spec that injects a remote image via 'exportHTMLAdditionalContent', forces the soffice path, and captures the generated temp HTML via an 'exportConvert' hook. Asserts that the raw HTML export contains the marker while the soffice converter input HTML does not, preventing SSRF regressions.

src/tests/backend/specs/sofficeExportRemoteImage.ts

[Copilot]

Pull request overview

This PR hardens the LibreOffice (soffice) export conversion path by stripping remote <img> URLs from the HTML written to the temporary file, matching the existing defense-in-depth behavior already used by the native in-process export path. This reduces the risk of blind SSRF via plugin-injected export HTML when soffice performs outbound fetches during conversion.

Changes:

• Apply the existing stripRemoteImages() sanitizer to the HTML written to the soffice temp input file.
• Add a backend regression test that injects a remote image via exportHTMLAdditionalContent, captures the temp HTML via exportConvert, and asserts the remote URL is removed.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/node/handler/ExportHandler.ts	Sanitizes HTML with stripRemoteImages() before writing the soffice conversion temp file.
src/tests/backend/specs/sofficeExportRemoteImage.ts	Adds a regression test ensuring remote image URLs do not reach the soffice converter input HTML.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Sanitizer drops doctype/comments ✓ Resolved 🐞 Bug ≡ Correctness

Description

ExportHandler now runs stripRemoteImages() on the full export HTML for the soffice path, but
stripRemoteImages reserializes via htmlparser2 without emitting processing instructions or comments,
so the template’s "<!doctype html>" (and any HTML comments) are removed. This is a behavior change
from the prior verbatim temp file and can subtly affect LibreOffice’s HTML parsing/rendering for all
soffice exports.

Code

src/node/handler/ExportHandler.ts[R166-172]

+    // Strip remote <img> tags before handing the document to LibreOffice.
+    // soffice fetches remote image URLs during conversion, so any plugin/hook
+    // that injects an <img src="http://..."> into export HTML would otherwise
+    // turn export into a blind SSRF sink. The native path already does this
+    // (see stripRemoteImages above); apply it here so both paths match.
+    const {stripRemoteImages} = require('../utils/ExportSanitizeHtml');
+    await fsp_writeFile(srcFile, stripRemoteImages(html));

Evidence

The soffice export temp file is now written with the output of stripRemoteImages(html). The export
HTML template contains a ` declaration, but stripRemoteImages uses an htmlparser2 Parser` with
only onopentag, ontext, and onclosetag handlers—so processing instructions like doctype (and
comments) are not appended to the output string and will be lost when sanitizing a full document.

src/node/handler/ExportHandler.ts[162-173]
src/templates/export_html.html[1-10]
src/node/utils/ExportSanitizeHtml.ts[179-215]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ExportHandler` now writes `stripRemoteImages(html)` to the LibreOffice (soffice) temp file. `stripRemoteImages()` rebuilds markup via `htmlparser2` but does not preserve processing instructions (e.g., `<!doctype html>`) or comments, so the exported HTML handed to LibreOffice is no longer identical to the prior output aside from removed remote `<img>` tags.
### Issue Context
The export template includes `<!doctype html>` and other document-level nodes. `stripRemoteImages()` only handles open tags, text, and close tags, so document directives are dropped.
### Fix Focus Areas
- src/node/handler/ExportHandler.ts[162-173]
- src/node/utils/ExportSanitizeHtml.ts[179-215]
- src/templates/export_html.html[1-10]
### Suggested fix approaches
Choose one:
1) **Preserve doctype/comments in `stripRemoteImages()`**:
 - Add `onprocessinginstruction` and `oncomment` handlers to append the raw directive/comment to `out`.
 - Ensure doctype is preserved exactly once.
2) **Sanitize only the `<body>` content for soffice** while keeping the original document wrapper:
 - Extract the `<body>...</body>` portion, run `stripRemoteImages()` on that fragment, then splice it back into the original HTML document so the doctype/head stay intact.
 - If there is no `<body>`, fall back to sanitizing the whole string.
Either approach keeps the SSRF hardening while minimizing unintended output changes to the soffice conversion input.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[JohnMcLear]

Thanks @qodo-free-for-open-source-projects — valid. Running stripRemoteImages() on the full document dropped <!doctype html> and comments. Fixed in the latest commit by adding onprocessinginstruction (doctype) and oncomment handlers so the serializer round-trips document directives while still stripping remote images; added regression tests.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.