refactor(platform): serve /uploads routes through storage

server/src/bootstrap.ts

@@ -16,6 +16,7 @@ import { validateBodyContracts } from './nest/common/validate-body-contracts';
 import { validateRouteGuards } from './nest/common/validate-route-guards';
 import { validateManagedRoutes } from './nest/common/validate-managed-routes';
 import { TrekWsAdapter } from './nest/realtime/trek-ws.adapter';
+import { StorageService } from './nest/storage/storage.service';
 
 /**
  * Builds the unified TREK NestJS application that serves the ENTIRE surface — the
@@ -84,7 +85,10 @@ export async function buildApp(): Promise<INestApplication> {
   // config instead of reading process.env itself.
   const http = app.get<ConfigType<typeof httpConfig>>(httpConfig.KEY);
   applyGlobalMiddleware(instance, { http });
-  applyPlatformUploads(instance);
+  // Same pre-init consumption bridge as httpConfig above: the StorageService
+  // instance is resolvable before init, and the handlers only *register* here —
+  // per-request resolution runs after app.init() completed the registry load.
+  applyPlatformUploads(instance, app.get(StorageService));
   // The SDK discovery router (+ its addon gate). Container-built so its deps
   // are injected (same pre-init consumption bridge as httpConfig above), but
   // applied here as a PATHLESS app.use: the SDK router matches absolute

server/src/nest/platform/platform.routes.ts

@@ -1,31 +1,132 @@
 import express, { Request, Response, NextFunction } from 'express';
 import path from 'node:path';
-import fs from 'node:fs';
 
 import { readEnv } from '../../app-config';
 import { verifyJwtAndLoadUser } from '../auth/jwt-verify';
 import { db } from '../../db/database';
+import { StorageService } from '../storage/storage.service';
+import { StorageInvalidKeyError, StorageNotFoundError, type StorageCategory } from '../storage/storage.types';
 
 // Platform / transport routes extracted verbatim from createApp() (app.ts) so they can be
 // mounted on either the legacy Express app or the NestJS Express instance (strangler A6/A8).
 //
-// IMPORTANT — path resolution: the original blocks lived in src/app.ts, where __dirname
-// resolves to the directory of app.js (one level above the uploads/public anchor), so they
-// used '../uploads/...' and '../public'. This file lives three levels deeper
-// (src/nest/platform/), so __dirname is three levels deeper too. The relative prefixes are
-// therefore '../../../uploads/...' and '../../../public' — which resolve to the EXACT same
-// absolute paths as before. This is the only intentional change; everything else is byte-for-byte
-// identical. (rootDir/outDir preserve the tree, so the offset holds in both source/test and
-// compiled/dist execution — matching the other nest controllers that use '../../../uploads/...'.)
+// IMPORTANT — path resolution: the original block lived in src/app.ts, where __dirname
+// resolves to the directory of app.js (one level above the public anchor), so it used
+// '../public'. This file lives three levels deeper (src/nest/platform/), so __dirname is
+// three levels deeper too — hence '../../../public', which resolves to the EXACT same
+// absolute path as before. (rootDir/outDir preserve the tree, so the offset holds in both
+// source/test and compiled/dist execution.) The /uploads/* routes no longer anchor paths
+// here at all: they address files as (category, name) through StorageService (slice 3).
 
-const UPLOADS_DIR = path.join(__dirname, '../../../uploads');
 export const PUBLIC_DIR = path.join(__dirname, '../../../public');
 
+/**
+ * express.static replacement for the four public /uploads mounts (storage
+ * slice 3). Parity contract (spec §Serving): identical ETag / Last-Modified /
+ * conditional-GET / Range / HEAD behavior — res.sendFile inside
+ * storage.sendToResponse reads res.req, so the same send() machinery runs —
+ * and every MISS (not-found, invalid key, dot segment, directory) calls
+ * next() so the request falls through to the Nest router's standard 404
+ * envelope, exactly like express.static's fallthrough. 412/416/5xx go to
+ * next(err) with send's own http-errors — the same finalhandler path
+ * express.static uses.
+ */
+export function storageStaticHandler(storage: StorageService, category: StorageCategory): express.RequestHandler {
+  return (req: Request, res: Response, next: NextFunction) => {
+    // serve-static fallthrough: non-GET/HEAD pass straight through (no 405).
+    if (req.method !== 'GET' && req.method !== 'HEAD') {
+      next();
+      return;
+    }
+    // Inside an app.use mount req.path is the mount-stripped, query-free
+    // pathname. Malformed percent-encoding falls through, matching
+    // serve-static's silent fallthrough on send's 400 decode error.
+    let decoded: string;
+    try {
+      decoded = decodeURIComponent(req.path);
+    } catch {
+      next();
+      return;
+    }
+    const name = decoded.replace(/^\/+/, '');
+    return storage.sendToResponse(category, name, res).catch((err: unknown) => {
+      // MISS contract: unknown object, invalid key ('' at the mount root,
+      // '..', dot segments, control chars) → Nest 404 envelope.
+      if (err instanceof StorageNotFoundError || err instanceof StorageInvalidKeyError) {
+        next();
+        return;
+      }
+      const e = err as { status?: number; code?: string; syscall?: string };
+      // stat→sendFile race (send 404) and directories: static falls through too.
+      if (e.status === 404 || e.code === 'EISDIR') {
+        next();
+        return;
+      }
+      // Client gone / bytes already on the wire: nothing useful to send —
+      // mirrors res.sendFile's own default callback (express response.js:454).
+      if (e.code === 'ECONNABORTED' || e.syscall === 'write') return;
+      next(err as Error);
+    });
+  };
+}
+
+async function servePhoto(storage: StorageService, req: Request, res: Response): Promise<void> {
+  const safeName = path.basename(req.params.filename);
+  // Parity: after basename(), the old resolve()+startsWith guard could only
+  // fire when the remaining segment was '..' — keep that exact 403.
+  if (safeName === '..') {
+    res.status(403).send('Forbidden');
+    return;
+  }
+  // Existence is (and was) checked BEFORE auth — the 404-vs-401 order is
+  // observable and pinned. Invalid keys ('.', dotfiles) read as a miss.
+  if (!(await storage.exists('photos', safeName).catch(() => false))) {
+    res.status(404).send('Not found');
+    return;
+  }
+  const sendPhoto = () =>
+    storage.sendToResponse('photos', safeName, res).catch((err: unknown) => {
+      // exists→send delete race: same 404 text as the miss branch.
+      if ((err instanceof StorageNotFoundError || err instanceof StorageInvalidKeyError) && !res.headersSent) {
+        res.status(404).send('Not found');
+        return;
+      }
+      throw err;
+    });
+
+  const authHeader = req.headers.authorization;
+  const rawToken = (req.query.token as string) || (authHeader?.startsWith('Bearer ') ? authHeader.slice(7) : null);
+  if (!rawToken) {
+    res.status(401).send('Authentication required');
+    return;
+  }
+
+  // JWT session path (with pv check).
+  const user = verifyJwtAndLoadUser(rawToken);
+  if (user) return sendPhoto();
+
+  // Share-token path: require the token to cover the exact trip the
+  // photo belongs to. Expired tokens fall through to 401.
+  const photo = db.prepare('SELECT trip_id FROM photos WHERE filename = ?').get(safeName) as { trip_id: number } | undefined;
+  if (!photo) {
+    res.status(401).send('Authentication required');
+    return;
+  }
+  const share = db
+    .prepare("SELECT trip_id FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))")
+    .get(rawToken) as { trip_id: number } | undefined;
+  if (!share || share.trip_id !== photo.trip_id) {
+    res.status(401).send('Authentication required');
+    return;
+  }
+  return sendPhoto();
+}
+
 /**
  * Static + guarded /uploads/* routes. Must be applied BEFORE the API route mounts
  * (identical to its original position near the top of createApp).
  */
