diff --git a/Caddyfile b/Caddyfile index 970381f..138ea14 100644 --- a/Caddyfile +++ b/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 - # visibility-checked alias (/api/v1/upload/{id}/{preview,thumbnail}) so moderation - # can revoke access; direct /media/previews|thumbnails is 404-blocked at the app. + # Preview/thumbnail images. These are served by the app through a visibility-checked + # alias (/api/v1/upload/{id}/{preview,thumbnail}) so moderation can revoke access; + # 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 diff --git a/backend/src/main.rs b/backend/src/main.rs index fd5b3a8..e7327e1 100644 --- a/backend/src/main.rs +++ b/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 - let media_service = ServeDir::new(&config.media_path); - + // NOTE: media is deliberately NOT served over HTTP. + // + // 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); diff --git a/e2e/specs/07-adversarial/media-gating.spec.ts b/e2e/specs/07-adversarial/media-gating.spec.ts index 29f1f16..d9b19c7 100644 --- a/e2e/specs/07-adversarial/media-gating.spec.ts +++ b/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',