fix(security): reject non-image content-types in image proxy and /picture/ to prevent stored XSS (#2067)

* fix(security): reject non-image content-types in image proxy and /picture/ to prevent stored XSS

The /api/v1/img proxy and /api/v1/picture/{user}/{id} endpoints emitted
http.DetectContentType on the served bytes as the response Content-Type. A
controlled upstream serving Content-Type: image/png with an HTML body passed
the upstream check (only the response header was inspected, not the body),
and the body bytes then sniffed back to text/html — so the proxy served the
attacker's HTML from the remark42 origin. Browsers honoured the declared
text/html and executed the response as a document with access to cookies and
CSRF tokens. Affected from v1.6.0 (April 2020) through v1.15.0; verified live
via published docker images.

Layered defense applied to both handlers:

- rest.SafeImgContentType (in backend/app/rest/) validates sniffed content
  against a strict allowlist: image/png, image/jpeg, image/gif, image/webp,
  image/bmp, image/x-icon. Anything else (HTML, XML, SVG, plain text,
  octet-stream, or any future image type the stdlib sniffer may learn) is
  rejected with no body echo. SVG is implicitly excluded — it sniffs as
  text/xml or text/plain, never image/svg+xml, and SVG can execute scripts
  when navigated to top-level. The previous octet-stream → image/* fallback
  is gone.
- Per-endpoint Content-Security-Policy override sets
  "default-src 'none'; sandbox; frame-ancestors 'none'" on every response
  (success, 304, or error). Sandbox neuters scripts even if Content-Type
  ever regresses. The same policy is also applied to all /api/v1/* via
  apiCSPMiddleware as defense-in-depth.
- Content-Disposition: inline; filename="image" frames the response as a
  file rather than a renderable document.
- /picture/ rejection paths set Cache-Control: no-store so 4xx responses
  are never cached.

The defense headers and the strict ETag matcher are extracted as
rest.SetImageDefenseHeaders and rest.EtagMatches in the shared rest package
(consumed by both proxy/image and api/rest_public — no package cycle).

The /api/v1/img path additionally bumps the ETag to a versioned `"v2:..."`
so revalidating clients (top-level navigation, Ctrl+R, intermediaries) get
a fresh 200 instead of a 304 against poisoned pre-fix cached HTML.

DELIBERATE TRADEOFF: Cache-Control on /api/v1/img success responses remains
max-age=2592000 (30 days), unchanged from before. An aggressive "force
revalidate on every reuse" policy was prototyped during review but reverted
because the perf cost (a server round-trip on every image view, even with
304 saving the body bytes) outweighed the corner-case mitigation. The
realistic exposure of cache carryover is narrow: cache carryover only
affects users who navigated top-level to an attacker URL pre-fix and still
have it in their local cache — the normal <img> embed path cached text/html
but never executed it. Local browser caches that hold pre-fix bytes
continue to serve them until their 30-day TTL expires or are evicted under
memory pressure. The ETag bump reaches all clients that DO revalidate
during the cached lifetime (Ctrl+R, intermediaries, post-expiry use); for
the rest, exposure self-limits via cache expiry. Operators running a
CDN/edge cache in front of remark42 should purge /api/v1/img after deploy.

The /api/v1/img handler short-circuits on a matching current-version
If-None-Match before any store Load or upstream fetch, returning a bodyless
304 with the defense headers set. Safe because the 304 carries no body and
the client's cached bytes came from a prior validated 200; an attacker
fabricating an etag value can only short-circuit fetches for URLs they
themselves crafted. This avoids upstream DoS amplification when clients
revalidate on hot comment pages.

The /api/v1/img route was moved from the "open routes" group (which uses
middleware.NoCache, stripping If-None-Match from incoming requests) to the
"open routes, cached" group alongside /picture/ and /qr/telegram so the
304 revalidation path is no longer broken upstream of the handler.

The /picture/{user}/{id} endpoint does not need the v2 etag prefix. Upload
validates input format via readAndValidateImage and the serve path
re-validates the stored bytes via rest.SafeImgContentType. Bytes within
the resize dimension limits are preserved verbatim, so the browser defense
relies on the response headers (validated Content-Type + nosniff + strict
CSP + Content-Disposition: inline), not on byte normalization.

Global CSP: font-src data: → font-src 'none'. Audit confirmed no @font-face,
no base64 fonts, no icon-font library in the bundle. Drops an unnecessary
attack surface; no behavioural change.

Tests: TestImage_ContentTypeHandling table-tests a real PNG and attack
shapes (HTML claimed as image/png, image/jpeg, image/gif, image/svg+xml,
image/webp; svg with onload; html fragment; polyglot PNG+HTML), proving
the defense holds across arbitrary upstream Content-Type variation.
Polyglot case is intentionally served as image/png — the browser cannot
execute the trailing HTML when the response type is image/png with nosniff.
TestImage_ContentTypeHandling_CacheHit exercises the cache-hit branch with
attacker bytes preloaded into the store. TestImage_PerRequestRevalidation
alternates upstream PNG/HTML across four proxy calls to prove no trust
accumulates between requests. TestImage_RoutesUsingCachedImage asserts
cache-poisoning is caught at serve time. TestImage_EtagVersioned asserts
the v2 prefix invalidates pre-fix etags AND that the revalidation 304
triggers no store Load. TestImage_RevalidationSkipsIO proves the
short-circuit works even with no upstream reachable. TestSafeImgContentType
covers the allowlist directly. TestRest_LoadPictureDefenseHeaders and
TestRest_LoadPictureRejectsNonImage exercise the /picture/ endpoint.
TestRest_apiCSP covers the strict CSP middleware on JSON API + RSS routes;
TestRest_securityHeaders confirms /web/ HTML pages keep the global CSP.

Verified end-to-end against the dev docker image: the original demo URL
(arbitrary HTML claimed as image/png) now returns 415 application/json with
CSP/nosniff/Content-Disposition set, no XSS in the browser.

* fix(security): set Cache-Control: no-store on image-proxy error paths, sync stale route comment

Addresses two review comments on #2067:

1. Cache-Control: max-age=2592000 and Etag were set before the
   load/download/validation block, so 404/400/415 error responses inherited
   the 30-day cache TTL and the versioned etag — a transient failure (or an
   intentionally triggered 415) would be pinned in browser/intermediary
   caches for that TTL, keeping users locked out even after the underlying
   cause was resolved. Now: etag is computed but not set as a header until
   after validation succeeds; error paths route through sendImageProxyError
   which sets Cache-Control: no-store and never sets Etag. The 304
   short-circuit still sets both because that path serves the same validated
   content the client already has cached.

2. The comment at rest.go:282 still described the prototyped
   no-cache/must-revalidate Cache-Control policy that was reverted before
   the PR landed. Updated to match the actual 30-day max-age behavior.

Tests: TestImage_ContentTypeHandling now asserts reject paths carry
Cache-Control: no-store and have no Etag header, and accept paths carry
the max-age=2592000 + v2: etag.

backend/app/rest/api/rest.go

@@ -249,6 +249,7 @@ func (s *Rest) routes() chi.Router {
 
 	// api routes
 	router.Route("/api/v1", func(rapi chi.Router) {
+		rapi.Use(apiCSPMiddleware)
 		rapi.Group(func(rava chi.Router) {
 			rava.Use(middleware.Timeout(5 * time.Second))
 			rava.Use(rateLimiter(100))
@@ -269,7 +270,6 @@ func (s *Rest) routes() chi.Router {
 			ropen.Post("/counts", s.pubRest.countMultiCtrl)
 			ropen.Get("/list", s.pubRest.listCtrl)
 			ropen.Get("/info", s.pubRest.infoCtrl)
-			ropen.Get("/img", s.ImageProxy.Handler)
 
 			ropen.Route("/rss", func(rrss chi.Router) {
 				rrss.Get("/post", s.rssRest.postCommentsCtrl)
@@ -278,11 +278,17 @@ func (s *Rest) routes() chi.Router {
 			})
 		})
 
-		// open routes, cached
+		// open routes, cached. /img lives here (not in the NoCache group above) because
+		// middleware.NoCache strips If-None-Match from incoming requests, which would
+		// defeat the proxy handler's 304 short-circuit. The handler sets a 30-day
+		// max-age on validated success responses (with a versioned etag for cache
+		// invalidation on revalidation); error responses get Cache-Control: no-store
+		// so transient failures aren't pinned in the cache.
 		rapi.Group(func(ropen chi.Router) {
 			ropen.Use(middleware.Timeout(30 * time.Second))
 			ropen.Use(rateLimiter(10))
 			ropen.Use(authMiddleware.Trace, logInfoWithBody)
+			ropen.Get("/img", s.ImageProxy.Handler)
 			ropen.Get("/picture/{user}/{id}", s.pubRest.loadPictureCtrl)
 			ropen.Get("/qr/telegram", s.pubRest.telegramQrCtrl)
 		})
@@ -620,6 +626,26 @@ func cacheControl(expiration time.Duration, version string) func(http.Handler) h
 	}
 }
 
+// apiCSPMiddleware overrides the global Content-Security-Policy on /api/v1 routes
+// with a strict, default-deny policy. The global CSP (securityHeadersMiddleware) keeps
+// 'self' 'unsafe-inline' for script-src/style-src because the widget HTML pages
+// (/web/*.html) need inline bootstrap blocks. API responses serve JSON, XML/RSS, or
+// images — none of those should ever execute scripts when rendered, so they get the
+// strictest policy available as defense-in-depth against future trust-boundary bugs.
+//
+// Image-serving handlers (/api/v1/img, /api/v1/picture/{user}/{id}) re-apply the same
+// rest.StrictImageCSP value at the handler level and additionally set Content-Disposition:
+// inline; filename="image" (framing the response as a file rather than a renderable
+// document) and X-Content-Type-Options: nosniff. The CSP re-apply is intentional belt-and-
+// braces: if a future route refactor bypasses this middleware, the image handlers still
+// emit the policy.
+func apiCSPMiddleware(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Security-Policy", rest.StrictImageCSP)
+		next.ServeHTTP(w, r)
+	})
+}
+
 // securityHeadersMiddleware sets security-related headers:
 //   - Content-Security-Policy: controls which resources the browser is allowed to load
 //   - Permissions-Policy: disables browser features (camera, mic, etc.) not needed by a comment widget
@@ -639,7 +665,8 @@ func securityHeadersMiddleware(imageProxyEnabled bool, allowedAncestors []string
 			if len(allowedAncestors) > 0 {
 				frameAncestors = strings.Join(allowedAncestors, " ")
 			}
-			w.Header().Set("Content-Security-Policy", fmt.Sprintf("default-src 'none'; base-uri 'none'; form-action 'none'; connect-src 'self'; frame-src 'self' mailto:; img-src %s; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; font-src data:; object-src 'none'; frame-ancestors %s;", imgSrc, frameAncestors))
+			// font-src is set to 'none' (no @font-face / no base64 fonts in the bundle).
+			w.Header().Set("Content-Security-Policy", fmt.Sprintf("default-src 'none'; base-uri 'none'; form-action 'none'; connect-src 'self'; frame-src 'self' mailto:; img-src %s; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; font-src 'none'; object-src 'none'; frame-ancestors %s;", imgSrc, frameAncestors))
 			w.Header().Set("Permissions-Policy", "accelerometer=(), autoplay=(), camera=(), cross-origin-isolated=(), display-capture=(), encrypted-media=(), fullscreen=(), geolocation=(), gyroscope=(), keyboard-map=(), magnetometer=(), microphone=(), midi=(), payment=(), picture-in-picture=(), publickey-credentials-get=(), screen-wake-lock=(), sync-xhr=(), usb=(), xr-spatial-tracking=(), clipboard-read=(), clipboard-write=(), gamepad=(), hid=(), idle-detection=(), interest-cohort=(), serial=(), unload=(), window-management=()")
 			w.Header().Set("X-Content-Type-Options", "nosniff")
 			w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")

backend/app/rest/api/rest_public.go

@@ -390,33 +390,56 @@ func safePictureSegment(seg string) bool {
 	return true
 }
 
+// sendPictureError writes a no-store Cache-Control header and delegates to rest.SendErrorJSON.
+// Used by every rejection branch in loadPictureCtrl so error responses never inherit the
+// 7-day client cache of the success path.
+func sendPictureError(w http.ResponseWriter, r *http.Request, status int, err error, details string, code int) {
+	w.Header().Set("Cache-Control", "no-store")
+	rest.SendErrorJSON(w, r, status, err, details, code)
+}
+
 // GET /picture/{user}/{id} - get picture
 func (s *public) loadPictureCtrl(w http.ResponseWriter, r *http.Request) {
+	rest.SetImageDefenseHeaders(w)
+
 	user, imgID := chi.URLParam(r, "user"), chi.URLParam(r, "id")
 	if user == "" || imgID == "" || !safePictureSegment(user) || !safePictureSegment(imgID) {
 		log.Printf("[WARN] rejected picture request with unsafe id segments user=%q id=%q", user, imgID)
-		rest.SendErrorJSON(w, r, http.StatusBadRequest, fmt.Errorf("invalid picture id"), "invalid picture id", rest.ErrAssetNotFound)
+		sendPictureError(w, r, http.StatusBadRequest, fmt.Errorf("invalid picture id"), "invalid picture id", rest.ErrAssetNotFound)
 		return
 	}
 	id := user + "/" + imgID
 	img, err := s.imageService.Load(id)
 	if err != nil {
 		log.Printf("[WARN] can't load image %s: %v", id, err)
-		rest.SendErrorJSON(w, r, http.StatusBadRequest, fmt.Errorf("image not found"), "can't get image", rest.ErrAssetNotFound)
+		sendPictureError(w, r, http.StatusBadRequest, fmt.Errorf("image not found"), "can't get image", rest.ErrAssetNotFound)
 		return
 	}
-	// enforce client-side caching
+
+	contentType, err := rest.SafeImgContentType(img)
+	if err != nil {
+		log.Printf("[WARN] rejecting non-image picture %s: %v", id, err)
+		sendPictureError(w, r, http.StatusUnsupportedMediaType, err, "invalid image content", rest.ErrAssetNotFound)
+		return
+	}
+
+	// /picture/ does not need a security-version etag prefix — the upload flow
+	// validates input format (readAndValidateImage) and the serve path re-validates
+	// the stored bytes via rest.SafeImgContentType. Bytes within the resize dimension
+	// limits ARE preserved verbatim by resize, so the browser defense relies on the
+	// response headers (validated Content-Type + nosniff + strict CSP +
+	// Content-Disposition: inline), not on byte normalization. Picture IDs are limited
+	// to safePictureSegment (alphanumeric xid-generated guids), so the comma split
+	// inside rest.EtagMatches cannot collide; if the ID format ever changes, revisit.
 	etag := `"` + id + `"`
 	w.Header().Set("Etag", etag)
 	w.Header().Set("Cache-Control", "max-age=604800") // 7 days
-	if match := r.Header.Get("If-None-Match"); match != "" {
-		if strings.Contains(match, etag) {
-			w.WriteHeader(http.StatusNotModified)
-			return
-		}
+	if match := r.Header.Get("If-None-Match"); match != "" && rest.EtagMatches(match, etag) {
+		w.WriteHeader(http.StatusNotModified)
+		return
 	}
 
-	w.Header().Set("Content-Type", s.imageService.ImgContentType(img))
+	w.Header().Set("Content-Type", contentType)
 	w.Header().Set("Content-Length", strconv.Itoa(len(img)))
 	w.WriteHeader(http.StatusOK)
 	if _, err = io.Copy(w, bytes.NewReader(img)); err != nil {

backend/app/rest/api/rest_public_test.go

@@ -1,9 +1,11 @@
 package api
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io"
+	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -12,6 +14,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/go-chi/chi/v5"
 	cache "github.com/go-pkgz/lcw/v2"
 	R "github.com/go-pkgz/rest"
 	"github.com/google/uuid"
@@ -19,6 +22,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/umputun/remark42/backend/app/store"
+	"github.com/umputun/remark42/backend/app/store/image"
 	"github.com/umputun/remark42/backend/app/store/service"
 )
 
@@ -1104,3 +1108,113 @@ func TestRest_LoadPictureRejectsControlCharsInSegment(t *testing.T) {
 		})
 	}
 }
+
+// TestRest_LoadPictureDefenseHeaders saves a real PNG via the standard upload handler
+// and asserts that GET /api/v1/picture/{user}/{id} carries the layered defense headers
+// (strict CSP, nosniff, Content-Disposition with filename) and that the strict ETag
+// matcher does not 304 on a substring-of-the-real-etag (the pre-fix matcher would).
+func TestRest_LoadPictureDefenseHeaders(t *testing.T) {
+	ts, _, teardown := startupT(t)
+	defer teardown()
+
+	// upload a real PNG via /api/v1/picture
+	bodyBuf := &bytes.Buffer{}
+	bodyWriter := multipart.NewWriter(bodyBuf)
+	fileWriter, err := bodyWriter.CreateFormFile("file", "picture.png")
+	require.NoError(t, err)
+	_, err = io.Copy(fileWriter, gopherPNG())
+	require.NoError(t, err)
+	contentType := bodyWriter.FormDataContentType()
+	require.NoError(t, bodyWriter.Close())
+
+	client := http.Client{}
+	defer client.CloseIdleConnections()
+	req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/api/v1/picture?site=remark42", ts.URL), bodyBuf)
+	require.NoError(t, err)
+	req.Header.Add("Content-Type", contentType)
+	req.Header.Add("X-JWT", devToken)
+	resp, err := client.Do(req)
+	require.NoError(t, err)
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	require.NoError(t, resp.Body.Close())
+	m := map[string]string{}
+	require.NoError(t, json.Unmarshal(body, &m))
+	require.NotEmpty(t, m["id"])
+
+	// fetch the picture and assert defense headers
+	resp, err = http.Get(fmt.Sprintf("%s/api/v1/picture/%s", ts.URL, m["id"]))
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	assert.Equal(t, "default-src 'none'; sandbox; frame-ancestors 'none'",
+		resp.Header.Get("Content-Security-Policy"))
+	assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"))
+	assert.Equal(t, `inline; filename="image"`, resp.Header.Get("Content-Disposition"))
+	assert.Equal(t, "image/png", resp.Header.Get("Content-Type"))
+	realEtag := resp.Header.Get("Etag")
+	require.NotEmpty(t, realEtag)
+
+	// strict matcher: an If-None-Match value that CONTAINS the real etag as a substring
+	// but is not equal to it must NOT trigger 304. The pre-fix matcher used
+	// strings.Contains(header, etag) and would have returned true here.
+	require.True(t, len(realEtag) > 4)
+	substringMatch := "prefix-" + realEtag + "-suffix"
+	req2, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/v1/picture/%s", ts.URL, m["id"]), http.NoBody)
+	require.NoError(t, err)
+	req2.Header.Set("If-None-Match", substringMatch)
+	resp2, err := client.Do(req2)
+	require.NoError(t, err)
+	defer resp2.Body.Close()
+	assert.Equal(t, http.StatusOK, resp2.StatusCode,
+		"strict etag matcher must NOT 304 when real etag appears only as a substring of If-None-Match; got %q vs real %q", substringMatch, realEtag)
+
+	// sanity: the exact real etag DOES validate
+	req3, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/v1/picture/%s", ts.URL, m["id"]), http.NoBody)
+	require.NoError(t, err)
+	req3.Header.Set("If-None-Match", realEtag)
+	resp3, err := client.Do(req3)
+	require.NoError(t, err)
+	defer resp3.Body.Close()
+	assert.Equal(t, http.StatusNotModified, resp3.StatusCode, "exact etag must round-trip as 304")
+}
+
+// TestRest_LoadPictureRejectsNonImage proves the /picture/ handler rejects bytes that
+// don't sniff as a real image — even when retrieved successfully from the image store.
+// Uses a StoreMock so we can return arbitrary attacker bytes for a valid-looking id.
+func TestRest_LoadPictureRejectsNonImage(t *testing.T) {
+	htmlBody := []byte("<html><body><script>alert(document.domain)</script></body></html>")
+
+	imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) {
+		return htmlBody, nil
+	}}
+	// minimal public struct on purpose: the reject path only exercises imageService.Load
+	// (other fields like dataService, cache, commentFormatter are not touched here).
+	p := &public{imageService: image.NewService(&imageStore, image.ServiceParams{})}
+
+	router := chi.NewRouter()
+	router.Get("/api/v1/picture/{user}/{id}", p.loadPictureCtrl)
+	ts := httptest.NewServer(router)
+	defer ts.Close()
+
+	resp, err := http.Get(ts.URL + "/api/v1/picture/dev_user/abc.png")
+	require.NoError(t, err)
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	require.NoError(t, resp.Body.Close())
+
+	assert.Equal(t, http.StatusUnsupportedMediaType, resp.StatusCode,
+		"non-image bytes must be rejected as 415")
+	assert.False(t, strings.HasPrefix(resp.Header.Get("Content-Type"), "text/html"),
+		"reject response must not be text/html; got %q", resp.Header.Get("Content-Type"))
+	assert.NotContains(t, string(body), "<script>",
+		"attacker payload must not be echoed back")
+	assert.Equal(t, "no-store", resp.Header.Get("Cache-Control"),
+		"rejection path must not be cacheable")
+	// defense headers still present on the reject path
+	assert.Equal(t, "default-src 'none'; sandbox; frame-ancestors 'none'",
+		resp.Header.Get("Content-Security-Policy"))
+	assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"))
+	assert.Equal(t, `inline; filename="image"`, resp.Header.Get("Content-Disposition"))
+}

backend/app/rest/api/rest_test.go

@@ -341,6 +341,44 @@ func TestRest_frameAncestors(t *testing.T) {
 	assert.Contains(t, resp.Header.Get("Content-Security-Policy"), "frame-ancestors *;")
 }
 
+// TestRest_apiCSP locks in that /api/v1/* responses get a strict default-src 'none'
+// override regardless of what the global CSP allows. The widget HTML pages
+// (/web/*.html) still get the global CSP (with 'unsafe-inline' for bootstrap),
+// so the test asserts the two policies diverge across origins.
+func TestRest_apiCSP(t *testing.T) {
+	ts, _, teardown := startupT(t)
+	defer teardown()
+	client := http.Client{}
+
+	// JSON API endpoint — must carry the strict policy
+	resp, err := client.Get(ts.URL + "/api/v1/config")
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	csp := resp.Header.Get("Content-Security-Policy")
+	assert.Contains(t, csp, "default-src 'none'",
+		"API responses must override the global CSP with default-src 'none'; got %q", csp)
+	assert.Contains(t, csp, "sandbox", "API CSP must include sandbox; got %q", csp)
+	assert.NotContains(t, csp, "'unsafe-inline'",
+		"API CSP must not allow inline scripts/styles; got %q", csp)
+
+	// RSS/XML endpoint — same strict policy, and the XML response itself must still be served
+	respRSS, err := client.Get(ts.URL + "/api/v1/rss/site?site=remark42")
+	require.NoError(t, err)
+	defer respRSS.Body.Close()
+	assert.Equal(t, http.StatusOK, respRSS.StatusCode, "RSS must still respond OK under strict CSP")
+	cspRSS := respRSS.Header.Get("Content-Security-Policy")
+	assert.Contains(t, cspRSS, "default-src 'none'", "RSS responses must carry the strict API CSP")
+	assert.Contains(t, cspRSS, "sandbox", "RSS CSP must include sandbox")
+
+	// widget HTML — must keep the global CSP (unchanged, lax to support inline bootstrap)
+	resp2, err := client.Get(ts.URL + "/web/index.html")
+	require.NoError(t, err)
+	defer resp2.Body.Close()
+	csp2 := resp2.Header.Get("Content-Security-Policy")
+	assert.Contains(t, csp2, "'unsafe-inline'",
+		"widget HTML CSP must keep unsafe-inline for bootstrap; got %q", csp2)
+}
+
 // check CSP, img-src should be 'self' with proxy enabled and * without it
 func TestRest_securityHeaders(t *testing.T) {
 	ts, _, teardown := startupT(t)

backend/app/rest/image_headers.go

@@ -0,0 +1,76 @@
+package rest
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+)
+
+// StrictImageCSP is the strictest default-deny Content-Security-Policy used both by
+// image-serving handlers (/api/v1/img, /api/v1/picture/{user}/{id}) and by the api-wide
+// apiCSPMiddleware (covering all /api/v1/* responses — JSON, XML/RSS, images). The name
+// keeps the "image" prefix for historical reasons; the policy itself is generic and
+// suitable for any non-document API response.
+//
+// Re-setting the same value inside the image handlers (after the middleware already set
+// it) is intentional defense-in-depth: if the middleware ever stops applying (route
+// refactor, mount point change), the handlers still emit the header.
+const StrictImageCSP = "default-src 'none'; sandbox; frame-ancestors 'none'"
+
+// SafeImgContentType returns the sniffed content type for provided bytes if and only
+// if it is in the strict allowlist of image formats safe to serve from a same-origin
+// proxy endpoint: image/png, image/jpeg, image/gif, image/webp, image/bmp, image/x-icon.
+// Anything else — HTML, XML, SVG, plain text, application/octet-stream, or any future
+// image format the stdlib sniffer may learn (e.g. AVIF, HEIC, JXL, TIFF) — is rejected.
+// SVG would also be rejected as it sniffs as text/xml or text/plain, never image/svg+xml.
+// The previous behavior silently mapped application/octet-stream to image/* and is gone.
+func SafeImgContentType(img []byte) (string, error) {
+	contentType := http.DetectContentType(img)
+	base, _, _ := strings.Cut(contentType, ";")
+	base = strings.TrimSpace(base)
+	switch base {
+	case "image/png", "image/jpeg", "image/gif", "image/webp", "image/bmp", "image/x-icon":
+		return base, nil
+	}
+	return "", fmt.Errorf("non-image content type %q", contentType)
+}
+
+// SetImageDefenseHeaders applies the layered defense headers shared by every response
+// from image-serving endpoints (success, 304, or error). Each header survives content-type
+// validation regressions, browser sniffing, and top-level navigation:
+//   - Content-Security-Policy: strict, with sandbox — blocks inline scripts and event handlers
+//   - X-Content-Type-Options: nosniff — prevents browsers from MIME-overriding the declared type
+//   - Content-Disposition: inline; filename="image" — frames the response as a file, not a document
+//
+// CSP is duplicated by apiCSPMiddleware for /api/v1/* — re-setting the same value here is
+// harmless and provides defense-in-depth if the middleware is bypassed or moved. The other
+// two headers (nosniff, Content-Disposition with filename) are image-specific and not set
+// by the middleware.
+func SetImageDefenseHeaders(w http.ResponseWriter) {
+	w.Header().Set("Content-Security-Policy", StrictImageCSP)
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.Header().Set("Content-Disposition", `inline; filename="image"`)
+}
+
+// EtagMatches reports whether If-None-Match header value contains the given etag.
+// Handles the * wildcard, comma-separated etag lists with the W/ weak-validator prefix.
+// NOTE: This is intentionally a simple splitter — it does not handle opaque-tags that
+// contain commas (allowed by RFC 7232 but never emitted by this codebase, whose etag
+// format is `"v2:<base64-url>"` or `"<user>/<xid>"`). If the etag format ever changes
+// to include comma-bearing values, revisit this parser.
+// Replaces a substring search that could match unrelated entries (e.g. an etag that
+// happens to be a prefix of another).
+func EtagMatches(header, etag string) bool {
+	header = strings.TrimSpace(header)
+	if header == "*" {
+		return true
+	}
+	for tag := range strings.SplitSeq(header, ",") {
+		tag = strings.TrimSpace(tag)
+		tag = strings.TrimPrefix(tag, "W/")
+		if tag == etag {
+			return true
+		}
+	}
+	return false
+}

backend/app/rest/image_headers_test.go

@@ -0,0 +1,103 @@
+package rest
+
+import (
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestEtagMatches covers the strict If-None-Match parser that replaced a substring
+// search prone to false positives (etag "abc" being matched inside "fooabc").
+func TestEtagMatches(t *testing.T) {
+	tbl := []struct {
+		name   string
+		header string
+		etag   string
+		want   bool
+	}{
+		{"exact match", `"v2:abc"`, `"v2:abc"`, true},
+		{"comma-separated, second matches", `"x", "v2:abc"`, `"v2:abc"`, true},
+		{"weak validator prefix", `W/"v2:abc"`, `"v2:abc"`, true},
+		{"wildcard matches anything", `*`, `"v2:abc"`, true},
+		{"leading/trailing whitespace", `   "v2:abc"   `, `"v2:abc"`, true},
+		{"substring not enough", `"v2:abcdef"`, `"v2:abc"`, false},
+		{"prefix-only mismatch", `"v2:ab"`, `"v2:abc"`, false},
+		{"pre-fix etag no longer matches v2", `"abc"`, `"v2:abc"`, false},
+		{"empty header", ``, `"v2:abc"`, false},
+		{"different etag", `"v2:xyz"`, `"v2:abc"`, false},
+	}
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, EtagMatches(tt.header, tt.etag))
+		})
+	}
+}
+
+func TestSetImageDefenseHeaders(t *testing.T) {
+	w := httptest.NewRecorder()
+	SetImageDefenseHeaders(w)
+	assert.Equal(t, StrictImageCSP, w.Header().Get("Content-Security-Policy"))
+	assert.Equal(t, "nosniff", w.Header().Get("X-Content-Type-Options"))
+	assert.Equal(t, `inline; filename="image"`, w.Header().Get("Content-Disposition"))
+}
+
+// TestSafeImgContentType exercises the strict allowlist. The previous behavior
+// (HasPrefix "image/" with an explicit image/svg+xml carve-out) is gone — the
+// allowlist is the source of truth, and the explicit svg branch was dead code
+// because http.DetectContentType never returns image/svg+xml (real SVG bodies
+// sniff as text/xml or text/plain depending on whether they carry an XML decl,
+// so they are rejected implicitly by not matching the allowlist).
+func TestSafeImgContentType(t *testing.T) {
+	// minimal magic-byte bodies — verified via http.DetectContentType to produce
+	// the expected image/* result without needing testdata files for every format
+	pngMagic := []byte("\x89PNG\r\n\x1a\n")
+	jpegMagic := []byte("\xff\xd8\xff\xe0\x00\x10JFIF\x00")
+	gifBytes := []byte("GIF89a")
+	webpBytes := []byte("RIFF\x00\x00\x00\x00WEBPVP8 ")
+	bmpBytes := []byte("BM\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")
+	icoBytes := []byte("\x00\x00\x01\x00\x01\x00")
+
+	// SVG with XML decl sniffs as text/xml — rejected because it's not in the allowlist
+	svgWithXMLDecl := []byte(`<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg" onload="alert(1)"></svg>`)
+	// SVG without XML decl sniffs as text/plain — also rejected
+	svgPlain := []byte(`<svg xmlns="http://www.w3.org/2000/svg" width="10"></svg>`)
+
+	tbl := []struct {
+		name    string
+		body    []byte
+		wantCT  string
+		wantErr bool
+	}{
+		{name: "nil rejected", body: nil, wantErr: true},
+		{name: "empty rejected", body: []byte{}, wantErr: true},
+		{name: "png magic accepted", body: pngMagic, wantCT: "image/png"},
+		{name: "jpeg magic accepted", body: jpegMagic, wantCT: "image/jpeg"},
+		{name: "gif accepted", body: gifBytes, wantCT: "image/gif"},
+		{name: "webp accepted", body: webpBytes, wantCT: "image/webp"},
+		{name: "bmp accepted", body: bmpBytes, wantCT: "image/bmp"},
+		{name: "ico accepted", body: icoBytes, wantCT: "image/x-icon"},
+		{name: "html doc rejected", body: []byte(`<!DOCTYPE html><html></html>`), wantErr: true},
+		{name: "html fragment rejected", body: []byte(`<body><img></body>`), wantErr: true},
+		{name: "plain text rejected", body: []byte("hello world"), wantErr: true},
+		{name: "octet-stream rejected", body: []byte{0x00, 0x01, 0x02, 0x03, 0x04}, wantErr: true},
+		{name: "svg with xml decl rejected (sniffs as text/xml)", body: svgWithXMLDecl, wantErr: true},
+		{name: "svg without xml decl rejected (sniffs as text/plain)", body: svgPlain, wantErr: true},
+	}
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := SafeImgContentType(tt.body)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Empty(t, got)
+				assert.Contains(t, err.Error(), "non-image content type")
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantCT, got)
+			// returned type must never carry a charset suffix (the strip code path)
+			assert.NotContains(t, got, ";")
+		})
+	}
+}

backend/app/rest/proxy/image.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -19,6 +20,11 @@ import (
 	"github.com/umputun/remark42/backend/app/store/image"
 )
 
+// errInvalidUpstreamContentType is returned by downloadImage when the upstream's
+// Content-Type header is not image/*. The handler checks via errors.Is to convert
+// it into a 400 (input rejected) instead of the generic 404 (fetch failed).
+var errInvalidUpstreamContentType = errors.New("invalid upstream content type")
+
 // Image extracts image src from comment's html and provides proxy for them
 // this is needed to keep remark42 running behind of HTTPS serve all images via https
 type Image struct {
@@ -84,32 +90,67 @@ func (p Image) replace(commentHTML string, imgs []string) string {
 	return commentHTML
 }
 
+// etagVersionPrefix is the security-version tag bumped whenever cached responses for the
+// same src need to be invalidated. Pre-fix responses were served as text/html and cached
+// by browsers/proxies under ETag `"<base64(src)>"`; the prefix invalidates those validators
+// so revalidating clients get a fresh 200 instead of letting the cached HTML 304.
+//
+// LIMITATION: with the 30-day max-age below, browsers serve pre-fix bytes from their
+// local cache without contacting the server until that TTL expires or the cache is
+// evicted under memory pressure. The prefix only helps clients that revalidate during
+// the cached lifetime (Ctrl+R, intermediaries, post-expiry use). Operators running a
+// CDN/edge cache in front of remark42 should purge /api/v1/img after deploy. The
+// realistic exposure is narrow: cache carryover only affects users who navigated
+// top-level to an attacker URL pre-fix and still have that URL cached — the normal
+// <img> embed path cached text/html but never executed it.
+const etagVersionPrefix = "v2:"
+
 // Handler returns http handler respond to proxied request
 func (p Image) Handler(w http.ResponseWriter, r *http.Request) {
-	src, err := base64.URLEncoding.DecodeString(r.URL.Query().Get("src"))
+	rest.SetImageDefenseHeaders(w)
+
+	srcParam := r.URL.Query().Get("src")
+	src, err := base64.URLEncoding.DecodeString(srcParam)
 	if err != nil {
-		rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't decode image url", rest.ErrDecode)
+		sendImageProxyError(w, r, http.StatusBadRequest, err, "can't decode image url", rest.ErrDecode)
 		return
 	}
 
 	imgURL := string(src)
-	var img []byte
 	imgID, err := image.CachedImgID(imgURL)
 	if err != nil {
-		rest.SendErrorJSON(w, r, http.StatusBadRequest, fmt.Errorf("invalid image url"), "can't parse image url", rest.ErrAssetNotFound)
+		sendImageProxyError(w, r, http.StatusBadRequest, fmt.Errorf("invalid image url"), "can't parse image url", rest.ErrAssetNotFound)
 		return
 	}
+
+	// compute the current-version etag once. We don't set it as a response header yet
+	// because error paths below must NOT inherit it — otherwise transient failures
+	// (4xx) would get cached alongside the 30-day Cache-Control of the success path.
+	// The etag (and Cache-Control) are set only on the 304 short-circuit and the
+	// validated 200 path.
+	etag := `"` + etagVersionPrefix + srcParam + `"`
+	// short-circuit revalidation before any cache lookup or upstream fetch: a matching
+	// current-version If-None-Match means the client already has bytes from a prior
+	// successful (post-fix, validated) 200, so a bodyless 304 is safe and avoids
+	// upstream DoS amplification on hot comment pages without CacheExternal.
+	if match := r.Header.Get("If-None-Match"); match != "" && rest.EtagMatches(match, etag) {
+		w.Header().Set("Etag", etag)
+		w.Header().Set("Cache-Control", "max-age=2592000") // 30 days
+		w.WriteHeader(http.StatusNotModified)
+		return
+	}
+
 	// try to load from cache for case it was saved when CacheExternal was enabled
-	img, _ = p.ImageService.Load(imgID)
+	img, _ := p.ImageService.Load(imgID)
 	if img == nil {
-		img, err = p.downloadImage(context.Background(), imgURL)
+		img, err = p.downloadImage(r.Context(), imgURL)
 		if err != nil {
 			log.Printf("[WARN] failed to download image: %v", err)
-			if strings.Contains(err.Error(), "invalid content type") {
-				rest.SendErrorJSON(w, r, http.StatusBadRequest, fmt.Errorf("invalid content type"), "invalid content type", rest.ErrImgNotFound)
+			if errors.Is(err, errInvalidUpstreamContentType) {
+				sendImageProxyError(w, r, http.StatusBadRequest, fmt.Errorf("invalid content type"), "invalid content type", rest.ErrImgNotFound)
 				return
 			}
-			rest.SendErrorJSON(w, r, http.StatusNotFound, fmt.Errorf("failed to fetch"), "can't get image", rest.ErrAssetNotFound)
+			sendImageProxyError(w, r, http.StatusNotFound, fmt.Errorf("failed to fetch"), "can't get image", rest.ErrAssetNotFound)
 			return
 		}
 		if p.CacheExternal {
@@ -117,24 +158,37 @@ func (p Image) Handler(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	// enforce client-side caching
-	etag := `"` + r.URL.Query().Get("src") + `"`
-	w.Header().Set("Etag", etag)
-	w.Header().Set("Cache-Control", "max-age=2592000") // 30 days
-	if match := r.Header.Get("If-None-Match"); match != "" {
-		if strings.Contains(match, etag) {
-			w.WriteHeader(http.StatusNotModified)
-			return
-		}
+	// validate body bytes are actually an image — never trust upstream Content-Type or cache
+	contentType, err := rest.SafeImgContentType(img)
+	if err != nil {
+		log.Printf("[WARN] rejecting non-image content from %s: %v", imgURL, err)
+		sendImageProxyError(w, r, http.StatusUnsupportedMediaType, err, "invalid image content", rest.ErrImgNotFound)
+		return
 	}
 
-	w.Header().Add("Content-Type", p.ImageService.ImgContentType(img))
+	// success path: long-lived client cache with etag for cheap revalidation. 30-day
+	// TTL keeps the proxy efficient for hot pages; when clients DO revalidate
+	// (Ctrl+R, intermediaries, post-expiry), the versioned etag ensures pre-fix
+	// poisoned validators don't match and a fresh validated 200 is returned. See
+	// etagVersionPrefix godoc for the limitation on browser-local caches.
+	w.Header().Set("Etag", etag)
+	w.Header().Set("Cache-Control", "max-age=2592000") // 30 days
+	w.Header().Set("Content-Type", contentType)
 	_, err = io.Copy(w, bytes.NewReader(img))
 	if err != nil {
 		log.Printf("[WARN] can't copy image stream, %s", err)
 	}
 }
 
+// sendImageProxyError writes a no-store error response so a transient failure (4xx)
+// cannot inherit the success path's 30-day Cache-Control or the versioned ETag, which
+// would otherwise pin the error in the browser/intermediary cache for that TTL.
+// Defense headers from SetImageDefenseHeaders at the top of the handler survive.
+func sendImageProxyError(w http.ResponseWriter, r *http.Request, status int, err error, details string, errCode int) {
+	w.Header().Set("Cache-Control", "no-store")
+	rest.SendErrorJSON(w, r, status, err, details, errCode)
+}
+
 // cache image from provided Reader using given ID
 func (p Image) cacheImage(r io.Reader, imgID string) {
 	err := p.ImageService.SaveWithID(imgID, r)
@@ -188,7 +242,7 @@ func (p Image) downloadImage(ctx context.Context, imgURL string) ([]byte, error)
 
 	contentType := resp.Header.Get("Content-Type")
 	if !strings.HasPrefix(contentType, "image/") {
-		return nil, fmt.Errorf("invalid content type %s", contentType)
+		return nil, fmt.Errorf("%w: %s", errInvalidUpstreamContentType, contentType)
 	}
 
 	maxSize := 5 * 1024 * 1024 // 5MB default

backend/app/rest/proxy/image_test.go

@@ -210,34 +210,59 @@ func TestImage_RoutesCachingImage(t *testing.T) {
 }
 
 func TestImage_RoutesUsingCachedImage(t *testing.T) {
-	// in order to validate that cached data used cache "will return" some other data from what http server would
-	testImage := fmt.Appendf(nil, "%256s", "X")
-	imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) {
-		return testImage, nil
-	}}
-	img := Image{
-		CacheExternal: true,
-		RemarkURL:     "https://demo.remark42.com",
-		RoutePath:     "/api/v1/proxy",
-		ImageService:  image.NewService(&imageStore, image.ServiceParams{}),
-	}
+	t.Run("cached image is served", func(t *testing.T) {
+		imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) {
+			return gopherPNGBytes(), nil
+		}}
+		img := Image{
+			CacheExternal: true,
+			RemarkURL:     "https://demo.remark42.com",
+			RoutePath:     "/api/v1/proxy",
+			ImageService:  image.NewService(&imageStore, image.ServiceParams{}),
+		}
 
-	ts := httptest.NewServer(http.HandlerFunc(img.Handler))
-	defer ts.Close()
-	httpSrv := imgHTTPTestsServer(t)
-	defer httpSrv.Close()
+		ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+		defer ts.Close()
+		httpSrv := imgHTTPTestsServer(t)
+		defer httpSrv.Close()
 
-	encodedImgURL := base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/img1.png"))
+		encodedImgURL := base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/img1.png"))
+		resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
+		require.NoError(t, err)
+		assert.NoError(t, resp.Body.Close())
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+		assert.Equal(t, "image/png", resp.Header.Get("Content-Type"))
+		assert.Equal(t, 1, len(imageStore.LoadCalls()))
+	})
 
-	resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
-	require.NoError(t, err)
-	assert.NoError(t, resp.Body.Close())
-	assert.Equal(t, http.StatusOK, resp.StatusCode)
-	assert.Equal(t, "256", resp.Header["Content-Length"][0])
-	assert.Equal(t, "text/plain; charset=utf-8", resp.Header["Content-Type"][0],
-		"if you save text you receive text/plain in response, that's only fair option you got")
+	t.Run("non-image cached bytes are rejected (cache poisoning defense)", func(t *testing.T) {
+		nonImage := fmt.Appendf(nil, "%256s", "X")
+		imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) {
+			return nonImage, nil
+		}}
+		img := Image{
+			CacheExternal: true,
+			RemarkURL:     "https://demo.remark42.com",
+			RoutePath:     "/api/v1/proxy",
+			ImageService:  image.NewService(&imageStore, image.ServiceParams{}),
+		}
 
-	assert.Equal(t, 1, len(imageStore.LoadCalls()))
+		ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+		defer ts.Close()
+		httpSrv := imgHTTPTestsServer(t)
+		defer httpSrv.Close()
+
+		encodedImgURL := base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/img1.png"))
+		resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL)
+		require.NoError(t, err)
+		body, _ := io.ReadAll(resp.Body)
+		assert.NoError(t, resp.Body.Close())
+		assert.Equal(t, http.StatusUnsupportedMediaType, resp.StatusCode,
+			"non-image bytes from cache must be rejected, not served as text/plain (XSS defense)")
+		assert.False(t, strings.HasPrefix(resp.Header.Get("Content-Type"), "text/html"),
+			"reject response must not be text/html; got %q", resp.Header.Get("Content-Type"))
+		assert.NotContains(t, string(body), "XXXXX", "non-image bytes must not be echoed back")
+	})
 }
 
 func TestImage_RoutesTimedOut(t *testing.T) {
@@ -420,6 +445,363 @@ func TestImage_ResponseSizeLimit(t *testing.T) {
 	assert.Contains(t, string(b), "failed to fetch")
 }
 
+// TestImage_ContentTypeHandling covers both the rock-solid acceptance of legitimate
+// images and the rejection of content-type-spoofing payloads (the XSS vector where
+// upstream lies about Content-Type and the proxy serves attacker HTML back from the
+// remark42 origin). Every response — accept or reject — must carry the layered
+// defense headers (strict CSP, nosniff, Content-Disposition: inline).
+//
+// The defense must not depend on the upstream Content-Type header: each row controls
+// it independently of the body so the matrix exercises attackers who flip the upstream
+// header on the fly, and polyglot bodies where image magic bytes prefix HTML payloads.
+func TestImage_ContentTypeHandling(t *testing.T) {
+	htmlBody := []byte("<html><body><script>alert(document.domain)</script></body></html>")
+	// polyglot: real PNG magic + trailing HTML. Sniffs as image/png, must be served
+	// as image/png so the browser renders as image (broken or otherwise) — never as HTML.
+	polyglot := append(append([]byte{}, gopherPNGBytes()...), []byte("<script>alert(1)</script>")...)
+
+	tbl := []struct {
+		name          string
+		upstreamCT    string // Content-Type header the upstream sends
+		body          []byte
+		accept        bool   // true: legitimate image, served back; false: attack, rejected
+		wantCT        string // exact Content-Type if accept
+		payloadMarker string // attack substring that must NOT appear in the response body
+	}{
+		// legitimate
+		{name: "real png", upstreamCT: "image/png", body: gopherPNGBytes(), accept: true, wantCT: "image/png"},
+
+		// upstream lies — body is HTML, header varies. All must be rejected at body-sniff.
+		{name: "html body claimed as image/png", upstreamCT: "image/png", body: htmlBody, payloadMarker: "<script>"},
+		{name: "html body claimed as image/jpeg", upstreamCT: "image/jpeg", body: htmlBody, payloadMarker: "<script>"},
+		{name: "html body claimed as image/gif", upstreamCT: "image/gif", body: htmlBody, payloadMarker: "<script>"},
+		// upstream claims svg+xml; body still sniffs as text/html (the stdlib sniffer
+		// never returns image/svg+xml, see rest.SafeImgContentType godoc).
+		{name: "html body upstream claims image/svg+xml", upstreamCT: "image/svg+xml", body: htmlBody, payloadMarker: "<script>"},
+		{name: "html body claimed as image/webp", upstreamCT: "image/webp", body: htmlBody, payloadMarker: "<script>"},
+
+		// svg payloads — even if upstream claims a valid image format, the sniffer sees XML/text and we must reject
+		{
+			name:          "svg with xml declaration and onload",
+			upstreamCT:    "image/png",
+			body:          []byte(`<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg" onload="alert(1)"></svg>`),
+			payloadMarker: "onload",
+		},
+		{
+			name:          "html fragment without doctype",
+			upstreamCT:    "image/png",
+			body:          []byte(`<body><img src=x onerror=alert(1)></body>`),
+			payloadMarker: "onerror",
+		},
+
+		// polyglot — image magic + appended HTML. Sniffs as image/png so we accept and serve as image/png.
+		// Safety comes from the response headers (Content-Type: image/png + X-Content-Type-Options: nosniff),
+		// not from body filtering: the bytes round-trip verbatim by design (assertion below). The browser
+		// cannot execute the trailing HTML when the response type is image/png with nosniff.
+		{name: "polyglot png+html served as png", upstreamCT: "image/png", body: polyglot, accept: true, wantCT: "image/png"},
+	}
+
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				w.Header().Set("Content-Type", tt.upstreamCT)
+				_, _ = w.Write(tt.body)
+			}))
+			defer upstream.Close()
+
+			imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) { return nil, nil }}
+			img := Image{
+				RemarkURL:    "https://demo.remark42.com",
+				RoutePath:    "/api/v1/proxy",
+				ImageService: image.NewService(&imageStore, image.ServiceParams{}),
+				Transport:    http.DefaultTransport,
+			}
+
+			ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+			defer ts.Close()
+
+			encodedURL := base64.URLEncoding.EncodeToString([]byte(upstream.URL + "/logo.png"))
+			resp, err := http.Get(ts.URL + "/?src=" + encodedURL)
+			require.NoError(t, err)
+			body, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.NoError(t, resp.Body.Close())
+
+			// every response — accept or reject — must carry the defense headers
+			assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"))
+			assert.Contains(t, resp.Header.Get("Content-Disposition"), "inline")
+			csp := resp.Header.Get("Content-Security-Policy")
+			assert.Contains(t, csp, "default-src 'none'", "strict CSP missing")
+			assert.Contains(t, csp, "sandbox", "CSP sandbox missing")
+
+			if tt.accept {
+				assert.Equal(t, http.StatusOK, resp.StatusCode)
+				assert.Equal(t, tt.wantCT, resp.Header.Get("Content-Type"))
+				assert.Equal(t, tt.body, body, "body bytes must round-trip")
+				assert.Contains(t, resp.Header.Get("Cache-Control"), "max-age=2592000",
+					"validated success path carries the 30-day TTL")
+				assert.True(t, strings.HasPrefix(resp.Header.Get("Etag"), `"v2:`),
+					"validated success path carries the versioned etag")
+				return
+			}
+
+			// reject path
+			assert.GreaterOrEqual(t, resp.StatusCode, 400, "must reject non-image content")
+			// reject responses must NOT inherit the success path's long-lived cache
+			// headers — a transient 4xx would otherwise be pinned in browser/intermediary
+			// caches alongside the versioned etag for 30 days.
+			assert.Contains(t, resp.Header.Get("Cache-Control"), "no-store",
+				"reject path must set Cache-Control: no-store; got %q", resp.Header.Get("Cache-Control"))
+			assert.NotContains(t, resp.Header.Get("Cache-Control"), "max-age=2592000",
+				"reject path must not carry the success-path 30-day TTL")
+			assert.Empty(t, resp.Header.Get("Etag"),
+				"reject path must not carry the versioned etag (would pin the failure in cache)")
+			ct := resp.Header.Get("Content-Type")
+			assert.False(t, strings.HasPrefix(ct, "text/html"),
+				"reject response must not be text/html; got %q", ct)
+			assert.NotContains(t, string(body), tt.payloadMarker,
+				"reject response must not echo attack payload; got body=%q", string(body))
+		})
+	}
+}
+
+// TestEtagMatches lives in the rest package alongside the shared EtagMatches helper
+// (see backend/app/rest/image_headers_test.go). The proxy handler delegates to it.
+
+// TestImage_ContentTypeHandling_CacheHit exercises the cache-hit branch of the handler:
+// the StoreMock returns attacker bytes directly, so the upstream is never contacted.
+// Without the body-sniff at serve time, pre-fix code would have echoed cached HTML as
+// text/html. After the fix the same content-type defense applies on the cache path.
+func TestImage_ContentTypeHandling_CacheHit(t *testing.T) {
+	htmlBody := []byte("<html><body><script>alert(document.domain)</script></body></html>")
+	polyglot := append(append([]byte{}, gopherPNGBytes()...), []byte("<script>alert(1)</script>")...)
+
+	tbl := []struct {
+		name          string
+		cached        []byte
+		accept        bool
+		wantCT        string
+		payloadMarker string
+	}{
+		{name: "html in cache claimed as image/png", cached: htmlBody, payloadMarker: "<script>"},
+		{name: "polyglot in cache served as png", cached: polyglot, accept: true, wantCT: "image/png"},
+	}
+
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) {
+				return tt.cached, nil
+			}}
+			img := Image{
+				CacheExternal: true,
+				RemarkURL:     "https://demo.remark42.com",
+				RoutePath:     "/api/v1/proxy",
+				ImageService:  image.NewService(&imageStore, image.ServiceParams{}),
+				// no Transport — cache hit must not reach upstream
+			}
+			ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+			defer ts.Close()
+
+			encodedURL := base64.URLEncoding.EncodeToString([]byte("https://attacker.example.com/logo.png"))
+			resp, err := http.Get(ts.URL + "/?src=" + encodedURL)
+			require.NoError(t, err)
+			body, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.NoError(t, resp.Body.Close())
+
+			assert.Equal(t, 1, len(imageStore.LoadCalls()), "served from cache")
+			assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"))
+			assert.Contains(t, resp.Header.Get("Content-Disposition"), "inline")
+			assert.Contains(t, resp.Header.Get("Content-Security-Policy"),
+				"default-src 'none'; sandbox; frame-ancestors 'none'")
+
+			if tt.accept {
+				assert.Equal(t, http.StatusOK, resp.StatusCode)
+				assert.Equal(t, tt.wantCT, resp.Header.Get("Content-Type"))
+				return
+			}
+			assert.GreaterOrEqual(t, resp.StatusCode, 400, "must reject non-image cached content")
+			assert.False(t, strings.HasPrefix(resp.Header.Get("Content-Type"), "text/html"),
+				"reject response must not be text/html; got %q", resp.Header.Get("Content-Type"))
+			assert.NotContains(t, string(body), tt.payloadMarker,
+				"reject response must not echo cached attack payload")
+		})
+	}
+}
+
+// TestImage_EtagVersioned proves browser/proxy caches with pre-fix etags (the
+// unversioned base64 of src that used to be served alongside text/html bodies)
+// no longer satisfy revalidation: the server returns a fresh 200 with image
+// content instead of 304-ing the poisoned cached entry. The 30-day Cache-Control
+// max-age is unchanged — local browser caches still serving pre-fix bytes within
+// their TTL are not reached; the prefix only helps clients that revalidate during
+// the cached lifetime (Ctrl+R, intermediaries, post-expiry). See etagVersionPrefix
+// godoc for the tradeoff.
+func TestImage_EtagVersioned(t *testing.T) {
+	imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) { return nil, nil }}
+	img := Image{
+		RemarkURL:    "https://demo.remark42.com",
+		RoutePath:    "/api/v1/proxy",
+		ImageService: image.NewService(&imageStore, image.ServiceParams{}),
+		Transport:    http.DefaultTransport,
+	}
+	ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+	defer ts.Close()
+	httpSrv := imgHTTPTestsServer(t)
+	defer httpSrv.Close()
+
+	srcRaw := httpSrv.URL + "/image/img1.png"
+	encodedSrc := base64.URLEncoding.EncodeToString([]byte(srcRaw))
+	preFixEtag := `"` + encodedSrc + `"` // what a pre-fix browser would have cached
+
+	req, err := http.NewRequest("GET", ts.URL+"/?src="+encodedSrc, http.NoBody)
+	require.NoError(t, err)
+	req.Header.Set("If-None-Match", preFixEtag)
+
+	resp, err := http.DefaultClient.Do(req)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+
+	assert.Equal(t, http.StatusOK, resp.StatusCode,
+		"pre-fix etag must NOT validate as 304 — old cached text/html must be replaced")
+	assert.Equal(t, "image/png", resp.Header.Get("Content-Type"))
+	assert.NotEqual(t, preFixEtag, resp.Header.Get("Etag"), "new etag must differ from pre-fix")
+	assert.True(t, strings.HasPrefix(resp.Header.Get("Etag"), `"v2:`), "new etag must carry the version prefix")
+	cc := resp.Header.Get("Cache-Control")
+	assert.Contains(t, cc, "max-age=2592000", "success path keeps 30-day TTL for cache efficiency")
+
+	// sanity: the NEW etag round-trips as 304 when sent back
+	loadsBefore := len(imageStore.LoadCalls())
+	req2, err := http.NewRequest("GET", ts.URL+"/?src="+encodedSrc, http.NoBody)
+	require.NoError(t, err)
+	req2.Header.Set("If-None-Match", resp.Header.Get("Etag"))
+	resp2, err := http.DefaultClient.Do(req2)
+	require.NoError(t, err)
+	defer resp2.Body.Close()
+	assert.Equal(t, http.StatusNotModified, resp2.StatusCode, "new etag must validate against itself")
+	body, _ := io.ReadAll(resp2.Body)
+	assert.Empty(t, body, "304 must have no body")
+	// 304 path must skip the store lookup entirely — revalidation must not amplify load
+	assert.Equal(t, loadsBefore, len(imageStore.LoadCalls()),
+		"revalidation 304 must not trigger any store Load (avoids upstream DoS amplification)")
+	// 304 path must still carry the layered defense headers
+	assert.Equal(t, "nosniff", resp2.Header.Get("X-Content-Type-Options"))
+	assert.Contains(t, resp2.Header.Get("Content-Disposition"), "inline")
+	assert.Contains(t, resp2.Header.Get("Content-Security-Policy"), "default-src 'none'")
+	assert.Contains(t, resp2.Header.Get("Content-Security-Policy"), "sandbox")
+}
+
+// TestImage_RevalidationSkipsIO proves that a matching current-version If-None-Match
+// short-circuits before any cache lookup or upstream fetch. With no Transport and no
+// upstream server reachable, the only way this test can pass with 304 is if Load is
+// never called and downloadImage is never attempted. This closes the DoS amplification
+// where every reuse on a hot comment page would otherwise re-hit the upstream when
+// CacheExternal is false.
+func TestImage_RevalidationSkipsIO(t *testing.T) {
+	imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) {
+		t.Fatal("Load must not be called on the revalidation short-circuit path")
+		return nil, nil
+	}}
+	img := Image{
+		RemarkURL:    "https://demo.remark42.com",
+		RoutePath:    "/api/v1/proxy",
+		ImageService: image.NewService(&imageStore, image.ServiceParams{}),
+		// no Transport — any downloadImage attempt would also fail
+	}
+	ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+	defer ts.Close()
+
+	encodedSrc := base64.URLEncoding.EncodeToString([]byte("https://example.com/whatever.png"))
+	currentEtag := `"v2:` + encodedSrc + `"`
+
+	req, err := http.NewRequest("GET", ts.URL+"/?src="+encodedSrc, http.NoBody)
+	require.NoError(t, err)
+	req.Header.Set("If-None-Match", currentEtag)
+
+	resp, err := http.DefaultClient.Do(req)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+
+	assert.Equal(t, http.StatusNotModified, resp.StatusCode,
+		"matching current-version etag must short-circuit to 304 without I/O")
+	assert.Equal(t, 0, len(imageStore.LoadCalls()),
+		"revalidation must not trigger store Load")
+	body, _ := io.ReadAll(resp.Body)
+	assert.Empty(t, body, "304 must have no body")
+	// defense headers must still be set on the short-circuit path
+	assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"))
+	assert.Contains(t, resp.Header.Get("Content-Disposition"), "inline")
+	assert.Contains(t, resp.Header.Get("Content-Security-Policy"), "default-src 'none'")
+	assert.Contains(t, resp.Header.Get("Content-Security-Policy"), "sandbox")
+	assert.Equal(t, currentEtag, resp.Header.Get("Etag"))
+	assert.Contains(t, resp.Header.Get("Cache-Control"), "max-age=2592000")
+}
+
+// TestImage_PerRequestRevalidation proves the defense holds when upstream flips its
+// response body between requests (give a real PNG once, HTML next time, etc.). Each
+// proxy response is independently validated against the body actually returned, so
+// trust never accumulates and an earlier "good" response cannot grant the next one a
+// free pass.
+func TestImage_PerRequestRevalidation(t *testing.T) {
+	htmlBody := []byte("<html><script>alert(1)</script></html>")
+
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "image/png") // always lie consistently
+		switch r.URL.Path {
+		case "/png":
+			_, _ = w.Write(gopherPNGBytes())
+		case "/html":
+			_, _ = w.Write(htmlBody)
+		}
+	}))
+	defer upstream.Close()
+
+	imageStore := image.StoreMock{LoadFunc: func(string) ([]byte, error) { return nil, nil }}
+	img := Image{
+		RemarkURL:    "https://demo.remark42.com",
+		RoutePath:    "/api/v1/proxy",
+		ImageService: image.NewService(&imageStore, image.ServiceParams{}),
+		Transport:    http.DefaultTransport,
+	}
+	ts := httptest.NewServer(http.HandlerFunc(img.Handler))
+	defer ts.Close()
+
+	// alternate calls: PNG, HTML, PNG, HTML — each must be judged on its own bytes.
+	type step struct {
+		path       string
+		wantStatus int
+		wantCT     string // prefix match
+	}
+	steps := []step{
+		{path: "/png", wantStatus: http.StatusOK, wantCT: "image/png"},
+		{path: "/html", wantStatus: http.StatusUnsupportedMediaType, wantCT: "application/json"},
+		{path: "/png", wantStatus: http.StatusOK, wantCT: "image/png"},
+		{path: "/html", wantStatus: http.StatusUnsupportedMediaType, wantCT: "application/json"},
+	}
+	for i, s := range steps {
+		t.Run(fmt.Sprintf("step_%d_%s", i, s.path), func(t *testing.T) {
+			encodedURL := base64.URLEncoding.EncodeToString([]byte(upstream.URL + s.path))
+			resp, err := http.Get(ts.URL + "/?src=" + encodedURL)
+			require.NoError(t, err)
+			body, _ := io.ReadAll(resp.Body)
+			require.NoError(t, resp.Body.Close())
+
+			assert.Equal(t, s.wantStatus, resp.StatusCode)
+			assert.True(t, strings.HasPrefix(resp.Header.Get("Content-Type"), s.wantCT),
+				"expected Content-Type prefix %q, got %q", s.wantCT, resp.Header.Get("Content-Type"))
+			assert.False(t, strings.HasPrefix(resp.Header.Get("Content-Type"), "text/html"),
+				"must never serve text/html under any flip")
+			assert.NotContains(t, string(body), "<script>",
+				"attacker payload must never appear in response body")
+			// every response must still carry the defense headers
+			assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"))
+			assert.Contains(t, resp.Header.Get("Content-Disposition"), "inline")
+			assert.Contains(t, resp.Header.Get("Content-Security-Policy"),
+				"default-src 'none'; sandbox; frame-ancestors 'none'")
+		})
+	}
+}
+
 func imgHTTPTestsServer(t *testing.T) *httptest.Server {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path == "/image/img1.png" {

backend/app/store/image/image.go

@@ -237,16 +237,6 @@ func (s *Service) SaveWithID(id string, r io.Reader) error {
 	return s.store.Save(id, img)
 }
 
-// ImgContentType returns content type for provided image
-func (s *Service) ImgContentType(img []byte) string {
-	contentType := http.DetectContentType(img)
-	if contentType == "application/octet-stream" {
-		// replace generic fallback with one which make sense in our scenario
-		return "image/*"
-	}
-	return contentType
-}
-
 // returns list of image IDs from the comment html, including proxied images if includeProxied is true
 func (s *Service) extractImageIDs(commentHTML string, includeProxied bool) (ids []string) {
 	doc, err := goquery.NewDocumentFromReader(strings.NewReader(commentHTML))

backend/app/store/image/image_test.go

@@ -359,3 +359,6 @@ func TestService_DoubleClose(*testing.T) {
 	// second call should not result in panic
 	svc.Close(context.TODO())
 }
+
+// TestSafeImgContentType now lives in the rest package alongside the SafeImgContentType
+// helper itself (see backend/app/rest/image_headers_test.go).