-export function applyPlatformUploads(app: express.Application): void {
+export function applyPlatformUploads(app: express.Application, storage: StorageService): void {
   // Static: avatars, covers, and journey photos.
   //
   // Security model (audit SEC-M9): these paths are unauthenticated by
@@ -44,48 +145,19 @@ export function applyPlatformUploads(app: express.Application): void {
   // not embedded in unauthenticated UI contexts, so that endpoint IS
   // gated (session JWT with pv, or a share token scoped to the photo's
   // trip).
-  app.use('/uploads/avatars', express.static(path.join(UPLOADS_DIR, 'avatars')));
-  app.use('/uploads/covers', express.static(path.join(UPLOADS_DIR, 'covers')));
-  app.use('/uploads/journey', express.static(path.join(UPLOADS_DIR, 'journey')));
-  app.use('/uploads/places', express.static(path.join(UPLOADS_DIR, 'places')));
+  app.use('/uploads/avatars', storageStaticHandler(storage, 'avatars'));
+  app.use('/uploads/covers', storageStaticHandler(storage, 'covers'));
+  app.use('/uploads/journey', storageStaticHandler(storage, 'journey'));
+  app.use('/uploads/places', storageStaticHandler(storage, 'places'));
 
   // Photos require either a valid logged-in session (via JWT with the
   // password_version gate) OR a share token that covers the SPECIFIC
   // photo's trip. Previously any share token for any trip could request
   // any photo filename by UUID — fine in practice because UUIDs are
   // unguessable, but the auth model was wrong.
-  app.get('/uploads/photos/:filename', (req: Request, res: Response) => {
-    const safeName = path.basename(req.params.filename);
-    const filePath = path.join(UPLOADS_DIR, 'photos', safeName);
-    const resolved = path.resolve(filePath);
-    if (!resolved.startsWith(path.resolve(UPLOADS_DIR, 'photos'))) {
-      return res.status(403).send('Forbidden');
-    }
-    // existsSync here is cheap and avoids a sendFile error frame; kept
-    // sync because the handler is already short-lived.
-    if (!fs.existsSync(resolved)) return res.status(404).send('Not found');
-
-    const authHeader = req.headers.authorization;
-    const rawToken = (req.query.token as string) || (authHeader?.startsWith('Bearer ') ? authHeader.slice(7) : null);
-    if (!rawToken) return res.status(401).send('Authentication required');
-
-    // JWT session path (with pv check).
-    const user = verifyJwtAndLoadUser(rawToken);
-    if (user) return res.sendFile(resolved);
-
-    // Share-token path: require the token to cover the exact trip the
-    // photo belongs to. Expired tokens fall through to 401.
-    const photo = db.prepare('SELECT trip_id FROM photos WHERE filename = ?').get(safeName) as { trip_id: number } | undefined;
-    if (!photo) return res.status(401).send('Authentication required');
-
-    const share = db.prepare(
-        "SELECT trip_id FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))"
-    ).get(rawToken) as { trip_id: number } | undefined;
-    if (!share || share.trip_id !== photo.trip_id) {
-      return res.status(401).send('Authentication required');
-    }
-    res.sendFile(resolved);
-  });
+  app.get('/uploads/photos/:filename', (req: Request, res: Response, next: NextFunction) =>
+    servePhoto(storage, req, res).catch(next),
+  );
 
   // Block direct access to /uploads/files
   app.use('/uploads/files', (_req: Request, res: Response) => {

server/tests/integration/uploads-static.test.ts

@@ -0,0 +1,297 @@
+/**
+ * /uploads static-mount parity tests (UPLOADS-P01…P24).
+ *
+ * Written BEFORE the storage slice-3 swap of the four express.static mounts
+ * (avatars / covers / journey / places) for a storage-backed handler on the
+ * same URLs. P01–P21 pin today's express.static behavior — ETag /
+ * Last-Modified / conditional-GET / Range / HEAD, miss-falls-through to the
+ * Nest 404 envelope — and must stay green across the swap. P22–P24 are the
+ * approved deliberate deltas (D1–D3 in the slice-3 plan): RED against
+ * express.static, green against the storage handler.
+ *
+ * These routes are unauthenticated BY DESIGN (audit SEC-M9): server-chosen
+ * UUIDv4 filenames; gating them would break share-link trip cards, journey
+ * public pages, and email-embedded avatars. No auth setup here is deliberate.
+ */
+import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
+import request from 'supertest';
+import type { Application } from 'express';
+import type { INestApplication } from '@nestjs/common';
+import path from 'path';
+import fs from 'fs';
+
+const { testDb, dbMock } = vi.hoisted(() => {
+  const Database = require('better-sqlite3');
+  const db = new Database(':memory:');
+  db.exec('PRAGMA journal_mode = WAL');
+  db.exec('PRAGMA foreign_keys = ON');
+  db.exec('PRAGMA busy_timeout = 5000');
+  const mock = {
+    db,
+    closeDb: () => {},
+    reinitialize: () => {},
+    canAccessTrip: () => undefined,
+    isOwner: () => false,
+  };
+  return { testDb: db, dbMock: mock };
+});
+
+vi.mock('../../src/db/database', () => dbMock);
+vi.mock('../../src/config', () => ({
+  JWT_SECRET: 'test-jwt-secret-for-trek-testing-only',
+  ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2',
+  updateJwtSecret: () => {},
+  SESSION_DURATION: '24h',
+  SESSION_DURATION_MS: 86400000,
+  SESSION_DURATION_SECONDS: 86400,
+  DEFAULT_LANGUAGE: 'en',
+}));
+vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn(), getOnlineUserIds: vi.fn(() => []) }));
+
+import { buildApp } from '../../src/bootstrap';
+import { createTables } from '../../src/db/schema';
+import { runMigrations } from '../../src/db/migrations';
+
+let nestApp: INestApplication;
+let app: Application;
+
+const uploadsDir = path.join(__dirname, '../../uploads');
+// 8 known bytes (the PNG magic) so Content-Length / Range math is exact.
+const BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
+const NAME = 'uploads-static-parity.png';
+
+// Every path this suite creates, for targeted afterAll cleanup — the uploads
+// tree is shared with other integration workers, so no directory-wide rm of
+// the category dirs themselves.
+const created: string[] = [];
+
+function writeFixture(rel: string, bytes: Buffer = BYTES): void {
+  const abs = path.join(uploadsDir, rel);
+  fs.mkdirSync(path.dirname(abs), { recursive: true });
+  fs.writeFileSync(abs, bytes);
+  created.push(abs);
+}
+
+beforeAll(async () => {
+  createTables(testDb);
+  runMigrations(testDb);
+  nestApp = await buildApp();
+  app = nestApp.getHttpAdapter().getInstance();
+  // Written AFTER buildApp(): LocalDriver init ensures + realpaths the
+  // category dirs at boot.
+  writeFixture(`avatars/${NAME}`);
+  writeFixture(`covers/${NAME}`);
+  writeFixture(`journey/thumbs/uploads-static-parity.jpg`);
+  writeFixture(`places/${NAME}`);
+  writeFixture('avatars/.uploads-static-parity-hidden');
+  writeFixture('avatars/.tmp/uploads-static-parity-spooled.bin');
+  fs.mkdirSync(path.join(uploadsDir, 'journey/parity-subdir'), { recursive: true });
+});
+
+afterAll(async () => {
+  await nestApp.close();
+  testDb.close();
+  for (const abs of created) fs.rmSync(abs, { force: true });
+  fs.rmSync(path.join(uploadsDir, 'avatars/.tmp'), { recursive: true, force: true });
+  fs.rmSync(path.join(uploadsDir, 'journey/parity-subdir'), { recursive: true, force: true });
+});
+
+describe('/uploads static parity — hit headers and bytes', () => {
+  it('UPLOADS-P01 — GET hit: 200 with express.static header set', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}`);
+    expect(res.status).toBe(200);
+    expect(res.body.equals ? res.body.equals(BYTES) : Buffer.from(res.body).equals(BYTES)).toBe(true);
+    expect(res.headers.etag).toMatch(/^W\/"[0-9a-f]+-[0-9a-f]+"$/);
+    expect(res.headers['last-modified']).toBeDefined();
+    expect(res.headers['cache-control']).toBe('public, max-age=0');
+    expect(res.headers['accept-ranges']).toBe('bytes');
+    expect(res.headers['content-type']).toBe('image/png');
+    expect(res.headers['content-length']).toBe('8');
+  });
+
+  it('UPLOADS-P02 — HEAD hit: 200, same headers, empty body', async () => {
+    const res = await request(app).head(`/uploads/avatars/${NAME}`);
+    expect(res.status).toBe(200);
+    expect(res.headers.etag).toMatch(/^W\/"[0-9a-f]+-[0-9a-f]+"$/);
+    expect(res.headers['content-length']).toBe('8');
+    expect(res.headers['content-type']).toBe('image/png');
+    expect(res.text ?? '').toBe('');
+  });
+
+  it('UPLOADS-P19 — query string is ignored', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}?x=1`);
+    expect(res.status).toBe(200);
+    expect(res.headers['content-length']).toBe('8');
+  });
+
+  it('UPLOADS-P20 — doubled leading slash still serves', async () => {
+    const res = await request(app).get(`/uploads/avatars//${NAME}`);
+    expect(res.status).toBe(200);
+    expect(res.headers['content-length']).toBe('8');
+  });
+});
+
+describe('/uploads static parity — conditional GET', () => {
+  it('UPLOADS-P03 — If-None-Match with the current ETag → 304', async () => {
+    const first = await request(app).get(`/uploads/avatars/${NAME}`);
+    const res = await request(app).get(`/uploads/avatars/${NAME}`).set('If-None-Match', first.headers.etag);
+    expect(res.status).toBe(304);
+    expect(res.headers.etag).toBe(first.headers.etag);
+    expect(res.text ?? '').toBe('');
+  });
+
+  it('UPLOADS-P04 — If-Modified-Since with the current Last-Modified → 304', async () => {
+    const first = await request(app).get(`/uploads/avatars/${NAME}`);
+    const res = await request(app)
+      .get(`/uploads/avatars/${NAME}`)
+      .set('If-Modified-Since', first.headers['last-modified']);
+    expect(res.status).toBe(304);
+  });
+
+  it('UPLOADS-P05 — stale If-None-Match → 200 full', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}`).set('If-None-Match', 'W/"0-0"');
+    expect(res.status).toBe(200);
+    expect(res.headers['content-length']).toBe('8');
+  });
+
+  it('UPLOADS-P06 — If-Match mismatch → 412', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}`).set('If-Match', '"mismatch"');
+    expect(res.status).toBe(412);
+  });
+});
+
+describe('/uploads static parity — Range', () => {
+  it('UPLOADS-P07 — single satisfiable range → 206 with Content-Range', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}`).set('Range', 'bytes=0-3');
+    expect(res.status).toBe(206);
+    expect(res.headers['content-range']).toBe('bytes 0-3/8');
+    expect(res.headers['content-length']).toBe('4');
+    expect(Buffer.from(res.body).equals(BYTES.subarray(0, 4))).toBe(true);
+  });
+
+  it('UPLOADS-P08 — unsatisfiable range → 416 with Content-Range */size', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}`).set('Range', 'bytes=999-');
+    expect(res.status).toBe(416);
+    expect(res.headers['content-range']).toBe('bytes */8');
+  });
+
+  it('UPLOADS-P09 — multi-range is ignored → 200 full', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}`).set('Range', 'bytes=0-0,4-5');
+    expect(res.status).toBe(200);
+    expect(res.headers['content-length']).toBe('8');
+  });
+
+  it('UPLOADS-P10 — non-bytes range unit is ignored → 200 full', async () => {
+    const res = await request(app).get(`/uploads/avatars/${NAME}`).set('Range', 'apples=0-1');
+    expect(res.status).toBe(200);
+  });
+
+  it('UPLOADS-P11 — stale If-Range drops the range → 200 full', async () => {
+    const res = await request(app)
+      .get(`/uploads/avatars/${NAME}`)
+      .set('Range', 'bytes=0-3')
+      .set('If-Range', 'W/"0-0"');
+    expect(res.status).toBe(200);
+    expect(res.headers['content-length']).toBe('8');
+  });
+
+  it('UPLOADS-P12 — fresh If-Range honors the range → 206', async () => {
+    const first = await request(app).get(`/uploads/avatars/${NAME}`);
+    const res = await request(app)
+      .get(`/uploads/avatars/${NAME}`)
+      .set('Range', 'bytes=0-3')
+      .set('If-Range', first.headers.etag);
+    expect(res.status).toBe(206);
+  });
+});
+
+describe('/uploads static parity — miss falls through to the Nest 404 envelope', () => {
+  it('UPLOADS-P13 — GET miss → Nest envelope, not a bespoke 404', async () => {
+    const res = await request(app).get('/uploads/avatars/nope.png');
+    expect(res.status).toBe(404);
+    expect(res.body).toEqual({ error: 'Cannot GET /uploads/avatars/nope.png' });
+  });
+
+  it('UPLOADS-P14 — HEAD miss → 404, empty body', async () => {
+    const res = await request(app).head('/uploads/avatars/nope.png');
+    expect(res.status).toBe(404);
+    expect(res.text ?? '').toBe('');
+  });
+
+  it('UPLOADS-P15 — POST to a hit URL falls through (no 405)', async () => {
+    const res = await request(app).post(`/uploads/avatars/${NAME}`);
+    expect(res.status).toBe(404);
+    expect(res.body.error).toBe(`Cannot POST /uploads/avatars/${NAME}`);
+  });
+
+  it('UPLOADS-P16 — malformed percent-encoding falls through', async () => {
+    const res = await request(app).get('/uploads/avatars/%ZZ');
+    expect(res.status).toBe(404);
+    expect(res.body.error).toMatch(/^Cannot GET /);
+  });
+
+  it('UPLOADS-P17 — traversal shapes fall through, raw and encoded', async () => {
+    const raw = await request(app).get('/uploads/avatars/../covers/x.png');
+    expect(raw.status).toBe(404);
+    const encoded = await request(app).get('/uploads/avatars/..%2Fcovers%2Fx.png');
+    expect(encoded.status).toBe(404);
+  });
+
+  it('UPLOADS-P18 — dotfile basename falls through', async () => {
+    const res = await request(app).get('/uploads/avatars/.uploads-static-parity-hidden');
+    expect(res.status).toBe(404);
+    expect(res.body.error).toBe('Cannot GET /uploads/avatars/.uploads-static-parity-hidden');
+  });
+});
+
+describe('/uploads static parity — all four mounts', () => {
+  it('UPLOADS-P21 — covers/journey(nested)/places each serve a hit and 404 a miss', async () => {
+    const coverHit = await request(app).get(`/uploads/covers/${NAME}`);
+    expect(coverHit.status).toBe(200);
+    const coverMiss = await request(app).get('/uploads/covers/nope.png');
+    expect(coverMiss.status).toBe(404);
+    expect(coverMiss.body.error).toBe('Cannot GET /uploads/covers/nope.png');
+
+    // Nested name — journey thumbs live at journey/thumbs/<hash>.jpg and are
+    // served through the same mount; pins multi-segment storage keys.
+    const journeyHit = await request(app).get('/uploads/journey/thumbs/uploads-static-parity.jpg');
+    expect(journeyHit.status).toBe(200);
+    expect(journeyHit.headers['content-type']).toBe('image/jpeg');
+    const journeyMiss = await request(app).get('/uploads/journey/thumbs/nope.jpg');
+    expect(journeyMiss.status).toBe(404);
+
+    const placeHit = await request(app).get(`/uploads/places/${NAME}`);
+    expect(placeHit.status).toBe(200);
+    const placeMiss = await request(app).get('/uploads/places/nope.png');
+    expect(placeMiss.status).toBe(404);
+  });
+});
+
+describe('/uploads static — approved deliberate deltas (RED against express.static)', () => {
+  // D3 (slice-3 plan): send's legacy dotfiles mode only ignores dot BASENAMES,
+  // so a file under a dot-directory is servable today. Central storage key
+  // validation rejects every dot segment — the spec's ".tmp must remain
+  // unservable" pin.
+  it('UPLOADS-P22 — a file under a dot-directory is NOT servable', async () => {
+    const res = await request(app).get('/uploads/avatars/.tmp/uploads-static-parity-spooled.bin');
+    expect(res.status).toBe(404);
+    expect(res.body.error).toBe('Cannot GET /uploads/avatars/.tmp/uploads-static-parity-spooled.bin');
+  });
+
+  // D1/D2: express.static answers directory URLs with a 301 trailing-slash
+  // redirect (and an index.html lookup on the slashed form). The storage
+  // handler treats both as a plain miss.
+  it('UPLOADS-P23 — mount root (with and without slash) is a miss', async () => {
+    const bare = await request(app).get('/uploads/avatars');
+    expect(bare.status).toBe(404);
+    expect(bare.body.error).toBe('Cannot GET /uploads/avatars');
+    const slashed = await request(app).get('/uploads/avatars/');
+    expect(slashed.status).toBe(404);
+  });
+
+  it('UPLOADS-P24 — a real subdirectory URL is a miss, not a redirect', async () => {
+    const res = await request(app).get('/uploads/journey/parity-subdir');
+    expect(res.status).toBe(404);
+    expect(res.body.error).toBe('Cannot GET /uploads/journey/parity-subdir');
+  });
+});

server/tests/unit/nest/platform.controller.test.ts

@@ -5,23 +5,27 @@ import { NotFoundException } from '@nestjs/common';
 const h = vi.hoisted(() => ({
   verifyJwtAndLoadUser: vi.fn(),
   dbPrepare: vi.fn(),
-  existsSync: vi.fn(),
+  exists: vi.fn(),
+  sendToResponse: vi.fn(),
 }));
 
 vi.mock('../../../src/nest/auth/jwt-verify', () => ({ verifyJwtAndLoadUser: h.verifyJwtAndLoadUser }));
 vi.mock('../../../src/db/database', () => ({ db: { prepare: h.dbPrepare } }));
 
-vi.mock('node:fs', async (orig) => {
-  const real = (await orig()) as Record<string, unknown>;
-  return { ...real, default: { ...(real.default as object), existsSync: h.existsSync }, existsSync: h.existsSync };
-});
-
 import {
   applyPlatformUploads,
   applyPlatformSpa,
   applyPlatformStatic,
+  storageStaticHandler,
 } from '../../../src/nest/platform/platform.routes';
 import { SpaFallbackFilter } from '../../../src/nest/platform/spa-fallback.filter';
+import { StorageNotFoundError, StorageInvalidKeyError } from '../../../src/nest/storage/storage.types';
+import type { StorageService } from '../../../src/nest/storage/storage.service';
+
+// The serving swap addresses files as (category, name) on the injected facade;
+// these unit tests only assert routing/auth/error mapping, so a two-method stub
+// is the whole storage surface.
+const storage = { exists: h.exists, sendToResponse: h.sendToResponse } as unknown as StorageService;
 
 // Tagged sentinel for express.static — we only need to know it was registered on
 // the right path, not run it.
@@ -76,18 +80,24 @@ beforeEach(() => {
 });
 
 describe('applyPlatformUploads', () => {
-  it('registers the static avatar/cover/journey mounts + the files block', () => {
+  it('registers the four static mounts + the files block', () => {
     const { app, calls } = fakeApp();
-    applyPlatformUploads(app);
+    applyPlatformUploads(app, storage);
     const paths = calls.filter((c) => c.method === 'use').map((c) => c.path);
     expect(paths).toEqual(
-      expect.arrayContaining(['/uploads/avatars', '/uploads/covers', '/uploads/journey', '/uploads/files']),
+      expect.arrayContaining([
+        '/uploads/avatars',
+        '/uploads/covers',
+        '/uploads/journey',
+        '/uploads/places',
+        '/uploads/files',
+      ]),
     );
   });
 
   it('the /uploads/files block always answers 401', () => {
     const { app, calls } = fakeApp();
-    applyPlatformUploads(app);
+    applyPlatformUploads(app, storage);
     const filesBlock = calls.find((c) => c.path === '/uploads/files')!.handlers[0];
     const res = makeRes();
     filesBlock({}, res);
@@ -98,103 +108,216 @@ describe('applyPlatformUploads', () => {
   describe('GET /uploads/photos/:filename', () => {
     function photoHandler() {
       const { app, calls } = fakeApp();
-      applyPlatformUploads(app);
+      applyPlatformUploads(app, storage);
       return calls.find((c) => c.method === 'get' && c.path === '/uploads/photos/:filename')!.handlers[0];
     }
+    const next = vi.fn();
 
-    it('403 when the resolved path escapes the photos dir', () => {
-      // basename() strips the traversal, but feed a name that resolves outside by
-      // stubbing path indirectly is hard — instead exercise the existsSync 404 etc.
-      // The startsWith guard is defensive; cover it via a filename of '..'.
-      const handler = photoHandler();
+    it('403 when the basename is a bare traversal segment', async () => {
+      // Parity pin: the old resolve()+startsWith guard could only fire after
+      // basename() when the remaining segment was '..'.
       const res = makeRes();
-      // path.basename('..') === '..' -> join(photos,'..') resolves to uploads -> not under photos
-      handler({ params: { filename: '..' }, headers: {}, query: {} }, res);
+      await photoHandler()({ params: { filename: '..' }, headers: {}, query: {} }, res, next);
       expect(res.statusCode).toBe(403);
       expect(res.body).toBe('Forbidden');
+      expect(h.exists).not.toHaveBeenCalled();
     });
 
-    it('404 when the file does not exist', () => {
-      h.existsSync.mockReturnValue(false);
+    it('404 when the object does not exist — checked before auth', async () => {
+      h.exists.mockResolvedValue(false);
       const res = makeRes();
-      photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: {} }, res);
+      await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: {} }, res, next);
+      expect(h.exists).toHaveBeenCalledWith('photos', 'a.jpg');
+      expect(res.statusCode).toBe(404);
+      expect(res.body).toBe('Not found');
+      expect(h.verifyJwtAndLoadUser).not.toHaveBeenCalled();
+    });
+
+    it('404 when the key is invalid (exists rejects) — still before auth', async () => {
+      h.exists.mockRejectedValue(new StorageInvalidKeyError('photos/.'));
+      const res = makeRes();
+      await photoHandler()({ params: { filename: '.' }, headers: {}, query: {} }, res, next);
       expect(res.statusCode).toBe(404);
       expect(res.body).toBe('Not found');
     });
 
-    it('401 when no token is supplied', () => {
-      h.existsSync.mockReturnValue(true);
+    it('401 when no token is supplied', async () => {
+      h.exists.mockResolvedValue(true);
       const res = makeRes();
-      photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: {} }, res);
+      await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: {} }, res, next);
       expect(res.statusCode).toBe(401);
       expect(res.body).toBe('Authentication required');
     });
 
-    it('serves the file for a valid JWT session (Bearer header)', () => {
-      h.existsSync.mockReturnValue(true);
+    it('serves the file for a valid JWT session (Bearer header)', async () => {
+      h.exists.mockResolvedValue(true);
+      h.sendToResponse.mockResolvedValue(undefined);
       h.verifyJwtAndLoadUser.mockReturnValue({ id: 1 });
       const res = makeRes();
-      photoHandler()(
+      await photoHandler()(
         { params: { filename: 'a.jpg' }, headers: { authorization: 'Bearer jwt123' }, query: {} },
         res,
+        next,
       );
       expect(h.verifyJwtAndLoadUser).toHaveBeenCalledWith('jwt123');
-      expect(String(res.body)).toContain('FILE:');
+      expect(h.sendToResponse).toHaveBeenCalledWith('photos', 'a.jpg', res);
     });
 
-    it('reads the token from the query string when there is no Bearer header', () => {
-      h.existsSync.mockReturnValue(true);
+    it('reads the token from the query string when there is no Bearer header', async () => {
+      h.exists.mockResolvedValue(true);
+      h.sendToResponse.mockResolvedValue(undefined);
       h.verifyJwtAndLoadUser.mockReturnValue({ id: 1 });
       const res = makeRes();
-      photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'qtok' } }, res);
+      await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'qtok' } }, res, next);
       expect(h.verifyJwtAndLoadUser).toHaveBeenCalledWith('qtok');
-      expect(String(res.body)).toContain('FILE:');
+      expect(h.sendToResponse).toHaveBeenCalledWith('photos', 'a.jpg', res);
     });
 
-    it('401 when the token is not a session and the photo row is missing', () => {
-      h.existsSync.mockReturnValue(true);
+    it('401 when the token is not a session and the photo row is missing', async () => {
+      h.exists.mockResolvedValue(true);
       h.verifyJwtAndLoadUser.mockReturnValue(null);
       h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue(undefined) });
       const res = makeRes();
-      photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res);
+      await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res, next);
       expect(res.statusCode).toBe(401);
     });
 
-    it('401 when a share token does not cover the photo trip', () => {
-      h.existsSync.mockReturnValue(true);
+    it('401 when a share token does not cover the photo trip', async () => {
+      h.exists.mockResolvedValue(true);
       h.verifyJwtAndLoadUser.mockReturnValue(null);
       const photoStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) };
       const shareStmt = { get: vi.fn().mockReturnValue({ trip_id: 8 }) };
       h.dbPrepare.mockImplementationOnce(() => photoStmt).mockImplementationOnce(() => shareStmt);
       const res = makeRes();
-      photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res);
+      await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res, next);
       expect(res.statusCode).toBe(401);
     });
 
-    it('401 when there is no matching share token at all', () => {
-      h.existsSync.mockReturnValue(true);
+    it('401 when there is no matching share token at all', async () => {
+      h.exists.mockResolvedValue(true);
       h.verifyJwtAndLoadUser.mockReturnValue(null);
       const photoStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) };
       const shareStmt = { get: vi.fn().mockReturnValue(undefined) };
       h.dbPrepare.mockImplementationOnce(() => photoStmt).mockImplementationOnce(() => shareStmt);
       const res = makeRes();
-      photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res);
+      await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res, next);
       expect(res.statusCode).toBe(401);
     });
 
-    it('serves the file when the share token covers the photo trip', () => {
-      h.existsSync.mockReturnValue(true);
+    it('serves the file when the share token covers the photo trip', async () => {
+      h.exists.mockResolvedValue(true);
+      h.sendToResponse.mockResolvedValue(undefined);
       h.verifyJwtAndLoadUser.mockReturnValue(null);
       const photoStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) };
       const shareStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) };
       h.dbPrepare.mockImplementationOnce(() => photoStmt).mockImplementationOnce(() => shareStmt);
       const res = makeRes();
-      photoHandler()(
+      await photoHandler()(
         { params: { filename: 'a.jpg' }, headers: { authorization: 'Bearer share1' }, query: {} },
         res,
+        next,
       );
-      expect(String(res.body)).toContain('FILE:');
+      expect(h.sendToResponse).toHaveBeenCalledWith('photos', 'a.jpg', res);
     });
+
+    it('404 when the object vanishes between the exists check and the send', async () => {
+      // Approved deviation D7: the delete race maps to the same 404 text.
+      h.exists.mockResolvedValue(true);
+      h.verifyJwtAndLoadUser.mockReturnValue({ id: 1 });
+      h.sendToResponse.mockRejectedValue(new StorageNotFoundError('photos/a.jpg'));
+      const res = makeRes();
+      const resAny = res as unknown as { headersSent?: boolean };
+      resAny.headersSent = false;
+      await photoHandler()(
+        { params: { filename: 'a.jpg' }, headers: { authorization: 'Bearer jwt123' }, query: {} },
+        res,
+        next,
+      );
+      expect(res.statusCode).toBe(404);
+      expect(res.body).toBe('Not found');
+    });
+  });
+});
+
+describe('storageStaticHandler', () => {
+  const req = (over: Record<string, unknown> = {}) => ({ method: 'GET', path: '/x.png', ...over });
+
+  function run(over: Record<string, unknown> = {}) {
+    const handler = storageStaticHandler(storage, 'avatars');
+    const res = makeRes();
+    const next = vi.fn();
+    const out = handler(req(over) as never, res as never, next as never);
+    return { res, next, out };
+  }
+
+  it('serves a hit through sendToResponse with the decoded name', async () => {
+    h.sendToResponse.mockResolvedValue(undefined);
+    const { res, next, out } = run({ path: '//caf%C3%A9.png' });
+    await out;
+    expect(h.sendToResponse).toHaveBeenCalledWith('avatars', 'café.png', res);
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it('non-GET/HEAD methods fall through without touching storage', async () => {
+    const { next } = run({ method: 'POST' });
+    expect(next).toHaveBeenCalledWith();
+    expect(h.sendToResponse).not.toHaveBeenCalled();
+  });
+
+  it('undecodable percent-encoding falls through', async () => {
+    const { next } = run({ path: '/%ZZ' });
+    expect(next).toHaveBeenCalledWith();
+    expect(h.sendToResponse).not.toHaveBeenCalled();
+  });
+
+  it('a miss calls next() with no args', async () => {
+    h.sendToResponse.mockRejectedValue(new StorageNotFoundError('avatars/x.png'));
+    const { next, out } = run();
+    await out;
+    expect(next).toHaveBeenCalledWith();
+  });
+
+  it('an invalid key calls next() with no args', async () => {
+    h.sendToResponse.mockRejectedValue(new StorageInvalidKeyError('avatars/..'));
+    const { next, out } = run();
+    await out;
+    expect(next).toHaveBeenCalledWith();
+  });
+
+  it('a send-layer 404 (stat→send race) falls through', async () => {
+    h.sendToResponse.mockRejectedValue(Object.assign(new Error('ENOENT'), { status: 404 }));
+    const { next, out } = run();
+    await out;
+    expect(next).toHaveBeenCalledWith();
+  });
+
+  it('EISDIR falls through', async () => {
+    h.sendToResponse.mockRejectedValue(Object.assign(new Error('dir'), { code: 'EISDIR' }));
+    const { next, out } = run();
+    await out;
+    expect(next).toHaveBeenCalledWith();
+  });
+
+  it('a client abort is swallowed', async () => {
+    h.sendToResponse.mockRejectedValue(Object.assign(new Error('aborted'), { code: 'ECONNABORTED' }));
+    const { next, out } = run();
+    await out;
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it('a mid-stream write error is swallowed', async () => {
+    h.sendToResponse.mockRejectedValue(Object.assign(new Error('write EPIPE'), { syscall: 'write' }));
+    const { next, out } = run();
+    await out;
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it('anything else goes to next(err) for finalhandler', async () => {
+    const boom = new Error('boom');
+    h.sendToResponse.mockRejectedValue(boom);
+    const { next, out } = run();
+    await out;
+    expect(next).toHaveBeenCalledWith(boom);
   });
 });