`/media/%70reviews/{id}.jpg` served a taken-down photo to anyone,
unauthenticated. Verified against the running stack: the literal path 404s,
the escaped one returned 200 with the full image. Same for displays,
thumbnails and originals, and any escaped byte in any position works.
Cause: the block was four `nest_service("/media/previews", 404)` route
matches sitting above a `ServeDir` on `/media`. axum matches on the RAW path
(matchit does no percent-decoding), while `ServeDir` percent-decodes when it
resolves the file. So `%70reviews` missed every blocker, fell through to the
ServeDir, and was decoded back to `previews/` on disk — reaching the bytes
with no soft-delete and no ban-hide check. That defeats a host takedown,
which is the entire point of the gate.
Remove the `/media` route tree outright instead of racing the decoder.
Nothing needs it: every media URL the backend emits is already a gated
`/api/v1/upload/{id}/{original,preview,display,thumbnail}` alias
(handlers::feed), the frontend contains zero `/media/` references, and the
`/media` in config.rs/disk.rs is the filesystem path while `media/` in
export.rs is a path inside the zip. `/media/**` now 404s regardless of
encoding. The route's own comment already said it "serves nothing" — it
wasn't a backstop, it was the vector.
Caddy keeps proxying /media/* deliberately: the app 404s it, and forwarding
means the e2e gating specs exercise the app's refusal exactly as production
would rather than being masked by the SvelteKit 404 page.
Extend the gating spec with the encoded variants — asserting only the literal
spelling is what let this sit undetected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
114 lines
4.9 KiB
TypeScript
114 lines
4.9 KiB
TypeScript
/**
|
|
* Security fix F2: preview/thumbnail images are now served through a visibility-checked
|
|
* alias (/api/v1/upload/{id}/preview) instead of the unauthenticated /media ServeDir,
|
|
* so moderation (delete / ban-hide) actually revokes access to the displayed image —
|
|
* not just the full-res original. Direct /media/previews access is blocked.
|
|
*/
|
|
import { test, expect } from '../../fixtures/test';
|
|
import { seedUpload } from '../../helpers/seed';
|
|
import { BASE } from '../../helpers/env';
|
|
|
|
test.describe('Media gating — moderation revokes preview access (F2)', () => {
|
|
test('preview served via gated alias, blocked directly, and 404 after delete', async ({
|
|
host,
|
|
api,
|
|
}) => {
|
|
test.setTimeout(30_000);
|
|
const id = await seedUpload(host.jwt, { caption: 'gated' });
|
|
|
|
// Wait for the compression worker to produce the preview — the feed exposes
|
|
// `preview_url` only once `preview_path` is set.
|
|
let previewUrl: string | undefined;
|
|
await expect
|
|
.poll(
|
|
async () => {
|
|
const feed = await api.getFeed(host.jwt);
|
|
const row = (feed.uploads ?? []).find((u: any) => u.id === id);
|
|
previewUrl = row?.preview_url ?? undefined;
|
|
return previewUrl;
|
|
},
|
|
{ timeout: 20_000, intervals: [500] }
|
|
)
|
|
.toBeTruthy();
|
|
|
|
// Feed now emits the gated alias, not a /media path.
|
|
expect(previewUrl).toBe(`/api/v1/upload/${id}/preview`);
|
|
|
|
// The gated alias serves the image with the app-layer nosniff header.
|
|
const ok = await fetch(`${BASE}${previewUrl}`);
|
|
expect(ok.status).toBe(200);
|
|
expect(ok.headers.get('content-type')).toContain('image/jpeg');
|
|
// App sets nosniff (F6); the edge proxy may also set it → value can be doubled.
|
|
expect(ok.headers.get('x-content-type-options')).toContain('nosniff');
|
|
|
|
// Direct /media access is blocked (404) — the alias is the only way in.
|
|
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',
|
|
headers: { Authorization: `Bearer ${host.jwt}` },
|
|
});
|
|
expect(del.status).toBe(204);
|
|
|
|
const afterDelete = await fetch(`${BASE}/api/v1/upload/${id}/preview`);
|
|
expect(afterDelete.status, 'moderation must revoke preview access').toBe(404);
|
|
});
|
|
|
|
test('the preview is revoked when the UPLOADER is banned (not just on delete)', async ({
|
|
host,
|
|
api,
|
|
guest,
|
|
}) => {
|
|
test.setTimeout(30_000);
|
|
// The header of this file claims delete AND ban-hide both revoke access, but only delete was
|
|
// ever exercised. A ban hides the user's content everywhere (the visibility check filters
|
|
// `is_banned` inside `find_by_id_visible`, which gates preview AND thumbnail identically), and
|
|
// its whole point is that a direct-URL holder loses the image — so it must 404 the gated
|
|
// preview too, exactly like a delete.
|
|
const offender = await guest('BannedUploader');
|
|
const id = await seedUpload(offender.jwt, { caption: 'to be hidden' });
|
|
|
|
// Wait for the compression worker to produce the preview.
|
|
await expect
|
|
.poll(
|
|
async () => {
|
|
const row = (await api.getFeed(host.jwt)).uploads?.find((u: any) => u.id === id);
|
|
return row?.preview_url;
|
|
},
|
|
{ timeout: 20_000, intervals: [500] }
|
|
)
|
|
.toBe(`/api/v1/upload/${id}/preview`);
|
|
|
|
// Served while the uploader is in good standing.
|
|
expect((await fetch(`${BASE}/api/v1/upload/${id}/preview`)).status).toBe(200);
|
|
|
|
// Ban the uploader (default: hide their uploads).
|
|
await api.banUser(host.jwt, offender.userId);
|
|
|
|
// Must now 404 — the direct-URL holder loses the image, same as a takedown.
|
|
expect(
|
|
(await fetch(`${BASE}/api/v1/upload/${id}/preview`)).status,
|
|
"a banned uploader's preview must be revoked"
|
|
).toBe(404);
|
|
});
|
|
});
|