🔒️(backend) force SVG attachments to download to prevent stored XSS

[bunlongheng]

Summary

Add image/svg+xml to DOCUMENT_UNSAFE_MIME_TYPES so uploaded SVG attachments are flagged is_unsafe=true and served with Content-Disposition: attachment (forced download), consistent with the existing handling of other active-content types (HTML, PHP, etc.) in that list.

Context (updated after review)

The original description claimed a reproducible stored XSS. That was wrong - as @maboukerfa pointed out, all 3 reference deployment paths apply Content-Security-Policy: default-src 'none' to /media/ at the proxy layer (dev nginx config, production nginx template, and the Helm chart's ingressMedia annotation), which blocks script execution in inline-served SVGs. The severity claim is withdrawn.

Why the change may still be worth it (defense-in-depth)

• The protection lives entirely in the proxy layer. The Helm path applies it via nginx.ingress.kubernetes.io/configuration-snippet, and ingress-nginx ships with allow-snippet-annotations: false by default since v1.9, so on a cluster with default admission settings the annotation is not applied. Other ingress classes (Traefik, ALB, etc.) ignore it entirely.
• DOCUMENT_UNSAFE_MIME_TYPES already enforces the same policy for other active-content types at the application layer regardless of proxy config. This change makes SVG consistent with that policy, so the app stays safe even when a deployment's proxy layer differs from the reference configs.

Behaviour change

SVG uploads are stored with the -unsafe suffix and download instead of rendering inline. Raster images (png, jpeg, gif, webp) are unaffected.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1fca5819-3bd2-40ee-a19e-99fb73611466

📥 Commits

Reviewing files that changed from the base of the PR and between 44c51ff and be0c9d6.

📒 Files selected for processing (1)

• src/backend/impress/settings.py

---

Walkthrough

The unsafe document MIME-type configuration now includes image/svg+xml. SVG files are classified as unsafe document attachments.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Mergeability Score: ⚪ Minimal · up to be0c9

SVG attachments will be forced to download instead of rendering inline; no actionable merge-blocking risk remains at the current head after normal checks and review.

Suggested reviewers: lunika

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: SVG attachments will download instead of render inline. It also states the security purpose.
Description check	✅ Passed	The description directly explains the SVG MIME-type change, its application behavior, security context, and effect on raster images.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[maboukerfa]

I've already come across this, but i think this claim is wrong:

Attachment files are served by the object storage backend via the nginx media-auth subrequest, so the Content-Type / Content-Disposition come from S3 object metadata, and no CSP or nosniff header is applied to them.

You can check the nginx config here: https://github.com/suitenumerique/docs/blob/main/docker/files/etc/nginx/conf.d/default.conf

location /media/ {
    ...
    proxy_pass .../impress-media-storage/;
    add_header Content-Security-Policy "default-src 'none'" always;
}

So all scripting falls back to 'none'. The XSS can't be triggered, and your POC isn't reproducible.

[bunlongheng]

You're right, @maboukerfa - thank you for the correction. I missed the add_header Content-Security-Policy "default-src 'none'" always; on the /media/ location. I checked all 3 supported deployment paths and the header is present in each: the dev nginx config, the production nginx template (docker/files/production/etc/nginx/conf.d/default.conf.template), and the Helm chart's default ingressMedia annotation. So with default-src 'none', script execution in an inline-served SVG is blocked and the POC as I described it is not reproducible on a reference deployment. I withdraw the stored-XSS severity claim - that part of the PR description was wrong, and I've learned to check the proxy layer before claiming exploitability.

If it's still useful, here is the case for the one-line change as defense-in-depth rather than a vulnerability fix:

• The protection currently lives entirely in the proxy layer. The Helm path applies it via nginx.ingress.kubernetes.io/configuration-snippet, and ingress-nginx ships with allow-snippet-annotations: false by default since v1.9, so on a cluster with default admission settings that annotation is not applied. A different ingress class (Traefik, ALB, etc.) ignores it entirely.
• DOCUMENT_UNSAFE_MIME_TYPES already treats other active-content types (HTML, PHP, etc.) as unsafe at the application layer regardless of proxy config. Adding image/svg+xml just makes SVG consistent with that existing policy, so the app stays safe even when the proxy layer differs from the reference configs.

Happy to update the PR description to reposition it as hardening (and drop the incorrect POC), or close it if you feel the proxy-layer guarantee is sufficient. Either way, thanks for taking the time to verify instead of just merging - that's exactly the kind of review that makes this project solid.