diff --git a/server/src/bootstrap.ts b/server/src/bootstrap.ts index 246761ee2..7471e3442 100644 --- a/server/src/bootstrap.ts +++ b/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 { // config instead of reading process.env itself. const http = app.get>(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 diff --git a/server/src/nest/platform/platform.routes.ts b/server/src/nest/platform/platform.routes.ts index da6963e3c..1d30bf779 100644 --- a/server/src/nest/platform/platform.routes.ts +++ b/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 { + 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) => { diff --git a/server/tests/integration/uploads-static.test.ts b/server/tests/integration/uploads-static.test.ts new file mode 100644 index 000000000..3e37c26f4 --- /dev/null +++ b/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/.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'); + }); +}); diff --git a/server/tests/unit/nest/platform.controller.test.ts b/server/tests/unit/nest/platform.controller.test.ts index af139f74e..9efcb7d3e 100644 --- a/server/tests/unit/nest/platform.controller.test.ts +++ b/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; - 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 = {}) => ({ method: 'GET', path: '/x.png', ...over }); + + function run(over: Record = {}) { + 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); }); });