Merge branch 'fix/media-gating-percent-escape'

Caddyfile

@@ -31,9 +31,9 @@
 	@hashed_assets path_regexp hashed /_app/immutable/.*\.[a-f0-9]{8,}\.(js|css|woff2)$
 	header @hashed_assets Cache-Control "public, max-age=31536000, immutable"
 
-	# Preview/thumbnail images. These are now served by the app through a
+	# Preview/thumbnail images. These are served by the app through a visibility-checked
-	# visibility-checked alias (/api/v1/upload/{id}/{preview,thumbnail}) so moderation
+	# alias (/api/v1/upload/{id}/{preview,thumbnail}) so moderation can revoke access;
-	# can revoke access; direct /media/previews|thumbnails is 404-blocked at the app.
+	# the app serves no /media route at all, so there is no direct path to the bytes.
 	# Privately cacheable for a short window (the app sets the same header; this is the
 	# edge carve-out from the blanket no-store below). Kept short so a moderated image
 	# stops being served to a direct-URL holder promptly.
@@ -47,7 +47,14 @@
 	}
 	header @api Cache-Control "no-store"
 
-	# Route API and media requests to the Rust backend
+	# Route API and media requests to the Rust backend.
+	#
+	# The app serves no /media route at all (see the note in backend/src/main.rs) — media
+	# bytes are reachable only through the visibility-checked /api/v1/upload aliases, so
+	# /media/* forwards to a plain 404. The proxy line is kept deliberately: it means the
+	# edge faithfully hands /media to the app, so if a future change ever re-introduces a
+	# static media route the e2e gating specs see it here exactly as production would,
+	# instead of being masked by the SvelteKit 404 page.
 	reverse_proxy /api/* app:3000
 	reverse_proxy /media/* app:3000
 

backend/src/main.rs

@@ -2,7 +2,6 @@ use anyhow::Result;
 use axum::Router;
 use axum::extract::DefaultBodyLimit;
 use axum::routing::{delete, get, patch, post};
-use tower_http::services::ServeDir;
 use tower_http::trace::TraceLayer;
 use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};
 
@@ -229,37 +228,25 @@ async fn main() -> Result<()> {
         api
     };
 
-    // Serve media files from disk
+    // NOTE: media is deliberately NOT served over HTTP.
-    let media_service = ServeDir::new(&config.media_path);
+    //
-
+    // Files live under `media_path` so the compression worker and the export job can read
+    // them off disk, but nothing may pull them straight from `/media/**` — that bypasses
+    // the visibility checks (soft-delete + ban-hide) that make a host takedown stick.
+    // Every legitimate fetch goes through `/api/v1/upload/{id}/{original,preview,display,
+    // thumbnail}`, which filter via `find_visible_media`; those are the only media URLs the
+    // backend ever emits (see `handlers::feed`).
+    //
+    // This used to be a `ServeDir` on `/media` with four `nest_service` blockers on the
+    // subtrees above it. That was bypassable: axum routes on the RAW path while `ServeDir`
+    // percent-decodes afterwards, so `/media/%70reviews/{id}.jpg` missed every blocker,
+    // fell through to the `ServeDir`, and was decoded back to `previews/` on disk — serving
+    // a taken-down photo to anyone, unauthenticated. Any single escaped byte worked, in all
+    // four subtrees. Deleting the route removes the vector outright rather than racing the
+    // decoder; `/media/**` now 404s regardless of encoding.
     let router = Router::new()
         .route("/health", get(|| async { "ok" }))
         .merge(api)
-        // Block direct HTTP access to ALL media subtrees. The files live under
-        // `media_path` (so the compression worker and export can read them off disk) but
-        // must NOT be pullable straight from `/media/**` — that bypasses the visibility
-        // checks (soft-delete + ban-hide) in the gated handlers. Every legitimate fetch
-        // goes through `/api/v1/upload/{id}/{original,preview,thumbnail}`, which filter
-        // via `find_visible_media`. The more specific nests take precedence over the
-        // `/media` ServeDir below (which, with all three subtrees blocked, now serves
-        // nothing — kept as a backstop).
-        .nest_service(
-            "/media/originals",
-            get(|| async { axum::http::StatusCode::NOT_FOUND }),
-        )
-        .nest_service(
-            "/media/previews",
-            get(|| async { axum::http::StatusCode::NOT_FOUND }),
-        )
-        .nest_service(
-            "/media/displays",
-            get(|| async { axum::http::StatusCode::NOT_FOUND }),
-        )
-        .nest_service(
-            "/media/thumbnails",
-            get(|| async { axum::http::StatusCode::NOT_FOUND }),
-        )
-        .nest_service("/media", media_service)
         .layer(TraceLayer::new_for_http())
         .with_state(state);
 

e2e/specs/07-adversarial/media-gating.spec.ts

@@ -45,6 +45,23 @@ test.describe('Media gating — moderation revokes preview access (F2)', () => {
     const direct = await fetch(`${BASE}/media/previews/${id}.jpg`);
     expect(direct.status, 'direct /media/previews must be blocked').toBe(404);
 
+    // …and it must stay blocked under percent-encoding. The block used to be four
+    // `nest_service("/media/previews", 404)` route matches sitting above a `/media`
+    // ServeDir. axum routes on the RAW path while ServeDir percent-decodes afterwards, so
+    // ONE escaped byte (`%70` = `p`) missed every blocker, fell through to the ServeDir,
+    // and was decoded back to `previews/` on disk — serving the bytes unauthenticated.
+    // Asserting only the literal spelling is what let that sit here undetected.
+    for (const variant of [
+      `/media/%70reviews/${id}.jpg`, // p
+      `/media/p%72eviews/${id}.jpg`, // r — any position works
+      `/media/%64isplays/${id}.jpg`, // d
+      `/media/%74humbnails/${id}.jpg`, // t
+      `/media/%6Friginals/${id}.jpg`, // o
+    ]) {
+      const res = await fetch(`${BASE}${variant}`, { redirect: 'manual' });
+      expect(res.status, `${variant} must not bypass the media block`).toBe(404);
+    }
+
     // Host deletes the upload → the preview must stop being served.
     const del = await fetch(`${BASE}/api/v1/host/upload/${id}`, {
       method: 'DELETE',