diff --git a/server/src/bootstrap.ts b/server/src/bootstrap.ts index 4a40200e6..196ea3f95 100644 --- a/server/src/bootstrap.ts +++ b/server/src/bootstrap.ts @@ -114,7 +114,7 @@ export async function buildApp(): Promise { // 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)); + applyPlatformUploads(instance, app.get(StorageService), orm); // 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/db/dialect/sql-functions.ts b/server/src/db/dialect/sql-functions.ts index 558980794..145f26e2c 100644 --- a/server/src/db/dialect/sql-functions.ts +++ b/server/src/db/dialect/sql-functions.ts @@ -48,3 +48,29 @@ export function currentTimestamp(platform: Platform): RawQueryFragment { if (platform instanceof SqlitePlatform) return raw('CURRENT_TIMESTAMP'); return unsupported(platform); } + +/** + * A bare reference to another column of the same row, for a filter or update + * value that must compare against or combine with a sibling column instead + * of a bound parameter (e.g. `used_count < max_uses` — `InviteTokensRepository + * .incrementUsedCount`'s capacity guard). Portable across dialects on its own, + * but still routed through this file (not spelled with `raw()` in a + * repository) so every escape hatch stays in one place ESLint's + * `no-restricted-syntax` rule for `src/db/repositories/**` can enforce. + */ +export function columnRef(platform: Platform, ref: string): RawQueryFragment { + if (platform instanceof SqlitePlatform) return raw(column(ref)); + return unsupported(platform); +} + +/** A column shifted by a signed integer amount: ` + n` / ` - n`. */ +export function columnIncrementedBy(platform: Platform, ref: string, amount: number): RawQueryFragment { + if (!Number.isInteger(amount)) { + throw new Error(`sql-functions: columnIncrementedBy needs an integer amount, got ${amount}`); + } + if (platform instanceof SqlitePlatform) { + const sign = amount < 0 ? '-' : '+'; + return raw(`${column(ref)} ${sign} ${Math.abs(amount)}`); + } + return unsupported(platform); +} diff --git a/server/src/db/repositories/InviteTokens.repository.ts b/server/src/db/repositories/InviteTokens.repository.ts index 68aa8d0c2..16ac0e5dc 100644 --- a/server/src/db/repositories/InviteTokens.repository.ts +++ b/server/src/db/repositories/InviteTokens.repository.ts @@ -1,4 +1,111 @@ import type { InviteTokens } from '../entities/InviteTokens.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; +import { columnIncrementedBy, columnRef } from '../dialect/sql-functions'; import { EntityRepository } from '@mikro-orm/sql'; -export class InviteTokensRepository extends EntityRepository {} +/** An `invite_tokens` row as the API emits it. */ +export interface InviteTokenRow { + id: number; + token: string; + max_uses: number; + used_count: number; + expires_at: string | null; + created_by: number; + created_at: string | null; + trip_id: number | null; +} + +const _inviteTokenRowKeys: AssertRowKeys = true; + +/** The column set `RegistrationInvitesService.createInvite` (Task 3) writes. */ +export interface NewInviteTokenRow { + token: string; + max_uses: number; + expires_at: string | null; + created_by: number; + trip_id?: number | null; +} + +/** + * Shared by `AuthService` (registration), `RegistrationInvitesService` (admin + * invite management, Task 3) and `OidcService` (SSO-bound registration, + * Task 6) — one table, one repository, per the inventory's §6 cross-domain + * ruling. Task 0 builds the three methods every later task needs in common; + * Task 3 extends this with the admin list/create/delete surface (RI1/RI5-RI7). + */ +export class InviteTokensRepository extends EntityRepository { + /** + * `SELECT * FROM invite_tokens WHERE token = ?` + * + * Not a primary-key filter — `token` is a unique column, not `id` — so this + * always re-queries; no `{ refresh: true }` needed (Task 3a review's + * precision ruling: the identity-map short-circuit only fires for a + * PK-only filter). + */ + async findValid(token: string): Promise { + const invite = await this.findOne({ token }); + return invite ? (toRow(invite) as InviteTokenRow) : null; + } + + /** + * `INSERT INTO invite_tokens (token, max_uses, expires_at, created_by, trip_id) + * VALUES (?, ?, ?, ?, ?)`, re-selected (RI4/RI5's column set — Task 3 is + * the first real caller). `created_by`/`trip_id` are FK columns backed by + * `persist(false)` scalar mirrors on the entity (same pattern as + * `Categories.user`/`user_id`); the repository writes them through the + * `Ref`-typed relation properties, which MikroORM accepts a raw primary + * key for. + */ + async insertInvite(row: NewInviteTokenRow): Promise { + const invite = this.create({ + token: row.token, + max_uses: row.max_uses, + expires_at: row.expires_at, + createdByRef: row.created_by, + trip: row.trip_id ?? null, + }); + await this.getEntityManager().persist(invite).flush(); + await this.getEntityManager().refresh(invite); + return toRow(invite) as InviteTokenRow; + } + + /** + * `UPDATE invite_tokens SET used_count = used_count + 1 + * WHERE token = ? AND (max_uses = 0 OR used_count < max_uses) + * RETURNING *` + * + * ONE conditional UPDATE … RETURNING statement (the QueryBuilder, D4's T5 + * tier) — race-safe: D6's single SQLite connection serialises writers, so + * the WHERE clause's capacity check and the increment commit as one + * statement. Two callers racing the same one-use invite can never both + * see `used_count < max_uses` true and both write: exactly one UPDATE + * matches and its RETURNING carries the new row back; the other's WHERE + * clause fails once the first has committed, and it gets `null` — no + * second SELECT, no SELECT→await→UPDATE window for either caller to lose. + * + * `null` means "no row matched" — a bad/unknown token OR an invite already + * at capacity are indistinguishable from inside this method by design + * (this repository never interprets the miss): `AuthService`'s `AU11` + * only warns and continues on a miss, `OidcService`'s `O13` throws — both + * keep their own interpretation, calling this one shared method. + * + * `used_count < max_uses` compares two columns of the SAME row, and + * `used_count = used_count + 1` needs a sibling-column reference too — both + * go through `sql-functions.ts` (`columnRef`/`columnIncrementedBy`), never + * a hand-spelled `raw()` call here: repositories don't spell dialect SQL + * inline (ESLint enforces this on `src/db/repositories/**`), even for an + * expression that happens to be portable across dialects already. + */ + async incrementUsedCount(token: string): Promise { + const platform = this.getEntityManager().getPlatform(); + const result = await this.qb() + .update({ used_count: columnIncrementedBy(platform, 'used_count', 1) }) + .where({ + token, + $or: [{ max_uses: 0 }, { used_count: { $lt: columnRef(platform, 'max_uses') } }], + }) + .returning('*') + .execute('run'); + return result.row ? (result.row as InviteTokenRow) : null; + } +} diff --git a/server/src/nest/mcp-transport/mcp-transport.service.ts b/server/src/nest/mcp-transport/mcp-transport.service.ts index 74327f251..4b5950970 100644 --- a/server/src/nest/mcp-transport/mcp-transport.service.ts +++ b/server/src/nest/mcp-transport/mcp-transport.service.ts @@ -125,6 +125,18 @@ export class McpTransportService { private readonly registry: McpRegistryService, ) {} + /** + * D6 (Plan 3b Task 0): this bearer-token verification step already runs + * inside a per-request EntityManager fork — no wrapper needed here. `/mcp` + * is an ordinary (`@Public()`) Nest-routed controller, so + * `@mikro-orm/nestjs`'s `MikroOrmMiddleware` (`MikroOrmModule.forRoot`'s + * default `registerRequestContext`, applied via `NestModule.configure()` + * for ALL routes) forks a context before any guard — and therefore before + * this method — ever runs. Verified, not assumed: see + * `tests/integration/mcp.test.ts`'s "MCP bearer-token verification runs + * inside the HTTP request context" suite (MCP-CTX-001/002), which forces a + * genuine repository read at this exact point and confirms it succeeds. + */ async verifyToken(authHeader: string | undefined): Promise { if (!authHeader) return null; // M8: strictly require "Bearer" scheme (RFC 6750) diff --git a/server/src/nest/platform/platform.routes.ts b/server/src/nest/platform/platform.routes.ts index e9816ed92..3a16fbe67 100644 --- a/server/src/nest/platform/platform.routes.ts +++ b/server/src/nest/platform/platform.routes.ts @@ -1,9 +1,11 @@ import express, { Request, Response, NextFunction } from 'express'; import path from 'node:path'; +import type { EntityManager } from '@mikro-orm/core'; import { readEnv } from '../../app-config'; import { verifyJwtAndLoadUser } from '../auth/jwt-verify'; import { db } from '../../db/database'; +import { withRequestContext } from '../database/request-context'; import { StorageService } from '../storage/storage.service'; import { StorageInvalidKeyError, StorageNotFoundError, type StorageCategory } from '../storage/storage.types'; @@ -125,8 +127,30 @@ async function servePhoto(storage: StorageService, req: Request, res: Response): /** * Static + guarded /uploads/* routes. Must be applied BEFORE the API route mounts * (identical to its original position near the top of createApp). + * + * `orm` (Plan 3b Task 0, D6/plan3b-sql-inventory.md §5/§6): `servePhoto` is + * mounted here, on the raw Express instance, BEFORE `app.init()` — every + * `/uploads/photos/*` request runs this handler entirely outside Nest's + * per-request EntityManager fork, forever, not just during a boot window. + * `verifyJwtAndLoadUser` (`jwt-verify.ts`) reads `users` through the legacy + * `db` proxy today, so nothing here needs the ORM yet — but the inventory's + * proposed conversion (`UsersRepository.findByIdWithPasswordVersion`, Task 1) + * would throw `cannotUseGlobalContext` on every photo request the instant it + * lands, because nothing forks a context for this pre-init route. Wrapping + * the handler in `withRequestContext` NOW, before any repository call exists + * on this path, means Task 1 converts `jwt-verify.ts` without touching this + * file again and without a single broken photo request in between. + * + * Optional at the type level only, the same shape `TrekWsAdapter` uses for + * its own D6 wrapper (`src/nest/realtime/trek-ws.adapter.ts`): `bootstrap.ts` + * always passes the real `MikroORM` it already resolved. Fail-closed shape + * per the Plan 3b Rulings — a request-time choke point THROWS when the ORM + * is absent rather than running `servePhoto` unwrapped: the photo route + * answers 500 through Express's error path (`next(err)`), the same shape the + * WS adapter's message dispatch already uses (throw before calling the + * handler at all, never a silent degrade). */ -export function applyPlatformUploads(app: express.Application, storage: StorageService): void { +export function applyPlatformUploads(app: express.Application, storage: StorageService, orm?: { em: EntityManager }): void { // Static: avatars, covers, and journey photos. // // Security model (audit SEC-M9): these paths are unauthenticated by @@ -155,9 +179,13 @@ export function applyPlatformUploads(app: express.Application, storage: StorageS // 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, next: NextFunction) => - servePhoto(storage, req, res).catch(next), - ); + app.get('/uploads/photos/:filename', (req: Request, res: Response, next: NextFunction) => { + if (!orm) { + next(new Error('applyPlatformUploads: no MikroORM available to build a request context for /uploads/photos/*')); + return; + } + return withRequestContext(orm, () => servePhoto(storage, req, res)).catch(next); + }); // Block direct access to /uploads/files app.use('/uploads/files', (_req: Request, res: Response) => { diff --git a/server/src/nest/realtime/trek-ws.adapter.ts b/server/src/nest/realtime/trek-ws.adapter.ts index a6bfabeeb..fa257fd46 100644 --- a/server/src/nest/realtime/trek-ws.adapter.ts +++ b/server/src/nest/realtime/trek-ws.adapter.ts @@ -166,10 +166,21 @@ export class TrekWsAdapter extends WsAdapter { // Must stay above anything that can close early: a socket that never gets // this listener can still crash the process while it finishes closing. socket.on('error', () => socket.terminate()); + // D6 (Plan 3b Task 0): this dispatches Nest's OnGatewayConnection hook + // (RealtimeGateway.handleConnection) exactly the way bindMessageHandlers + // below dispatches a @SubscribeMessage handler — no HTTP request behind + // it, so nothing forks an EntityManager for it unless this ONE wrapper + // does. Confirmed unwrapped before this change (WSAD-050): a repository + // read during the handshake threw cannotUseGlobalContext. Same + // fail-closed shape as bindMessageHandlers: throw before calling rather + // than run the connection handler unwrapped. + if (!this.orm) { + throw new Error('TrekWsAdapter: no MikroORM available to build a request context for this connection'); + } // The request rides along: the handshake reads the ws token off its query // string, so dropping it here would leave handleConnection with nothing // to authenticate. - callback(socket, request); + withRequestContext(this.orm, () => callback(socket, request)); }); } diff --git a/server/tests/integration/mcp.test.ts b/server/tests/integration/mcp.test.ts index bff10718b..4272fdc10 100644 --- a/server/tests/integration/mcp.test.ts +++ b/server/tests/integration/mcp.test.ts @@ -43,6 +43,8 @@ import { AuditService } from '../../src/nest/audit/audit.service'; import { createTestOrm, type TestOrm } from '../helpers/test-orm'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; import { Users } from '../../src/db/entities/Users.entity'; +import { TokenService } from '../../src/nest/tokens/token.service'; +import { MikroORM } from '@mikro-orm/core'; const oauthDbs = new DatabaseService(testDb); let oauthSvc: OauthService; @@ -757,3 +759,53 @@ describe('MCP rate limiting', () => { } }); }); + +/** + * Plan 3b Task 0 (D6): the MCP transport's bearer-token verification step + * (`McpTransportService.verifyToken`, called from `handle()` before any tool + * dispatch) is expected to already run inside the HTTP request context — + * `/mcp` is an ordinary (if `@Public()`) Nest-routed controller, so + * `@mikro-orm/nestjs`'s per-request EntityManager-fork middleware + * (`MikroOrmModule.forRoot`'s default `registerRequestContext`, registered + * via `NestModule.configure()`/`forRoutes(ALL)`) applies to it exactly like + * every other route, BEFORE any guard runs. Verified here rather than + * assumed: `TokenService.verifyMcpToken` (TK13 in the inventory) is not yet + * repository-backed (Task 2), so this forces a genuine repository read at + * the exact point that method will occupy once converted, through the same + * DI-provided `TokenService` singleton the real `/mcp` route calls. + */ +describe('MCP bearer-token verification runs inside the HTTP request context (Plan 3b Task 0, D6)', () => { + it('MCP-CTX-001: outside of any request, the SAME ORM refuses a query (allowGlobalContext: false is genuinely enforced, not just permissively configured)', async () => { + const orm = nestApp.get(MikroORM); + await expect(orm.em.getRepository(Users).findOne({ id: 1 })).rejects.toThrow(/context/i); + }); + + it('MCP-CTX-002: a repository read forced inside verifyMcpToken (the trek_ bearer-token branch) succeeds — the /mcp route already forks a request context before the auth step runs', async () => { + testDb.prepare("UPDATE addons SET enabled = 1 WHERE id = 'mcp'").run(); + const { user } = createUser(testDb); + const orm = nestApp.get(MikroORM); + const tokenService = nestApp.get(TokenService); + let found: unknown; + let caught: unknown; + const spy = vi.spyOn(tokenService, 'verifyMcpToken').mockImplementation(async () => { + try { + found = await orm.em.getRepository(Users).findOne({ id: user.id }); + } catch (e) { + caught = e; + } + return found ? { id: user.id, username: user.username, email: user.email, role: user.role } : null; + }); + try { + const res = await request(app) + .post('/mcp') + .set('Authorization', 'Bearer trek_forces_a_repository_read') + .set('Accept', 'application/json, text/event-stream') + .send({ jsonrpc: '2.0', method: 'initialize', id: 1, params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 'test', version: '1' } } }); + expect(caught).toBeUndefined(); + expect(found).toBeTruthy(); + expect(res.status).toBe(200); + } finally { + spy.mockRestore(); + } + }); +}); diff --git a/server/tests/integration/orm-request-context-seams.test.ts b/server/tests/integration/orm-request-context-seams.test.ts index 2dbe5e905..a878c6e90 100644 --- a/server/tests/integration/orm-request-context-seams.test.ts +++ b/server/tests/integration/orm-request-context-seams.test.ts @@ -49,9 +49,15 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi. import { db as testDb } from '../../src/db/database'; import { MikroORM } from '@mikro-orm/core'; +import request from 'supertest'; +import type { Application } from 'express'; import { buildApp } from '../../src/bootstrap'; import { CronRegistrarService } from '../../src/nest/scheduling/cron-registrar.service'; import { PluginRuntimeService } from '../../src/nest/plugins/plugin-runtime.service'; +import { StorageService } from '../../src/nest/storage/storage.service'; +import { Users } from '../../src/db/entities/Users.entity'; +import { createUser } from '../helpers/factories'; +import { generateToken } from '../helpers/auth'; describe('ORM request-context seams populated in production', () => { let app: INestApplication; @@ -82,4 +88,46 @@ describe('ORM request-context seams populated in production', () => { const wsAdapter = (app as unknown as { config: { getIoAdapter(): { orm?: MikroORM } } }).config.getIoAdapter(); expect(wsAdapter.orm).toBe(orm); }); + + /** + * SEAM-002 (Plan 3b Task 0): `applyPlatformUploads`'s `orm` parameter is a + * NEW seam — `bootstrap.ts:117` now passes `app.get(MikroORM)` into it, and + * unlike the four seams SEAM-001 pins, nothing keeps that reference on an + * introspectable field (it's captured in `platform.routes.ts`'s closure + * around the registered `/uploads/photos/:filename` handler). So this is a + * behavioural identity proof instead of a structural one: drive the REAL + * pre-init route through the REAL booted app, force a repository read at + * the earliest point inside the handler, and confirm it resolves against + * the exact `MikroORM` this app booted with — the same one every other row + * in this file checks — by reading back a user this same test just created + * through the SAME connection (a foreign/unwired ORM instance would not + * see that row at all). + */ + it('SEAM-002: the pre-init /uploads/photos/* route (applyPlatformUploads, bootstrap.ts) forks a request context off the SAME MikroORM app.get(MikroORM) does', async () => { + const orm = app.get(MikroORM); + const { user } = createUser(testDb); + const storage = app.get(StorageService); + let found: unknown; + let caught: unknown; + const spy = vi.spyOn(storage, 'exists').mockImplementation(async () => { + try { + found = await orm.em.getRepository(Users).findOne({ id: user.id }); + } catch (e) { + caught = e; + } + return true; + }); + try { + const httpApp = app.getHttpAdapter().getInstance() as Application; + // The file need not exist: storage.exists is stubbed above, and the + // identity proof is the repository read, not the HTTP response body. + await request(httpApp) + .get('/uploads/photos/seam-002-does-not-need-to-exist.jpg') + .set('Authorization', `Bearer ${generateToken(user.id)}`); + expect(caught).toBeUndefined(); + expect(found).toBeTruthy(); + } finally { + spy.mockRestore(); + } + }); }); diff --git a/server/tests/integration/uploads-photo-request-context.test.ts b/server/tests/integration/uploads-photo-request-context.test.ts new file mode 100644 index 000000000..793d5f716 --- /dev/null +++ b/server/tests/integration/uploads-photo-request-context.test.ts @@ -0,0 +1,147 @@ +/** + * Plan 3b Task 0 (D6): `servePhoto` — the pre-init `/uploads/photos/:filename` + * Express route mounted by `applyPlatformUploads` (`bootstrap.ts`, BEFORE + * `app.init()`) — now runs inside `withRequestContext(orm, …)`. + * + * Per the inventory (§5/§6): `applyPlatformUploads` is mounted permanently on + * the raw Express instance, not just during a boot window, so every request + * to `/uploads/photos/*` forever ran outside Nest's per-request EntityManager + * fork. Converting `jwt-verify.ts`'s read off the raw `db` proxy (Task 1) + * without this wrapper first would break every authenticated photo request + * the moment it landed. + * + * Three proofs, mirroring Plan 3a's WSAD-040/041 shape: + * - PHOTOCTX-001: parity — an authenticated request still 200s exactly as + * before the wrap. + * - PHOTOCTX-002: load-bearing (positive) — a repository read forced inside + * the handler (as Task 1's `jwt-verify.ts` conversion will add one) + * succeeds, because `withRequestContext` genuinely covers the whole async + * chain of `servePhoto`, not just its synchronous prologue. + * - PHOTOCTX-003: load-bearing (negative) — the SAME real, exported + * `applyPlatformUploads`, called with no ORM, answers 500 through + * Express's error path BEFORE `servePhoto` ever runs (fail-closed, per + * the Plan 3b Rulings) — proving the wrapper is what makes PHOTOCTX-002 + * succeed, not an accident of the test setup. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach } from 'vitest'; +import request from 'supertest'; +import express from 'express'; +import type { Application } from 'express'; +import type { INestApplication } from '@nestjs/common'; +import fs from 'node:fs'; +import path from 'node:path'; +import { MikroORM } from '@mikro-orm/core'; + +vi.mock('../../src/db/database', async () => { + const { createSnapshotTestDb, buildDbMock } = await import('../helpers/db-mock'); + return buildDbMock(createSnapshotTestDb()); +}); +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() })); + +import { db as testDb } from '../../src/db/database'; +import { buildApp } from '../../src/bootstrap'; +import { resetTestDb } from '../helpers/test-db'; +import { createUser } from '../helpers/factories'; +import { generateToken } from '../helpers/auth'; +import { applyPlatformUploads } from '../../src/nest/platform/platform.routes'; +import { StorageService } from '../../src/nest/storage/storage.service'; +import { Users } from '../../src/db/entities/Users.entity'; + +let nestApp: INestApplication; +let app: Application; + +// A real file under the local driver's actual 'photos' root (DEFAULT_UPLOADS_ROOT, +// server/uploads — same fixture pattern as tests/e2e/share.e2e.test.ts's +// photos-google fixture, one category over). +const photoName = 'trek-context-photo.integration.jpg'; +const photoFile = path.join(__dirname, '../../uploads/photos', photoName); +const photoBytes = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); // JPEG-ish header + +beforeAll(async () => { + fs.mkdirSync(path.dirname(photoFile), { recursive: true }); + fs.writeFileSync(photoFile, photoBytes); + nestApp = await buildApp(); + app = nestApp.getHttpAdapter().getInstance(); +}); + +afterAll(async () => { + await nestApp.close(); + try { fs.unlinkSync(photoFile); } catch { /* ignore */ } +}); + +beforeEach(() => { + resetTestDb(testDb); +}); + +describe('GET /uploads/photos/:filename — request context (Plan 3b Task 0, D6)', () => { + it('PHOTOCTX-001: a valid JWT session still serves the photo — the wrap changes nothing observable', async () => { + const { user } = createUser(testDb); + const res = await request(app) + .get(`/uploads/photos/${photoName}`) + .set('Authorization', `Bearer ${generateToken(user.id)}`); + expect(res.status).toBe(200); + expect(Buffer.from(res.body as Buffer)).toEqual(photoBytes); + }); + + it('PHOTOCTX-002: a repository read forced inside the handler succeeds under the wrap (load-bearing positive proof)', async () => { + const { user } = createUser(testDb); + const storage = nestApp.get(StorageService); + const orm = nestApp.get(MikroORM); + let caught: unknown; + let found: unknown; + // storage.exists is the very first thing servePhoto calls — forcing a + // genuine repository read here, as if a converted jwt-verify.ts (Task 1) + // already read `users` through UsersRepository, proves withRequestContext + // covers the wrapped call's entire async chain, not just its synchronous + // prologue (RequestContext.create is AsyncLocalStorage.run; see + // trek-ws.adapter.ts's docstring for the same property on the WS side). + const spy = vi.spyOn(storage, 'exists').mockImplementation(async () => { + try { + found = await orm.em.getRepository(Users).findOne({ id: user.id }); + } catch (e) { + caught = e; + } + return true; + }); + try { + const res = await request(app) + .get(`/uploads/photos/${photoName}`) + .set('Authorization', `Bearer ${generateToken(user.id)}`); + expect(caught).toBeUndefined(); + expect(found).toBeTruthy(); + expect(res.status).toBe(200); + } finally { + spy.mockRestore(); + } + }); +}); + +describe('applyPlatformUploads without an ORM — fail-closed (Plan 3b Task 0, D6; mutation proof for PHOTOCTX-001/002)', () => { + it('PHOTOCTX-003: the SAME real registration, called with no ORM, answers 500 BEFORE servePhoto ever runs', async () => { + const bare = express(); + const storage = nestApp.get(StorageService); + const existsSpy = vi.spyOn(storage, 'exists'); + // The real, exported applyPlatformUploads — no test double, no hand-call + // of a private wrapper — with its optional `orm` param simply omitted, + // the same shape TrekWsAdapter's WSAD-040 uses to prove its own D6 wrap. + applyPlatformUploads(bare, storage); + try { + const res = await request(bare).get(`/uploads/photos/${photoName}`); + expect(res.status).toBe(500); + // The fail-closed branch throws BEFORE servePhoto calls storage.exists — + // the handler never even started, exactly like WSAD-040's handlerRan check. + expect(existsSpy).not.toHaveBeenCalled(); + } finally { + existsSpy.mockRestore(); + } + }); +}); diff --git a/server/tests/unit/db/dialect/sql-functions.test.ts b/server/tests/unit/db/dialect/sql-functions.test.ts index bbf85052d..a21901cd8 100644 --- a/server/tests/unit/db/dialect/sql-functions.test.ts +++ b/server/tests/unit/db/dialect/sql-functions.test.ts @@ -5,7 +5,7 @@ import { resetTestDb } from '../../../helpers/test-db'; import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; import { createUser } from '../../../helpers/factories'; import { Users } from '../../../../src/db/entities/Users.entity'; -import { currentTimestamp, dateAdd, dateOf } from '../../../../src/db/dialect/sql-functions'; +import { columnIncrementedBy, columnRef, currentTimestamp, dateAdd, dateOf } from '../../../../src/db/dialect/sql-functions'; const testDb = createSnapshotTestDb(); let t: TestOrm; @@ -71,5 +71,81 @@ describe('sql-functions (sqlite)', () => { expect(() => dateOf(foreign, 'u.created_at')).toThrow(/no implementation for platform FakePlatform/); expect(() => dateAdd(foreign, 'u.created_at', 1)).toThrow(/no implementation for platform FakePlatform/); expect(() => currentTimestamp(foreign)).toThrow(/no implementation for platform FakePlatform/); + expect(() => columnRef(foreign, 'u.max_uses')).toThrow(/no implementation for platform FakePlatform/); + expect(() => columnIncrementedBy(foreign, 'u.used_count', 1)).toThrow(/no implementation for platform FakePlatform/); + }); + + // Plan 3b Task 0: InviteTokensRepository.incrementUsedCount needs a + // sibling-column comparison (`used_count < max_uses`) and a sibling-column + // increment (`used_count = used_count + 1`) inside a QueryBuilder filter — + // ESLint bans a repository from spelling `raw()` itself (src/db/repositories/**), + // so both go through these two dialect helpers instead. + it('SQLF-009: columnRef compares one column against another (not a bound value)', async () => { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET login_count = 5, password_version = 3 WHERE id = ?').run(user.id); + const below = await t.em.createQueryBuilder(Users, 'u') + .select('*') + .where({ id: user.id, password_version: { $lt: columnRef(t.em.getPlatform(), 'login_count') } }) + .execute('get', false); + expect((below as { id: number } | undefined)?.id).toBe(user.id); + + testDb.prepare('UPDATE users SET login_count = 1 WHERE id = ?').run(user.id); + const notBelow = await t.em.createQueryBuilder(Users, 'u') + .select('*') + .where({ id: user.id, password_version: { $lt: columnRef(t.em.getPlatform(), 'login_count') } }) + .execute('get', false); + expect(notBelow).toBeUndefined(); + }); + + it('SQLF-010: columnIncrementedBy writes + n / - n, never a bound-parameter add', async () => { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET login_count = 10 WHERE id = ?').run(user.id); + await t.em.createQueryBuilder(Users, 'u') + .update({ login_count: columnIncrementedBy(t.em.getPlatform(), 'login_count', 3) }) + .where({ id: user.id }) + .execute('run'); + t.clear(); + expect((testDb.prepare('SELECT login_count FROM users WHERE id = ?').get(user.id) as { login_count: number }).login_count).toBe(13); + + await t.em.createQueryBuilder(Users, 'u') + .update({ login_count: columnIncrementedBy(t.em.getPlatform(), 'login_count', -5) }) + .where({ id: user.id }) + .execute('run'); + t.clear(); + expect((testDb.prepare('SELECT login_count FROM users WHERE id = ?').get(user.id) as { login_count: number }).login_count).toBe(8); + }); + + it('SQLF-011: columnIncrementedBy rejects a non-integer amount', () => { + expect(() => columnIncrementedBy(t.em.getPlatform(), 'u.login_count', 1.5)).toThrow(/integer amount/); + expect(() => columnIncrementedBy(t.em.getPlatform(), 'u.login_count', Number.NaN)).toThrow(/integer amount/); + }); + + // Plan 3b Task 0: OA8 (oauth_tokens' client-secret-rotation revoke) is the + // one call site in the whole codebase that spells "now" as `datetime('now')` + // instead of `CURRENT_TIMESTAMP` — same effective SQLite value, but D10 + // wants exactly one dialect-function path. `currentTimestamp` already + // renders the literal `CURRENT_TIMESTAMP` fragment (SQLF-003 pins its wire + // format); this proves that fragment's stored text is byte-identical to + // what the legacy `datetime('now')` spelling produces, so OA8 (Task 4) can + // route through `currentTimestamp(platform)` with no behaviour change. + it("SQLF-008: currentTimestamp's CURRENT_TIMESTAMP spelling is byte-identical, in the same statement, to the legacy datetime('now') spelling", async () => { + // One atomic SELECT evaluates both expressions at the exact same instant — + // no timing window between two separate writes, so this is not a + // usually-true "close enough" comparison, it is a real strict equality. + const row = testDb.prepare("SELECT CURRENT_TIMESTAMP as a, datetime('now') as b").get() as { a: string; b: string }; + expect(row.a).toBe(row.b); + expect(row.a).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/); + + // And a real write through the repository-facing dialect helper, read + // back through the raw connection, stores that exact same shape — not + // just the fragment text in isolation. + const { user } = createUser(testDb); + await t.em.createQueryBuilder(Users, 'u') + .update({ created_at: currentTimestamp(t.em.getPlatform()) }) + .where({ id: user.id }) + .execute('run'); + t.clear(); + const stored = testDb.prepare('SELECT created_at FROM users WHERE id = ?').get(user.id) as { created_at: string }; + expect(stored.created_at).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/); }); }); diff --git a/server/tests/unit/db/repositories/InviteTokens.repository.test.ts b/server/tests/unit/db/repositories/InviteTokens.repository.test.ts new file mode 100644 index 000000000..fe2fba43e --- /dev/null +++ b/server/tests/unit/db/repositories/InviteTokens.repository.test.ts @@ -0,0 +1,112 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createInviteToken, createTrip, createUser } from '../../../helpers/factories'; +import { InviteTokens } from '../../../../src/db/entities/InviteTokens.entity'; +import type { InviteTokensRepository } from '../../../../src/db/repositories/InviteTokens.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let invites: InviteTokensRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + invites = t.repo(InviteTokens); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +function rawInvite(id: number): unknown { + return testDb.prepare('SELECT * FROM invite_tokens WHERE id = ?').get(id); +} + +describe('InviteTokensRepository', () => { + describe('findValid', () => { + it('INVREPO-001: a row read through findValid is the SELECT * row, key for key', async () => { + const created = createInviteToken(testDb, { token: 'find-me' }); + const row = await invites.findValid('find-me'); + expect(row).toStrictEqual(rawInvite(created.id)); + }); + + it('INVREPO-002: an unknown token is null', async () => { + expect(await invites.findValid('does-not-exist')).toBeNull(); + }); + }); + + describe('insertInvite', () => { + it('INVREPO-003: inserts and returns exactly the row the legacy re-select would return', async () => { + const { user } = createUser(testDb); + const row = await invites.insertInvite({ + token: 'insert-me', + max_uses: 3, + expires_at: null, + created_by: user.id, + }); + expect(row).toStrictEqual(rawInvite(row.id)); + expect(row.token).toBe('insert-me'); + expect(row.max_uses).toBe(3); + expect(row.used_count).toBe(0); + expect(row.created_by).toBe(user.id); + expect(row.trip_id).toBeNull(); + expect(typeof row.created_at).toBe('string'); + }); + + it('INVREPO-004: trip_id is written when given (per-trip invite)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const row = await invites.insertInvite({ + token: 'trip-bound', + max_uses: 1, + expires_at: null, + created_by: user.id, + trip_id: trip.id, + }); + expect(row.trip_id).toBe(trip.id); + }); + }); + + describe('incrementUsedCount', () => { + it('INVREPO-005: under capacity — increments and returns the updated row', async () => { + const created = createInviteToken(testDb, { token: 'under-cap', max_uses: 3 }); + const row = await invites.incrementUsedCount('under-cap'); + expect(row).not.toBeNull(); + expect(row?.used_count).toBe(1); + expect(row).toStrictEqual(rawInvite(created.id)); + }); + + it('INVREPO-006: at capacity — no row matches, returns null, used_count unchanged', async () => { + const created = createInviteToken(testDb, { token: 'at-cap', max_uses: 1 }); + testDb.prepare('UPDATE invite_tokens SET used_count = 1 WHERE id = ?').run(created.id); + const row = await invites.incrementUsedCount('at-cap'); + expect(row).toBeNull(); + expect((rawInvite(created.id) as { used_count: number }).used_count).toBe(1); + }); + + it('INVREPO-007: max_uses = 0 means unlimited — always increments', async () => { + createInviteToken(testDb, { token: 'unlimited', max_uses: 0 }); + testDb.prepare("UPDATE invite_tokens SET used_count = 500 WHERE token = 'unlimited'").run(); + const row = await invites.incrementUsedCount('unlimited'); + expect(row?.used_count).toBe(501); + }); + + it('INVREPO-008: an unknown token returns null', async () => { + expect(await invites.incrementUsedCount('nonexistent-token')).toBeNull(); + }); + + it('INVREPO-009: two concurrent increments on a one-use invite — exactly one wins (D6 single-connection race safety)', async () => { + const created = createInviteToken(testDb, { token: 'race-me', max_uses: 1 }); + const [a, b] = await Promise.all([ + invites.incrementUsedCount('race-me'), + invites.incrementUsedCount('race-me'), + ]); + const results = [a, b]; + const winners = results.filter((r) => r !== null); + const losers = results.filter((r) => r === null); + expect(winners).toHaveLength(1); + expect(losers).toHaveLength(1); + expect(winners[0]?.used_count).toBe(1); + expect((rawInvite(created.id) as { used_count: number }).used_count).toBe(1); + }); + }); +}); diff --git a/server/tests/unit/nest/platform.controller.test.ts b/server/tests/unit/nest/platform.controller.test.ts index b48c44b9a..ab1b79465 100644 --- a/server/tests/unit/nest/platform.controller.test.ts +++ b/server/tests/unit/nest/platform.controller.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; import { NotFoundException } from '@nestjs/common'; // --- hoisted mock fns so the vi.mock factories can reference them ----------------- @@ -21,6 +21,8 @@ import { 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'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; // 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 @@ -75,6 +77,23 @@ function makeRes() { return res; } +// Task 0 (D6): applyPlatformUploads now wraps servePhoto in withRequestContext, +// so every call site below needs a real `{ em }` to hand it — none of these +// cases touch the ORM (jwt-verify and db/database are both mocked above), but +// RequestContext.create needs a genuine EntityManager to open the ALS scope +// around, not a hand-built stub. +const uploadsTestDb = createSnapshotTestDb(); +let uploadsOrm: TestOrm; + +beforeAll(async () => { + uploadsOrm = await createTestOrm(uploadsTestDb); +}); + +afterAll(async () => { + await uploadsOrm.close(); + uploadsTestDb.close(); +}); + beforeEach(() => { vi.clearAllMocks(); }); @@ -82,7 +101,7 @@ beforeEach(() => { describe('applyPlatformUploads', () => { it('registers the four static mounts + the files block', () => { const { app, calls } = fakeApp(); - applyPlatformUploads(app, storage); + applyPlatformUploads(app, storage, uploadsOrm.orm); const paths = calls.filter((c) => c.method === 'use').map((c) => c.path); expect(paths).toEqual( expect.arrayContaining([ @@ -97,7 +116,7 @@ describe('applyPlatformUploads', () => { it('the /uploads/files block always answers 401', () => { const { app, calls } = fakeApp(); - applyPlatformUploads(app, storage); + applyPlatformUploads(app, storage, uploadsOrm.orm); const filesBlock = calls.find((c) => c.path === '/uploads/files')!.handlers[0]; const res = makeRes(); filesBlock({}, res); @@ -108,7 +127,7 @@ describe('applyPlatformUploads', () => { describe('GET /uploads/photos/:filename', () => { function photoHandler() { const { app, calls } = fakeApp(); - applyPlatformUploads(app, storage); + applyPlatformUploads(app, storage, uploadsOrm.orm); return calls.find((c) => c.method === 'get' && c.path === '/uploads/photos/:filename')!.handlers[0]; } const next = vi.fn(); diff --git a/server/tests/unit/nest/trek-ws-adapter.test.ts b/server/tests/unit/nest/trek-ws-adapter.test.ts index 8f9297ece..ac6242a54 100644 --- a/server/tests/unit/nest/trek-ws-adapter.test.ts +++ b/server/tests/unit/nest/trek-ws-adapter.test.ts @@ -260,6 +260,61 @@ describe('TrekWsAdapter connection binding', () => { }); }); +/** + * D6 request context on the CONNECTION dispatch (Plan 3b Task 0), the same + * property WSAD-040/041 above already pin for the MESSAGE dispatch. + * `bindClientConnect` fires Nest's OnGatewayConnection hook + * (RealtimeGateway.handleConnection) — no HTTP request behind it either, and + * a different lifecycle hook than bindMessageHandlers, so its own wrapper (or + * lack of one) had to be checked separately (plan3b-sql-inventory.md §5 + * flagged it unverified). Global context disallowed on purpose, same as + * WSAD-040/041 — allowGlobalContext: false is the production setting. + */ +describe('TrekWsAdapter D6 request context on connect (Plan 3b Task 0)', () => { + function connectServer(socket: ReturnType, request: unknown) { + return { on: (_event: string, cb: (s: unknown, r: unknown) => void) => cb(socket, request) }; + } + + it('WSAD-050: without MikroORM passed to the adapter, the connection dispatch THROWS rather than running handleConnection unwrapped', () => { + const ad = new TrekWsAdapter({} as HttpServer); // no orm + const socket = fakeSocket(); + let handlerRan = false; + expect(() => + ad.bindClientConnect(connectServer(socket, { url: '/ws?token=abc' }) as never, () => { + handlerRan = true; + }), + ).toThrow(/no MikroORM available/i); + // Same fail-closed shape as WSAD-040: the wrapper throws BEFORE the + // connection callback ever runs. + expect(handlerRan).toBe(false); + }); + + it('WSAD-051: with MikroORM passed to the adapter, a repository read inside the connection callback succeeds — the wrapper is load-bearing', async () => { + const t = await createTestOrm(testDb, { allowGlobalContext: false }); + try { + let result: unknown; + let caught: unknown; + const ad = new TrekWsAdapter({} as HttpServer, t.orm); + const socket = fakeSocket(); + let captured: Promise | undefined; + ad.bindClientConnect(connectServer(socket, { url: '/ws?token=abc' }) as never, () => { + captured = (async () => { + try { + result = await t.orm.em.find(Users, {}); + } catch (e) { + caught = e; + } + })(); + }); + await captured; + expect(caught).toBeUndefined(); + expect(Array.isArray(result)).toBe(true); + } finally { + await t.close(); + } + }); +}); + /** * The pointer exemption (#1973). *